From 36c6d0cc77d3445c786047ce52a48d8ef82eb5f1 Mon Sep 17 00:00:00 2001 From: Syed Is Saqlain Date: Thu, 10 Sep 2026 17:47:57 +0400 Subject: [PATCH 1/4] fix(image-refresh): gate the fresh-install re-pin roll on a digest-applied marker (backend#3556) On `recorded == latest` with the workload on `:tag`, the loop could not tell a genuine fresh install (first-observation recorded the digest but applied nothing) from a helm re-render that reverted an already-applied pin. It re-pinned both, so every fresh install paid the shared #563 flap path (resource-monitor DaemonSet under `tolerations: Exists`) and a `strategy: Recreate` downtime on jobs-manager for byte-identical content the install had just pulled. Add a durable per-image `tracebloc.io/digest-applied-` marker, written alongside the digest record only after a re-image rollout succeeds (never on the first-observation record). Its ABSENCE gates the off-digest re-pin out: a fresh install is left on `:tag` (IfNotPresent-safe) until the first genuine upstream digest change pins it. The client-runtime#199 helm-revert repair is unaffected -- that path has the marker set, so it still rolls. Marker and digest record share one annotate, so a later `recorded == latest` tick sees both or neither. Item 2 of backend#3556 (split from client#1008). Item 1 (the latched-flap Pass-0 annotation drop) is a separate fix, not included here. Tests: - image-refresh-repin-on-revert.bats: new "fresh install does NOT roll" case; the existing revert case now carries the applied marker (established edge). - image_refresh_test.yaml: render guard locking the marker derive/read/write. Header comments updated to describe the gate (the old "fresh edge reproducible ~one interval post-install" and "a follow-up may gate this" notes are now false). Co-Authored-By: Claude Opus 4.8 --- client/templates/image-refresh-cronjob.yaml | 91 +++++++++++++++---- client/tests/image_refresh_test.yaml | 31 +++++++ .../tests/image-refresh-repin-on-revert.bats | 44 +++++++-- 3 files changed, 141 insertions(+), 25 deletions(-) diff --git a/client/templates/image-refresh-cronjob.yaml b/client/templates/image-refresh-cronjob.yaml index d664eab1..32c1777b 100644 --- a/client/templates/image-refresh-cronjob.yaml +++ b/client/templates/image-refresh-cronjob.yaml @@ -98,13 +98,25 @@ data: # would rewrite `repo:tag` to `repo@digest` on every fresh install — a spec # change, therefore a rollout, for byte-identical content, and for the # resource-monitor DaemonSet that is a rollout across every node. So the - # first tick only RECORDS. The NEXT tick, seeing `recorded == latest` but the - # workload still on `:tag`, pins the digest (client-runtime#199) — so a fresh edge - # becomes reproducible ~one interval post-install, NOT "at the next upstream - # release". Between the two it is still restart-safe offline (IfNotPresent). + # first tick only RECORDS. It stays restart-safe offline throughout + # (IfNotPresent). # - # COST, stated honestly (@shujaatTracebloc / @LukasWodka on #1008): that - # re-pin is not "one cheap rollout". It enters the shared #563 flap path — + # A fresh install then STAYS on `:tag` (backend#3556). The re-pin on + # `recorded == latest` reads the live workload and rolls only when a digest + # was ALREADY applied here once — the `tracebloc.io/digest-applied-` + # marker, written below only after a successful re-image rollout — and a helm + # re-render has since reverted it onto the bare tag (the client-runtime#199 + # repair). A genuine fresh install applied nothing, so the marker is absent + # and the tick leaves the workload on `:tag`: the install already pulled the + # current tag, so there is no stale-`:tag`-layer exposure to repair, and the + # first genuine UPSTREAM digest change is what pins it (and sets the marker). + # This trades "a fresh edge becomes digest-reproducible ~one interval + # post-install" for skipping the fresh-install roll outright — the cost below + # is why (@shujaatTracebloc / @LukasWodka on #1008; backend#3556). + # + # COST of that roll — which the marker gate now avoids on a fresh install — + # stated honestly (@shujaatTracebloc / @LukasWodka on #1008): the re-pin is + # not "one cheap rollout". It enters the shared #563 flap path — # `rollout status` on the resource-monitor DaemonSet, whose # `desiredNumberScheduled` counts every node (tolerations: Exists), so it can # never settle on a fleet with one NotReady/cordoned node; three such ticks @@ -112,11 +124,10 @@ data: # lockout and stop refresh for ALL control-plane images until a human clears # ATTEMPT_KEY, while the CronJob stays green. And jobs-manager is # `strategy: Recreate`, so its extra rollout is full downtime + wait-for-mysql - # for byte-identical content, on every fresh install and again after each - # chart-version bump. The widening is kept deliberately — it also repairs a - # reinstall onto a node whose `:tag` layer is already stale — but that is the - # price, and a follow-up may gate the re-pin on "have we ever applied a digest - # here?" so a genuine fresh install skips the flap path entirely. + # for byte-identical content. That price is now paid ONLY when it buys + # something — repairing an ESTABLISHED edge whose applied pin a helm re-render + # reverted, including a chart-version bump on such an edge. A fresh install, + # with no applied pin to repair, no longer pays it at all. # # Parsing: awk/sed/grep + jq. jq used only where JSON-with-dotted-keys # or container/env-array filtering motivates it; the rest stays in pure @@ -507,6 +518,16 @@ data: pinned="${rest%%|*}" pin_digest="${rest#*|}" + # backend#3556: a durable "has a digest ever been APPLIED to this workload + # here?" marker, written (below, alongside the digest record) only after a + # re-image rollout for this image has succeeded -- never on the + # first-observation record, which applies nothing. Its ABSENCE is what + # distinguishes a genuine fresh install (workload has been on :tag since + # install, no pin to have reverted FROM) from a helm-re-render revert of an + # earlier pin (marker present) in the `recorded == latest` / off-digest arm + # below. Same `${repo#*/}` suffix convention as the stale-pin keys. + applied_key="tracebloc.io/digest-applied-${repo#*/}" + log "checking ${repo}:${IMAGE_TAG} (annotation=${key})" if [ "$pinned" = "1" ]; then @@ -620,11 +641,13 @@ data: # pin whenever the live workload is not already on the pinned digest. # # The re-pin is BOUNDED TO ONE TICK: it writes the digest back and the - # next tick sees the workload on it and no-ops. It also converges a FRESH - # install to the digest one tick after first observation -- the first-tick - # contract in the header records without re-imaging, and this completes - # it, because staying on `:tag` is exactly the steady-state stale-`:tag` - # exposure this fix closes. + # next tick sees the workload on it and no-ops. It fires only to repair an + # ESTABLISHED edge -- one where a digest was applied here before (the + # ${applied_key} marker) and a helm re-render has since reverted it onto + # `:tag`, the steady-state stale-`:tag` exposure this fix closes. A fresh + # install, which never applied a digest, is GATED OUT of this roll (see + # the marker check in the off-digest arm below) and left on `:tag` + # (backend#3556); the first genuine upstream digest change pins it. # # Compare on the @sha256 DIGEST, not the whole image reference: a mutating # admission webhook that rewrites the registry PREFIX to an internal mirror @@ -714,8 +737,34 @@ data: log " deployment/${REQUESTS_PROXY_DEPLOYMENT} runs '${rp_have}', not digest ${latest}" log " -- re-pinning the requests-proxy digest" else + # The workload is fully on :tag, not the digest. TWO causes reach here, + # and only one warrants the roll (backend#3556): + # * HELM REVERT of an applied pin -- a prior tick pinned the digest + # (rollout succeeded, so ${applied_key} is set), then a helm + # re-render discarded it back to :tag. On a node whose :tag layer + # is stale this silently runs an OLD image, so re-pinning is the + # client-runtime#199 repair and MUST run. + # * FRESH INSTALL -- the first-observation tick recorded the digest + # but applied nothing (${applied_key} absent), and the workload has + # been on :tag since install. There is no prior digest to have + # reverted FROM: the install just pulled the current :tag, so + # re-pinning buys no staleness protection -- it only pays the full + # fresh-install roll (the #563 flap path on the resource-monitor + # DaemonSet under `tolerations: Exists`, and a `strategy: Recreate` + # downtime on jobs-manager) for byte-identical content. + # The marker's ABSENCE gates the roll out: leave a fresh install on + # :tag (IfNotPresent-safe; the install pulled current). A genuine + # upstream digest change still rolls -- that goes through the + # `recorded != latest` arm below, which is where the FIRST apply, and + # this marker, happen. Read via `|| true`: an unreadable marker skips + # the roll this tick and retries, the same fail-closed stance the + # unreadable-`have` skip above already takes. + if [ -z "$(get_annotation "$applied_key" || true)" ]; then + log " digest unchanged and workload runs '${have}' on :${IMAGE_TAG}, but no digest was ever applied here (${applied_key} absent) -- treating as a fresh install and NOT rolling (backend#3556); it stays on :${IMAGE_TAG} (IfNotPresent-safe) until a genuine upstream digest change pins it." + continue + fi log " digest unchanged, but the workload runs '${have}', not digest ${latest}" - log " (fresh install, or a helm re-render reverted the pin onto :${IMAGE_TAG}) -- re-pinning the digest" + log " (a helm re-render reverted the pin onto :${IMAGE_TAG}) -- re-pinning the digest" fi # Fall through to the re-image path (ref + case block) with recorded # already == latest: the annotate below is an idempotent re-write, and @@ -723,7 +772,13 @@ data: else log " digest changed (${recorded} -> ${latest}); re-image needed" fi - annotate_args="$annotate_args ${key}=${latest}" + # Record the digest AND mark that a digest has now been applied here + # (backend#3556). Both land in the SAME annotate below, after a successful + # rollout, so they share one fate: whenever a later tick reads + # `recorded == latest`, the applied marker it gates on is present too. The + # first-observation arm above deliberately writes NEITHER (it applies + # nothing), which is exactly what leaves a fresh install ungated on :tag. + annotate_args="$annotate_args ${key}=${latest} ${applied_key}=1" restart_needed=1 # The reference we pin. IMAGE_REGISTRY is docker.io here — a mirror diff --git a/client/tests/image_refresh_test.yaml b/client/tests/image_refresh_test.yaml index 3787fbc7..6f6740d5 100644 --- a/client/tests/image_refresh_test.yaml +++ b/client/tests/image_refresh_test.yaml @@ -970,3 +970,34 @@ tests: - matchRegex: path: data["image-refresh.sh"] pattern: 'workload already on the pinned digest; no-op' + + - it: a fresh install (no digest ever applied) is gated OUT of the re-pin roll + # Guards backend#3556 item 2. With `recorded == latest` and the workload on + # :tag, the loop must tell a FRESH install (no digest ever applied here) from + # a helm-revert of an applied pin, and roll ONLY the latter -- else every + # fresh install pays the #563 flap-path / Recreate cost for byte-identical + # content the install already pulled. The `tracebloc.io/digest-applied-` + # marker is the discriminator: derived per image, READ in the off-digest arm + # to gate the roll, and WRITTEN with the digest record only on a re-image. + # image-refresh-repin-on-revert.bats asserts the BEHAVIOUR (an inverted + # comparison reddens there); these lock the code shapes so a refactor cannot + # silently drop the gate. + template: templates/image-refresh-cronjob.yaml + documentIndex: 0 + asserts: + # the marker key, derived per image with the same suffix as stale-pin keys + - matchRegex: + path: data["image-refresh.sh"] + pattern: 'applied_key="tracebloc\.io/digest-applied-\$\{repo#\*/\}"' + # READ to gate the roll: an absent marker is a fresh install -> skip + - matchRegex: + path: data["image-refresh.sh"] + pattern: '\[ -z "\$\(get_annotation "\$applied_key" \|\| true\)" \]' + - matchRegex: + path: data["image-refresh.sh"] + pattern: 'treating as a fresh install and NOT rolling' + # WRITTEN only on a re-image, in the SAME annotate as the digest record so + # the two share one fate (a later recorded==latest tick sees both or neither) + - matchRegex: + path: data["image-refresh.sh"] + pattern: 'annotate_args="\$annotate_args \$\{key\}=\$\{latest\} \$\{applied_key\}=1"' diff --git a/scripts/tests/image-refresh-repin-on-revert.bats b/scripts/tests/image-refresh-repin-on-revert.bats index 9ebfb874..693a6c6b 100644 --- a/scripts/tests/image-refresh-repin-on-revert.bats +++ b/scripts/tests/image-refresh-repin-on-revert.bats @@ -70,10 +70,14 @@ teardown() { rm -rf "$TMP"; } # Runs the shipped re-pin branch with the registry HEAD (already known: recorded # == latest) and the two LIVE-image reads stubbed. -# $1 = STUB_API what workload_image_for_repo returns ("" = unreadable) -# $2 = STUB_PROXY what requests_proxy_image returns ("" = unreadable) -# $3 = RP_PINNED "1" opts the requests-proxy out of following the digest -# $4 = PENDING the ATTEMPT_KEY value carried in (0 = no unfinished re-image) +# $1 = STUB_API what workload_image_for_repo returns ("" = unreadable) +# $2 = STUB_PROXY what requests_proxy_image returns ("" = unreadable) +# $3 = RP_PINNED "1" opts the requests-proxy out of following the digest +# $4 = PENDING the ATTEMPT_KEY value carried in (0 = no unfinished re-image) +# $5 = STUB_APPLIED what get_annotation returns for ${applied_key} +# ("" = no digest ever applied here = FRESH INSTALL; +# non-empty = a digest was applied before = ESTABLISHED edge, +# e.g. a helm re-render reverted an earlier pin). backend#3556. # # The branch is wrapped in a ONE-ITERATION loop so its `continue` statements run # as they ship, rather than being stripped (which would change control flow). @@ -82,6 +86,7 @@ run_branch() { set -eu repo="tracebloc/jobs-manager" key="tracebloc.io/last-refreshed-jobs-manager-digest" +applied_key="tracebloc.io/digest-applied-jobs-manager" IMAGE_REGISTRY="docker.io" IMAGE_TAG="dev" REQUESTS_PROXY_DEPLOYMENT="t-requests-proxy" @@ -91,6 +96,7 @@ recorded="sha256:aaa" STUB_API="\${1:-}" STUB_PROXY="\${2:-}" pending_attempt="\${4:-0}" +STUB_APPLIED="\${5:-}" MAX_REFRESH_ATTEMPTS=3 restart_needed=0 annotate_args="" @@ -105,6 +111,8 @@ log() { printf '%s\n' "\$*"; } kubectl() { printf 'KUBECTL:%s\n' "\$*"; } workload_image_for_repo() { [ -n "\$STUB_API" ] && printf '%s' "\$STUB_API"; } requests_proxy_image() { [ -n "\$STUB_PROXY" ] && printf '%s' "\$STUB_PROXY"; } +# Only the applied marker is read inside this branch; every other key is absent. +get_annotation() { case "\$1" in "\$applied_key") [ -n "\$STUB_APPLIED" ] && printf '%s' "\$STUB_APPLIED" ;; esac; } for _once in 1; do $(sed 's/^/ /' "$TMP/branch.sh") done @@ -113,15 +121,37 @@ printf 'JM:%s\n' "\$jm_set_args" printf 'RP:%s\n' "\$rp_set_args" printf 'ANNOTATE:%s\n' "\$annotate_args" EOF - sh "$TMP/harness.sh" "${1:-}" "${2:-}" "${3:-}" "${4:-0}" + sh "$TMP/harness.sh" "${1:-}" "${2:-}" "${3:-}" "${4:-0}" "${5:-}" } -@test "workload reverted to :tag re-pins the digest (restart_needed=1)" { - run run_branch "docker.io/tracebloc/jobs-manager:dev" "" "1" +@test "ESTABLISHED edge reverted to :tag re-pins the digest (restart_needed=1)" { + # A digest was applied here before (applied marker present), then a helm + # re-render reverted the workload onto :tag -- the client-runtime#199 repair + # must roll. $5="1" is the digest-applied marker (backend#3556). + run run_branch "docker.io/tracebloc/jobs-manager:dev" "" "1" "0" "1" [ "$status" -eq 0 ] || return 1 [[ "$output" == *"RESTART:1"* ]] || return 1 [[ "$output" == *"api=docker.io/tracebloc/jobs-manager@sha256:aaa"* ]] || return 1 [[ "$output" == *"re-pinning the digest"* ]] || return 1 + # the re-image records the digest AND stamps the applied marker, in one annotate + [[ "$output" == *"tracebloc.io/digest-applied-jobs-manager=1"* ]] || return 1 +} + +@test "FRESH install (no digest ever applied) does NOT roll -- stays on :tag" { + # backend#3556 item 2. recorded == latest and the workload is on :tag, but the + # digest-applied marker is absent ($5=""), so this is a genuine fresh install: + # the first-observation tick recorded the digest and applied nothing. Rolling + # here would pay the full #563 flap-path / Recreate cost for byte-identical + # content the install already pulled. The tick must leave it on :tag. + run run_branch "docker.io/tracebloc/jobs-manager:dev" "" "1" "0" "" + [ "$status" -eq 0 ] || return 1 + [[ "$output" == *"fresh install"* ]] || return 1 + [[ "$output" == *"NOT rolling"* ]] || return 1 + [[ "$output" == *"RESTART:0"* ]] || return 1 + # nothing queued for a rollout, and no digest/marker write this tick + [[ "$output" != *"JM:api="* ]] || return 1 + [[ "$output" != *"re-pinning the digest"* ]] || return 1 + [[ "$output" != *"digest-applied-jobs-manager=1"* ]] || return 1 } @test "api and proxy both on the digest is a no-op (restart_needed=0, no set args)" { From c5337245bc2c8e38d75c5022c2997ffa683a6cdc Mon Sep 17 00:00:00 2001 From: Syed Is Saqlain Date: Thu, 10 Sep 2026 18:22:41 +0400 Subject: [PATCH 2/4] fix(image-refresh): don't strand the pre-marker fleet; strip private refs; bump chart Addresses review on #1033. 1. Migration gap (High, Bugbot + LukasWodka): the single digest-applied marker is written only by this version, so every edge pinned by an earlier version has none. The upgrade shipping this chart re-renders the Deployment back to :tag (the same client-runtime#199 premise), so on the next tick each such edge is byte-for-byte the fresh-install shape and, under the old gate, took the skip -- stranding the whole existing fleet on a possibly-stale :tag until the next upstream digest change. Fix (Lukas's shape): skip only on POSITIVE evidence. Add first-observed-, stamped by the first-observation arm; skip the off-digest roll iff first_observed present AND applied absent. Three shapes now fall out: fresh (first_observed only) -> skip; established (applied) -> roll; pre-marker/legacy (neither) -> roll (repair), which then stamps applied. Reads are fail-closed: a marker read ERROR skips the tick and retries. New bats case for the legacy shape; existing fresh/established cases updated; render guard updated to the two-marker code shapes. 2. Public repo: stripped the private tracker ref from the shipped header, the in-script comments, the log line, the helm-unittest and bats comments, and the PR title/body -- the #1008 split note carries the context. 3. Chart version guard: bump client/Chart.yaml 1.9.111 -> 1.9.112 (merged develop first, which had advanced to 1.9.111). Verified: image-refresh-repin-on-revert.bats 13/13; helm unittest 729/729; make check green; shellcheck unchanged (only the pre-existing intentional SC2086). Co-Authored-By: Claude Opus 4.8 --- client/Chart.yaml | 2 +- client/templates/image-refresh-cronjob.yaml | 161 +++++++++++------- client/tests/image_refresh_test.yaml | 41 +++-- .../tests/image-refresh-repin-on-revert.bats | 62 +++++-- 4 files changed, 175 insertions(+), 91 deletions(-) diff --git a/client/Chart.yaml b/client/Chart.yaml index 15c1fd38..aa7fadb5 100644 --- a/client/Chart.yaml +++ b/client/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v2 name: client description: A unified Helm chart for tracebloc on AKS, EKS, bare-metal, and OpenShift type: application -version: 1.9.111 +version: 1.9.112 appVersion: "1.9.111" keywords: - tracebloc diff --git a/client/templates/image-refresh-cronjob.yaml b/client/templates/image-refresh-cronjob.yaml index c3016763..794ee592 100644 --- a/client/templates/image-refresh-cronjob.yaml +++ b/client/templates/image-refresh-cronjob.yaml @@ -103,18 +103,28 @@ data: # first tick only RECORDS. It stays restart-safe offline throughout # (IfNotPresent). # - # A fresh install then STAYS on `:tag` (backend#3556). The re-pin on - # `recorded == latest` reads the live workload and rolls only when a digest - # was ALREADY applied here once — the `tracebloc.io/digest-applied-` - # marker, written below only after a successful re-image rollout — and a helm - # re-render has since reverted it onto the bare tag (the client-runtime#199 - # repair). A genuine fresh install applied nothing, so the marker is absent - # and the tick leaves the workload on `:tag`: the install already pulled the - # current tag, so there is no stale-`:tag`-layer exposure to repair, and the - # first genuine UPSTREAM digest change is what pins it (and sets the marker). + # A fresh install then STAYS on `:tag` (#1008). The re-pin on + # `recorded == latest` reads the live workload and, when it is off the digest, + # skips the roll ONLY on positive fresh-install evidence — two markers: + # `tracebloc.io/first-observed-` (stamped by the first-observation arm + # above) and `tracebloc.io/digest-applied-` (stamped only after a + # re-image rollout succeeds). first_observed present AND applied absent is a + # workload we watched born on `:tag` here, so it is left on `:tag`: the + # install already pulled the current tag, there is no stale-`:tag`-layer + # exposure to repair, and the first genuine UPSTREAM digest change pins it + # (and stamps applied). applied present is an established edge and rolls — + # the client-runtime#199 repair of a helm re-render that reverted the pin. + # + # NEITHER marker is a PRE-MARKER / legacy edge (pinned by a version that + # predates these markers): it is NOT skipped. The upgrade shipping this chart + # re-renders the Deployment back to `:tag` (the same #199 premise), so on the + # next tick every established-but-unmarked edge is byte-for-byte the fresh + # shape; skipping on absence alone would strand the WHOLE existing fleet on a + # possibly-stale `:tag` until the next upstream digest change (LukasWodka on + # #1008). So a legacy edge takes the repair roll once, which stamps applied. # This trades "a fresh edge becomes digest-reproducible ~one interval # post-install" for skipping the fresh-install roll outright — the cost below - # is why (@shujaatTracebloc / @LukasWodka on #1008; backend#3556). + # is why (@shujaatTracebloc / @LukasWodka on #1008). # # COST of that roll — which the marker gate now avoids on a fresh install — # stated honestly (@shujaatTracebloc / @LukasWodka on #1008): the re-pin is @@ -127,9 +137,9 @@ data: # ATTEMPT_KEY, while the CronJob stays green. And jobs-manager is # `strategy: Recreate`, so its extra rollout is full downtime + wait-for-mysql # for byte-identical content. That price is now paid ONLY when it buys - # something — repairing an ESTABLISHED edge whose applied pin a helm re-render - # reverted, including a chart-version bump on such an edge. A fresh install, - # with no applied pin to repair, no longer pays it at all. + # something — repairing an established or legacy edge whose pin a helm + # re-render reverted, including a chart-version bump on such an edge. A fresh + # install, watched born on `:tag` here, no longer pays it at all. # # Parsing: awk/sed/grep + jq. jq used only where JSON-with-dotted-keys # or container/env-array filtering motivates it; the rest stays in pure @@ -528,15 +538,26 @@ data: pinned="${rest%%|*}" pin_digest="${rest#*|}" - # backend#3556: a durable "has a digest ever been APPLIED to this workload - # here?" marker, written (below, alongside the digest record) only after a - # re-image rollout for this image has succeeded -- never on the - # first-observation record, which applies nothing. Its ABSENCE is what - # distinguishes a genuine fresh install (workload has been on :tag since - # install, no pin to have reverted FROM) from a helm-re-render revert of an - # earlier pin (marker present) in the `recorded == latest` / off-digest arm - # below. Same `${repo#*/}` suffix convention as the stale-pin keys. + # #1008: two durable markers that let the `recorded == latest` / off-digest + # arm below tell a genuine fresh install from a helm-revert of an applied + # pin -- WITHOUT mistaking the pre-marker fleet for either. The skip fires + # only on POSITIVE fresh-install evidence, never on mere absence: + # first_observed_key written by the first-observation arm (we saw this + # workload with no annotation and recorded the digest + # WITHOUT applying it -- proof it was born on :tag here). + # applied_key written alongside the digest record only after a + # re-image rollout succeeds -- proof a digest was + # actually rolled onto this workload. + # Fresh install = first_observed present AND applied absent -> skip the roll. + # Established edge = applied present -> roll (the client-runtime#199 repair). + # Pre-marker / legacy edge = NEITHER marker (nothing wrote them before this + # version): it is NOT positively a fresh install, so it keeps the repair + # roll -- the fleet upgrading INTO this version reverts to :tag on the + # upgrade hop, and skipping it on absence alone would strand every such edge + # on a possibly-stale :tag until the next upstream digest change (LukasWodka + # on #1008). Same `${repo#*/}` suffix convention as the stale-pin keys. applied_key="tracebloc.io/digest-applied-${repo#*/}" + first_observed_key="tracebloc.io/first-observed-${repo#*/}" log "checking ${repo}:${IMAGE_TAG} (annotation=${key})" @@ -633,9 +654,14 @@ data: # First-observation path: record without re-imaging. No evidence # the running pod is on an older digest, and rewriting repo:tag to # repo@digest for identical content would roll every pod on install - # (see the first-tick contract in the header). + # (see the first-tick contract in the header). Stamp first_observed_key + # too (#1008): this is the POSITIVE evidence that lets the off-digest arm + # skip the fresh-install roll -- a workload we watched from its first + # annotation-less tick. A pre-marker edge upgrading into this version + # never gets this stamp (its first observation predates the marker), so + # it is not mistaken for a fresh install and keeps the repair roll. log " first observation; recording without re-imaging" - annotate_args="$annotate_args ${key}=${latest}" + annotate_args="$annotate_args ${key}=${latest} ${first_observed_key}=1" continue fi @@ -651,13 +677,14 @@ data: # pin whenever the live workload is not already on the pinned digest. # # The re-pin is BOUNDED TO ONE TICK: it writes the digest back and the - # next tick sees the workload on it and no-ops. It fires only to repair an - # ESTABLISHED edge -- one where a digest was applied here before (the - # ${applied_key} marker) and a helm re-render has since reverted it onto + # next tick sees the workload on it and no-ops. It fires to repair an + # established or legacy edge whose pin a helm re-render reverted onto # `:tag`, the steady-state stale-`:tag` exposure this fix closes. A fresh - # install, which never applied a digest, is GATED OUT of this roll (see - # the marker check in the off-digest arm below) and left on `:tag` - # (backend#3556); the first genuine upstream digest change pins it. + # install -- one we watched born on `:tag` (first_observed set, applied + # absent) -- is GATED OUT of this roll (see the two-marker check in the + # off-digest arm below) and left on `:tag` (#1008); the first genuine + # upstream digest change pins it. A pre-marker/legacy edge has neither + # marker and is NOT gated out, so the existing fleet is not stranded. # # Compare on the @sha256 DIGEST, not the whole image reference: a mutating # admission webhook that rewrites the registry PREFIX to an internal mirror @@ -747,34 +774,53 @@ data: log " deployment/${REQUESTS_PROXY_DEPLOYMENT} runs '${rp_have}', not digest ${latest}" log " -- re-pinning the requests-proxy digest" else - # The workload is fully on :tag, not the digest. TWO causes reach here, - # and only one warrants the roll (backend#3556): - # * HELM REVERT of an applied pin -- a prior tick pinned the digest + # The workload is fully on :tag, not the digest. THREE shapes reach + # here, and the skip must fire only on the fresh-install one (#1008): + # * ESTABLISHED edge, helm-reverted -- a prior tick pinned the digest # (rollout succeeded, so ${applied_key} is set), then a helm # re-render discarded it back to :tag. On a node whose :tag layer # is stale this silently runs an OLD image, so re-pinning is the - # client-runtime#199 repair and MUST run. + # client-runtime#199 repair and MUST run. (applied present) # * FRESH INSTALL -- the first-observation tick recorded the digest - # but applied nothing (${applied_key} absent), and the workload has - # been on :tag since install. There is no prior digest to have - # reverted FROM: the install just pulled the current :tag, so - # re-pinning buys no staleness protection -- it only pays the full - # fresh-install roll (the #563 flap path on the resource-monitor - # DaemonSet under `tolerations: Exists`, and a `strategy: Recreate` - # downtime on jobs-manager) for byte-identical content. - # The marker's ABSENCE gates the roll out: leave a fresh install on - # :tag (IfNotPresent-safe; the install pulled current). A genuine - # upstream digest change still rolls -- that goes through the - # `recorded != latest` arm below, which is where the FIRST apply, and - # this marker, happen. Read via `|| true`: an unreadable marker skips - # the roll this tick and retries, the same fail-closed stance the - # unreadable-`have` skip above already takes. - if [ -z "$(get_annotation "$applied_key" || true)" ]; then - log " digest unchanged and workload runs '${have}' on :${IMAGE_TAG}, but no digest was ever applied here (${applied_key} absent) -- treating as a fresh install and NOT rolling (backend#3556); it stays on :${IMAGE_TAG} (IfNotPresent-safe) until a genuine upstream digest change pins it." + # and applied nothing, and the workload has been on :tag since + # install. No prior digest to have reverted FROM: the install just + # pulled the current :tag, so re-pinning buys no staleness + # protection -- it only pays the full fresh-install roll (the #563 + # flap path on the resource-monitor DaemonSet under + # `tolerations: Exists`, and a `strategy: Recreate` downtime on + # jobs-manager) for byte-identical content. + # (first_observed present AND applied absent) + # * PRE-MARKER / LEGACY edge -- pinned by a version that predates + # these markers, so it has NEITHER. The upgrade that ships this + # chart re-renders the Deployment back to :tag (the same #199 + # premise), so on the next tick it is byte-for-byte the fresh + # shape. Skipping on absence alone would strand the WHOLE existing + # fleet on a possibly-stale :tag until the next upstream digest + # change (weeks on :prod) -- the exact #199 exposure, fleet-wide. + # So a legacy edge (no first_observed) is NOT skipped: it takes the + # repair roll, which then stamps applied_key. (neither marker) + # Skip ONLY on positive fresh-install evidence. A read ERROR on either + # marker (as opposed to genuine absence) skips the re-pin this tick and + # retries -- the same fail-closed stance the unreadable-`have` skip + # above takes -- rather than guessing the shape. + if ! first_observed="$(get_annotation "$first_observed_key")"; then + log " digest unchanged and workload off-digest, but ${first_observed_key} was unreadable (API/jq error) -- skipping re-pin this tick, will retry when readable" + continue + fi + if ! applied="$(get_annotation "$applied_key")"; then + log " digest unchanged and workload off-digest, but ${applied_key} was unreadable (API/jq error) -- skipping re-pin this tick, will retry when readable" continue fi - log " digest unchanged, but the workload runs '${have}', not digest ${latest}" - log " (a helm re-render reverted the pin onto :${IMAGE_TAG}) -- re-pinning the digest" + if [ -n "$first_observed" ] && [ -z "$applied" ]; then + log " digest unchanged and workload runs '${have}' on :${IMAGE_TAG}, but this is a fresh install (observed here, no digest ever applied) -- NOT rolling (#1008); it stays on :${IMAGE_TAG} (IfNotPresent-safe) until a genuine upstream digest change pins it." + continue + fi + if [ -z "$first_observed" ] && [ -z "$applied" ]; then + log " digest unchanged, but the workload runs '${have}', not digest ${latest}, and this edge predates the fresh-install markers -- re-pinning to repair a possibly-stale :${IMAGE_TAG} on the upgrade hop (client-runtime#199)" + else + log " digest unchanged, but the workload runs '${have}', not digest ${latest}" + log " (a helm re-render reverted the pin onto :${IMAGE_TAG}) -- re-pinning the digest" + fi fi # Fall through to the re-image path (ref + case block) with recorded # already == latest: the annotate below is an idempotent re-write, and @@ -782,12 +828,13 @@ data: else log " digest changed (${recorded} -> ${latest}); re-image needed" fi - # Record the digest AND mark that a digest has now been applied here - # (backend#3556). Both land in the SAME annotate below, after a successful - # rollout, so they share one fate: whenever a later tick reads - # `recorded == latest`, the applied marker it gates on is present too. The - # first-observation arm above deliberately writes NEITHER (it applies - # nothing), which is exactly what leaves a fresh install ungated on :tag. + # Record the digest AND stamp applied_key -- proof a digest has now been + # rolled onto this workload (#1008). Both land in the SAME annotate below, + # after a successful rollout, so they share one fate: whenever a later tick + # reads `recorded == latest`, the applied marker it gates on is present too. + # The first-observation arm stamps first_observed_key instead (it applies + # nothing), and the two markers are what let the off-digest arm above tell a + # fresh install from an established edge without stranding the legacy fleet. annotate_args="$annotate_args ${key}=${latest} ${applied_key}=1" restart_needed=1 diff --git a/client/tests/image_refresh_test.yaml b/client/tests/image_refresh_test.yaml index c812a106..c7fcfb42 100644 --- a/client/tests/image_refresh_test.yaml +++ b/client/tests/image_refresh_test.yaml @@ -973,33 +973,40 @@ tests: path: data["image-refresh.sh"] pattern: 'workload already on the pinned digest; no-op' - - it: a fresh install (no digest ever applied) is gated OUT of the re-pin roll - # Guards backend#3556 item 2. With `recorded == latest` and the workload on - # :tag, the loop must tell a FRESH install (no digest ever applied here) from - # a helm-revert of an applied pin, and roll ONLY the latter -- else every - # fresh install pays the #563 flap-path / Recreate cost for byte-identical - # content the install already pulled. The `tracebloc.io/digest-applied-` - # marker is the discriminator: derived per image, READ in the off-digest arm - # to gate the roll, and WRITTEN with the digest record only on a re-image. - # image-refresh-repin-on-revert.bats asserts the BEHAVIOUR (an inverted - # comparison reddens there); these lock the code shapes so a refactor cannot - # silently drop the gate. + - it: a fresh install is gated OUT of the re-pin roll, but the legacy fleet is not + # Guards #1008 item 2. With `recorded == latest` and the workload on :tag, the + # loop must tell a FRESH install (watched born on :tag here) from a helm-revert + # of an applied pin AND from a pre-marker/legacy edge, and skip ONLY the fresh + # one -- else either every fresh install pays the #563 flap-path / Recreate + # cost for byte-identical content, or (if it skipped on absence alone) the + # whole existing fleet is stranded on a stale :tag on the upgrade hop. Two + # markers discriminate: `first-observed-` (stamped by first-observation) + # and `digest-applied-` (stamped only on a successful re-image). Skip = + # first_observed present AND applied absent. image-refresh-repin-on-revert.bats + # asserts the BEHAVIOUR across all three shapes (fresh / established / legacy); + # these lock the code shapes so a refactor cannot silently drop the gate. template: templates/image-refresh-cronjob.yaml documentIndex: 0 asserts: - # the marker key, derived per image with the same suffix as stale-pin keys + # both marker keys, derived per image with the same suffix as stale-pin keys - matchRegex: path: data["image-refresh.sh"] pattern: 'applied_key="tracebloc\.io/digest-applied-\$\{repo#\*/\}"' - # READ to gate the roll: an absent marker is a fresh install -> skip - matchRegex: path: data["image-refresh.sh"] - pattern: '\[ -z "\$\(get_annotation "\$applied_key" \|\| true\)" \]' + pattern: 'first_observed_key="tracebloc\.io/first-observed-\$\{repo#\*/\}"' + # the skip fires ONLY on positive fresh-install evidence, not on absence - matchRegex: path: data["image-refresh.sh"] - pattern: 'treating as a fresh install and NOT rolling' - # WRITTEN only on a re-image, in the SAME annotate as the digest record so - # the two share one fate (a later recorded==latest tick sees both or neither) + pattern: '\[ -n "\$first_observed" \] && \[ -z "\$applied" \]' + - matchRegex: + path: data["image-refresh.sh"] + pattern: 'this is a fresh install .* -- NOT rolling' + # first-observation stamps first_observed (the positive fresh-install signal) + - matchRegex: + path: data["image-refresh.sh"] + pattern: 'annotate_args="\$annotate_args \$\{key\}=\$\{latest\} \$\{first_observed_key\}=1"' + # a re-image stamps applied alongside the digest record (one shared fate) - matchRegex: path: data["image-refresh.sh"] pattern: 'annotate_args="\$annotate_args \$\{key\}=\$\{latest\} \$\{applied_key\}=1"' diff --git a/scripts/tests/image-refresh-repin-on-revert.bats b/scripts/tests/image-refresh-repin-on-revert.bats index 693a6c6b..e838ef2a 100644 --- a/scripts/tests/image-refresh-repin-on-revert.bats +++ b/scripts/tests/image-refresh-repin-on-revert.bats @@ -74,10 +74,14 @@ teardown() { rm -rf "$TMP"; } # $2 = STUB_PROXY what requests_proxy_image returns ("" = unreadable) # $3 = RP_PINNED "1" opts the requests-proxy out of following the digest # $4 = PENDING the ATTEMPT_KEY value carried in (0 = no unfinished re-image) -# $5 = STUB_APPLIED what get_annotation returns for ${applied_key} -# ("" = no digest ever applied here = FRESH INSTALL; -# non-empty = a digest was applied before = ESTABLISHED edge, -# e.g. a helm re-render reverted an earlier pin). backend#3556. +# $5 = STUB_APPLIED get_annotation value for ${applied_key} +# (non-empty = a digest was rolled here before) +# $6 = STUB_FIRST_OBSERVED get_annotation value for ${first_observed_key} +# (non-empty = we recorded this workload's first, +# annotation-less tick here) +# The off-digest arm skips ONLY on first_observed present AND applied absent +# (fresh install). applied present rolls (established edge); NEITHER marker rolls +# (pre-marker/legacy edge -- not stranded on the upgrade hop). #1008. # # The branch is wrapped in a ONE-ITERATION loop so its `continue` statements run # as they ship, rather than being stripped (which would change control flow). @@ -87,6 +91,7 @@ set -eu repo="tracebloc/jobs-manager" key="tracebloc.io/last-refreshed-jobs-manager-digest" applied_key="tracebloc.io/digest-applied-jobs-manager" +first_observed_key="tracebloc.io/first-observed-jobs-manager" IMAGE_REGISTRY="docker.io" IMAGE_TAG="dev" REQUESTS_PROXY_DEPLOYMENT="t-requests-proxy" @@ -97,6 +102,7 @@ STUB_API="\${1:-}" STUB_PROXY="\${2:-}" pending_attempt="\${4:-0}" STUB_APPLIED="\${5:-}" +STUB_FIRST_OBSERVED="\${6:-}" MAX_REFRESH_ATTEMPTS=3 restart_needed=0 annotate_args="" @@ -111,8 +117,17 @@ log() { printf '%s\n' "\$*"; } kubectl() { printf 'KUBECTL:%s\n' "\$*"; } workload_image_for_repo() { [ -n "\$STUB_API" ] && printf '%s' "\$STUB_API"; } requests_proxy_image() { [ -n "\$STUB_PROXY" ] && printf '%s' "\$STUB_PROXY"; } -# Only the applied marker is read inside this branch; every other key is absent. -get_annotation() { case "\$1" in "\$applied_key") [ -n "\$STUB_APPLIED" ] && printf '%s' "\$STUB_APPLIED" ;; esac; } +# Only the two markers are read inside this branch. Model the real get_annotation: +# return 0 with the value (empty = annotation ABSENT), never non-zero -- a +# non-zero return means a kubectl/jq READ ERROR, which the branch handles +# separately. Using \`[ -n ] && printf\` here would return non-zero on an empty +# stub and be misread as a read error. +get_annotation() { + case "\$1" in + "\$applied_key") printf '%s' "\$STUB_APPLIED" ;; + "\$first_observed_key") printf '%s' "\$STUB_FIRST_OBSERVED" ;; + esac +} for _once in 1; do $(sed 's/^/ /' "$TMP/branch.sh") done @@ -121,13 +136,13 @@ printf 'JM:%s\n' "\$jm_set_args" printf 'RP:%s\n' "\$rp_set_args" printf 'ANNOTATE:%s\n' "\$annotate_args" EOF - sh "$TMP/harness.sh" "${1:-}" "${2:-}" "${3:-}" "${4:-0}" "${5:-}" + sh "$TMP/harness.sh" "${1:-}" "${2:-}" "${3:-}" "${4:-0}" "${5:-}" "${6:-}" } @test "ESTABLISHED edge reverted to :tag re-pins the digest (restart_needed=1)" { - # A digest was applied here before (applied marker present), then a helm + # A digest was applied here before (applied marker present, $5="1"), then a helm # re-render reverted the workload onto :tag -- the client-runtime#199 repair - # must roll. $5="1" is the digest-applied marker (backend#3556). + # must roll. #1008. run run_branch "docker.io/tracebloc/jobs-manager:dev" "" "1" "0" "1" [ "$status" -eq 0 ] || return 1 [[ "$output" == *"RESTART:1"* ]] || return 1 @@ -137,13 +152,12 @@ EOF [[ "$output" == *"tracebloc.io/digest-applied-jobs-manager=1"* ]] || return 1 } -@test "FRESH install (no digest ever applied) does NOT roll -- stays on :tag" { - # backend#3556 item 2. recorded == latest and the workload is on :tag, but the - # digest-applied marker is absent ($5=""), so this is a genuine fresh install: - # the first-observation tick recorded the digest and applied nothing. Rolling - # here would pay the full #563 flap-path / Recreate cost for byte-identical - # content the install already pulled. The tick must leave it on :tag. - run run_branch "docker.io/tracebloc/jobs-manager:dev" "" "1" "0" "" +@test "FRESH install (first-observed here, never applied) does NOT roll -- stays on :tag" { + # #1008. recorded == latest, workload on :tag, first_observed set ($6="1") and + # applied absent ($5="") -- a workload we watched born on :tag here. Rolling + # would pay the full #563 flap-path / Recreate cost for byte-identical content + # the install already pulled. The tick must leave it on :tag. + run run_branch "docker.io/tracebloc/jobs-manager:dev" "" "1" "0" "" "1" [ "$status" -eq 0 ] || return 1 [[ "$output" == *"fresh install"* ]] || return 1 [[ "$output" == *"NOT rolling"* ]] || return 1 @@ -154,6 +168,22 @@ EOF [[ "$output" != *"digest-applied-jobs-manager=1"* ]] || return 1 } +@test "PRE-MARKER / legacy edge (NEITHER marker) still gets the repair roll" { + # LukasWodka on #1008: an edge pinned by a version predating these markers has + # neither ($5="" $6=""). The upgrade shipping this chart reverts it to :tag, so + # its first post-upgrade tick is byte-for-byte the fresh shape. Skipping on + # absence alone would strand the whole existing fleet on a possibly-stale :tag + # until the next upstream digest change -- the exact #199 exposure. A legacy + # edge must therefore ROLL (repair), which then stamps the applied marker. + run run_branch "docker.io/tracebloc/jobs-manager:dev" "" "1" "0" "" "" + [ "$status" -eq 0 ] || return 1 + [[ "$output" == *"RESTART:1"* ]] || return 1 + [[ "$output" == *"predates the fresh-install markers"* ]] || return 1 + [[ "$output" == *"api=docker.io/tracebloc/jobs-manager@sha256:aaa"* ]] || return 1 + [[ "$output" == *"tracebloc.io/digest-applied-jobs-manager=1"* ]] || return 1 + [[ "$output" != *"fresh install"* ]] || return 1 +} + @test "api and proxy both on the digest is a no-op (restart_needed=0, no set args)" { run run_branch "docker.io/tracebloc/jobs-manager@sha256:aaa" \ "docker.io/tracebloc/jobs-manager@sha256:aaa" "0" From f0856ce8f8b7ef718b3ad86657a6499962581d84 Mon Sep 17 00:00:00 2001 From: Syed Is Saqlain Date: Thu, 10 Sep 2026 18:24:20 +0400 Subject: [PATCH 3/4] fix(chart): bump appVersion to 1.9.112 in lockstep with version The chart-version guard requires version and appVersion to move together (_helpers.tpl feeds appVersion into app.kubernetes.io/version); the earlier bump moved version only. Co-Authored-By: Claude Opus 4.8 --- client/Chart.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/Chart.yaml b/client/Chart.yaml index aa7fadb5..14a392bd 100644 --- a/client/Chart.yaml +++ b/client/Chart.yaml @@ -3,7 +3,7 @@ name: client description: A unified Helm chart for tracebloc on AKS, EKS, bare-metal, and OpenShift type: application version: 1.9.112 -appVersion: "1.9.111" +appVersion: "1.9.112" keywords: - tracebloc - kubernetes From 5962509f4c6b341deb3ec5a510e7425240a534f8 Mon Sep 17 00:00:00 2001 From: Syed Is Saqlain Date: Thu, 10 Sep 2026 19:11:20 +0400 Subject: [PATCH 4/4] fix(image-refresh): fail-closed the digest-record read so an error can't mis-stamp first-observed Bugbot Medium on #1008. The first-observation arm now stamps first_observed, which suppresses the client-runtime#199 repair on later ticks. But the digest record was still read with `recorded="$(get_annotation "$key" || true)"`, which collapses a transient kubectl/jq ERROR into "" -- so on a legacy edge (which HAS a recorded digest) a read error would fall into the first-observation arm, stamp first_observed, and strand that edge on :tag until the next upstream digest change. Read fail-closed instead: get_annotation returns non-zero on a read error and zero+empty only on genuine absence, so `if ! recorded=...; then skip; fi` makes the first-observation arm reachable only on a real absence. Render guard asserts the guarded read is present and the fail-open `|| true` form is gone. Verified: image_refresh helm-unittest 39/39; repin bats 13/13; shellcheck unchanged (pre-existing SC2086 only). Co-Authored-By: Claude Opus 4.8 --- client/templates/image-refresh-cronjob.yaml | 13 ++++++++++++- client/tests/image_refresh_test.yaml | 10 ++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/client/templates/image-refresh-cronjob.yaml b/client/templates/image-refresh-cronjob.yaml index 794ee592..0c81780d 100644 --- a/client/templates/image-refresh-cronjob.yaml +++ b/client/templates/image-refresh-cronjob.yaml @@ -645,7 +645,18 @@ data: continue fi - recorded="$(get_annotation "$key" || true)" + # FAIL-CLOSED, not `|| true` (Bugbot on #1008). A read ERROR here must NOT + # collapse to "" and fall into the first-observation arm below: that arm now + # stamps first_observed_key, which SUPPRESSES the client-runtime#199 repair + # on later ticks. So a transient kubectl/jq error on a legacy edge (which + # genuinely HAS a recorded digest) would mis-stamp it as first-observed and + # strand it on :tag. get_annotation returns non-zero on a read error and + # zero+empty only on genuine absence, so skip this image this tick on error + # and retry -- the same stance the marker reads and the settled guard take. + if ! recorded="$(get_annotation "$key")"; then + log " WARN: could not read ${key} (kubectl/jq error); skipping this image this tick rather than mistaking a read error for a first observation. Will retry next tick." + continue + fi log " latest=$latest" log " recorded=${recorded:-}" diff --git a/client/tests/image_refresh_test.yaml b/client/tests/image_refresh_test.yaml index c7fcfb42..c332414c 100644 --- a/client/tests/image_refresh_test.yaml +++ b/client/tests/image_refresh_test.yaml @@ -1010,3 +1010,13 @@ tests: - matchRegex: path: data["image-refresh.sh"] pattern: 'annotate_args="\$annotate_args \$\{key\}=\$\{latest\} \$\{applied_key\}=1"' + # the digest-record read is FAIL-CLOSED: a read error must not collapse to "" + # and enter the first-observation arm (which stamps first_observed and + # suppresses the #199 repair on a legacy edge) -- Bugbot on #1008. The + # fail-open `|| true` form must be gone; the guarded read must be present. + - matchRegex: + path: data["image-refresh.sh"] + pattern: 'if ! recorded="\$\(get_annotation "\$key"\)"; then' + - notMatchRegex: + path: data["image-refresh.sh"] + pattern: 'recorded="\$\(get_annotation "\$key" \|\| true\)"'