From 58aabe3f0a238c2955b650d27b9e94c4c3b8c03b Mon Sep 17 00:00:00 2001 From: Syed Saqlain Date: Tue, 8 Sep 2026 18:24:41 +0400 Subject: [PATCH 1/8] fix(image-refresh): re-pin the digest when a helm re-render reverts the workload to :tag (client-runtime#199) image-refresh only re-pinned on a registry digest CHANGE. After `helm upgrade --reset-then-reuse-values` (the hourly auto-upgrade) re-renders the workload back to repo:tag and discards the `set image repo@digest` pin, the next tick saw `recorded == latest` and no-op`d -- leaving the workload on the bare tag (IfNotPresent), where a stale node :tag layer silently runs an OLD control-plane image. That is how a pre-#416 jobs-manager ran under a sealed egress netpol on the stg/prod fleets. Read the live workload image in the recorded==latest branch and re-pin when it is not repo@latest. shellcheck-clean; helm-unittest guard added. Co-Authored-By: Claude Opus 4.8 --- client/Chart.yaml | 4 +- client/templates/image-refresh-cronjob.yaml | 68 +++++++++++++++++++-- client/tests/image_refresh_test.yaml | 20 ++++++ 3 files changed, 86 insertions(+), 6 deletions(-) diff --git a/client/Chart.yaml b/client/Chart.yaml index 36f9beaf..99f71ce9 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.107 -appVersion: "1.9.107" +version: 1.9.108 +appVersion: "1.9.108" keywords: - tracebloc - kubernetes diff --git a/client/templates/image-refresh-cronjob.yaml b/client/templates/image-refresh-cronjob.yaml index 2b371ebe..c4944247 100644 --- a/client/templates/image-refresh-cronjob.yaml +++ b/client/templates/image-refresh-cronjob.yaml @@ -300,6 +300,38 @@ data: printf '%s\n' "$_json" | jq -r --arg k "$_key" '.metadata.annotations[$k] // empty' } + # The image reference the LIVE workload currently runs for $repo's primary + # container. Used to detect when `helm upgrade --reset-then-reuse-values` + # (the hourly auto-upgrade) has re-rendered the workload back to the + # chart's `repo:tag` and so DISCARDED an earlier `set image repo@digest` + # pin. That revert is invisible to the digest comparison below -- the + # annotation still equals the registry digest, so `recorded == latest` + # reads "unchanged" and never re-pins -- while the workload sits on the + # bare tag (IfNotPresent per tracebloc.controlPlanePullPolicy). On a node + # whose `:tag` layer is stale that silently runs an OLD control-plane image + # (backend#2896-adjacent; it ran a pre-#416 jobs-manager under a sealed + # egress netpol on the stg/prod fleets, client-runtime#199). Container + # names are contractual with the deployment/daemonset templates -- keep in + # sync with the `case` block in the reconcile loop below. An empty result + # (read error / container absent) makes the caller re-assert the pin rather + # than assume agreement (fail-safe, same stance as get_annotation). + workload_image_for_repo() { + case "$1" in + tracebloc/jobs-manager) + kubectl get deployment -n "$RELEASE_NAMESPACE" "$DEPLOYMENT_NAME" \ + -o jsonpath='{.spec.template.spec.containers[?(@.name=="api")].image}' \ + --request-timeout=15s 2>/dev/null ;; + tracebloc/pods-monitor) + kubectl get deployment -n "$RELEASE_NAMESPACE" "$DEPLOYMENT_NAME" \ + -o jsonpath='{.spec.template.spec.containers[?(@.name=="pods-monitor-container")].image}' \ + --request-timeout=15s 2>/dev/null ;; + tracebloc/resource-monitor) + kubectl get daemonset -n "$NODE_AGENTS_NAMESPACE" "$RESOURCE_MONITOR_DAEMONSET" \ + -o jsonpath='{.spec.template.spec.containers[?(@.name=="tracebloc-resource-monitor")].image}' \ + --request-timeout=15s 2>/dev/null ;; + esac + } + # Skip the whole tick if the deployment isn't currently SETTLED (#546). A rollout # already in progress, or a pod stuck (e.g. Pending on volume binding), means a restart # can't help — it only churns ReplicaSets, and on a single-node local-path cluster that @@ -525,11 +557,39 @@ data: fi if [ "$recorded" = "$latest" ]; then - log " digest unchanged since last refresh; no-op" - continue + # The registry digest has not moved since we recorded it -- but that + # alone does NOT prove the workload is running it. A `helm upgrade + # --reset-then-reuse-values` (the hourly auto-upgrade) re-renders the + # Deployment back to `repo:tag` and discards our `set image repo@digest` + # pin; with `recorded == latest` this used to no-op, leaving the workload + # on the bare tag until the NEXT registry publish -- and on a node whose + # `:tag` layer is stale that silently runs an OLD image (client-runtime#199: + # a pre-#416 jobs-manager under a sealed egress netpol). So re-assert the + # pin whenever the live workload is not already on `repo@latest`. + # + # DESIGN NOTE (for review): this also converges a FRESH install to a + # digest pin on the first tick where `recorded == latest` but the + # workload is still on `:tag` (i.e. ~one interval post-install, one extra + # rollout). That is a deliberate widening of the header's "first-tick + # contract" -- which recorded-without-re-imaging to avoid install churn, + # but by staying on `:tag` left exactly the steady-state stale-`:tag` + # exposure this fix closes. If the install-churn cost is unwanted, gate + # this re-pin on a per-image "have we ever applied a digest here?" marker + # so it fires only on genuine reverts (see PR discussion). + want="${IMAGE_REGISTRY}/${repo}@${latest}" + have="$(workload_image_for_repo "$repo" || true)" + if [ "$have" = "$want" ]; then + log " digest unchanged and workload already on the pinned digest; no-op" + continue + fi + log " digest unchanged, but the workload runs '${have:-}', not '${want}'" + log " -- a helm re-render reverted the pin onto :${IMAGE_TAG}; re-pinning the digest" + # Fall through to the re-image path (ref + case block) with recorded + # already == latest: the annotate below is an idempotent re-write, and + # restart_needed drives the rollout that puts the digest back on. + else + log " digest changed (${recorded} -> ${latest}); re-image needed" fi - - log " digest changed (${recorded} -> ${latest}); re-image needed" annotate_args="$annotate_args ${key}=${latest}" restart_needed=1 diff --git a/client/tests/image_refresh_test.yaml b/client/tests/image_refresh_test.yaml index 0e48630a..08468610 100644 --- a/client/tests/image_refresh_test.yaml +++ b/client/tests/image_refresh_test.yaml @@ -935,3 +935,23 @@ tests: - matchRegex: path: data["image-refresh.sh"] pattern: 'rm_set_args tracebloc-resource-monitor=' + + - it: reconcile re-pins when a helm re-render reverted the workload off the digest + # Guards the client-runtime#199 fix: with `recorded == latest` the loop must + # NOT unconditionally no-op -- it must read the live workload image and + # re-pin when it is not `repo@latest` (a `helm upgrade --reset-then-reuse-values` + # reverted the pin onto the bare :tag, where a stale node cache serves an old + # image). The helper + the fall-through into the re-image path are the fix. + template: templates/image-refresh-cronjob.yaml + documentIndex: 0 + asserts: + - matchRegex: + path: data["image-refresh.sh"] + pattern: 'workload_image_for_repo\(\)' + - matchRegex: + path: data["image-refresh.sh"] + pattern: 'have="\$\(workload_image_for_repo "\$repo"' + # the true no-op now requires BOTH digest-unchanged AND workload-on-digest + - matchRegex: + path: data["image-refresh.sh"] + pattern: 'workload already on the pinned digest; no-op' From e24fe6d1d20ca87bd3f921aaff8bcb97250011be Mon Sep 17 00:00:00 2001 From: Syed Saqlain Date: Wed, 9 Sep 2026 10:34:26 +0400 Subject: [PATCH 2/8] chore(chart): rebump to 1.9.109 (develop advanced to 1.9.108) Co-Authored-By: Claude Opus 4.8 --- client/Chart.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/Chart.yaml b/client/Chart.yaml index 99f71ce9..bdaf83cb 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.108 -appVersion: "1.9.108" +version: 1.9.109 +appVersion: "1.9.109" keywords: - tracebloc - kubernetes From 1c05afce441129e2ba9a3c275b6e6b31dac33ea7 Mon Sep 17 00:00:00 2001 From: Syed Saqlain Date: Wed, 9 Sep 2026 12:16:33 +0400 Subject: [PATCH 3/8] fix(image-refresh): re-pin the requests-proxy when only it reverts, not just the api container (backend#199) Bugbot Medium on #1008: the no-op decision for tracebloc/jobs-manager read only the deployment api container. When it already matched repo@latest the loop continued and never wrote rp_set_args -- so a tick that pinned the api then died before the requests-proxy rollout, or a helm re-render that reverted only the proxy, left the proxy on :tag and later ticks skipped it forever off the api match alone. Read the requests-proxy proxy container too (when it follows the jobs-manager digest) and fall through to the re-image path -- which re-derives both jm_set_args and rp_set_args -- when it is off want. Guard the fall-through logs so they stay accurate. Regression-guarded in the script test. Co-Authored-By: Claude Opus 4.8 --- client/templates/image-refresh-cronjob.yaml | 38 +++++++++++++++++++-- client/tests/image_refresh_test.yaml | 12 +++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/client/templates/image-refresh-cronjob.yaml b/client/templates/image-refresh-cronjob.yaml index c4944247..17aba5ca 100644 --- a/client/templates/image-refresh-cronjob.yaml +++ b/client/templates/image-refresh-cronjob.yaml @@ -332,6 +332,21 @@ data: esac } + # The requests-proxy is a SEPARATE deployment that runs the SAME + # tracebloc/jobs-manager image (container `proxy`). `workload_image_for_repo` + # above reads only the jobs-manager `api` container, so the no-op decision + # below cannot see the proxy on its own. Read it here so a proxy left on + # `:tag` -- a tick that pinned `api` then died before the rp rollout, or a + # helm re-render that reverted only the proxy -- is still re-pinned instead + # of being declared "unchanged" forever because `api` happens to match + # (Bugbot on #1008). Empty (read error / absent) re-asserts the pin, same + # fail-safe stance as `workload_image_for_repo`. + requests_proxy_image() { + kubectl get deployment -n "$RELEASE_NAMESPACE" "$REQUESTS_PROXY_DEPLOYMENT" \ + -o jsonpath='{.spec.template.spec.containers[?(@.name=="proxy")].image}' \ + --request-timeout=15s 2>/dev/null + } + # Skip the whole tick if the deployment isn't currently SETTLED (#546). A rollout # already in progress, or a pod stuck (e.g. Pending on volume binding), means a restart # can't help — it only churns ReplicaSets, and on a single-node local-path cluster that @@ -578,12 +593,29 @@ data: # so it fires only on genuine reverts (see PR discussion). want="${IMAGE_REGISTRY}/${repo}@${latest}" have="$(workload_image_for_repo "$repo" || true)" - if [ "$have" = "$want" ]; then + # jobs-manager: the requests-proxy runs this SAME image and is its own + # deployment. When it follows this digest (not operator-pinned) it must + # ALSO be on `want`, or a partial re-pin (api pinned, proxy still on + # :tag) is declared no-op forever off the api match alone and never + # retried (Bugbot on #1008). Any mismatch falls through to the re-image + # path, whose `case` block re-derives BOTH `jm_set_args`/`rp_set_args`. + proxy_off_digest=0 + if [ "$repo" = "tracebloc/jobs-manager" ] && [ "$REQUESTS_PROXY_PINNED" != "1" ]; then + rp_have="$(requests_proxy_image || true)" + [ "$rp_have" = "$want" ] || proxy_off_digest=1 + fi + if [ "$have" = "$want" ] && [ "$proxy_off_digest" = "0" ]; then log " digest unchanged and workload already on the pinned digest; no-op" continue fi - log " digest unchanged, but the workload runs '${have:-}', not '${want}'" - log " -- a helm re-render reverted the pin onto :${IMAGE_TAG}; re-pinning the digest" + if [ "$have" = "$want" ] && [ "$proxy_off_digest" = "1" ]; then + log " digest unchanged and jobs-manager already on the pinned digest, but" + log " deployment/${REQUESTS_PROXY_DEPLOYMENT} runs '${rp_have:-}', not '${want}'" + log " -- re-pinning the requests-proxy digest" + elif [ "$have" != "$want" ]; then + log " digest unchanged, but the workload runs '${have:-}', not '${want}'" + 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 # restart_needed drives the rollout that puts the digest back on. diff --git a/client/tests/image_refresh_test.yaml b/client/tests/image_refresh_test.yaml index 08468610..4a69b6b2 100644 --- a/client/tests/image_refresh_test.yaml +++ b/client/tests/image_refresh_test.yaml @@ -223,6 +223,18 @@ tests: - matchRegex: path: data["image-refresh.sh"] pattern: "pinned by digest in values" + # Regression guard (Bugbot #1008): the requests-proxy is a SEPARATE + # deployment running the SAME jobs-manager image, so the no-op "already on + # the pinned digest" decision MUST also read the proxy and fall through + # when it is off `want` -- else a partial re-pin (api pinned, proxy still + # on :tag) is declared unchanged forever off the api match alone and never + # retried. Lock both the reader and the check in place. + - matchRegex: + path: data["image-refresh.sh"] + pattern: 'requests_proxy_image\(\)' + - matchRegex: + path: data["image-refresh.sh"] + pattern: 'proxy_off_digest' # Regression guard: the script must HEAD the manifest with all # four Accept media types in a SINGLE comma-separated Accept # header per the Docker registry v2 spec (some proxies have been From 461eeaa961b145f40f0b5941fa113fab5255f043 Mon Sep 17 00:00:00 2001 From: Syed Saqlain Date: Wed, 9 Sep 2026 14:28:11 +0400 Subject: [PATCH 4/8] fix(image-refresh): compare the re-pin on the @sha256 digest, skip on unreadable, and align the docs (backend#199) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses LukasWodka's review on #1008. Ask 1 — a BEHAVIOURAL test, not text-presence. Adds scripts/tests/image-refresh-repin-on-revert.bats, which extracts the shipped `recorded == latest` branch from the rendered chart and drives it with the registry HEAD and the two live-workload reads stubbed: a `:tag` revert re-pins, api+proxy both on the digest no-ops, a proxy-only revert re-pins the proxy, an unreadable read skips the tick, and a registry-prefix rewrite is not a revert. Inverting the comparison reddens these, which the helm-unittest text asserts could not detect. Design question — persistent mismatch under a mutating image webhook. Compare on the @sha256 DIGEST, not the whole reference: a webhook that rewrites the registry prefix to an internal mirror keeps the digest, so a prefix-only rewrite no longer reads as a revert -- which otherwise re-pinned every tick and tripped the #563 flap lockout for all control-plane images after three ticks. A genuine revert to `:tag` carries no @sha256 and still re-pins. Nit — unreadable live image now SKIPS the re-pin this tick and retries, instead of re-asserting (which burned a #563 flap attempt on a healthy edge and logged a revert that may not have happened). The two helper comments are corrected to this fail-closed stance, and the "reverted the pin" log is neutral wording that also covers the fresh-install case. Ask 2 — the header no longer contradicts the code. Rewrites the HELM RE-RENDER limitation as HANDLED (bounded to one tick), the first-tick contract as "pins on the tick after first observation", and the two matching values.yaml bullets; and drops the "DESIGN NOTE (for review)" block from the shipped ConfigMap, since a customer operator cannot follow a PR thread. Co-Authored-By: Claude Opus 4.8 --- client/templates/image-refresh-cronjob.yaml | 148 ++++++++++------ client/tests/image_refresh_test.yaml | 11 +- client/values.yaml | 15 +- .../tests/image-refresh-repin-on-revert.bats | 166 ++++++++++++++++++ 4 files changed, 274 insertions(+), 66 deletions(-) create mode 100644 scripts/tests/image-refresh-repin-on-revert.bats diff --git a/client/templates/image-refresh-cronjob.yaml b/client/templates/image-refresh-cronjob.yaml index 17aba5ca..1c8bc660 100644 --- a/client/templates/image-refresh-cronjob.yaml +++ b/client/templates/image-refresh-cronjob.yaml @@ -41,33 +41,36 @@ data: # resource-monitor was likewise untouched (and a chart release does not # change its image ref either), so it only ever moved by accident. # - # TWO KNOWN, BOUNDED LIMITATIONS of keeping the annotation (rather than the - # live pod spec) as the source of truth. Both are self-healing at the next - # upstream image change, which at the control plane's release cadence is - # days, and neither can break a running edge — the pods stay offline-safe + # ONE KNOWN, BOUNDED LIMITATION of keeping the annotation (rather than the + # live pod spec) as the DIGEST source of truth. It is self-healing at the + # next upstream image change, which at the control plane's release cadence is + # days, and cannot break a running edge — the pods stay offline-safe # throughout because IfNotPresent does not depend on any of this. # - # 1. HELM RE-RENDER. `helm upgrade --reset-then-reuse-values` (the fleet - # auto-upgrade path) re-renders the templates, which write `repo:tag` - # and so revert an earlier `set image` pin. This tick will NOT re-pin: - # the annotation still records that digest, so `recorded == latest` - # and the loop no-ops. The edge floats on the tag until the next - # upstream release re-pins it. Note this only happens on a chart - # VERSION bump — auto-upgrade compares versions and skips otherwise — - # so it is not an hourly revert. + # PRE-EXISTING SKEW. requests-proxy and jobs-manager may already be + # running different builds of the same image on an edge upgrading INTO + # this version, because nothing reconciled requests-proxy before now. + # This script converges them on the next digest change (both are set + # in the same tick); it does not detect and repair skew that predates + # it, because it compares registry-vs-annotation, never pod-vs-pod. # - # 2. PRE-EXISTING SKEW. requests-proxy and jobs-manager may already be - # running different builds of the same image on an edge upgrading INTO - # this version, because nothing reconciled requests-proxy before now. - # This script converges them on the next digest change (both are set - # in the same tick); it does not detect and repair skew that predates - # it, because it compares registry-vs-annotation, never pod-vs-pod. + # HELM RE-RENDER, by contrast, is HANDLED rather than tolerated (backend#199). + # `helm upgrade --reset-then-reuse-values` (the fleet auto-upgrade path) + # re-renders the templates, which write `repo:tag` and so revert an earlier + # `set image` pin. `recorded == latest` no longer means no-op: the loop reads + # each workload's LIVE image and re-pins the digest whenever the workload is + # off it. The revert therefore lasts ONE tick — the next tick puts the digest + # back — instead of floating on the tag until the next upstream release. It + # is compared on the @sha256 digest, so a registry-prefix rewrite (a mutating + # webhook) is not mistaken for a revert. This only happens on a chart VERSION + # bump anyway — auto-upgrade compares versions and skips otherwise — so it is + # not an hourly revert. # - # Fixing either properly means reconciling against each workload's LIVE - # container image instead of a shared annotation — a declarative reconcile, - # which `set image` makes possible for the first time (`rollout restart` - # was a blind action, which is why the annotation existed at all). That is - # a deliberate follow-up, not an oversight. + # Reconciling against the live spec is what makes that possible — the + # declarative reconcile `set image` enables (`rollout restart` was a blind + # action, which is why the annotation existed at all). The remaining + # pre-existing-skew case would need pod-vs-pod comparison; that is a + # deliberate follow-up, not an oversight. # # Source of truth: annotations on the JOBS-MANAGER deployment metadata # (`tracebloc.io/last-refreshed--digest`) — comparing the registry @@ -91,14 +94,16 @@ data: # # First-tick contract: annotation missing → record the current registry # digest WITHOUT touching the workload (no evidence of drift, no reason to - # churn pods). #569 keeps this deliberately. Pinning on the first tick + # churn pods). #569 keeps this deliberately. Pinning on the FIRST tick # 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. The cost - # is that a freshly installed edge runs `repo:tag` until the first real - # digest change: still restart-safe offline (IfNotPresent), just not yet - # reproducible. Offline-safety is what #569 is fixing; reproducibility - # follows on the next upstream release. + # 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 (backend#199) — so a fresh edge + # becomes reproducible ~one interval post-install (one rollout), NOT "at the + # next upstream release". Between the two it is still restart-safe offline + # (IfNotPresent); offline-safety is what #569 fixed, reproducibility is what + # this completes. # # 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 @@ -313,8 +318,10 @@ data: # egress netpol on the stg/prod fleets, client-runtime#199). Container # names are contractual with the deployment/daemonset templates -- keep in # sync with the `case` block in the reconcile loop below. An empty result - # (read error / container absent) makes the caller re-assert the pin rather - # than assume agreement (fail-safe, same stance as get_annotation). + # (read error / container absent) makes the caller SKIP the re-pin this tick + # and retry -- NOT re-assert (which would burn a #563 flap attempt on a + # healthy edge) and NOT assume agreement. That is the fail-closed stance + # get_annotation and the settled guard already take. workload_image_for_repo() { case "$1" in tracebloc/jobs-manager) @@ -339,8 +346,8 @@ data: # `:tag` -- a tick that pinned `api` then died before the rp rollout, or a # helm re-render that reverted only the proxy -- is still re-pinned instead # of being declared "unchanged" forever because `api` happens to match - # (Bugbot on #1008). Empty (read error / absent) re-asserts the pin, same - # fail-safe stance as `workload_image_for_repo`. + # (Bugbot on #1008). Empty (read error / absent) makes the caller SKIP this + # tick, same fail-closed stance as `workload_image_for_repo`. requests_proxy_image() { kubectl get deployment -n "$RELEASE_NAMESPACE" "$REQUESTS_PROXY_DEPLOYMENT" \ -o jsonpath='{.spec.template.spec.containers[?(@.name=="proxy")].image}' \ @@ -580,41 +587,68 @@ data: # on the bare tag until the NEXT registry publish -- and on a node whose # `:tag` layer is stale that silently runs an OLD image (client-runtime#199: # a pre-#416 jobs-manager under a sealed egress netpol). So re-assert the - # pin whenever the live workload is not already on `repo@latest`. + # pin whenever the live workload is not already on the pinned digest. # - # DESIGN NOTE (for review): this also converges a FRESH install to a - # digest pin on the first tick where `recorded == latest` but the - # workload is still on `:tag` (i.e. ~one interval post-install, one extra - # rollout). That is a deliberate widening of the header's "first-tick - # contract" -- which recorded-without-re-imaging to avoid install churn, - # but by staying on `:tag` left exactly the steady-state stale-`:tag` - # exposure this fix closes. If the install-churn cost is unwanted, gate - # this re-pin on a per-image "have we ever applied a digest here?" marker - # so it fires only on genuine reverts (see PR discussion). - want="${IMAGE_REGISTRY}/${repo}@${latest}" + # 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. + # + # Compare on the @sha256 DIGEST, not the whole image reference: a mutating + # admission webhook that rewrites the registry PREFIX to an internal mirror + # (seen behind hospital proxies) keeps the digest, so a prefix-only rewrite + # must NOT read as a revert -- otherwise the ref never equals the pinned one, + # every tick re-pins, the webhook rewrites it again, and three ticks trip the + # #563 flap lockout for ALL control-plane images (LukasWodka on #1008). A + # genuine revert to `:tag` carries no `@sha256` suffix, so `${ref##*@}` (the + # digest for `repo@sha256:...`, the whole ref otherwise) still mismatches a + # bare `sha256:...` and re-pins. have="$(workload_image_for_repo "$repo" || true)" - # jobs-manager: the requests-proxy runs this SAME image and is its own + # jobs-manager: the requests-proxy runs this SAME image as its own # deployment. When it follows this digest (not operator-pinned) it must - # ALSO be on `want`, or a partial re-pin (api pinned, proxy still on - # :tag) is declared no-op forever off the api match alone and never - # retried (Bugbot on #1008). Any mismatch falls through to the re-image - # path, whose `case` block re-derives BOTH `jm_set_args`/`rp_set_args`. - proxy_off_digest=0 + # ALSO be on it, or a partial re-pin (api pinned, proxy still on :tag) is + # declared no-op forever off the api match alone and never retried (Bugbot + # on #1008). Any mismatch falls through to the re-image path, whose `case` + # block re-derives BOTH `jm_set_args`/`rp_set_args`. + proxy_follows=0 + rp_have="" if [ "$repo" = "tracebloc/jobs-manager" ] && [ "$REQUESTS_PROXY_PINNED" != "1" ]; then + proxy_follows=1 rp_have="$(requests_proxy_image || true)" - [ "$rp_have" = "$want" ] || proxy_off_digest=1 fi - if [ "$have" = "$want" ] && [ "$proxy_off_digest" = "0" ]; then + # Unreadable live image (read error / container absent): SKIP the re-pin + # this tick and retry, rather than re-assert. Re-asserting on an unreadable + # read would burn a #563 flap attempt on a possibly-healthy edge and log a + # revert that may not have happened -- the same fail-closed stance the + # SKIP_KEY read and the settled guard take. A real `:tag` ref is readable + # and is NOT this case; it falls through and re-pins. + if [ -z "$have" ]; then + log " digest unchanged, but the live ${repo} image is unreadable (API read error / container absent) -- skipping re-pin this tick, will retry when readable" + continue + fi + if [ "$proxy_follows" = "1" ] && [ -z "$rp_have" ]; then + log " digest unchanged, but the live requests-proxy image is unreadable (API read error / container absent) -- skipping re-pin this tick, will retry when readable" + continue + fi + api_on_digest=1 + [ "${have##*@}" = "$latest" ] || api_on_digest=0 + proxy_on_digest=1 + if [ "$proxy_follows" = "1" ]; then + [ "${rp_have##*@}" = "$latest" ] || proxy_on_digest=0 + fi + if [ "$api_on_digest" = "1" ] && [ "$proxy_on_digest" = "1" ]; then log " digest unchanged and workload already on the pinned digest; no-op" continue fi - if [ "$have" = "$want" ] && [ "$proxy_off_digest" = "1" ]; then + if [ "$api_on_digest" = "1" ]; then log " digest unchanged and jobs-manager already on the pinned digest, but" - log " deployment/${REQUESTS_PROXY_DEPLOYMENT} runs '${rp_have:-}', not '${want}'" + log " deployment/${REQUESTS_PROXY_DEPLOYMENT} runs '${rp_have}', not digest ${latest}" log " -- re-pinning the requests-proxy digest" - elif [ "$have" != "$want" ]; then - log " digest unchanged, but the workload runs '${have:-}', not '${want}'" - log " -- a helm re-render reverted the pin onto :${IMAGE_TAG}; re-pinning the 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" fi # Fall through to the re-image path (ref + case block) with recorded # already == latest: the annotate below is an idempotent re-write, and diff --git a/client/tests/image_refresh_test.yaml b/client/tests/image_refresh_test.yaml index 4a69b6b2..3787fbc7 100644 --- a/client/tests/image_refresh_test.yaml +++ b/client/tests/image_refresh_test.yaml @@ -226,15 +226,18 @@ tests: # Regression guard (Bugbot #1008): the requests-proxy is a SEPARATE # deployment running the SAME jobs-manager image, so the no-op "already on # the pinned digest" decision MUST also read the proxy and fall through - # when it is off `want` -- else a partial re-pin (api pinned, proxy still - # on :tag) is declared unchanged forever off the api match alone and never - # retried. Lock both the reader and the check in place. + # when it is off the digest -- else a partial re-pin (api pinned, proxy + # still on :tag) is declared unchanged forever off the api match alone and + # never retried. Lock the reader and the per-workload digest check in + # place. (The BEHAVIOUR -- that an inverted check reddens -- is asserted in + # scripts/tests/image-refresh-repin-on-revert.bats; this only pins that the + # two pieces still exist in the shipped script.) - matchRegex: path: data["image-refresh.sh"] pattern: 'requests_proxy_image\(\)' - matchRegex: path: data["image-refresh.sh"] - pattern: 'proxy_off_digest' + pattern: 'proxy_on_digest' # Regression guard: the script must HEAD the manifest with all # four Accept media types in a SINGLE comma-separated Accept # header per the Docker registry v2 spec (some proxies have been diff --git a/client/values.yaml b/client/values.yaml index 4dbebf55..dab7834c 100644 --- a/client/values.yaml +++ b/client/values.yaml @@ -1718,11 +1718,16 @@ autoUpgrade: # - First observation (annotation absent on a fresh install): record # the current digest without re-imaging. Rewriting repo:tag to # repo@digest for byte-identical content would roll every workload — -# including the DaemonSet on every node — for nothing. The cost is -# that a fresh edge runs repo:tag until the first real digest change: -# still restart-safe offline, just not yet reproducible. -# - Idle-cheap: when the recorded digest matches today's digest, the -# script exits without touching anything. Steady state is one HEAD +# including the DaemonSet on every node — for nothing. The NEXT tick, +# seeing the digest recorded but the workload still on repo:tag, pins +# it (backend#199) — so a fresh edge becomes reproducible ~one interval +# post-install, not at the next upstream release. Restart-safe offline +# throughout. +# - Idle-cheap: when the recorded digest matches today's digest AND the +# workload already runs that digest, the script exits without touching +# anything. If a helm re-render reverted the pin back to repo:tag it +# re-pins that one tick (compared on the @sha256 digest, so a mirror +# prefix rewrite is not mistaken for a revert). Steady state is one HEAD # per image per tick, well under Docker Hub's 100/6h anonymous # pull-rate limit. # - Private mirrors (global.imageRegistry): the script resolves digests diff --git a/scripts/tests/image-refresh-repin-on-revert.bats b/scripts/tests/image-refresh-repin-on-revert.bats new file mode 100644 index 00000000..c7f9c5a7 --- /dev/null +++ b/scripts/tests/image-refresh-repin-on-revert.bats @@ -0,0 +1,166 @@ +#!/usr/bin/env bats +# image-refresh RE-PINS the digest when a helm re-render reverted the workload +# to `repo:tag`, instead of no-op'ing off the annotation alone. +# +# backend#199. `recorded == latest` proves the REGISTRY digest has not moved; it +# does NOT prove the workload is running it. `helm upgrade --reset-then-reuse-values` +# (the fleet auto-upgrade) re-renders the Deployment back to `repo:tag` and discards +# an earlier `set image repo@digest` pin -- and on a node whose `:tag` layer is +# stale that silently runs an OLD control-plane image (client-runtime#199). So the +# loop reads each workload's LIVE image and re-pins whenever it is off the digest. +# +# These assert BEHAVIOUR, not text presence: the earlier helm-unittest checks that +# `workload_image_for_repo`/`have=`/`proxy_off_digest` merely APPEAR in the script +# still pass if the comparison is inverted. This extracts the shipped branch from +# the RENDERED chart and drives it with the registry + live-workload reads stubbed, +# so an inverted comparison reddens. + +setup() { + TMP="$(mktemp -d)" + CHART="${BATS_TEST_DIRNAME}/../../client" + helm template t "$CHART" --set clientId=x --set clientPassword=y \ + --set storageClass.create=false > "$TMP/rendered.yaml" + python3 - "$TMP/rendered.yaml" "$TMP/branch.sh" <<'PYX' +import sys + +try: + import yaml +except ImportError: + sys.exit("[ERROR] PyYAML required (pip install pyyaml)") + +MARKER = "already on the pinned digest; no-op" + +def walk(o): + if isinstance(o, str) and MARKER in o: + return o + if isinstance(o, dict): + for v in o.values(): + r = walk(v) + if r: + return r + if isinstance(o, list): + for v in o: + r = walk(v) + if r: + return r + +script = None +for d in yaml.safe_load_all(open(sys.argv[1])): + if not d: + continue + script = walk(d) + if script: + break +assert script, "no rendered script containing the re-pin branch" + +lines = script.splitlines() +start = next(i for i, l in enumerate(lines) + if l.strip() == 'if [ "$recorded" = "$latest" ]; then') +# The branch ends at the `esac` that closes the `case "$repo" in` re-image block +# (no nested `case`, so the first `esac` after it closes it). +case_at = next(i for i in range(start, len(lines)) + if lines[i].strip() == 'case "$repo" in') +end = next(i for i in range(case_at, len(lines)) if lines[i].strip() == "esac") +body = "\n".join(l[6:] if l.startswith(" " * 6) else l.lstrip() + for l in lines[start:end + 1]) +open(sys.argv[2], "w").write(body) +PYX +} +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 +# +# 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). +run_branch() { + cat > "$TMP/harness.sh" < Date: Wed, 9 Sep 2026 14:44:42 +0400 Subject: [PATCH 5/8] fix(image-refresh): surface a re-pin whose rollout never settled, don't no-op it (backend#199) Bugbot High on #1008: a re-pin does `set image` (which moves the spec to repo@digest immediately) then waits on `rollout status`. If that rollout times out on requests-proxy or the resource-monitor DaemonSet -- both OUTSIDE the top-of-tick settled guard, which only covers jobs-manager -- the tick exits under `set -e` with ATTEMPT_KEY still raised, and the next tick's live-image check reads the (already-moved) spec as on-digest and no-ops. The stuck rollout is never retried and stale :tag pods keep running while the CronJob stays green. The digest-CHANGED path self-heals here (recorded stays old until the rollout settles, so it retries); the re-pin path could not, because recorded is already == latest. Read ATTEMPT_KEY once before the loop: a raised value means a prior re-image never reached its success-reset, so the "both on digest" no-op re-enters the re-image path instead of continuing. `rollout status` then runs again -- a settled workload resets the counter (idempotent `set image`, one fast status call), a genuinely stuck one advances the counter to the #563 flap lockout, which SURFACES it rather than hiding it. Best-effort read: unreadable/absent is treated as 0, since the restart block's own read stays the fail-closed authority. Adds two bats cases: on-digest + raised ATTEMPT_KEY re-runs the rollout (restart_needed=1, no no-op); on-digest + no pending attempt stays a clean no-op. Co-Authored-By: Claude Opus 4.8 --- client/templates/image-refresh-cronjob.yaml | 38 +++++++++++++++++-- .../tests/image-refresh-repin-on-revert.bats | 25 +++++++++++- 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/client/templates/image-refresh-cronjob.yaml b/client/templates/image-refresh-cronjob.yaml index 1c8bc660..a91e8d7c 100644 --- a/client/templates/image-refresh-cronjob.yaml +++ b/client/templates/image-refresh-cronjob.yaml @@ -463,6 +463,23 @@ data: rp_set_args="" rm_set_args="" + # An UNFINISHED re-image attempt (backend#199, Bugbot High on #1008). The + # restart block below increments ATTEMPT_KEY BEFORE the rollout and only + # resets it on a settled one; a rollout that times out exits the tick under + # set -e with the counter still raised. `kubectl set image` has by then + # updated the SPEC to repo@digest, so the live-image check reads "on digest" + # even though the rollout never completed -- and requests-proxy / + # resource-monitor sit OUTSIDE the top-of-tick settled guard, so a stuck + # rollout on either would no-op here forever, the counter raised and stale + # :tag pods still running. Read the counter once up front: a raised value + # forces the no-op branch to re-enter the re-image path so `rollout status` + # is retried -- resolving it (success resets the counter) or advancing it to + # the #563 flap lockout, which SURFACES the stuck rollout rather than hiding + # it. Best-effort: an unreadable/absent counter is treated as 0 (no forced + # retry), since the restart block's own read is the fail-closed authority. + pending_attempt="$(get_annotation "$ATTEMPT_KEY" || true)" + case "$pending_attempt" in ''|*[!0-9]*) pending_attempt=0 ;; esac + # Each entry: "|||". set -- \ "tracebloc/jobs-manager|tracebloc.io/last-refreshed-jobs-manager-digest|${JOBS_MANAGER_PINNED}|${JOBS_MANAGER_PIN:-}" \ @@ -639,10 +656,23 @@ data: [ "${rp_have##*@}" = "$latest" ] || proxy_on_digest=0 fi if [ "$api_on_digest" = "1" ] && [ "$proxy_on_digest" = "1" ]; then - log " digest unchanged and workload already on the pinned digest; no-op" - continue - fi - if [ "$api_on_digest" = "1" ]; then + if [ "$pending_attempt" -gt 0 ]; then + # The SPEC reads on-digest, but a prior re-image attempt never reached + # its success-reset (ATTEMPT_KEY is raised): its rollout timed out and + # `set image` had already moved the spec, so this "on digest" can be a + # rollout that never settled -- and for requests-proxy / resource-monitor + # nothing else would catch it (they are outside the settled guard). + # Re-enter the re-image path so `rollout status` runs again: a settled + # workload resets the counter, a stuck one advances it to the #563 flap + # lockout, which surfaces it (Bugbot High on #1008). Re-`set image` with + # the same ref is an idempotent no-op patch, so a genuinely-settled + # workload pays only one fast `rollout status`. + log " workload spec is on the pinned digest, but ATTEMPT_KEY=${pending_attempt} marks an unfinished re-image (a rollout that never settled) -- re-running the rollout to resolve it or surface it via the flap guard" + else + log " digest unchanged and workload already on the pinned digest; no-op" + continue + fi + elif [ "$api_on_digest" = "1" ]; then log " digest unchanged and jobs-manager already on the pinned digest, but" log " deployment/${REQUESTS_PROXY_DEPLOYMENT} runs '${rp_have}', not digest ${latest}" log " -- re-pinning the requests-proxy digest" diff --git a/scripts/tests/image-refresh-repin-on-revert.bats b/scripts/tests/image-refresh-repin-on-revert.bats index c7f9c5a7..04c4a802 100644 --- a/scripts/tests/image-refresh-repin-on-revert.bats +++ b/scripts/tests/image-refresh-repin-on-revert.bats @@ -73,6 +73,7 @@ teardown() { rm -rf "$TMP"; } # $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) # # 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). @@ -89,6 +90,7 @@ latest="sha256:aaa" recorded="sha256:aaa" STUB_API="\${1:-}" STUB_PROXY="\${2:-}" +pending_attempt="\${4:-0}" restart_needed=0 annotate_args="" jm_set_args="" @@ -105,7 +107,7 @@ 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:-}" + sh "$TMP/harness.sh" "${1:-}" "${2:-}" "${3:-}" "${4:-0}" } @test "workload reverted to :tag re-pins the digest (restart_needed=1)" { @@ -126,6 +128,27 @@ EOF [[ "$output" == *"JM:"* ]] && [[ "$output" != *"JM:api="* ]] || return 1 } +@test "on-digest with an UNFINISHED attempt re-runs the rollout, not a silent no-op" { + # A prior re-pin's rollout timed out on requests-proxy / resource-monitor (both + # outside the settled guard): the spec reads on-digest but ATTEMPT_KEY is still + # raised. This must re-enter the re-image path so rollout status retries and the + # flap guard can surface a genuinely-stuck rollout (Bugbot High on #1008). + run run_branch "docker.io/tracebloc/jobs-manager@sha256:aaa" \ + "docker.io/tracebloc/jobs-manager@sha256:aaa" "0" "2" + [ "$status" -eq 0 ] || return 1 + [[ "$output" == *"unfinished re-image"* ]] || return 1 + [[ "$output" == *"RESTART:1"* ]] || return 1 + [[ "$output" != *"; no-op"* ]] || return 1 +} + +@test "on-digest with NO pending attempt is still a clean no-op" { + run run_branch "docker.io/tracebloc/jobs-manager@sha256:aaa" \ + "docker.io/tracebloc/jobs-manager@sha256:aaa" "0" "0" + [ "$status" -eq 0 ] || return 1 + [[ "$output" == *"; no-op"* ]] || return 1 + [[ "$output" == *"RESTART:0"* ]] || return 1 +} + @test "api on digest but proxy reverted re-pins the PROXY, not the api" { run run_branch "docker.io/tracebloc/jobs-manager@sha256:aaa" \ "docker.io/tracebloc/jobs-manager:dev" "0" From ea89c76159f95a4b5da055fcbbd71b52a2997168 Mon Sep 17 00:00:00 2001 From: Syed Saqlain Date: Wed, 9 Sep 2026 16:54:29 +0400 Subject: [PATCH 6/8] fix(image-refresh): gate the forced retry below MAX so a latched flap stops suppressing the annotation write (backend#199) blocking 1+2 (@shujaatTracebloc on #1008): the pending_attempt>0 forced re-run is a no-op once ATTEMPT_KEY latches -- the flap guard exit 0s before any set image/rollout status -- and worse, every latched tick then skips the annotation write (first-observation records, stale-pin clears) forever. Gate it on pending_attempt < MAX_REFRESH_ATTEMPTS so a latched image falls to the no-op path and the tick completes. blocking 3: state the fresh-install re-pin cost honestly in the header (the resource-monitor DaemonSet rollout can latch the shared lockout on a NotReady-node fleet; jobs-manager Recreate downtime). bats 9/9. Co-Authored-By: Claude Opus 4.8 --- client/templates/image-refresh-cronjob.yaml | 33 ++++++++++++++++--- .../tests/image-refresh-repin-on-revert.bats | 15 +++++++++ 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/client/templates/image-refresh-cronjob.yaml b/client/templates/image-refresh-cronjob.yaml index a91e8d7c..f3289fe9 100644 --- a/client/templates/image-refresh-cronjob.yaml +++ b/client/templates/image-refresh-cronjob.yaml @@ -100,10 +100,23 @@ data: # 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 (backend#199) — so a fresh edge - # becomes reproducible ~one interval post-install (one rollout), NOT "at the - # next upstream release". Between the two it is still restart-safe offline - # (IfNotPresent); offline-safety is what #569 fixed, reproducibility is what - # this completes. + # becomes reproducible ~one interval post-install, NOT "at the next upstream + # release". Between the two it is still restart-safe offline (IfNotPresent). + # + # COST, stated honestly (@shujaatTracebloc / @LukasWodka on #1008): that + # 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 + # (~45 min at the default 15m schedule) latch the SHARED MAX_REFRESH_ATTEMPTS + # 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. # # 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 @@ -656,7 +669,17 @@ data: [ "${rp_have##*@}" = "$latest" ] || proxy_on_digest=0 fi if [ "$api_on_digest" = "1" ] && [ "$proxy_on_digest" = "1" ]; then - if [ "$pending_attempt" -gt 0 ]; then + if [ "$pending_attempt" -gt 0 ] && [ "$pending_attempt" -lt "$MAX_REFRESH_ATTEMPTS" ]; then + # Only re-enter the rollout while there is budget left to resolve it. + # Once ATTEMPT_KEY has reached MAX_REFRESH_ATTEMPTS the flap guard below + # annotates FLAP_KEY and `exit 0`s BEFORE any `set image`/`rollout status` + # runs, so a latched forced-retry resolves nothing and surfaces nothing -- + # it only forces restart_needed=1 and skips the rest of the tick, dropping + # the annotation write (first-observation records, stale-pin clears) every + # tick, forever. Gating on `< MAX` lets a latched image fall to the no-op + # branch so the tick completes and its annotations land (@shujaatTracebloc + # on #1008, blocking 1 & 2). + # # The SPEC reads on-digest, but a prior re-image attempt never reached # its success-reset (ATTEMPT_KEY is raised): its rollout timed out and # `set image` had already moved the spec, so this "on digest" can be a diff --git a/scripts/tests/image-refresh-repin-on-revert.bats b/scripts/tests/image-refresh-repin-on-revert.bats index 04c4a802..815f7a3c 100644 --- a/scripts/tests/image-refresh-repin-on-revert.bats +++ b/scripts/tests/image-refresh-repin-on-revert.bats @@ -91,6 +91,7 @@ recorded="sha256:aaa" STUB_API="\${1:-}" STUB_PROXY="\${2:-}" pending_attempt="\${4:-0}" +MAX_REFRESH_ATTEMPTS=3 restart_needed=0 annotate_args="" jm_set_args="" @@ -149,6 +150,20 @@ EOF [[ "$output" == *"RESTART:0"* ]] || return 1 } +@test "on-digest with a LATCHED attempt (>= MAX) is a no-op, not a forced re-run" { + # Once ATTEMPT_KEY has reached MAX_REFRESH_ATTEMPTS the flap guard downstream + # annotates FLAP_KEY and exit 0s BEFORE any set image / rollout status, so a + # forced re-run there resolves nothing and, worse, skips the tick's annotation + # write forever. The branch must fall to the no-op path instead + # (@shujaatTracebloc on #1008, blocking 1 & 2). MAX is 3, so pending=3 latches. + run run_branch "docker.io/tracebloc/jobs-manager@sha256:aaa" \ + "docker.io/tracebloc/jobs-manager@sha256:aaa" "0" "3" + [ "$status" -eq 0 ] || return 1 + [[ "$output" == *"; no-op"* ]] || return 1 + [[ "$output" == *"RESTART:0"* ]] || return 1 + [[ "$output" != *"unfinished re-image"* ]] || return 1 +} + @test "api on digest but proxy reverted re-pins the PROXY, not the api" { run run_branch "docker.io/tracebloc/jobs-manager@sha256:aaa" \ "docker.io/tracebloc/jobs-manager:dev" "0" From 79b61601fd0d5bef54e5ba3dcc6f332bf5b8f8ab Mon Sep 17 00:00:00 2001 From: Syed Saqlain Date: Wed, 9 Sep 2026 22:39:46 +0400 Subject: [PATCH 7/8] fix(image-refresh): surface a latched flap (WARN + FLAP_KEY) on the on-digest no-op tick (client-runtime#199) With the < MAX gate, a latched tick (pending_attempt >= MAX_REFRESH_ATTEMPTS) keeps restart_needed=0 and never enters the downstream flap guard -- the only other writer of FLAP_KEY and the MANUAL ATTENTION WARN. Refresh is then dead for ALL control-plane images while the CronJob stays green, which #1964 forbids ("images did not update" must never be inferable only from the Job colour). Emit the WARN naming the refresh-attempt clear and annotate FLAP_KEY in the latched arm before the no-op continue. Two bats cases pin it (WARN+FLAP at pending=3; silence at pending client-runtime#199 (public repo, correct issue), proxy_off_digest typo, and the "hourly auto-upgrade" overstatement. Co-Authored-By: Claude Opus 4.8 --- client/templates/image-refresh-cronjob.yaml | 24 +++++++++--- client/values.yaml | 2 +- .../tests/image-refresh-repin-on-revert.bats | 37 ++++++++++++++++++- 3 files changed, 55 insertions(+), 8 deletions(-) diff --git a/client/templates/image-refresh-cronjob.yaml b/client/templates/image-refresh-cronjob.yaml index f3289fe9..d664eab1 100644 --- a/client/templates/image-refresh-cronjob.yaml +++ b/client/templates/image-refresh-cronjob.yaml @@ -54,7 +54,7 @@ data: # in the same tick); it does not detect and repair skew that predates # it, because it compares registry-vs-annotation, never pod-vs-pod. # - # HELM RE-RENDER, by contrast, is HANDLED rather than tolerated (backend#199). + # HELM RE-RENDER, by contrast, is HANDLED rather than tolerated (client-runtime#199). # `helm upgrade --reset-then-reuse-values` (the fleet auto-upgrade path) # re-renders the templates, which write `repo:tag` and so revert an earlier # `set image` pin. `recorded == latest` no longer means no-op: the loop reads @@ -99,7 +99,7 @@ data: # 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 (backend#199) — so a fresh edge + # 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). # @@ -320,7 +320,7 @@ data: # The image reference the LIVE workload currently runs for $repo's primary # container. Used to detect when `helm upgrade --reset-then-reuse-values` - # (the hourly auto-upgrade) has re-rendered the workload back to the + # (the auto-upgrade) has re-rendered the workload back to the # chart's `repo:tag` and so DISCARDED an earlier `set image repo@digest` # pin. That revert is invisible to the digest comparison below -- the # annotation still equals the registry digest, so `recorded == latest` @@ -476,7 +476,7 @@ data: rp_set_args="" rm_set_args="" - # An UNFINISHED re-image attempt (backend#199, Bugbot High on #1008). The + # An UNFINISHED re-image attempt (client-runtime#199, Bugbot High on #1008). The # restart block below increments ATTEMPT_KEY BEFORE the rollout and only # resets it on a settled one; a rollout that times out exits the tick under # set -e with the counter still raised. `kubectl set image` has by then @@ -611,7 +611,7 @@ data: if [ "$recorded" = "$latest" ]; then # The registry digest has not moved since we recorded it -- but that # alone does NOT prove the workload is running it. A `helm upgrade - # --reset-then-reuse-values` (the hourly auto-upgrade) re-renders the + # --reset-then-reuse-values` (the auto-upgrade) re-renders the # Deployment back to `repo:tag` and discards our `set image repo@digest` # pin; with `recorded == latest` this used to no-op, leaving the workload # on the bare tag until the NEXT registry publish -- and on a node whose @@ -692,6 +692,20 @@ data: # workload pays only one fast `rollout status`. log " workload spec is on the pinned digest, but ATTEMPT_KEY=${pending_attempt} marks an unfinished re-image (a rollout that never settled) -- re-running the rollout to resolve it or surface it via the flap guard" else + # Latched on-digest: the spec is on the pinned digest but a prior + # re-image never reset ATTEMPT_KEY, and the `< MAX` gate above now + # keeps restart_needed=0 so the flap guard below (the only other + # writer of FLAP_KEY / the MANUAL ATTENTION WARN) never runs on this + # tick. Surface the latch HERE, mirroring that guard, so a + # stopped-and-silent refresh is never inferable only from the + # CronJob's green (#1964): without this the tick would log a bare + # "no-op" on the exact tick refresh is dead for ALL control-plane + # images (@shujaatTracebloc / @LukasWodka / @saadqbal on #1008). + if [ "$pending_attempt" -ge "$MAX_REFRESH_ATTEMPTS" ]; then + log " WARN: workload is on the pinned digest but ${ATTEMPT_KEY}=${pending_attempt} (>= MAX_REFRESH_ATTEMPTS=${MAX_REFRESH_ATTEMPTS}) -- FLAP LATCHED: image refresh is STOPPED for ALL control-plane images and does not auto-resume. MANUAL ATTENTION NEEDED: clear the ${ATTEMPT_KEY} annotation on deployment/${DEPLOYMENT_NAME} to re-arm refresh." + kubectl annotate deployment -n "$RELEASE_NAMESPACE" "$DEPLOYMENT_NAME" \ + "${FLAP_KEY}=${pending_attempt}" --overwrite --request-timeout=15s + fi log " digest unchanged and workload already on the pinned digest; no-op" continue fi diff --git a/client/values.yaml b/client/values.yaml index dab7834c..564c0962 100644 --- a/client/values.yaml +++ b/client/values.yaml @@ -1720,7 +1720,7 @@ autoUpgrade: # repo@digest for byte-identical content would roll every workload — # including the DaemonSet on every node — for nothing. The NEXT tick, # seeing the digest recorded but the workload still on repo:tag, pins -# it (backend#199) — so a fresh edge becomes reproducible ~one interval +# it (client-runtime#199) — so a fresh edge becomes reproducible ~one interval # post-install, not at the next upstream release. Restart-safe offline # throughout. # - Idle-cheap: when the recorded digest matches today's digest AND the diff --git a/scripts/tests/image-refresh-repin-on-revert.bats b/scripts/tests/image-refresh-repin-on-revert.bats index 815f7a3c..9ebfb874 100644 --- a/scripts/tests/image-refresh-repin-on-revert.bats +++ b/scripts/tests/image-refresh-repin-on-revert.bats @@ -2,7 +2,7 @@ # image-refresh RE-PINS the digest when a helm re-render reverted the workload # to `repo:tag`, instead of no-op'ing off the annotation alone. # -# backend#199. `recorded == latest` proves the REGISTRY digest has not moved; it +# client-runtime#199. `recorded == latest` proves the REGISTRY digest has not moved; it # does NOT prove the workload is running it. `helm upgrade --reset-then-reuse-values` # (the fleet auto-upgrade) re-renders the Deployment back to `repo:tag` and discards # an earlier `set image repo@digest` pin -- and on a node whose `:tag` layer is @@ -10,7 +10,7 @@ # loop reads each workload's LIVE image and re-pins whenever it is off the digest. # # These assert BEHAVIOUR, not text presence: the earlier helm-unittest checks that -# `workload_image_for_repo`/`have=`/`proxy_off_digest` merely APPEAR in the script +# `workload_image_for_repo`/`have=`/`proxy_on_digest` merely APPEAR in the script # still pass if the comparison is inverted. This extracts the shipped branch from # the RENDERED chart and drives it with the registry + live-workload reads stubbed, # so an inverted comparison reddens. @@ -97,7 +97,12 @@ annotate_args="" jm_set_args="" rp_set_args="" rm_set_args="" +RELEASE_NAMESPACE="tracebloc" +DEPLOYMENT_NAME="jobs-manager" +ATTEMPT_KEY="tracebloc.io/refresh-attempt" +FLAP_KEY="tracebloc.io/refresh-flap-detected" 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"; } for _once in 1; do @@ -164,6 +169,34 @@ EOF [[ "$output" != *"unfinished re-image"* ]] || return 1 } +@test "a LATCHED tick still SURFACES the stopped refresh (WARN + FLAP_KEY), not a bare no-op" { + # With the `< MAX` gate a latched tick keeps restart_needed=0 and never enters + # the downstream flap guard -- the only other writer of FLAP_KEY and the MANUAL + # ATTENTION WARN. So refresh is dead for ALL control-plane images while the + # CronJob stays green, and #1964 forbids "images did not update" being + # inferable only from the Job's colour. The latched arm must itself emit the + # WARN naming the refresh-attempt clear and annotate FLAP_KEY before the no-op + # (@shujaatTracebloc / @LukasWodka / @saadqbal on #1008). MAX is 3. + run run_branch "docker.io/tracebloc/jobs-manager@sha256:aaa" \ + "docker.io/tracebloc/jobs-manager@sha256:aaa" "0" "3" + [ "$status" -eq 0 ] || return 1 + [[ "$output" == *"FLAP LATCHED"* ]] || return 1 + [[ "$output" == *"MANUAL ATTENTION NEEDED"* ]] || return 1 + [[ "$output" == *"clear the tracebloc.io/refresh-attempt annotation"* ]] || return 1 + [[ "$output" == *"KUBECTL:annotate deployment"*"tracebloc.io/refresh-flap-detected=3"* ]] || return 1 +} + +@test "a NON-latched no-op (pending=MAX; a clean on-digest + # tick (pending=0) and a bounded-attempt tick must not annotate FLAP_KEY. + run run_branch "docker.io/tracebloc/jobs-manager@sha256:aaa" \ + "docker.io/tracebloc/jobs-manager@sha256:aaa" "0" "0" + [ "$status" -eq 0 ] || return 1 + [[ "$output" == *"; no-op"* ]] || return 1 + [[ "$output" != *"FLAP LATCHED"* ]] || return 1 + [[ "$output" != *"KUBECTL:"* ]] || return 1 +} + @test "api on digest but proxy reverted re-pins the PROXY, not the api" { run run_branch "docker.io/tracebloc/jobs-manager@sha256:aaa" \ "docker.io/tracebloc/jobs-manager:dev" "0" From eafc68c2e5cf91f51d877a621094edcc00769813 Mon Sep 17 00:00:00 2001 From: Syed Saqlain Date: Thu, 10 Sep 2026 10:48:41 +0400 Subject: [PATCH 8/8] chore(image-refresh): rebump chart to 1.9.110 (develop took 1.9.109 via client#1017) (client-runtime#199) Co-Authored-By: Claude Opus 4.8 --- client/Chart.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/Chart.yaml b/client/Chart.yaml index bdaf83cb..de42aa09 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.109 -appVersion: "1.9.109" +version: 1.9.110 +appVersion: "1.9.110" keywords: - tracebloc - kubernetes