release-train: develop -> staging - #630
Merged
Merged
Conversation
…70) (#628) * fix(doctor): node-fit checks FREE memory, not allocatable, so an over-committed control plane is caught (backend#2870) `checkNodeFit` fit the training envelope against each node' Allocatable and never against what is already requested on it. So a node whose control plane claims memory passed -- allocatable said "big enough" -- while the pod went Pending / Insufficient memory. This is the layer the ticket says owns nothing: the installer sizes from allocatable, cli doctor checked against allocatable, and the one number that would expose it (free memory) was computed and thrown away. 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 cannot be read, free is UNKNOWN -- say so and fall back to allocatable rather than passing it off as free. Terminal pods hold nothing and are skipped (mirrors requestedMemory). Scope: this is DoD item 3 of backend#2870 (the cli-doctor layer). The installer-side refusal (1), the footprint schedulability test tied to the #2460 constant (2), the jobs-manager permanent-vs-transient message (4) and the CPU envelope audit (5) remain. Test: 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. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(release): bump VERSION to 0.10.22 (backend#2870) v0.10.21 is already released and this PR changes published files under internal/*; the version-bump-gate requires the develop VERSION to lead the released tag so the next train hop cuts an unreleased version. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(doctor): exclude running jobs from the free sum, warn on unknown free, name the short dimension (backend#2870) Three findings on the first cut (Saqlain + Bugbot): - HIGH: the free sum counted every non-terminal pod INCLUDING a running training job that already holds the envelope, so on a single-node install doctor returned StatusFail and exited 2 during HEALTHY training -- a false negative on the state it exists to bless. Skip batch-Job pods (the job-name label the batch/v1 controller stamps); the envelope must fit beside the STEADY-STATE control plane, not a transient workload. - HIGH: when the pod list could not be read the OK path returned StatusOK with no caveat, asserting free schedulability it never verified. Now WARN with an allocatable-only caveat. - MEDIUM: the over-commit message always blamed FREE memory; it now names the actual short dimension (cpu / memory / both). Tests: a running job labelled job-name is excluded (healthy training stays ok); an unreadable pod list warns; a cpu over-commit names cpu. All mutation-adjacent and green; golden updated for the new copy. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(doctor): surface the unknown-free warn in the rollup, and do not blame disk shortfalls on memory (backend#2870) Two follow-on findings from the free-memory fix: - HIGH: the unknown-free StatusWarn detail started with "a Ready node can schedule...", which summarizeDoctor does not recognise as a can't-check, so the rollup fell through to "Ready to run training" at exit 0 -- the over-commit stayed invisible on the default path. Give the warn a distinct "could not read the pod list" prefix and teach the rollup to treat it as a can't-check (Unknown), beside the RESOURCE_REQUESTS / nodes-unlistable ones. - MEDIUM: the over-commit arm fired on !cpuMemFits && allocOnlyFit, which a DISK-only shortfall also satisfies (allocOnlyFit is cpu+mem only), so a disk short was reported as a FREE-memory over-commit. Guard the arm with (overCPU || overMem); a disk short now falls through to the generic fail that names ephemeral-storage. Tests: disk-only shortfall is not blamed on memory; the unknown-free warn rolls up to "couldnt check free compute", not Ready. golden updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(doctor): unknown free compute must outrank the soft GPU warn (backend#2870) Bugbot High. `summarizeDoctor` classifies a Node-capacity Warn as a can't-check by PREFIX, and the `gpuRequested && !fullFits` case sits ABOVE the `!freeKnown` branch with a detail carrying no such prefix. So with a GPU requested and the pod list unreadable, the soft GPU Warn won, the can't-check was never emitted, and the rollup fell through to "Ready to run training" at exit 0 -- a clean bill over a cluster whose free compute doctor had not looked at. Not an exotic path: the chart stamps `nvidia.com/gpu` on CPU-only installs, so `gpuRequested` is commonly true where no node exposes a GPU, which IS this case. The GPU fallback stays soft and stays reported; it just no longer suppresses the stronger statement. Both facts ride in one Warn, can't-check FIRST because the rollup matches on the prefix. AND THE PREFIX IS NOW DEFINED ONCE. It was written out three times -- producer, classifier, and the classifier's test -- which is a check holding its own copy of the rule it verifies. `doctor.CantVerifyFreeCompute` is the single definition; internal/cli already imports internal/doctor, so the classifier reads it rather than retyping it, and the test builds its fixture from it. A producer that rewords the phrase now moves all three together instead of leaving a test green against a string nothing emits. Three guards, two of them new: - unknown free AND gpu requested -> asserts the detail STARTS with the constant (Contains would pass on a detail that merely mentions it somewhere and still greens the run), and that the GPU fact survives. - gpu requested with free KNOWN -> asserts NO can't-check prefix, so the fix cannot be read as "always warn about free". - the classifier's can't-check table gains the COMBINED detail, which is what a GPU-requesting install with an unreadable pod list actually produces. Mutation-proved: deleting the new `!freeKnown` branch reddens the first test with the pre-fix detail verbatim. Verified: `go build ./...`, `go vet ./...` clean, `go test ./...` all packages ok. `zz-all-strings.golden` regenerated for the two new user-facing strings -- additions only, no deletions and no orphaned literal. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(doctor): the over-commit Fail must not advise sizing runs to the machine (backend#2870) Bugbot Medium, and the advice was not merely vague -- it was the opposite of the fix. `computeRemedy` ends every variant with "size runs to this machine with `resources set max`", and `set max` measures the machine's TOTAL, which is the figure the #2870 Fail just rejected. A user who follows the top line asks for MORE and the training stays stuck; the accurate wording was only behind `--verbose`. The two Node-capacity Fails need opposite advice and were sharing one rollup arm: - "no Ready node can fit" -> the machine is too small; sizing runs to it, or giving it more, is right. - "a Ready node IS large enough, but not beside what is already on it" -> the machine is fine; asking for more is what breaks it. So the over-commit Fail gets its own arm, keyed on a new `doctor.OverCommitted` prefix that the producer now composes its detail FROM -- the same one-definition treatment `CantVerifyFreeCompute` got in the previous commit, and for the same reason: this classification is by prefix, so a reworded producer would silently fall back to the generic arm and restore the bug. Plain terms, no Kubernetes vocabulary, like its two neighbours -- `renderDoctorDetails` is documented as the only place that appears, and the granular Remedy one `--verbose` away already names the knob. Two tests, both directions, because a one-sided fix here is easy to get wrong: - over-commit -> must NOT contain "resources set max", must warn against it. Its fixture detail is BUILT from `doctor.OverCommitted` so arm and producer cannot drift. - generic too-small Fail -> must STILL offer the sizing fix, so this cannot strip correct advice from the case it is correct for. Mutation-proved: deleting the new arm reddens the first test with the generic remedy verbatim ("... or size runs to this machine with `tracebloc resources set max`."). Verified: `go build`, `go vet` clean, `go test ./...` all packages ok. `zz-all-strings.golden` regenerated -- three new strings, one replaced by its constant-composed form. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(doctor): the resize nudge must not advise past what is FREE (backend#2870) Bugbot Medium, confirmed by @saqlainsyed007 against the code: the fit moved to FREE and the `resources set max` nudge stayed on allocatable-minus-overhead. `bestCPU`/`bestMem` are the largest ALLOCATABLE across Ready nodes; the verdict beside them is computed from per-node free. On a node that is big enough and already partly claimed those disagree, and the nudge won -- so an operator who followed "this machine could give a run up to cpu=...,memory=...Gi" recreated the exact over-commit this check now fails on, and the next `doctor` run told them not to size to their own machine. Advice that contradicts the verdict printed beside it is worse than no advice, so the ceiling is now bounded by free: the largest FREE cpu/memory on any one Ready node is tracked alongside the allocatable-derived best, and the machine handed to `MaxRunCores`/`MaxRunGiB` is the smaller of the two. When free is UNKNOWN the nudge is suppressed entirely rather than falling back to allocatable. There is nothing to bound it with, and that arm is already a Warn saying free could not be verified -- emitting a confident ceiling from the number we just said we could not trust is the same defect one step along. Test: a 32/64Gi node with 24 cores + 40Gi held by a non-job pod. The run (2/8Gi) still fits the free 8/24Gi so the result stays OK, and the detail must not advertise the allocatable ceiling `cpu=31,memory=61Gi`. Mutation-proved: forcing the bound off reddens exactly that test and nothing else; restored, the suite is green. go build, go vet, gofmt: clean. `go test ./...`: all packages ok. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(doctor): the nudge must name the command that applies its own numbers (backend#2870) Bugbot Medium on db56239, and it is the second half of my own last fix. Bounding the ceiling by free corrected the FIGURE and left the ATTRIBUTION behind: `tracebloc resources set max` sizes from allocatable via `LargestReadyNode`, so on a claimed node it applies more than the number printed beside it. An operator who read "up to cpu=7,memory=21Gi" and ran the command it named got the allocatable ceiling instead -- reapplying the over-commit this check had just started refusing. Same defect as the finding before it, one step along: the verdict and the advice beside it disagreed. `set max` is now named only when it would genuinely land on the printed numbers -- when nothing meaningful is claimed and the free-bounded ceiling equals the allocatable one. Otherwise the nudge names the explicit form, `tracebloc resources set --cores N --memory NGi`, which applies exactly what it printed. The drift signal survives on a busy node instead of being suppressed, and it stays truthful. The existing test asserted the FIGURE only, which is why a wrong command could sit beside a right number and stay green. It now also asserts that a claimed node does not name `set max` and does name the explicit form. Mutation-proved with a mutation that COMPILES -- my first attempt left `allocM` unused, and a build failure is not a caught mutation, it is an invalid one. Relaxing the equality to `<=` (always true for a free-bounded machine) makes the nudge say `set max` again and reddens exactly that assertion. `internal/cli`'s zz-all-strings golden is regenerated: the two format strings this changes, and nothing else (`git diff --numstat` = 2 added, 1 removed). go build, go vet, gofmt: clean. `go test ./...`: 18 packages ok. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(doctor): the over-commit Fail must outrank the stuck-Pending it causes (backend#2870) The previous commit fixed the FIGURE the nudge prints and left the ROLLUP still recommending the thing. summarizeDoctor consulted the Pod-health stuck-Pending arm before the over-commit arm, and that arm ends in computeRemedy -- "size runs to this machine with `resources set max`" -- which sizes from allocatable, the number the over-commit Fail just rejected. Following it raises the ask and leaves the job stuck. The two states CO-OCCUR BY CONSTRUCTION, which is what makes this a correctness bug rather than a preference about ordering: the producer's own Detail ends "so the pod schedules Pending" (internal/doctor/doctor.go:778), so an over-committed node is EXPECTED to also have pods stuck Pending. The over-commit arm was therefore close to unreachable in the field. Moved above the stuck-Pending WARN and no further: a hard Pod-health Fail is a different problem with a different fix (reinstall) and is not caused by this one, so it still wins. Both directions are asserted. The existing over-commit test could not have caught this -- it builds its fixture from allOK, so Pod health is OK and the over-commit arm wins either way. The new test constructs the state where both fire, which is the state the producer says is the normal one. Verified: go test ./... -> 18 packages ok, 0 failed; gofmt clean; go vet clean. Mutation-proved -- putting the stuck-Pending arm back on top reddens the new case with the real `set max` remedy in the message, and reddens ONLY the new case, which is the coverage gap Bugbot found. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Bumps the k8s group with 2 updates: [k8s.io/apimachinery](https://github.com/kubernetes/apimachinery) and [k8s.io/client-go](https://github.com/kubernetes/client-go). Updates `k8s.io/apimachinery` from 0.36.4 to 0.37.0 - [Commits](kubernetes/apimachinery@v0.36.4...v0.37.0) Updates `k8s.io/client-go` from 0.36.4 to 0.37.0 - [Changelog](https://github.com/kubernetes/client-go/blob/master/CHANGELOG.md) - [Commits](kubernetes/client-go@v0.36.4...v0.37.0) --- updated-dependencies: - dependency-name: k8s.io/apimachinery dependency-version: 0.37.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: k8s - dependency-name: k8s.io/client-go dependency-version: 0.37.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: k8s ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [softprops/action-gh-release](https://github.com/softprops/action-gh-release) from 3.0.2 to 3.0.3. - [Release notes](https://github.com/softprops/action-gh-release/releases) - [Changelog](https://github.com/softprops/action-gh-release/blob/master/CHANGELOG.md) - [Commits](softprops/action-gh-release@3d0d988...efb3536) --- updated-dependencies: - dependency-name: softprops/action-gh-release dependency-version: 3.0.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [sigstore/cosign-installer](https://github.com/sigstore/cosign-installer) from 3.9.1 to 4.1.2. - [Release notes](https://github.com/sigstore/cosign-installer/releases) - [Commits](sigstore/cosign-installer@398d4b0...6f9f177) --- updated-dependencies: - dependency-name: sigstore/cosign-installer dependency-version: 4.1.2 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 7.0.0 to 8.0.1. - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@37930b1...3e5f45b) --- updated-dependencies: - dependency-name: actions/download-artifact dependency-version: 8.0.1 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [dorny/paths-filter](https://github.com/dorny/paths-filter) from 4.0.2 to 4.0.3. - [Release notes](https://github.com/dorny/paths-filter/releases) - [Changelog](https://github.com/dorny/paths-filter/blob/master/CHANGELOG.md) - [Commits](dorny/paths-filter@7b450ff...ceb8a2b) --- updated-dependencies: - dependency-name: dorny/paths-filter dependency-version: 4.0.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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 a06b542. Configure here.
tracebloc-release-train
Bot
added
gate-nudge
Contributor
Author
|
/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 ( Two caveats, stated rather than glossed:
|
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
1 participant
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-stagingbranch (a mirror ofdevelop), so it never collides with a human PR. Merged only when the fr-gate is green.Note
Medium Risk
Kubernetes dependency upgrade plus changed doctor scheduling diagnostics and user-facing remedies affect training readiness signals; changes are well-tested but touch core cluster health paths.
Overview
Release train promotion bumps
VERSIONto 0.10.22, upgrades Kubernetes client libraries (k8s.io/*v0.36.4 → v0.37.0) with matchinggo.sumchurn, and pins several GitHub Actions (paths-filter, cosign-installer, download-artifact, gh-release).The substantive product change is
tracebloc doctornode-capacity logic (backend#2870 / #628). Node fit is now judged against free CPU/memory on each node (allocatable minus steady-state pod requests, excluding batch Jobs), not raw allocatable. That surfaces a distinct over-committed control plane failure with remediation that explicitly does not recommendresources set max, and fixes rollup ordering so that case wins over co-occurring stuck-Pending pod warnings. When the pod list cannot be read, capacity is a can't-check (CantVerifyFreeCompute) instead of a green “ready”; GPU-soft warnings are merged with that caveat when both apply. The drift nudge caps suggested run sizes by free capacity and namesresources set --cores … --memory …Giwhenset maxwould disagree with the printed ceiling.Tests and golden strings cover the new messages and rollup behavior.
Reviewed by Cursor Bugbot for commit a06b542. Bugbot is set up for automated code reviews on this repo. Configure here.