diff --git a/README.md b/README.md index 93a98ac..3bbbcae 100644 --- a/README.md +++ b/README.md @@ -87,6 +87,7 @@ vmType: # hypervisor; empty picks the host's — see below cpus: 4 memory: 12GiB disk: 100GiB +swap: 4GiB # guest swapfile; 0B disables it — see below extraDomains: # added to the Squid allowlist - registry.mycompany.com containerProxy: false # see below @@ -97,6 +98,17 @@ file inside a workspace: the workspace is mounted writable and is exactly what the agent edits, so anything there is agent-authored input. The host config is the whole knob surface. +### `swap` + +The guest image ships without swap and mounts `/tmp` as a tmpfs sized at half +of RAM. Under a memory burst (Gradle daemons plus Testcontainers, with agent +scratch piling up in `/tmp`) the guest went straight to the OOM killer, which +kills `dockerd` first and leaves the VM unresponsive. `code-vm` therefore +provisions a swapfile of this size on the guest disk and moves `/tmp` onto the +root filesystem, so a burst degrades into slowness instead. The size must be +at least `1MiB` and smaller than `disk`; `0B` disables the swapfile. `/tmp` is +always disk-backed. + ### `vmType` The Lima hypervisor driver. Leave it unset — the default — and `code-vm` picks diff --git a/internal/config/config.go b/internal/config/config.go index e3a5f46..b9c3abf 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -3,9 +3,11 @@ package config import ( "errors" "fmt" + "math" "os" "path/filepath" "regexp" + "strconv" "strings" "gopkg.in/yaml.v3" @@ -41,10 +43,15 @@ type Config struct { // VMType selects the Lima hypervisor driver. Empty (default) means // the host's accelerated one, so nothing has to be set on either platform; // see ResolveVMType. - VMType string `yaml:"vmType,omitempty"` - CPUs int `yaml:"cpus"` - Memory string `yaml:"memory"` - Disk string `yaml:"disk"` + VMType string `yaml:"vmType,omitempty"` + CPUs int `yaml:"cpus"` + Memory string `yaml:"memory"` + Disk string `yaml:"disk"` + // Swap sizes the guest swapfile. The guest image ships without swap, so a + // memory burst (a Gradle build plus Testcontainers, say) goes straight to + // the OOM killer, which picks dockerd first. Swap turns that into + // slowness the user can notice and act on. "0B" disables it. + Swap string `yaml:"swap"` ExtraDomains []string `yaml:"extraDomains,omitempty"` ContainerProxy bool `yaml:"containerProxy"` // Profiles names the customization bundles applied to the guest, in @@ -66,6 +73,7 @@ func Default() Config { CPUs: 4, Memory: "12GiB", Disk: "100GiB", + Swap: "4GiB", ContainerProxy: false, } } @@ -145,6 +153,35 @@ func (c Config) Validate() error { if !sizeRe.MatchString(c.Disk) { return fmt.Errorf("disk must look like \"100GiB\", got %q", c.Disk) } + if !sizeRe.MatchString(c.Swap) { + return fmt.Errorf("swap must look like \"4GiB\" (or \"0B\" to disable), got %q", c.Swap) + } + // The swapfile lives on the guest disk, so it must fit there with room + // to spare. Checking it here also bounds the value: the guest converts + // it with numfmt, which rejects anything beyond 64 bits and would abort + // provisioning under set -e, so this must never pass an overflow along. + swap, err := parseSize(c.Swap) + if err != nil { + return fmt.Errorf("swap: %w", err) + } + disk, err := parseSize(c.Disk) + if err != nil { + return fmt.Errorf("disk: %w", err) + } + if swap >= disk { + return fmt.Errorf("swap (%s) must be smaller than disk (%s)", c.Swap, c.Disk) + } + // The guest script compares the byte count with bash's `[`, which stops + // at MaxInt64 and would silently skip every swap step above it. + if swap > math.MaxInt64 { + return fmt.Errorf("swap (%s) is too large", c.Swap) + } + // mkswap refuses areas under ten pages (40 KiB on 4 KiB pages, 640 KiB + // on 64 KiB pages); a round minimum above both keeps a host-accepted + // value from being one the guest can never format. + if swap != 0 && swap < minSwap { + return fmt.Errorf("swap (%s) must be at least 1MiB, or 0B to disable", c.Swap) + } for i, d := range c.ExtraDomains { if err := ValidateDomain(d); err != nil { return fmt.Errorf("extraDomains[%d]: %w", i, err) @@ -168,6 +205,27 @@ func (c Config) Validate() error { return nil } +// minSwap is the smallest nonzero swap size Validate accepts. +const minSwap = 1 << 20 + +// sizeUnits maps the suffixes sizeRe accepts to bytes. +var sizeUnits = map[string]uint64{"B": 1, "KiB": 1 << 10, "MiB": 1 << 20, "GiB": 1 << 30, "TiB": 1 << 40} + +// parseSize converts a sizeRe-shaped string to bytes, rejecting values that +// do not fit in 64 bits. +func parseSize(s string) (uint64, error) { + m := sizeRe.FindStringSubmatch(s) + if m == nil { + return 0, fmt.Errorf("not a size: %q", s) + } + unit := sizeUnits[m[1]] + n, err := strconv.ParseUint(strings.TrimSuffix(s, m[1]), 10, 64) + if err != nil || n > math.MaxUint64/unit { + return 0, fmt.Errorf("size %q is too large", s) + } + return n * unit, nil +} + // ValidateDomain reports whether d is usable as an allowlist entry. func ValidateDomain(d string) error { if !domainRe.MatchString(d) { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 8f1daf2..b1cd051 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -15,6 +15,9 @@ func TestDefaultValues(t *testing.T) { if d.ContainerProxy { t.Error("containerProxy must default to false") } + if d.Swap != "4GiB" { + t.Errorf("Swap = %q, want 4GiB: the guest has no swap otherwise, and a build burst then OOM-kills dockerd", d.Swap) + } } func TestLoadMissingFileReturnsDefaults(t *testing.T) { @@ -69,6 +72,23 @@ func TestValidate(t *testing.T) { {"zero cpus", func(c *Config) { c.ProjectsRoot = "/p"; c.CPUs = 0 }, true}, {"bad memory", func(c *Config) { c.ProjectsRoot = "/p"; c.Memory = "lots" }, true}, {"bad disk", func(c *Config) { c.ProjectsRoot = "/p"; c.Disk = "12" }, true}, + {"bad swap", func(c *Config) { c.ProjectsRoot = "/p"; c.Swap = "4G" }, true}, + {"empty swap", func(c *Config) { c.ProjectsRoot = "/p"; c.Swap = "" }, true}, + {"zero swap disables", func(c *Config) { c.ProjectsRoot = "/p"; c.Swap = "0B" }, false}, + // Both would fail in the guest instead of here: the first overflows + // numfmt's conversion (aborting provisioning under set -e), the + // second can never be allocated on the guest disk. + {"swap too large to convert", func(c *Config) { c.ProjectsRoot = "/p"; c.Swap = "999999999999999999999TiB" }, true}, + {"swap not smaller than disk", func(c *Config) { c.ProjectsRoot = "/p"; c.Disk = "100GiB"; c.Swap = "100GiB" }, true}, + {"swap smaller than disk", func(c *Config) { c.ProjectsRoot = "/p"; c.Disk = "100GiB"; c.Swap = "102399MiB" }, false}, + // The guest compares the value with bash's `[`, which stops at + // MaxInt64; anything above is silently skipped there. + {"swap above MaxInt64", func(c *Config) { c.ProjectsRoot = "/p"; c.Disk = "16777215TiB"; c.Swap = "8388608TiB" }, true}, + {"swap at MaxInt64 bound", func(c *Config) { c.ProjectsRoot = "/p"; c.Disk = "16777215TiB"; c.Swap = "8388607TiB" }, false}, + // mkswap refuses tiny areas; a size the guest can never format + // must not pass here, while 0B stays the disable value. + {"swap below minimum", func(c *Config) { c.ProjectsRoot = "/p"; c.Swap = "1023KiB" }, true}, + {"swap at minimum", func(c *Config) { c.ProjectsRoot = "/p"; c.Swap = "1MiB" }, false}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { diff --git a/internal/guest/files/scripts/provision-system.sh b/internal/guest/files/scripts/provision-system.sh index 026bebf..17068ec 100644 --- a/internal/guest/files/scripts/provision-system.sh +++ b/internal/guest/files/scripts/provision-system.sh @@ -7,7 +7,7 @@ # # Inputs come from /etc/sandbox/provision.env, a mode:data file rendered by # code-vm: AGENT_USER, AGENT_UID, AGENT_GID, EXTRA_ALLOWED_DOMAINS, -# CONTAINER_PROXY. +# CONTAINER_PROXY, SWAP_SIZE. ############################################################################### set -euo pipefail @@ -147,6 +147,99 @@ MemoryMax=${MEM_MAX_MB}M EOF systemctl daemon-reload +# ── /tmp on disk ───────────────────────────────────────────────────────────── +# The stock image mounts /tmp as a tmpfs sized at half of RAM (tmp.mount, a +# static unit). Agent scratch written there — Claude Code's own scratchpad +# among it — then competes with the workload for memory: 3.7 GB of scratch on +# a 12 GB guest was what tipped a Gradle-plus-Testcontainers run into the OOM +# killer. On the root filesystem the same scratch is just disk, which +# systemd-tmpfiles still ages out (stock tmp.conf: 10 days). +# +# The mask is what persists across boots. Unmounting here as well makes it +# effective on this very boot: provisioning runs before anything +# sandbox-specific has touched /tmp, so the tmpfs is empty and unused. If it +# is busy, it is left alone (a lazy unmount would let its holders keep +# writing into an orphaned tmpfs) and takes effect on the next boot; the +# marker tells the integration suite which of the two happened. +# +# The mask's result is checked rather than assumed: the unmount below would +# make this boot look right even if the mask silently failed, and the tmpfs +# would be back on the next one. +systemctl mask tmp.mount > /dev/null 2>&1 || true +if [ "$(systemctl is-enabled tmp.mount 2> /dev/null || true)" != "masked" ]; then + log "WARNING: could not mask tmp.mount; /tmp returns to tmpfs on the next boot" +fi +install -d -m 0755 /run/sandbox +rm -f /run/sandbox/tmp-unmount-deferred +if [ "$(findmnt -no FSTYPE /tmp 2> /dev/null || true)" = "tmpfs" ]; then + if umount /tmp 2> /dev/null; then + chmod 1777 /tmp + log "/tmp: tmpfs unmounted; now on the root filesystem" + else + touch /run/sandbox/tmp-unmount-deferred + log "/tmp: tmpfs is busy, stays until the next boot (tmp.mount is masked)" + fi +fi + +# ── Swap ───────────────────────────────────────────────────────────────────── +# The image ships without swap, so a memory burst goes straight to the OOM +# killer. That picks dockerd (highest oom_score_adj), the workload restarts +# it, and the loop leaves the guest unresponsive. A swapfile on the guest disk +# turns the burst into slowness instead. It also changes what the agent +# slice's MemoryMax above does: with swap present, the slice hitting its +# limit swaps (MemorySwapMax is unlimited) instead of being OOM-killed. +# +# SWAP_SIZE is a Lima-style size ("4GiB") validated on the host; "0B" means +# no swap. A wrong-sized swapfile is recreated so a config change takes +# effect on the next start. Swap is a mitigation, not a prerequisite, so +# failing to set it up is logged and provisioning continues. +SWAPFILE=/swapfile +SWAP_HAVE=0 +[ -f "$SWAPFILE" ] && SWAP_HAVE=$(stat -c %s "$SWAPFILE") +swap_active() { swapon --show=NAME --noheadings | grep -qx "$SWAPFILE"; } +# The host bounds the value, so this only fails on a hand-edited provision.env. +# An unparseable size is not a request for no swap: keep whatever exists +# rather than delete it, and skip the reconcile below by wanting what we have. +if ! SWAP_WANT=$(numfmt --from=iec-i "${SWAP_SIZE%B}" 2> /dev/null); then + log "WARNING: cannot parse SWAP_SIZE=${SWAP_SIZE}; leaving swap as it is" + SWAP_WANT=$SWAP_HAVE +fi +if [ "$SWAP_WANT" -eq 0 ] || [ "$SWAP_HAVE" -ne "$SWAP_WANT" ]; then + if [ -f "$SWAPFILE" ]; then + if swap_active && ! swapoff "$SWAPFILE"; then + log "WARNING: swapoff $SWAPFILE failed; keeping the existing $(numfmt --to=iec-i "$SWAP_HAVE")B swapfile" + else + rm -f "$SWAPFILE" + SWAP_HAVE=0 + fi + fi +fi +if [ "$SWAP_WANT" -ne 0 ] && [ "$SWAP_HAVE" -eq 0 ]; then + log "Creating ${SWAP_SIZE} swapfile at $SWAPFILE" + # fallocate is fine for ext4, which the image's root filesystem is. + if fallocate -l "$SWAP_WANT" "$SWAPFILE" && chmod 0600 "$SWAPFILE" && mkswap -q "$SWAPFILE"; then + SWAP_HAVE=$SWAP_WANT + else + log "WARNING: could not create the swapfile; the guest runs without swap" + rm -f "$SWAPFILE" + fi +fi +# Activate first, persist second: fstab mirrors what is actually working, +# not what was attempted. An entry for a swapfile that fails to activate +# would give every later boot a failing swap unit, and since the file would +# already be the right size, provisioning would never rebuild it either. So +# on activation failure the file goes too, and the next boot starts clean. +if [ "$SWAP_HAVE" -ne 0 ] && ! swap_active && ! swapon "$SWAPFILE"; then + log "WARNING: swapon $SWAPFILE failed; removing it, the guest runs without swap" + rm -f "$SWAPFILE" + SWAP_HAVE=0 +fi +if [ "$SWAP_HAVE" -eq 0 ]; then + sed -i "\|^${SWAPFILE} |d" /etc/fstab +else + grep -q "^${SWAPFILE} " /etc/fstab || echo "${SWAPFILE} none swap sw 0 0" >> /etc/fstab +fi + # ── Rootless Docker for the agent ──────────────────────────────────────────── # Lima's `mode: user` scripts run as limaadmin, so the agent's rootless setup is # driven from here into the agent's own systemd user session. diff --git a/internal/lima/template.go b/internal/lima/template.go index bfcc8c9..1dd2a1c 100644 --- a/internal/lima/template.go +++ b/internal/lima/template.go @@ -43,6 +43,9 @@ func provisionEnv(c config.Config, p RenderParams) guest.DataFile { } fmt.Fprintf(&b, "EXTRA_ALLOWED_DOMAINS=%q\n", strings.Join(domains, " ")) fmt.Fprintf(&b, "CONTAINER_PROXY=%t\n", c.ContainerProxy) + // Render has already run Validate, so this matched sizeRe and is safe + // to write unquoted. + fmt.Fprintf(&b, "SWAP_SIZE=%s\n", c.Swap) return guest.DataFile{Path: "/etc/sandbox/provision.env", Permissions: "0444", Content: b.String()} } diff --git a/internal/lima/template_test.go b/internal/lima/template_test.go index 6c71441..35d9f23 100644 --- a/internal/lima/template_test.go +++ b/internal/lima/template_test.go @@ -455,3 +455,17 @@ func TestRenderAllowDomains(t *testing.T) { t.Error("nil AllowDomains must fall back to the config's extraDomains") } } + +// provision.env carries the swap size so provision-system.sh can size the +// guest swapfile; "0B" is how a config opts out. +func TestRenderSwapSize(t *testing.T) { + c := testConfig() + c.Swap = "2GiB" + out, err := Render(c, testParams(nil)) + if err != nil { + t.Fatalf("Render: %v", err) + } + if !strings.Contains(out, "SWAP_SIZE=2GiB\n") { + t.Errorf("rendered template missing SWAP_SIZE=2GiB") + } +} diff --git a/internal/lima/testdata/golden-template.yaml b/internal/lima/testdata/golden-template.yaml index d4782f4..49e329c 100644 --- a/internal/lima/testdata/golden-template.yaml +++ b/internal/lima/testdata/golden-template.yaml @@ -95,6 +95,7 @@ provision: AGENT_GID=1000 EXTRA_ALLOWED_DOMAINS="registry.example.com raw.githubusercontent.com" CONTAINER_PROXY=false + SWAP_SIZE=4GiB - mode: data path: "/usr/local/share/sandbox-profiles/manifest.env" owner: "root:root" diff --git a/test-vm-sandbox.sh b/test-vm-sandbox.sh index 5fb25d3..3f2c4fc 100644 --- a/test-vm-sandbox.sh +++ b/test-vm-sandbox.sh @@ -696,6 +696,49 @@ else fail "TasksMax is applied to the agent slice" fi +# The stock image mounts /tmp as a tmpfs sized at half of RAM, so agent +# scratch there (Claude Code's own scratchpad, for one) competes with the +# workload for memory. Provisioning masks tmp.mount and unmounts the tmpfs +# on the very boot it first runs, so a fresh VM never has to be restarted +# for /tmp to land on disk. +# -T resolves the filesystem containing /tmp: once the tmpfs is gone, /tmp is +# a plain directory on the root filesystem and is no longer a mount point. +TMP_FSTYPE=$(adm findmnt -no FSTYPE -T /tmp 2>/dev/null || echo "") +if [ -n "$TMP_FSTYPE" ] && [ "$TMP_FSTYPE" != "tmpfs" ]; then + pass "/tmp is disk-backed, not tmpfs (on $TMP_FSTYPE)" +else + fail "/tmp is disk-backed, not tmpfs (got '${TMP_FSTYPE:-nothing}')" +fi + +if adm test -e /run/sandbox/tmp-unmount-deferred; then + fail "the /tmp tmpfs was unmounted during provisioning, not deferred to next boot" +else + pass "the /tmp tmpfs was unmounted during provisioning, not deferred to next boot" +fi + +# The same-boot unmount would make this boot look right even if the mask had +# failed, so the persistent half is asserted directly. +if [ "$(adm systemctl is-enabled tmp.mount 2> /dev/null)" = "masked" ]; then + pass "tmp.mount is masked, so /tmp stays disk-backed across reboots" +else + fail "tmp.mount is masked, so /tmp stays disk-backed across reboots" +fi + +# The image ships without swap, so a memory burst goes straight to the OOM +# killer, which picks dockerd first. The config's swap size (default 4GiB) +# becomes /swapfile. +if adm swapon --show=NAME --noheadings | grep -qx /swapfile; then + pass "the guest swapfile is active" +else + fail "the guest swapfile is active" +fi + +if adm grep -q "^/swapfile none swap" /etc/fstab; then + pass "the swapfile is in fstab" +else + fail "the swapfile is in fstab" +fi + echo "" echo "── Workspace file ownership ──────────────────────────────────────" @@ -852,10 +895,32 @@ echo "── Restart hygiene ───────────────── # restart must revert to allowlist and leave the guest holding only what the # host config puts back. +# The two restarts here double as the swap transitions: the config's swap +# size reaches the guest through provision.env on every start, so disabling +# swap across the first restart and resizing it across the second exercises +# the destructive paths (swapoff, delete, recreate) with no extra reboots. +# swap_state prints " ". +swap_state() { + # shellcheck disable=SC2016 # expands in the guest, not here + adm sh -c 'printf "%s %s %s\n" \ + "$(stat -c %s /swapfile 2>/dev/null || echo absent)" \ + "$(swapon --show=NAME --noheadings | grep -qx /swapfile && echo active || echo inactive)" \ + "$(grep -q "^/swapfile " /etc/fstab && echo fstab || echo nofstab)"' +} +cp "$CONFIG_FILE" "$CONFIG_FILE.restart-backup" +yq -i '.swap = "0B"' "$CONFIG_FILE" + "${CODE_VM_ARGS[@]}" firewall audit > /dev/null "${CODE_VM_ARGS[@]}" stop > /dev/null 2>&1 "${CODE_VM_ARGS[@]}" start > /dev/null 2>&1 +SWAP_STATE=$(swap_state) +if [ "$SWAP_STATE" = "absent inactive nofstab" ]; then + pass "swap 0B removes the swapfile, its swap entry and its fstab line" +else + fail "swap 0B removes the swapfile, its swap entry and its fstab line (got: $SWAP_STATE)" +fi + if "${CODE_VM_ARGS[@]}" firewall | grep -qx allowlist; then pass "firewall mode reverts to allowlist on VM restart" else @@ -877,11 +942,19 @@ assert_ok "settings stay locked after restart" \ # Host-config domains must survive a restart — that is the difference from the # tmpfs-only mechanism this replaced. init-firewall.sh writes the fragment at # boot from provision.env, so the domain is live before any invocation. -cp "$CONFIG_FILE" "$CONFIG_FILE.restart-backup" +# Swap goes from disabled to a non-default size across the same restart. +yq -i '.swap = "1GiB"' "$CONFIG_FILE" "${CODE_VM_ARGS[@]}" allow --yes example.org > /dev/null 2>&1 "${CODE_VM_ARGS[@]}" stop > /dev/null 2>&1 "${CODE_VM_ARGS[@]}" start > /dev/null 2>&1 +SWAP_STATE=$(swap_state) +if [ "$SWAP_STATE" = "1073741824 active fstab" ]; then + pass "swap 1GiB creates a swapfile of that size, active and in fstab" +else + fail "swap 1GiB creates a swapfile of that size, active and in fstab (got: $SWAP_STATE)" +fi + if adm test -f /run/sandbox/squid-allow.d/10-host-config.conf; then pass "host-config fragment is rebuilt at boot" else