diff --git a/client/Chart.yaml b/client/Chart.yaml index 15c1fd38..14a392bd 100644 --- a/client/Chart.yaml +++ b/client/Chart.yaml @@ -2,8 +2,8 @@ apiVersion: v2 name: client description: A unified Helm chart for tracebloc on AKS, EKS, bare-metal, and OpenShift type: application -version: 1.9.111 -appVersion: "1.9.111" +version: 1.9.112 +appVersion: "1.9.112" keywords: - tracebloc - kubernetes diff --git a/client/templates/image-refresh-cronjob.yaml b/client/templates/image-refresh-cronjob.yaml index d57bdaae..0c81780d 100644 --- a/client/templates/image-refresh-cronjob.yaml +++ b/client/templates/image-refresh-cronjob.yaml @@ -100,13 +100,35 @@ 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` (#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). + # + # 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 @@ -114,11 +136,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 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 @@ -517,6 +538,27 @@ data: pinned="${rest%%|*}" pin_digest="${rest#*|}" + # #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})" if [ "$pinned" = "1" ]; then @@ -603,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:-}" @@ -612,9 +665,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 @@ -630,11 +688,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 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 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 -- 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 @@ -724,8 +785,53 @@ data: log " deployment/${REQUESTS_PROXY_DEPLOYMENT} runs '${rp_have}', not digest ${latest}" log " -- re-pinning the requests-proxy digest" else - 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" + # 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. (applied present) + # * FRESH INSTALL -- the first-observation tick recorded the digest + # 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 + 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 @@ -733,7 +839,14 @@ data: else log " digest changed (${recorded} -> ${latest}); re-image needed" fi - annotate_args="$annotate_args ${key}=${latest}" + # 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 # The reference we pin: the SAME registry the digest above was resolved diff --git a/client/tests/image_refresh_test.yaml b/client/tests/image_refresh_test.yaml index e1f8ddda..c332414c 100644 --- a/client/tests/image_refresh_test.yaml +++ b/client/tests/image_refresh_test.yaml @@ -972,3 +972,51 @@ tests: - matchRegex: path: data["image-refresh.sh"] pattern: 'workload already on the pinned digest; no-op' + + - 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: + # 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#\*/\}"' + - matchRegex: + path: data["image-refresh.sh"] + 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: '\[ -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"' + # 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\)"' diff --git a/scripts/tests/image-refresh-repin-on-revert.bats b/scripts/tests/image-refresh-repin-on-revert.bats index 9ebfb874..e838ef2a 100644 --- a/scripts/tests/image-refresh-repin-on-revert.bats +++ b/scripts/tests/image-refresh-repin-on-revert.bats @@ -70,10 +70,18 @@ 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 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). @@ -82,6 +90,8 @@ run_branch() { 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" @@ -91,6 +101,8 @@ recorded="sha256:aaa" 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="" @@ -105,6 +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 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 @@ -113,15 +136,52 @@ 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:-}" "${6:-}" } -@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, $5="1"), then a helm + # re-render reverted the workload onto :tag -- the client-runtime#199 repair + # 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 [[ "$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 (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 + [[ "$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 "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)" {