k8s: StateDB reflector's TransformMany to return objects to delete #39330
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
|
Marking with |
joamaki
commented
May 5, 2025
|
/test |
e779c2a to
21e49a2
Compare
21e49a2 to
9e950b5
Compare
ovidiutirla
approved these changes
May 8, 2025
|
/test |
9e950b5 to
8167498
Compare
|
/test |
Use the DecodeKubernetesObject as fallback in DecodeObject. This way we can decode core objects that don't have a slim counterpart without having to separately use DecodeKubernetestObject. Signed-off-by: Jussi Maki <jussi@isovalent.com>
TransformMany only worked for cases where an object had a static mapping from 1 to many objects that did not change over the lifetime of the object. This was not how it was used by pkg/dynamicconfig where the mapping changed depending on the contents of the object. To support this use-case change TransformMany to return a set of objects to insert and set of objects to delete. Fixes: cilium#38621 Fixes: 3ae0b14 ("k8s: Support multiple objects to be returned by reflector transform") Signed-off-by: Jussi Maki <jussi@isovalent.com>
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>
The test case uses multiple reflectors for ConfigMap which is problematic as the k8s fake client uses a single channel for events and thus with multiple watchers events get lost: $ go test -c $ stress -count 1000 ./dynamicconfig.test -test.run TestDynamicConfigMap /tmp/go-stress-20250508T120956-1349213616 --- FAIL: TestDynamicConfigMap (2.02s) --- FAIL: TestDynamicConfigMap/default (2.01s) logger.go:256: time=2025-05-08T12:09:57.194+02:00 level=INFO source=/home/jussi/go/src/github.com/cilium/cilium/vendor/github.com/cilium/hive/hive.go:357 msg="Starting hive" logger.go:256: time=2025-05-08T12:09:57.194+02:00 level=INFO source=/home/jussi/go/src/github.com/cilium/cilium/vendor/github.com/cilium/hive/hive.go:361 msg="Started hive" duration=59.227µs logger.go:256: time=2025-05-08T12:09:57.198+02:00 level=WARN source=/home/jussi/go/src/github.com/cilium/cilium/pkg/k8s/client/fake.go:501 msg="Multiple watches for resource intercepted. This highlights a potential cause for flakes" module=k8s-fake-client resource=v1.configmaps table_test.go:189: waiting for confing table: timeout reached while waiting for condition Fix the test by not starting the Hive until after the ConfigMap objects have been created. This way no Watch events are created and we get all the objects with the initial List(). $ ,stress -count 10000 ./dynamicconfig.test -test.run TestDynamicConfigMap 33s: 10000 runs total, 0 failures Signed-off-by: Jussi Maki <jussi@isovalent.com>
8167498 to
44d86cd
Compare
|
/test |
This was referenced May 13, 2025
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 fixes the issue #38621 by changing the
TransformManyin the StateDB reflector to also return the objects that should be deleted.Additionally adding a script test suite to pkg/dynamicconfig to verify that the issue has been solved and lsolving a flaky test that I encountered.