release-train: staging -> main - #645
Merged
Merged
Conversation
…ady computes, and tells a permanent shortage from a transient one (backend#2870) (#639) * fix(doctor): the node-capacity check compares the free memory it already computes, and tells a permanent shortage from a transient one (backend#2870) Node capacity: batch-Job pods (the `job-name` label the CLI already uses to find a training pod) go into their own per-node sum instead of being dropped. The steady-state fit still decides the PERMANENT Fail (OverCommitted, now with the node's free figures); a node that fits beside the platform but not beside its running Jobs is the new TRANSIENT Warn (HeldByRunningJob) with the opposite remedy — wait or stop the job, do not resize. The rollup gets a matching arm so the top line says the next run waits instead of a green. Machine capacity: requestedMemory returns its read error instead of 0, so an unreadable pod list renders "unrequested: unknown" and StatusUnknown rather than the whole VM as free under a check-mark. The over-commit Warn, which needs no pod list, still fires. VERSION 0.10.24 (v0.10.23 is tagged). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(doctor): a Pending pod beside a running job names the job, not "not enough compute" (backend#2870) Bugbot High on #639: the transient shortage's own symptom is a second training pod Pending until the running job frees the room, and the stuck-Pending Fail arm matched it first -- "Not ready ... not enough free compute" with `resources set max` at exit 2, in exactly the case the HeldByRunningJob Warn was written for. A combined arm now sits above the stuck-Pending arm: when Pod health reports a pod Pending past grace AND Node capacity has measured that a running job holds the room, the top line says the next training is waiting for the running one, the remedy says wait or stop the job and that resizing will not help, and -- since checkPods cannot see WHY a pod is Pending -- what to do if the wait outlives the job. Warn, not Fail: the inference "Pending so cannot schedule" is refuted by the measured cause, and a Fail would exit 2 on healthy training (the Bugbot High on #628). A Pod-health FAIL and the OverCommitted Fail still outrank it; a Pending pod with no running job keeps the generic Fail and its sizing advice. Mutation: moving the combined arm back below the stuck-Pending arm reddens the new both-signals test with Bugbot's exact output. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…2953) (#640) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…nning job (backend#3247) (#642) * fix(doctor): don't treat an assigned-but-Pending training pod as a running job (backend#3247) checkNodeFit counted every non-terminal `job-name` pod with a NodeName into the "a running job holds the room" sum. A training pod scheduled to a node but still Pending (ImagePullBackOff / ContainerCreating) was therefore reported as HeldByRunningJob, and combined with the same pod showing up stuck-Pending in Pod health, summarizeDoctor rolled it up to "a training is already running, wait for it to finish" at exit 0 — on a pod that is wedged and never will. The operator waits forever on a dead pod. A Pending pod (even with a NodeName) is not running: - checkNodeFit routes a `job-name` pod into the running-job sums only when it is genuinely Running; an assigned-but-Pending one feeds neither sum, so it can no longer emit HeldByRunningJob. - past the same grace window checkPods uses, such a pod is recorded and surfaced as an actionable Fail (new StuckJobPod prefix) that names its real state (ImagePullBackOff / ContainerCreating / Pending) and says to inspect the pod, not wait. A run legitimately waiting for a running job has no NodeName, so it is never mistaken for a wedged one. - summarizeDoctor gains a matching arm above the inferred stuck-Pending arm (measured beats inferred), so the wedged pod fails at exit 2 with the right remedy instead of the "wait for the running job" Warn at exit 0. The genuinely-running case is unchanged. Tests pin each pod state (Running vs assigned-Pending vs ImagePullBackOff/ContainerCreating vs within-grace) at both the checkNodeFit and summarizeDoctor levels. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(doctor): scope the stuck-pod Fail to wedged reasons; keep the rollup plain-terms (backend#3247) Addresses Saqlain's review on cli#642: - summarizeDoctor's stuck-pod ready line no longer names `kubectl` — it ends at "… names the pod", restoring the plain-terms invariant the function documents three times. The kubectl form stays in the granular checkNodeFit remedy, one --verbose away. - checkNodeFit escalates to the StuckJobPod Fail only for a genuinely-wedged waiting reason (ImagePullBackOff / ErrImagePull / ErrImageNeverPull / InvalidImageName / CreateContainerConfigError / CreateContainerError), via a new stuckReasonWedged allowlist. A pod still pulling/creating past the grace window (ContainerCreating / Pulling / bare Pending) is left to checkPods' age-based inference, so "waiting will not clear it" is only asserted where the kubelet has actually reported a failure — a large first pull on a cold node no longer trips the confident Fail. Tests updated to pin wedged reasons (→ stuck Fail) apart from still-progressing ones (→ deferred, node OK), and to guard the rolled-up line against Kubernetes vocabulary. Copy-catalog golden regenerated. go build/vet/test ./... green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…apacity Warn (backend#3248) (#643) * fix(doctor): measured image-pull/dataset Fail outranks the wait-for-capacity Warn (backend#3248) The `stuckPending && heldByJob` arm in summarizeDoctor returned StatusWarn above the Image-pull-secret and Dataset-volume StatusFail arms. checkImagePull and checkPVC are independent of Pod health and Node capacity, so either can be Fail while a pod is Pending past grace beside a running job — the exact co-occurrence the Wait-Warn matches. Sitting below the Warn, those measured failures were shadowed: `doctor` exited 0 (Warn) instead of 2 (Fail), hiding a real training-blocker behind a "wait for the job" warning. Reorder so the two measured Fail arms are read before the Wait-Warn. The Wait-Warn must stay above the plain stuck-Pending Fail (backend#2870), so the measured Fails necessarily move above stuck-Pending too — consistent with the switch's measured-beats-inferred rule. A test pins that an image-pull-secret or dataset-volume Fail beside a running job now wins (exit 2), while the Wait-Warn still applies when there is no measured Fail. Closes tracebloc/backend#3248 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(doctor): classify pull-secret/PVC read failures as can't-checks, not measured Fails (backend#3248) Addresses LukasWodka's review on cli#643. checkImagePull and checkPVC returned StatusFail for ANY error from the secret/PVC read — Forbidden, timeout, transient — conflating a can't-check with a measured absence. Once the reorder promoted those Fail arms above the wait-for-capacity Warn, a read blip beside a running job flipped a healthy environment from exit 0 to exit 2 with a detail that falsely said "not found". - checkImagePull: a non-NotFound Get error is now a StatusWarn can't-check (CantReadImagePullSecret prefix); StatusFail only for a genuine not-found / wrong-type / malformed secret. - checkPVC: a DiscoverSharedPVC read failure (PVCReadErrPrefix, now a shared constant) is a StatusWarn can't-check; StatusFail only for a read-and-found missing / unbound PVC. - summarizeDoctor: two new can't-check Unknown arms drop those reads to the honest "couldn't check ..." tier instead of the promoted measured Fail. - Update the StuckJobPod arm comment (backend#3247), now stale: the image-pull and dataset Fails sit above it too. - Thread 3: the over-commit Warn can't be lifted above the Wait-Warn in the Pending variant (the stuck-Pending Fail sits between them), so pin the current message-only behavior rather than add a fragile special-case arm. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(doctor): close the jobs-manager-unreadable false green; tighten the Wait-Warn precondition (backend#3248) Two review points from cli#643: - Saqlain: checkImagePull's OTHER can't-read path — the jobs-manager Deployment itself unreadable (dep == nil) — returned a StatusWarn with no CantReadImagePullSecret prefix, so summarizeDoctor matched no arm and it fell through to the OK default: a false green at exit 0, the same class this PR closes for the secret read. Route it to the Unknown tier by carrying the prefix. Producer test added. - LukasWodka: the co-occurrence test's precondition asserted only StatusWarn, so a fall-through to the bare heldByJob Warn (also exit 0) would keep it green if the reorder were undone — leaving the exit-0 → exit-2 claim unpinned. Assert the Wait-Warn's own top line instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(doctor): plain-terms wording for the image-pull can't-check rollup line (backend#3248) Bugbot on cli#643: the new image-pull can't-check ready line said "image pull secret" — Kubernetes jargon. The rolled-up summarizeDoctor lines stay in plain terms (renderDoctorDetails is the only place k8s vocabulary appears), and the twin PVC line already says "dataset storage". Reword to mirror the Fail arm's "training images can't be pulled". Test assertion + golden updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…t N prefix-matched arms (backend#3282) (#644) Follow-up to Saqlain's review on cli#643. summarizeDoctor classified "a check couldn't READ its subject" into the Unknown tier with four near-identical prefix-matched arms, each needing three coordinated edits across three packages (an exported prefix constant, a checkX branch stamping it into Detail, and a matching CLI arm) coupled by fragile cross-package strings. Add a structural doctor.Result.CantCheck, set by the checkX producers on their can't-read paths, and replace the four arms with one rule (cantCheckReady) that surfaces the first flagged check's line. The per-check messages stay CLI copy in an ordered table (healthLine values, so the copy backstop still harvests them), so no user-facing output changes — both goldens are a zero diff — and stuckPending's can't-check exclusion moves from a Detail-prefix to the marker too. A new can't-read probe now needs only "producer sets CantCheck" plus a table row, with no shared prefix constant to keep in lockstep. Producer tests assert the marker on the can't-read paths (and its absence on the HeldByRunningJob soft finding); a rollup test pins the arm order and the unmapped-check fall-through. Closes tracebloc/backend#3282 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
release-train: develop -> staging
Contributor
Author
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ 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 d071ab1. Configure here.
tracebloc-release-train
Bot
added
gate-nudge
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated promotion by the release train (RFC-0008 D14). Head is the train-managed
release-train/to-mainbranch (a mirror ofstaging), so it never collides with a human PR. Merged only when the fr-gate is green.Note
Medium Risk
Changes doctor exit codes and rollup precedence across many Kubernetes edge cases; mistakes could mislead operators during training outages, but scope is diagnostic CLI logic rather than auth or data paths.
Overview
Release train promotion bumps VERSION to 0.10.24 and extends the version-bump gate publish-paths to include the install scripts.
tb doctorreadiness rollup is reworked so measured problems beat inferred ones: image-pull and dataset PVC Fails rank above generic stuck-Pending and “wait for capacity” Warns; pending pod + running job holding compute surfaces a wait-for-job Warn instead of a resize Fail; scheduled-but-wedged training pods get a dedicated Fail (not “wait for a job”). Can't-read probes (RBAC, timeouts) use a structuralResult.CantCheckflag and sharedcantCheckReadyinstead of matching Detail prefixes.Underlying checks distinguish permanent over-commit vs transient “job holds the room” in
checkNodeFit, treat assigned Pending job pods with wedged kubelet reasons asStuckJobPod, classify PVC/image-pull read errors as can't-checks, and stop the machine-capacity chain from going green when the pod list is unreadable. User-facing strings and broad tests/goldens follow.Reviewed by Cursor Bugbot for commit d071ab1. Bugbot is set up for automated code reviews on this repo. Configure here.