[thxCode]

What type of PR is this?

/kind enhancement
/area testing

What this PR does / why we need it:

Add a disposable macOS arm64 kind recipe that proves a Kubernetes v1.36.5 control plane can reach a listener on the host. The recipe provisions a task-local kubectl, builds or reuses the pinned native node image, and verifies aggregator routing and disabled EndpointSlice mirroring from the running cluster. A positive EndpointSlice control distinguishes disabled mirroring from a broken fixture or failed API read.

The recipe records cluster ownership, uses an isolated kubeconfig, and refuses to adopt or delete pre-existing clusters or fixture namespaces. Add the development guide, module-map routing and generated site navigation with the fixture tests.

Which issue(s) this PR links to:

NONE

Special notes for your reviewer:

This is the local network prerequisite for the staged Go 1.27.3 / Kubernetes 1.36.5 upgrade. It proves host reachability only; authenticated worker aggregation, TLS and EndpointSlice routing changes remain later work.

Validation: 43 fast assertions and 24 live assertions pass on native macOS arm64 with Kubernetes v1.36.5. Regression and mutation checks detect failed slice reads, deletion of a namespace the fixture did not create, an extraArgs entry without a value, and a cleanup scope that arms its trap in the caller. Bash syntax, ShellCheck and documentation lint pass. The final run removed its disposable cluster and state, preserved the node-image/download caches and left the default kubeconfig unchanged.

Full make lint passes with process-scoped GOTOOLCHAIN=go1.26.9, the current module-declared toolchain, using unchanged tool pins. The same gate under host Go 1.27.3 fails because the pinned golangci-lint cannot read its export data. Target-toolchain lint compatibility remains part of the dependency upgrade; this PR does not change the host compiler or linter configuration.

Independent qwen review accepted the final scope with no required findings. Its one test-soundness note, the caller-trap assertion comparing a trap table the cleanup scope could not touch, is addressed: the assertion compares the trap table of the shell that runs the cleanup scope, and a mutant that arms the trap in the caller makes it fail. The review-round findings are triaged and answered in the review threads and the summary comment.

Does this PR introduce a user-facing change?

NONE

[gpustack-code-review]

🔍 OpenCodeReview found 2 issue(s) in this PR.

• ✅ Successfully posted inline: 0 comment(s)
• 📋 Routed to summary by policy: 2 comment(s)

---

📄 testing/infra/local_worker/run.sh (L523-L525)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category bug)

lw_die inside lw_probe_from_node (e.g. the missing bash/timeout guard) exits the whole script without calling lw_stop_listener, so the python listener process is left running (up to its 120s deadline) and LW_LISTENER_FILES linger in the state directory. The listener.py, port and log files are only removed on the explicit failure returns, not on any die between the listener start and lw_stop_listener. Consider arming a trap (e.g. trap 'lw_stop_listener' EXIT around the probe, or converting the die paths in lw_probe_from_node to return 1 returns handled by the caller) so every exit path after the listener starts reaps it.

💡 Suggested Change

Before:

  if ! docker exec "${node}" sh -c 'command -v bash >/dev/null 2>&1 && command -v timeout >/dev/null 2>&1'; then
    lw_die "the node image carries no HTTP client and no bash with timeout for the raw-socket probe"
  fi

After:

  if ! docker exec "${node}" sh -c 'command -v bash >/dev/null 2>&1 && command -v timeout >/dev/null 2>&1'; then
    printf '[local-worker] error: the node image carries no HTTP client and no bash with timeout for the raw-socket probe\n' >&2
    return 1
  fi

---

📄 testing/infra/local_worker/run.sh (L352-L353)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category maintainability)

control is used to capture the exit status of lw_wait_for_slice but is not declared with local, unlike window and observation on the same line. It leaks into the calling scope (here the subshell of lw_with_fixture_cleanup, so it is contained, but the inconsistency invites a leak if the function is ever reused). Declare it alongside the other locals.

💡 Suggested Change

Before:

  local window=60 observation=0
  lw_create_mirror_fixture

After:

  local window=60 observation=0 control=0
  lw_create_mirror_fixture

[thxCode]

Review round 1 summary — all 8 findings from the OpenCodeReview run triaged, plus the test-soundness note already disclosed in the description.

Inline threads (4) — all addressed, threads resolved

1. run.sh listener fallback to 0.0.0.0 (security·medium): the availability half is real and fixed — the listener now answers requests until a two-minute deadline instead of exactly one, so a stray connection cannot consume the probe. The binding half stays: no narrower bind target answers a request to an address the Mac cannot bind, and the token is a random per-run value with no authority. Thread reply has the detail.
2. cmd_cleanup cluster-name mismatch (bug·medium): confirmed real, fixed — cleanup refuses to act when the record names another cluster, keeping the record. New live case proves the refusal and that the cluster survives.
3. default-kubeconfig fingerprint not re-checked at cleanup (bug·medium): confirmed real, fixed — re-verified after kind delete, dying on a change (rather than warning) so the record holding the original fingerprint is preserved as evidence.
4. tampered ownership record stranded by an abort (bug·medium): confirmed real, fixed — the EXIT-trap handler restores the backup before invoking cleanup.

Summary-routed findings (4)

5. window accounting and lw_wait_for_slice read handling (maintainability·low): declined. The observation loop counts polls, not wall-clock, and that bound is load-bearing for the deterministic fast cases (a stubbed clock would spin unbounded under SECONDS); a slow read only lengthens the real window, which strengthens an absence proof. The control loop asserts presence, so a failed read can only postpone the positive signal, never manufacture absence, and a persistent failure already ends in a diagnostic lw_die.
6. pause:3.10 fallback tag (maintainability·low): declined. The fallback pull is documented behavior for a node image that carries no pause image; when parsing yields a <none> tag the resulting invalid reference fails loudly through the pod-readiness wait rather than silently pulling a different image.
7. hard-coded v1.36.5 in live cases (maintainability·low): accepted, fixed — the two version expectations now compare against LW_KUBECTL_VERSION / LW_KUBERNETES_VERSION. The table's expected column is not evaluated, so the checks became explicit run_case calls after the loop.
8. extraArgs entry with a name but no value accepted (bug·low): confirmed real, fixed — the validator rejects an incomplete entry at the next entry, at the end of the block, and at the end of the file, with two new fast cases; both fail against the previous validator.

Disclosed test-soundness note

The caller-trap assertion compared a trap table the cleanup scope could not touch; it now compares the trap table of the shell that runs the scope, and a mutant that arms the trap in the caller makes the case fail. The description's remaining-work paragraph is updated accordingly.

Verification

Fast group 43/0 and the full live group 67/0 (43 fast + 24 live) on native macOS arm64, exit 0, teardown clean; bash -n and pinned ShellCheck green over the three scripts; make lint docs green; make lint green under process-scoped GOTOOLCHAIN=go1.26.9 with an unchanged tree, and the known host-Go-1.27.3 pinned-golangci failure is unchanged from the accepted baseline. The fixes are folded into the single commit, which keeps its original message and sign-off.

[thxCode]

Review round 2 summary — 1 finding from the OpenCodeReview run on 109c8ff, no suppressed comments.

1. /dev/tcp fallback probe has no timeout (run.sh, bug·medium): confirmed real, addressed in c15a4fc — the fallback is wrapped in timeout 15, bounding connect and response the way --max-time 15 bounds the curl path and --timeout=15 the wget path. Verified through the cached node image: the exact snippet returns the token from a host listener, and against an accept-but-silent listener the bounded version exits 124 at exactly 15 seconds while the unbounded form has to be killed from outside. The full suite still passes 67/0 with clean teardown; thread resolved.

All four round-1 threads remain resolved with their replies, and the description still reflects the current state (43 fast + 24 live assertions).

[thxCode]

Review round 3 summary — 5 findings from the OpenCodeReview run on c15a4fc, no suppressed comments. All five are addressed in 193a7e4; every thread carries its reply and is resolved.

1. stale cached release tarball aborts prepare (bug·medium): confirmed real, addressed — the checksum check is now the predicate lw_release_tarball_matches, a cache that no longer matches is logged and ignored, and the build falls through to --type release, which downloads and verifies its own tarball. The unused lw_verify_release_tarball is removed and the docs state the behavior.

2. lw_wait_for_slice swallows read errors (bug·medium): addressed — the control wait reads through lw_read_slice_names, ends at once on a failed read with LW_SLICE_READ_FAILED, and the caller reports the read failure with its own message instead of "produced no EndpointSlice". The ShellCheck unreachability check caught a dead-path bug in the first version of this change before anything was pushed; the success path is verified to fall through.

3. ownership record written with empty identifiers (bug·high): confirmed real, addressed — lw_ownership_write itself refuses an empty container or image identifier before any file is created, so no record can exist that a later cleanup would refuse forever.

4. raw-socket fallback assumes bash and timeout (bug·medium): partly right — the pinned node image demonstrably ships both (verified by running the exact snippet through it), but the assumption is now guarded for an overridden image: a probe of bash and timeout precedes the fallback and names what is missing.

5. CLUSTER_WAS_PREPARED set only after prepare succeeds (test·medium): addressed — the flag goes up before prepare runs, so a partial failure after the record exists still gets the exit-trap cleanup. A recordless cluster is deliberately never deleted (ownership contract), and finding 3's guard keeps the record path from creating that state silently.

Verification

Standalone logic proofs A–F (read-failure fail-fast, success fall-through, stale-cache fall-through, matching-cache build, both empty-identifier refusals), fast group 43/0, full live suite 67/0 with clean teardown, pinned ShellCheck green over the three scripts, make lint docs green, and one full make lint green under process-scoped GOTOOLCHAIN=go1.26.9 with an unchanged tree on this exact source. The host-Go-1.27.3 pinned-golangci failure is unchanged from the accepted baseline (no Go or module source changed in this round). All fixes are folded into the single commit, keeping its original message and sign-off.

[thxCode]

Review round 4 summary — 1 finding from the OpenCodeReview run on 193a7e4, no suppressed comments. Addressed in 5092ff5; the thread carries its reply and is resolved.

1. a zero window reports an absence without a single read (bug·medium): confirmed real and latent — both call sites pass 60/120, but lw_wait_for_no_slice with timeout <= 0 returned LW_SLICE_ABSENT without reading, contradicting its own contract, and lw_wait_for_slice had the same shape. Both loops now perform at least their first read (while (( waited == 0 )) || (( waited < timeout ))), a new fast case fails against the previous loop and passes with the guard, and the control wait is proven to read exactly once on a zero window.

Verification

Fast group 44/0, full live suite 68/0 with clean teardown, pinned ShellCheck green over the three scripts, make lint docs unchanged-green from round 3 (no docs edits this round), one full make lint green under process-scoped GOTOOLCHAIN=go1.26.9 with an unchanged tree on this exact source, generation attempted (known Go 1.27 GODEBUG startup failure, no generated changes), and the host-Go-1.27.3 pinned-golangci result retained from the accepted baseline. The fix is folded into the single commit, keeping its original message and sign-off, so the branch's own lint gate stays green at the pushed head.

[gpustack-code-review]

The listener process and its temp files are only cleaned up through explicit lw_stop_listener calls on the success/failure paths of lw_probe_address. There is no EXIT trap in run.sh that calls lw_stop_listener, so any exit that happens while the listener is live leaves a stray python process running for up to 120 seconds plus files under <state_dir>/network. Concretely reachable: lw_probe_from_node calls lw_die when the node image lacks curl/wget/bash/timeout — that aborts run.sh while the listener is still bound and serving. Suggestion: arm trap lw_stop_listener EXIT for the duration of cmd_network (or in lw_probe_address's caller), matching the discipline used by lw_with_fixture_cleanup.

[thxCode]

Valid finding, and confirmed reachable as described: lw_probe_from_node's availability guard (and any other lw_die) exits run.sh while the listener is live, and no EXIT trap calls lw_stop_listener, so the python process can serve for up to its 120-second deadline and the files under the state directory's network/ subdirectory outlive the run until the next cleanup.

This arrived in the review run on the previous head, after the five feedback rounds authorized for this pull request had closed, so it is not being fixed here: the thread is left open and the finding is reported to the pull request's coordinator in the round summary, with this assessment. The bounded fix shape, for whoever picks it up: arm trap lw_stop_listener EXIT for the duration of cmd_network (mirroring lw_with_fixture_cleanup's discipline), keeping the explicit stops on the normal paths. Impact today is self-limiting: a stray listener exits at its own 120-second deadline, binds an ephemeral port, serves a random per-run token, and its files live under the recipe's own state directory, which cleanup removes.

[thxCode]

Review round 5 summary — this was the fifth and final authorized feedback round. The OpenCodeReview runs on 5092ff5 and 3ba530b produced 4 findings total; all 4 are answered below, 3 with fixes in 6aa14a4 and 1 deferred as out-of-round. No suppressed comments in either run.

1. observation window measured by sleep intervals with no request timeout (performance·medium): bounded half fixed — lw_read_slice_names passes --request-timeout=10s, so a hung API read becomes a LW_SLICE_READ_FAILED both loops already handle, and the window is bounded. The wall-clock deadline half is declined: the interval count is the tested contract (the fast cases stub lw_sleep and assert exact read counts; a SECONDS loop with a stubbed clock spins unboundedly), and a slow-but-answering API only lengthens the real window, which strengthens an absence proof.

2. failed-create rollback deletes a same-name cluster it cannot prove it owns (bug·medium): accepted and fixed after an initial decline was corrected internally before publication. The wrapper's unconditional kind delete cluster is removed — kind's own default already removes the resources it created on failure (--retain is opt-in and unset here), the ownership record does not exist before a successful create, and a name checked free earlier does not attribute a cluster that appeared during the create window. The failure path now preserves any remnant with actionable diagnostics. A non-destructive stub regression ("a failed cluster creation deletes nothing by name") fails against the previous wrapper and passes against the fix; no live race was run.

3. success-path namespace delete waits unbounded and looks redundant (performance·medium): bounded half fixed — the delete now carries --timeout=120s, so a stuck namespace ends the proof instead of hanging it. Dropping the line is declined: the exit trap's delete is best-effort --wait=false and cannot keep the "fixture namespace is gone after prepare" assertion deterministic.

4. listener has no EXIT trap in run.sh (bug·medium, from the run on the previous head): valid and reachable as described, but it arrived after the five authorized rounds closed, so it is not fixed here. The thread stays open with an assessment and a bounded fix shape (trap lw_stop_listener EXIT for the duration of cmd_network); today's impact is self-limiting (120-second listener deadline, ephemeral port, random token, files under the recipe's own state directory).

Verification for this round

Fast group 45/0 (including the new failed-create regression), pinned ShellCheck green over the three scripts, make lint docs green from round 3 (no docs edits since), generation attempted on the final source (known Go 1.27 GODEBUG startup failure, zero generated changes), one full make lint green under process-scoped GOTOOLCHAIN=go1.26.9 with an unchanged tree on this exact source, and the host-Go-1.27.3 pinned-golangci failure retained unchanged from the accepted baseline. The 68/0 live suite evidence is reused for this round because the only live-path change is the failed-create branch, which the live suite does not enter (its creation succeeds); the success path is byte-identical. All work remains folded into the single commit with its original message and sign-off.

[gpustack-code-review]

The network fetches in lw_download_checksum and lw_provision_kubectl run curl -fsSL with no deadline (--max-time or --connect-timeout) and no retry. A stalled or black-holed connection hangs preflight/prepare indefinitely — note the recipe itself acknowledges deadline sensitivity elsewhere (the cached release tarball exists precisely because kind's download can exceed a deadline, and lw_read_slice_names gained --request-timeout=10s for the same reason). A truncated download is caught by the checksum, but a hung one is not. Suggest bounding both fetches, e.g. curl -fsSL --max-time 120 ....

Suggestion:

Suggested change

	lw_download_checksum() {
	curl -fsSL "$1" | awk '{print $1}'
	}
	lw_download_checksum() {
	curl -fsSL --max-time 60 "$1" | awk '{print $1}'
	}

[thxCode]

Valid finding, same hazard class as the request timeout the previous round added to lw_read_slice_names: lw_download_checksum and lw_provision_kubectl fetch over curl -fsSL with no --max-time/--connect-timeout, so a black-holed connection can hang preflight/prepare where the recipe elsewhere is careful about deadlines. A truncated download is caught by the checksum comparison, but a hung one is not caught by anything.

This arrived in the review run on the final head, after the five feedback rounds authorized for this pull request had closed, so it is not being fixed here: the thread stays open and the finding is reported to the pull request's coordinator with this assessment. The bounded fix shape, for whoever picks it up: add --max-time (and optionally --connect-timeout) to both fetches — the checksum fetch is a tiny document, so a short bound like 30–60 seconds fits, while the kubectl binary download justifies the 120 seconds you suggested.