Uh oh!
There was an error while loading. Please reload this page.
release-train: develop -> staging - #568
Merged
Merged
Conversation
…550) Saqlain flagged (PR #541, thread on e2e-seal-check.sh:36) that the bring-up prelude is copy-pasted near-verbatim across scripts/tests/e2e-*.sh, and multiple Bugbot rounds have had to edit every copy in lockstep. Extract the two truly-identical, drift-prone blocks into scripts/tests/lib/e2e-common.sh: - e2e_isolate_env <name> — USER + CLUSTER_NAME default + TRACEBLOC_NO_AUTOSTART - e2e_install_prereqs — has docker + umask + install_{kubectl,k3d,helm} e2e-cluster / e2e-proxy / e2e-journey / e2e-auto-upgrade now source the lib and call these; each keeps its own CLUSTER_NAME default (passed as the arg) and its distinct logic. auto-upgrade keeps its extra `has jq` guard before the call. Deliberately NOT unified (would change behavior): the sub-lib `source` set (proxy/journey source 3 libs, not preflight — a pre-existing inconsistency, flagged not fixed), the cleanup/trap bodies (each reaps its own squid/work dirs), and CHART_DIR (only the chart-installing scripts). e2e-seal-check.sh (open on #541) adopts the lib as a fast-follow once both land — kept non-stacked. Added the lib to both shellcheck gates (installer-tests + the required standard-checks Lint). shellcheck --severity=error/warning clean; bash -n ok. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ollow) (#566) Promised on the #541 review threads, now that #550 (the shared lib) and #541 (the seal-check script) are both on develop: - e2e-seal-check.sh now sources scripts/tests/lib/e2e-common.sh and uses e2e_isolate_env / e2e_install_prereqs, matching the other e2e-*.sh (drops the inlined isolation-env + install block). Keeps its own NS=$CLUSTER_NAME and local fail() for its assertions. - Harden the positive control (Saqlain nit): pin networkPolicy.training. enforcementProbeHost to a single $HOST var the install passes AND the positive control targets, so the probe and the control can never drift onto different hosts (was: HOST hardcoded while the probe used the chart default). shellcheck --severity=error/warning clean; helm template renders the probe Job with HOST=1.1.1.1 pinned. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
) * fix(installer): stop the k3s version pin from silently drifting (#547) Root-caused from the Windows stuck-install incident: a client ran k3s v1.35.5 while the pin was v1.29.4-k3s1. Addresses three of the four compounding gaps the audit found (F3 left as a tracked checklist item): - F1: the header docs advertised `default: latest`, inviting users to set K8S_VERSION=latest, which floats to k3d's bundled default k3s. Fix the docs in both installers, and warn loudly at create time when `latest` is used. - F2: the reuse/adopt path never re-checked the running node's k3s version, so a cluster born unpinned (old installer / latest / manual create) persisted forever across later correctly-pinned re-runs — the single best explanation for the observation. Add _check_existing_cluster_k8s_version (bash) and a parity check in New-K3dCluster (PowerShell): warn + recreate remedy on drift. - F4: check-facts.sh only compared the pinned version STRINGS, not the create wiring, so `--image rancher/k3s:` could be dropped while CI stayed green. Add a structural guard asserting the pin is wired in cluster.sh + install-k8s.ps1. With --image now guaranteed on create, k3d's own version no longer floats k3s, so F3 (winget installs unpinned k3d) is de-risked and tracked in #547. Tests (only added): +7 bats for _check_existing_cluster_k8s_version, +5 Pester source guards, check-facts.bats fixture extended with the wiring line. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(installer): regenerate manifest.sha256 for the k3s-pin edits (#547) install-k8s.sh / cluster.sh / install-k8s.ps1 hashes changed; the supply-chain R8 gate (gen-manifest.sh --check) requires the committed manifest to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(installer): run k3s-drift check on the healthy fast-path too (Bugbot #565) Bugbot: the drift check only lived on the full reuse path (_handle_existing_cluster / New-K3dCluster), but both installers short-circuit earlier when a re-run classifies as healthy (bash assess_existing_install, PS completed+healthy fast-path). A healthy-but-drifted cluster — the #547 STEADY STATE — would hit "already set up / nothing to do" and never see the warning, exactly the population the check is meant to help. - bash: assess_existing_install's healthy branch now calls _check_existing_cluster_k8s_version before the handoff (guarded by declare -F). - PS: extracted the inline reuse-path check into Test-K3sVersionDrift and call it from BOTH New-K3dCluster and the completed+healthy fast-path in main. Tests: +2 assess.bats (healthy runs it; --force skips it); Pester #547 block updated to assert the shared function + both call sites. Manifest regenerated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(check-facts): report a missing --image pin as a WIRING gap, not "run --write" (Bugbot #565) Bugbot: the F4 wiring guard incremented the same `drift` counter as version-string mismatches, so a missing create-time --image pin ended with "fact(s) drifted... Run 'check-facts.sh --write'". But --write only restamps version strings and cannot restore create-time wiring — the summary pointed developers at a no-op fix. Track wiring failures in a separate counter and emit a wiring-specific message (this is a WIRING gap; restore the --image rancher/k3s:${K8S_VERSION} flag by hand). +1 check-facts.bats: a missing --image pin fails with the WIRING message and never the --write hint. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(install): idempotent hostpath dataset staging for Windows/macOS/Linux (#547) A client re-running the dataset-copy step hit "already exists" from non-idempotent `mkdir` + `Copy-Item -Recurse`. The repo only documented the Linux `kubectl cp` staging path, with no hostpath/Windows guidance. Add an idempotent hostpath staging section: Windows uses `New-Item -Force` + `robocopy /E` (merges into an existing target, safe to re-run); macOS/Linux use `mkdir -p` + `cp -R`. Notes the plain-mkdir "already exists" error is harmless (data already staged). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(installer): bound the k3s-drift docker inspect probe (Bugbot #565) Bugbot: _check_existing_cluster_k8s_version (bash) and Test-K3sVersionDrift (PS) ran a bare `docker inspect` with no deadline, and both healthy fast-paths now call them — a wedged Docker engine could hang a headless "already healthy" re-run AFTER success was printed, violating the installer's bounded-probe rule. - bash: wrap the inspect in _bounded (timeout/gtimeout; 124 on timeout → the existing `|| return 0` makes it a silent no-op). - PS: run it via Start-Job + Wait-JobWithProgress -TimeoutSec 15 (mirrors Test-ClusterRunning); on timeout, skip the check with a log line. Tests: cluster.bats setup overrides _bounded so the docker shell-function mock is exercised on Linux CI too (timeout can't exec a function); Pester asserts the bounded Start-Job pattern tied to the "Checking k3s version" probe. Manifest regenerated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(check-facts): wiring-gap hint names the correct pin literal per shell (Bugbot #565) Bugbot: the remediation hint told devs to restore `rancher/k3s:${K8S_VERSION}` in both files, but the PowerShell guard matches the fixed string `rancher/k3s:$K8S_VERSION` (no braces) — following the hint in the PS create path would leave CI red even though --image is correctly wired. Reword the hint to name BOTH shell forms (bash cluster.sh uses ${K8S_VERSION}; PowerShell install-k8s.ps1 uses $K8S_VERSION) and point at the exact literal each ✖ line already prints. +2 assertions in check-facts.bats locking both forms. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 3, 2026
ContributorAuthor
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 6cba706. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
Automated promotion by the release train (RFC-0008 D14). Head is the train-managed
release-train/to-stagingbranch (a mirror ofdevelop), so it never collides with a human PR. Merged only when the fr-gate is green.Note
Medium Risk
Changes affect cluster creation and reuse paths for all OS installers; mistakes could block installs or mis-pin k3s, though behavior is mostly additive warnings plus stricter CI gates.
Overview
Hardens k3s version pinning after #547: bash (
cluster.sh,assess.sh) and PowerShell (install-k8s.ps1) pass--image rancher/k3s:<pin>on cluster create, warn loudly onK8S_VERSION=latest, and non-fatally warn when an existing cluster’s running k3s tag doesn’t match the validated pin (reuse path, assess “healthy” handoff, and PS healthy fast-path).check-facts.sh --checknow also asserts create-time wiring (not just version strings) and fails with a hand-fix message instead of suggesting--write.Adds
scripts/tests/lib/e2e-common.shand switches the e2e harness scripts to shared isolation + tool-install prereqs; CI shellcheck and.gitignoretrack the new lib.e2e-seal-check.shpinsenforcementProbeHoston install so the positive control and probe share one host.docs/INSTALL.mddocuments idempotent dataset staging for hostpath laptop installs (Windowsrobocopy/ Unixcp -R).Unit/Pester coverage for drift checks, wiring guard, and manifest hash updates for touched installer scripts.
Reviewed by Cursor Bugbot for commit 6cba706. Bugbot is set up for automated code reviews on this repo. Configure here.