Uh oh!
There was an error while loading. Please reload this page.
fix(#555): add activeDeadlineSeconds to the auto-upgrade Job - #620
Merged
Conversation
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
divyasinghdsforce-pushed
the
fix/555-auto-upgrade-deadline
branch
from
August 6, 2026 11:03
40071a2 to
e7f51c5CompareThere was a problem hiding this comment.
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 e7f51c5. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
shujaatTracebloc
previously approved these changes
Aug 6, 2026
shujaatTracebloc
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the code: LGTM.
- New
tracebloc.durationSecondshelper parses Go/Helm duration strings (sums<int><unit>components, handles compound like1h30m, empty→0) — correct. activeDeadlineSeconds: max($deadlineFloor, parsed timeout + 300s)never undercuts a longer configuredautoUpgrade.timeout, so a healthyhelm upgrade --waitcan't be killed early; the 900s floor backstops a hunghelm repo add/updateunder concurrencyPolicy: Forbid. Sound.- schema + values.yaml wire the new knob (integer, min 1, default 900); Chart bumped 1.9.21→1.9.22. No unresolved Bugbot/review threads.
All checks green except the org-wide pii-gate (PII_DENYLIST secret unset, backend#1409) — a known infra false-positive tracked separately, not a concern in this diff. Satisfies #555.
autoUpgrade.timeout ("10m") bounds only `helm upgrade --wait`, not the
earlier `helm repo add`/`helm repo update` HTTPS fetches. On a TCP black-hole
(a corporate proxy that drops packets with no RST) those fetches can hang
with no hard timeout; with concurrencyPolicy: Forbid the hung Job then makes
every future scheduled tick "still active" and skipped, wedging auto-upgrade
until a human kills the pod.
Add activeDeadlineSeconds (new autoUpgrade.activeDeadlineSeconds knob,
default 900s) to the Job spec as a hard wall-clock backstop, mirroring
storage-assertions-check.yaml. Kept comfortably above the 600s helm timeout
plus repo-update headroom so a slow-but-healthy upgrade is never killed
mid-flight. Also declared in values.schema.json alongside the sibling knobs.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>Bump chart version 1.9.21 -> 1.9.22 so the chart-content change can be
published (CI gate), and keep appVersion in lockstep with version
("1.9.22") per the repo convention — the previous divergence was the
Bugbot finding.
Default autoUpgrade.activeDeadlineSeconds to 900 in the CronJob template
so a --reuse-values upgrade that omits the new key still renders a valid
integer instead of an empty/invalid value.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>Bugbot: the fixed default activeDeadlineSeconds (900s) could kill a healthy `helm upgrade --wait` when autoUpgrade.timeout was configured longer (e.g. 30m), firing before helm's own timeout. Render activeDeadlineSeconds as max(configured floor, parsed timeout + 300s repo-fetch headroom) via a new tracebloc.durationSeconds helper. The 300s mirrors the baked-in 900-600 default headroom, so the default case is a no-op (900) and the `| default 900` reuse-values nil-guard is preserved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
divyasinghdsforce-pushed
the
fix/555-auto-upgrade-deadline
branch
from
August 7, 2026 06:19
80543a7 to
a362b76CompareshujaatTracebloc
approved these changes
Aug 7, 2026
shujaatTracebloc
left a comment
Contributor
There was a problem hiding this comment.
LGTM. Correctly caps the auto-upgrade Job's wall-clock time to unwedge the Forbid concurrency policy when a helm repo fetch hangs (#555).
activeDeadlineSeconds: max(floor, parsed timeout + 300s)correctly never undercuts a longer configuredautoUpgrade.timeout, so a healthyhelm upgrade --waitwon't be killed early.- New
tracebloc.durationSecondshelper parses compound durations and nil-guards to 0. - Chart bumped 1.9.23 → 1.9.24; schema + values documented.
- All CI green incl. Fleet auto-upgrade E2E (k3d); Cursor Bugbot clean.
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes#555.
Fix
Add
activeDeadlineSecondsto the auto-upgrade Job spec (newautoUpgrade.activeDeadlineSeconds, default 900) so a hunghelm repo updatecan no longer run unbounded.Files
client/templates/auto-upgrade-cronjob.yaml,client/values.yaml,client/values.schema.jsonValidation
helm template, schema JSON valid.🤖 Generated with Claude Code
Note
Medium Risk
Changes only the auto-upgrade CronJob lifecycle timing on fleet edges; logic is designed not to kill in-flight upgrades before
helm upgrade --waittimes out, but mis-tuned deadlines could still fail long upgrades.Overview
Fixes #555 by giving the auto-upgrade CronJob Job a hard wall-clock limit so a stuck
helm repo add/helm repo updatecannot block every future run underconcurrencyPolicy: Forbid.The Job now sets
activeDeadlineSecondstomax(autoUpgrade.activeDeadlineSeconds, parsed timeout + 300s), with a new configurable floor (default 900s) and atracebloc.durationSecondshelper that turnsautoUpgrade.timeout(e.g.10m) into seconds. Chart version bumps to 1.9.24; values and JSON schema document the new knob.Reviewed by Cursor Bugbot for commit a362b76. Bugbot is set up for automated code reviews on this repo. Configure here.