Skip to content

feat(metal): check free disk space before a model download and report the model store size - #2548

Open
defilan-agent[bot] wants to merge 3 commits into
mainfrom
agent/metal-download-disk-preflight
Open

defilan-agent[bot] wants to merge 3 commits into
mainfrom
agent/metal-download-disk-preflight

Conversation

@defilan-agent

@defilan-agent defilan-agent Bot commented Nov 14, 2026 •

Copy link
Copy Markdown
Contributor

What

The metal agent now checks for disk space before a model download and reports the model store's size. This covers two of the three things #2409 asks for:

  • Disk preflight. When the source declares its size, a download needs room for the bytes it still has to fetch plus --download-free-space-headroom (new flag, default 10Gi). If the model store's filesystem has less free than that, the start is refused with the new reason ModelDownloadNoSpace before a byte is written. A body of unknown length is not checked up front.
    • A resumed download counts only its remaining bytes, and its partial file is kept.
    • A filesystem that fills up during the copy (ENOSPC) is the same refusal, and the partial is kept so the download resumes.
    • The refusal is retried by the agent's refusal re-check, so freeing space starts the service on a later check. Before, a full disk failed the start as a generic StartFailed on the restart backoff.
    • When the agent cannot read the free space, the download proceeds as before.
  • Store gauges. llmkube_metal_agent_model_store_used_bytes (regular files under the store; a symlink is neither followed nor counted) and llmkube_metal_agent_model_store_free_bytes (free space on its filesystem), refreshed every minute.

Not in this PR: the opt-in cleanup of store entries no Model references, the issue's third item. Deciding what is unreferenced is not mechanical:

  • the store layout is <store>/<model name>/<file> with no namespace;
  • the other runtimes keep their own layouts;
  • an agent may watch one namespace or all of them.

A wrong answer deletes model weights, so it deserves its own design. Hence Refs, not Fixes.

Why

Model churn fills the Mac's disk with nothing warning about it, and a full disk then fails new starts silently (#2409). --max-download-size bounds one download, not the store.

Refs #2409

How

  • pkg/agent/executor.go.
    • copyToFileResume, the one copy every agent download goes through (http, https, hf, s3), runs checkFreeSpace for contentLength - resumeFrom before it opens the partial. It does this only when the length is known.
    • A copy or close error that errors.Is syscall.ENOSPC becomes ModelDownloadNoSpaceError{DuringCopy: true}.
    • WithDownloadFreeSpaceHeadroom sets the headroom.
  • pkg/agent/diskspace_unix.go. unix.Statfs, f_bavail × f_bsize: the space an unprivileged writer can still use. diskspace_other.go reports it as unknown off Unix.
  • pkg/agent/refusal.go. handleStartProcessError refuses a ModelDownloadNoSpaceError with EventReasonModelDownloadNoSpace.
    • It is not memoized like ModelDownloadTooLarge. Freeing disk writes nothing to the cluster, so the refusal re-check (publishRefusal's requeue) is what retries it.
    • The message gives free space in whole GiB. publishRefusal rewrites status and emits an Event whenever the message changes, so exact byte counts would churn on every re-check.
  • api/v1alpha1/constants.go. ReasonModelDownloadNoSpace is added to MetalAgentRefusalReasons, so the controller keeps it instead of overwriting it with WaitingForMetalAgent. The repo's refusal_reasons_scan_test caught the missing entry in its own table, which I added.
  • pkg/agent/modelstore_metrics.go. The gauges and watchModelStore, started from Start when --model-store is set.
  • cmd/metal-agent/main.go. The flag, parsed by parseByteQuantity like --max-download-size, passed through MetalAgentConfig.DownloadFreeSpaceHeadroom. Both download flags now resolve in resolveDownloadLimits, which keeps main under the linter's complexity limit; each error still names its flag.
  • pkg/cli/metal_join_args.go. The flag is added to llmkube metal join's table of agent flags, so --agent-arg download-free-space-headroom=20Gi is accepted. TestMetalAgentFlagsMatchTheAgent caught it missing on the first full run.
  • Docs.
    • deployment/macos/README.md: a "--download-free-space-headroom flag (disk preflight)" section, the refusal in "What the agent checks before an engine starts" and in the Events list, a ModelDownloadNoSpace troubleshooting entry, and the two gauges in the metrics table.
    • docs/site/guides/macos-metal.md: the troubleshooting entry.
    • docs/site/reference/metrics.md: regenerated.

Tests (pkg/agent/diskspace_test.go):

  • RED on the old code.
    • copyToFileResume error = <nil>, want a ModelDownloadNoSpaceError
    • error = write model.gguf.partial: no space left on device, want a ModelDownloadNoSpaceError during the copy
    • schedulingStatus = "StartFailed", want ModelDownloadNoSpace
    • the gauges at 0
  • Mutation-checked. Each test failed with its guarded line broken:
    • counting the whole file on a resume;
    • dropping the headroom from the comparison;
    • not mapping ENOSPC;
    • refusing when free space is unknown;
    • not refusing the start;
    • counting symlinks in the store: used bytes = 3270, want 3150;
    • not passing the headroom to the executor: executor headroom = 0.
  • cmd/metal-agent: TestParseByteQuantity, and TestResolveDownloadLimits (each error names its flag).
  • make gate. Passed every step on the final tree: regen, drift, fmt, copyright, vet, lint-all and the full test suite. The first run failed two steps, and both are fixed:
    • lint, on main's complexity and two long lines;
    • TestMetalAgentFlagsMatchTheAgent, which caught the join table entry I had missed.

Review round 1 (qwen38-flash-next)

All three must-fixes were correct, fixed in f8093e7:

  • The new option split WithMaxDownloadBytes from its doc comment. godoc gave its "zero leaves downloads uncapped" to the headroom option. Restored, and the headroom comment now says zero still refuses a download that does not fit.
  • The README said the check runs "before a download writes anything". It runs only when the source declares a size. The README now says so: a body of unknown length is caught only when it fills the disk.
  • handleStartProcessError's comment still listed a full disk under StartFailed. Rewritten to name the new refusal and why it is not memoized.

Nits:

  • Applied.
    • The error now unwraps to the filesystem error, so errors.Is(err, syscall.ENOSPC) holds. The disk-full test checks it, and returning nil from Unwrap fails it.
    • ENOSPC while opening the partial maps to the same refusal.
    • The refusal test injects the error wrapped the way ensureModel wraps it.
    • The controller's refusal-preservation table gets a ModelDownloadNoSpace entry.
    • The README says the re-check backs off to once a minute with no attempt limit.
  • Not changed: the gauges read 0 without --model-store. That does not happen. An empty --model-store resolves to the default store under the agent's data directory (resolveModelStorePath), and the agent exits if it cannot prepare one, so a running agent always has a store to measure.
  • Not changed.
    • The walk takes no context.
    • The digest-stamp write's ENOSPC is not mapped (a stamp is tens of bytes, written after the model).

Open questions for a maintainer:

  • The 10Gi default. On a Mac with less free space than the headroom, downloads that succeed today are refused. That is the point, but it is a visible change; lower it if you prefer.
  • An alert rule on the new gauges. Not in this PR. The chart's rules target in-cluster pods, and the agent's metrics are scraped from the Mac.

Review round 2 (qwen38-flash-next)

Both must-fixes were correct; fixed in 1150805.

  • The README section was inserted mid-way through --max-download-size. Three of that section's paragraphs (the plist snippet, the startup checks, and "the refusal is remembered") read as describing the headroom flag. The memo claim in particular is false for ModelDownloadNoSpace. The new section now follows the whole cap section.
  • The guide still listed a full disk among failures that are "logged but not shown" and retried only on a change. That was also stale for every start failure since fix(metal): report failed starts and act on engine exits at once #2436. It now says a failed start shows StartFailed and retries on the restart backoff, from ReasonStartFailed's own documentation, and that a full store is refused as ModelDownloadNoSpace and retried once space is freed.

Nits:

  • A store the agent cannot read. The comment said the gauge keeps its last value, but the walk's callback swallowed the root's error, so the gauge reported 0. The root error now ends the walk and the gauge keeps its value. TestReportModelStore_KeepsTheLastValueWhenTheStoreIsUnreadable bites without it: used bytes = 0 ..., want the last value 700.
  • Under 1 GiB free. The message printed "about 0 GiB free". It now says "less than 1 GiB free", with a test.
  • The README said the headroom is "for the engine's logs". Reworded: the logs live in the store, and the headroom protects the rest of the filesystem.
  • Not changed: enospcReader injects through Read. Production ENOSPC comes from write or close. All of them go through the same noSpace mapping, which the test pins.

Open questions:

  • statfs block size on Linux. Darwin's statfs counts in f_bsize units, which is what this uses. On Linux, statvfs counts in f_frsize units, which usually but not always equals f_bsize. The Linux agent is experimental ([Raspberry Pi] Run the metal-agent on Linux hosts (experimental) #2520), so I left Linux as is rather than change it without checking it on a target filesystem; worth a follow-up there.
  • The 10Gi default. Still yours to call.

Checklist

  • Tests added/updated
  • make test passes locally (make gate, full suite)
  • make lint passes locally (make lint-all in the gate)
  • Commit messages follow conventional commits
  • All commits are signed off (git commit -s) per DCO (a maintainer signs off with signoff-pr before approving)
  • AI assistance (if any) is disclosed above, per CONTRIBUTING.md
  • Documentation updated (if user-facing change)

Assisted-by: Claude Code (Claude Opus 5.5) (traced the agent's download paths to the shared copy, the refusal re-check and the refusal-reason list, wrote the tests first and watched them fail, implemented the preflight, the ENOSPC mapping, the refusal, the gauges and the flag and the docs, mutation-checked each test, ran make gate; scoped the store cleanup out for its own design; verified and fixed the review findings)

Assisted-by: qwen38-flash-next (unsloth/Qwen3.8-Flash-Next-GGUF UD-Q4_K_XL, generic (llmkube-gufo-rocm-gfx1151) runtime, self-hosted with LLMKube on AMD Strix Halo (Ryzen AI Max, Radeon iGPU); roles: read-only review, two rounds. Round 1's three must-fixes (doc comment placement, the README's overclaim, the stale routing comment) were correct and are fixed in f8093e7 with four of its nits; round 2's two docs must-fixes (the README section placement, the guide's stale full-disk line) were correct and are fixed in 1150805 with two nits)

… the model store size

Model churn filled the Mac disk with nothing warning about it, and a full disk then failed new starts as a generic StartFailed. A download now needs room for its remaining bytes plus --download-free-space-headroom (default 10Gi), checked before anything is written; a disk that fills mid-copy is the same ModelDownloadNoSpace refusal, and the partial is kept to resume. Two gauges report the store size and its free space every minute. Cleanup of unreferenced store entries is left for its own design.
@defilan-agent
defilan-agent Bot requested a review from Defilan as a code owner November 14, 2026 12:38
The README said the free-space check runs before every download, but a body of unknown length is only caught when it fills the disk. handleStartProcessError still documented a full disk as StartFailed, and the new option had split WithMaxDownloadBytes from its own comment. The no-space error now unwraps to the filesystem error, opening the partial on a full disk maps to the same refusal, and the controller refusal table lists the new reason.
@codecov

codecov Bot commented Nov 14, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.78723% with 19 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cmd/metal-agent/main.go 54.54% 10 Missing ⚠️
pkg/agent/executor.go 88.88% 4 Missing ⚠️
pkg/agent/modelstore_metrics.go 85.18% 4 Missing ⚠️
pkg/agent/diskspace_unix.go 75.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

…ction it had split

The new README section was inserted in the middle of the --max-download-size section, so that section's plist snippet, startup checks and remembered-refusal note read as describing the headroom flag. The guide no longer lists a full disk among failures that are hidden and only retried on a change; it names StartFailed and the ModelDownloadNoSpace refusal. A store the agent cannot read keeps the used-bytes gauge at its last value, and the refusal says less than 1 GiB rather than about 0 GiB.

This branch has not been deployed

No deployments
Sign up for free to 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.

0 participants