fix(doctor): the node-capacity check compares the free memory it already computes, and tells a permanent shortage from a transient one (backend#2870)

[LukasWodka]

Summary

cli doctor's Node capacity check now uses the free-memory figure it computes as the verdict, and tells a permanent shortage from a transient one. Part of tracebloc/backend#2870 (DoD part 3).

What was already on develop from #628: the fit is evaluated per node against free (allocatable − steady-state requests), for cpu and memory, with a CantVerifyFreeCompute Warn when the pod list cannot be read. This PR closes the two gaps that were left:

1. Running jobs were dropped from the sum entirely, so "it fits the machine, but a running job holds the room right now" read as an unqualified ✔ — the doctor-side half of the ticket's "a permanent shortage is indistinguishable from a transient one". Batch-Job pods (identified by the job-name label the batch/v1 controller stamps — the same label internal/submit/watch.go already uses to find a training pod; no new label) now go into a separate per-node sum. Two sums, two verdicts:
   • Permanent → StatusFail, prefix OverCommitted (unchanged), now printing the node's actual free figures beside the request. Remedy: lower RESOURCE_REQUESTS / bigger node.
   • Transient → StatusWarn, new prefix HeldByRunningJob. Remedy: let the job finish or stop it; resizing does not help. The rollup (summarizeDoctor) gets a matching arm so the top line says the next run waits instead of printing "Ready to run training / Everything looks good".
2. machine.go's fourth level failed open. The line the ticket quotes (freeMem := trueCeiling - requestedMemory(ctx, cs)) had requestedMemory swallow a pod-list error into 0, so an unreadable list printed the whole VM as "unrequested" under a ✔. It now returns the error; the chain prints unrequested: unknown (pod list unreadable) and the check is StatusUnknown with the read error — while the over-commit Warn (which needs no pod list) still fires.

Why the comparison lives in Node capacity and not in the machine chain. A pod takes every resource it requests from one node, so schedulability is a per-node question. On a capped two-node cluster the VM can have 4 GiB unrequested while no single node has 2 GiB free — a VM-level "fits" would be wrong in the direction that goes Pending. On an honest single-node cluster the two figures are identical, so a second comparison would be a duplicate finding with different wording. The machine chain now says so in a comment and owes the operator honesty about the number, not a verdict.

Why Warn for transient, not Fail. Doctor's vocabulary: Fail = training-blocking (exit 2), Warn = a real finding, training still runs (exit 0), Unknown = no signal. A run in progress is the healthy state — failing on it was the Bugbot High on #628 — but a green hides why a second run is waiting_for_capacity. Warn is the honest middle, and it is what the Machine-capacity over-commit already uses for the same reason.

Before / after — the ticket's 8 GiB reproduction

k3d node 8126672Ki (7.75 GiB); installer envelope = allocatable − 3 GiB = 5100486656 B (4864.2 Mi); platform requests 3008 Mi (chart) + 140 Mi (k3s) = 3148 Mi → free 4788.2 Mi → the envelope over-asks by 76.0 Mi.

state	before the ticket	on develop today (#628)	this PR
permanent (3148 Mi platform)	✔ OK a Ready node can schedule a training job (…)	✖ Fail a Ready node is large enough … over-asks the node's FREE memory, so the pod schedules Pending	✖ Fail a Ready node is large enough for a training job (cpu=1, memory=5100486656) but not beside what is already running on it — the envelope over-asks the node's FREE memory (k3d-tracebloc-server-0 has cpu=4, memory=4903120Ki free beside the platform's own pods), so the pod schedules Pending
transient (2000 Mi platform + a running job holding the envelope)	✔ OK	✔ OK a Ready node can schedule a training job (…)	⚠ Warn a running job holds the room: a Ready node fits a training job (cpu=1, memory=5100486656) beside the platform's own pods, but running job(s) on k3d-tracebloc-server-0 hold cpu=1, memory=4980944Ki right now, so the next run waits Pending until they finish — rollup: Ready to run training — but a job is already using this machine's free compute, so the next run waits for it.
pod list unreadable, Machine capacity	✔ OK … → 7.75 GiB unrequested	✔ OK … → 7.75 GiB unrequested	· Unknown … → unrequested: unknown (pod list unreadable) — could not list pods: <err>

(Lines produced by running checkNodeFit / checkMachineChain against fake clientsets built with exactly these numbers; no 8 GiB k3d node was available in this session, so the k3d/k3s output itself is not re-measured here.)

CPU

Covered: #628 already reads the cpu request; the transient sum covers cpu and memory the same way (transient on cpu alone is still the transient Warn in the table).

Tests

• internal/doctor: TestCheckNodeFitPermanentVsTransient — table-driven with the ticket's numbers: fits → OK; permanent → Fail with OverCommitted prefix and free figures; transient → Warn with HeldByRunningJob prefix; transient on cpu; job on another node → OK; permanent + running job → permanent wins; non-Job pod holding the envelope → permanent; transient + GPU absent → transient wins, GPU fact kept; pod list unreadable → not OK, CantVerifyFreeCompute prefix; and the two remedies are pinned as different. The existing "running job excluded → OK" subtest is rewritten to "never Fail, is the transient Warn" — that behaviour change is deliberate and is the point.
• internal/doctor: TestMachineChain_UnreadablePodListIsNotAGreen, TestMachineChain_OverCommitStillWarnsWithoutAPodList.
• internal/cli: rollup cases — transient Warn is a ready-Warn that says wait (no set max, no "ask for less"), verdict is Warn; the over-commit Warn still outranks it. Copy-catalog golden regenerated; the diff is exactly the eight strings this PR introduces.

make ci exit 0 (vet, go test -race -cover ./..., errcheck/ineffassign/misspell/staticcheck, golangci-lint, fmt-check, schema-check, vulncheck, file-budget, deadcode, check-style, check-tool-pins); scripts/coverage-floor.sh ok (internal/cli 85.3% ≥ 80%). internal/doctor at 89.1%.

Mutation evidence (by hand, anchor asserted applied each time, then restored)

mutation	expected	result
A: steady-state fit compares allocatable instead of free (print left intact)	permanent no longer Fails	red — 3 permanent cases read as Warn, TestCheckNodeFitFreeMemory red ×2, remedies-are-opposites red
B: drop the running-job subtraction	transient reads OK	red — 3 transient cases read ok ("a Ready node can schedule…")
C: requestedMemory swallows the error into 0 again	Machine capacity greens on an unreadable list	red — unreadable pod list => OK ("… → 7.75 GiB unrequested")
D: rollup arm never matches	rollup falls to "Ready to run training"	red — want ready Warn, got ok ("Ready to run training"). First attempt was inert (comment ate the case colon → build failed, proves nothing); redone with a compiling mutant.

Docs

No prose outside the code describes the Node-capacity check (README line 16 lists doctor generically; docs/ has nothing on it), so "prose = code" here is the doc comments on checkNodeFit, checkMachineChain and requestedMemory, all updated.

What this PR does NOT do

• Installer-side ownership (DoD 1) and the footprint test (DoD 2) — those are the installer's; the derivation landed as feat(tests): track and ratchet the control-plane request footprint (backend#2870) client#944.
• jobs-manager's waiting_for_capacity report (DoD 4) — client-runtime; jobs-manager keeps admitting on WAIT.
• No VM-level comparison in Machine capacity — deliberately, see above; the figure is displayed there and compared per node in Node capacity.
• The 76 Mi constant itself is tracebloc/backend#2460, not this PR.

VERSION → 0.10.24 because v0.10.23 is already tagged and this touches internal/*.

---

Note

Medium Risk
Changes doctor exit codes and operator guidance for common training-wait states; logic is heavily tested but misclassification could still mislead users about resize vs wait.

Overview
tracebloc doctor now separates a permanent capacity problem (the training envelope cannot fit beside the platform’s steady-state pods) from a transient one (it fits, but a running batch job currently holds the resources). Node capacity tracks platform requests and running job-name pods in separate per-node sums, emits Fail with richer free-CPU/memory detail for over-commit, and a new Warn (HeldByRunningJob) that tells operators to wait or stop the job—not resize.

The readiness rollup matches that behavior: when a pending pod coincides with a measured “job holds the room” warn, it surfaces a Warn (exit 0) instead of the generic stuck-pending Fail and resources set max advice. Standalone transient and co-occurring cases get plain-language top lines and remedies.

Machine capacity stops treating a failed pod list as “whole VM unrequested” under a green check—requestedMemory returns errors, the chain shows unknown unrequested memory, and status becomes Unknown while over-commit detection still works without pod visibility.

Version 0.10.24; tests and user-string goldens updated for the new messages.

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

[LukasWodka]

bugbot run

[cursor]

Stale Bugbot comment from a previous 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 a14acfc. Configure here.}

[saqlainsyed007]

Reviewed the full diff against the source. The core fix is correct: node-capacity now compares the free figure it computes (allocatable − steady-state requests) against the request, keeps running batch-Job requests in a separate per-node sum, and tells a permanent over-commit (Fail, OverCommitted) from a transient job-held shortage (Warn, HeldByRunningJob). Operands, units (millicores/bytes on both sides), and comparison direction (free >= req to fit, free < req to flag) are all right, and the equal-boundary is consistent between the fit and over-commit predicates — no off-by-one. The rollup arms are ordered correctly and the Bugbot High (Pending pod shadowing the transient Warn) is addressed by the stuckPending && heldByJob arm with a test. machine.go's fail-open (pod-list error → whole VM as "unrequested" under a green) is properly closed to StatusUnknown while preserving the pod-list-free over-commit Warn. Tests are mutation-proven (four reverting mutations each redden). CI green, Bugbot clean. LGTM.