fix(installer): a cordoned node must not anchor the training envelope (backend#2237)

[LukasWodka]

The defect

scripts/tests/fixtures/envelope_contract.json:91 declares spec.unschedulable (cordoned) a node the envelope sizing skips. Neither installer honoured it — neither even asked the API server for the field.

On a heterogeneous cluster a cordoned large node became the sizing anchor, so the installer wrote an envelope no live node can satisfy and every training pod sat Pending with no obvious cause.

The contract's own one-cordoned-out vector did not catch this, because scripts/gen-envelope-embed.sh:191 pre-filtered cordoned nodes out of the golden:

live = [n for n in v["nodes"] if not n.get("unschedulable")]

That is the golden generator, not the installer. So the row replayed as a lone 4 16Gi node and the code under test was never handed a cordoned one. The fixture was inert, and had been all along.

---

The four acceptance criteria

1. install-client-helm.sh excludes cordoned nodes at every ranking site ✅

There were two ranking sites (_machine_training_resources L173, _machine_training_ceiling L218) running a byte-identical 17-line loop. Rather than add the skip twice, the loop is now one function, _anchor_largest_schedulable, that both call — CLAUDE.md rule 1. Two copies of a selection rule is exactly how the (memory, cpu) / (cpu, memory) split in backend#2220 happened; one copy cannot drift from itself.

The jsonpath is also a single constant, _TB_NODE_JSONPATH, now emitting a third field.

One subtlety worth flagging: the skip is written [[ "$unsched" != "true" ]] || continue, not == "true" && continue. The latter evaluates to 1 for every schedulable node, which under the installer's set -euo pipefail aborts the whole run — the shape scripts/tests/pipefail-early-close.bats exists to catch.

2. install-k8s.ps1 does the same ✅

PowerShell has exactly one envelope-ranking site (Get-TrainingResources, L4273). The other kubectl get nodes calls the ticket cited (L3204, L4058) query allocatable."nvidia.com/gpu" — GPU probes, a different question, deliberately untouched.

Unschedulable is omitempty, so a live node emits an empty third field that .Trim() drops entirely. Both readers therefore key on the literal true, never on non-emptiness — otherwise an API server that ever serialises false would drop every node from sizing, silently and totally. That value-domain decision is pinned by a test in both languages rather than assumed (CLAUDE.md rule 6).

3. The golden stops pre-filtering ✅

gen-envelope-embed.sh now emits the whole cluster and applies no rule of its own. one-cordoned-out went from

"one-cordoned-out|4 16Gi|cpu=3,memory=13Gi"                 # inert
"one-cordoned-out|16 64Gi true\n4 16Gi |cpu=3,memory=13Gi"  # live

envelope_contract.json itself is unchanged — it is byte-compared against tracebloc/client-runtime at a pinned ref by envelope-contract-drift.yml, so only the generator moved.

Pester previously replayed vectors.single_node only, so the contract's whole multi_node block — the ANCHOR_LARGEST rule and the cordoned vector — was asserted on the bash side alone. That is how the ps1 could ignore spec.unschedulable with a fully green suite. It now replays multi_node too.

4. A regression case that fails before the fix ✅

Covered three ways, all derived from fixtures rather than hand-written per twin:

• the contract replay (one-cordoned-out), now live in both languages;
• 4 new rows in installer_parity.json — the shared cluster-state fixture driven through both installers, so a row forces both languages to answer it;
• named regressions in both suites, including the mirror case (cordoning the small node must change nothing — a filter that just dropped the largest node would pass the headline test while being completely wrong).

---

Mutation proof

Every mutation was verified to have actually applied before running the tests, per CLAUDE.md rule 5 — "an inert mutation and good coverage look identical in a log." The first attempt at mutation 1 silently failed to apply (the perl regex matched only a comment); the anchor check caught it.

M1 — revert the bash skip (grep -c anchor: 1 → 0)

not ok 1 envelope contract: ANCHOR_LARGEST picks the same node the contract says
# anchor-rule drift:
#   one-cordoned-out: want 'cpu=3,memory=13Gi' got 'cpu=15,memory=61Gi'
not ok 2 envelope contract: a cordoned node never takes the anchor
not ok 3 envelope contract: every node cordoned is UNMEASURED, not too small

not ok 3 installer parity: every cluster state produces the declared verdict
#   cordoned-large-node-skipped: size want 'cpu=3,memory=13Gi' got 'cpu=15,memory=61Gi'
#   all-nodes-cordoned: size want 'cpu=2,memory=8Gi' got 'cpu=15,memory=61Gi'

M2 — revert the ps1 skip (anchor: 1 → 0)

[-] every MULTI-NODE golden vector replays (incl. the cordoned one)
    one-cordoned-out [16 64Gi true | 4 16Gi ]: want 'cpu=3,memory=13Gi' got 'cpu=15,memory=61Gi'
[-] a cordoned node never takes the anchor, whichever node it is
[-] every node cordoned reads as UNMEASURED, not as too small
[-] every cluster state produces the declared verdict
    cordoned-large-node-skipped: size want 'cpu=3,memory=13Gi' got 'cpu=15,memory=61Gi'
    all-nodes-cordoned: size want 'cpu=2,memory=8Gi' got 'cpu=15,memory=61Gi'
Tests Passed: 26, Failed: 4

an explicit 'false' third field is schedulable correctly stays green under M2 — it does not exercise the skip.

Proof the fixture really was inert

Restoring the old pre-filtered golden while leaving the bash skip removed:

one-cordoned-out|4 16Gi|cpu=3,memory=13Gi     # the old row
installer cordon filter present: 0            # the bug, reintroduced
ok 1 envelope contract: ANCHOR_LARGEST picks the same node the contract says

Green, with the defect present. That is criterion 3 in one line: fixing the installers without fixing the golden would have fixed nothing testable.

---

A hole the mocks could not see (and the guard that closes it)

Both suites mock kubectl and inject node lines directly, so they exercise the parser and never the query. I reverted only the jsonpath in both installers — leaving both skips intact — and:

ok 1..4 installer parity: every cluster state produces the declared verdict   (bash)
ok 1     envelope contract: ANCHOR_LARGEST picks the same node the contract says
Tests Passed: 3, Failed: 0                                                    (Pester)

Everything green, while in the field the field never arrives, unsched is empty for every node, and cordoned nodes are ranked again. Textbook backend#1729: a mechanism disconnected from the half it claims to check.

Closed by scripts/tests/node-jsonpath-agreement.sh, added to DRIFT_GUARDS (the required Drift checks / Source-of-truth drift job, so it is a gate and not advice — rule 2). It parses both jsonpaths out of the installers, writes neither down, and asserts they are byte-identical and both request {.spec.unschedulable}. Mutation-proven in three directions:

mutation	result
revert ps1 jsonpath only	FAIL — "request DIFFERENT node fields" + missing-field
revert both (the case every mocked suite passed)	FAIL — missing-field on both
break the extraction anchor	FAIL closed — "could not read one or both declarations"

Can the rule be derived in one place?

Within bash, yes — and it now is: one _anchor_largest_schedulable, one _TB_NODE_JSONPATH, replacing two hand-copied loops.

Across bash and PowerShell, genuinely no. One is a sourced bash lib; the other is a signed standalone PowerShell bootstrap that must not fetch anything unsigned at install time — the same constraint that forces the envelope constants to be embedded rather than read (envelope-contract-drift.yml spells this out). The jsonpath is unavoidably written twice.

What is not unavoidable is the two copies drifting, so that is machine-checked in two independent ways rather than left to review: node-jsonpath-agreement.sh pins the query, and the shared installer_parity.json pins the behaviour — one table, two readers, a row forces both languages to answer it.

---

Test evidence

$ bats scripts/tests/*.bats
total=1329 failures=0

$ pwsh -c 'Invoke-Pester scripts/tests'      # Pester 6.0.1, pwsh 7.5.2
✔ all installer scripts verified against the signed manifest
Tests Passed: 884, Failed: 0, Skipped: 13

$ make lint
all 52 shell scripts parse
shellcheck: 59 file(s), severity=error

$ make drift
node jsonpath agreement: both installers request identical node fields, including spec.unschedulable
drift: all 15 guards green

scripts/manifest.sha256 is regenerated: both installer payloads changed, and the signed-manifest verification above confirms it.

Two notes for anyone reproducing locally:

• pipefail-early-close.bats earned its keep. The first full run failed on my new guard — grep -oE ... | head -1 closes the pipe early, and under set -euo pipefail that aborts on SIGPIPE. Rewritten to the house capture-then-slice idiom; the gate is green and the guard still mutation-detects after the rewrite.
• install-client-helm.bats stalls on a workstation that has a real kubectl plus a kubeconfig pointing at an unreachable cluster: several tests don't stub kubectl, so the sizing probe blocks on TCP connect well past --request-timeout. Pre-existing (a pristine-tree baseline stalls in the same test) and invisible in CI, which has no kubeconfig. KUBECONFIG=/dev/null bats ... runs all 191 in 26s.

Not done here

• cli's nodeLarger (tracebloc/cli, Go) is the third reader of ANCHOR_LARGEST and is out of scope for this repo. Whether it skips cordoned nodes is worth a follow-up — the contract binds it too.
• envelope_contract.json is untouched by design; adopting an upstream contract change is a separate, pinned flow.

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes installer node-selection used to size training resources; a mistake can write unschedulable envelopes on BYO clusters. Not auth or data-handling, but it is core install-time resource logic.

Overview
Cordoned nodes no longer win the training-envelope anchor. Both installers now query .spec.unschedulable and skip a node only when that field is the literal true, so a cordoned large node cannot produce an envelope no live node can schedule.

Bash ranking is extracted into one _anchor_largest_schedulable used by both sizing and ceiling. A fully cordoned (or unreadable) cluster stays unmeasured and keeps the historical literal, without undersized warnings.

Goldens stop pre-filtering cordoned nodes, so the contract’s skip is actually tested. Shared parity rows plus bats/Pester regressions cover large-cordoned, small-cordoned, all-cordoned, and explicit false. A new required drift guard (node-jsonpath-agreement.sh) pins the two jsonpaths as identical and both requesting spec.unschedulable, because mocked kubectl never exercises the real query.

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

[saadqbal]

Approving. The best thing in here is the finding underneath the finding: the contract vector existed, the suite was green, and the golden generator was filtering cordoned nodes out before the installer ever saw one — so one-cordoned-out replayed as a lone 4 16Gi node and tested nothing. A fixture that pre-applies the rule under test is the same class as the two other inert-fixture bugs this repo has had, and finding it from a Pending pod rather than from the suite is the point.

Verified the structural claims rather than taking them:

• envelope_contract.json is genuinely untouched — no diff — so the drift check against client-runtime at the pinned ref still means what it meant. Only the generator moved.
• One _anchor_largest_schedulable (scripts/lib/install-client-helm.sh:204), called from both 240 and 273, and one _TB_NODE_JSONPATH at 170 emitting the third field. Two byte-identical ranking loops collapsing into one is the right fix given the (memory, cpu)/(cpu, memory) split in #2220 came from exactly that duplication.
• The value-domain call is right and is pinned in both languages: omitempty means a live node emits an empty third field, so keying on the literal true (install-k8s.ps1:4298, bash :218) rather than on non-emptiness is what stops a future explicit unschedulable: false from dropping every node from sizing. That's the failure that would have been silent and total.
• Pester now replays multi_node with a Count | Should -BeGreaterThan 0 guard, so the block can't quietly become empty and pass. That gap is why the ps1 could ignore spec.unschedulable with a fully green suite.

One correction, non-blocking, on the comment rather than the code. The :214-217 note says the == "true" && continue form "evaluates to 1 for every SCHEDULABLE node, which under the installer's set -euo pipefail aborts the whole run". That isn't quite the mechanism, and I checked both shapes:

# does NOT abort — errexit exempts a failing command in a && list
set -euo pipefail; for n in a b; do u=""; [[ "$u" == "true" ]] && continue; echo "body $n"; done
# → body a, body b, exit 0

# DOES abort — the && list is the function's last statement, so f returns 1
set -euo pipefail; f() { for n in a; do u=""; [[ "$u" == "true" ]] && continue; done; }; f
# → exit 1

So the hazard is real but positional: it bites when the && list lands in tail position of a function (or of the loop that ends one), not on every schedulable node. As written at :218 there is ranking code after it, so both idioms would have been safe here — which means the reason to prefer || continue is robustness against a later edit moving it to the tail, not that the && form is unconditionally fatal. Worth saying precisely, since the next person will reason from that comment.

[saqlainsyed007]

Verified against the code, not the description — this is a strong fix.

The fix is real and in both twins. install-client-helm.sh and install-k8s.ps1 now request .spec.unschedulable and skip cordoned nodes before ranking, and both key on the literal true (not non-emptiness), so a future explicit unschedulable: false can't read as cordoned. The bash ranking is extracted into one _anchor_largest_schedulable, so the sizing path and the ceiling/warning path can no longer describe different nodes.

The vacuity catch is the best part. The golden generator used to pre-filter cordoned nodes, so the contract's one-cordoned-out vector replayed as a lone live node and never handed the installer a cordoned one — the rule was untestable. Now the generator emits the whole cluster and lets the code under test apply the skip. The parity fixtures are non-vacuous: cordoned-small-node-ignored exists specifically to defeat an "always drop the largest" cheat, all-nodes-cordoned pins the unreadable-vs-too-small distinction (no false undersized warning), and explicit-unschedulable-false-is-schedulable pins the reader to true.

node-jsonpath-agreement.sh is derived, mutation-proof, and fail-closed. It parses both jsonpaths out of the two installers and compares byte-identity + asserts both request spec.unschedulable — no path is written down in the guard. It documents (and measured) the precise hole it closes: the mocked suites inject node lines directly, so reverting either jsonpath to two fields keeps them green while the real query stops fetching the field. Two absences are a FAIL, not an equal-compare. Added to DRIFT_GUARDS. manifest.sha256 updated for both changed libs.

CI green, no review threads, mergeable clean. LGTM.