-
Notifications
You must be signed in to change notification settings - Fork 66
Support dual-stack NICs in the public API #9508
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
Conversation
cd0803a to
b276b93
Compare
|
I'm working on the NIC-update APIs, but need to get #9542 to make that work. That'll let us update a NIC's transit IPs on a per-stack basis, rather than some RMW with the DB. |
b276b93 to
bc3f12c
Compare
2df5da9 to
a54fdb0
Compare
- Move IP traits and config types to common modules so they can be used in the public API - Add private IP stack types to external API, used when fetching NICs - Add new `IpConfig` type to the parameters for creating NICs. This gives a more flexible scheme for selecting an IP address automatically or explicitly, and independently for each IP stack. - Add a new version of the external API for the new types, and implement conversions from the previous - Ensure external IPs are allocated for NICs with a corresponding private IP stack, checking at both IP creation and attachment. - Check that candidate secondary NICs have an IP stack for every external IP when setting them to a new primary
a54fdb0 to
342aee5
Compare
|
There are a few tests that will fail until I rebase on @zeeshanlakhani's changes in #9452 once that merges. This is ready for a review now though. |
rcgoodfellow
left a comment
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.
Thanks for pulling all this together @bnaecker! Lots of details to line up and some hairy database work.
I've been rebasing my ipv6/all-the-things onto this branch and things are working e2e for IPv6 comms with probes.
A few minor comments/questions follow.
|
Alright, I've integrated this with the updated handling of default pools, and have now gotten all the tests to pass and my own TODOs addressed. @rcgoodfellow thanks for your feedback, I'd love another pass when you get a minute. |
|
Still need to resolve another conflict with main. |
rcgoodfellow
left a comment
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.
LGTM. All my comments have been addressed.
|
The multicast failure looks like a flake. There's a timeout we're exceeding, but it passes locally quite quickly. I'll fixup the API check test now. |
IpConfigtype to the parameters for creating NICs. This gives a more flexible scheme for selecting an IP address automatically or explicitly, and independently for each IP stack.