Skip to content

fix(installer): Windows was the one OS that let a sub-floor Docker VM proceed - #520

Merged
LukasWodka merged 2 commits into
developfrom
fix/windows-runtime-mem-floor
Jul 31, 2026
Merged

fix(installer): Windows was the one OS that let a sub-floor Docker VM proceed#520
LukasWodka merged 2 commits into
developfrom
fix/windows-runtime-mem-floor

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

#513 decided that a Docker VM below the client's memory floor must stop the install rather than proceed and OOM-crashloop — "proceeding is worse than the jarring stop the WARN path used to avoid" — and implemented that in bash for every OS (_pf_recheck_runtime_memerrorexit 1).

The Windows installer never got it. Test-PreflightRuntimeMem just called Show-MemoryStatus, which is warn-only, so Windows printed "it will OOM" and then carried on and OOM-crashlooped. The platform this entire memory story (#417 / #418 / #428 / #444 / #516) is about was the one platform still shipping the crash.

The fix

Enforcement now lives in Test-PreflightRuntimeMem, mirroring bash's split: Show-MemoryStatus stays purely presentational (its documented job — and the function #444 and #516 just contended over), while the recheck grades then enforces. It runs as New-K3dCluster's first statement, so exiting leaves no half-built cluster.

The subtlety that makes this safe

Get-PfRuntimeMemGb floors to whole GB, and a guest reports a few hundred MiB below its configured size — so a VM set to exactly the documented 5 GB floor reports ~4.8 GB and floors to 4. A bare -lt 5 would have hard-failed a correctly configured machine: the same trap #513's reviewer caught in bash, but worse here, because flooring to whole GB discards up to a full GB of information.

So the gate compares MiB against floor − grace:

  • Get-PfRuntimeMemMib — the same docker info value at MiB precision.
  • Get-PfVmMemGraceMib (512, PF_VM_MEM_GRACE_MIB) — the same constant and the same comparison bash uses, so both installers put the floor in the same place.
  • The recheck reads the budget once, in MiB, and derives GB from it, so the number printed and the number enforced on cannot disagree. Flooring (not rounding) is kept deliberately: Step-1 floors too, and fix(installer): memory check flip-flops between WSL VM budget and physical RAM; impossible recommendations #417 exists precisely so the reported figure doesn't flip-flop between the two reads.

Remedies stay honest

Matching the copy the advice path already prints:

situationoutcome
sub-floor, host can reach the floorhard fail + resize target clamped to the host (min(warn, physical − reserve) — bash's clamped warn target)
sub-floor, host cannot (physical − reserve < floor)hard fail + practical minimum + "run the client on a larger machine" — never an impossible resize
between floor and warnwarns only — it can run, just tightly
VM at the documented floor (reports ~4.8 GB)passes, via the grace band
daemon not reportingno-op

TRACEBLOC_SKIP_PREFLIGHT still overrides, and Err names it.

Test plan

The Describe that asserted warn-only is replaced by the new contract: sub-floor 4 GB hard-fails; a floor-sized VM reporting 4800 MiB passes; the grace band is bounded on both sides (4607 fails, 4608 passes); daemon-silent is a no-op; between-floor-and-warn only warns; the rec is still capped at host RAM (#417); big-host vs host-too-small remedies; host RAM unreadable still fails; the skip env overrides; and the budget is read exactly once.

Plus a parity Describe that reads both sources and asserts bash still hard-fails, Windows hard-fails too and is no longer warn-only, and both name the same grace constant — so the next divergence fails a test instead of shipping.

⚠️ Worth noting for review: the old tests mocked Get-PfRuntimeMemGb, which this no longer calls. Left as-is they would have passed while testing nothing — they now mock the MiB reader.

Mutation-tested rather than trusted: neutering the gate back to warn-only fails 4 enforcement tests; restoring passes all 11.

Gates

gateresult
Invoke-Pester scripts/tests/401 passed, 0 failed, 9 skipped
Invoke-ScriptAnalyzer (CI scope)0 errors
bash scripts/check-style.shclean
bash scripts/tests/check-drift.shno drift
bash scripts/gen-manifest.shmanifest.sha256 regenerated + committed (R8)

Refs #417, #513

🤖 Generated with Claude Code


Note

Medium Risk
Changes installer exit behavior on Windows at cluster creation when Docker memory is low; mistakes in the grace band could block valid installs or still allow OOM, but scope is limited to preflight and is heavily tested against bash parity.

Overview
Windows post-Docker memory recheck now enforces the client floor instead of warn-only, aligning with bash’s _pf_recheck_runtime_mem (#513). Test-PreflightRuntimeMem still runs first in New-K3dCluster, but sub-floor budgets call Write-PfFail / Err and exit; between-floor-and-warn budgets remain warn-only via Show-MemoryStatus.

Sub-GB precision and grace are added so a VM configured at the documented 5 GB floor (reporting ~4.8 GiB) is not rejected: Get-PfRuntimeMemMib reads docker info once in MiB, Get-PfVmMemGraceMib (default 512, PF_VM_MEM_GRACE_MIB) mirrors bash, and both grading and the gate use the same grace so messaging and enforcement stay aligned.

Failure copy distinguishes a host that cannot ever reach the floor (practical minimum / larger machine) from one that can (clamped .wslconfig or Docker Desktop resize target). TRACEBLOC_SKIP_PREFLIGHT still bypasses the hard fail.

Pester coverage replaces the old warn-only contract, adds grace-boundary and grade/gate parity cases, and a cross-script parity Describe against preflight.sh; manifest.sha256 is updated for install-k8s.ps1.

Reviewed by Cursor Bugbot for commit c3125c6. Bugbot is set up for automated code reviews on this repo. Configure here.

… proceed
#513 decided a Docker VM below the client's memory floor must STOP the install
rather than proceed and OOM-crashloop -- "proceeding is worse than the jarring
stop the WARN path used to avoid" -- and implemented that in bash for every OS
(_pf_recheck_runtime_mem -> error -> exit 1). The Windows installer never got it.
Test-PreflightRuntimeMem just called Show-MemoryStatus, which is warn-only, so
Windows printed "it will OOM" and then carried on and OOM-crashlooped. The
platform this whole memory story (#417/#418/#428/#444/#516) is about was the one
platform still shipping the crash.
Enforcement now lives in Test-PreflightRuntimeMem, mirroring bash's split:
Show-MemoryStatus stays purely presentational (its documented job -- and the
function two PRs just contended over), the recheck grades then enforces. It runs
as New-K3dCluster's FIRST statement, so exiting leaves no half-built cluster.
The subtlety that makes this safe: Get-PfRuntimeMemGb floors to whole GB, and a
guest reports a few hundred MiB BELOW its configured size, so a VM set to exactly
the documented 5 GB floor reports ~4.8 and floors to 4. A bare `-lt 5` would have
hard-failed a correctly configured machine -- the same trap #513's reviewer caught
in bash, but worse here because flooring to whole GB discards up to a GB. So the
gate compares MiB against floor - grace:
- New Get-PfRuntimeMemMib: the same `docker info` value at MiB precision.
- New Get-PfVmMemGraceMib (512, PF_VM_MEM_GRACE_MIB) -- the same constant and the
same comparison bash uses, so both installers put the floor in the same place.
- The recheck now reads the budget ONCE, in MiB, and derives GB from it, so the
number printed and the number enforced on cannot disagree. Flooring (not
rounding) is kept deliberately: Step-1 floors too, and #417 exists so the
reported figure doesn't flip-flop between the two reads.
Remedies stay honest and achievable, matching the copy the advice path already
prints: a host that CAN reach the floor gets a resize target clamped to it
(min(warn, physical - reserve) -- bash's clamped warn target); a host that cannot
(physical - reserve < floor) gets the practical minimum and "run the client on a
larger machine", never a resize that repeats an impossible size. A
between-floor-and-warn budget still only warns -- it can run, just tightly.
TRACEBLOC_SKIP_PREFLIGHT still overrides, and Err names it.
Tests: the Describe that asserted warn-only is replaced by the new contract --
sub-floor 4 GB hard-fails; a floor-sized VM reporting 4800 MiB passes; the grace
band is bounded on both sides (4607 fails, 4608 passes); daemon-silent is a
no-op; between-floor-and-warn only warns; the rec is still capped at host RAM;
big-host vs host-too-small remedies; host RAM unreadable still fails; the skip
env overrides; and the budget is read exactly once. Plus a parity Describe that
reads BOTH sources and asserts bash still hard-fails, Windows hard-fails too and
is no longer warn-only, and both name the same grace constant -- so the next
divergence fails a test instead of shipping.
The old tests mocked Get-PfRuntimeMemGb, which this no longer calls; left as-is
they would have passed while testing nothing, so they now mock the MiB reader.
Mutation-tested rather than trusted: neutering the gate back to warn-only fails 4
enforcement tests; restoring passes all 11.
Gates: Invoke-Pester scripts/tests/ -> 401 passed / 0 failed;
Invoke-ScriptAnalyzer as CI scopes it -> 0 errors; check-style.sh clean;
check-drift.sh no drift; manifest.sha256 regenerated.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodkaLukasWodka self-assigned this Jul 31, 2026
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

👋 Heads-up — Code review queue is at 31 / 30

Above the WIP limit. The team convention is to review existing PRs before opening new work.

Open PRs currently in Code review (oldest first):

Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.)

Comment threadscripts/install-k8s.ps1 Outdated
Bugbot caught a self-contradiction I had reasoned about and wrongly accepted: the
grade was computed from floor($mib / 1024), so a VM configured at exactly the 5 GB
floor (reporting ~4800 MiB) became budget 4, and Show-MemoryStatus printed
hard-floor "it will OOM" copy plus a resize hint -- for a machine the grace-aware
gate immediately ACCEPTED. We told a correctly configured box it would crash and
then carried on. That is precisely the "installer contradicts itself in the same
run" pattern #418/#516 existed to remove; bash classifies that band warn-only.
I had rejected rounding because Step-1 floors and #417 exists so the reported
figure doesn't flip-flop. The fix avoids that trade-off entirely: fold the SAME
grace in before flooring.
$budget = floor(($mib + $grace) / 1024)
- 4800 + 512 -> 5: reports the CONFIGURED size (what the user set and can change),
grades in the warn band, gate passes. Consistent.
- 4096 + 512 -> 4: still sub-floor, still "it will OOM", gate still fails.
Consistent.
Because the grade and the gate now pivot on the same constant, their boundaries
are the same boundary -- (floor * 1024 - grace) MiB. There is no band that warns
"will OOM" yet proceeds, and none that passes while being called sub-floor. The
contradiction is impossible by construction, not merely absent at the values I
happened to test.
Tests: the floor-sized VM is asserted NOT to be told it will OOM and to report
its configured 5 GB; plus a boundary-coincidence test sweeping 4096/4607/4608/
4800/5120 that asserts at EVERY point the copy and the gate agree.
Mutation-tested: reverting the grade to floor($mib / 1024) fails both new tests;
restoring passes all 13.
Gates: Invoke-Pester scripts/tests/ -> 403 passed / 0 failed; Invoke-ScriptAnalyzer
as CI scopes it -> 0 errors; check-style.sh clean; check-drift.sh no drift;
manifest.sha256 regenerated.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

Fixed in c3125c6 — you were right, and I'd reasoned about this exact band and wrongly accepted it.

The grade came from floor($mib / 1024), so a VM configured at the 5 GB floor (reporting ~4800 MiB) graded as 4 and got hard-floor "it will OOM" copy — for a machine the grace-aware gate then accepted. Telling a correctly configured box it will crash and carrying on is the same self-contradiction #418/#516 existed to remove.

I'd rejected rounding because Step-1 floors and #417 exists so the reported figure doesn't flip-flop. Folding the same grace in before flooring avoids that trade-off:

$budget= [int][math]::Floor(($mib+$grace) /1024)
  • 4800 + 512 → 5 — reports the configured size, grades in the warn band, gate passes ✅
  • 4096 + 512 → 4 — still sub-floor, still "it will OOM", gate still fails ✅

Because the grade and the gate now pivot on the same constant, their boundaries are the same boundary (floor * 1024 − grace MiB). No band warns "will OOM" yet proceeds, and none passes while being called sub-floor — impossible by construction rather than merely absent at the values I happened to test.

Added a boundary-coincidence test sweeping 4096 / 4607 / 4608 / 4800 / 5120 asserting the copy and the gate agree at every point, and mutation-tested it: reverting the grade fails both new tests, restoring passes all 13. Suite now 403 / 0.

bugbot run

@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@shujaatTraceblocshujaatTracebloc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving.

Verified against HEAD c3125c6:

  • Bugbot clean: the earlier finding (grade printing "it will OOM" for a machine the gate accepts) is fixed by folding the same grace into the grade — grade and gate now pivot on one constant, so their boundaries coincide at (floor*1024 − grace) MiB. Bugbot re-review is SUCCESS on this commit.
  • Safe placement: Test-PreflightRuntimeMem is the first real statement in New-K3dCluster (only Log precedes it), so the hard-fail leaves no half-built cluster.
  • Logic: MiB reader guarded (^\d+$, int64/1MB); null-host short-circuits to the raw warn target; host-too-small (physical − reserve < floor) yields the practical-minimum copy with no impossible resize; TRACEBLOC_SKIP_PREFLIGHT still overrides.
  • Tests: boundary sweep asserts copy⇔gate agree at every point; grace band bounded both sides (4607 fails / 4608 passes); single docker info read; bash↔Windows parity guard. Old tests correctly repointed from the now-unused Get-PfRuntimeMemGb to the MiB reader.
  • CI fully green.

Closes the last-remaining OS gap in the #513 memory-floor story.

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 c3125c6. Configure here.

@LukasWodka
LukasWodka merged commit 1a3427b into developJul 31, 2026
38 checks passed
LukasWodka added a commit that referenced this pull request Jul 31, 2026
develop moved a long way while this sat open (#513 macOS memory floor, #518
storage remedy, #516/#444/#517/#520 on the Windows side, #434 RFC docs). Only
scripts/manifest.sha256 conflicted textually — the same single install-k8s.ps1
hash line that has now collided four times today. preflight.sh and
preflight.bats auto-merged.
Resolution: regenerate the manifest (it is a DERIVED artifact — 18 digests, no
secrets; authenticity comes from the release workflow's cosign signature, not
from git), so regenerating is the only correct resolution. Taking either side
would leave a wrong digest, which the R8 gate then rejects.
Verified the auto-merge rather than trusting it. The real hazard here was not the
conflict but the clean-looking merge: this branch DELETES _pf_total_mem_kb (the
"prefer the runtime" memory selector whose conflation of host RAM and VM budget
is the bug it fixes), so any caller that landed on develop meanwhile would have
merged into a call to a function that no longer exists — a silent break git
reports as success.
- _pf_total_mem_kb: undefined and unreferenced after the merge; the only mentions
are this branch's own guard test asserting its absence, and a comment.
- #518's _pf_storage_type network-FS remedy survived intact.
- This branch's _pf_runtime_mem_status is present and still wired into both
_pf_memory and _pf_recheck_runtime_mem.
Gates: bats scripts/tests/*.bats -> 683 ok / 0 not ok (full TAP plan reported,
not a truncated read); shellcheck --severity=error over the CI file set -> rc=0;
bash -n clean; Pester -> 403 passed / 0 failed (install-k8s.ps1 arrived via this
merge); check-style clean; check-drift no drift; gen-manifest.sh --check current.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

/fr-pass

@LukasWodka
LukasWodka deleted the fix/windows-runtime-mem-floor branch August 14, 2026 13:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@LukasWodka@shujaatTracebloc