Skip to content

Fix GC of possible duplicated identities in kvstore mode#43287

Merged
giorio94 merged 2 commits intocilium:mainfrom
giorio94:mio/kvstore-allocator-gc
Dec 12, 2025
Merged

Fix GC of possible duplicated identities in kvstore mode#43287
giorio94 merged 2 commits intocilium:mainfrom
giorio94:mio/kvstore-allocator-gc

Conversation

@giorio94
Copy link
Copy Markdown
Member

Currently, the [RunGC] logic does not delete duplicated identities (i.e., pointing to the same labels), as long as at least one of them is actually used. Although duplicated identities should never occur in kvstore mode thanks to locking, this has been observed happening in the wild, and it is better to be robust in this case as well. Hence, let's fine tune the GC logic to consider a given identity as used only if it has any associated secondary key that also matches the actual ID.

In preparation for addressing a bug affecting this logic, and validating
the fix, let's scaffold a basic test to cover the [RunGC] function.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Currently, the [RunGC] logic does not delete duplicated identities
(i.e., pointing to the same labels), as long as at least one of them
is actually used. Although duplicated identities should never occur
in kvstore mode thanks to locking, this has been observed happening in
the wild, and it is better to be robust in this case as well. Hence,
let's fine tune the GC logic to consider a given identity as used only
if it has any associated secondary key that also matches the actual ID.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
@giorio94 giorio94 added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Dec 12, 2025
@giorio94 giorio94 requested a review from a team as a code owner December 12, 2025 10:24
@giorio94 giorio94 added area/kvstore Impacts the KVStore package interactions. needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch labels Dec 12, 2025
@giorio94 giorio94 requested a review from marseel December 12, 2025 10:24
@giorio94
Copy link
Copy Markdown
Member Author

/test

Copy link
Copy Markdown
Member

@marseel marseel left a comment

Choose a reason for hiding this comment

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

Amazing testing, looks good!

@giorio94 giorio94 enabled auto-merge December 12, 2025 14:02
@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 Dec 12, 2025
@giorio94 giorio94 added this pull request to the merge queue Dec 12, 2025
Merged via the queue into cilium:main with commit c331d94 Dec 12, 2025
81 checks passed
@giorio94 giorio94 deleted the mio/kvstore-allocator-gc branch December 12, 2025 14:27
@Artyop Artyop mentioned this pull request Dec 18, 2025
4 tasks
@Artyop Artyop added backport-pending/1.18 The backport for Cilium 1.18.x for this PR is in progress. and removed needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch labels Dec 18, 2025
@github-actions github-actions Bot added backport-done/1.18 The backport for Cilium 1.18.x for this PR is done. and removed backport-pending/1.18 The backport for Cilium 1.18.x for this PR is in progress. labels Dec 19, 2025
@cilium-release-bot cilium-release-bot Bot moved this to Released in cilium v1.19.0 Feb 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/kvstore Impacts the KVStore package interactions. backport-done/1.18 The backport for Cilium 1.18.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.

Projects

No open projects
Status: Released

Development

Successfully merging this pull request may close these issues.

4 participants