Skip to content

feat(#154): auto-refresh jobs-manager image on Docker Hub publish - #155

Merged
saadqbal merged 8 commits into
developfrom
feat/154-image-refresh-cronjob
May 25, 2026
Merged

feat(#154): auto-refresh jobs-manager image on Docker Hub publish#155
saadqbal merged 8 commits into
developfrom
feat/154-image-refresh-cronjob

Conversation

@saadqbal

@saadqbalsaadqbal commented May 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds an imageRefresh CronJob that polls Docker Hub for new manifest digests of tracebloc/jobs-manager and tracebloc/pods-monitor under the floating CLIENT_ENV tag and runs kubectl rollout restart on the jobs-manager Deployment only when at least one digest has changed. Closes the gap left by imagePullPolicy: Always — that policy only pulls on pod creation, so running pods stay on the old image until something restarts them. Customers should not have to run any commands when we publish a new image.

Closes#154.

Design

  • Mirrors the existing autoUpgrade pattern (ConfigMap + CronJob + RBAC) so operators have one consistent mental model for the chart's background loops.
  • Narrower trust boundary than autoUpgrade: namespace-scoped Role with get,list pods and get,patch deployments only — no cluster-admin. A compromise of this Pod is bounded to kicking the jobs-manager Deployment in the release namespace.
  • Idle-cheap: each tick HEADs two manifests on Docker Hub; when both digests match the running pod, the script exits without touching the deployment. Default schedule (every 15m at :07/:22/:37/:52) stays well under Docker Hub's 100/6h anonymous pull-rate limit.
  • Per-image opt-out: when images.jobsManager.digest or images.podsMonitor.digest is set, that image is skipped at runtime via env flags — digest pinning is an explicit reproducibility contract that auto-restart must not fight. If BOTH are pinned, the chart renders no CronJob, Role, RoleBinding, ConfigMap, or ServiceAccount at all.
  • Manifest HEAD sends four Accept media types (Docker v2 + v2 list + OCI manifest + OCI index) so digest resolution works regardless of which variant the registry serves for that tag.
  • Pod is PSA-restricted (runAsNonRoot, readOnlyRootFilesystem, dropped caps, RuntimeDefault seccomp); HOME points at a writable emptyDir so kubectl's discovery cache works under read-only root.

Why not Keel?

Considered — Keel is the canonical "watch the registry, restart on new digest" controller. The benefits (semver policies, approval gates, multi-workload auto-discovery) don't pay off at two deployments on a mutable env tag with anonymous Docker Hub pulls. Revisit if we grow to ~5+ auto-refreshed workloads or need approval gates. See #154 discussion.

Why not the ingestor?

Out of scope. Ingestor hookImage is curlimages/curl (third-party, pinned) and the workload is a Helm post-install,post-upgrade Job — rollout restart doesn't apply, and re-running a side-effecting hook on every image push would be wrong. Ingestor image refreshes ride along with chart upgrades via autoUpgrade.

Test plan

  • helm lint client/ --set clientId=test --set clientPassword=test
  • helm template renders stg-image-refresh ServiceAccount, Role, RoleBinding, ConfigMap, and CronJob in the release namespace
  • helm unittest client/ — 135/135 (15 new in image_refresh_test.yaml)
  • Manual on a dev cluster: install the chart at this branch, confirm the CronJob runs on its first tick and exits with "all images up to date"
  • Manual: push a new tracebloc/jobs-manager:prod image, wait for the next tick, confirm the deployment is rolled and pulls the new digest
  • Manual: set images.jobsManager.digest to a pinned digest, helm upgrade, confirm the script logs "pinned by digest in values; skipping" and does not restart
  • Manual: set BOTH images.*.digest values, helm upgrade, confirm none of the image-refresh resources render

Chart version

Bumped 1.3.5 → 1.4.0 (feature add). autoUpgrade on already-deployed releases will pick this up on its next tick.

🤖 Generated with Claude Code


Note

Medium Risk
Adds a new background CronJob that polls Docker Hub and patches/restarts the jobs-manager Deployment, introducing operational risk around unexpected rollouts and reliance on external registry availability, though RBAC is kept namespace-scoped and limited to deployments.

Overview
Bumps the Helm chart version to 1.4.0 and adds an imageRefresh controller loop that periodically checks Docker Hub manifest digests for tracebloc/jobs-manager and tracebloc/pods-monitor under the current CLIENT_ENV tag, then performs kubectl rollout restart (and records digests via deployment annotations) only when a digest changes.

This introduces new rendered resources (ConfigMap script, CronJob, ServiceAccount, namespace-scoped Role/RoleBinding) gated by imageRefresh.enabled, with additional logic to render nothing when both images are digest-pinned, plus new values.yaml defaults, values.schema.json validation (including rejecting imageRefresh.image.tag: latest), and Helm unit tests covering rendering/guardrails.

Reviewed by Cursor Bugbot for commit 877e11f. Bugbot is set up for automated code reviews on this repo. Configure here.

Adds an `imageRefresh` CronJob that polls Docker Hub for new manifest
digests of tracebloc/jobs-manager and tracebloc/pods-monitor under the
floating CLIENT_ENV tag, and runs `kubectl rollout restart` on the
jobs-manager Deployment only when at least one digest has changed.
Closes the gap left by `imagePullPolicy: Always`: that policy only
pulls on pod creation — running pods stay on the old image until
something restarts them. Customers should not have to run any
commands when we publish a new image.
Design
* Mirrors the existing `autoUpgrade` pattern (ConfigMap + CronJob +
RBAC), so operators have one consistent mental model for the chart's
background loops.
* Trust boundary is narrower than autoUpgrade: a namespace-scoped Role
with `get,list pods` and `get,patch deployments` only — no
cluster-admin. A compromise of this Pod is bounded to kicking the
jobs-manager Deployment in the release namespace.
* Idle-cheap: each tick HEADs two manifests on Docker Hub; when both
digests match the running pod, the script exits without touching
the deployment. Default schedule (every 15m at :07/:22/:37/:52)
stays well under Docker Hub's 100/6h anonymous pull-rate limit.
* Per-image opt-out: when `images.jobsManager.digest` or
`images.podsMonitor.digest` is set, that image is skipped at runtime
via env flags — digest pinning is an explicit reproducibility
contract that auto-restart must not fight. If BOTH are pinned, the
chart renders no CronJob, Role, RoleBinding, ConfigMap, or
ServiceAccount at all.
* Manifest HEAD sends four Accept media types (Docker v2 + v2 list +
OCI manifest + OCI index) so digest resolution works regardless of
which variant the registry serves for that tag.
* Pod is PSA-restricted (runAsNonRoot, readOnlyRootFilesystem, dropped
caps, RuntimeDefault seccomp); HOME pointed at a writable emptyDir
so kubectl's discovery cache works under read-only root.
Ingestor is intentionally out of scope: its `hookImage` is
`curlimages/curl` (third-party, pinned) and the workload is a Helm
post-install/post-upgrade Job — `rollout restart` doesn't apply, and
re-running a side-effecting hook on every image push would be wrong.
The ingestor's image-refresh channel is the chart-upgrade path that
autoUpgrade already drives.
Tests
15 new helm-unittest cases covering enabled/disabled, both-pinned,
one-pinned, schedule/image/timeout overrides, RBAC scope (must be
Role not ClusterRole), the docker-content-digest accept-media-type
guards, the digest-pin runtime skip, and the schema rejection of
`image.tag=latest`. Full suite: 135/135 pass.
Closes#154
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@saadqbalsaadqbal self-assigned this May 22, 2026
@LukasWodka

Copy link
Copy Markdown
Contributor

👋 Heads-up — Code review queue is at 20 / 8

Above the WIP limit. The team convention is to review existing PRs before opening new work.

Open PRs currently in Code review (oldest first):

Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.)

Per the "nil-guard every new top-level value key" rule in CLAUDE.md.
The autoUpgrade CronJob uses --reset-then-reuse-values, which layers
new chart defaults on top of stored user overrides, so the new
imageRefresh defaults flow through automatically — that path is fine.
But a customer who instead runs `helm upgrade --reuse-values
--version 1.4.0` (the broken-by-default flag, still a common habit)
would replay stored 1.3.x values that have no imageRefresh block at
all. Without guards, .Values.imageRefresh.enabled nil-coalesces OK,
but .Values.images.jobsManager.digest would crash if .Values.images
were ever absent. Belt-and-suspenders.
* `tracebloc.imageRefreshEnabled` now wraps every dereference in
`default dict`, so a missing block silently renders nothing.
* New regression test verifies the cronjob + rbac templates render
zero documents (no crash) when `imageRefresh: null` is set, which
simulates the stale-values upgrade path exactly.
Verified end-to-end: `helm template ... --set imageRefresh=null` now
emits 0 image-refresh-* resources instead of failing template rendering.
Closes the last "could a customer ever have to run a command" hole in
the auto-update chain.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Comment threadclient/templates/image-refresh-rbac.yaml Outdated
Caught in PR #155 review (bugbot, high severity).
`kubectl rollout status` uses a ListWatch on the deployment resource
to wait for rollout completion. The image-refresh script runs it after
every `rollout restart`. Without the `watch` verb the call enters a
reflector error loop (kubectl#580) and hangs until --timeout (10m)
fires — every successful restart silently gets marked as a failed Job,
each failure burns ~30m of pod time under backoffLimit:2, and the
CronJob looks broken in `kubectl get cronjob` even though rollouts
actually succeed.
Also tightens the RBAC test to lock the full verb set with `equal`
instead of `contains`, so a future refactor that drops `watch` fails
loudly instead of silently regressing back to the same hang.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Comment threadclient/templates/image-refresh-cronjob.yaml Outdated
Caught in PR #155 review (bugbot, medium severity).
`BEGIN{IGNORECASE=1}` is a gawk extension. alpine/k8s ships BusyBox
awk, which silently ignores it — the lowercase pattern
`/^docker-content-digest:/` only matches because HTTP/2 mandates
lowercase headers (RFC 7540), and modern curl defaults to HTTP/2 over
HTTPS. The moment something downgrades to HTTP/1.1 — a corporate
proxy terminating TLS, a network blocking ALPN, a future curl
release changing defaults — the response arrives with mixed-case
`Docker-Content-Digest`, awk's lowercase pattern misses it, and the
script silently logs "could not resolve latest digest" forever
without ever restarting on a new image publish.
Pipe through `tr '[:upper:]' '[:lower:]'` BEFORE awk so the case-
fold doesn't depend on IGNORECASE. Digests are lowercase-by-spec
(sha256:<64 hex>), so the value we capture is unaffected.
Two regression tests pin this in place:
* must include `tr '[:upper:]' '[:lower:]'`
* must NOT include `BEGIN{IGNORECASE` in awk code (regex targets
the code form so the word can still appear in prose comments)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Comment threadclient/templates/image-refresh-cronjob.yaml Outdated
Comment threadclient/templates/image-refresh-rbac.yaml Outdated
Two more findings from PR #155 review (bugbot, both medium).
* Consolidate the four manifest-media-type Accept values into a single
comma-separated header. The Docker registry v2 spec documents the
single-header form, and while RFC 7230 §3.2.2 allows the multi-line
equivalent, TLS-terminating proxies have been observed dropping or
reordering repeated Accept headers — silently breaking digest
resolution for any image published only as an OCI index. Single
header is the safer, spec-canonical form.
* Add `list` to the deployments verbs. `kubectl rollout status` uses
a ListWatch reflector on the deployment; ListWatch needs BOTH `list`
(initial sync) and `watch` (incremental). The previous fix added
`watch` but missed `list` — symmetric failure mode (403 retry loop
until --timeout, every successful restart marked as a failed Job).
Tests:
* Lock the consolidated Accept header with a matchRegex covering all
four media types joined by commas on one line.
* Update the verbs assertion to ["get", "list", "patch", "watch"]
(still `equal`, not `contains`, so dropping any verb fails loudly).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Comment threadclient/templates/image-refresh-cronjob.yaml
Caught in PR #155 review (bugbot, medium severity).
Multi-arch images publish a manifest list (or OCI index) at the tag.
Docker Hub's HEAD on the tag returns the INDEX digest, but containerd
records the PLATFORM-specific manifest digest in the pod's
containerStatuses[].imageID (kubernetes/kubernetes#108689, #115199).
Those values are by-construction different — comparing them directly,
as the previous code did, would trigger a `kubectl rollout restart` on
every 15-minute tick for any multi-arch image, even when nothing
changed. Restart-on-every-tick churn defeats the whole feature.
Fix: when the top-level digest doesn't match the running digest, GET
the manifest body. If it's an index (has a `"manifests"` key), extract
every per-platform manifest digest and check whether the running digest
matches ANY of them. Only restart when no match anywhere.
The grep -oE 'sha256:[a-f0-9]{64}' approach is safe on index bodies
because the only sha256: references in an index are the per-platform
`manifests[].digest` values. Single-platform manifest bodies contain
config + layer digests which are NOT manifest digests; the
`"manifests"` key gate skips those (the top-level HEAD digest is
already authoritative for single-arch).
Rate-limit impact: the extra GET only fires on top-level mismatch,
which is the rare path. Steady state stays at one HEAD per image per
tick — same as before.
Tests pin:
* `get_index_digests` helper presence
* The `"manifests"` key gate so single-platform bodies don't get
treated as indexes
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Comment threadclient/templates/image-refresh-cronjob.yaml
Caught in PR #155 review (bugbot, medium severity). Followup to the
multi-arch fix in cb3e180.
The previous fallback ran `for _d in $(get_index_digests "$repo" || true)`
and concluded "no match -> restart" when the loop body never ran. For
multi-arch images that's the common path (HEAD-vs-imageID always
mismatches at the top level), so any transient on the index GET —
network blip, rate-limit, expired-token retry — would turn into a
spurious deployment restart on the next tick.
Asymmetric with `get_latest_digest`, where the equivalent failure
explicitly continues to the next tick.
Fix: `get_index_digests` now returns exit 1 on transient (token
failure, curl -fsS non-zero), exit 0 + empty stdout when the body was
fetched but is a single-platform manifest (no `"manifests"` key —
HEAD digest already authoritative, caller falls through to restart).
The caller captures exit status separately via `if ! _index_digests=...`
and `continue`s on failure with a WARN log, matching the top-level
skip-this-tick contract.
Tests pin the `if !` capture pattern and the "could not fetch index
body" log line so the asymmetric-failure regression can't creep back.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Fix All in Cursor

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

Reviewed by Cursor Bugbot for commit a937c75. Configure here.

Comment threadclient/templates/image-refresh-cronjob.yaml
Replaces the registry-digest-vs-pod-imageID compare with
registry-digest-vs-deployment-annotation. Source of truth is now an
annotation we write ourselves (`tracebloc.io/last-refreshed-<image>-
digest`) rather than `containerStatuses[].imageID`, which has been the
root cause of every bugbot finding on this PR.
Why
The five preceding fixes in this PR were all rooted in disagreement
between "what Docker Hub publishes for the tag" and "what's actually
recorded on the running pod":
* imageID format varies across container runtimes
(kubernetes/kubernetes#108689 + #115199)
* multi-arch tags publish an index digest at the registry side,
containerd records a per-platform manifest digest on the pod side
* HTTP header case depends on HTTP version
* BusyBox awk silently ignores gawk's IGNORECASE
* transient GET failures on the multi-arch fallback path could be
misread as "no match found" and trigger spurious restarts
The annotation pattern makes the comparison entirely server-vs-our-
own-record. The runtime never enters the loop, so all of the above
classes of bug stop applying.
What changed
* image-refresh.sh now reads `tracebloc.io/last-refreshed-<image>-
digest` off the deployment metadata via `kubectl get -o jsonpath`,
compares to the registry digest, and on mismatch runs
`rollout restart` + `rollout status` and only THEN updates the
annotation. Order matters: annotating first would let a failed
rollout silently freeze the deployment on the old image.
* First-observation contract: when the annotation is absent on a
fresh install, the script records the current digest WITHOUT
restarting. No evidence of drift, no reason to churn the pod.
* `get_index_digests`, `get_running_digest`, the multi-arch fallback
path, and the transient-failure plumbing for it are all gone.
Script shrank ~170 → ~115 lines despite gaining a comment block
explaining the design.
* `log()` now writes to stderr unconditionally (bugbot finding from
the previous review pass). Command-substitution capture sites can
no longer be polluted by log output.
* RBAC tightened: dropped the pods rule entirely. The Role now grants
ONLY get/list/patch/watch on apps/deployments in the release
namespace. A compromise of this Pod is now bounded to kicking and
annotating one Deployment.
* values.yaml and tests rewritten to describe and pin the new
contract. Regression guards stay for the Accept-header form,
case-folding pipe, BusyBox-safe awk, and the
no-imageID-comparison invariant (pattern targets `kubectl get pod`
as the smoking-gun call shape).
136/136 unit tests pass. Five resolved bugbot threads on this PR
each pinned a regression with at least one test; those tests are
preserved (or replaced with stricter equivalents) so the lessons
from the iteration aren't lost.
Closes#154
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@saadqbal

Copy link
Copy Markdown
ContributorAuthor

/fr-pass

Dev smoke-tested on tb-client-dev-templates: first-tick records both digests without restart, second tick is a no-op, forced annotation mismatch triggers the expected rollout-restart-then-annotate sequence. Rollout history bumped 11 → 12 on the forced-change test. See PR #157 description for the full verification table.

saadqbal added a commit that referenced this pull request May 25, 2026
…159)
* feat(#158): auto-refresh ingestor image digest without chart release
Extends the existing image-refresh CronJob (#155) to also reconcile
the ghcr.io/tracebloc/ingestor digest onto the live jobs-manager
deployment's INGESTOR_IMAGE_DIGEST env var. New ingestor image
publishes to the floating tag are now picked up within the cronjob's
poll interval (~15 min) instead of requiring a full chart release.
Why
Today, shipping a new ghcr.io/tracebloc/ingestor image required
bumping client/values.yaml images.ingestor.digest + client/Chart.yaml
+ PR + sync to main + release tag. That's hours of overhead per
bump and asymmetric with jobs-manager (which already gets the
~15-min image-refresh path). The asymmetry hurts because the
ingestor changes frequently as the data-ingestors team iterates.
Design
Two image classes in one CronJob now:
Class 1 (jobs-manager, pods-monitor):
Registry: docker.io
Tag: CLIENT_ENV
Source of truth: deployment annotation
`tracebloc.io/last-refreshed-<image>-digest` (#154)
Action on change: kubectl rollout restart
Class 2 (ingestor):
Registry: ghcr.io
Tag: images.ingestor.tag (default "prod")
Source of truth: live INGESTOR_IMAGE_DIGEST env value on the
api container of the jobs-manager deployment (no annotation
needed — the env IS the digest jobs-manager passes to each
spawned ingestion Job, so the most direct read of "what
will be used next" is THIS value).
Action on change: kubectl set env (triggers natural rollout
via ReplicaSet rotation — no explicit `rollout restart`).
get_token + get_latest_digest parameterized by registry; both
docker.io and ghcr.io support anonymous pull tokens for public
images with only the issuer URL differing.
Per-image opt-out
* jobs-manager / pods-monitor: same as #154 — set
`images.<image>.digest` non-empty.
* ingestor: explicit `images.ingestor.autoRefresh: false` flag.
Asymmetric because ingestor.digest must be non-empty for
jobs-manager to function (an empty env would 503 every ingestion
submit), so we can't use digest-presence as the signal.
When ALL THREE pin signals are active, the chart renders no
image-refresh resources at all (helper `imageRefreshEnabled`).
When at least one is unpinned, the cronjob is rendered and the
script skips pinned images via env flags at runtime.
Chart-default ingestor digest stays pinned (v0.3.0) as the
baseline for greenfield installs; image-refresh dynamically
updates the live env from there. Helm's 3-way merge preserves
image-refresh's writes across future helm upgrades as long as
the chart's pinned baseline doesn't change.
Subtle gotcha caught in dev
`default true $autoRefresh` in Go templates returns `true` even
when $autoRefresh is explicitly `false` (Go treats bool false as
falsy, so default overrides it). Switched to `eq $autoRefresh
false` directly — absence (nil) and explicit `true` both fall
through to "not pinned" as intended. Test pinned against the
correct idiom.
Other changes
* `log()` continues to write to stderr (#155 fix).
* `get_container_env` helper for jq-based env-var reads —
same kubectl-jsonpath caveat as `get_annotation` (#156).
* Chart version bumped 1.4.0 → 1.4.1.
Tests
20 image-refresh-suite tests (was 17), 140 total pass. New
assertions:
* all-three-pinned renders zero resources
* only-jobs-manager+pods-monitor-pinned still renders (regression
guard for the asymmetric pin signal — without this, the
ingestor would never auto-refresh on default installs)
* INGESTOR_PINNED flips correctly on autoRefresh=false
* INGESTOR_TAG is overridable, `latest` rejected by schema
* Script must include `kubectl set env`, `ghcr.io`,
`auth.docker.io`, `get_container_env`, the empty-env
fill-from-registry path, and the autoRefresh-skip log line
Closes#158
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(#158): annotation as source of truth for ingestor (rollout-failure retry)
Caught in PR #159 review (bugbot, medium severity).
The original design used the live spec env (`INGESTOR_IMAGE_DIGEST` on
the api container) as the source of truth for "what image-refresh has
reconciled to." `kubectl set env` commits the new spec to etcd BEFORE
`kubectl rollout status` waits for the rollout to complete. If the
rollout times out or the new ReplicaSet's pods fail to come up:
* `set -eu` aborts the script.
* But the spec already matches the registry.
* Next tick: `get_container_env` returns the new digest, compares
equal to registry, no-op → script appears successful.
* Meanwhile the old ReplicaSet's pods are still running with the
OLD env, and the new ReplicaSet is stuck failing. The deployment
is frozen on the old version with no retry signal.
Fix: mirror the jobs-manager/pods-monitor pattern from #154. Use a
`tracebloc.io/last-refreshed-ingestor-digest` annotation on the
deployment as the source of truth. Update the annotation as the LAST
step, only after `rollout status` succeeds. A failed rollout aborts
before the annotate → next tick sees stale annotation → retries.
First-observation contract for ingestor:
* Non-empty spec env (the normal case — chart populates a default):
adopt as baseline annotation, don't touch env. Same "don't churn
on install" principle as jobs-manager first-observation.
* Empty spec env (corrupted state, manual kubectl edit, stale
--reuse-values): fill from registry on first tick. Empty would
otherwise cause jobs-manager to 503 on every ingestion submit,
so the "don't churn" trade is wrong in that case.
Tests pin:
* Annotation key `tracebloc.io/last-refreshed-ingestor-digest`
appears in the script.
* Order-of-operations: set env → rollout status → annotate (the
annotate MUST come last; regex matches the full sequence).
140/140 tests pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* docs(#158): correct stale top-of-script comment + add first-obs test
Found during PR #159 self-review.
The top-of-script comment block still described the pre-bugbot-fix
design — claimed "Source of truth: the live env value itself (no
annotation needed)" and "no 'first observation' empty-state, each
tick is a normal compare-and-patch-if-different." Both were stale
after e7cf829 switched ingestor to annotation-based source of truth
and added the first-observation branch. Anyone reading the
script-level overview would have been misled about the actual loop.
Comment now matches the code: annotation as source of truth, two-
case first-observation contract (non-empty → adopt as baseline;
empty → fill from registry).
Also adds a positive regression test for the previously-untested
first-observation "adopting spec env as baseline" branch. The
empty-spec-env branch was already covered indirectly by the existing
"would 503 on ingestion submit" regex.
140/140 tests pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(#158): also reconcile env drift (rollout undo / kubectl edit / GitOps)
Caught in PR #159 review (bugbot, medium severity). Follow-up to the
annotation-source-of-truth switch in e7cf829.
The previous no-op branch fired whenever annotation == registry and
NEVER read the live spec env. That meant any external actor that
reverted the spec env without touching the annotation would leave the
deployment on a stale env indefinitely. The annotation continued to
match the registry, so image-refresh kept skipping. Real scenarios
this affects:
* `kubectl rollout undo deployment/X` — reverts pod template to a
previous ReplicaSet's spec, including its INGESTOR_IMAGE_DIGEST
env. Annotation on deployment metadata is untouched.
* `kubectl edit deployment X` — operator manually changes the env.
* Certain `helm upgrade` flag combos can reset env to the chart's
pre-image-refresh baseline while preserving annotations (e.g.,
--reset-values or upgrade from a chart where the digest baseline
differs from what image-refresh had reconciled to).
* GitOps reconcilers (Argo CD, Flux) that own the deployment spec
will revert image-refresh's env writes back to the rendered
template values.
In all of these, the live deployment runs a stale ingestor image
forever — exactly the failure mode #158 was meant to prevent.
Fix: each tick now reads both the annotation AND the live spec env.
Three reconciliation paths:
* recorded != registry → "registry drift". Set env to registry,
wait for rollout, update annotation. (Existing behaviour.)
* recorded == registry AND spec env != recorded → "env drift". Set
env to recorded value (NOT registry — registry matches recorded
by definition here, but recorded is the value we last decided to
roll to). Wait for rollout. Don't update the annotation; it's
already correct.
* recorded == registry AND spec env == recorded → fully in sync,
no-op.
Tests pin:
* The "spec env drifted" log line.
* The drift-recovery branch sets env to `${recorded_ingestor}`,
not `${latest_ingestor}` (different from the registry-drift
branch). Regex catches the variable used in `INGESTOR_IMAGE_DIGEST=`.
Top-of-script comment block updated to document the drift recovery.
140/140 tests pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(#158): wait for rollout status in adopt-as-baseline branch
Caught in PR #159 review (bugbot, high severity).
Scenario the existing code mishandles:
1. Tick N: empty-spec-first-obs branch runs `kubectl set env`
(commits new spec to etcd) → `kubectl rollout status` times out
→ `set -eu` aborts before the annotate. Annotation stays empty.
2. Tick N+1: annotation still empty. spec env is now non-empty
(the failed-rollout's spec change persists). get_container_env
returns that value, so the adopt-as-baseline branch fires.
3. Adopt-as-baseline only annotates — it never checks rollout
health. Annotation records "we're at D1" while running pods
are still on the old/empty env from before tick N.
The deployment now appears reconciled (annotation == registry on
subsequent ticks) while actually being stuck on the wrong image.
Fix: call `kubectl rollout status` inside the adopt-as-baseline
branch before the annotate. On a healthy deployment it returns
near-instantly; on a stuck rollout from a previous failed
set-env it times out, `set -eu` aborts before the annotate, next
tick retries. No latency cost on the happy path.
Regression test pins the (?s)-multiline order:
adopting → rollout status → annotate.
140/140 tests pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
saadqbal added a commit that referenced this pull request May 26, 2026
* Merge pull request #161 from tracebloc/chore/pin-ingestor-v0.3.1-160
chore: pin client chart's ingestor digest to v0.3.1
* feat(#158): auto-refresh ingestor image digest without chart release (#159)
* feat(#158): auto-refresh ingestor image digest without chart release
Extends the existing image-refresh CronJob (#155) to also reconcile
the ghcr.io/tracebloc/ingestor digest onto the live jobs-manager
deployment's INGESTOR_IMAGE_DIGEST env var. New ingestor image
publishes to the floating tag are now picked up within the cronjob's
poll interval (~15 min) instead of requiring a full chart release.
Why
Today, shipping a new ghcr.io/tracebloc/ingestor image required
bumping client/values.yaml images.ingestor.digest + client/Chart.yaml
+ PR + sync to main + release tag. That's hours of overhead per
bump and asymmetric with jobs-manager (which already gets the
~15-min image-refresh path). The asymmetry hurts because the
ingestor changes frequently as the data-ingestors team iterates.
Design
Two image classes in one CronJob now:
Class 1 (jobs-manager, pods-monitor):
Registry: docker.io
Tag: CLIENT_ENV
Source of truth: deployment annotation
`tracebloc.io/last-refreshed-<image>-digest` (#154)
Action on change: kubectl rollout restart
Class 2 (ingestor):
Registry: ghcr.io
Tag: images.ingestor.tag (default "prod")
Source of truth: live INGESTOR_IMAGE_DIGEST env value on the
api container of the jobs-manager deployment (no annotation
needed — the env IS the digest jobs-manager passes to each
spawned ingestion Job, so the most direct read of "what
will be used next" is THIS value).
Action on change: kubectl set env (triggers natural rollout
via ReplicaSet rotation — no explicit `rollout restart`).
get_token + get_latest_digest parameterized by registry; both
docker.io and ghcr.io support anonymous pull tokens for public
images with only the issuer URL differing.
Per-image opt-out
* jobs-manager / pods-monitor: same as #154 — set
`images.<image>.digest` non-empty.
* ingestor: explicit `images.ingestor.autoRefresh: false` flag.
Asymmetric because ingestor.digest must be non-empty for
jobs-manager to function (an empty env would 503 every ingestion
submit), so we can't use digest-presence as the signal.
When ALL THREE pin signals are active, the chart renders no
image-refresh resources at all (helper `imageRefreshEnabled`).
When at least one is unpinned, the cronjob is rendered and the
script skips pinned images via env flags at runtime.
Chart-default ingestor digest stays pinned (v0.3.0) as the
baseline for greenfield installs; image-refresh dynamically
updates the live env from there. Helm's 3-way merge preserves
image-refresh's writes across future helm upgrades as long as
the chart's pinned baseline doesn't change.
Subtle gotcha caught in dev
`default true $autoRefresh` in Go templates returns `true` even
when $autoRefresh is explicitly `false` (Go treats bool false as
falsy, so default overrides it). Switched to `eq $autoRefresh
false` directly — absence (nil) and explicit `true` both fall
through to "not pinned" as intended. Test pinned against the
correct idiom.
Other changes
* `log()` continues to write to stderr (#155 fix).
* `get_container_env` helper for jq-based env-var reads —
same kubectl-jsonpath caveat as `get_annotation` (#156).
* Chart version bumped 1.4.0 → 1.4.1.
Tests
20 image-refresh-suite tests (was 17), 140 total pass. New
assertions:
* all-three-pinned renders zero resources
* only-jobs-manager+pods-monitor-pinned still renders (regression
guard for the asymmetric pin signal — without this, the
ingestor would never auto-refresh on default installs)
* INGESTOR_PINNED flips correctly on autoRefresh=false
* INGESTOR_TAG is overridable, `latest` rejected by schema
* Script must include `kubectl set env`, `ghcr.io`,
`auth.docker.io`, `get_container_env`, the empty-env
fill-from-registry path, and the autoRefresh-skip log line
Closes#158
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(#158): annotation as source of truth for ingestor (rollout-failure retry)
Caught in PR #159 review (bugbot, medium severity).
The original design used the live spec env (`INGESTOR_IMAGE_DIGEST` on
the api container) as the source of truth for "what image-refresh has
reconciled to." `kubectl set env` commits the new spec to etcd BEFORE
`kubectl rollout status` waits for the rollout to complete. If the
rollout times out or the new ReplicaSet's pods fail to come up:
* `set -eu` aborts the script.
* But the spec already matches the registry.
* Next tick: `get_container_env` returns the new digest, compares
equal to registry, no-op → script appears successful.
* Meanwhile the old ReplicaSet's pods are still running with the
OLD env, and the new ReplicaSet is stuck failing. The deployment
is frozen on the old version with no retry signal.
Fix: mirror the jobs-manager/pods-monitor pattern from #154. Use a
`tracebloc.io/last-refreshed-ingestor-digest` annotation on the
deployment as the source of truth. Update the annotation as the LAST
step, only after `rollout status` succeeds. A failed rollout aborts
before the annotate → next tick sees stale annotation → retries.
First-observation contract for ingestor:
* Non-empty spec env (the normal case — chart populates a default):
adopt as baseline annotation, don't touch env. Same "don't churn
on install" principle as jobs-manager first-observation.
* Empty spec env (corrupted state, manual kubectl edit, stale
--reuse-values): fill from registry on first tick. Empty would
otherwise cause jobs-manager to 503 on every ingestion submit,
so the "don't churn" trade is wrong in that case.
Tests pin:
* Annotation key `tracebloc.io/last-refreshed-ingestor-digest`
appears in the script.
* Order-of-operations: set env → rollout status → annotate (the
annotate MUST come last; regex matches the full sequence).
140/140 tests pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* docs(#158): correct stale top-of-script comment + add first-obs test
Found during PR #159 self-review.
The top-of-script comment block still described the pre-bugbot-fix
design — claimed "Source of truth: the live env value itself (no
annotation needed)" and "no 'first observation' empty-state, each
tick is a normal compare-and-patch-if-different." Both were stale
after e7cf829 switched ingestor to annotation-based source of truth
and added the first-observation branch. Anyone reading the
script-level overview would have been misled about the actual loop.
Comment now matches the code: annotation as source of truth, two-
case first-observation contract (non-empty → adopt as baseline;
empty → fill from registry).
Also adds a positive regression test for the previously-untested
first-observation "adopting spec env as baseline" branch. The
empty-spec-env branch was already covered indirectly by the existing
"would 503 on ingestion submit" regex.
140/140 tests pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(#158): also reconcile env drift (rollout undo / kubectl edit / GitOps)
Caught in PR #159 review (bugbot, medium severity). Follow-up to the
annotation-source-of-truth switch in e7cf829.
The previous no-op branch fired whenever annotation == registry and
NEVER read the live spec env. That meant any external actor that
reverted the spec env without touching the annotation would leave the
deployment on a stale env indefinitely. The annotation continued to
match the registry, so image-refresh kept skipping. Real scenarios
this affects:
* `kubectl rollout undo deployment/X` — reverts pod template to a
previous ReplicaSet's spec, including its INGESTOR_IMAGE_DIGEST
env. Annotation on deployment metadata is untouched.
* `kubectl edit deployment X` — operator manually changes the env.
* Certain `helm upgrade` flag combos can reset env to the chart's
pre-image-refresh baseline while preserving annotations (e.g.,
--reset-values or upgrade from a chart where the digest baseline
differs from what image-refresh had reconciled to).
* GitOps reconcilers (Argo CD, Flux) that own the deployment spec
will revert image-refresh's env writes back to the rendered
template values.
In all of these, the live deployment runs a stale ingestor image
forever — exactly the failure mode #158 was meant to prevent.
Fix: each tick now reads both the annotation AND the live spec env.
Three reconciliation paths:
* recorded != registry → "registry drift". Set env to registry,
wait for rollout, update annotation. (Existing behaviour.)
* recorded == registry AND spec env != recorded → "env drift". Set
env to recorded value (NOT registry — registry matches recorded
by definition here, but recorded is the value we last decided to
roll to). Wait for rollout. Don't update the annotation; it's
already correct.
* recorded == registry AND spec env == recorded → fully in sync,
no-op.
Tests pin:
* The "spec env drifted" log line.
* The drift-recovery branch sets env to `${recorded_ingestor}`,
not `${latest_ingestor}` (different from the registry-drift
branch). Regex catches the variable used in `INGESTOR_IMAGE_DIGEST=`.
Top-of-script comment block updated to document the drift recovery.
140/140 tests pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(#158): wait for rollout status in adopt-as-baseline branch
Caught in PR #159 review (bugbot, high severity).
Scenario the existing code mishandles:
1. Tick N: empty-spec-first-obs branch runs `kubectl set env`
(commits new spec to etcd) → `kubectl rollout status` times out
→ `set -eu` aborts before the annotate. Annotation stays empty.
2. Tick N+1: annotation still empty. spec env is now non-empty
(the failed-rollout's spec change persists). get_container_env
returns that value, so the adopt-as-baseline branch fires.
3. Adopt-as-baseline only annotates — it never checks rollout
health. Annotation records "we're at D1" while running pods
are still on the old/empty env from before tick N.
The deployment now appears reconciled (annotation == registry on
subsequent ticks) while actually being stuck on the wrong image.
Fix: call `kubectl rollout status` inside the adopt-as-baseline
branch before the annotate. On a healthy deployment it returns
near-instantly; on a stuck rollout from a previous failed
set-env it times out, `set -eu` aborts before the annotate, next
tick retries. No latency cost on the happy path.
Regression test pins the (?s)-multiline order:
adopting → rollout status → annotate.
140/140 tests pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
* fix(#158): two bugbot follow-ups + chart default tag
Caught in PR #162 review (bugbot, two medium-severity issues).
1. Env-drift rollout retry gap
The no-op branch (annotation == registry AND spec env == recorded)
was a bare log statement with no rollout-health verification. A
previous tick's env-drift `kubectl set env` commits its spec change
to etcd BEFORE `kubectl rollout status` waits for the new
ReplicaSet to come up. If the rollout fails, `set -eu` aborts —
but the spec write persists. Next tick: annotation, registry, and
spec env all match (because the spec write committed), so the
no-op branch fires and silently masks the stuck rollout. Running
pods may be on the old or empty INGESTOR_IMAGE_DIGEST while the
script reports success.
Fix: call `kubectl rollout status` in the no-op branch too. On a
healthy deployment it returns near-instantly (no active rollout
to wait for). On a stuck deployment it times out, set -eu aborts,
and the Job is visibly failed in `kubectl get cronjob`. The
operator then sees the stuck state and can investigate. Image-
refresh can't autonomously recover from a bad image push, but
making the failure visible is the right behaviour.
2. Default ingestor tag mismatched team's publishing convention
Chart defaulted `images.ingestor.tag: prod`. The team's
ghcr.io/tracebloc/ingestor repo uses semver-style float tags
(`0`, `0.3`) — there is no `prod` tag. Default install would
silently no-op every tick because manifest resolution 404'd:
curl ... ghcr.io/v2/.../manifests/prod → 404
log " WARN: could not resolve latest digest; skipping"
The whole ingestor auto-refresh feature wouldn't work for any
customer running the chart's defaults, despite `autoRefresh:
true`.
Fix: changed default to "0.3" (conservative — patch-only auto-
track; won't pick up a future 0.4 with breaking changes).
Operators can override to "0" if they want major-version
auto-tracking. Long-term, the team should consider standardising
the chart default once the data-ingestors release-image.yml
formalises its tag-publishing contract — for now this matches
what we tested with on the dev cluster.
Regression tests:
* Default tag asserted as "0.3" with `notContains` of "prod" to
guard against silent revert.
* No-op branch asserted to call `kubectl rollout status` via
(?s)-multiline regex matching the "verifying deployment health"
log line + the kubectl rollout status call.
* Existing test updated from value: prod to value: "0.3".
141/141 unit tests pass.
NB: these commits are landing on the sync branch directly to avoid
another full develop-PR cycle before release. After #162 merges,
the same content will need to flow back to develop — either via a
"sync main → develop" PR or by cherry-picking the two commits. The
divergence is two commits and is easy to resolve.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(#158): align INGESTOR_TAG runtime fallback with chart default
Caught in PR #162 review (bugbot, medium severity).
The chart default was changed from "prod" to "0.3" in the values
block — matches the team's ghcr.io publishing convention — but the
CronJob template's runtime fallback was left at `| default "prod"`.
Two render paths:
* helm install / helm upgrade --reset-then-reuse-values: the
chart's new default ("0.3") flows through, runtime fallback
never fires, INGESTOR_TAG="0.3". OK.
* helm upgrade --reuse-values from a pre-v1.4.1 stored manifest:
the stored values lack `images.ingestor.tag` entirely. Runtime
fallback fires, renders INGESTOR_TAG="prod", which 404s on
ghcr.io because that tag doesn't exist. Ingestor refresh
silently no-ops every tick.
Failure mode is graceful (log warning, no crash), but inconsistent
with the per-customer expectation that v1.4.1 enables ingestor
auto-refresh. autoUpgrade itself uses --reset-then-reuse-values, so
this only hits manual --reuse-values upgrades — narrow but real.
Fix: change runtime fallback to "0.3" so both render paths converge.
Regression test simulates the --reuse-values scenario by setting
images.ingestor.tag=null, exercising the runtime fallback. 142/142
tests pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
@saadqbal
saadqbal deleted the feat/154-image-refresh-cronjob branch July 9, 2026 11:41
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@saadqbal@LukasWodka@aptracebloc