fix(#563): back off image-refresh on a flapping-readiness deployment - #626
Conversation
…bound annotate (#626 review) Address PR #626 review: - Backoff was a dead end: after MAX_REFRESH_ATTEMPTS failures the tick exited before restarting forever, so a deployment that later became healthy/settled never got the reconciling rollout and stayed on the old image until a newer digest appeared. Now, while in back-off (which is only reached on a tick that already passed the settled guard), each settled observation advances a cooldown; after BACKOFF_RESUME_AFTER settled ticks the attempt counter resets and refresh retries. A cleared flap resumes and reaches `recorded == latest`; a still-flapping deployment is retried at most once per cooldown, never every tick. - Bound the new pre-restart `kubectl annotate` calls with --request-timeout=15s (repo rule for non-watch kubectl). - values.yaml comment updated to describe the resume-on-settle behaviour. - Chart.yaml version 1.9.15 -> 1.9.16 (chart content changed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…bound annotate (#626 review) Address PR #626 review: - Backoff was a dead end: after MAX_REFRESH_ATTEMPTS failures the tick exited before restarting forever, so a deployment that later became healthy/settled never got the reconciling rollout and stayed on the old image until a newer digest appeared. Now, while in back-off (which is only reached on a tick that already passed the settled guard), each settled observation advances a cooldown; after BACKOFF_RESUME_AFTER settled ticks the attempt counter resets and refresh retries. A cleared flap resumes and reaches `recorded == latest`; a still-flapping deployment is retried at most once per cooldown, never every tick. - Bound the new pre-restart `kubectl annotate` calls with --request-timeout=15s (repo rule for non-watch kubectl). - values.yaml comment updated to describe the resume-on-settle behaviour. - Chart.yaml version 1.9.15 -> 1.9.16 (chart content changed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
0273d97 to
7703c4b
Compare
The attempt-counter read used `get_annotation ... || true`, and get_annotation piped `kubectl | jq` with no pipefail. A failed kubectl/jq therefore collapsed to an empty value that the back-off logic read as "no prior attempts" (0) -- failing OPEN past the guard and re-restarting a possibly-flapping deployment (violates the no-fail-open-guards rule). get_annotation now captures kubectl separately and returns non-zero on either a kubectl or jq error, so a caller can distinguish a genuinely absent annotation (success + empty output -> 0 attempts) from a read error (non-zero return). The attempt read takes the SAFE branch on a read error: skip the restart this tick rather than churn the deployment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…y-one (#626 review) Two back-off bugs in the image-refresh cooldown logic: 1. Cooldown resume restarted in bursts (CID 3728456918). On resume the counter was reset to 0, handing a still-flapping deployment a fresh budget of MAX_REFRESH_ATTEMPTS restarts across the next consecutive settled ticks -- a ReplicaSet-churn burst that broke the "at most once per cooldown" invariant. Now resume sets prev_count = MAX-1 so exactly ONE restart is issued; if it fails, the persisted count is MAX and the next settled tick re-enters back-off immediately. A healthy deployment still succeeds on that one attempt and clears the counter. 2. Cooldown off-by-one delayed resume (CID 3728456929). The branch compared settled_ticks (excluding the current tick) against BACKOFF_RESUME_AFTER, so the tick logging "Settled 4/4" still incremented and exited, and resume happened only the following tick (~15m late). Now it compares the count INCLUDING this settled tick, so 4/4 resumes on that tick, matching the log and the "after BACKOFF_RESUME_AFTER settled ticks" contract. Both fixes reuse the existing single-counter encoding; no new state added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…wedged API fails closed, not hangs (#626 Bugbot CID 3728757993) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…bound annotate (#626 review) Address PR #626 review: - Backoff was a dead end: after MAX_REFRESH_ATTEMPTS failures the tick exited before restarting forever, so a deployment that later became healthy/settled never got the reconciling rollout and stayed on the old image until a newer digest appeared. Now, while in back-off (which is only reached on a tick that already passed the settled guard), each settled observation advances a cooldown; after BACKOFF_RESUME_AFTER settled ticks the attempt counter resets and refresh retries. A cleared flap resumes and reaches `recorded == latest`; a still-flapping deployment is retried at most once per cooldown, never every tick. - Bound the new pre-restart `kubectl annotate` calls with --request-timeout=15s (repo rule for non-watch kubectl). - values.yaml comment updated to describe the resume-on-settle behaviour. - Chart.yaml version 1.9.15 -> 1.9.16 (chart content changed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The attempt-counter read used `get_annotation ... || true`, and get_annotation piped `kubectl | jq` with no pipefail. A failed kubectl/jq therefore collapsed to an empty value that the back-off logic read as "no prior attempts" (0) -- failing OPEN past the guard and re-restarting a possibly-flapping deployment (violates the no-fail-open-guards rule). get_annotation now captures kubectl separately and returns non-zero on either a kubectl or jq error, so a caller can distinguish a genuinely absent annotation (success + empty output -> 0 attempts) from a read error (non-zero return). The attempt read takes the SAFE branch on a read error: skip the restart this tick rather than churn the deployment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…y-one (#626 review) Two back-off bugs in the image-refresh cooldown logic: 1. Cooldown resume restarted in bursts (CID 3728456918). On resume the counter was reset to 0, handing a still-flapping deployment a fresh budget of MAX_REFRESH_ATTEMPTS restarts across the next consecutive settled ticks -- a ReplicaSet-churn burst that broke the "at most once per cooldown" invariant. Now resume sets prev_count = MAX-1 so exactly ONE restart is issued; if it fails, the persisted count is MAX and the next settled tick re-enters back-off immediately. A healthy deployment still succeeds on that one attempt and clears the counter. 2. Cooldown off-by-one delayed resume (CID 3728456929). The branch compared settled_ticks (excluding the current tick) against BACKOFF_RESUME_AFTER, so the tick logging "Settled 4/4" still incremented and exited, and resume happened only the following tick (~15m late). Now it compares the count INCLUDING this settled tick, so 4/4 resumes on that tick, matching the log and the "after BACKOFF_RESUME_AFTER settled ticks" contract. Both fixes reuse the existing single-counter encoding; no new state added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
591ef71 to
bcc4803
Compare
…wedged API fails closed, not hangs (#626 Bugbot CID 3728757993) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ilure counter (#626 Bugbot CID 3728806670) Simplify the over-engineered cooldown/auto-resume/signature machinery down to the minimal design #563 calls for. The single-integer annotation is no longer overloaded (signature + count + cooldown offset); it is now a plain count of consecutive failed refresh attempts that means one thing. Removed: - the cooldown counter, BACKOFF_RESUME_AFTER, cooldown_seen/settled-tick counting, and the resume-to-MAX-1 single-restart logic (auto-resume); - the signature built from annotate_args and its target-change reset. This is the fragile "partial resolve resets back-off" path (Bugbot CID 3728806670) -- deleting it eliminates the finding. Kept (the correct #563 core): - settled-guard short-circuit for unsettled deployments; - digest-change detection (refresh only when the resolved digest changed); - fail-closed annotation reads (skip the tick on a kubectl/jq read error, never treat an error as "0 attempts"); - --request-timeout=15s on every non-watch kubectl call. New semantics: increment the counter (persisted before the restart) when a rollout is issued but does not settle; reset to 0 ONLY on a genuinely successful settled rollout; after MAX_REFRESH_ATTEMPTS in a row, STOP restarting, WARN, and annotate tracebloc.io/refresh-flap-detected for a human / monitoring. No auto-resume -- a human clears the counter (or a rollout that finally settles resets it). Net -30 lines. helm lint + template render clean; embedded script passes sh -n / dash -n; tick-by-tick simulation confirms flap->stop+flag (no burst), healthy->reset, read-error->skip, new-digest->refresh, unsettled->short-circuit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Simplified the image-refresh flap handling per maintainer direction. Dropped the auto-resume / cooldown / signature-reset machinery (BACKOFF_RESUME_AFTER, cooldown/settled-tick counting, resume-to-MAX-1, and the annotate_args signature target-change reset) in favor of a plain consecutive-failure counter:
Kept the #563 core: settled-guard, digest-change detection, fail-closed annotation reads, and |
There 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 bcc4803. Configure here.
…bound annotate (#626 review) Address PR #626 review: - Backoff was a dead end: after MAX_REFRESH_ATTEMPTS failures the tick exited before restarting forever, so a deployment that later became healthy/settled never got the reconciling rollout and stayed on the old image until a newer digest appeared. Now, while in back-off (which is only reached on a tick that already passed the settled guard), each settled observation advances a cooldown; after BACKOFF_RESUME_AFTER settled ticks the attempt counter resets and refresh retries. A cleared flap resumes and reaches `recorded == latest`; a still-flapping deployment is retried at most once per cooldown, never every tick. - Bound the new pre-restart `kubectl annotate` calls with --request-timeout=15s (repo rule for non-watch kubectl). - values.yaml comment updated to describe the resume-on-settle behaviour. - Chart.yaml version 1.9.15 -> 1.9.16 (chart content changed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The attempt-counter read used `get_annotation ... || true`, and get_annotation piped `kubectl | jq` with no pipefail. A failed kubectl/jq therefore collapsed to an empty value that the back-off logic read as "no prior attempts" (0) -- failing OPEN past the guard and re-restarting a possibly-flapping deployment (violates the no-fail-open-guards rule). get_annotation now captures kubectl separately and returns non-zero on either a kubectl or jq error, so a caller can distinguish a genuinely absent annotation (success + empty output -> 0 attempts) from a read error (non-zero return). The attempt read takes the SAFE branch on a read error: skip the restart this tick rather than churn the deployment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bcc4803 to
a4d48a6
Compare
…y-one (#626 review) Two back-off bugs in the image-refresh cooldown logic: 1. Cooldown resume restarted in bursts (CID 3728456918). On resume the counter was reset to 0, handing a still-flapping deployment a fresh budget of MAX_REFRESH_ATTEMPTS restarts across the next consecutive settled ticks -- a ReplicaSet-churn burst that broke the "at most once per cooldown" invariant. Now resume sets prev_count = MAX-1 so exactly ONE restart is issued; if it fails, the persisted count is MAX and the next settled tick re-enters back-off immediately. A healthy deployment still succeeds on that one attempt and clears the counter. 2. Cooldown off-by-one delayed resume (CID 3728456929). The branch compared settled_ticks (excluding the current tick) against BACKOFF_RESUME_AFTER, so the tick logging "Settled 4/4" still incremented and exited, and resume happened only the following tick (~15m late). Now it compares the count INCLUDING this settled tick, so 4/4 resumes on that tick, matching the log and the "after BACKOFF_RESUME_AFTER settled ticks" contract. Both fixes reuse the existing single-counter encoding; no new state added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…wedged API fails closed, not hangs (#626 Bugbot CID 3728757993) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ilure counter (#626 Bugbot CID 3728806670) Simplify the over-engineered cooldown/auto-resume/signature machinery down to the minimal design #563 calls for. The single-integer annotation is no longer overloaded (signature + count + cooldown offset); it is now a plain count of consecutive failed refresh attempts that means one thing. Removed: - the cooldown counter, BACKOFF_RESUME_AFTER, cooldown_seen/settled-tick counting, and the resume-to-MAX-1 single-restart logic (auto-resume); - the signature built from annotate_args and its target-change reset. This is the fragile "partial resolve resets back-off" path (Bugbot CID 3728806670) -- deleting it eliminates the finding. Kept (the correct #563 core): - settled-guard short-circuit for unsettled deployments; - digest-change detection (refresh only when the resolved digest changed); - fail-closed annotation reads (skip the tick on a kubectl/jq read error, never treat an error as "0 attempts"); - --request-timeout=15s on every non-watch kubectl call. New semantics: increment the counter (persisted before the restart) when a rollout is issued but does not settle; reset to 0 ONLY on a genuinely successful settled rollout; after MAX_REFRESH_ATTEMPTS in a row, STOP restarting, WARN, and annotate tracebloc.io/refresh-flap-detected for a human / monitoring. No auto-resume -- a human clears the counter (or a rollout that finally settles resets it). Net -30 lines. helm lint + template render clean; embedded script passes sh -n / dash -n; tick-by-tick simulation confirms flap->stop+flag (no burst), healthy->reset, read-error->skip, new-digest->refresh, unsettled->short-circuit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… of the digest write (#626 Bugbot CID 3729110045) On a successful settled rollout the consecutive-failure counter (tracebloc.io/refresh-attempt) was cleared only by being batched into the final digest-record annotate, which had no --request-timeout. If that annotate hung or failed under `set -e`, the counter stayed elevated and the digest went unrecorded, so later ticks re-restarted, burned the budget, and could permanently trip the flap lockout while the CronJob stayed green. Clear the counter (and flap marker) in its own --request-timeout=15s annotate BEFORE the digest record, so the success reset is bounded and no longer contingent on the digest write succeeding. Also bound the digest-record annotate and the rollout restart with --request-timeout=15s. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
shujaatTracebloc
left a comment
There was a problem hiding this comment.
Reviewed the code and all checks: LGTM.
- Consecutive-failure counter (
refresh-attempt) persisted BEFORE the restart so a rollout that exits the tick underset -estill advances it; resets to 0 only on a genuinely settled rollout. Sound flap guard (#563). - Success reset is its own bounded annotate BEFORE the digest-record write (CID 3729110045), so a hung/failed digest annotate can no longer strand an elevated count and permanently trip the flap lockout.
get_annotationreads FAIL-CLOSED: kubectl captured separately from the jq pipe (no pipefail here) and returns non-zero on either error, so a read failure skips the tick rather than collapsing to "0 attempts" and re-restarting a flapping deployment.- All kubectl calls now
--request-timeout-bounded; schema + values.yaml exposemaxRefreshAttempts(default 3); Chart bumped 1.9.22→1.9.23.
Green across the board; no unresolved Bugbot/review threads. Satisfies #563.
A deployment whose readiness oscillates (becomes Ready, then crashes after warmup right around the rollout-status timeout) fails the acting tick's `rollout status`. Under `set -e` the tick exits before the success annotation lands, so `recorded` never advances to the new digest and every later settled tick re-issues the identical `rollout restart` (~every schedule) — churning ReplicaSets on a genuinely flapping deployment. Record the ATTEMPT (target-digest signature + a consecutive-failure count) in a `tracebloc.io/refresh-attempt` annotation BEFORE the restart, so the counter survives the set -e exit. After imageRefresh.maxRefreshAttempts (default 3) in-a-row failures for the same target, skip the restart and surface the flapping deployment instead of re-restarting forever. The counter resets when the target digest changes or a rollout finally succeeds. New value + schema entry + env wiring; default installs stay byte-identical apart from the new env var. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…bound annotate (#626 review) Address PR #626 review: - Backoff was a dead end: after MAX_REFRESH_ATTEMPTS failures the tick exited before restarting forever, so a deployment that later became healthy/settled never got the reconciling rollout and stayed on the old image until a newer digest appeared. Now, while in back-off (which is only reached on a tick that already passed the settled guard), each settled observation advances a cooldown; after BACKOFF_RESUME_AFTER settled ticks the attempt counter resets and refresh retries. A cleared flap resumes and reaches `recorded == latest`; a still-flapping deployment is retried at most once per cooldown, never every tick. - Bound the new pre-restart `kubectl annotate` calls with --request-timeout=15s (repo rule for non-watch kubectl). - values.yaml comment updated to describe the resume-on-settle behaviour. - Chart.yaml version 1.9.15 -> 1.9.16 (chart content changed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The attempt-counter read used `get_annotation ... || true`, and get_annotation piped `kubectl | jq` with no pipefail. A failed kubectl/jq therefore collapsed to an empty value that the back-off logic read as "no prior attempts" (0) -- failing OPEN past the guard and re-restarting a possibly-flapping deployment (violates the no-fail-open-guards rule). get_annotation now captures kubectl separately and returns non-zero on either a kubectl or jq error, so a caller can distinguish a genuinely absent annotation (success + empty output -> 0 attempts) from a read error (non-zero return). The attempt read takes the SAFE branch on a read error: skip the restart this tick rather than churn the deployment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…y-one (#626 review) Two back-off bugs in the image-refresh cooldown logic: 1. Cooldown resume restarted in bursts (CID 3728456918). On resume the counter was reset to 0, handing a still-flapping deployment a fresh budget of MAX_REFRESH_ATTEMPTS restarts across the next consecutive settled ticks -- a ReplicaSet-churn burst that broke the "at most once per cooldown" invariant. Now resume sets prev_count = MAX-1 so exactly ONE restart is issued; if it fails, the persisted count is MAX and the next settled tick re-enters back-off immediately. A healthy deployment still succeeds on that one attempt and clears the counter. 2. Cooldown off-by-one delayed resume (CID 3728456929). The branch compared settled_ticks (excluding the current tick) against BACKOFF_RESUME_AFTER, so the tick logging "Settled 4/4" still incremented and exited, and resume happened only the following tick (~15m late). Now it compares the count INCLUDING this settled tick, so 4/4 resumes on that tick, matching the log and the "after BACKOFF_RESUME_AFTER settled ticks" contract. Both fixes reuse the existing single-counter encoding; no new state added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…wedged API fails closed, not hangs (#626 Bugbot CID 3728757993) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ilure counter (#626 Bugbot CID 3728806670) Simplify the over-engineered cooldown/auto-resume/signature machinery down to the minimal design #563 calls for. The single-integer annotation is no longer overloaded (signature + count + cooldown offset); it is now a plain count of consecutive failed refresh attempts that means one thing. Removed: - the cooldown counter, BACKOFF_RESUME_AFTER, cooldown_seen/settled-tick counting, and the resume-to-MAX-1 single-restart logic (auto-resume); - the signature built from annotate_args and its target-change reset. This is the fragile "partial resolve resets back-off" path (Bugbot CID 3728806670) -- deleting it eliminates the finding. Kept (the correct #563 core): - settled-guard short-circuit for unsettled deployments; - digest-change detection (refresh only when the resolved digest changed); - fail-closed annotation reads (skip the tick on a kubectl/jq read error, never treat an error as "0 attempts"); - --request-timeout=15s on every non-watch kubectl call. New semantics: increment the counter (persisted before the restart) when a rollout is issued but does not settle; reset to 0 ONLY on a genuinely successful settled rollout; after MAX_REFRESH_ATTEMPTS in a row, STOP restarting, WARN, and annotate tracebloc.io/refresh-flap-detected for a human / monitoring. No auto-resume -- a human clears the counter (or a rollout that finally settles resets it). Net -30 lines. helm lint + template render clean; embedded script passes sh -n / dash -n; tick-by-tick simulation confirms flap->stop+flag (no burst), healthy->reset, read-error->skip, new-digest->refresh, unsettled->short-circuit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… of the digest write (#626 Bugbot CID 3729110045) On a successful settled rollout the consecutive-failure counter (tracebloc.io/refresh-attempt) was cleared only by being batched into the final digest-record annotate, which had no --request-timeout. If that annotate hung or failed under `set -e`, the counter stayed elevated and the digest went unrecorded, so later ticks re-restarted, burned the budget, and could permanently trip the flap lockout while the CronJob stayed green. Clear the counter (and flap marker) in its own --request-timeout=15s annotate BEFORE the digest record, so the success reset is bounded and no longer contingent on the digest write succeeding. Also bound the digest-record annotate and the rollout restart with --request-timeout=15s. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
a4d48a6 to
be73029
Compare

Closes #563.
Fix
Record per-target attempt count before the restart and back off after
maxRefreshAttempts(default 3) consecutive failures, so a flapping-readiness deployment is no longer re-restarted every cycle.Files
client/templates/image-refresh-cronjob.yaml,client/values.yaml,client/values.schema.jsonValidation
helm template,sh -n, schema JSON valid.🤖 Generated with Claude Code
Note
Medium Risk
Changes production auto-refresh behavior for jobs-manager rollouts and deployment annotations; mis-tuned thresholds or annotation read failures could delay image updates until manual intervention.
Overview
Stops the image-refresh CronJob from endlessly
rollout restarting jobs-manager when digest drift is detected but rollouts never settle (readiness flaps). AftermaxRefreshAttemptsconsecutive failed attempts (default 3, configurable via values/schema), it skips further restarts, setstracebloc.io/refresh-flap-detected, and expects an operator to cleartracebloc.io/refresh-attemptor fix the image; a successful rollout clears both counters.Hardens the refresh script (#626):
get_annotationfails closed on kubectl/jq errors (no silent “0 attempts”), adds--request-timeout=15son deployment reads/annotates/restarts, and resets attempt/flap annotations in a separate annotate before recording digest annotations so a hung digest write cannot leave a false permanent flap lockout.Chart version 1.9.24 → 1.9.25.
Reviewed by Cursor Bugbot for commit be73029. Bugbot is set up for automated code reviews on this repo. Configure here.