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

VERSION

@@ -1 +1 @@
-0.10.23
+0.10.24

internal/cli/doctor.go

@@ -444,6 +444,16 @@ func summarizeDoctor(results []doctor.Result, tok tokenState) (connected, ready
 	// was actually detected (Bugbot on #561/#566: "Warn rollup shadowed by
 	// Unknown", backend#2438). StatusUnknown carries no signal, so it must be the
 	// last thing consulted — it may only win when nothing above it fired.
+	// Two signals that are read in more than one arm, named once so the arms
+	// cannot drift apart on the prefix (the same discipline as the producer
+	// constants they match). stuckPending is checkPods' Pending-past-grace Warn,
+	// with its "could not list pods" can't-check excluded; heldByJob is
+	// checkNodeFit's transient verdict (backend#2870).
+	stuckPending := by["Pod health"].Status == doctor.StatusWarn &&
+		!strings.HasPrefix(by["Pod health"].Detail, "could not list pods")
+	heldByJob := by["Node capacity"].Status == doctor.StatusWarn &&
+		strings.HasPrefix(by["Node capacity"].Detail, doctor.HeldByRunningJob)
+
 	switch {
 	// ── Fail: a real, training-blocking problem (worst wins) ──
 	case by["Pod health"].Status == doctor.StatusFail:
@@ -482,7 +492,34 @@ func summarizeDoctor(results []doctor.Result, tok tokenState) (connected, ready
 		ready = healthLine{doctor.StatusFail,
 			"Not ready — this machine is big enough, but the platform's own services have already claimed the room.",
 			fmt.Sprintf("Ask for less per training run, or give the machine more memory/CPU. Do NOT size runs to the machine here — that measures the machine's total, not what is free, so it would ask for MORE and leave the training stuck. `%s doctor --verbose` shows the exact numbers and the knob to turn.", launcher())}
-	case by["Pod health"].Status == doctor.StatusWarn && !strings.HasPrefix(by["Pod health"].Detail, "could not list pods"):
+	case stuckPending && heldByJob:
+		// A PENDING POD WHOSE CAUSE HAS BEEN MEASURED (Bugbot High on #639,
+		// backend#2870). This is the transient shortage's own symptom: a job is
+		// running, the next training pod is Pending until it frees the room -- the
+		// `waiting_for_capacity` state the HeldByRunningJob Warn exists to name.
+		// Left to the arm below, the stuck-Pending Fail matched first and the top
+		// line read "Not ready ... not enough free compute" with `computeRemedy`
+		// (`resources set max`) at exit 2 -- in exactly the case this change was
+		// made for, the wait-for-the-job advice never appeared.
+		//
+		// A WARN ABOVE A FAIL, deliberately, and the reason has to be stated
+		// because the tiers below are ordered by severity: the stuck-Pending arm
+		// is not a measured failure but an INFERENCE ("Pending past grace, so it
+		// cannot schedule, so compute or image"). checkNodeFit has measured the
+		// cause -- the envelope fits this machine and a running job holds it --
+		// which refutes the inference, and a Fail here would exit 2 on healthy
+		// training (the Bugbot High on #628) while recommending a resize that
+		// changes nothing. The two arms that still outrank this one are measured:
+		// a Pod-health FAIL (crash-loop) and the OverCommitted Fail.
+		//
+		// checkPods does not know WHY a pod is Pending, so a pod stuck on an
+		// image pull beside a running job would land here too; the remedy names
+		// what to do if the wait outlives the job rather than pretending the
+		// attribution is certain.
+		ready = healthLine{doctor.StatusWarn,
+			"Ready to run training — a training is already running, and the next one is waiting for it to finish.",
+			fmt.Sprintf("A pod is waiting to start because a running job holds this machine's free compute. Let the job finish, or stop it if it is not needed; asking for less per run or resizing will not help — the room comes back when the job ends. If the pod is still waiting after that, something else is holding it: `%s doctor --verbose`.", launcher())}
+	case stuckPending:
 		// Pods stuck Pending past the grace window (unschedulable / image can't
 		// pull) mean training can't actually schedule — so this is NOT ready, even
 		// though the granular Pod-health check rates it a softer ⚠. Without this,
@@ -532,6 +569,22 @@ func summarizeDoctor(results []doctor.Result, tok tokenState) (connected, ready
 		ready = healthLine{doctor.StatusWarn,
 			"Ready to run training — but your environment thinks this machine is bigger than it is.",
 			fmt.Sprintf("It reports more memory than the machine really has, so two trainings that each look like they fit can together run it out of memory and take the environment down. Run one training at a time; to fix it for good, recreate the environment as a single-node one. `%s doctor --verbose` shows the numbers and the exact flags.", launcher())}
+	case heldByJob:
+		// backend#2870, the TRANSIENT shortage with no pod Pending yet (the
+		// co-occurring case is the Warn in the Fail tier above). checkNodeFit found the envelope
+		// fits this machine beside the platform, but a job that is running holds
+		// the room now, so the next run waits. This arm exists because the two
+		// things an operator could otherwise be told are both wrong: the capacity
+		// Fail's advice (ask for less / grow the machine) changes nothing about a
+		// job already running, and the plain green hides why a second run is
+		// waiting. Warn keeps exit 0 -- training IS running -- while doctorVerdict
+		// withholds "everything looks good". Below the over-commit Warn: a machine
+		// that lies about its size is the more consequential finding.
+		//
+		// Plain terms, no Kubernetes vocabulary, like its neighbours.
+		ready = healthLine{doctor.StatusWarn,
+			"Ready to run training — but a job is already using this machine's free compute, so the next run waits for it.",
+			fmt.Sprintf("Nothing is wrong with the machine: let the running job finish, or stop it if it is not needed. Asking for less per run or resizing will not help here — the room comes back when the job ends. `%s doctor --verbose` shows the numbers.", launcher())}
 	// ── Unknown: a check couldn't complete — no signal, so it never shadows a
 	// Fail or Warn above and only surfaces when nothing real was found. ──
 	case by["Pod health"].Status == doctor.StatusWarn && strings.HasPrefix(by["Pod health"].Detail, "could not list pods"):

internal/cli/doctor_test.go

@@ -508,6 +508,106 @@ func TestSummarizeDoctor(t *testing.T) {
 		}
 	})
 
+	// backend#2870: the TRANSIENT shortage. The envelope fits the machine beside
+	// the platform, but a running job holds the room, so the next run waits. It
+	// must roll up as a Warn that says so -- not the green (which hides why a
+	// second run is waiting), not a Fail (training IS running: the Bugbot High on
+	// #628), and never the capacity Fail's "ask for less / grow the machine"
+	// advice, which changes nothing about a job already running.
+	t.Run("a running job holding the room → ready Warn that says to wait, not resize", func(t *testing.T) {
+		// DETAIL BUILT FROM THE PRODUCER'S CONSTANT, same discipline as the
+		// over-commit cases above: the arm classifies by prefix.
+		_, r := summarizeDoctor(withDetail(allOK, "Node capacity", doctor.StatusWarn,
+			doctor.HeldByRunningJob+": a Ready node fits a training job (cpu=1, memory=4864Mi) beside the platform's own pods, but running job(s) on n1 hold cpu=1, memory=4864Mi right now, so the next run waits Pending until they finish"), tokenOK)
+		if r.status != doctor.StatusWarn {
+			t.Fatalf("want ready Warn, got %v (%q)", r.status, r.text)
+		}
+		if !strings.Contains(r.text, "waits for it") {
+			t.Errorf("the top line should say the next run waits, got %q", r.text)
+		}
+		if strings.Contains(r.remedy, "resources set max") || strings.Contains(r.remedy, "Ask for less") {
+			t.Errorf("the transient remedy must not send the operator to resize or shrink: %q", r.remedy)
+		}
+		if !strings.Contains(r.remedy, "let the running job finish") {
+			t.Errorf("the remedy should say to wait for or stop the running job: %q", r.remedy)
+		}
+		c, _ := summarizeDoctor(allOK, tokenOK)
+		if v := doctorVerdict(c.status, r.status); v != doctor.StatusWarn {
+			t.Errorf("verdict must own the Warn (exit 0, no 'everything looks good'), got %v", v)
+		}
+	})
+
+	// Bugbot High on #639. The transient shortage's own SYMPTOM is a second
+	// training pod sitting Pending until the running job frees the room -- the
+	// `waiting_for_capacity` case. With the stuck-Pending Fail arm first, that
+	// state rolled up to "Not ready ... not enough free compute" with
+	// `computeRemedy` (`resources set max`) at exit 2, and the wait-for-the-job
+	// line never appeared in exactly the case it was written for.
+	t.Run("a Pending pod AND a running job holding the room → the cause is named, not the generic stuck-Pending Fail", func(t *testing.T) {
+		results := withDetail(allOK, "Node capacity", doctor.StatusWarn,
+			doctor.HeldByRunningJob+": a Ready node fits a training job (cpu=1, memory=4864Mi) beside the platform's own pods, but running job(s) on n1 hold cpu=1, memory=4864Mi right now, so the next run waits Pending until they finish")
+		results = withDetail(results, "Pod health", doctor.StatusWarn,
+			"Pending > 5m0s: [train-second]")
+		c, r := summarizeDoctor(results, tokenOK)
+		if r.status == doctor.StatusFail {
+			t.Fatalf("the stuck-Pending arm shadowed the transient cause and failed the command on healthy training: %q / %q", r.text, r.remedy)
+		}
+		if r.status != doctor.StatusWarn {
+			t.Fatalf("want ready Warn, got %v (%q)", r.status, r.text)
+		}
+		if !strings.Contains(r.text, "waiting for it") {
+			t.Errorf("the top line should say the next run waits for the running one, got %q", r.text)
+		}
+		if !strings.Contains(r.remedy, "running job holds") {
+			t.Errorf("the remedy should name the cause -- a running job -- got %q", r.remedy)
+		}
+		if strings.Contains(r.remedy, "resources set max") || strings.Contains(r.remedy, "Ask for less") {
+			t.Errorf("the remedy must not send the operator to resize or shrink: %q", r.remedy)
+		}
+		if !strings.Contains(r.remedy, "still waiting after") {
+			t.Errorf("checkPods cannot see WHY a pod is Pending, so the remedy must say what to do if the wait outlives the job: %q", r.remedy)
+		}
+		if v := doctorVerdict(c.status, r.status); v != doctor.StatusWarn {
+			t.Errorf("verdict must own the Warn (exit 0), got %v", v)
+		}
+	})
+
+	t.Run("a Pending pod with NO running job is still the stuck-Pending Fail", func(t *testing.T) {
+		// The other side: the arm above is scoped to the co-occurrence. A pod
+		// Pending on a machine where nothing holds the room is the generic,
+		// measured-nowhere case and keeps its Fail and its sizing advice.
+		_, r := summarizeDoctor(withDetail(allOK, "Pod health", doctor.StatusWarn,
+			"Pending > 5m0s: [trainer-x]"), tokenOK)
+		if r.status != doctor.StatusFail || !strings.Contains(r.remedy, "resources set max") {
+			t.Errorf("want the generic stuck-Pending Fail with the sizing remedy, got %v %q", r.status, r.remedy)
+		}
+	})
+
+	t.Run("a crash-looping pod still outranks the running-job explanation", func(t *testing.T) {
+		// The exception is scoped to the stuck-Pending WARN; a Pod-health FAIL is
+		// a measured failure with a different fix, and must keep winning.
+		results := withDetail(allOK, "Node capacity", doctor.StatusWarn,
+			doctor.HeldByRunningJob+": a Ready node fits a training job beside the platform's own pods, but running job(s) on n1 hold cpu=1, memory=4864Mi right now")
+		results = withDetail(results, "Pod health", doctor.StatusFail, "crash-looping: [jobs-manager]")
+		_, r := summarizeDoctor(results, tokenOK)
+		if r.status != doctor.StatusFail || !strings.Contains(r.text, "isn't running") {
+			t.Errorf("a Pod-health Fail must still win, got %v (%q)", r.status, r.text)
+		}
+	})
+
+	t.Run("a machine that lies about its size outranks a running job", func(t *testing.T) {
+		// Both are Warns; the over-commit is the more consequential finding and
+		// sits first. Pin the order so a later reshuffle cannot demote it.
+		results := withDetail(allOK, "Machine capacity", doctor.StatusWarn,
+			"Docker VM 7.75 GiB → 2 nodes claiming 15.50 GiB — Kubernetes believes 2.00× the memory this machine has")
+		results = withDetail(results, "Node capacity", doctor.StatusWarn,
+			doctor.HeldByRunningJob+": a Ready node fits a training job beside the platform's own pods, but running job(s) on n1 hold cpu=1, memory=4864Mi right now")
+		_, r := summarizeDoctor(results, tokenOK)
+		if r.status != doctor.StatusWarn || !strings.Contains(r.text, "bigger than it is") {
+			t.Errorf("want the over-commit Warn first, got %v (%q)", r.status, r.text)
+		}
+	})
+
 	t.Run("node capacity GPU-soft warn → still ready", func(t *testing.T) {
 		_, r := summarizeDoctor(withDetail(allOK, "Node capacity", doctor.StatusWarn,
 			"no single Ready node satisfies cpu+memory AND nvidia.com/gpu — GPU jobs rely on the CPU fallback (needs cpu=2, memory=8Gi)"), tokenOK)

internal/cli/testdata/golden/zz-all-strings.golden

@@ -57,7 +57,7 @@ screen. %s/%d are runtime placeholders.
 "%s (unreadable: %v)"
 "%s Bound, mounted at %s"
 "%s contains a NUL byte — the file is corrupt or not really a CSV. The cluster rejects it after the upload; re-export the file and re-run."
-"%s for a training job (%s) but not beside what is already running on it — the envelope over-asks the node's FREE %s, so the pod schedules Pending"
+"%s for a training job (%s) but not beside what is already running on it — the envelope over-asks the node's FREE %s (%s has %s free beside the platform's own pods), so the pod schedules Pending"
 "%s has a header but no data rows (0 ingestable records). Add at least one data row and re-run."
 "%s has duplicate column name(s): %s. Each column must be unique — the cluster rejects duplicates, and the schema would map onto the wrong column. Rename them and re-run."
 "%s is empty — add a header and at least one data row, then re-run"
@@ -83,6 +83,7 @@ screen. %s/%d are runtime placeholders.
 "%s/%s"
 "%s: %v"
 "%s: %w"
+"%s: a Ready node fits a training job (%s) beside the platform's own pods, but running job(s) on %s hold %s right now, so the next run waits Pending until they finish"
 "%s=%s,%s=%s"
 "%v (policy: %v)"
 "%w in namespace %q, but tracebloc clients are running in: %s. Pass --namespace to pick one."
@@ -119,10 +120,12 @@ screen. %s/%d are runtime placeholders.
 "--time-column is time_to_event_prediction only; it doesn't apply to task %q"
 "--timeout has no effect without --wait or --seal"
 "--wait and --seal are separate modes — run them one at a time"
+". Also, no single Ready node satisfies cpu+memory AND %s, so GPU jobs would rely on the CPU fallback"
 "0:%d"
 "3 GiB"
 "A dataset named %q already exists — replace it?"
 "A newer tracebloc is available: %s (you have %s). Update: tracebloc upgrade"
+"A pod is waiting to start because a running job holds this machine's free compute. Let the job finish, or stop it if it is not needed; asking for less per run or resizing will not help — the room comes back when the job ends. If the pod is still waiting after that, something else is holding it: `%s doctor --verbose`."
 "A real run continues with step 2 (copy into your secure environment) and step 3 (validate and load)."
 "A tracebloc client is already running on this cluster — adopting it. Couldn't read the cluster identity, so its idempotency anchor was left unchanged; point --kubeconfig/--context at a cluster where kube-system is readable to stamp it."
 "A training run is allocated up to:"
@@ -201,7 +204,7 @@ screen. %s/%d are runtime placeholders.
 "Diagnose auth / cluster problems with: tracebloc doctor"
 "Do you want to change the allocation? Run `%s resources set` (guided walkthrough on a terminal)."
 "Do you want to ingest training or test data?"
-"Docker VM %s (%d cpu) → %d node%s claiming %s → %s unrequested"
+"Docker VM %s (%d cpu) → %d node%s claiming %s → %s"
 "Docker and related tools — remove them yourself if you no longer need them"
 "Dry run — nothing was changed"
 "Dry-run complete — your data and secure environment check out; nothing was created."
@@ -213,6 +216,7 @@ screen. %s/%d are runtime placeholders.
 "Email it to support@tracebloc.io."
 "Email support@tracebloc.io with the output of `%s doctor --diagnose`."
 "Ensure your kubeconfig user can list nodes."
+"Ensure your kubeconfig user can list pods cluster-wide, then re-run doctor to measure what is unrequested."
 "Ensure your kubeconfig user can list pods cluster-wide, then re-run doctor to verify free capacity."
 "Ensure your kubeconfig user can list pods cluster-wide, then re-run doctor to verify free capacity. If GPU training is expected, also ensure one node has both the compute and the GPU capacity, with its device plugin."
 "Enter"
@@ -290,6 +294,8 @@ screen. %s/%d are runtime placeholders.
 "Not yet in the CLI:"
 "Note: %d file(s) in images/ have no labels.csv row and won't be part of the dataset: %s"
 "Note: %d sequence(s) grouped by %q — the platform counts this dataset in sequences, not rows"
+"Nothing is wrong with the machine: let the running job finish, or stop it if it is not needed. Asking for less per run or resizing will not help here — the room comes back when the job ends. `%s doctor --verbose` shows the numbers."
+"Nothing on the machine needs changing: let the running job finish, or stop it if it is not needed (kubectl get jobs -A). Lowering RESOURCE_REQUESTS or resizing does not free room a running job holds."
 "Offboarded %q. This machine is no longer connected to tracebloc."
 "Offboarded %q: the machine credential is revoked, so it can no longer connect to tracebloc — but some cleanup above didn't complete. Finish the flagged steps by hand."
 "Only tracebloc's small jobs-manager restarts — running training isn't interrupted."
@@ -314,6 +320,8 @@ screen. %s/%d are runtime placeholders.
 "Reading your files locally first — nothing has touched your secure environment yet — so a layout or settings problem shows up right away."
 "Ready for `tracebloc data ingest`."
 "Ready to run training"
+"Ready to run training — a training is already running, and the next one is waiting for it to finish."
+"Ready to run training — but a job is already using this machine's free compute, so the next run waits for it."
 "Ready to run training — but your environment thinks this machine is bigger than it is."
 "Ready to run training — can't check yet"
 "Ready to run training — couldn't check free compute (run with --verbose)"