Fix readiness start-ordering race; correct stranded-CRD semantics - #44
Merged
Merged
Conversation
…docs The cache-sync readyz check called WaitForCacheSync before mgr.Start, trivially passing against an empty informer set — a false-Ready window that let a stranded-CRD upgrade complete its rollout and kill the healthy old pod before failing. Readiness now derives from a manager Runnable (executes only after caches genuinely sync): the stranded upgrade stalls with the previous pod still serving. Docs corrected to the verified stalled-rollout behavior. Found by the v1.3.0 rc boundary test on GKE.
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
1 participant
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.
The v1.3.0-rc.1 boundary test on GKE falsified our loud-failure hypothesis as stated and exposed a real bug shipping since v1.1.0:
Observed: upgrading v1.2.0 → rc.1 without the CRD apply briefly passed readiness, completed the rollout (killing the healthy old pod), and then failed — pod now NotReady/restarting with
no matches for kind ... v1beta1every 10s, reconciliation dead. Loud eventually, but through a false-Ready window that destroyed the working instance first.Root cause: the readyz cache-sync gate called
WaitForCacheSyncbeforemgr.Start()registered informers — an empty informer set syncs trivially, so the gate passed from t=0 and has been decorative since v1.1.0. (The v1.2.0strictConfigcheck is unaffected — it evaluates per-probe against the config store and was genuinely verified.)Fix: readiness derives from a manager
RunnableFunc— non-leader-election runnables execute only after the manager's caches actually sync, so reaching the runnable is the condition. Corrected stranded-CRD behavior (to be re-verified on rc.2): the new pod never reports Ready, the rolling update stalls with the old pod still serving (zero inventory downtime),ProgressDeadlineExceededsignals the operator, and the CRD apply completes the rollout.Docs (migration guide, Design 001) corrected from the hypothesized behavior to the verified one; changelog entry in the 1.3.0 section.
Full suite green. Next: rc.2 and the boundary test rerun.