Skip to content

fix(doctor): surface container restart history, not just crash-loop-now (#1028 item 4)#231

Open
LukasWodka wants to merge 1 commit into
developfrom
fix/1028-doctor-restart-counts
Open

fix(doctor): surface container restart history, not just crash-loop-now (#1028 item 4)#231
LukasWodka wants to merge 1 commit into
developfrom
fix/1028-doctor-restart-counts

Conversation

@LukasWodka

@LukasWodka LukasWodka commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What

cluster doctor reported crash-loops from current pod state only, so it missed pods that had already restarted (crashed) earlier — backend#1028 (Item 4 of the prod-audit lows).

Fix

  • New additive checkRestartHistory — scans both init and regular container statuses across the tracebloc pods and warns when any container's RestartCount >= 3, naming each offender + a kubectl logs --previous remedy. Matches the existing doctor check/Result pattern.
  • Threshold 3: one or two restarts are routine (node drain, image warm-up, first-boot dep, a one-off probe miss); 3+ is a genuine flap.
  • The crash-loop-now check is unchanged — this is a restart-history signal alongside it.

Test

TestCheckRestartHistory: no-restarts / below-threshold → OK; at/above-threshold → Warn; init-container restarts → Warn; crash-loop-now pod (RestartCount 0) → OK here (proves no overlap). go build/test/vet/gofmt clean.

Refs backend#1008. (Items 1/2/3 — digest pin, OOM-orphan sweep, correlation id — tracked separately.)

🤖 Generated with Claude Code


Note

Low Risk
Read-only diagnostic addition with warn-only severity; existing pod crash-loop detection is unchanged.

Overview
tracebloc cluster doctor now includes a Restart history check so operators see containers that have flapped even when they are healthy right now.

The existing Pod health check only treats active CrashLoopBackOff as failure, so pods that restarted several times and recovered (or jobs that retried before succeeding) could still look fine. The new check lists pods in the namespace, scans init and app container statuses, and emits when any RestartCount is ≥ 3 (threshold chosen to avoid noise from 1–2 routine restarts). It names each pod/container and suggests kubectl logs … --previous; it does not replace or duplicate the crash-loop-now failure path.

Tests cover threshold edge cases, init-container restarts, and that crash-looping pods stay this check’s responsibility vs Pod health; the integration test expects 9 checks instead of 8.

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

…ow (#1028)

checkRestartHistory warns when any init/regular container RestartCount >= 3 — the current-state check missed pods that already crashed. Additive. Refs #1008.

@aptracebloc aptracebloc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving. Clean, low-risk additive check — warn-only, correctly-motivated, and well-tested. Two non-blocking notes below.

Nice separation from podCrashLooping: RestartCount is the wrong signal for crash-loop-now (recovered/completed pods carry high counts) but the right one for the history signal this check is about, and it's honest ("may be flapping even if it's up now") and capped at ⚠ so it never overrides the crash-loop-now failure. Threshold 3 is well-reasoned, and it matches the doctor Result pattern (init+app scan, list-error → Warn, sorted output).

Non-blocking notes

  1. No pod-phase / recency filter. It counts RestartCount on all pods present, including Succeeded/Failed (completed jobs) — so a long-finished job that retried ≥3 times keeps this check ⚠ until that pod is GC'd, which could be persistent noise where completed pods linger. Contrast podCrashLooping, which deliberately excludes terminal phases. Intended per the "history" framing, but worth a thought (filter to non-terminal pods, or accept the tradeoff).
  2. "No overlap" is really "independent, not gating." A real crash-looping pod has RestartCount ≥ 3, so it'll trip both this ⚠ and checkPods' ✖ — acceptable (different facets; Worst() stays ✖), but the test's RestartCount-0 crash-loop doesn't reflect actual crash-loopers, so the "no overlap" claim is a touch optimistic in practice.

Neither blocks — approving as-is.

— drafted with Claude (Opus 4.8), sent by @aptracebloc

Sign up for free to 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