Skip to content

fix(installer): report host RAM consistently + achievable memory advice (#417) - #483

Merged
shujaatTracebloc merged 7 commits into
developfrom
fix/417-memory-truth
Jul 30, 2026
Merged

fix(installer): report host RAM consistently + achievable memory advice (#417)#483
shujaatTracebloc merged 7 commits into
developfrom
fix/417-memory-truth

Conversation

@shujaatTracebloc

@shujaatTraceblocshujaatTracebloc commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

What & why

The Windows preflight memory check preferred Docker's WSL2 VM budget over physical RAM, so the same 15 GB laptop reported Memory: 7 GB with Docker up and Memory: 15 GB ✓ with it down — flip-flopping across re-runs mid-install — and told a 15 GB machine to "give Docker >= 16 GB", physically impossible advice. Seen on three machines (7/6/15 GB readings; the 16-on-15 hint).

Changes (scripts/install-k8s.ps1)

  • Get-PfMemGb now returns HOST RAM only (physical, via CIM) — identical whether Docker is up or down. The container runtime's smaller VM budget is read separately (Get-PfRuntimeMemGb) and shown as its own labeled line: Memory: 15 GB (Docker's current share: 7 GB).
  • Get-PfMemRecommendation caps every suggestion at (host − 2 GB) — never more than the machine physically has; floors at 1 GB.
  • Step-1 (Test-Preflight) and Step-2 (Test-PreflightRuntimeMem) now give one consistent, host-aware message, and Step-2's recommendation is capped too.

Acceptance (from #417)

  • Always two labeled lines: host RAM + Docker's current budget (when a runtime is up)
  • Cap every recommendation at (physical − 2 GB); never recommend more than the machine has
  • De-duplicate the Step-1/Step-2 warnings into one consistent message
  • Same machine, Docker up or down, reports identical host RAM; recommendations always achievable

Tests

  • Get-PfMemRecommendation: cap at host−2, returns desired when it fits, floors at 1, the 16-on-15→13 case.
  • Get-PfMemGb reports host RAM regardless of the Docker budget (Windows CIM-mocked =16; cross-platform decoupling from Get-PfRuntimeMemGb).
  • Test-PreflightRuntimeMem caps its recommendation at host RAM (4 GB on a 6 GB host, never 8).
  • Updated the former "Get-PfMemGb prefers docker" test — it asserted the flip-flop bug.

Local verification

Pester 261/0/9 · check-style pass · drift clean · manifest regenerated · PS parse OK. (No bash changes — bats unaffected.)

Closes#417


Note

Low Risk
Warn-only preflight UX and installer messaging in install-k8s.ps1; no auth, cluster, or data-path changes.

Overview
Fixes Windows preflight memory reporting where host RAM flipped between Docker up/down and advice could exceed physical RAM (e.g. “give Docker 16 GB” on a 15 GB machine).

Get-PfMemGb now always returns physical host RAM (CIM only); Docker’s VM budget stays on Get-PfRuntimeMemGb. Show-MemoryStatus centralizes warn/ok text: the label is always host GB (plus optional “Docker’s current share”), while pass/fail uses the effective memory (Docker budget when known, else host) so a throttled Docker cap is never green-OK on a large host. Get-PfMemRecommendation caps hints at host − 2 GB (min 1 GB); when host RAM is unknown, hints use raw targets instead of capping at the current budget (#483).

Test-Preflight and Test-PreflightRuntimeMem both call Show-MemoryStatus so Step 1 and post-Docker checks share the same wording. Pester coverage and manifest.sha256 updated for install-k8s.ps1.

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

The preflight memory check preferred Docker's WSL2 VM budget over physical RAM,
so the same 15 GB laptop reported "7 GB" with Docker up and "15 GB" with it down
-- flip-flopping across re-runs -- and recommended "give Docker >= 16 GB" on a
15 GB host (impossible).
- Get-PfMemGb now returns HOST RAM only (physical, via CIM) -- identical whether
Docker is up or down. The runtime VM budget is read separately (Get-PfRuntimeMemGb)
and shown as its own labeled line ("Docker's current share: N GB").
- Get-PfMemRecommendation caps every suggestion at (host - 2 GB), so we never
advise more memory than the machine physically has; floors at 1 GB.
- Step-1 (Test-Preflight) and Step-2 (Test-PreflightRuntimeMem) now give one
consistent, host-aware message; Step-2's recommendation is capped too.
Tests: Get-PfMemRecommendation (cap/floor/16-on-15 cases), Get-PfMemGb reports
host RAM regardless of the Docker budget (Windows + cross-platform decoupling),
and Test-PreflightRuntimeMem caps its recommendation at host RAM. Updated the
former "Get-PfMemGb prefers docker" test (it asserted the flip-flop bug).
Closes#417
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@shujaatTraceblocshujaatTracebloc self-assigned this Jul 30, 2026
@shujaatTracebloc
shujaatTracebloc marked this pull request as ready for review July 30, 2026 06:46
@LukasWodka

Copy link
Copy Markdown
Contributor

👋 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
Comment threadscripts/install-k8s.ps1 Outdated
… (Bugbot)
Two follow-ups to the capped-recommendation logic:
- Step-1's middle branch (host below the training threshold) told 5-7 GB hosts to
give Docker host-2 GB (3-5 GB) 'to train locally' — which can't train (~8 GB/job).
It now states the truth: runs fine, but local training needs a bigger machine
(~warnMemGb+2 GB+), with no impossible target.
- Test-PreflightRuntimeMem said 'Raise Docker to N' even when N <= the current
budget (a no-op on a host already at its achievable cap). It now only recommends
raising when that's actually possible; otherwise it names the real fix (more RAM).
Tests updated: the capped-rec test uses a 9 GB host (cap 7, not the 8 target), and
a new test asserts no no-op 'raise to' when already at the cap.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment threadscripts/install-k8s.ps1 Outdated
Step-1 marked memory Ok at host >= warnMemGb (8), but sparing an 8 GB Docker
budget also needs ~2 GB for the OS (the cap in Get-PfMemRecommendation), so an
8-9 GB host got a green check that Step-2 then contradicted with 'can't spare
more'. Extend the too-small-for-training branch to host < warnMemGb + 2 so
Step-1 agrees with Step-2. Adds a test that a 9 GB host is flagged, not Ok.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment threadscripts/tests/install-k8s.Tests.ps1 Outdated
The cross-platform decoupling test asserted Get-PfMemGb -Not -Be 8 while mocking
docker to 8 GiB but not CIM, so on a real 8 GB Windows host (where host RAM is
genuinely 8) it would flakily fail even though the fix is correct. Assert instead
that Get-PfMemGb never invokes docker (Should -Invoke docker -Times 0) - the true
decoupling guarantee, host-independent - and add a separate positive test that
Get-PfRuntimeMemGb still follows the docker budget. The exact host figure stays
locked by the Windows-gated CIM-mocked sibling test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@saadqbalsaadqbal 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.

Good fix for the flip-flop itself — the reported number is stable now and the cap logic is right. But I think Step-1 gave up too much: by grading host RAM only, a throttled Docker budget now gets a green tick where develop warned, including budgets below the 5 GB floor. Details inline at 3065 — I ran the ladder on both branches to confirm rather than eyeball it.

Two parity things I'd rather not lose, not blockers:

  • Get-PfCpu (2974) still has the exact "prefer runtime, fall back to host" line you deleted from Get-PfMemGb, so the CPU row still flip-flops between Docker up and down. Same bug, same machine, and there's no (Docker's current share: N) label on that row to soften it.
  • preflight.sh is untouched, so on macOS/Linux #417 still reproduces as filed: _pf_total_mem_kb (:157) prefers the runtime view, and :248 still says "raise to >= 8 GB (16 GB to train)" with no host cap. Closing #417 on the Windows fix alone leaves the reported repro live on the other platforms — worth either porting the policy or narrowing the issue to Windows.

Also worth a note: check-drift.sh gates ps1<->sh parity for API hosts and download hosts but nothing for preflight resource policy, which is why "drift clean" passed on a change that split the two installers' memory semantics.

Pester 264/0/9 green locally and the manifest hash matches the new file.

Comment threadscripts/install-k8s.ps1 Outdated
Comment threadscripts/install-k8s.ps1 Outdated
Comment threadscripts/install-k8s.ps1
Comment threadscripts/install-k8s.ps1 Outdated
Comment threadscripts/install-k8s.ps1 Outdated
shujaatTraceblocand others added 2 commits July 30, 2026 09:50
…bel (Asad)
Reworked per review: Step-1 graded host RAM and demoted the Docker budget to a
decorative string, so a throttled budget (e.g. 32 GB host / 2 GB Docker) showed a
green Ok and the 15/7 machine from #417 lost its warning. New Show-MemoryStatus
(shared by Step-1 and the post-Docker re-check):
- Grades the EFFECTIVE figure the client actually gets (Docker's VM budget when
known, else host RAM), so a throttled budget is never green-OK'd; both the min
'will OOM' and warn 'training may OOM' floors apply to the budget.
- Always REPORTS host RAM as the label (no flip-flop); when host RAM is unreadable
(CIM blocked) but the budget is, reports the budget labelled as Docker's share
instead of skipping.
- Threads recMemGb back into the training target (was dead on Windows), capped at
host - OS reserve, so the number is achievable (13 on a 15 GB host, not 10/16).
- Single $script:PfOsReserveGb constant (was the literal 2 in three places); the
warnMemGb+2 rung is gone, so PF_WARN_MEM_GB no longer means two things by OS.
Tests: comprehensive Show-MemoryStatus grading (reviewer's 32/2, 16/4, 15/7, 10/5,
host-down, CIM-blocked, healthy) + Step-1/Step-2 delegation.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@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.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 2b2cb5a. Configure here.

Comment threadscripts/install-k8s.ps1
Comment threadscripts/install-k8s.ps1
…RAM is unknown (Bugbot)
When CIM was blocked (host RAM unreadable), $capHost fell back to the Docker
budget, so recommendations were capped at (budget - reserve) -- producing
backwards, contradictory hints like 'Give Docker at least 5 GB (up to 2 GB)' on a
4 GB budget. The budget is the current throttled value, not a ceiling. Now only
cap at the host when host RAM is known; when it isn't, advise the raw targets
(at least minMemGb, up to warnMemGb). Adds a regression test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@shujaatTracebloc
shujaatTracebloc merged commit 40bed86 into developJul 30, 2026
34 of 35 checks passed
@shujaatTracebloc
shujaatTracebloc deleted the fix/417-memory-truth branch July 30, 2026 08:27
LukasWodka added a commit that referenced this pull request Jul 31, 2026
… too-small host says so (#417 residual)
This PR's ORIGINAL scope is superseded. #483 ("report host RAM consistently +
achievable memory advice (#417)") merged to develop on 2026-07-30 and closed#417, delivering the same Windows half by a different route: `Get-PfMemGb`
returns host RAM only, `Get-PfRuntimeMemGb` is shown as its own labelled line,
`Get-PfMemRecommendation` caps at host − 2 GB, and `Show-MemoryStatus` is the
single copy shared by Step 1 and the post-Docker recheck. Re-landing this
branch's `Get-PfHostMemGb` / `Get-PfMemTargets` / `Write-PfRuntimeMemStatus`
would only rename what already works.
What survives is the finding behind this branch's second commit, which is STILL
LIVE on develop: `Get-PfMemRecommendation` floors at 1 GB, so on a small host it
returns a number below the client's own minimum. Measured on develop:
Show-MemoryStatus -HostGb 6 -BudgetGb 3
⚠ Memory: 6 GB (Docker's current share: 3 GB) - below the 5 GB the client needs; it will OOM.
Give Docker at least 5 GB (up to 4 GB): ... [wsl2] memory=4GB ...
"at least 5 GB (up to 4 GB)" is an empty range, and the concrete value it tells
the operator to write is below the 5 GB the same sentence demands — the warning
cannot be cleared by following the advice. #483's Bugbot pass fixed this only for
the host-RAM-unreadable case; a KNOWN small host still hits it.
- `Get-PfMemRecommendation` now floors at PF_MIN_MEM_GB instead of 1, matching
bash's `_pf_clamp_mem_gb` exactly so both installers advise the same on the
same hardware.
- `Show-MemoryStatus` treats a host that cannot reach the floor even with the OS
reserve honoured (host − PF_OS_RESERVE_GB < PF_MIN_MEM_GB) as NOT a budget
bottleneck, so it gets the honest "use a larger machine" line rather than a
resize remedy it can never satisfy. This mirrors the bash recheck's
host-too-small branch, and the sibling fix in #445.
After:
Show-MemoryStatus -HostGb 6 -BudgetGb 3
⚠ Memory: 6 GB (Docker's current share: 3 GB) - below the 5 GB the client needs; it will OOM.
This machine has 6 GB of RAM total; the client needs at least 5 GB. Free up memory or use a larger machine.
Hosts that can reach the floor are unchanged (8 GB host still offers the resize;
15 GB → 13 and 16 GB → 14 recommendations are untouched).
Tests: 7 new/updated — the floor is the client minimum not 1, a 6 GB host never
yields a sub-floor number, PF_MIN_MEM_GB overrides the floor, an invariant sweep
over hosts 1..24 GB, the too-small host gets "larger machine" with no memory=1-4GB
value, and a host that CAN reach the floor still gets the resize hint. The former
"floors at 1 GB" assertion is replaced — it pinned the bug. Pester 374 passed /
0 failed / 9 skipped; PSScriptAnalyzer 0 errors; check-style, check-drift and
gen-manifest --check clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LukasWodka added a commit that referenced this pull request Jul 31, 2026
…n unachievable range (#444)
* fix(installer): memory advice never drops below the client minimum; a too-small host says so (#417 residual)
This PR's ORIGINAL scope is superseded. #483 ("report host RAM consistently +
achievable memory advice (#417)") merged to develop on 2026-07-30 and closed#417, delivering the same Windows half by a different route: `Get-PfMemGb`
returns host RAM only, `Get-PfRuntimeMemGb` is shown as its own labelled line,
`Get-PfMemRecommendation` caps at host − 2 GB, and `Show-MemoryStatus` is the
single copy shared by Step 1 and the post-Docker recheck. Re-landing this
branch's `Get-PfHostMemGb` / `Get-PfMemTargets` / `Write-PfRuntimeMemStatus`
would only rename what already works.
What survives is the finding behind this branch's second commit, which is STILL
LIVE on develop: `Get-PfMemRecommendation` floors at 1 GB, so on a small host it
returns a number below the client's own minimum. Measured on develop:
Show-MemoryStatus -HostGb 6 -BudgetGb 3
⚠ Memory: 6 GB (Docker's current share: 3 GB) - below the 5 GB the client needs; it will OOM.
Give Docker at least 5 GB (up to 4 GB): ... [wsl2] memory=4GB ...
"at least 5 GB (up to 4 GB)" is an empty range, and the concrete value it tells
the operator to write is below the 5 GB the same sentence demands — the warning
cannot be cleared by following the advice. #483's Bugbot pass fixed this only for
the host-RAM-unreadable case; a KNOWN small host still hits it.
- `Get-PfMemRecommendation` now floors at PF_MIN_MEM_GB instead of 1, matching
bash's `_pf_clamp_mem_gb` exactly so both installers advise the same on the
same hardware.
- `Show-MemoryStatus` treats a host that cannot reach the floor even with the OS
reserve honoured (host − PF_OS_RESERVE_GB < PF_MIN_MEM_GB) as NOT a budget
bottleneck, so it gets the honest "use a larger machine" line rather than a
resize remedy it can never satisfy. This mirrors the bash recheck's
host-too-small branch, and the sibling fix in #445.
After:
Show-MemoryStatus -HostGb 6 -BudgetGb 3
⚠ Memory: 6 GB (Docker's current share: 3 GB) - below the 5 GB the client needs; it will OOM.
This machine has 6 GB of RAM total; the client needs at least 5 GB. Free up memory or use a larger machine.
Hosts that can reach the floor are unchanged (8 GB host still offers the resize;
15 GB → 13 and 16 GB → 14 recommendations are untouched).
Tests: 7 new/updated — the floor is the client minimum not 1, a 6 GB host never
yields a sub-floor number, PF_MIN_MEM_GB overrides the floor, an invariant sweep
over hosts 1..24 GB, the too-small host gets "larger machine" with no memory=1-4GB
value, and a host that CAN reach the floor still gets the resize hint. The former
"floors at 1 GB" assertion is replaced — it pinned the bug. Pester 374 passed /
0 failed / 9 skipped; PSScriptAnalyzer 0 errors; check-style, check-drift and
gen-manifest --check clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(installer): name the OS reserve in the host-too-small hint (Bugbot #444)
The previous commit routed 5-6 GB hosts into `Show-MemoryStatus`'s generic
too-small hint, which compares total RAM to the floor and stops there:
host=6 GB, budget=3
⚠ Memory: 6 GB (Docker's current share: 3 GB) - below the 5 GB the client needs; it will OOM.
This machine has 6 GB of RAM total; the client needs at least 5 GB. Free up memory or use a larger machine.
6 >= 5, so as written the operator is told they have enough and still need a
bigger machine. The shortfall only adds up once the ~2 GB the OS needs is named —
which is exactly what bash's `_pf_recheck_runtime_mem` already says. That hint
predates this PR, but this PR is what made 5-6 GB hosts reach it, so it fixes it.
host=6 GB, budget=3 # after
⚠ Memory: 6 GB (Docker's current share: 3 GB) - below the 5 GB the client needs; it will OOM.
This machine has 6 GB of RAM total - too little for tracebloc: the client needs a 5 GB Docker budget and the OS needs ~2 GB, so about 7 GB physical is the practical minimum. Use a larger machine.
Hosts that can reach the floor are untouched (8 GB still gets the resize remedy;
16 GB still gets the training recommendation), and a host below the floor outright
keeps the plain copy.
Tests: 3 new — the reserve and the practical minimum are both named, and the
arithmetic is explained for every too-small host (4-6 GB). Pester 376 passed /
0 failed / 9 skipped; PSScriptAnalyzer 0 errors; parse, check-style, check-drift
and gen-manifest --check all clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(installer): the too-small-host guard also covers the training branch (Bugbot #444 r2)
`hostTooSmall` was consulted only inside the below-floor branch, so a 5-6 GB host
with Docker DOWN graded as "enough to run" and fell into the TRAINING branch,
which printed a concrete budget to write:
host=6 GB, budget=<none>
⚠ Memory: 6 GB - enough to run the client, but training (~8 GB/job) may OOM; 5 GB recommended to train locally.
For local training, give Docker up to 5 GB: WSL2 backend - [wsl2] memory=5GB ...
memory=5GB on a 6 GB machine leaves the OS 1 GB — a budget this same function
calls unachievable two branches up. The previous commit's floor change is what
lifted that number from 4 to 5 and made it reachable, so this closes the hole it
opened rather than trading one inconsistency for another.
Such a machine cannot be tuned into a training box at all, so the branch now says
that instead of printing a number:
host=6 GB, budget=<none> # after
⚠ Memory: 6 GB - enough to run the client, but too little to train locally (~8 GB/job).
This machine has 6 GB of RAM total and the OS needs ~2 GB, so it cannot give Docker a training-sized budget. Run the client here and train on a larger machine.
Machines that CAN reach the floor keep the actionable number (16 GB host with a
6 GB budget still gets "give Docker up to 14 GB" / memory=14GB), and the healthy
paths are untouched.
Tests: 4 new, including the invariant that closes this class for good — across
every branch and every budget shape (host 1-6 GB x budget none/1-6), no branch may
emit a concrete memory= value for a host that cannot reach the floor while keeping
the OS reserve. Pester 379 passed / 0 failed / 9 skipped; PSScriptAnalyzer
0 errors; parse, check-style, check-drift, gen-manifest --check clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.

4 participants

@shujaatTracebloc@LukasWodka@saadqbal@divyasinghds