Uh oh!
There was an error while loading. Please reload this page.
fix(installer): bound healthy-path GPU-consistency Helm calls on macOS (backend#2685) - #874
Conversation
…S (backend#2685) _check_healthy_cluster_gpu_consistent (the client#835 healthy fast-path GPU guard) wraps `helm list` and `helm get values` in `_bounded`. On a stock Mac `_bounded` runs the BARE command — neither timeout(1) nor gtimeout(1) ships there (both are GNU coreutils) — and Helm has no `--request-timeout` of its own, so a healthy re-run could hang with no output against a wedged kube-apiserver. The function's own header even claimed "Bounded". Gate the Helm calls on a self-bounding `kubectl cluster-info --request-timeout=5s` reachability probe (client-side, coreutils-free) — the same gate diagnose.sh already puts in front of its unbounded Helm calls. An unreachable API just means we can't tell, which is already this guard's silent no-op. The later `kubectl get nodes` already carries --request-timeout=5s; the `docker inspect` keeps `_bounded` (matching detect_gpu's sibling inspect) and is reached only after this probe proves the docker-hosted API answers. Verified on macOS bash 3.2.57 with no timeout/gtimeout on PATH. Adds a test that an unreachable API returns 0 without shelling out to Helm; tightens the AMD/CPU no-node-probe assertions to the node probe specifically (cluster-info now legitimately calls kubectl). Regenerated scripts/manifest.sha256. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal
left a comment
There was a problem hiding this comment.
The defect is a function whose header claimed "Bounded" while _bounded degrades to running the bare command on a stock Mac — neither timeout(1) nor gtimeout(1) is there, and Helm has no --request-timeout of its own. A wrapper named for a property it doesn't provide on the platform most of the team develops on is worse than no wrapper, because the name is what stops anyone checking.
Gating on kubectl cluster-info --request-timeout=5s is the right fix for the right reason: it's self-bounding and coreutils-free, so it doesn't depend on the thing that's missing, and it's the same gate diagnose.sh already puts in front of its own unbounded Helm calls rather than a new invention. And "an unreachable API just means we can't tell, which is already this guard's silent no-op" means the failure direction doesn't change — the guard was always allowed to abstain.
Leaving docker inspect on _bounded and writing down why is the honest choice, and it's the part I'd have pushed on otherwise. The argument holds: it's reached only after the reachability probe and a successful bounded kubectl get nodes, so the docker-hosted k3d API answered sub-second earlier and the daemon is provably live. Recording that in the Bugbot deferral so a future re-review sees the reasoning — rather than leaving a silent claim of coverage — is exactly the distinction that matters when the next person greps for remaining _bounded calls.
Verified two things rather than taking them:
manifest.sha256 is genuinely regenerated — it records 4d199369…9c3b for scripts/lib/cluster.sh and the shipped file hashes to exactly that. That check has now mattered three times in this repo, and it fails at install time rather than in CI when it's wrong.
The new test proves the negative behaviourally: helm() is stubbed to emit a should-not-run marker, the API is made unreachable via the cluster-info stub returning 1, and the assertions are status -eq 0 plus nothing surfaced — so a Helm shell-out would show up rather than being assumed absent.
Reproducing on macOS bash 3.2.57, arm64, with neither timeout nor gtimeout on PATH is the only environment where this bug exists, so testing there rather than on Linux is what makes the verification mean anything.
Tightening the AMD/CPU "no node probe" assertions to the node probe specifically — because cluster-info now legitimately calls kubectl — is the collateral precision most PRs skip, and skipping it would have left two tests quietly weaker than their names.
Green, no threads. 👍
What
_check_healthy_cluster_gpu_consistent(the client#835 healthy fast-path GPU guard, run onassesshand-off and ontracebloc upgrade) wrapshelm listandhelm get valuesin_bounded. On a stock Mac_boundedruns the bare command — neithertimeout(1)norgtimeout(1)ships there (both are GNU coreutils) — and Helm has no--request-timeoutof its own. So a healthy re-run could hang with no further output against a wedged kube-apiserver, even though the function's own header claimed "Bounded".Deferred Cursor Bugbot Medium from the refused client#869 → staging hop (thread), filed as backend#2685.
Fix
Gate the Helm calls on a self-bounding
kubectl cluster-info --request-timeout=5sreachability probe (client-side, coreutils-free) before shelling out to Helm — the exact gatediagnose.shalready puts in front of its own unbounded Helm calls. An unreachable API just means we can't tell, which is already this guard's silent no-op.kubectl get nodesalready carries--request-timeout=5s.docker inspectkeeps_bounded(matching detect_gpu's sibling inspect at ~L912) and is reached only after this probe + a successful boundedkubectl get nodesprove the docker-hosted k3d API answers — the daemon is provably live sub-second earlier. Deliberately not diverging to a bespoke bound there; noted in the Bugbot deferral so a re-review sees the reasoning rather than a silent claim of coverage.Verification
Reproduced and verified on the finding's exact environment — macOS bash 3.2.57, arm64, with neither
timeoutnorgtimeouton PATH:bats scripts/tests/cluster.bats scripts/tests/gen-manifest.bats→ 190 passing, 0 failing, including a new test: an unreachable API returns 0 without shelling out to Helm.cluster-infonow legitimately callskubectl).scripts/check-style.sh→ clean.scripts/gen-manifest.sh --check→ up to date (regeneratedscripts/manifest.sha256;scripts/lib/change is@saadqbalCODEOWNERS-gated as expected).One pre-existing, unrelated macOS failure in
assess.bats(_assess_classify: all signals true) is not touched by this change (different file; fails on pristinedeveloptoo).Closes tracebloc/backend#2685
Note
Low Risk
Installer-only change to a diagnostic fast-path with silent no-op on failure; no auth, data, or chart behavior changes.
Overview
On stock macOS,
_boundeddoes not time out Helm (notimeout/gtimeout, and Helm has no--request-timeout), so_check_healthy_cluster_gpu_consistentcould hang onhelm list/helm get valuesduring a healthy re-run against a wedged API.The guard now runs
kubectl cluster-info --request-timeout=5sfirst; if the API is unreachable it returns silently and never calls Helm, matching the gatediagnose.shalready uses. Comments document the macOS-safe bound behavior.cluster.batsadds coverage for unreachable API (no Helm), and narrows assertions socluster-infois expected whileget nodesstays absent for AMD/CPU-only paths.Reviewed by Cursor Bugbot for commit 5669178. Bugbot is set up for automated code reviews on this repo. Configure here.