Skip to content

operator/multi-pool: Streamline implementation and improve reusability#43937

Merged
pippolo84 merged 9 commits intocilium:mainfrom
pippolo84:pr/pippolo84/operator-multipool-impro
Jan 27, 2026
Merged

operator/multi-pool: Streamline implementation and improve reusability#43937
pippolo84 merged 9 commits intocilium:mainfrom
pippolo84:pr/pippolo84/operator-multipool-impro

Conversation

@pippolo84
Copy link
Copy Markdown
Member

@pippolo84 pippolo84 commented Jan 22, 2026

In preparation for the upcoming Multi Pool Resource IPAM for the Cilium Network Driver (see the CfP for more information) this PR aims to:

  • remove no more useful abstractions around node handler and allocator that were introduced before operator: Modularize IPAM allocators #43628 to make the multi pool IPAM compatible with the operator expectations for each IPAM implementation
  • separate the parts of the code specific for the existing multi pool Pod IPAM and move them into the multi pool IPAM cell. Doing so allows to reuse all the internal implementation for multi pool in the context of DRA resources

The PR does not intend to change anything regarding the current behavior of multi pool IPAM.

Note to reviewers: please review each commit individually

Depends on #43693 and #43726

@pippolo84 pippolo84 added the dont-merge/preview-only Only for preview or testing, don't merge it. label Jan 22, 2026
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 22, 2026
@pippolo84
Copy link
Copy Markdown
Member Author

/test

@pippolo84 pippolo84 force-pushed the pr/pippolo84/operator-multipool-impro branch 3 times, most recently from f37c2de to f5fc329 Compare January 23, 2026 10:54
@pippolo84 pippolo84 added area/operator Impacts the cilium-operator component area/multipool Affects Multi-Pool IPAM release-note/misc This PR makes changes that have no direct user impact. labels Jan 23, 2026
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 23, 2026
@pippolo84 pippolo84 changed the title operator: multi-pool IPAM improvements operator/multi pool IPAM: Streamline implementation and improve reusability Jan 23, 2026
@pippolo84 pippolo84 changed the title operator/multi pool IPAM: Streamline implementation and improve reusability operator/multi-pool: Streamline implementation and improve reusability Jan 23, 2026
@pippolo84 pippolo84 force-pushed the pr/pippolo84/operator-multipool-impro branch from f5fc329 to 9f3f7d5 Compare January 23, 2026 14:20
@pippolo84 pippolo84 marked this pull request as ready for review January 23, 2026 14:21
@pippolo84 pippolo84 requested review from a team as code owners January 23, 2026 14:21
@pippolo84 pippolo84 removed the dont-merge/preview-only Only for preview or testing, don't merge it. label Jan 23, 2026
Copy link
Copy Markdown
Contributor

@rastislavs rastislavs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK for BGP changes

Copy link
Copy Markdown
Member

@HadrienPatte HadrienPatte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great refactor 👍

@pippolo84 pippolo84 enabled auto-merge January 27, 2026 10:28
@pippolo84
Copy link
Copy Markdown
Member Author

/test

The operator relies on a different module for each IPAM mode it
supports. This means that there is no need for a common IPAM
AllocatorProvider interface anymore, since each implementation is
completely independent from the others. Therefore, remove the stale
unused interface.

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
Multi Pool IPAM mode requires both an operator component as well as an
agent one. Currently, both components, despite being completely
separate, are kept under pkg/ipam/allocator/multipool.  Following the
structure of other packages, move all the operator related logic under
operator/pkg/ipam. This allows to recognize at a glance if a certain
portion of the code is part of the agent or part of the operator.

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
The PooledAllocatorProvider interface was meant to abstract the use of
concrete pooled allocators (like multipool.Allocator) into the IP Pools
watcher. Since the multipool allocator is the only concrete
implementation, in order to reduce the complexity, remove the interface
and use the type directly.

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
In order to generalize the multi pool IPAM implementation and make it
reusable for the upcoming multi pool Resource IPAM for the Network
Driver, let's remove the direct reference to the CiliumNode resource in
both the PoolAllocator and the NodeHandler.  Both flavors of multi pool
IPAM reads and writes to the CiliumNode, but using different fields:

- the existing Pod IPAM relies on Spec.IPAM.Pools
- the upcoming Resource IPAM relies on Spec.IPAM.ResourcePools

but since the type of those fields is the same, it is possible to reuse
the same multi pool implementation simply passing the proper pools
reference from each module.

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
The ciliumNodeUpdateImplementation struct was meant to simplify the
interactions with the CiliumNode resource in each different IPAM
implementation (multi-pool, cluster-pool, ENI and so on).  Since all the
IPAM operator subsystem is now implemented as a hive cell and each IPAM
flavor has an independent implementation with a reference to the
dependecy-injected ClientSet, the abstraction is not useful anymore.

In order to simplify the multi-pool implementation, let's use the
ClientSet directly.  Moreover, this removes the redundant double check
for equality of the old and new version of the resource done with
DeepEqual when calling Update and UpdateStatus.

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
Since those CIDRs will be used in the context of multi pool Resource
IPAM too, where we allocate addresses to DRA resources, let's avoid
mentioning "Pod" in the type name.

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
Just like in the previous commit, those CIDRSlices will be used in
contexts where the allocated CIDRs will be used not by pods but by DRA
resources, therefore remove "Pod" from the type name.

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
Just like the multi pool IPAM implementation, the auto creation of
CiliumPodIPPool feature can be split into two parts:

- a reusable set of functions to auto create pools based on a generic
  description
- a set of functions that finalize the creation of the pools in a
  specific kind of k8s object

While the first part could be reused for auto creation of multi pool
Resource IPAM pools too (that is, for CiliumResourceIPPool objects), the
second one implements the creation of pools specific for multi pool Pod
IPAM, namely CiliumPodIPPool objects.

The commit move the pod specific part into the multi pool cell for Pods
IPAM.

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
This allows to reuse the code to handle pool events in multipool package
in the upcoming multi pool Resource IPAM too.

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
@pippolo84 pippolo84 force-pushed the pr/pippolo84/operator-multipool-impro branch from 9f3f7d5 to cdda6e1 Compare January 27, 2026 15:10
@pippolo84
Copy link
Copy Markdown
Member Author

/test

@pippolo84 pippolo84 added this pull request to the merge queue Jan 27, 2026
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 27, 2026
Merged via the queue into cilium:main with commit 271e2b5 Jan 27, 2026
76 checks passed
@pippolo84 pippolo84 deleted the pr/pippolo84/operator-multipool-impro branch January 27, 2026 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/multipool Affects Multi-Pool IPAM area/operator Impacts the cilium-operator component ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants