Repository navigation
[shell-operator] fix: wait for kube events to finish processing - #939
Merged
Merged
Conversation
Signed-off-by: Ruslan Gorbunov <[email protected]>
Keep synchronization helpers out of public interfaces while retaining fake-cluster test support Signed-off-by: Ruslan Gorbunov <[email protected]>
fuldaxxx
marked this pull request as ready for review
September 30, 2026 08:02
ldmonster
approved these changes
Oct 21, 2026
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
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.
Description
BindingContextController.ChangeState(what deckhouse'sKubeStateSetcalls) no longer stops collecting events on a fixed 100ms timer (STOP_EVENTS). Every 10ms it checks whether informers have handled every change in the fake cluster. It returns once three conditions hold:Listof the cluster by checksum;The 20s timeout stays as a backstop.
Added
Synced(ctx) booltoKubeEventsManagerandMonitor. It lists objects on every call and is meant for the fake cluster in tests; the operator never calls it at runtime. The only change on the production event path is an atomic in-flight counter inhandleWatchEvent.NewStateControllerno longer takes the unusedKubeEventsManagerargument. deckhouse does not call it.Two regression tests:
Test_ChangeState_SlowEvents: a filter that sleeps 50ms per event, standing in for a loaded runner. It fails on the old harness.Test_ChangeState_FilterError:ChangeStatemust not wait for objects the filter fails on. It hangs until the 20s timeout if the check does not mirrorhandleWatchEvent.Why do we need it, and what problem does it solve?
Symptom. deckhouse hook unit tests flake on CI in hooks and tests that nobody changed. For example:
user-authn/hooks/get_dex_client_crds_test.go: the credentials Secret is missing from the snapshot, so the hook generates a random password instead of"test". After a deletion,BindingContextscomes back empty.000-common/hooks/internal_tls_test.go: the existing TLS Secret is missing from the snapshot, so a new certificate is generated. This one probably has the same cause but was not checked separately.Root cause.
ChangeStatewrites objects to the fake cluster. Events reach the harness asynchronously: fake watch → reflector → DeltaFIFO → listener goroutine →handleWatchEvent(filter, checksum, cache) → event channel. The harness sentSTOP_EVENTS100ms after the writes and dropped anything that arrived later. Late events stayed in the channel and leaked into the nextChangeState. Withgo test -raceon a loaded runner, 100ms is not always enough.Reproduced: with a filter that takes about 50ms per event, the old
ChangeStatereturns 0 of 3 binding contexts on every run. CPU load alone on a 12-core laptop did not reproduce it (30/30 runs passed).