CFP-18405: ENI IPAM Multi-Pool Migration#87
Open
HadrienPatte wants to merge 2 commits intocilium:mainfrom
Open
CFP-18405: ENI IPAM Multi-Pool Migration#87HadrienPatte wants to merge 2 commits intocilium:mainfrom
HadrienPatte wants to merge 2 commits intocilium:mainfrom
Conversation
Signed-off-by: Hadrien Patte <[email protected]>
gandro
approved these changes
Mar 9, 2026
Member
gandro
left a comment
gandro
left a comment
There was a problem hiding this comment.
I am so happy to see this being tackled! 🥹
The plan here sounds very reasonable and implementable to me. I have left some comments on the key questions, but the major concerns of such a change are already addressed in the document from my perspective, so I am approving of this.
| * `Spec.IPAM.Pool` remains populated on all nodes even when all agents have upgraded, until 1.21. | ||
| * Slightly more data written per CiliumNode object. | ||
|
|
||
| #### Option 2: Temporary Opt-Out Flag to Disable Dual-Write |
Member
There was a problem hiding this comment.
I'm mildly in favor of providing this.
We could even go a step further and and enable this by default for new installations. While it is a potentially dangerous flag to enable, I do feel like with proper documentation this risk is mitigated.
Member
Author
There was a problem hiding this comment.
Do we have an established way of doing "if new installation" type of logic today? I feel like a lot of the great options on this page currently default to being disabled (even on new installations) because they're not safe to be enabled on existing clusters.
Member
There was a problem hiding this comment.
Yes, we do, the upgradeCompatibility flag. Though it is possible that many users are not using it when upgrading, which would of course be pretty bad for them. If we want to be safe, maybe your original proposal to make this strictly opt-in is the way to go.
Update based on feedback Signed-off-by: Hadrien Patte <[email protected]>
Member
Author
Thanks, I have updated the CFP based on your comments and changed its status from |
HadrienPatte
added a commit
to cilium/cilium
that referenced
this pull request
Mar 27, 2026
Extend `NewCIDRRange` with a `WithAllowFirstLastIPs` functional option that includes the first and last IPs of a CIDR in the allocatable range. This is needed for AWS ENI prefix delegation where /28 prefixes are exclusively assigned to a node and all 16 IPs are usable as there is no shared network segment requiring base/broadcast reservation. Also refactor `ForEach` to use `r.base` directly instead of assuming a +1 offset from the CIDR base, making it correct for both default and `AllowFirstLastIPs` ranges. Relates to [cilium/design-cfps#87](cilium/design-cfps#87) Followup to #34618 See #28637 (comment) Signed-off-by: Hadrien Patte <[email protected]>
HadrienPatte
added a commit
to cilium/cilium
that referenced
this pull request
Mar 27, 2026
Extract ENI device configuration into a standalone CiliumNode observer that runs independently of the IPAM allocator. Previously, `configureENIDevices` was called from `nodeStore.updateLocalNodeResource`, coupling ENI network device setup to the CRD allocator code path. The new observer follows the same `job.Observer` pattern used by `startLocalNodeAllocCIDRsSync` in the multi-pool allocator. It watches CiliumNode updates and configures newly attached ENI devices regardless of which allocator is active. Relates to [cilium/design-cfps#87](cilium/design-cfps#87) Signed-off-by: Hadrien Patte <[email protected]>
HadrienPatte
added a commit
to cilium/cilium
that referenced
this pull request
Mar 27, 2026
Extract ENI device configuration into a standalone CiliumNode observer that runs independently of the IPAM allocator. Previously, `configureENIDevices` was called from `nodeStore.updateLocalNodeResource`, coupling ENI network device setup to the CRD allocator code path. The new observer follows the same `job.Observer` pattern used by `startLocalNodeAllocCIDRsSync` in the multi-pool allocator. It watches CiliumNode updates and configures newly attached ENI devices regardless of which allocator is active. Relates to [cilium/design-cfps#87](cilium/design-cfps#87) Signed-off-by: Hadrien Patte <[email protected]>
HadrienPatte
added a commit
to cilium/cilium
that referenced
this pull request
Mar 28, 2026
Extract ENI device configuration into a standalone CiliumNode observer that runs independently of the IPAM allocator. Previously, `configureENIDevices` was called from `nodeStore.updateLocalNodeResource`, coupling ENI network device setup to the CRD allocator code path. The new observer follows the same `job.Observer` pattern used by `startLocalNodeAllocCIDRsSync` in the multi-pool allocator. It watches CiliumNode updates and configures newly attached ENI devices regardless of which allocator is active. Relates to [cilium/design-cfps#87](cilium/design-cfps#87) Signed-off-by: Hadrien Patte <[email protected]>
HadrienPatte
added a commit
to cilium/cilium
that referenced
this pull request
Mar 30, 2026
Extend `NewCIDRRange` with a `WithAllowFirstLastIPs` functional option that includes the first and last IPs of a CIDR in the allocatable range. This is needed for AWS ENI prefix delegation where /28 prefixes are exclusively assigned to a node and all 16 IPs are usable as there is no shared network segment requiring base/broadcast reservation. Also refactor `ForEach` to use `r.base` directly instead of assuming a +1 offset from the CIDR base, making it correct for both default and `AllowFirstLastIPs` ranges. Relates to [cilium/design-cfps#87](cilium/design-cfps#87) Followup to #34618 See #28637 (comment) Signed-off-by: Hadrien Patte <[email protected]>
HadrienPatte
added a commit
to cilium/cilium
that referenced
this pull request
Mar 30, 2026
Extend `NewCIDRRange` with a `WithAllowFirstLastIPs` functional option that includes the first and last IPs of a CIDR in the allocatable range. This is needed for AWS ENI prefix delegation where /28 prefixes are exclusively assigned to a node and all 16 IPs are usable as there is no shared network segment requiring base/broadcast reservation. Also refactor `ForEach` to use `r.base` directly instead of assuming a +1 offset from the CIDR base, making it correct for both default and `AllowFirstLastIPs` ranges. Relates to [cilium/design-cfps#87](cilium/design-cfps#87) Followup to #34618 See #28637 (comment) Signed-off-by: Hadrien Patte <[email protected]>
HadrienPatte
added a commit
to cilium/cilium
that referenced
this pull request
Mar 30, 2026
Extract ENI device configuration into a standalone CiliumNode observer that runs independently of the IPAM allocator. Previously, `configureENIDevices` was called from `nodeStore.updateLocalNodeResource`, coupling ENI network device setup to the CRD allocator code path. The new observer follows the same `job.Observer` pattern used by `startLocalNodeAllocCIDRsSync` in the multi-pool allocator. It watches CiliumNode updates and configures newly attached ENI devices regardless of which allocator is active. Relates to [cilium/design-cfps#87](cilium/design-cfps#87) Signed-off-by: Hadrien Patte <[email protected]>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
This CFP is a more detailed version of the design I mentioned in cilium/cilium#19251 (comment)