builder-agent: use a pre-mounted /var/lib/buildkit and bound BuildKit GC

go.mod

@@ -11,6 +11,7 @@ require (
 	github.com/digitalocean/go-qemu v0.0.0-20250212194115-ee9b0668d242
 	github.com/distribution/reference v0.6.0
 	github.com/docker/distribution v2.8.3+incompatible
+	github.com/docker/go-units v0.5.0
 	github.com/getkin/kin-openapi v0.133.0
 	github.com/ghodss/yaml v1.0.0
 	github.com/go-chi/chi/v5 v5.3.0
@@ -85,7 +86,6 @@ require (
 	github.com/docker/docker v28.5.1+incompatible // indirect
 	github.com/docker/docker-credential-helpers v0.9.3 // indirect
 	github.com/docker/go-connections v0.6.0 // indirect
-	github.com/docker/go-units v0.5.0 // indirect
 	github.com/dustin/go-humanize v1.0.1 // indirect
 	github.com/fatih/structs v1.1.0 // indirect
 	github.com/felixge/httpsnoop v1.0.4 // indirect

lib/builds/builder_agent/buildkitroot.go

@@ -0,0 +1,67 @@
+package main
+
+import (
+	"fmt"
+	"log"
+	"os"
+	"os/exec"
+	"strings"
+)
+
+// ensureBuildkitRoot prepares BuildKit's data directory. If root is already a
+// mountpoint — e.g. a persistent disk attached by the host — it is used
+// directly. Otherwise a tmpfs is mounted there: the VM rootfs is an overlayfs
+// (read-only ext4 + writable ext4 upper layer) and BuildKit's native overlayfs
+// snapshotter creates char device 0:0 for whiteout markers, but mknod(char 0:0)
+// fails on an overlayfs mount because the kernel treats it as an overlayfs
+// whiteout rather than a regular device node. tmpfs avoids this
+// nested-overlayfs conflict.
+//
+// A pre-mounted root is shared state: only attach a volume there when builds
+// are serialized per volume, never to two builder VMs at once.
+func ensureBuildkitRoot(root string, requirePersistent bool, isMounted func(string) (bool, error), mountTmpfs func(string) error) error {
+	mounted, err := isMounted(root)
+	if err != nil {
+		return fmt.Errorf("check mountpoint %s: %w", root, err)
+	}
+	if mounted {
+		log.Printf("Using existing mount at %s for BuildKit", root)
+		return nil
+	}
+	if requirePersistent {
+		return fmt.Errorf("persistent BuildKit cache configured but %s is not mounted", root)
+	}
+	if err := os.MkdirAll(root, 0755); err != nil {
+		return fmt.Errorf("create buildkit root dir: %w", err)
+	}
+	return mountTmpfs(root)
+}
+
+func mountBuildkitTmpfs(root string) error {
+	mountCmd := exec.Command("mount", "-t", "tmpfs", "-o", "size=3G", "tmpfs", root)
+	if output, err := mountCmd.CombinedOutput(); err != nil {
+		return fmt.Errorf("mount tmpfs at %s (required for native overlayfs snapshotter): %v: %s", root, err, output)
+	}
+	log.Printf("Mounted tmpfs at %s for BuildKit snapshotter", root)
+	return nil
+}
+
+// isMountPoint reports whether path is a mountpoint according to
+// /proc/self/mounts, which reflects the guest's own mount namespace.
+func isMountPoint(path string) (bool, error) {
+	data, err := os.ReadFile("/proc/self/mounts")
+	if err != nil {
+		return false, err
+	}
+	return mountsContain(string(data), path), nil
+}
+
+func mountsContain(mounts, path string) bool {
+	for _, line := range strings.Split(mounts, "\n") {
+		fields := strings.Fields(line)
+		if len(fields) >= 2 && fields[1] == path {
+			return true
+		}
+	}
+	return false
+}

lib/builds/builder_agent/main.go

@@ -52,6 +52,13 @@ type BuildConfig struct {
 	NetworkMode      string            `json:"network_mode"`
 	IsAdminBuild     bool              `json:"is_admin_build,omitempty"`
 	GlobalCacheKey   string            `json:"global_cache_key,omitempty"`
+
+	// CacheGCReservedBytes and CacheGCMaxUsedBytes bound BuildKit's garbage
+	// collector when the build root is a fixed-size persistent disk. Both
+	// must be positive to take effect; zero values leave GC at BuildKit
+	// defaults (used for the tmpfs root, which needs no explicit bound).
+	CacheGCReservedBytes int64 `json:"cache_gc_reserved_bytes,omitempty"`
+	CacheGCMaxUsedBytes  int64 `json:"cache_gc_max_used_bytes,omitempty"`
 }
 
 // SecretRef references a secret to inject during build
@@ -663,6 +670,10 @@ func setupBuildkitdConfig(config *BuildConfig) error {
 	// - RegistryInsecure=false (default) means use HTTPS
 	isHTTPS := !config.RegistryInsecure
 	hasCA := config.RegistryCACert != ""
+	gcConfig, err := buildkitWorkerGCConfig(config.CacheGCReservedBytes, config.CacheGCMaxUsedBytes)
+	if err != nil {
+		return fmt.Errorf("configure BuildKit GC: %w", err)
+	}
 
 	log.Printf("BuildKit config for registry %s (https=%v, insecure=%v, hasCA=%v)",
 		registryHost, isHTTPS, config.RegistryInsecure, hasCA)
@@ -730,6 +741,8 @@ func setupBuildkitdConfig(config *BuildConfig) error {
 	tomlContent.WriteString("[registry.\"docker.io\"]\n")
 	tomlContent.WriteString(fmt.Sprintf("  mirrors = [\"%s\"]\n", registryHost))
 
+	tomlContent.WriteString(gcConfig)
+
 	// Ensure config directory exists
 	buildkitDir := "/home/builder/.config/buildkit"
 	if err := os.MkdirAll(buildkitDir, 0755); err != nil {
@@ -748,6 +761,31 @@ func setupBuildkitdConfig(config *BuildConfig) error {
 	return nil
 }
 
+// buildkitWorkerGCConfig returns the buildkitd.toml OCI worker section that
+// enables BuildKit garbage collection bounded by reservedBytes (the retention
+// floor GC never reclaims below) and maxUsedBytes (the ceiling that triggers
+// reclaim), or an empty string when both bounds are absent. Sizes are quoted
+// human-readable strings: BuildKit decodes them with units.RAMInBytes, so a
+// bare integer would be interpreted as bytes. The deprecated gckeepstorage is
+// deliberately not emitted: it maps to reservedSpace only, so nothing would
+// reclaim above the floor and a fixed-size disk could fill to ENOSPC.
+// gcpolicy string sizes require BuildKit >= v0.13.
+func buildkitWorkerGCConfig(reservedBytes, maxUsedBytes int64) (string, error) {
+	if reservedBytes == 0 && maxUsedBytes == 0 {
+		return "", nil
+	}
+	const minGCBytes = int64(1024 * 1024)
+	if reservedBytes < minGCBytes || maxUsedBytes < minGCBytes {
+		return "", fmt.Errorf("reserved and maximum GC bounds must both be at least 1 MiB")
+	}
+	if reservedBytes >= maxUsedBytes {
+		return "", fmt.Errorf("reserved GC bytes must be less than maximum used bytes")
+	}
+	reservedMB := reservedBytes / minGCBytes
+	maxUsedMB := maxUsedBytes / minGCBytes
+	return fmt.Sprintf("\n[worker.oci]\n  gc = true\n  [[worker.oci.gcpolicy]]\n    reservedSpace = \"%dMB\"\n    maxUsedSpace = \"%dMB\"\n    all = true\n", reservedMB, maxUsedMB), nil
+}
+
 func runBuild(ctx context.Context, config *BuildConfig, logWriter io.Writer) (string, string, error) {
 	var buildLogs bytes.Buffer
 
@@ -856,21 +894,11 @@ func runBuild(ctx context.Context, config *BuildConfig, logWriter io.Writer) (st
 	buildkitdConfig := "/home/builder/.config/buildkit/buildkitd.toml"
 	log.Printf("Using buildkitd config: %s", buildkitdConfig)
 
-	// Mount a tmpfs for BuildKit's data directory.
-	// The VM rootfs is an overlayfs (read-only ext4 + writable ext4 upper layer).
-	// BuildKit's native overlayfs snapshotter creates char device 0:0 for whiteout
-	// markers, but mknod(char 0:0) fails on an overlayfs mount because the kernel
-	// treats it as an overlayfs whiteout rather than a regular device node.
-	// Using tmpfs avoids this nested-overlayfs conflict.
 	buildkitRoot := "/var/lib/buildkit"
-	if err := os.MkdirAll(buildkitRoot, 0755); err != nil {
-		return "", "", fmt.Errorf("create buildkit root dir: %w", err)
-	}
-	mountCmd := exec.Command("mount", "-t", "tmpfs", "-o", "size=3G", "tmpfs", buildkitRoot)
-	if output, err := mountCmd.CombinedOutput(); err != nil {
-		return "", "", fmt.Errorf("mount tmpfs at %s (required for native overlayfs snapshotter): %v: %s", buildkitRoot, err, output)
+	requirePersistentRoot := config.CacheGCReservedBytes != 0 || config.CacheGCMaxUsedBytes != 0
+	if err := ensureBuildkitRoot(buildkitRoot, requirePersistentRoot, isMountPoint, mountBuildkitTmpfs); err != nil {
+		return "", "", err
 	}
-	log.Printf("Mounted tmpfs at %s for BuildKit snapshotter", buildkitRoot)
 
 	log.Printf("Running: buildctl-daemonless.sh %s", strings.Join(args, " "))
 

lib/builds/builder_agent/main_test.go

@@ -0,0 +1,157 @@
+package main
+
+import (
+	"errors"
+	"fmt"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/docker/go-units"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestEnsureBuildkitRootAlreadyMounted(t *testing.T) {
+	root := filepath.Join(t.TempDir(), "buildkit")
+
+	mountCalled := false
+	err := ensureBuildkitRoot(root, false,
+		func(path string) (bool, error) {
+			assert.Equal(t, root, path)
+			return true, nil
+		},
+		func(path string) error {
+			mountCalled = true
+			return nil
+		},
+	)
+
+	require.NoError(t, err)
+	assert.False(t, mountCalled, "tmpfs mount must not be attempted when root is already a mountpoint")
+}
+
+func TestEnsureBuildkitRootMountsTmpfsWhenUnmounted(t *testing.T) {
+	root := filepath.Join(t.TempDir(), "buildkit")
+
+	var mountedPath string
+	err := ensureBuildkitRoot(root, false,
+		func(path string) (bool, error) { return false, nil },
+		func(path string) error {
+			mountedPath = path
+			return nil
+		},
+	)
+
+	require.NoError(t, err)
+	assert.Equal(t, root, mountedPath)
+	assert.DirExists(t, root)
+}
+
+func TestEnsureBuildkitRootRequiresPersistentMountForGC(t *testing.T) {
+	root := filepath.Join(t.TempDir(), "buildkit")
+	mountCalled := false
+
+	err := ensureBuildkitRoot(root, true,
+		func(path string) (bool, error) { return false, nil },
+		func(path string) error {
+			mountCalled = true
+			return nil
+		},
+	)
+
+	require.ErrorContains(t, err, "persistent BuildKit cache configured")
+	assert.False(t, mountCalled, "configured persistent cache must not fall back to tmpfs")
+}
+
+func TestEnsureBuildkitRootPropagatesMountFailure(t *testing.T) {
+	root := filepath.Join(t.TempDir(), "buildkit")
+
+	err := ensureBuildkitRoot(root, false,
+		func(path string) (bool, error) { return false, nil },
+		func(path string) error { return errors.New("mount failed") },
+	)
+
+	require.ErrorContains(t, err, "mount failed")
+}
+
+func TestEnsureBuildkitRootPropagatesMountpointCheckFailure(t *testing.T) {
+	root := filepath.Join(t.TempDir(), "buildkit")
+
+	err := ensureBuildkitRoot(root, false,
+		func(path string) (bool, error) { return false, errors.New("proc unavailable") },
+		func(path string) error { return nil },
+	)
+
+	require.ErrorContains(t, err, "check mountpoint")
+}
+
+func TestMountsContain(t *testing.T) {
+	mounts := strings.Join([]string{
+		"overlay / overlay rw,relatime,lowerdir=/ro,upperdir=/rw/upper,workdir=/rw/work 0 0",
+		"/dev/vdb /var/lib/buildkit ext4 rw,relatime 0 0",
+		"tmpfs /run tmpfs rw,nosuid,nodev 0 0",
+	}, "\n")
+
+	assert.True(t, mountsContain(mounts, "/var/lib/buildkit"))
+	assert.False(t, mountsContain(mounts, "/var/lib/buildkit2"), "prefix match must not count")
+	assert.False(t, mountsContain(mounts, "/var"))
+	assert.False(t, mountsContain("", "/var/lib/buildkit"))
+}
+
+func TestBuildkitWorkerGCConfig(t *testing.T) {
+	// No bounds: no worker section, GC stays at BuildKit defaults (tmpfs path).
+	cfg, err := buildkitWorkerGCConfig(0, 0)
+	require.NoError(t, err)
+	assert.Empty(t, cfg)
+
+	for name, tc := range map[string]struct {
+		reserved int64
+		maximum  int64
+		wantErr  string
+	}{
+		"missing maximum":    {5 * 1024 * 1024, 0, "both be at least 1 MiB"},
+		"missing reserved":   {0, 5 * 1024 * 1024, "both be at least 1 MiB"},
+		"reserved too small": {512 * 1024, 5 * 1024 * 1024, "both be at least 1 MiB"},
+		"maximum too small":  {1024 * 1024, 512 * 1024, "both be at least 1 MiB"},
+		"equal bounds":       {5 * 1024 * 1024, 5 * 1024 * 1024, "must be less than"},
+		"reserved above max": {6 * 1024 * 1024, 5 * 1024 * 1024, "must be less than"},
+	} {
+		t.Run(name, func(t *testing.T) {
+			_, err := buildkitWorkerGCConfig(tc.reserved, tc.maximum)
+			require.ErrorContains(t, err, tc.wantErr)
+		})
+	}
+
+	cfg, err = buildkitWorkerGCConfig(5*1024*1024*1024, 45*1024*1024*1024)
+	require.NoError(t, err)
+	golden := "\n[worker.oci]\n" +
+		"  gc = true\n" +
+		"  [[worker.oci.gcpolicy]]\n" +
+		"    reservedSpace = \"5120MB\"\n" +
+		"    maxUsedSpace = \"46080MB\"\n" +
+		"    all = true\n"
+	assert.Equal(t, golden, cfg)
+	assert.NotContains(t, cfg, "gckeepstorage", "deprecated gckeepstorage must not be emitted")
+}
+
+// TestBuildkitWorkerGCConfigSizeDecode is the regression test for the
+// incorrectly-scaled size bug: BuildKit decodes gcpolicy sizes through
+// units.RAMInBytes, so the emitted strings must decode back to the intended
+// byte counts.
+func TestBuildkitWorkerGCConfigSizeDecode(t *testing.T) {
+	cfg, err := buildkitWorkerGCConfig(5*1024*1024*1024, 45*1024*1024*1024)
+	require.NoError(t, err)
+
+	for key, wantBytes := range map[string]int64{
+		"reservedSpace": 5 * 1024 * 1024 * 1024,
+		"maxUsedSpace":  45 * 1024 * 1024 * 1024,
+	} {
+		var size string
+		_, err = fmt.Sscanf(strings.TrimSpace(strings.Split(strings.Split(cfg, key+" = ")[1], "\n")[0]), "%q", &size)
+		require.NoError(t, err)
+		decoded, err := units.RAMInBytes(size)
+		require.NoError(t, err)
+		assert.Equal(t, wantBytes, decoded, "%s must decode to the intended byte count", key)
+	}
+}

lib/builds/types.go

@@ -175,6 +175,12 @@ type BuildConfig struct {
 
 	// ImageName optionally sets a custom image name for the build output.
 	ImageName string `json:"image_name,omitempty"`
+
+	// CacheGCReservedBytes and CacheGCMaxUsedBytes bound BuildKit's garbage
+	// collector when the build root is a fixed-size persistent disk. The host
+	// leaves both zero for tmpfs-backed builds.
+	CacheGCReservedBytes int64 `json:"cache_gc_reserved_bytes,omitempty"`
+	CacheGCMaxUsedBytes  int64 `json:"cache_gc_max_used_bytes,omitempty"`
 }
 
 // BuildEvent represents a typed SSE event for build streaming