fix(image-refresh): gate the fresh-install re-pin roll on a digest-applied marker

[saqlainsyed007]

Summary

On recorded == latest with the workload on :tag, the image-refresh loop could not distinguish a fresh install (first-observation recorded a digest but applied nothing) from a helm re-render that reverted an already-applied pin — so every fresh install paid the shared #563 flap path (a rollout status on the resource-monitor DaemonSet, whose desiredNumberScheduled counts every node under tolerations: Exists) plus a strategy: Recreate downtime on jobs-manager, for byte-identical content the install had just pulled.

The gate skips the fresh-install roll only on positive evidence, via two durable per-image markers:

• tracebloc.io/first-observed-<image> — stamped by the first-observation arm (we watched this workload born on :tag here).
• tracebloc.io/digest-applied-<image> — stamped alongside the digest record only after a re-image rollout succeeds.

Decision in the off-digest arm:

• first_observed present and applied absent → fresh install → leave on :tag (IfNotPresent-safe); the first genuine upstream digest change pins it.
• applied present → established edge → roll (the client-runtime#199 helm-revert repair).
• neither marker → pre-marker / legacy edge → roll (repair), then stamp applied. This is the key correction from review: the upgrade shipping this chart reverts the whole existing fleet to :tag, so skipping on marker absence alone would strand every pre-marker edge on a possibly-stale :tag until the next upstream digest change. A marker read error (vs genuine absence) skips the tick and retries — fail-closed.

Related

Split from #1008 (item 2 — the fresh-install digest gate). Item 1 from that split (the latched-flap Pass-0 annotation drop on an off-digest tick) is a separate fix and is not in this PR.

Type of change

• Bug fix

Test plan

• bats scripts/tests/image-refresh-repin-on-revert.bats — 13/13. Cases for all three shapes: fresh (first-observed only → no roll), established (applied → roll), legacy (neither → repair roll). Harness gained a 6th arg + a get_annotation stub modelling absent-vs-error.
• helm unittest ./client — 729/729 (image_refresh guard updated to the two-marker code shapes).
• make check — green. shellcheck -s sh on the rendered script — unchanged (only the pre-existing intentional SC2086 word-split notes).
• Merged current develop (which had advanced to chart 1.9.111); bumped client/Chart.yaml → 1.9.112 for the chart-version guard.

Deployment notes

Behavioural change scoped to fresh installs only: a freshly-installed edge stays on the floating :tag (IfNotPresent) instead of being pinned to a digest ~one interval post-install; it is pinned on the first genuine upstream digest change. Established and pre-marker/legacy edges are unaffected — the client-runtime#199 helm-revert repair still runs for both.

Checklist

• Tests added / updated and passing locally
• Docs updated if behavior or config changed (header + in-script comments rewritten for the two-marker gate)
• No secrets / credentials in the diff
• Cross-repo issues use owner-qualified form

---

Note

Medium Risk
Changes control-plane image reconciliation and rollout gating fleet-wide; logic is heavily tested but a mistake could skip needed re-pins or trigger extra DaemonSet/jobs-manager rollouts on upgrade.

Overview
Fixes image-refresh so fresh installs no longer pay an expensive digest re-pin rollout when the registry digest is unchanged but the workload still runs the floating :tag.

The CronJob script now uses two per-image deployment annotations (first-observed on first observation, digest-applied after a successful re-image). When recorded == latest and the live image is off-digest, it skips rolling only if both markers show a fresh install (first_observed set, applied absent). Established edges (helm reverted an applied pin) and legacy edges (neither marker, e.g. fleet upgrading into this chart) still get the client-runtime#199 repair roll so the fleet is not left on a possibly stale tag.

Fail-closed behavior: digest and marker annotation reads no longer use || true; API/jq errors skip the image for that tick instead of mis-stamping first_observed. Chart version 1.9.112; helm-unittest and bats tests lock the gate and three edge shapes (fresh / established / legacy).

^{Reviewed by Cursor Bugbot for commit 5962509. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Stale Bugbot comment from a previous run.

[LukasWodka]

Read at 36c6d0c. The intent is right and the mechanics of the marker are clean: derived once per image, read only in the off-digest arm, written in the same annotate as the digest record and only after a successful rollout, with the first-observation arm writing neither. The bats pair reddens on an inverted or deleted gate, which is the proof that matters. Three things before this can go in, one of them a design gap.

1. The gate has no evidence for the fleet that already exists. The marker is written in exactly one place (:781, the re-image path), so every edge pinned before this chart version has no marker. The header's own premise (:634-637) is that the auto-upgrade re-renders the Deployment back to repo:tag and discards the pin. So the upgrade that ships this chart reverts every established edge to :tag, and on the next tick each one has recorded == latest, a workload on :tag, and no marker, which is byte-for-byte the fresh-install shape. They all take the continue at :762 and sit on :tag until the next upstream digest change. On :dev that is minutes; on :prod it can be weeks, and on a node whose :tag layer is stale it is the exact exposure the client-runtime#199 repair exists to close, fleet-wide, for one hop. "Established edges (any edge that has ever applied a digest) are unaffected" in the body and the header is not true for any edge that applied its digest before this version. A backfill in the no-op arm does not help either, because those edges are already reverted before their first post-upgrade tick.

The fail-closed shape is to skip only on positive evidence. Have the first-observation arm (:628) also write a tracebloc.io/first-observed-<image>=1 marker, and gate the skip at :762 on first-observed present and digest-applied absent. Then: a workload with neither marker is a pre-marker edge and keeps the repair roll; a new install has first-observed only and is skipped; an established edge has digest-applied and rolls. Three bats cases fall out of that (legacy neither, fresh first-observed only, established applied), and the "unaffected" claim becomes true rather than asserted.

2. Public repo, private tracker. The Related section names the private backend tracker twice (owner-qualified and bare), and the same reference appears in the new shipped header, the in-script comments, the log line at :763, the helm unittest comment and the bats comments. This repo is public. Please strip it from the title and body (a #1008 split note carries the context on its own) and, since this PR introduces every one of those comment occurrences, from the shipped comments too.

3. Chart version gate. chart content ⇒ Chart.yaml version bump is red: the template changed and client/Chart.yaml is still 1.9.110, while develop is already at 1.9.111. Bump to 1.9.112.

Happy to approve once those land and mutation-check, Unit tests and Bugbot are green.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit f0856ce. Configure here.}

[saqlainsyed007]

Thanks @LukasWodka — all three addressed, pushed to f0856ce.

1. Pre-marker fleet (the design gap). Fixed exactly as you prescribed: skip only on positive evidence. Added tracebloc.io/first-observed-<image>, stamped by the first-observation arm; the off-digest skip now fires iff first_observed present and applied absent. The three shapes fall out:

• fresh (first_observed only) → skip, stays on :tag
• established (applied) → roll (the Release v1.4.5: promote develop → main #199 repair)
• pre-marker / legacy (neither) → roll (repair), which then stamps applied

So the upgrade hop no longer strands the existing fleet, and "unaffected" is now true rather than asserted. Marker reads are fail-closed: a read error (as opposed to genuine absence) skips the tick and retries, matching the unreadable-have stance. Three bats cases cover legacy/fresh/established.

2. Public repo / private tracker. Stripped from the title, body, shipped header, in-script comments, the log line, and the helm-unittest + bats comments — the #1008 split note carries the context.

3. Chart version. Merged current develop (1.9.111) and bumped client/Chart.yaml version and appVersion to 1.9.112 in lockstep.

Green on f0856ce: Unit tests, Helm unit tests, mutation-check, bugbot / review, and the chart-version guard all pass.

[saqlainsyed007]

bugbot run

[LukasWodka]

Re-read at 5962509. All three asks from my first pass are in, and the fail-open read Bugbot caught after my second pass is now closed too.

• The gate skips only on positive evidence. The first-observation arm stamps tracebloc.io/first-observed-<image> beside the digest record; the off-digest arm skips the roll only when first-observed is present AND digest-applied is absent, rolls an established edge (applied present), and rolls a pre-marker edge (neither) so the fleet upgrading into this chart is repaired once rather than stranded. The bats PRE-MARKER / legacy edge case asserts the roll, the repair log line, the pin and the applied stamp, and that the fresh-install log line is absent.
• The digest-record read is fail-closed. recorded="$(get_annotation "$key" || true)" was the one read that could still collapse a kubectl or jq error into "absent" and walk a legacy edge into the first-observation arm, where the new marker would then suppress the repair for good. It is now if ! recorded="$(get_annotation "$key")"; then … continue; fi, skipping the image for this tick and retrying, the same stance the two marker reads and the settled guard already take. I checked get_annotation's contract on this head: success with empty output on genuine absence, non-zero only on a read failure, so the guard cannot mistake absence for an error. The Helm unit test pins both the guarded form present and the || true form absent, so a revert reddens. The remaining || true annotation reads (pending_attempt and the three stale markers) are best-effort by design, with the fail-closed authority documented at the restart block, so I did not ask for them to change.
• Private tracker refs are gone from the title, body, shipped header, in-script comments, log lines and test comments; the #1008 split note carries the context.
• client/Chart.yaml is 1.9.112 over develop's 1.9.111.

Approving once the suite, mutation-check, the version-bump gate and Bugbot are green on this head.