Uh oh!
There was an error while loading. Please reload this page.
fix(installer): wire the GPU-capable k3s-cuda node image on Linux (Closes tracebloc/client#835) - #852
Conversation
On a Linux GPU host the installer detected the GPU, set the host Docker runtime and deployed the NVIDIA device plugin, but cluster.sh always created the k3d node from stock rancher/k3s — which has no in-node NVIDIA container toolkit. The in-node containerd could never hand the GPU to pods: the device plugin reported "No devices found", the node never advertised nvidia.com/gpu, GPU pods couldn't schedule — yet the installer claimed "GPU access". This brings the Linux path to parity with the Windows/WSL2 twin (which already uses the k3s-cuda image). - cluster.sh: _gpu_node_image derives the ghcr.io/tracebloc/k3s-cuda pull ref (k3s pin + CUDA base, mirror/override aware); _create_new_cluster swaps --image to it when GPU is wired; _generate_node_cdi_specs writes the native CDI spec inside each node (the image is CDI-mode and its boot drop-in only covers WSL2); _check_existing_cluster_gpu falls back to CPU on a reused stock node; the drift check recognises the k3s-cuda tag. - install-client-helm.sh: gate the GPU request + device plugin on GPU actually wired (not bare detection) and set RUNTIME_CLASS_NAME=nvidia. - chart: device plugin runs under runtimeClassName: nvidia so it can init NVML on native Linux (new gpu.devicePlugin.nvidia.runtimeClassName). - summary.sh: "NVIDIA GPU" mode + GPU test hint only when actually wired, so the claim is no longer false on a CPU fallback. - common.sh: TB_CUDA_BASE_TAG pin + _gpu_wired predicate; check-facts.sh enforces the new pin and the GPU --image wiring across both installers. Tests: cluster.bats (+16), install-client-helm.bats (+2), summary.bats (+2), gpu_device_plugin_test.yaml (+3). check-facts, shellcheck (error), helm-unittest and the existing GPU/cluster suites pass. End-to-end GPU-pod scheduling verified via the documented runbook in docker/k3s-cuda/README.md (no Linux GPU host in this environment). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…g (client#835) Chart templates (gpu-device-plugin runtimeClassName) and values changed, so the chart-version gate requires a version bump; and the installer libs changed, so the signed-bootstrap manifest must be regenerated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aptracebloc
commented
Aug 26, 2026
bugbot run |
…node-image # Conflicts: # scripts/manifest.sha256
LukasWodka
left a comment
There was a problem hiding this comment.
Reviewed c0dd2050. The substance is good — I checked the two parts that carry the risk, and both are right. One thing to fix that isn't in the code.
The fallback is loud, which is the whole point. The bug being fixed is "installer printed your environment will have GPU access while the GPU was unreachable", so a fix whose own failure path went quiet would reproduce it one layer down. It doesn't:
warn "Couldn't generate the NVIDIA CDI spec on node '${node}' — pods on it won't be able to use the GPU."
...
K3D_GPU_FLAGS=()
warn "No cluster node could generate a usable NVIDIA CDI spec — running CPU mode so GPU jobs aren't stranded Pending."
hint "Check the NVIDIA driver + 'docker run --rm --gpus all …' works on this host, then re-run."Per-node warning, an all-nodes warning, and a hint carrying a command the operator can actually run. And it checks test -s /etc/cdi/nvidia.yaml rather than trusting nvidia-ctk's exit code — a tool that exits 0 having written nothing is exactly how this class comes back.
And the two halves compose, which is the part worth checking rather than assuming._generate_node_cdi_specs clears K3D_GPU_FLAGS=() on total failure; _gpu_wired requires that array to be non-empty; summary.sh now keys the "NVIDIA GPU" claim on _gpu_wired rather than on GPU_VENDOR. So a failed CDI generation propagates all the way to the summary telling the truth. That chain is the actual fix for #835 — the node image is just what makes GPU possible.
_gpu_wired is careful in a way that's easy to get wrong too: [[ "${K3D_GPU_FLAGS+set}" == set ]] before indexing, so it doesn't trip set -u on an unset array.
On hardware verification — you got there before I did, and I want to reinforce it rather than ask it again. The body says plainly that no Linux GPU host was available, adds the runbook to docker/k3s-cuda/README.md, and says a maintainer with hardware should run it before GPU is relied on for a customer.
That's the right disclosure, and it matters more here than usual: this bug existed because CI has no GPU, and the fix is verified by the same CI that has no GPU. Unit tests can prove the installer chooses the CUDA image and writes a CDI spec; nothing here can prove a pod gets a GPU. So the runbook isn't a nice-to-have, it's the only thing that closes the loop — worth making sure it actually gets run rather than living in a README, since that handoff is the step that tends to evaporate after merge.
One thing to fix: the PR title.
Closes tracebloc/client#835
That's a link, not a description. develop squash-merges, so this becomes the commit subject on the branch and the line this change is known by in the log and on the board — and it tells a reader nothing about what shipped. Someone bisecting or scanning the changelog in three months gets a bare issue reference.
The org convention is type(scope): summary. Something like:
fix(installer): wire the GPU-capable k3s-cuda node image on Linux (Closes tracebloc/client#835)
To be clear about what's not wrong: the closing link works — it's in the body in the full owner/repo#n form, so set-status / closing-ref is satisfied and the issue will close on merge. This is purely about the title being readable.
One check still pending on this head, so nothing was approvable this pass regardless. Retitle and I'll approve once it lands.
Uh oh!
There was an error while loading. Please reload this page.
aptracebloc
commented
Aug 26, 2026
bugbot run |
…nt#835) Bugbot High: _generate_node_cdi_specs cleared K3D_GPU_FLAGS (downgrading to CPU) whenever `nvidia-ctk cdi generate` failed — even on a reused cluster whose /etc/cdi/nvidia.yaml already existed from a prior install, or when `docker ps` returned an empty list. Make the spec's PRESENCE the authority: regenerate best-effort, then count a node on `test -s /etc/cdi/nvidia.yaml` (fresh or pre-existing). And treat a node-listing failure as "cannot tell" (leave the request as-is), not "no GPU". +2 cluster.bats cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
aptracebloc
commented
Aug 26, 2026
Thanks for the careful read, @LukasWodka — both points addressed:
And +1 on the hardware-handoff point — the runbook only closes the loop if someone with a GPU box actually runs it. I'll make sure it's flagged for that rather than left to evaporate in the README. |
aptracebloc
commented
Aug 26, 2026
bugbot run |
LukasWodka
left a comment
There was a problem hiding this comment.
24620032 — title fixed, and the CDI change is a real improvement on something I'd signed off.
On the title:fix(installer): wire the GPU-capable k3s-cuda node image on Linux (Closes tracebloc/client#835) reads as a changelog line now. That was my only finding and it's closed.
On the CDI gating — I praised the old version and I was half-right, which is worth saying plainly. I said it "checks test -s rather than trusting nvidia-ctk's exit code". It checked presence in addition to the exit code:
if generate … &&test -s /etc/cdi/nvidia.yaml;thenThe && short-circuits, so a non-zero generate meant the presence check never ran and the node was written off — even with a perfectly good spec already on it from a prior install. Bugbot caught that; I didn't, and it's the more consequential half. Presence-as-authority is now unconditional:
generate … ||true# then: presence decidesWhich makes the reused-cluster case behave correctly — a transient regeneration hiccup no longer tears down a working GPU cluster.
The listed_ok distinction is the right call too, and correctly made loud. "Couldn't enumerate the nodes" is genuinely cannot tell, not no GPU, so guessing CPU would downgrade a cluster that may already have a valid spec. The reason it doesn't reintroduce #835 is that it isn't silent:
Couldn't list cluster nodes to set up the GPU CDI spec — leaving the GPU request as-is; if GPU pods stay Pending, re-run.
#835 was a silent false claim. A warned-about uncertainty with the symptom named and a remedy attached is a different thing, and the honest one given the alternative breaks working clusters on a docker hiccup.
Both tests assert the specific outcome rather than absence of a crash:
generate fails but a prior spec exists -> stays wired— stubs regeneration to fail andtest -sto succeed, then assertsK3D_GPU_FLAGSis not downgraded. Revert to the&&form and it reddens.node listing fails -> leaves the GPU request as-is— asserts the warn text and that the flags survive.
Nothing outstanding from me. One check still running on this head; I'll approve once it lands.
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 2462003. Configure here.
LukasWodka
left a comment
There was a problem hiding this comment.
CI went red on 24620032 — the same head I reviewed, no new push — so this is that commit's own suite. Handing you the first failure since the run is still mid-matrix and the log isn't retrievable through the normal route yet.
check-facts.bats is the failing file, 8+ tests. The one that matters is the baseline:
not ok 168 check-facts --check: all consumers match the spec -> passes (#435)
# (in test file scripts/tests/check-facts.bats, line 119)
# `[ "$status" -eq 0 ] || return 1' failed
That test asserts check-facts.sh --check exits 0 on a tree where everything already agrees. It's now non-zero, so the checker is reporting drift on a fixture that should be clean — which points at the TB_CUDA_BASE_TAG pin or the new GPU --image wiring enforcement not agreeing with what the harness sets up.
--write is unhappy in the same direction:
not ok 170 check-facts --write: a K8S_VERSION bump stamps BOTH bash and PowerShell
# `_facts --write' failed
# ↻ common.sh:K8S_VERSION: v1.36.3-k3s1 → v1.31.0-k3s1
And your own new test is in the failing set:
not ok 181 check-facts --check: a missing create-time --image pin fails with a
WIRING message, not the --write hint (#547 / Bugbot)
So the new enforcement isn't landing the way the fixture expects — worth starting there, since a new check that can't pass its own case usually explains the baseline failures too.
Unit tests failed alongside bats; both run the same suite, so I'd expect one cause.
This doesn't change anything I said about the CDI logic. Presence-as-authority and the listed_ok distinction are in cluster.sh and still read correctly to me; cluster.bats isn't in the failing set. The breakage is in the facts/spec plumbing — check-facts.sh, facts.env and their consumers — which is a separate limb of the same PR.
Three Prereqs legs are still running, so there may be more once they land.
…le (client#835) Bugbot High: _create_new_cluster handed the derived k3s-cuda --image straight to k3d with no pullability check, so a blocked/unpublished ghcr.io tag or a private TRACEBLOC_IMAGE_REGISTRY without a docker login would HARD-FAIL cluster create on a host that could still run CPU-only. Pre-pull with the host daemon first (bounded + spinner); on failure drop the GPU request and continue CPU-only with an actionable hint. k3d reuses the cached image, so it is not wasted work. Mirrors the Windows twin's Confirm-GpuImagePullable. TB_SKIP_GPU_IMAGE_PREPULL bypasses it. +2 cluster.bats cases (pull-fail fallback, and the pre-pull itself). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aptracebloc
commented
Aug 26, 2026
bugbot run |
saadqbal
left a comment
There was a problem hiding this comment.
Being explicit about scope: I read the diagnosis, cluster.sh's image selection and the GPU-capability guard closely, and skimmed the chart/values changes. I have not read all 647 lines, and I'd rather say that than imply I had.
The failure mode is the one worth having found: the installer detected the GPU, set the host runtime, deployed the device plugin, and printed "your environment will have GPU access" — while the k3d node ran stock Alpine-based rancher/k3s with no in-node NVIDIA toolkit, so containerd could never hand a GPU to a pod and the plugin sat there reporting "No devices found". A claim made with none of the capability behind it, and the Windows path had been wiring k3s-cuda correctly all along. That asymmetry is the part I'd highlight: the twin gap means the Linux path was the only one lying.
One thing I went looking for and want to record as cleared, since it's the shape that usually bites: _is_gpu_node_image ends with [[ "$image" == "$(_gpu_node_image)" ]], and an empty $image compared against an empty computed ref would have read as GPU-capable — a fail-open on an unreadable image, which the comment above it explicitly promises not to do. It doesn't happen: _gpu_node_image always prints something, because even with K8S_VERSION and TB_CUDA_BASE_TAG unset it still emits ghcr.io/tracebloc/k3s-cuda:-cuda-. So the empty case is correctly false. Worth a line in that comment saying the computed ref is never empty, since that's what makes the promise true.
_check_existing_cluster_gpu dropping the GPU request on a reused CPU-only node is the right call and the reason is the important half — node image capability is fixed at create time, so the alternative is jobs stranded Pending forever with no signal. Same principle as the rest of this PR: don't claim a capability you can't deliver.
Not approving: the head moved to 36ca7b44 and the whole matrix is re-running — sixteen checks pending, nothing green yet. The earlier bats and bugbot / review reds were the previous head and the Bugbot thread has been resolved since. Ping me when it settles.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
LukasWodka
left a comment
There was a problem hiding this comment.
36ca7b44 — the pre-pull is the right answer to that finding, and I checked the one thing that could have made it silently useless.
The regression it prevents is real and easy to miss. Before this PR, Linux always handed k3d a stock rancher/k3s image, which is always pullable. Switching to k3s-cuda introduces a class of host — ghcr blocked, tag not yet published for the pin, private TRACEBLOC_IMAGE_REGISTRY without a login — where k3d cluster createhard-fails. That would turn a machine that could have run CPU-only into a failed install. Catching that before shipping is worth more than the GPU feature itself.
The fallback fires on both failure modes, which I verified rather than assumed. It hinges on spin's contract:
wait"$pid"local rc=$?
…
return$rcSo spin propagates the child's exit code and returns 124 on the deadline path. ! spin "$!" … is therefore true for a failed pull (non-zero) and a timeout — if it only reported timeouts, a fast docker pull failure would sail through and leave the GPU wired into a create that then hard-fails. It doesn't.
And the fallback test asserts the absence, not just the presence:
[ "$status"-eq 0 ] # install still succeeds
[[ "$output"==*"CPU-only"* ]] # announced
[[ "$output"==*"--image rancher/k3s:v1.36.3-k3s1"* ]]
[[ "$output"!=*"--image ghcr.io/tracebloc/k3s-cuda"* ]]
[[ "$output"!=*"--gpus=all"* ]] # ← the one that mattersFalling back on the image alone would leave --gpus=all on the create and reintroduce the hard-fail from the other side. Asserting both are gone is what makes the fallback complete rather than half-done.
The hint naming TRACEBLOC_IMAGE_REGISTRY plus the credential vars is the right level of detail too — it tells an operator behind a private mirror exactly which knob turns GPU back on.
Everything is pending on this head, so no verdict yet. Worth watching whether the check-facts.bats failures from 24620032 clear — that fix isn't in this commit, so I'd expect them to still be there when the run lands.
The check-facts.bats fixture builds a synthetic consumer tree; it didn't carry the additions this PR made to check-facts.sh, so its baseline "everything agrees" case went red (and the wiring-message test pinned the old string). Seed the fixture common.sh with TB_CUDA_BASE_TAG, add the GPU --image pin literal to the fixture cluster.sh + install-k8s.ps1 (the new #835 wiring guards), and update the missing-pin test to assert the generalized ${K8S_VERSION}/$K8S_VERSION hint (it now covers both the k3s pin and the GPU node image). Whole check-facts.bats green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aptracebloc
commented
Aug 26, 2026
Good catch, and exactly right — thanks for the triage.
|
aptracebloc
commented
Aug 26, 2026
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
2 issues from previous reviews remain unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit de93f67. Configure here.
LukasWodka
left a comment
There was a problem hiding this comment.
de93f67c — the check-facts.bats fix is right, and Bugbot's two new findings on the pre-pull block both hold. One of them lands on something I explicitly praised, so I'll take that first.
Bugbot's HIGH is correct, and the hint I called "the right level of detail" is the part that's wrong.
Windows logs in before pulling:
2203: function Connect-GpuRegistry {
2199: # docker login to the GPU image's registry with the supplied creds. Called BEFORE both the GPU…
The Linux pre-pull doesn't:
1422: ( docker pull "$_prepull_image">>"${LOG_FILE:-/dev/null}"2>&1 ) &No docker login, and nothing on this path consumes TRACEBLOC_REGISTRY_USERNAME/PASSWORD. So on a private registry the pull fails, the CPU fallback fires, and line 1426 tells the operator:
…or set
TRACEBLOC_IMAGE_REGISTRY(withTRACEBLOC_REGISTRY_USERNAME/PASSWORDfor a private mirror)…
An operator who has already set all three gets told to set them. That's the installer stating something untrue to a user, which is the exact class #835 exists to remove — reintroduced inside the fallback added to prevent it. Last pass I singled that hint out as telling an operator "exactly which knob turns GPU back on"; it does the opposite when the knob is already turned.
Worth noting the comment at line 1411 already names the case — "a private TRACEBLOC_IMAGE_REGISTRY with no docker login" — so the scenario was in view when the block was written; only the login wasn't wired.
The MEDIUM holds too, and it's the same twin gap. Windows doesn't stop at pullable:
2219: function Confirm-GpuImagePullable {
2229: if (Test-GpuImageRunsK3s) { Log "GPU node image pulled + verified OK"; return $true }
2265: function Test-GpuImageRunsK3s {
Linux pre-pulls and stops. A mis-tagged mirror copy — the thing a private TRACEBLOC_K3S_CUDA_IMAGE makes possible — passes the gate, and then k3d cluster create hard-fails, which is precisely the outcome the pre-pull block was added to prevent. The gate lets through the case it was built for.
Both are the pattern this PR is about: the Windows installer already does it, Linux doesn't. Confirm-GpuImagePullable is a good shape to mirror — login, pull, verify-it-runs, and only then call it wired.
On check-facts.bats — the fixture seeding is right, one assertion got weaker than it needed to.
Teaching the fixtures about TB_CUDA_BASE_TAG and the GPU image derivation is correct: new facts and a new consumer genuinely need rows, or --check reports drift on a fixture that lacks them. Not papering over.
But this one lost a property:
- grep -qF 'rancher/k3s:${K8S_VERSION}'- grep -qF 'rancher/k3s:$K8S_VERSION'+ grep -qF '${K8S_VERSION}'+ grep -qF '$K8S_VERSION'Honest, because the message genuinely changed shape — it's image-agnostic now and points at "the EXACT literal each ✖ line above shows". But the test cites #565, whose finding was which shell gets which form, and the new assertion only proves both tokens appear somewhere. Swap the attribution —
bash cluster.sh uses
$K8S_VERSION; PowerShell install-k8s.ps1 uses${K8S_VERSION}
— and it still passes, while being exactly the #565 bug. Pin the pairing instead:
grep -qF "bash cluster.sh uses '\${K8S_VERSION}'"
grep -qF "PowerShell install-k8s.ps1 uses"Non-blocking, but it's a regression guard that can no longer catch its own regression.
Four checks pending and two Bugbot threads open, so nothing was approvable this pass regardless.
saadqbal
left a comment
There was a problem hiding this comment.
de93f67c cleared the bats red and 36ca7b44 adds the pre-pull with a CPU fallback, which is the right instinct. Both of Bugbot's findings are real, and I verified the framing they rest on rather than relaying it: Connect-GpuRegistry and Test-GpuImageRunsK3s both exist in scripts/install-k8s.ps1 on develop, and scripts/lib/cluster.sh contains zerodocker login occurrences.
Worth naming the pattern, because it's this PR's own. The bug you're fixing is a twin gap — Windows wires k3s-cuda, Linux never did. The pre-pull that fixes it reproduces the same shape twice: Windows authenticates to the registry before pulling and Linux doesn't; Windows verifies the pulled image actually runs k3s and Linux doesn't. Same asymmetry, one layer down. That's not a criticism of the fix so much as a signal that the Windows installer is the de-facto reference implementation here, and porting a behaviour means porting its guards too — not just its happy path.
The High is the one I'd fix first, for the reason Bugbot gives and one it doesn't spell out: it makes an existing message lie. On a private TRACEBLOC_IMAGE_REGISTRY the pull fails, the install falls back to CPU, and the hint tells the operator to set credentials they have already set. And the obvious instinct — wire imagePullSecret — cannot help, because the node image is pulled by the host daemon, not by kubelet, so nothing in the chart is in that path. An operator following the hint will burn real time before that becomes clear.
The Medium is the narrower one but it defeats the block's stated purpose: a mis-tagged TRACEBLOC_K3S_CUDA_IMAGE or a bad mirror copy passes a pull-succeeded gate and then k3d cluster create hard-fails, which is precisely the outcome the CPU fallback was added to avoid. "It pulled" is not "it runs k3s".
Held on those two threads and the bugbot / review red that reflects them. Nothing further from me.
…s (client#835) Two follow-on Bugbot findings on the new pre-pull: - High: it never authenticated the host daemon, so credentials the operator set (TRACEBLOC_REGISTRY_USERNAME/PASSWORD) went unused on a private registry and the hint telling them to set those was false. Now `docker login` the image's registry host (via --password-stdin) before the pull — the node image is host-pulled, not kubelet-pulled, so a chart imagePullSecret can't help. New _registry_host_for mirrors the Windows Get-RegistryHost (docker.io for a bare owner/name ref). - Medium: a pulled-but-broken image (mis-tagged override / mirror copy) passed the gate then hard-failed k3d create. Now verify it runs k3s (`docker run --gpus all <img> --version`, capture-then-match) and fall back to CPU otherwise. Mirrors Connect-GpuRegistry + Test-GpuImageRunsK3s. +2 cluster.bats cases; setup mock now answers the verify. cluster.bats 162 green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aptracebloc
commented
Aug 26, 2026
bugbot run |
LukasWodka
left a comment
There was a problem hiding this comment.
69bccfd8 — fixed, and I re-ran the exact two mutations that caught nothing before. Both now redden, each on the test that names the branch:
drop the found_req early-return → not ok … release does NOT request a GPU -> no node probe
drop the alloc early-return → not ok … release requests GPU, node advertises one -> no image inspect
Previously both of those mutations left the whole file green.
You fixed the right thing — the assertion, not the mock. The problem was never that the tests lacked coverage of the code; it was that [ -z "$output" ] couldn't observe either branch, because the kubectl call captures into $alloc and the fall-through log doesn't reach $output either. Switching to record / mock_calls makes the assertion about whether the call happened, which is the actual property. And asserting the positive half too —
[[ "$output"==*"kubectl"* ]] # the alloc probe DID run
[[ "$output"!=*"docker inspect"* ]] # returned before the image inspect— is what stops the test passing for the wrong reason if the function starts returning early for some other cause. A pure absence assertion would have been satisfied by a function that does nothing at all.
Renaming both tests to describe the observable (-> no node probe, -> no image inspect) rather than -> silent is worth as much as the assertion change. "Silent" was what made the old version look adequate; a name that states the mechanism is one a future reader can check against the body.
Also confirmed the conflict is resolved — MERGEABLE again after the develop merge picked up #847 and #848. 45 checks are running; nothing outstanding from me once they land, and the production code was already verified two passes ago (the GPU_REQUESTS regex across every realistic value shape, the node-image split on the remedy, and in-memory capture instead of mktemp).
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
LukasWodka
commented
Aug 26, 2026
The Chart.yaml thing I flagged two passes ago has landed, and unlike The cause is exactly the merge you just did: So you're carrying #847's version rather than your own. This PR changes published files under Worth being precise about which of the two reds matters, since I got this wrong earlier today in the other direction and told Lukas the Bugbot check was gating things when it isn't. Measured on Everything else is settled from my side — the conflict is resolved, the branch-discriminating test fix is verified (both previously-inert mutations now redden the right test), and the production code was verified two passes back. This is the last mechanical step. |
…lient#835) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…(client#835) Bugbot/#554 house rule: helm list must carry --deployed --failed --pending --uninstalling so a wedged release (which may still request a GPU) is not invisible to _check_healthy_cluster_gpu_consistent. Matches detect_installed_client. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aptracebloc
commented
Aug 26, 2026
bugbot run |
Uh oh!
There was an error while loading. Please reload this page.
…create (client#835) Bugbot: after the backend#2033 merge AMD requests amd.com/gpu on a stock rancher/k3s node (AMD does not use the k3s-cuda image), so _check_healthy_cluster_gpu_consistent — which probes nvidia.com/gpu and treats stock k3s as CPU-only — warned every healthy AMD re-run to recreate a WORKING cluster. Match only an nvidia.com/gpu request; amd and empty no longer trip it. +1 cluster.bats case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aptracebloc
commented
Aug 26, 2026
bugbot run |
LukasWodka
commented
Aug 26, 2026
The AMD false-positive is a genuine cross-PR interaction, and catching it before it shipped is the good outcome.#855 taught the installer to request Checked against every shape that reaches it: And the status enumeration is now consistent with the rest of the codebase rather than a third variant — I checked, not just took the comment's word: That matters more than it reads: a release wedged in Four checks still running, so no verdict yet. Nothing outstanding from me — the guard's branch-discriminating tests are verified, the production logic is verified, and these two narrow it correctly. |
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 460b017. Configure here.
LukasWodka
commented
Aug 26, 2026
Bugbot passed, and with it as the only context the aggregate rollup computes That's why I checked whether this was org-wide and it isn't — other open PRs are carrying 9–19 contexts and Actions is servicing other repos normally. So this looks like a dropped push event on this commit specifically. Re-firing is the fix. Toggling draft ( Flagging it rather than silently waiting, because "green rollup + zero runs" is indistinguishable from "all green" in every summary view — the board, the PR header, and my own scan's |
LukasWodka
commented
Aug 26, 2026
Correction to my previous comment — I was wrong, and the advice in it would have made things worse. I said this head had zero workflow runs and suggested re-firing them via a draft toggle. Both parts are wrong. The runs exist and have existed since before I looked: My error was mechanical: I queried The real situation is an Actions backlog, not a dropped event. Sampling the org right now: All on What stands from the original comment: the rollup genuinely does read Nothing to do but wait for the queue to drain. Sorry for the noise. |
LukasWodka
commented
Aug 27, 2026
Overnight settles this: these runs are wedged, not queued behind capacity. I've gone back and forth on this PR twice, so here is the measurement rather than another opinion. Your head's runs, still Meanwhile the same repo ran and completed fresh work this morning: So Actions is healthy for new runs while a cohort created in a ~15-minute window yesterday afternoon never started. The visible consequence is the part worth knowing: with only the Correcting myself. Yesterday afternoon I told you there were zero runs and to re-fire; that was my error (abbreviated SHA to Nothing here is about your diff. Everything I raised on the code is settled: on |
…9.74) Only manifest.sha256 conflicted (regenerated); develop #859 also reached chart 1.9.73, so bump to 1.9.74. GPU handling intact. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
LukasWodka
left a comment
There was a problem hiding this comment.
Re-checked after f685009. The new head is a develop merge (bringing in #859's secrets resolution plus the 1.9.74 chart bump), not a change to this PR's own work — so I looked specifically at whether the merge disturbs anything I'd flagged, rather than re-reading the guard.
It doesn't:
scripts/lib/cluster.shis untouched by the merge —git diff 460b0179 f685009 -- scripts/lib/cluster.shis empty. The NVIDIA-narrowed guard and the healthy-fast-path work are byte-identical to what I verified.The
helm listenumeration still agrees across all three call sites, which was the finding worth re-testing since #859 added 55 lines toinstall-client-helm.sh. All three are stillhelm list -A --deployed --failed --pending --uninstalling:scripts/lib/cluster.sh:1263 scripts/lib/install-client-helm.sh:774 (was :735 before the merge) scripts/lib/install-client-helm.sh:1162 (was :1113)Only the line numbers moved. Worth noting for anyone reading the old thread against the current file.
Holding only on CI — the fresh runs are still going on this head. Nothing outstanding from me; I'll approve when they land green.
Separately, and not a request: this PR was stuck for ~15h behind five Actions runs orphaned in a 2026-08-26 15:00–15:32 UTC window that never started (the whole org caught 55 of them; cli#588 and client#858 are still in it). Your push cleared it by firing fresh runs — contexts went 1 → 49. If either of those two stalls for you, a push or draft-toggle is the way out.
…e-run (client#835) Bugbot: after a first-install CDI-gen or image-pull CPU fallback, the remedy said "ix the host and re-run"\ — but the completed CPU client assesses healthy, so the next run fast-paths (assess.sh) and never re-reaches _generate_node_cdi_specs or a Helm rewrite. GPU wiring is fixed at create time, so the honest remedy is to recreate. Both fallbacks now print _recreate_cluster_hint after the host-fix check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aptracebloc
commented
Aug 27, 2026
bugbot run |
LukasWodka
left a comment
There was a problem hiding this comment.
Re-reviewed da8edb2c. The CPU-fallback remedy fix is right, and it's the good kind of catch — the fast path this PR introduces is what turned "then re-run" into an instruction that silently does nothing, so fixing the advice in the same PR that invalidated it is exactly the right scope. Both call sites in _generate_node_cdi_specs and _create_new_cluster now point at _recreate_cluster_hint, which leads with tracebloc delete --keep-data before the k3d delete — the ordering that keeps the secure environment from being stranded.
I swept for the rest of the class rather than assuming two sites was all of it. Most remaining "then re-run" advice is fine and shouldn't change — assess.sh:404/419 (Docker not running), common.sh:134 (truncated download), cluster.sh:88/149/343/841 (permissions, docker probe) are all transient host conditions a re-run genuinely re-evaluates. cluster.sh:1347 is fine too: CDI specs are generated against a running cluster, so that one really is retryable without a recreate.
One site I think is still in the class, posted as a question because I can't run the installer to confirm the values state — scripts/lib/setup-linux.sh:933:
hint "To enable GPU, have an admin install and configure nvidia-container-toolkit on this host, then re-run."
The Tier-0 flow this serves is a non-admin on an NVIDIA host: toolkit missing → warn → cluster created CPU-only. The natural next step is the admin installs the toolkit and the user re-runs — and at that point _tier0_gpu_flags correctly sets K3D_GPU_FLAGS=("--gpus=all"), but the cluster already exists, so those flags never apply.
The question is whether _check_healthy_cluster_gpu_consistent catches it, and I don't think it does on this path. It gates on found_req:
if grep -Eq '^[[:space:]]*GPU_REQUESTS:[[:space:]]*"?nvidia\.com/gpu'<<<"$vals";then found_req=1;break;fi
...
(( found_req ))||return 0 # no NVIDIA GPU request → not this guard's concernfound_req comes from the installed release's values. On a run that went CPU-only because the toolkit was absent, the release shouldn't be requesting nvidia.com/gpu at all — so found_req stays 0, the guard returns early, and the user gets neither GPU nor the recreate hint. Same no-op as the two you just fixed, and arguably the likeliest to be hit, since "admin installs it later" is the designed path for Tier 0.
If that reading is wrong — if a CPU-only install still writes an nvidia GPU request into the values, or something else re-derives it on the second run — then the guard fires and the advice is fine; say so and I'll drop it. Non-blocking either way: the two sites you fixed were the ones this PR created, and this one predates it.
Holding on CI only — 36 passing, 9 still running (the distro prereq matrix and bats), nothing failing. No other concerns.
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 da8edb2. Configure here.
LukasWodka
left a comment
There was a problem hiding this comment.
Approving da8edb2c. Clean gate: 45 passing, 4 path-skipped, nothing pending or failing, no open threads, mergeable=MERGEABLE.
To be explicit, because I left a question open and don't want it read as a hold: the setup-linux.sh:933 point is not a condition of this approval and I'm not waiting on an answer to it. It predates this PR, the two sites that were genuinely this PR's to fix are fixed, and if your read differs from mine that's a separate follow-up rather than anything to re-open here. Answer it whenever, or not at all.
What I checked across the three heads this PR moved through:
The GPU guard work — cluster.sh untouched by the develop merge in f685009, so the NVIDIA-narrowed guard and the healthy fast path are the code I verified earlier. The helm list -A --deployed --failed --pending --uninstalling enumeration still agrees across all three call sites after #859 shifted them (cluster.sh:1263, install-client-helm.sh:774, :1162) — that agreement was my original finding, so it was the thing worth re-testing rather than assuming.
The CPU-fallback remedy fix in da8edb2c is the part I'd single out. The fast path this PR introduces is precisely what turned "then re-run" into an instruction that silently does nothing, and catching that inside the same PR that created it is the right instinct — a stale remedy is worse than no remedy, because the user follows it and concludes the product is broken. Routing both sites through _recreate_cluster_hint also inherits the tracebloc delete --keep-data-before-k3d delete ordering, so nobody strands their secure environment following the advice.
I swept the rest of the class rather than trusting two sites was all of it. The remaining "then re-run" hints are correct and shouldn't change — assess.sh:404/419, common.sh:134, cluster.sh:88/149/343/841 are transient host conditions a re-run genuinely re-evaluates, and cluster.sh:1347 is fine because CDI specs regenerate against a running cluster. The comments at :900 and :993 show the same reasoning was already applied to storage drift and the CA remedy, which is why I only ended up with one open question.
Value: a self-serve Linux install with a GPU actually gets a GPU-capable node — and when it can't, the operator is told something that works instead of an instruction that quietly no-ops.
Uh oh!
There was an error while loading. Please reload this page.
…ories (backend#2674) (#870) * test(assess): guard every early-exit path reaches the drift/GPU advisories (backend#2674) assess short-circuits an already-set-up machine before the normal flow reaches _handle_existing_cluster, where the k3s-drift (#547/#565) and GPU-consistency (client#835/#852) advisories run. Every early-exit terminal must run both itself — a rule we kept re-learning one instance at a time (the k3s check, the GPU check, then the cli-behind-latest → upgrade_cli_only path in backend#2253, each patched only after the omission was spotted). New suite scripts/tests/assess-early-exit-drift.bats catches the CLASS: - behavioral: drives the healthy hand-off and upgrade_cli_only, asserts BOTH advisories run (and, for the hand-off, before it); - static enumeration that FAILS CLOSED on a new uncovered terminal: pins the exit-bearing functions in assess.sh (_assess_handoff) and install-cli.sh (upgrade_cli_only), pins _assess_handoff to one call site, and asserts each early-exit decision calls both advisories at the source level; - a fixture proving the enumeration actually detects an unguarded early-exit. Mutation-verified: dropping either advisory fails the behavioral + static tests; adding a new exit-bearing function fails the pin. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(assess): derive the advisory set instead of hardcoding it (backend#2674) Bugbot: the ADVISORIES pair was restated, so a new advisory added to the healthy hand-off but not upgrade_cli_only would pass both the static loop and the behavioral stubs — the same class this suite stops, on the advisory axis. Derive the set from the `declare -F X && X` guard idiom in the reference path (the healthy hand-off) and assert upgrade_cli_only runs the SAME set. Divergence in either direction now fails. Mutation-verified: a 3rd advisory on one path only fails the parity test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(assess): harden the scanner against compound exits + both guard idioms (backend#2674) Two Bugbot findings on the scanner itself — the blind spots that matter most for a class-catcher: - Exit scan matched only a leading `exit`, so a terminal written `foo && exit`, `foo; exit`, or `then exit` slipped the pin. Now matches `exit` as a word in any position — but strips single/double-quoted spans first so an embedded `awk '... exit }'` (as in _assess_cluster_servers_running) is not a false positive, and skips/strips comments. - Advisory derivation saw only the one-liner `declare -F X && X`, so a _check_ advisory added via the `if declare -F X; then X; fi` block (the form install_tracebloc_cli already uses) slipped parity. Now keys on `declare -F _check_*` in either idiom, skipping comments so a commented-out guard cannot pad the set. Removed the now-unused _funcs_calling helper. Fixture extended to every exit spelling + an embedded-awk-exit + a commented-out guard. Mutation-verified: a compound-exit terminal fails the pin; a 3rd advisory via if-then on one path only fails parity. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(assess): count hand-off invocations in any spelling, not just line-leading (backend#2674) Bugbot: the _assess_handoff call-site pin used a line-leading grep, so a second hand-off in the file OWN case-arm style (state) … _assess_handoff ;;), or via &&/then, never incremented the count — the exact inline early-exit this suite exists to catch. Add _count_calls, which counts invocations of a symbol as a word in any position (excluding the definition token and comments/quoted spans, same handling as the exit scan), and use it for the pin. Fixture now plants a case-arm one-liner hand-off and asserts the count. Mutation-verified: a syntactically-valid inline second hand-off fails the pin. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

Closes#835
Problem
On a Linux GPU host the installer detected the GPU, set the host Docker runtime, and deployed the NVIDIA device plugin — but
scripts/lib/cluster.shalways created the k3d node from stockrancher/k3s:${K8S_VERSION}, which is Alpine-based and has no in-node NVIDIA container toolkit. So the k3scontainerdinside the node could never hand the GPU to pods: the device plugin reported "No devices found", the node never advertisednvidia.com/gpu, GPU pods couldn't schedule — yet the installer printed "your environment will have GPU access."This is a twin gap: the Windows/WSL2 installer (
install-k8s.ps1) already wires the GPU-capablek3s-cudanode image; the Linux path never did.Fix (installer-side, no node-image rebuild required)
cluster.sh_gpu_node_imagederives theghcr.io/tracebloc/k3s-cuda:<k8s>-cuda-<cuda>pull ref (mirror/override aware), mirroring the PS$K3S_CUDA_IMAGE._create_new_clusterswaps--imageto it when GPU is wired (stockrancher/k3sotherwise — CPU path byte-for-byte unchanged)._generate_node_cdi_specsgenerates the native CDI spec inside each node (nvidia-ctk cdi generate) after create — the image runs the NVIDIA runtime in CDI mode and its boot drop-in only covers WSL2, so native Linux had no spec. Falls back to CPU if no node can produce one._check_existing_cluster_gpudrops the GPU request on a reused CPU-only node (capability is fixed at create time) so jobs aren't stranded Pending.k3s-cudatag.install-client-helm.sh— gate the GPU request and device plugin on GPU actually wired (not bare detection), and setRUNTIME_CLASS_NAME=nvidia.runtimeClassName: nvidiaso it can init NVML on native Linux (newgpu.devicePlugin.nvidia.runtimeClassName).summary.sh— "NVIDIA GPU" mode + GPU test hint only when actually wired, so the claim is no longer false on a CPU fallback.common.sh/check-facts.sh/facts.env—TB_CUDA_BASE_TAGpin +_gpu_wiredpredicate;check-factsenforces the new pin and the GPU--imagewiring across both installers.Scope note
Per the chosen approach, the CDI spec is generated from the installer (
docker exec), so this needs no edit todocker/k3s-cuda/*and no ghcr.io republish — the already-published image is used as-is. (The image is committed to CDI mode; making a native Linux GPU pod run therefore requires a native CDI spec, which the installer now writes.)Verification
Everything unit-testable is green:
bats scripts/tests/cluster.bats(156, +16 GPU),summary.bats(+2), the two newinstall-client-helm.batsGPU-value testshelm unittest tests/gpu_device_plugin_test.yaml(16, +3)scripts/check-facts.sh --check,shellcheck(error severity), the existing GPU/cluster/agreement suitesNo Linux GPU host was available in this environment, so the end-to-end GPU-pod path is covered by the documented runbook added to
docker/k3s-cuda/README.md("Verifying GPU on a Linux host"): confirm the node runs thek3s-cudaimage,/etc/cdi/nvidia.yamlexists in-node, thenvidiaRuntimeClass + device plugin are up, the node advertisesnvidia.com/gpu, and aruntimeClassName: nvidiaGPU pod runsnvidia-smi. A maintainer with hardware should run it before relying on GPU on a customer host.🤖 Generated with Claude Code
Note
Medium Risk
Changes cluster create-time node image selection and GPU chart values—incorrect wiring could still strand GPU jobs or misreport mode, but the PR adds explicit fallbacks, reuse guards, and broad unit coverage.
Overview
Fixes client#835: on Linux, the installer detected NVIDIA GPUs but created k3d nodes from stock
rancher/k3s, so the node never advertisednvidia.com/gpuand jobs stayed Pending while the summary still claimed GPU access.The Linux path now mirrors Windows: when the GPU is wired (
_gpu_wired— NVIDIA +--gpus=all), cluster create uses thetracebloc/k3s-cudaimage (derived fromK8S_VERSIONandTB_CUDA_BASE_TAG), pre-pulls/validates it with CPU fallback on failure, and runsnvidia-ctk cdi generatein-node for native Linux CDI. Reused CPU-only clusters drop the GPU request; healthy fast-path and reuse checks warn when the release requests a GPU the node cannot schedule.Helm/chart: NVIDIA device plugin pod can set
runtimeClassName: nvidiaviagpu.devicePlugin.nvidia.runtimeClassName; values, adopt reconcile, and summary only enable GPU requests,RUNTIME_CLASS_NAME, and the plugin when wired—not on bare detection. Chart 1.9.74.Tooling:
check-facts.shsyncs the CUDA base tag and enforces GPU node-image wiring in both installers;docker/k3s-cuda/README.mddocuments native Linux and an E2E verification runbook.Reviewed by Cursor Bugbot for commit da8edb2. Bugbot is set up for automated code reviews on this repo. Configure here.