fix(installer): bound k3d create and the helm calls — no indefinite hangs (bash half of #426) - #442
Merged
Merged
Conversation
Contributor
Author
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
LukasWodka
added a commit
that referenced
this pull request
Jul 27, 2026
…bot #442) The create backstop passes the `( k3d … )` wrapper-subshell PID into spin, and the deadline path signalled only that PID — orphaning k3d, which kept creating the cluster after the installer had already exited as failed, racing any retry. The deadline path now TERMs the children while the parent is still alive (afterwards they reparent to init and pkill -P can't see them), then the wrapper, with a KILL sweep after the grace period. Harmless when bash exec-optimizes the wrapper away. Test proves the child dies with the wrapper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
LukasWodka
added a commit
that referenced
this pull request
Jul 27, 2026
… (Bugbot #442 r2) 1. spin's deadline path captured no child PIDs, so once the TERM'd wrapper died, its children reparented to init and the pkill -KILL -P sweep found nothing — a TERM-immune k3d could survive the backstop. Child PIDs are now captured BEFORE any signal and the KILL sweep addresses them directly; test proves a trap-''-TERM child dies. 2. When the create backstop fires, the k3d log is often empty (hung daemon) — the operator saw a bare failure with no timeout hint. The 124 path now names the timeout, points at TB_CREATE_TIMEOUT_MIN, and deletes the partially created cluster (bounded) so a re-run cannot adopt it via the 'already exists' branch — parity with the Windows fix on #439. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
LukasWodka
added a commit
that referenced
this pull request
Jul 27, 2026
Bare pkill (returns 1 with no children), kill on a reaped pid, and wait after a kill (reports the signal) could each abort the deadline path before 'return 124' under the installer's set -e — callers would see 143/1: no timeout copy, no partial-cluster cleanup, no bounded note. Every signal step is now failure-proofed, and a bash -c 'set -euo pipefail' end-to-end test pins the 124 contract for the childless (exec-optimized) case that reproduces the abort. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
LukasWodka
added a commit
that referenced
this pull request
Jul 27, 2026
…#442 r4) A SIGKILLed helm (spin_cmd_bounded deadline) can leave the release wedged as pending-install/pending-upgrade — the next run then fails with Helm's 'another operation is in progress' and no guidance. Both call sites now capture the rc (if-! discarded it) and, on 124, print the exact unwedge commands (uninstall for a half-installed release, rollback for an upgrade) before the error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
1 similar comment
Contributor
Author
|
bugbot run |
LukasWodka
added a commit
that referenced
this pull request
Jul 27, 2026
…ugbot #442 r5) The adopt path tracks release ($_rel) and namespace ($_ns) separately; the reconcile-timeout unwedge hint said 'helm rollback $_ns', which is a non-working command whenever they differ. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
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 424e5ad. Configure here.
…angs (#426, bash half) k3d cluster create --wait had no deadline of its own and its spinner none either: a stalled image pull (rate-limited registry, TLS-intercepting proxy) span the create forever. Both helm invocations (install + reconcile) ran under the deadline-less spin_cmd and could hang the same way against a wedged kube-apiserver. - create: --wait now always pairs with --timeout (default 15m, TB_CREATE_TIMEOUT_MIN override — the same env knob the Windows installer adopted); k3d aborts with a real error that the existing failure path dumps. spin() gained an optional deadline, used as a +5min backstop in case k3d itself wedges past its own timeout. - helm install + reconcile: new spin_cmd_bounded (spin_cmd + hard deadline, rc 124 with an explicit timeout note + log tail), default 10m via TB_HELM_TIMEOUT_MIN. Completes #426 (the Windows half shipped with the #439 stack). Closes #426 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…bot #442) The create backstop passes the `( k3d … )` wrapper-subshell PID into spin, and the deadline path signalled only that PID — orphaning k3d, which kept creating the cluster after the installer had already exited as failed, racing any retry. The deadline path now TERMs the children while the parent is still alive (afterwards they reparent to init and pkill -P can't see them), then the wrapper, with a KILL sweep after the grace period. Harmless when bash exec-optimizes the wrapper away. Test proves the child dies with the wrapper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… (Bugbot #442 r2) 1. spin's deadline path captured no child PIDs, so once the TERM'd wrapper died, its children reparented to init and the pkill -KILL -P sweep found nothing — a TERM-immune k3d could survive the backstop. Child PIDs are now captured BEFORE any signal and the KILL sweep addresses them directly; test proves a trap-''-TERM child dies. 2. When the create backstop fires, the k3d log is often empty (hung daemon) — the operator saw a bare failure with no timeout hint. The 124 path now names the timeout, points at TB_CREATE_TIMEOUT_MIN, and deletes the partially created cluster (bounded) so a re-run cannot adopt it via the 'already exists' branch — parity with the Windows fix on #439. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bare pkill (returns 1 with no children), kill on a reaped pid, and wait after a kill (reports the signal) could each abort the deadline path before 'return 124' under the installer's set -e — callers would see 143/1: no timeout copy, no partial-cluster cleanup, no bounded note. Every signal step is now failure-proofed, and a bash -c 'set -euo pipefail' end-to-end test pins the 124 contract for the childless (exec-optimized) case that reproduces the abort. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…#442 r4) A SIGKILLed helm (spin_cmd_bounded deadline) can leave the release wedged as pending-install/pending-upgrade — the next run then fails with Helm's 'another operation is in progress' and no guidance. Both call sites now capture the rc (if-! discarded it) and, on 124, print the exact unwedge commands (uninstall for a half-installed release, rollback for an upgrade) before the error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Count invocation/hint lines, not comment mentions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ugbot #442 r5) The adopt path tracks release ($_rel) and namespace ($_ns) separately; the reconcile-timeout unwedge hint said 'helm rollback $_ns', which is a non-working command whenever they differ. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
Rebased onto |
LukasWodka
force-pushed
the
fix/426-bash-timeouts
branch
from
July 27, 2026 15:45
424e5ad to
599d5b6
Compare
…sed (Bugbot #442 r6) TB_CREATE_TIMEOUT_MIN / TB_HELM_TIMEOUT_MIN sanitizers accepted 08/09, which bash arithmetic reads as invalid octal — aborting $(( … )) under set -e mid-create (k3d already backgrounded → partial cluster left behind), and 010 silently became 8. One shared, unit-tested helper (tb_minutes_or) now normalizes via 10# at all three call sites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
1 similar comment
Contributor
Author
|
bugbot run |
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 6b2fa42. Configure here.
saadqbal
approved these changes
Jul 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The bash half of #426 (the Windows half shipped in #439): the two places the installer could hang forever with a live spinner now have deadlines.
k3d cluster create --waithas no timeout of its own — a stalled image pull (rate-limited registry, TLS-intercepting proxy) spun "Creating your secure environment…" indefinitely. The code even predicted this hang class in its own comments.spin_cmdand could wedge the same way against a stuck kube-apiserver.Change
--waitnow always pairs with--timeout(default 15m,TB_CREATE_TIMEOUT_MINoverride — the same env knob as the Windows installer), so k3d aborts with a real error that the existing failure path already dumps.spin()gained an optional deadline argument, used as a +5 min backstop in case k3d itself wedges past its own timeout (hung docker daemon).spin_cmd_bounded—spin_cmdplus a hard deadline; returns the command's real exit code, or 124 with an explicit "timed out after Ns" note + log tail. Install + reconcile run under it (default 10m,TB_HELM_TIMEOUT_MIN).spin_cmd_boundedpass-through/failure/timeout, and grep-gates pinning--wait --timeoutpairing + both helm sites bounded.manifest.sha256regenerated.Independent of the #437→#440 stack and of #441 (different manifest lines).
Note for reviewers: 3 unrelated bats tests fail locally on macOS only (
validate_config: valid config passes,un-stamped DEFAULT_REF fails closed,_extract_yaml_value: single-quoted) — they fail identically on a cleandevelopcheckout on the same machine, so they're a local-env artifact (macOS bash), not from this PR. CI (ubuntu) is the arbiter.Type
Bug fix
Test plan
Full bats suite: all tests green except the 3 pre-existing local-macOS failures listed above (verified present on clean develop).
bash -n+shellcheck --severity=errorclean; copy catalog unchanged;gen-manifest.sh --checkclean.🤖 Generated with Claude Code
Note
Medium Risk
Touches core install paths (cluster creation and helm) with aggressive process killing on timeout; mistakes could abort valid slow installs or leave helm releases pending, though defaults and hints mitigate that.
Overview
The bash installer no longer spins forever on k3d cluster create or helm when image pulls or the API wedge.
k3d create now passes
--timeoutwith--wait(default 15m,TB_CREATE_TIMEOUT_MIN). The create spinner uses a +5 minutespin()backstop; on timeout (124) it warns, hints, and runsk3d cluster deleteto drop a partial cluster so the next run does not hit “already exists.”Helm install and adopt-reconcile use new
spin_cmd_bounded(default 10m,TB_HELM_TIMEOUT_MIN). Helm timeouts print helm rollback/uninstall hints for pending-release wedging.Shared helpers:
tb_minutes_or(base-10 minute env parsing),spin()optional deadline with child-process kill semantics safe underset -e, plus bats coverage and updatedmanifest.sha256.Reviewed by Cursor Bugbot for commit 6b2fa42. Bugbot is set up for automated code reviews on this repo. Configure here.