-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Include all APIserver addresses for nodeup config #16813
Conversation
Skipping CI for Draft Pull Request. |
From office hours: I will add back the filtering logic but always include any ipv6 addresses. It will still exclude the DNS names though. |
b1861b8
to
f63508a
Compare
f63508a
to
6261b0f
Compare
/test pull-kops-e2e-cni-cilium-ipv6 |
the cluster validated and e2e tests started, but the prow job pod was interrupted /test pull-kops-e2e-cni-cilium-ipv6 |
tests pass too 🎉 |
It looks like this is only needed for dns=none clusters. normal DNS clusters are already passing with my IMDS and controller-runtime changes: https://testgrid.k8s.io/kops-ipv6#kops-aws-cni-calico-ipv6-flatcar |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hakman The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Awesome work! |
With this plus #16812 i'm able to get an ipv6 dns=none cluster to pass validation.
Before we were only including ipv4 addresses because those are the only CIDRs included in the cluster spec - ipv6 CIDRs are provided by AWS. This resulted in nodes failing to bootstrap because they couldn't reach the kops-controller endpoint:
Sep 06 01:23:11 i-0250853b64092f19d nodeup[1323]: W0906 01:23:11.157536 1323 main.go:133] got error running nodeup (will retry in 30s): failed to get node config from server: Post "https://172.20.5.248:3988/bootstrap": dial tcp 172.20.5.248:3988: connect: network is unreachable
Even though the ipv6 address and load balancer DNS name both work:
With this change, the nodeup config userdata changes as such:
Marking this as draft because its possible we could find a better approach, and because this may break other cluster configurations