Skip to content

fix(chart): image-refresh CronJob must not treat every error as a benign skip (#571) - #572

Merged
LukasWodka merged 4 commits into
developfrom
fix/571-cronjob-refresh-failopen
Aug 4, 2026
Merged

fix(chart): image-refresh CronJob must not treat every error as a benign skip (#571)#572
LukasWodka merged 4 commits into
developfrom
fix/571-cronjob-refresh-failopen

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

The bug (Medium)

The image-refresh CronJob's settled-gate ran kubectl rollout status and treated any non-zero as "rollout in progress — skip this tick" (exit 0). But non-zero is also NotFound, RBAC denial, and API errors — so a misconfigured or unreachable deployment left the CronJob green forever while refresh never ran.

Second issue: the call had no --request-timeout (only --timeout, which is the rollout wait, not the API-call bound). With concurrencyPolicy: Forbid and no activeDeadlineSeconds, a wedged API could hang the tick and block every later tick.

Fix

  • Disambiguate non-zero: if kubectl get deployment still succeeds, it's genuinely present-but-unsettled → legit skip (exit 0). Otherwise it's surfaced (exit 1) so the CronJob shows failed instead of green.
  • --request-timeout=15s on both calls — bounds each API request, so a wedged server can't hang the tick.
  • activeDeadlineSeconds: 300 on the Job — caps the whole run as a backstop.

Found by Bugbot on the prod promotion PR (client#571). Fixed on develop; it reaches prod via the normal develop → staging → prod path.

Note

The first commit on this branch mistakenly committed the file unmodified (a patch anchor mismatched the real 4-space indentation); the second commit is the actual fix. Net diff is the fix only.

Refs: tracebloc/backend#1426


Note

Medium Risk
Changes operational behavior of a cluster CronJob that restarts jobs-manager; misclassification fixes reduce silent outage risk but failed ticks may now alert where skips were previously hidden.

Overview
Fixes silent failure and hang risk in the image-refresh CronJob when the jobs-manager deployment cannot be reconciled normally.

The settled-gate no longer treats every failed kubectl rollout status as a benign skip. It now confirms the deployment still exists with kubectl get deployment; only then does it skip the tick (exit 0). NotFound, RBAC, or API errors fail the job (exit 1) so the CronJob shows failed instead of staying green while refresh never runs.

Both kubectl calls in that gate use --request-timeout=15s so a wedged API cannot block the tick indefinitely under concurrencyPolicy: Forbid.

The Job spec adds activeDeadlineSeconds: 1800 as a wall-clock backstop sized above the default rolloutTimeout (10m) so legitimate rollout waits are not killed mid-flight (which would prevent digest annotations and cause repeat restarts, #572).

Chart version bumps to 1.9.12.

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

…ign skip
Bugbot on the prod promotion (client#571), Medium. The settled-gate ran
`kubectl rollout status` and treated ANY non-zero as 'in progress -- skip'
(exit 0). But non-zero is also NotFound, RBAC denial, and API errors -- so a
misconfigured or unreachable deployment left the CronJob GREEN forever while
image refresh never ran. And the call had no --request-timeout (only --timeout,
the rollout wait), so a wedged API server could hang the tick with no
activeDeadlineSeconds, and concurrencyPolicy: Forbid would then block every later
tick.
Now: a non-zero rollout status is disambiguated -- if `kubectl get deployment`
still succeeds the deployment is genuinely present-but-unsettled (legit skip,
exit 0); otherwise it is an error the job SURFACES (exit 1). --request-timeout=15s
bounds each API call, and activeDeadlineSeconds=300 caps the whole tick as a
backstop.
Refs: tracebloc/backend#1426
…ign skip
Bugbot on the prod promotion (client#571), Medium. The settled-gate treated ANY
non-zero `kubectl rollout status` as 'in progress -- skip' (exit 0). But non-zero
is also NotFound, RBAC denial, and API errors, so a misconfigured or unreachable
deployment left the CronJob GREEN forever while refresh never ran. The call also
had no --request-timeout (only --timeout, the rollout wait), so a wedged API could
hang the tick with no activeDeadlineSeconds -- and concurrencyPolicy: Forbid then
blocks every later tick.
Now a non-zero status is disambiguated: if `kubectl get deployment` still succeeds
the deployment is genuinely present-but-unsettled (legit skip, exit 0); otherwise
it is surfaced (exit 1). --request-timeout=15s bounds each API call and
activeDeadlineSeconds=300 caps the whole tick.
(Corrects a prior no-op commit on this branch that committed the file unmodified
because a patch anchor mismatched the real indentation.)
Refs: tracebloc/backend#1426
@LukasWodkaLukasWodka self-assigned this Aug 3, 2026

@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 b191ff7. Configure here.

Comment threadclient/templates/image-refresh-cronjob.yaml Outdated
…ercut it
Bugbot on client#572, High. My 300s deadline was BELOW the default
imageRefresh.rolloutTimeout of 10m -- a tick doing rollout restart + rollout
status could be DeadlineExceeded mid-wait, so the post-success annotation never
lands and later ticks re-restart forever. Raised to 1800s (well above 10m) with a
comment tying it to rolloutTimeout; it is only a backstop since every call is
--request-timeout-bounded.
…ent)
The chart-version-guard requires a Chart.yaml bump when chart content changes; the
CronJob template fix (#571/#572) is chart content, so a Helm repo would not
republish it without a new version.
@LukasWodka
LukasWodka merged commit 6f503d2 into developAug 4, 2026
22 checks passed
@cursorcursorBot mentioned this pull request Aug 4, 2026
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

/fr-pass

aptracebloc added a commit that referenced this pull request Aug 4, 2026
…end#1497) (#580)
Two unbounded external calls (Bugbot on the client#571 prod promotion) that can
hang a CI job to the GitHub Actions cap with no useful failure — same class as
the image-refresh CronJob fix (#572).
- scripts/tests/e2e-seal-check.sh: add --request-timeout=10s to the point-in-time
kubectl API calls (get/run/logs/delete/describe) so a wedged API server fails
the seal-check fast. `kubectl wait` (L57) is deliberately left as-is: it is
already bounded by --timeout=180s, and a --request-timeout on its underlying
watch would truncate it and risk flakes.
- .github/workflows/helm-ci.yaml: add --connect-timeout 15 --max-time 120 to the
pinned kubeconform download so a stalled endpoint fails the template matrix
instead of hanging. Retries/-f behaviour unchanged.
Values mirror the repo's existing bounds (--request-timeout=5s/10s elsewhere;
--connect-timeout/--max-time in install.sh / setup-linux.sh). Neither file is in
the R8 signed manifest (test script + workflow), so no manifest bump.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka
LukasWodka deleted the fix/571-cronjob-refresh-failopen branch August 14, 2026 13:53
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.

2 participants

@LukasWodka@saadqbal