Prevent spurious canary rollouts using a server-side change fence#1942
Open
stefanprodan wants to merge 5 commits into
Open
Prevent spurious canary rollouts using a server-side change fence#1942stefanprodan wants to merge 5 commits into
stefanprodan wants to merge 5 commits into
Conversation
stefanprodan
added
the
kind/enhancement
stefanprodan
force-pushed
the
feat/server-side-change-fence
branch
from
July 2, 2026 09:35
d402007 to
4204312
Compare
The field records the server-side change fence of the target workload as <uid>/<counter>[/<deployment-revision>], where the counter is metadata.generation for Deployments and DaemonSets, resourceVersion for Services and the latest created revision name for Knative Services. It identifies the exact object state that lastAppliedSpec was computed from, allowing hash drift caused by Flagger or cluster upgrades to be told apart from real spec changes. No codegen changes required: the generated deepcopy covers the string field via the struct value copy. Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
Additive change applied to all three CRD copies (artifacts, Helm chart and kustomize base). Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
Flagger detected target changes by comparing spew-based hashes of decoded Go structs. Spew prints every struct field, so the hash encoded the k8s.io/api schema and changed on every dependency upgrade, triggering spurious canary analyses for unchanged workloads (#1673). The spec hash is now computed from the canonical JSON representation (unset fields omitted, sorted keys, sha256, 'v2:' prefix) and stored together with the change fence in status.lastTrackedRevision. When the fence proves the spec did not change, a differing hash is an upgrade artifact and is absorbed by re-baselining the status; when the fence moved, hashes decide between replicas-only changes, manual rollbacks and real changes. For Deployments, a revision annotation component (recorded only while rollouts are not paused and the deployment controller has observed the current generation) lets scale-only generation bumps coincide with hash drift without false triggers. Volatile counters (Service resourceVersion) are never chased with status writes, and untrusted fences (generation 0, empty uid) fall back to hash comparison and never absorb. Flagger's own scale-down writes refresh the fence from the write response, guarded so a concurrent change is never absorbed. Canaries with legacy status migrate once: settled phases adopt the current state as baseline (guarded by a canary-vs-primary image comparison for Deployments/DaemonSets and a latest-vs-promoted revision comparison for Knative), while in-flight and failed canaries restart their analysis once — loud but safe. All new status writes go through a DeepCopy and abort when a concurrent writer moved the tracked spec, so the in-memory canary object is never mutated and the (lastAppliedSpec, lastTrackedRevision) pair always describes the same observed object state. Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
The freshly initialized canary status is copied back into the local object before shouldAdvance runs; without the fence the next evaluation would take the migration branch spuriously. Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
Covers the decision table per rule (uid gate, drift absorption with lastPromotedSpec lockstep, replicas-only fence refresh, revision backfill, manual rollback, secondary revision guards, incapable and volatile fences), the phase-aware migration incl. the image comparison heuristic and its failure mode, persistence freshness checks under conflict retries, persist-failure verdict stability, the DaemonSet scale-to-zero normalization edge and golden hash values pinning the canonical JSON representation against future k8s.io/api upgrades. The Knative change detection test now exercises the v2 path instead of passing vacuously through the migration branch. Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
stefanprodan
force-pushed
the
feat/server-side-change-fence
branch
from
July 2, 2026 10:15
4204312 to
ba973e4
Compare
Member
|
is the canary vs primary image comparison guard to detect the "common" way of active rollouts? since rollouts can be triggered by any spec change. |
Member
Author
No, the image diff is a one-time migration event from the legacy hash to v2, which happens once per cluster, first time the new Flagger version is rolled out. There is no other stable field that we can use to detect if the primary/canary drifted while Flagger was shutdown (this window is very short). |
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
2 participants
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.
Upgrading Flagger triggers canary analyses for unchanged workloads because the spec hash is computed with spew over the decoded Go structs: every field, including unset ones, ends up in the hash, so any
k8s.io/apiupgrade that adds a field toPodSpecchanges the hash of an unchanged deployment.This PR makes change detection independent of the client schema:
v2:prefix), so new struct fields no longer move the hash.status.lastTrackedRevision(<uid>/<counter>[/<deployment-revision>], counter =metadata.generationfor Deployments/DaemonSets,resourceVersionfor Services, latest revision name for Knative). When the fence proves the spec did not change, a hash mismatch is an upgrade artifact and is absorbed by re-baselining the status instead of triggering an analysis. When the fence moved, hashes decide between replicas-only changes, manual rollbacks and real changes.Fixes #1673
Closes: #1923