[ipam/multi-pool] Fix races in the manager#44183
Merged
pippolo84 merged 2 commits intocilium:mainfrom Feb 4, 2026
Merged
Conversation
Trying to set a custom trigger to reconcile the CiliumNode in the
multi-pool manager unit tests is racy (see the error returned by the
race detector when running the tests below). Moreover, the trigger is
not even used anymore after the latest changes in the tests. To avoid
races and tests flakyness, let's simply set a very short (1 ns) debounce
interval for CiliumNode reconciliation.
```
=== RUN Test_MultiPoolManager_ReleaseUnusedCIDR
==================
WARNING: DATA RACE
Read at 0x00c0005ef8a8 by goroutine 53:
github.com/cilium/cilium/pkg/ipam.(*multiPoolManager).ciliumNodeUpdated()
github.com/cilium/cilium/pkg/ipam/multipool_manager.go:403 +0x20c
github.com/cilium/cilium/pkg/ipam.newMultiPoolManager.func3()
github.com/cilium/cilium/pkg/ipam/multipool_manager.go:305 +0x264
github.com/cilium/hive/job.(*jobOneShot).start()
github.com/cilium/hive@v0.0.0-20260108104938-97756f6ff54c/job/oneshot.go:138 +0x808
github.com/cilium/hive/job.(*group).add.func1.gowrap1()
github.com/cilium/hive@v0.0.0-20260108104938-97756f6ff54c/job/job.go:263 +0x131
Previous write at 0x00c0005ef8a8 by goroutine 48:
github.com/cilium/cilium/pkg/ipam.Test_MultiPoolManager_ReleaseUnusedCIDR()
github.com/cilium/cilium/pkg/ipam/multipool_test.go:541 +0xe58
testing.tRunner()
testing/testing.go:1934 +0x21c
testing.(*T).Run.gowrap1()
testing/testing.go:1997 +0x44
```
Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
The CiliumNode pointer in the manager is accessed concurrently by
multiple goroutines:
- it is updated when handling an upsert event from the LocalNodeResource
- it is read to get the IPAM pools when an update of the multi-pool
status is triggered by "multi-pool-cilium-node-updater" timer job
This can be seen below in the error returned by the race detector when
running the multi-pool manager unit test.
To fix this, add another mutex to specifically protect the access to
node in the manager, alongside two unexported methods to read and update
it.
```
==================
WARNING: DATA RACE
Write at 0x00c000848990 by goroutine 66:
github.com/cilium/cilium/pkg/ipam.(*multiPoolManager).ciliumNodeUpdated()
github.com/cilium/cilium/pkg/ipam/multipool_manager.go:406 +0x22f
github.com/cilium/cilium/pkg/ipam.newMultiPoolManager.func3()
github.com/cilium/cilium/pkg/ipam/multipool_manager.go:305 +0x264
github.com/cilium/hive/job.(*jobOneShot).start()
github.com/cilium/hive@v0.0.0-20260108104938-97756f6ff54c/job/oneshot.go:138 +0x808
github.com/cilium/hive/job.(*group).add.func1.gowrap1()
github.com/cilium/hive@v0.0.0-20260108104938-97756f6ff54c/job/job.go:263 +0x131
Previous read at 0x00c000848990 by goroutine 67:
github.com/cilium/cilium/pkg/ipam.(*multiPoolManager).updateLocalNode()
github.com/cilium/cilium/pkg/ipam/multipool_manager.go:587 +0xd64
github.com/cilium/cilium/pkg/ipam.(*multiPoolManager).updateLocalNode-fm()
<autogenerated>:1 +0x47
github.com/cilium/hive/job.(*jobTimer).start()
github.com/cilium/hive@v0.0.0-20260108104938-97756f6ff54c/job/timer.go:191 +0x708
github.com/cilium/hive/job.(*group).add.func1.gowrap1()
github.com/cilium/hive@v0.0.0-20260108104938-97756f6ff54c/job/job.go:263 +0x131
```
Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
Member
Author
|
/test |
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
3 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.
Fix some races in the manager and in the related unit tests. Please refer to individual commit for more details.