fix(doctor): node-fit checks FREE memory, not allocatable (backend#2870)

[LukasWodka]

Part of tracebloc/backend#2870 — the cli-doctor layer (DoD item 3).

The bug

checkNodeFit (the "Node capacity" check) fit the training envelope against each Ready node's Allocatable, never against what is already requested on it. So a node whose own control plane claims memory passed the check — allocatable said "a Ready node can schedule a training job" — while the pod then went Pending / Insufficient memory. The ticket's framing: no layer owned the question "is this number schedulable here?", and the one value that would expose it — free memory — was computed elsewhere and thrown away.

The fix

Sum the requests already on each Ready node and fit cpu + memory against FREE (allocatable − requested, floored at 0):

• A node large enough by allocatable but not beside its own control plane now FAILs, with a message that names the over-commit — distinct from "no node is big enough at all".
• Fail-closed: if the pod list can't be read, free is unknown — the check says so and falls back to allocatable rather than passing it off as free.
• Terminal pods (Succeeded/Failed) hold nothing and are skipped (mirrors requestedMemory).

Verification

• go test ./internal/doctor/ ./internal/cli/ — green. New TestCheckNodeFitFreeMemory: a 16Gi node whose control plane requests 12Gi (4Gi free) refuses an 8Gi envelope; the same node without the neighbour passes (the subtraction is load-bearing); a terminal neighbour is ignored.
• Mutation-proven: dropping the memory subtraction flips the over-commit case to a false OK.
• gofmt/go vet clean. The copy-catalog golden was regenerated — it added only the two new over-commit strings.

Scope (what remains on backend#2870)

This is one of five DoD items. Still open: the installer-side refusal (1), the footprint schedulability test tied to the #2460 overhead constant (2 — currently red-by-construction until that lands), the jobs-manager permanent-vs-transient waiting_for_capacity message (4), and the CPU envelope audit (5). This PR makes the class detectable at the diagnostic layer without depending on the #2460 measurement.

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes doctor readiness verdicts, exit semantics, and remediation text; cluster-wide pod listing adds an RBAC dependency. Behavior is heavily tested but rollup ordering is subtle.

Overview
tracebloc doctor now judges whether a training job can schedule using free CPU/memory on each node (allocatable minus steady-state pod requests), not raw allocatable. That surfaces over-committed machines where the control plane already consumes the headroom and jobs stay Pending despite looking “big enough.”

The Node capacity check lists pods cluster-wide (skipping terminal and batch Job pods), adds a distinct Fail with its own remedy, and warns instead of passing when the pod list is unreadable. The drift nudge caps suggested run sizes by free memory and names resources set --cores/--memory when set max would disagree with the printed ceiling.

summarizeDoctor adds a rollup arm for the over-commit Fail (explicitly not recommending resources set max) and places it above stuck-Pending pod warnings so co-occurring symptoms get the right top-line advice. Shared prefixes OverCommitted and CantVerifyFreeCompute keep classification in sync. Version 0.10.22; tests and copy golden updated.

^{Reviewed by Cursor Bugbot for commit c3d8d4d. Bugbot is set up for automated code reviews on this repo. Configure here.}

[LukasWodka]

bugbot run

[saqlainsyed007]

Holding — CI is red (bugbot / review failing) and three Bugbot threads are open, so this isn't landable yet. I verified the top one against the code rather than taking it on trust:

"Doctor fails during healthy training" (High) is real. checkNodeFit sums requests from every non-terminal, node-assigned pod — which includes a currently running training job that already holds the envelope — then tests whether a fresh cpuReq/memReq fits in the remainder. On a single-node install with a healthy training job running, freeMemB = allocatable − (that job's request) is below the envelope, so nodeCPUMem goes false, the node reports as not fitting, and doctor exits 2 while training is fine. That's a false negative on the exact healthy state doctor is meant to bless.

The other two are worth a look while you're in there:

• "Unknown free reported as schedulable" (High): the code comment says the unknown-free caveat "is reported below" — please confirm the StatusOK path actually emits it when freeKnown == false, since that's precisely what the finding says is missing.
• "Over-commit message always blames memory" (Medium): the over-commit copy naming FREE memory unconditionally will misreport a CPU or ephemeral-storage shortfall.

Requesting changes; happy to re-review as soon as these are addressed and CI is green.

[LukasWodka]

bugbot run

[saqlainsyed007]

Thanks for the fast turnaround — my three original findings are genuinely addressed on this commit: running jobs are excluded from the free sum via the job-name label (no more false-fail during healthy training), unknown-free now returns StatusWarn rather than a clean OK, and the over-commit message tracks overCPU/overMem.

Still holding, because Bugbot's pass on the new commit caught two real follow-ons to those same two fixes — and I've confirmed both against the code:

• Disk shortfall still blamed on memory (doctor.go:769): the over-commit arm fires on !cpuMemFits && allocOnlyFit, but allocOnlyFit only inspects allocatable cpu+memory — a node that fits cpu+memory and fails only on ephemeral-storage still takes this arm, and overCPU/overMem are both false, so short defaults to "memory". The message fix covered cpu but not the disk dimension — the same misdiagnosis class TestCheckNodeFitDisk exists to prevent.
• Rollup still greens unverified free capacity (doctor.go:818): the new StatusWarn detail begins a Ready node can schedule a training job, but summarizeDoctor only treats the two older Node-capacity Warn prefixes as a can't-check — so it falls through to "Ready to run training" and prints "Everything looks good" at exit 0 on the default path. The Warn is there but the rollup doesn't act on it, so the over-committed control plane this PR exists to surface stays invisible without --verbose.

Fold the ephemeral-storage dimension into the short-dimension logic, and teach summarizeDoctor to treat the new Warn prefix as a can't-check, and this is good to go. CI is also red on bugbot / review for these.

[LukasWodka]

bugbot run

[LukasWodka]

bugbot run

[LukasWodka]

bugbot run

[saqlainsyed007]

Good progress — everything from my earlier passes is genuinely in: running jobs excluded from the free sum, unknown-free warns (and now, in this commit, that can't-check correctly OUTRANKS the soft GPU warn via the CantVerifyFreeCompute prefix so summarizeDoctor no longer greens it), disk-vs-memory shortfall named, and the shared OverCommitted/CantVerifyFreeCompute prefixes tidy that up nicely.

One open item, and I confirmed it against the code — Bugbot's "OK nudge still quotes allocatable max" (doctor.go:832) is real: the fit now moves to FREE, but the StatusOK path still appends the resources set max nudge computed from allocatable-minus-overhead (maxCores/maxGiB). On a node that's large enough but already partly claimed, that advertised ceiling can exceed free — so an operator who follows "size up to cpu=…,memory=…Gi" recreates exactly the over-commit this check now fails on, and the next doctor run tells them not to size to the machine. The nudge should be bounded by free (or suppressed when free < the allocatable-derived max) so the advice can't contradict the verdict. Once that's addressed it's good to go — CI is green and every other thread is resolved.

[LukasWodka]

bugbot run

[saqlainsyed007]

My own asks are all addressed now — the resize nudge is bounded by free and suppressed when free is unknown, exactly right, and I'm satisfied with that.

The one thing still blocking is Bugbot's new finding (doctor.go:861, "Nudge ceiling diverges from set max"), and I think it's a fair one: the nudge now prints a free-bounded ceiling, but the command it points the operator at — resources set max — sizes from allocatable via nodeLarger, so on a claimed node the number shown and the number set max would actually apply disagree, and following the parenthetical reapplies the over-commit this check now refuses. Either recompute the advertised ceiling the same way set max will (or teach set max/nodeLarger to bound by free too), or drop the resources set max pointer from the free-bounded nudge so the display and the command can't contradict. If you consider set max's allocatable sizing out of scope for this PR, say so on the Bugbot thread and we can treat it as a follow-up — either way that thread needs closing before this can land. Everything else is green and resolved.

[LukasWodka]

bugbot run

[LukasWodka]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit c3d8d4d. Configure here.}

[saqlainsyed007]

Approving — this supersedes my earlier change-request. The Bugbot precedence finding is genuinely fixed, not papered over, and I verified the fix and its tests against the code.

The reorder is the correct fix: the over-commit Node capacity Fail arm now sits ABOVE the stuck-Pending Pod health Warn arm. That ordering is a correctness requirement, not a preference — the two states co-occur by construction (the over-commit producer's own Detail ends "so the pod schedules Pending", internal/doctor/doctor.go:778), so an over-committed node is expected to also show a stuck-Pending pod. With the old ordering the stuck-Pending arm matched first and printed computeRemedy → resources set max, putting the exact remedy this Fail exists to refuse back in front of the operator. The earlier commit fixed the figure; this fixes the rollup that was still recommending it. Only a hard Pod health Fail (pods not running at all — a different, reinstall-class problem, not caused by over-commit) still outranks it, which is right.

The two added tests pin both directions and are mutation-proof:

• "over-commit outranks stuck-Pending, which it CAUSES" sets BOTH Node capacity=Fail(OverCommitted) and Pod health=Warn(stuck Pending) and asserts the remedy has no resources set max, carries the over-commit top line ("already claimed the room"), and the "Do NOT" wording — so a regression to the old ordering reddens it.
• "a hard Pod-health Fail still outranks over-commit" is the bound: it proves the reorder didn't slide up past a harder failure.
  Both build their details from the producer's own doctor.OverCommitted constant rather than retyping, so a reworded producer reddens them instead of silently falling through.

CI green (28 pass), no open threads.

[LukasWodka]

/fr-pass

Functional review on staging — passed.

Tier-A journey against staging (this hop deployed), run 33724330593: install → cluster-anchor → ingest (15/16 task types) → use case → train → leaderboard (accuracy=0.9, row on the board). This repo's change is on that runtime path, so this is direct functional evidence.

Two caveats, stated rather than glossed:

• object_detection ingest is mid-migration and NOT covered here. data-ingestors#552 (Phase 1 of backend#1006) now enumerates Pascal-VOC XML and rejects the legacy labels.csv; cli (Phase 2) / backend (Phase 3) are pending. The journey's object_detection path fails on that intended change, so it is not part of this evidence.
• No KMS authorization. This hop carries STAGE 1.5 (read-decrypt-always) with prod values at null; the STAGE-2 flip is reverted/parked. Ready for prod here does not authorize the weight-store KMS flip, which stays gated on STAGE 1.5's prod deploy.