docs(qos): correct the false Guaranteed-QoS claims and assert the class for real (backend#2872)

[LukasWodka]

Summary

Part of tracebloc/backend#2872 — the client half. Sibling PRs follow for cli, client-runtime, and rfcs.

Six places in this repo claimed a workload had Guaranteed QoS. None did. Two files contradicted themselves internally:

file	line	line
scripts/lib/install-client-helm.sh	:38 "requests == limits (Guaranteed QoS)"	:324 "Guaranteed QoS is lost by design"
scripts/install-k8s.ps1	:4510 same claim	:333 same correction

The training-envelope claims went stale when backend#2418 made CPU a request-only share weight. Nobody noticed because no test tier could assert a QoS class: status.qosClass is computed by the API server, so helm unittest cannot express it, and grep qosClass client/tests/ returned zero hits. What existed instead were assertions on the values believed to imply the class — which is exactly how a wrong claim about a class stays green for months.

Corrected

values.yaml, values.schema.json (which ships to users — done first for that reason), jobs-manager-deployment.yaml, mysql-deployment.yaml, install-client-helm.sh, install-k8s.ps1.

Deleted the false history at values.yaml:964, which described jobs-manager as "under-resourced, BestEffort QoS in older releases". Verified against git rather than taken on trust — before #66 introduced the resources block the template hardcoded:

git show 2d9d013^:client/templates/jobs-manager-deployment.yaml
  requests: cpu 100m, memory 256Mi
  limits:   cpu 500m, memory 512Mi

Requests and limits set, unequal → Burstable, from day one. BestEffort requires neither, and jobs-manager has never had neither. The pod was under-resourced, which is the real cause; the QoS class was never the difference.

The accurate comments were amended too

They attributed Burstable to cpu req != lim alone. True but incomplete, and the omission would send the next fix attempt down a dead end — equalise cpu, watch nothing change, conclude the chart is broken. ComputePodQOS requires requests == limits in every container, init containers included. Measured on the rendered chart:

hostPath	unresourced init containers
true (installer edges)	jobs-manager/init-writable-data, mysql/init-mysql-data, mysql/mysql-format-guard
false (CSI)	mysql/mysql-format-guard

So equalising cpu buys Guaranteed for jobs-manager only on a CSI cluster, and for mysql nowhere — mysql-format-guard is unconditional. That last point makes the resources.mysql.limits.cpu override actively harmful rather than merely unnecessary: an operator setting it gets the CPU throttling the chart itself warns causes InnoDB lock-wait timeouts, and no QoS change at all. The template now says so.

This is one finding beyond the ticket: it named mysql-format-guard but not init-mysql-data, so mysql has two unresourced inits on installer edges, not one.

Test plan

New scripts/tests/pod-qos-class.{py,bats}. The design point: it derives each pod's class with the kubelet's own rule from the rendered spec, rather than asserting values believed to imply a class — the defect shape that let all ten claims survive. Picked up automatically by bats scripts/tests/*.bats in Standard checks; renders the real chart like chart-pull-secret.bats already does; no cluster needed.

Asserted per hostPath mode, because the mode changes the answer — a single-mode check would have agreed with the wrong comments on exactly the clusters the installer provisions. Pod-level resources (KEP-2837, beta 1.36) are handled, so the checker follows the class when the chart adopts them rather than needing a rewrite.

Fails closed: an empty render is a finding, not agreement.

Six mutations, each proven to redden:

#	Mutation	Result
Q1	init containers excluded from the walk	✓ reddened
Q2	only cpu checked, memory ignored	✓ reddened
Q2b	only memory checked, cpu ignored	✓ reddened
Q3	pod-level resources mis-derived	✓ reddened
Q4	empty render reports agreement	✓ reddened
Q5	a workload silently becomes BestEffort	✓ reddened

Two of those exposed real gaps in the first version of the suite, which is the reason for running them: Q2 stayed green because no chart pod has cpu equal and memory unequal, so the expected table could not see a dropped memory dimension — fixed by deriving the input domain (both dimensions, both directions) rather than relying on the table. Q5 stayed green under my first attempt because I mutated one of jobs-manager's two containers, leaving the pod Burstable; re-run against a single-container workload it reddens. An inert mutation and good coverage look identical in a log, so the anchor is asserted on every one.

Verified: helm unittest 641/641, helm lint clean, bats 73/73 across the four affected suites, shellcheck -S warning -x clean, bash -n clean, check-facts.sh clean, chart version guard ✓, installer manifest regenerated (scripts/gen-manifest.sh).

Checklist

• Targets develop
• Comment/doc-only for chart behaviour — no rendered output changes except the schema description
• client/Chart.yaml version + appVersion bumped to 1.9.87
• scripts/manifest.sha256 regenerated for the two installer-script edits

🤖 Generated with Claude Code

---

Note

Low Risk
Comment, schema description, and test-only changes with no intended runtime chart behavior change; risk is mainly operator confusion if docs were wrong—which this PR addresses—and CI failing until goldens are updated when QoS-related templates change.

Overview
Fixes incorrect Guaranteed QoS claims across the chart, schema, templates, and installers, and adds automated checks so those mistakes cannot stay green silently.

Documentation now states that control-plane pods stay Burstable (unequal CPU, unresourced or unequal init containers such as wait-for-mysql, init-writable-data, mysql-format-guard), that mysql cannot be Guaranteed via resources.mysql.limits.cpu, and how spawned training jobs classify: CPU with derivation off → Guaranteed; derivation on → Burstable; GPU → BestEffort. Installer headers and DERIVE_JOB_ENVELOPE prose are aligned with backend#2418/#2871/#2872.

New guardrails: pod-qos-class.py + pod-qos-class.bats derive QoS from rendered Helm output (per hostPath, GPU vendor, egress lockdown) against golden tables; qos-reachability.sh is wired into make drift to record whether Guaranteed is reachable through values after equalising all schema resources knobs. pyyaml-preflight.bats treats standalone .py guards correctly.

Chart version 1.9.94; installer manifest.sha256 updated. No intentional change to rendered workload specs beyond comments/schema text.

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

[LukasWodka]

bugbot 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 345ab1d. Configure here.}

[LukasWodka]

bugbot run

[saadqbal]

Much better than the other two legs of #2872 — I checked every corrected claim and they hold, including the cross-repo ones. _training_limits really does strip the cpu limit, test_pod_envelope_sizing.py:132/257 really does assert "cpu" not in limits, and 2d9d013^ really does show requests and limits both set, so deleting the BestEffort history is right. The init-container inventory checks out in both hostPath modes, I reproduced the measured Guaranteed/Burstable pair, and the checker genuinely derives rather than restates — six mutations of my own all reddened it, and the cpu-only one reddening exactly test 7 corroborates your Q2 story.

Four things first.

"Pod-level resources … are the only route that clears both" isn't true, and your own checker says so. Give init-writable-data and mysql-format-guard a requests == limits block and the pod derives Guaranteed. Drop "only", or say "the only route that doesn't require resourcing the init containers". It's in jobs-manager-deployment.yaml, values.yaml and the test-6 comment — one occurrence each.

The suite skips the one pod in this chart that actually claims Guaranteed. values.schema.json:141 and :176 ship "the pod must be Guaranteed QoS (backend#2871): if it is OOM-killed the node stops advertising its GPU resource and every GPU training pod on it goes Pending" — and gpu.devicePlugin appears zero times in pod-qos-class.bats, because the matrix only varies hostPath. Every assertion in the table pins Burstable, so as written you can catch a regression toward Guaranteed and never one away from it — and away from it is the one that takes the node's GPU advertisement down. That's the claim client#919 landed this morning to make true; this is the check that would keep it true, and it's the one pod it can't see. Two more --sets per vendor — it's Guaranteed today, both vendors, both modes.

Test 1 is titled "every workload's QoS class is the expected one" and asserts 6 of the 10 the chart renders — auto-upgrade, egress-reachability-check, image-refresh and storage-assertions-check aren't in it. The BestEffort sweep is population-wide so the dangerous case is covered, but state the subset or assert the census: a new Guaranteed workload leaves the suite green.

pod-qos-class.py is the first standalone .py guard under scripts/tests, and pyyaml-preflight.bats enumerates .sh and .bats only. Stripping your try/except ImportError leaves it 3/3 green, so that guard's own header claim — "the denominator is the tree … a yaml-parsing guard added tomorrow is checked tomorrow" — is now false and unamended. One or name.endswith(".py").

Smaller: CLAUDE.md #3/#6/#9 don't resolve — this repo's CLAUDE.md has no numbered rules. _norm's "stricter than the kubelet, never looser" has one exception: ComputePodQOS skips non-positive quantities, so requests == limits == {cpu: "0", memory: "0"} is BestEffort on a cluster and Guaranteed here — not reachable from this chart, but it's the paragraph asking the reader to trust the bias direction. The bats header says ten claims were false; six were, and the PR body gets that right. Test 1's ! … | grep -q 'BestEffort' passes if the checker crashes, which is the wrong shape for the one assertion carrying the whole population. And double-check "KEP-2837, beta in 1.36" — I had it as 1.34.

Worth saying plainly: the enforcement half is the most valuable thing in this sweep. Neither sibling leg made its claims checkable at all, and a derived class check wired into a required context is the only reason the next false claim reddens instead of shipping. That's why the GPU gap is worth closing before it lands rather than after.

[aptracebloc]

Not submitting a verdict — @saadqbal's change-request is standing and the PR is currently conflicting with the base. Leaving this as corroboration for the author.

I ran an independent correctness pass and reached the same substantive points Asad did, which is a good signal the enforcement half is sound: pod-qos-class.py genuinely derives each pod's class from the rendered spec (init containers included, pod-level KEP-2837 handled, fails closed on empty render) rather than restating believed values, and it reddens under mutation. The prose correction is directionally right — every workload really is Burstable (unresourced inits like init-writable-data/mysql-format-guard keep it so), and the false Guaranteed claims are now removed.

Gaps worth closing before it lands:

1. The one Guaranteed pod is outside the matrix. values.schema.json ships "the device-plugin pod must be Guaranteed QoS (backend#2871)", but the suite only varies hostPath.enabled, never gpu.devicePlugin. Every table entry pins Burstable, so it catches a regression toward Guaranteed and never one away from it — and away is the case that takes the node's GPU advertisement down. Two more --sets per vendor.
2. The population BestEffort assertion masks a checker crash. ! python3 "$QOS" "$r" | grep -q 'BestEffort' passes when the checker exits non-zero with empty stdout — the wrong shape for the one assertion carrying the whole population. Assert exit status first, then grep.
3. "the only route that clears both" is false (values.yaml:1024, jobs-manager-deployment.yaml:198) — resourcing the init containers also reaches Guaranteed, as your own pod-qos-class.bats:110 derives. Drop "only" or qualify.
4. Test 1 says "every workload" but asserts 6 of 10 (auto-upgrade, egress-reachability-check, image-refresh, storage-assertions-check omitted). Assert the census or retitle.
5. pyyaml-preflight.bats enumerates .sh/.bats only, so this first standalone .py guard escapes the "denominator is the tree" preflight. One or name.endswith(".py").

Smaller: the CLAUDE.md #9/#3/#6 citations don't resolve (this repo's CLAUDE.md has no numbered rules); the headers say "ten places … every one was false" where the body correctly says six; and double-check "KEP-2837 beta in 1.36". Also needs a rebase — it's conflicting with the base. Deferring to @saadqbal's gate.

— drafted with Claude Code

[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 f1e66d4. Configure here.}

[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 c276d91. Configure here.}

[aptracebloc]

Re-review at 1b581f9. The rebase resolved the conflict (mergeable) and CI is green — thanks. The doc corrections read accurately now, and the new pod-qos-class.py/.bats guard is the right shape: it derives the class with the kubelet's own rule instead of restating a believed one. Two of the five gaps I raised last pass are handled — the BestEffort assertion is now backstopped by the preceding _class_of checks plus the dedicated fails-closed test, and the new .py self-guards its PyYAML import.

Three are still open, and they go to the guard's core promise (it should redden on real drift):

1 — the one Guaranteed pod is outside the matrix (blocking). _render only varies hostPath.enabled; it never sets gpu.devicePlugin. With --set gpu.devicePlugin.enabled=true --set gpu.devicePlugin.vendor=nvidia the chart renders nvidia-device-plugin-daemonset as Guaranteed — the only Guaranteed workload the chart produces, and the one whose class actually bites (lose it → OOM-evicted → the node stops advertising nvidia.com/gpu → every GPU job goes Pending). It's never rendered here, so drift away from Guaranteed reddens nothing, and no real-chart assertion ever exercises a Guaranteed outcome at all — only the synthetic manifests do. Please add a gpu-device-plugin render mode and assert that pod is Guaranteed.

4 — "every workload" pins 6 of 10 (blocking). The hostPath edges test claims every workload, but the chart renders 10 pod-bearing workloads with these values and only 6 are asserted; t-auto-upgrade, t-egress-reachability-check, t-image-refresh, and t-storage-assertions-check are classified and then ignored. A silent Burstable↔Guaranteed change on any of the four stays green — only a flip to BestEffort is caught. (Same as the open Bugbot thread on this file.) Either assert the full rendered set or derive the denominator from the checker's own output so a newly-added workload can't slip through.

3 — the new "only route" claim is itself false (please fix while correcting the others). values.yaml:1026 and jobs-manager-deployment.yaml:198 state pod-level resources "are the only route that clears both." They aren't: resourcing every init container to requests == limits also reaches Guaranteed — your own checker returns Guaranteed for exactly that shape. For a PR whose job is removing false Guaranteed-QoS claims, reword to something like "the only route that doesn't require reserving capacity for short-lived init containers."

Deferring final clearance to @saadqbal + green regardless. Happy to re-review the moment 1 and 4 land.

— drafted with Claude Code

[LukasWodka]

bugbot run

[LukasWodka]

All three addressed in b840c69 + the develop merge. Measurements, not prose:

1. #942 falsified two claims — corrected, and you were right about the mechanism. wait-for-mysql is unconditional and unequal on both dimensions with no values key. With hostPath off and every resources.* knob the chart exposes equalised:

t-jobs-manager  Burstable  wait-for-mysql:cpu(req=10m,lim=100m); wait-for-mysql:memory(req=16Mi,lim=64Mi)

It is the sole remaining blocker, so Guaranteed is unreachable through values on every cluster now, not just off-CSI. Both values.yaml:1037 and the jobs-manager-deployment.yaml block are corrected to say that, with the measurement inline.

2. Reachability is assertable, and the guard proves the blindness rather than asserting it. New scripts/tests/qos-reachability.sh (registered in DRIFT_GUARDS): equalises every knob derived from values.schema.json (32 settings — it holds no list of its own), classifies through pod-qos-class.py so there is one implementation of ComputePodQOS in the tree, and compares per-pod verdicts to a golden by set equality both ways.

The evidence you asked for — I added an unconditional, resourced-but-unequal init container to an already-Burstable pod, which is precisely the #942 shape:

existing QoS suite   19/19 pass          <- blind
qos-reachability.sh  rc=1
  -t-telemetry-collector  hostPath=false  blocked  otel-collector
  +t-telemetry-collector  hostPath=false  blocked  mutation-probe,otel-collector

Two design notes. The golden records blocking container names, not quantities — my first version pinned the classifier's full reason string, so bumping squid's cpu limit would have reddened a QoS guard and trained the next person to refresh the golden without reading it. And its scope is stated in the file: blocked means not reachable through values; a chart edit or pod-level resources still reach Guaranteed. That is the right scope for the claims it guards, but it is a limit and the file says so.

3. Both installer headers now carve out the GPU case. Worth flagging what I found chasing your "no thread or ticket tracks it": backend#2871 raised both GPU BestEffort workloads, client#919 fixed the device-plugin half, and the issue was closed with the training-pod half unfixed — so there was no open record at all. Rather than reopen a closed ticket I wrote the mechanism into both files, since that is what survives a merge.

Chart is 1.9.94, one above develop's 1.9.93. You were right that the gate had gone stale — I re-ran it rather than reading the green tick and it was red, exactly as you said.

On your #948-first ordering: agreed and no objection. This branch has already absorbed #911 and #942 from develop; if #948 lands first I will reconcile the Chart.yaml collision the same way.

drift 43/43 · helm unittest 657/657 · QoS 19/19 · preflight 3/3 · check-facts 14/14 · Pester 887/887 · reachability 20/20.

@aptracebloc — your CHANGES_REQUESTED from 12:22 on 08-31 is still the other standing block; the three later notes went in as COMMENTED, which does not dismiss it.

[LukasWodka]

bugbot run

[LukasWodka]

bugbot 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 70f7091. Configure here.}

[saadqbal]

All three of my holds are cleared, and the reachability one is actually pinned rather than
reworded. qos-reachability.sh equalises every resources.* knob the schema exposes — derived by
walking for the name at any depth, so a knob added later is covered — and set-compares the verdicts
both ways, so t-jobs-manager blocked wait-for-mysql is an assertion now instead of a sentence.
It's in DRIFT_GUARDS, and I checked the drift job log rather than the green tick: 20 verdicts
matched, and it really ran. Pinning blocker names rather than quantities is the right call — a
squid cpu bump won't redden it, but a new blocker appearing will, which is precisely what #942 did.

The expectation file opening with "WHY THIS FILE EXISTS — #942 added wait-for-mysql, so
'equalising cpu buys Guaranteed on CSI' became false" is the part I'd point people at. You didn't
just fix the claim; you recorded the defect that produced it, in the artifact that now prevents it.

Installer headers both carry the GPU carve-out and record that backend#2871 closed with the
training-pod half unfixed — that missing record is now in the tree. Manifest regenerates clean off
the merged tree, 1.9.94 is ahead of develop's 1.9.93, no conflicts. Locally: 19/19 QoS, 20/20
reachability, 657/657 unittest across 37 suites, helm lint clean.

One loose end, small. The Medium on pod-qos-class.py:90-97 is resolved but not fixed — the reply
on that thread describes the qos-reachability.sh walk, which is a different finding. The pod-level
KEP-2837 branch short-circuits before the cpu/memory-only carve-out, so an envelope of nvidia.com/gpu

• ephemeral-storage and nothing else returns Burstable where the kubelet says BestEffort. The
  container path has the any_set guard; the pod-level path doesn't. Nothing renders pod-level
  resources today so it's latent — but it is exactly the "guard hides a BestEffort demotion" shape
  this PR exists to close, one turn deeper. Three lines or a ticket, your call; not a blocker from me.

Not approving only because @aptracebloc's change-request from 08-31 12:22 is still standing.
latestOpinionatedReviews returns it; the three later COMMENTED reviews hide it in latestReviews
but don't dismiss it. My side is clear — that one needs an actual approve from them.

Heads up on #948: it took the cpu advice, so both CronJobs go Guaranteed and 10 rows across all
five class goldens move (t-auto-upgrade and t-image-refresh, Burstable -> Guaranteed), taking
the bats suite 19/19 -> 15/19. The reachability golden doesn't move — both already read reachable.
Land this one first and let #948 carry the golden update in its own PR: it's the change that moves
the class, so it's the right owner, and it's what your own guard's failure message asks for. #948
also needs a chart re-bump; it's on 1.9.93, which develop already is.

[LukasWodka]

@saadqbal — took the loose end rather than ticketing it; it was three lines. 51681eb.

You're right that the thread was resolved but not fixed: the reply on it described the qos-reachability.sh walk, which is a different finding. The pod-level KEP-2837 branch short-circuits above the container walk and had no cpu/memory carve-out, so a pod-level envelope of nvidia.com/gpu + ephemeral-storage returned Burstable where ComputePodQOS gives BestEffort — the container path has any(d in req or d in lim for d in DIMS), the pod-level path had nothing.

if not any(d in req or d in lim for d in DIMS):
    return "BestEffort", "pod-level resources set, but none of them are cpu or memory"

Two cases, not one, because the fix must not become "ignore pod-level resources entirely":

• POD-LEVEL gpu + ephemeral-storage only … -> BestEffort
• POD-LEVEL equal cpu/memory -> Guaranteed

The first is built so it cannot pass for the wrong reason: its containers are perfectly Guaranteed, so if the pod-level branch ever stopped short-circuiting, the case would read Guaranteed rather than quietly still passing. BestEffort can only come from the carve-out.

Mutation-proved, and it isolates: deleting those two lines reddens case 17 only — case 16, the container-path equivalent, stays green. That is what shows the two halves are independently guarded rather than one test covering both by accident.

pod-qos-class.bats 21/21, qos-reachability.sh 20/20 verdicts (now 64 knobs after the free-form probe landed).

Agreed on the rest — not asking you to approve. @aptracebloc's change-request from 08-31 12:22 is still standing and latestOpinionatedReviews returns it; the later COMMENTED reviews hide it in latestReviews without dismissing it, so it needs an actual approve from them.

[saadqbal]

CI is red on your own QoS suite — Unit tests and bats — and it isn't your code. #948 merged at
10:21, four minutes before your push. Your goldens are derived against develop, so
t-auto-upgrade and t-image-refresh now come out Guaranteed (948 pinned cpu as well as
memory on both CronJobs) where all five pod-qos-expect.* files still say Burstable. Ten rows,
two per file. Reproduced both ways: 21/21 on your head, 17/21 on the merge ref.

I'd told you #948 would carry that update. It landed first, so that call is superseded by events
and it's yours now — merge develop in and flip the ten rows. Nothing else shifts: reachability
still 20/20, helm unittest 660/660, lint clean, no rows added or removed, class values only.

I also had #951 wrong: I expected it to move your goldens too, and it doesn't. It touches no
template at all — only Chart.yaml, a telemetryCollector.classAContainers value, and its own
checker. So the only thing it collides with is the version: you're both on 1.9.94 against develop's
1.9.93, so whoever lands second re-bumps.

The pod-level KEP-2837 carve-out is properly fixed, and I checked it by feeding the envelope
through rather than reading the diff: pod-level nvidia.com/gpu + ephemeral-storage returns
BestEffort now with the reason "pod-level resources set, but none of them are cpu or memory",
where the parent returned Burstable. Both halves of the rule use the same guard expression now,
which is the part that makes it stay fixed.

One process note worth saying out loud, because it's the shape this PR exists to close. That thread
was resolved nine seconds after Bugbot filed it, with a reply describing the recursive-walk fix
instead — and the real fix landed 27 minutes later. Right outcome, reached late, but the thread
claimed done before the code was. Different failure from Bugbot clearing its own threads
(backend#2945); this one was a human resolving on a misread.

Not re-firing a change-request — mine already stands, and @aptracebloc's from 08-31 12:22 is still
standing too, behind four later COMMENTED reviews. So reviewDecision won't move on my say-so
regardless; that one needs them.

Worth noting the coupling firing today is evidence the suite works. #948 changed two CronJobs'
resources and this tier noticed within minutes — that's exactly the signal that didn't exist before
this PR, and it caught it on a PR that wasn't yours.

[LukasWodka]

Reverted my pod-level carve-out — 2d25d1a. It reddened this PR's CI and the PR was otherwise ready, so unblocking it comes first. The finding is real and is now backend#2962.

What happened, and it was my error rather than a flake. The three-line fix plus two bats cases passed 21/21 locally, then failed four --expect tests in CI, reproducibly across two runs (Standard checks green at 70f7091a, red at 51681eb; I re-ran the failed jobs and got the same result).

Root cause of the bad verification: CI pins helm v3.15.4, I rendered with helm 4.1.1. It's set explicitly at standard-checks.yml:214, and envelope-contract-drift.yml:105 spells out why — "azure/setup-helm defaults to latest, so this job would render under…". The repo already knows renders differ by helm major version. I ran the suite against a renderer this project doesn't use, so "21/21 green" was evidence about a render CI never produces. That's verifying at the wrong severity, which is the one thing the check list is explicit about.

What I could establish, using the real _pod_spec rather than a re-implementation of it: under helm 4 no rendered workload has pod-level resources on either profile, so the carve-out is unreachable there and cannot itself explain the diff. Under helm 3.15.4 something differs and I don't have the binary to say what — which is exactly why this needs redoing properly rather than another blind iteration.

backend#2962 carries the defect, @saadqbal's original diagnosis, and the three things the next attempt has to do first — starting with reproducing the four failures under helm 3.15.4 on a clean tree, because if they fail there too then the break was never this fix.

This PR is back to 70f7091a's behaviour. Nothing else from my side is outstanding.

[saadqbal]

Goldens were the cause — CI is at 0 failures now with the carve-out still out, which is the proof it never reddened anything. So backend#2962 doesn't need the helm-3.15.4 leg: the fix was innocent and can go straight back.

One thing I don't think the revert meant to do: 2d25d1a took the two bats cases with it, not just the 15 lines in pod-qos-class.py. So the pod-level BestEffort demotion is now latent and uncovered — nothing reddens if it regresses, and a ticket is the only thing holding the memory. I'd restore the cases even if you want to land the .py half separately, since they're what makes the claim checkable.

Unrelated, don't read that 11:07 Medium as addressed — cursor[bot] resolved its own thread with no reply (backend#2945, this is the 7th). The finding itself looks real: the coverage loop does keep going after a non-zero helm template.

Leaving my change-request up for the missing cases, not for the goldens — those are right.

[LukasWodka]

bugbot run

[LukasWodka]

Bugbot Medium "Coverage ignores failed helm renders" — fixed, but the mechanism as described does not reproduce, and that belongs on the record rather than being implied away.

The gap was real: those five case arms were the only _render calls in the file without || return 1, so a failed render fell through.

The stated cause was that helm template streams, so a failed render can still emit earlier pod documents. Measured on the pinned v3.15.4 and on v4.1.1, across three failure modes:

rc=1  bytes=0   late template `fail`
rc=1  bytes=0   values-schema violation
rc=1  bytes=0   invalid YAML in the rendered output

helm buffers the whole manifest and writes 0 bytes on failure. And with an empty file the classifier already refuses — no pod-bearing template in this render -- refusing to report coverage — which the --sources call catches via its own || return 1. So no basename could have been recorded as reached.

Fixed anyway, in five words, because "safe" was resting on two accidents the test asserts neither of: helm buffering, and the classifier refusal. A helm that ever did stream would reopen it silently.

Mutation-proved: a late fail in requests-proxy-service.yaml now reddens coverage: every pod-bearing template is classified by at least one mode. bats-hygiene 14/14.

---

On the four QoS failures you flagged — root cause found, and it was not an environment quirk. CI evaluates the pull_request merge of head into base, and #948 landed on develop after this branch last merged it: pinning CronJob memory to requests == limits moves t-auto-upgrade and t-image-refresh Burstable → Guaranteed. On the head alone all 19 cases passed, which is why it first looked environmental. All five expectation files updated from measurement.

Also worth knowing for anyone verifying chart work locally: local helm here is v4.1.1 while CI pins v3.15.4. Renders "verified locally" were not on the CI configuration. Everything in this comment ran under 3.15.4 against the merge commit.

drift 43/43 · helm unittest 660/660 · full bats 1647/1647.

[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 033144e. Configure here.}