builder-agent: use a pre-mounted /var/lib/buildkit and bound BuildKit GC - #339
Conversation
Extract ensureBuildkitRoot: when the host has already attached a disk at /var/lib/buildkit the agent uses it directly; otherwise it mounts the same 3 GB tmpfs as before with identical command and error behavior. Mountpoint detection reads /proc/self/mounts, which reflects the guest's own mount namespace. Add optional GC bounds to the guest BuildConfig (unused by the host for now). When both bounds are set, buildkitd.toml gains an OCI worker gcpolicy with gc=true and quoted human-readable reservedSpace/maxUsedSpace sizes. The deprecated gckeepstorage is not emitted: it only maps to reservedSpace, so nothing would reclaim above the floor and a fixed-size disk could fill to ENOSPC. Sizes are quoted because BuildKit decodes them with units.RAMInBytes; a bare integer would be read as bytes. Tests cover the mount decision (mounted, unmounted, check and mount failures), /proc/self/mounts parsing, a TOML golden for the worker section, and a decode regression proving the emitted sizes parse back to the intended byte counts via docker/go-units (the same decoder BuildKit uses).
hiroTamada
left a comment
There was a problem hiding this comment.
approved — the mount detection and tests look solid. two non-blocking edge cases worth considering before the host enables persistent caches:
lib/builds/builder_agent/main.go:886-889— if GC bounds are configured but the persistent mount is missing, this silently falls back to a 3 GB tmpfs with limits intended for a larger disk, which can reach ENOSPC before GC starts. consider failing fast when nonzero bounds imply that the mount must exist.lib/builds/builder_agent/main.go:769-775— consider rejectingreservedBytes >= maxUsedBytesand positive values below 1 MiB; the current conversion can emit a nonsensical policy, including"0MB", from positive inputs.
sjmiller609
left a comment
There was a problem hiding this comment.
builder_agent/main.go: crosses 1k lines by end of stack, optionally extract some code to buildkitroot.go.
Otherwise, no objections, looks good.
|
@hiroTamada addressed both edge cases in 69a4179: configured GC bounds now require an existing |
|
@sjmiller609 took the optional split in 02706f7: BuildKit root setup and mount detection now live in |
Extract ensureBuildkitRoot: when the host has already attached a disk at
/var/lib/buildkit the agent uses it directly; otherwise it mounts the same
3 GB tmpfs as before with identical command and error behavior. Mountpoint
detection reads /proc/self/mounts, which reflects the guest's own mount
namespace. If persistent-cache GC bounds are configured, a missing mount
fails fast instead of applying large-disk thresholds to the tmpfs fallback.
Add optional GC bounds to the guest BuildConfig (unused by the host for
now). When both bounds are set, buildkitd.toml gains an OCI worker gcpolicy
with gc=true and quoted human-readable reservedSpace/maxUsedSpace sizes.
Bounds must both be at least 1 MiB, and reservedSpace must be less than
maxUsedSpace. The deprecated gckeepstorage is not emitted: it only maps to
reservedSpace, so nothing would reclaim above the floor and a fixed-size
disk could fill to ENOSPC. Sizes are quoted because BuildKit decodes them
with units.RAMInBytes; a bare integer would be read as bytes.
Tests cover the mount decision (mounted, unmounted, check and mount
failures), /proc/self/mounts parsing, a TOML golden for the worker
section, and a decode regression proving the emitted sizes parse back to
the intended byte counts via docker/go-units (the same decoder BuildKit
uses).
Stack created with GitHub Stacks CLI • Give Feedback 💬
Note
Medium Risk
Changes how BuildKit stores cache data and when builds fail (persistent mount required with GC bounds); host wiring is not in this diff, but misconfigured bounds or shared volumes could cause failed builds or ENOSPC without GC.
Overview
The builder agent no longer always mounts a 3 GB tmpfs at
/var/lib/buildkit.ensureBuildkitRootreuses an existing mount (e.g. host-attached persistent disk) when/proc/self/mountsshows one; otherwise it keeps the same tmpfs behavior. If GC byte bounds are set but the path is not mounted, the build fails fast instead of applying disk-sized GC to tmpfs.CacheGCReservedBytesandCacheGCMaxUsedBytesare added on guest and hostBuildConfig(host still leaves them zero). When both are set,buildkitd.tomlgets an OCI workergcpolicywith quotedreservedSpace/maxUsedSpace(BuildKitunits.RAMInBytesdecoding); validation enforces ≥1 MiB and reserved < max, andgckeepstorageis omitted on purpose.Tests cover mount decisions, mount table parsing, GC TOML output, and size decode regression.
docker/go-unitsis a direct dependency.Reviewed by Cursor Bugbot for commit 02706f7. Bugbot is set up for automated code reviews on this repo. Configure here.