dynamicconfig: detect and remove config options on delete #39303
Conversation
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
6725a9b to
e136d30
Compare
ConfigMap key deletions do not have entry.deleted set to true, and so are leaked on update. Add an option to capture the diff in rows between the existing table and updated rows, so that keys that need to be deleted can be calculated. Signed-off-by: jshr-w <shjayaraman@microsoft.com>
Signed-off-by: jshr-w <shjayaraman@microsoft.com>
e136d30 to
01ea19d
Compare
|
/test |
joamaki
requested changes
May 5, 2025
There was a problem hiding this comment.
This seems like a difficult to understand and use API. The problem is that TransformMany assumed a persistent 1-to-many mapping. I would instead suggest that we make TransformMany return a set of objects to upsert and objects to delete.
E.g. something like:
type TranformManyFunc[Obj any] func(txn statedb.ReadTxn, deleted bool, obj any) (toUpsert, toDelete []Obj)
If it is OK to you I would like to fix this myself.
joamaki
added a commit
to joamaki/cilium
that referenced
this pull request
May 5, 2025
Add a script test suite with test cases to verify that the previous commit fixes the issue of removed keys actually being deleted from the table. Issue: cilium#39303 Signed-off-by: Jussi Maki <jussi@isovalent.com>
joamaki
added a commit
to joamaki/cilium
that referenced
this pull request
May 8, 2025
Add a script test suite with test cases to verify that the previous commit fixes the issue of removed keys actually being deleted from the table. Issue: cilium#39303 Signed-off-by: Jussi Maki <jussi@isovalent.com>
joamaki
added a commit
to joamaki/cilium
that referenced
this pull request
May 8, 2025
Add a script test suite with test cases to verify that the previous commit fixes the issue of removed keys actually being deleted from the table. Issue: cilium#39303 Signed-off-by: Jussi Maki <jussi@isovalent.com>
joamaki
added a commit
to joamaki/cilium
that referenced
this pull request
May 9, 2025
Add a script test suite with test cases to verify that the previous commit fixes the issue of removed keys actually being deleted from the table. Issue: cilium#39303 Signed-off-by: Jussi Maki <jussi@isovalent.com>
joamaki
added a commit
to joamaki/cilium
that referenced
this pull request
May 12, 2025
Add a script test suite with test cases to verify that the previous commit fixes the issue of removed keys actually being deleted from the table. Issue: cilium#39303 Signed-off-by: Jussi Maki <jussi@isovalent.com>
github-merge-queue bot
pushed a commit
that referenced
this pull request
May 12, 2025
Add a script test suite with test cases to verify that the previous commit fixes the issue of removed keys actually being deleted from the table. Issue: #39303 Signed-off-by: Jussi Maki <jussi@isovalent.com>
hsalluri259
pushed a commit
to hsalluri259/cilium
that referenced
this pull request
May 14, 2025
Add a script test suite with test cases to verify that the previous commit fixes the issue of removed keys actually being deleted from the table. Issue: cilium#39303 Signed-off-by: Jussi Maki <jussi@isovalent.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 change enables the calculation of the diff of an existing statedb table and the values being inserted, so that the dynamicconfig reflector can detect removed options and delete them from the statedb table.
Fixes: #38621