bootstrapper ipv6: plumb ip-family through kubeadm, kubelet and control-plane alias #22064
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Contributor
| [APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: kartikjoshi21 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
k8s-ci-robot added size/XXL Contributor
| Hi @kartikjoshi21. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot added the cncf-cla: yes 
Collaborator
| Can one of the admins verify this patch? |
Contributor Author
| Logs: |
kartikjoshi21 force-pushed the pr3-kubeadm-ipfamily branch from a1f0157 to bf634c4 Compare Contributor Author
|
illume reviewed Dec 18, 2025
illume left a comment
illume left a comment There was a problem hiding this comment.
Nice one 🎉
I added a few questions/notes.
| ExtraArgs []string // a list of any extra option to pass to oci binary during creation time, for example --expose 8080... | ||
| ListenAddress string // IP Address to listen to | ||
| GPUs string // add GPU devices to the container | ||
| Subnetv6 string |
There was a problem hiding this comment.
Add missing docs, and align them with the other comments.
| klog.Infof("%s network %s %s created", ociBin, networkName, p.CIDR) | ||
| return gw, nil | ||
| } | ||
| // don't retry if error is not address is taken |
There was a problem hiding this comment.
There is a second "kic: plumb ip-family and ipv6 for docker bridge" commit that could be merged with the first one.
| nodeRegistration: | ||
| criSocket: {{if .CRISocket}}{{.CRISocket}}{{else}}/var/run/dockershim.sock{{end}} | ||
| name: "{{.NodeName}}" | ||
| {{- if .NodeIP }} |
There was a problem hiding this comment.
I'm wondering about rendering these things like NodeIP with ipv6 stuff in them, and : which breaks yaml? For example [::]:10249 ?
I see you have quotes elsewhere, but maybe a few spots are missing?
| return fmt.Sprintf(`^[[:space:]]*[:0-9A-Fa-f]+[[:space:]]+%s$`, qName) | ||
| } | ||
| | ||
| func addHostAliasCommand(dropRegex, record string, sudo bool, destPath string) *exec.Cmd { |
| script := fmt.Sprintf( | ||
| `{ grep -v $'\t%s$' "%s"; echo "%s"; } > /tmp/h.$$; %s cp /tmp/h.$$ "%s"`, | ||
| name, | ||
| `{ grep -v -F "%s" "%s"; echo "%s"; } > /tmp/h.$$; %s cp /tmp/h.$$ "%s"`, |
There was a problem hiding this comment.
I see you have dropRegex param... but maybe forgot to use it here?
| case "ipv6": | ||
| return "[::]:10249" | ||
| default: // ipv4 or dual | ||
| return "0.0.0.0:10249" |
There was a problem hiding this comment.
In dual stack does this mean it's exposed only in ipv4? I would expect in the case "dual": to return "[::]:10249" so it binds to both?
Or maybe a comment somewhere to say that dual only listens on ipv4 if that is the intention.
| } | ||
| | ||
| return hostname, ips[0], port, nil | ||
| return host, ips[0], port, nil |
There was a problem hiding this comment.
Do you want to always take the first ip resolved? For ipv6/dual you probably want to prefer an AAAA if present?
What I mean is, something like this code (untested):
// pickResolvedIP deterministically prefers AAAA for ipv6/dual, A for ipv4. func pickResolvedIP(ips []net.IP, ipFamily string) net.IP { preferV6 := strings.EqualFold(ipFamily, "ipv6") || strings.EqualFold(ipFamily, "dual") var v6, v4 []net.IP for _, ip := range ips { if ip.To4() == nil { v6 = append(v6, ip) } else { v4 = append(v4, ip) } } if preferV6 { if len(v6) > 0 { return v6[0] } if len(v4) > 0 { return v4[0] } } else { if len(v4) > 0 { return v4[0] } if len(v6) > 0 { return v6[0] } } return nil }Or maybe just take what the resolver gives as you first (as you do now) is better?
| return nil | ||
| } | ||
| | ||
| func advertiseIP(cc config.ClusterConfig, n config.Node) string { |
| certificatesDir: {{.CertDir}} | ||
| clusterName: mk | ||
| controlPlaneEndpoint: {{.ControlPlaneAddress}}:{{.APIServerPort}} | ||
| controlPlaneEndpoint: "{{.ControlPlaneEndpoint}}" |
There was a problem hiding this comment.
Use {{.ControlPlaneEndpoint}} in v1beta2/3 templates as well (instead of concatenating address+port) so IPv6 bracketing/alias is uniform across all kubeadm configs?
| {{end -}}{{end -}} | ||
| certificatesDir: {{.CertDir}} | ||
| clusterName: mk | ||
| controlPlaneEndpoint: {{.ControlPlaneAddress}}:{{.APIServerPort}} |
There was a problem hiding this comment.
Use {{.ControlPlaneEndpoint}} like v1beta4 templates as well (instead of concatenating address+port) so IPv6 bracketing/alias is uniform across all kubeadm configs?
Contributor
| PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot added the needs-rebase Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Make kubeadm configs IP-family aware:
for dual-stack so kubelet can advertise both families
bind address to 0.0.0.0:10249 (ipv4/dual) or [::]:10249 (ipv6-only)
Update kubelet/certs/bootstrapper:
(IPv4-only, IPv6-only, or both in dual-stack)
Make endpoints & kubeconfig IPv6 safe:
kubeconfig URLs (https://[::1]:8443)
Ensure core Services follow the chosen family:
desired ipFamilyPolicy + ipFamilies (SingleStack IPv4/IPv6, or
PreferDualStack [IPv4, IPv6])
Extend ServiceClusterIP() to handle comma-separated Service CIDRs and
prefer IPv4 when present, so dual-stack serviceSubnet values do not
break certificate generation.
Fixes: #8535
Refer to this for testing steps: #22064 (comment)
Tested and verified on: