Uh oh!
There was an error while loading. Please reload this page.
fix(cronjobs): a failing tick must leave a Pod somebody can read (backend#2620) - #860
Conversation
…kend#2620) Both of the chart's CronJobs used `restartPolicy: OnFailure`. Kubernetes DELETES an OnFailure Job's Pod once `backoffLimit` is exhausted, and `failedJobsHistoryLimit` retains the JOB — it cannot retain a Pod that no longer exists. So a repeatedly-failing CronJob leaves a row of `Failed` Jobs and no logs at all. The upstream docs say it outright: "your Pod running the Job will be terminated once the job backoff limit has been reached. This can make debugging the Job's executable more difficult. We suggest setting restartPolicy = 'Never'". MEASURED, NOT THEORETICAL. A customer prod edge's auto-upgrade had been failing hourly for 2.7 days (backend#2620). Five `Failed` Jobs were retained; every one of their Pods was gone and the namespace event window had rolled past the first failure, so the reason was UNRECOVERABLE FROM THE CLUSTER. The only surviving evidence that anything was wrong was the CronJob's `lastSuccessfulTime`, which nothing watches. Each Job failed 39 seconds in, so this was never a timeout — the logs would have said what it was. BOTH CronJobs, not just the one that broke. `image-refresh` carried the same policy and is on a 15-minute schedule, so it had the same blind spot with four times the frequency. Fixing only the instance that happened to fail would have left the class. `scripts/tests/cronjob-failures-are-readable.sh` sweeps every CronJob out of the RENDERED manifests, so a third one is covered the day it lands rather than when someone remembers. It holds no list of CronJob names. Fails closed twice: zero CronJobs rendered is a refusal, not a clean sweep, and a template declaring `kind: CronJob` that no value combination reaches is reported UNREACHED by name. Wired into `DRIFT_GUARDS` (21 entries; the target counts its iterations and refuses to report green on fewer). Scoped to CronJob deliberately, and the argument is at the declaration: a Helm hook Job loses its Pod the same way, but its failure fails the release and is reported to whoever ran it — somebody is already looking. A CronJob's failure is reported to nobody and repeats forever. Four mutations run, each with the baseline restored green afterwards: one template reverts to OnFailure -> FAIL, names the template restartPolicy removed entirely -> FAIL, "silence is the defect" render matrix produces no CronJobs -> FAIL, refuses to check nothing a new CronJob template goes unreached -> FAIL, names it as UNREACHED The two suites that pinned `OnFailure` are updated rather than deleted — they were right to pin it, and the comment now records why the pinned value changed. make helm-unittest: 579 passed, 34 suites. make drift: all 21 guards green. shellcheck clean. Ticket: tracebloc/backend#2620 Parent epic: tracebloc/backend#1872 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`chart content ⇒ Chart.yaml version bump` is a REQUIRED check on this repo and was red: the branch changes chart content while `version`/`appVersion` both still read 1.9.73, which is what `develop` carries. The gate exists because the chart's image-refresh CronJob resolves the published tag every 15 minutes, so an unbumped chart is not a cosmetic omission -- nothing downstream can tell the new content from the old. Both fields moved together to 1.9.74, one patch above develop, which is the shape the gate asks for. They are kept EQUAL deliberately: `appVersion` is what the rendered image tags follow, so a bump of `version` alone would advertise a release that pulls the previous images. Verified: `helm lint` clean (only the pre-existing "icon is recommended" note), and the WHOLE chart suite rather than the file touched -- `helm unittest client`, 34 suites, 579 tests, all passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
saadqbal
left a comment
There was a problem hiding this comment.
The measurement is what makes this a bug report rather than a style note: five Failed Jobs retained, every Pod gone, the event window rolled past the first failure, and each Job failing 39 seconds in — so not a hang, and the logs would have said exactly what it was. "The reason was unrecoverable from the cluster" is the sentence that matters, and the consequence is worse than the mechanism: an edge silently stopped accepting chart upgrades, security fixes included, while looking healthy from every angle anyone checks.
Fixing image-refresh too — same policy, four times the frequency — is the right instinct, and "fixing only the instance that happened to fail would have left the class" is exactly the failure I flagged on client-runtime#417 an hour ago, where a guard existed in two sibling publishers and was missing from the one shipping the control plane. Good to see it applied ahead of the incident rather than after.
The guard fails closed in three places, not the two you claimed. I checked each:
if not seen: sys.exit("the render matrix produced ZERO CronJobs. Nothing was checked, and a guard that checks nothing passes — refusing.")unreached = expected - seen, each reported by name with the reason its policy went unchecked, and folded into the exit condition- and the one the description doesn't mention: a Pod spec stating no
restartPolicyat all lands inmissingand fails, so an unstated policy isn't read as an acceptable one
The declared set coming from grep -rlE '^kind: CronJob$' is line-anchored, so a template that merely mentions CronJob in a comment can't inflate expected and manufacture a phantom UNREACHED. And the [OK] line printing both len(seen) and len(expected) means a passing run is falsifiable rather than just green.
Two judgement calls I'd have made the same way. Scoping to CronJob and recording the argument at the declaration — a Helm hook Job loses its Pod identically, but its failure fails the release in front of the person who ran it, whereas a CronJob's failure is reported to nobody and repeats forever. And the failure message telling a future maintainer to "make that argument at the declaration and narrow this guard deliberately rather than adding an exception list here" — which is the right lesson to encode, given .github just spent three PRs burning an exemption list down to empty.
Green, no threads. 👍
Closes tracebloc/backend#2620
The defect
Both CronJobs the chart renders used
restartPolicy: OnFailure. Kubernetes deletes anOnFailureJob's Pod oncebackoffLimitis exhausted, andfailedJobsHistoryLimitretains the Job — it cannot retain a Pod that no longer exists.So a repeatedly-failing CronJob leaves a row of
FailedJobs and no logs at all. The upstream docs say it plainly:Measured, not theoretical
An edge's
auto-upgradeCronJob had been failing hourly for 2.7 days. FiveFailedJobs were retained; every one of their Pods was gone, and the namespace event window had rolled past the first failure — so the reason was unrecoverable from the cluster.The only surviving evidence that anything was wrong was the CronJob's
lastSuccessfulTime. Each Job failed 39 seconds in, so this was never a timeout or a hang: the logs would have said exactly what it was, and they were thrown away.The consequence was that the edge silently stopped accepting chart upgrades — including any security fix — while looking healthy from every angle anyone checks.
Both CronJobs, not just the one that broke
image-refreshcarried the same policy on a 15-minute schedule — the same blind spot at four times the frequency. Fixing only the instance that happened to fail would have left the class.The guard
scripts/tests/cronjob-failures-are-readable.shreads every CronJob out of the rendered manifests, so a third one is covered the day it lands rather than when someone remembers. It holds no list of CronJob names.Fails closed twice:
kind: CronJobthat no value combination in the render matrix reaches is reported UNREACHED by name and fails, rather than passing silently as "nothing to check".Wired into
DRIFT_GUARDS(now 21 entries — thedrifttarget counts its iterations and refuses to report green on fewer, so adding a guard without wiring it cannot pass).Scoped to
CronJobdeliberately, with the argument recorded at the declaration rather than assumed: a Helm hookJobloses its Pod the same way, but its failure fails the release and is reported to whoever ran it — somebody is already looking. A CronJob's failure is reported to nobody and repeats forever, which is what makes unreadable logs fatal rather than inconvenient. Widening it would need that argument made.Mutation evidence
Four mutations, each run with the baseline confirmed green afterwards:
OnFailurerestartPolicyremoved entirelyOnFailure)[UNREACHED]On the two tests that changed
auto_upgrade_test.yamlandimage_refresh_test.yamleach explicitly assertedOnFailure. They were right to pin it — a policy that matters should be pinned — so they are updated rather than deleted, and the comment now records why the pinned value changed and where the class-level sweep lives.Checks
make helm-unittest— 579 passed, 34 suitesmake drift— all 21 guards green (verified the new one actually executed, not just that the count matched)make helm-lint,make helm-template— cleanshellcheckon the new guard — cleanNote on rollout
This change only takes effect on an edge once that edge receives the chart. An edge whose auto-upgrade is already wedged cannot pull it — so unwedging those is separate operational work, tracked in the ticket. The immediate mitigation there is a direct
kubectl patchof the CronJob'srestartPolicy, which this change makes permanent and unnecessary going forward.Ticket: tracebloc/backend#2620
Note
Low Risk
Operational observability fix for scheduled jobs with no auth or data-path changes; slightly more failed Pods retained per Job tick, which is intentional.
Overview
Fixes backend#2620: chart CronJobs used
restartPolicy: OnFailure, which lets Kubernetes delete the Pod oncebackoffLimitis exhausted whilefailedJobsHistoryLimitonly keeps the Job—so repeated failures left no logs (observed on a prod edge with hourly auto-upgrade failures for 2.7 days).auto-upgradeandimage-refreshCronJob pod templates now setrestartPolicy: Neverso each attempt keeps a Pod for debugging. Chart bumps to 1.9.74.A new required drift guard
scripts/tests/cronjob-failures-are-readable.shrenders client/ingestor charts and requires every rendered CronJob to declareNever(or fails on missing policy / unreached CronJob templates). It is wired intoDRIFT_GUARDS. Helm unittest suites for both CronJobs pinNeverinstead ofOnFailure.Rollout note: edges already wedged on auto-upgrade still need operational unwedging; this only applies after they receive the chart (or a
kubectl patchon the CronJob).Reviewed by Cursor Bugbot for commit 92b0f6e. Bugbot is set up for automated code reviews on this repo. Configure here.