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
✨ Add support to IPv6 in Machine's status #1633
✨ Add support to IPv6 in Machine's status #1633
Conversation
✅ Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Welcome @MaysaMacedo! |
Hi @MaysaMacedo. Thanks for your PR. I'm waiting for a kubernetes-sigs 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. Instructions 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/test-infra repository. |
Here is how Machine's address field looks like once this change is included:
|
/cc @mdbooth |
@MaysaMacedo maybe this is really an edge case, but would it make sense for us to try and find a way to order IPv4 addreses first then IPv6 next? I'm just wondering in the edge case scenario that someone is relying on the first item of the It's totally a small thing though, I think it's probably fine as is, but thought i'd call it out. |
/ok-to-test |
f9c39df
to
e0f6590
Compare
86bb01b
to
e40ef02
Compare
Dual-stack clusters require that the Machine's status contains the IPv6 address. This commit removed the check that would skip the addition of IPv6 to the Machines. Also, it ensures the IPv4 addresses of each Network remains being the first ones listed in the Machines given there are operations that rely on the first address of each network.
e40ef02
to
5926f47
Compare
Here is how Machine's status looks like with the latest changes:
|
/lgtm |
/assign @justinsb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
Looks good, thanks!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: MaysaMacedo, mdbooth, mnaser 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 |
Dual-stack clusters require that the Machine's status contains the
IPv6 address. This commit removed the check that would skip the addition
of IPv6 to the Machines. Also, it ensures the IPv4 addresses of each
Network remains being the first ones listed in the Machines given there are
operations that rely on the first address of each network.