fix(chart): a declared CPU-only install must emit an explicit empty GPU_LIMITS (backend#2216)

[LukasWodka]

Closes part of tracebloc/backend#2216 — the upstream half. Found by @shujaatTracebloc reviewing tracebloc/client-runtime#355.

Three situations, not two

jobs-manager's _gpu_available_from_env (client-runtime#80) reads three states, deliberately:

values	meaning to jobs-manager
GPU_LIMITS: "nvidia.com/gpu=2"	GPU install
GPU_LIMITS: ""(explicit empty)	no GPU on this cluster
key absent	nobody said — assume a GPU (legacy default)

client#343 gated both GPU vars on a non-emptyGPU_LIMITS, which collapsed rows 2 and 3 into one silence. An operator who wrote GPU_LIMITS: "" emitted nothing, so their edge was indistinguishable from a GPU install.

What that cost

Measured on client-runtime#355, on a CPU-only multi-node edge:

1. _gpu_available came back True — the declaration had been thrown away;
2. _configure_resources took the GPU branch and every job requested nvidia.com/gpu;
3. the pod wedged Pending; check_pending_jobs took the elastic_no_gpu_node fall-through;
4. it waited out the 600s scale-from-zero grace before being respun on CPU — once per experiment.

That is backend#1874's Pending-then-respin cycle, and worse than the ~206s tax backend#2089 removed.

The change

The gate becomes hasKey: key present → emit it, empty value included. Key absent → emit neither.

#343's actual defect stays fixed. The phantom GPU in the CLI resources view came from the "nvidia.com/gpu=1"default, not from the var being present — and that default now applies only when GPU_LIMITS is non-empty. An explicit empty renders as an explicit empty.

Both containers carry their own copy of the gate, so both are changed, and both are asserted separately rather than assumed to agree — a lone GPU request with no matching limit is rejected by the API server.

This gate had no test at all

From #343 until now, nothing asserted the GPU env vars in either direction — which is how one silence standing for two meanings survived. Six tests added, covering all three situations plus the phantom-GPU regression #343 was opened for.

Mutation-proved:

mutation	reddens
restore #343's non-empty gate (the exact defect)	2
emit the phantom default on a declared-empty install	3
emit unconditionally (loses the can't-tell case)	1

488 chart tests pass; helm lint clean. Chart bumped 1.9.48 → 1.9.49.

Sequencing

Ahead of client-runtime#355. With the declaration restored, a CPU-only multi-node edge takes the CPU branch outright and does not need that PR's spawn-time gate at all — which is exactly why @shujaatTracebloc asked for this to land before/with it.

---

Note

^{Cursor Bugbot is generating a summary for commit 98f1547. Configure here.}

[shujaatTracebloc]

Approving — the severed signal is genuinely restored, and I verified it end to end

This is the upstream half of client-runtime#355, and the reasoning in the template comment is the right one: hasKey vs non-emptiness is the difference between "the operator declared CPU-only" and "nobody said anything", and jobs-manager has always read those differently on purpose.

The declaration now actually reaches the runtime. Rendered both ways rather than trusting the tests:

--set env.GPU_LIMITS="" → GPU_REQUESTS: "" GPU_LIMITS: ""
--set env.GPU_LIMITS="nvidia.com/gpu=2" → GPU_REQUESTS: "nvidia.com/gpu=1" GPU_LIMITS: "nvidia.com/gpu=2"
no env.GPU_LIMITS at all → neither var

The first line is the fix. _gpu_available_from_env("", "") is False, so _configure_resources takes the CPU branch outright and that cluster shape stops needing the node-capacity gate at all — which is exactly what closes the ⚠️CPU-only, SINGLE_NODE=false row recorded on #355.

And it reaches it on the paths that matter, which is the thing I most wanted to check, since hasKey is only a fix if somebody actually writes the key. Both installers write it unconditionally with an empty value for CPU-only — scripts/lib/install-client-helm.sh:1407 (gpu_val="") and scripts/install-k8s.ps1:4912 ($gpuVal = "", gated on K3D_GPU_FLAG per #616) — so every installer-provisioned CPU-only edge picks this up. Chart-direct installs that never mention GPU keep the legacy assume-a-GPU default, which is the right call and is what the fourth test pins.

The phantom GPU stays fixed, and for the reason stated: #343's defect was the "nvidia.com/gpu=1"default, not the var's presence. The new else if makes that explicit — an empty GPU_LIMITS now yields an empty GPU_REQUESTS rather than the default — and it is pinned in the right direction.

Mutation-proved, mine not the description's. Six tests, and I checked each one bites:

gate back to (default "" …) → 2 failed # the #343 bug itself
drop the empty-aware GPU_REQUESTS → 3 failed # incl. the phantom-GPU notContains
emit unconditionally (no gate) → 1 failed # "absent key emits NEITHER"

I was initially unsure the fourth test could bite, since notContains matches an exact name/value pair and would not catch the var being emitted with some other value — but under the realistic mutation both vars render as "", so it does redden. Good enough, and worth noting the gate had no chart test at all in either direction from #343 until now.

Chart version bumped in lockstep (1.9.48 → 1.9.49). The 4 locally-failing suites are identical on develop (5 failed, 5 errored both sides — a local helm/plugin-version artefact), and this PR moves the count 477 → 483 passed. CI green.

One non-blocking note, pre-existing and not yours to fix here

GPU_LIMITS: "nvidia.com/gpu=2" with no explicit GPU_REQUESTS renders requests: nvidia.com/gpu=1 against limits: nvidia.com/gpu=2. Kubernetes rejects extended resources whose request and limit differ, so that pod is invalid at admission — and _get_gpu_resources takes the env-override branch, so nothing downstream reconciles them. That asymmetry has been there since #343 and this PR does not change it.

The reason I mention it: the first new test now pins that pairing under the name "a GPU install emits both vars with the configured value" — and GPU_REQUESTS is not the configured value, it is the hardcoded 1. A future reader takes that name as a statement that 2/1 is intended. Either renaming it to say what it checks, or deriving the GPU_REQUESTS default from GPU_LIMITS, would do — happy for it to be a follow-up either way, it does not hold up the fix.

[saadqbal]

Checked the three-state claim against the chart rather than the description, and it stands up:

• values.yaml keeps GPU_LIMITScommented out, so "key absent" is genuinely the default and the legacy assume-a-GPU path stays reachable. Had the chart shipped GPU_LIMITS: "" as a default, hasKey would be always-true and a GPU install that forgot to set it would now declare itself CPU-only — that's the failure mode this would have had, and it isn't there.
• The declared-empty case is real, not hypothetical: install-client-helm.sh:1407 writes GPU_LIMITS: "$gpu_val" with gpu_val empty on a no-GPU host, and install-k8s.ps1:5025 does the same via --set-string. So every installer-provisioned CPU-only edge was hitting the collapsed silence.
• Both gate sites changed, and the tests assert containers[0] and containers[1] separately rather than assuming they agree — right call, since they're independent copies.
• hasKey .Values.env needs .Values.env to be a dict, which is already assumed two lines up by the RESOURCE_LIMITS gate, so no new --reuse-values nil exposure.

Tests are derived rather than restated: the declared-empty test fails against #343's non-empty gate, and the absent-key test fails against an unconditional emit. That matches the mutation table.

One non-blocking note: GPU_REQUESTS: "" set explicitly alongside a non-empty GPU_LIMITS still emits a limit with an empty request. Pre-existing, no caller does it, and the installer can't — just noting it since the comment above the gate makes a point of request/limit travelling together.