From 07a17b3c343c42be12434bbe99f85c1a905fbc0e Mon Sep 17 00:00:00 2001 From: Luther Monson Date: Thu, 22 Oct 2026 13:19:58 -0700 Subject: [PATCH 1/3] feat(vm): grow macOS job disks past the base image ([vm.macos] disk_size_gb) Every macOS job boots from an APFS clone of base.img, so the guest got the base image's 50 GB (~41 GiB usable). ePHPm's nightly PHP-linked debug test build (each test binary embeds libphp + ICU) filled it every night since at least 2026-10-14 with "No space left on device". [vm.macos] disk_size_gb (GiB, default 0 = unchanged) now: - extends each job's clone with os.Truncate before boot. Sparse on APFS, and the blocks shared with base.img stay shared; - grows the guest's boot container over SSH before the runner starts: `diskutil repairDisk ` then `diskutil apfs resizeContainer 0`. The store is read from `diskutil info`, not assumed, and validated before it is pasted into the command. The fleet's base image has no Recovery partition after the container (ISC, APFS 44.1 GB, free space), so the container can grow in place. Best-effort throughout: a value not larger than the base image is ignored with a warning (never shrinks), and any guest-side failure is a WARN and the job runs on the base-size disk. Each guest command has a 90 s deadline, well inside the 5 m provisioning timeout. The same one-shot setup step also untaps the base image's buildkite/buildkite Homebrew tap, as the admin user. Nothing uses it, and every brew command in a job warned that it is untrusted. --- cmd/ephemerd/main.go | 18 ++-- pkg/config/config.go | 6 ++ pkg/config/config_test.go | 4 + pkg/vm/macos_disk.go | 190 +++++++++++++++++++++++++++++++++++ pkg/vm/macos_disk_test.go | 202 ++++++++++++++++++++++++++++++++++++++ pkg/vm/macosvm_darwin.go | 78 +++++++++++++++ pkg/vm/vm.go | 5 + 7 files changed, 497 insertions(+), 6 deletions(-) create mode 100644 pkg/vm/macos_disk.go create mode 100644 pkg/vm/macos_disk_test.go diff --git a/cmd/ephemerd/main.go b/cmd/ephemerd/main.go index 6997429..a7fea3f 100644 --- a/cmd/ephemerd/main.go +++ b/cmd/ephemerd/main.go @@ -921,13 +921,19 @@ func serve(ctx context.Context, configFile, imagesDirFlag string, containerdTCPP log.Error("macOS VM disk provisioning failed — macOS jobs will be unavailable", "error", err) return } + // No macOS job can be running yet: clean up what earlier daemon + // runs left behind before accepting new ones. + if n, _ := vm.SweepStaleMacOSClones(configDir, 24*time.Hour, time.Now(), log); n > 0 { + log.Info("swept stale macOS VM clones", "removed", n) + } sched.SetMacOSVMConfig(&vm.MacOSVMConfig{ - DataDir: configDir, - DiskImage: files.DiskImage, - SSHSigner: sshSigner, - SSHPubKey: sshPubKey, - CPUs: cfg.VM.MacOS.CPUs, - MemoryMB: cfg.VM.MacOS.MemoryMB, + DataDir: configDir, + DiskImage: files.DiskImage, + SSHSigner: sshSigner, + SSHPubKey: sshPubKey, + CPUs: cfg.VM.MacOS.CPUs, + MemoryMB: cfg.VM.MacOS.MemoryMB, + DiskSizeGB: vm.ResolveMacOSDiskSize(files.DiskImage, cfg.VM.MacOS.DiskSizeGB, log), // Container jobs get this env from the runtime; a macOS VM // job has no container, so without it every macOS job // bypassed the caches entirely. diff --git a/pkg/config/config.go b/pkg/config/config.go index 75f5c27..0601033 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1294,6 +1294,12 @@ type MacOSVMToml struct { MemoryMB uint64 `toml:"memory_mb"` // memory per VM in MB (default: 8192) MaxConcurrent int `toml:"max_concurrent"` // max simultaneous macOS VMs (default: auto-detected from host CPUs) + // DiskSizeGB grows each job's disk to this many GiB. 0 (the default) + // keeps the base image's size. The clone is sparse, so host disk is only + // spent as the guest writes. A value not larger than the base image is + // ignored with a warning — the disk is never shrunk. See pkg/vm/macos_disk.go. + DiskSizeGB uint64 `toml:"disk_size_gb"` + // ProvisionTimeout bounds the pre-registration provisioning phase of a // macOS VM job (boot + wait for the runner to become reachable). If a VM // has not registered its runner within this window it is force-stopped and diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index d1ff2cc..57f9d8d 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -1814,6 +1814,7 @@ disk_size_gb = 100 disk_image = "/path/to/macos.img" cpus = 8 memory_mb = 16384 +disk_size_gb = 100 `), 0644); err != nil { t.Fatal(err) } @@ -1847,6 +1848,9 @@ memory_mb = 16384 if cfg.VM.MacOS.MemoryMB != 16384 { t.Errorf("VM.MacOS.MemoryMB = %d, want 16384", cfg.VM.MacOS.MemoryMB) } + if cfg.VM.MacOS.DiskSizeGB != 100 { + t.Errorf("VM.MacOS.DiskSizeGB = %d, want 100", cfg.VM.MacOS.DiskSizeGB) + } } // --- Additional Providers() and validate() coverage --- diff --git a/pkg/vm/macos_disk.go b/pkg/vm/macos_disk.go new file mode 100644 index 0000000..05fadb7 --- /dev/null +++ b/pkg/vm/macos_disk.go @@ -0,0 +1,190 @@ +package vm + +import ( + "fmt" + "log/slog" + "os" + "path/filepath" + "regexp" + "strings" + "time" +) + +// Per-job macOS disk growth. +// +// Every macOS job boots from an APFS clone of base.img, so the guest gets +// whatever the base image was built with — 50 GB for the Tart image this +// fleet uses, ~41 GiB usable once macOS is on it. That is too small for +// ePHPm's nightly PHP-linked test build (each test binary embeds libphp + +// ICU), which filled the guest every night with "No space left on device". +// +// Growing it takes two steps, one on each side of the boot: +// +// 1. Host, before boot: extend the clone file (os.Truncate). On APFS this is +// sparse — it costs nothing until the guest writes — and the blocks the +// clone shares with base.img stay shared. +// 2. Guest, over SSH: `diskutil repairDisk` moves the GPT's backup header to +// the new end of the disk, then `diskutil apfs resizeContainer 0` +// grows the boot container into the space. This only works when nothing +// sits after the container; the fleet's base image (verified 2026-10-22) +// is ISC (524 MB), the APFS container (44.1 GB), then free space — no +// Recovery partition in the way. +// +// Step 2 is best-effort. A failure is logged and the job runs on the disk it +// would have had anyway; it never fails the job. + +// gib is one GiB. disk_size_gb is GiB, matching the Linux VM's DiskSizeGB. +const gib = 1 << 30 + +// macOSDiskGrowTimeout bounds each guest-side resize command. It stays well +// inside the 5 minute provisioning deadline: a wedged diskutil must cost the +// job its extra space, never the job itself. +const macOSDiskGrowTimeout = 90 * time.Second + +// ResolveMacOSDiskSize validates the configured macOS job disk size against +// the base image and returns the size to use, in GiB. 0 means "leave the +// clone exactly as big as the base image" — the default, and what is +// returned for any value that would not grow the disk. A shrink is never +// attempted: truncating below the base image would cut off the guest's +// filesystem. +func ResolveMacOSDiskSize(baseImage string, wantGB uint64, log *slog.Logger) uint64 { + if wantGB == 0 { + return 0 + } + fi, err := os.Stat(baseImage) + if err != nil { + log.Warn("vm.macos.disk_size_gb ignored: cannot read the base image size", + "disk_size_gb", wantGB, "base_image", baseImage, "error", err) + return 0 + } + if _, grow := macOSDiskGrowBytes(fi.Size(), wantGB); !grow { + log.Warn("vm.macos.disk_size_gb ignored: not larger than the base image, and the disk is never shrunk", + "disk_size_gb", wantGB, "base_image_gib", fmt.Sprintf("%.1f", float64(fi.Size())/gib)) + return 0 + } + return wantGB +} + +// macOSDiskGrowBytes returns the byte size a clone of curSize bytes should be +// truncated to for a wantGB GiB disk, and whether that is a growth at all. +func macOSDiskGrowBytes(curSize int64, wantGB uint64) (int64, bool) { + if wantGB == 0 { + return curSize, false + } + want := int64(wantGB) * gib + if want <= curSize { + return curSize, false + } + return want, true +} + +// diskIdentRe accepts only plain diskutil partition identifiers. The value is +// pasted into a guest shell command, so anything else is refused. +var diskIdentRe = regexp.MustCompile(`^disk[0-9]+s[0-9]+$`) + +// parseAPFSPhysicalStore extracts the boot container's physical store (the +// GPT partition, e.g. "disk0s2") and its whole disk ("disk0") from +// `diskutil info` output for a volume in that container. +// +// It asks rather than assuming disk0s2: the partition layout belongs to the +// base image, which is not ours, and resizing the wrong partition is not a +// mistake worth risking. +func parseAPFSPhysicalStore(info string) (store, whole string, err error) { + for _, line := range strings.Split(info, "\n") { + k, v, ok := strings.Cut(strings.TrimSpace(line), ":") + if !ok || strings.TrimSpace(k) != "APFS Physical Store" { + continue + } + store = strings.TrimSpace(v) + if !diskIdentRe.MatchString(store) { + return "", "", fmt.Errorf("unexpected APFS physical store %q", store) + } + return store, store[:strings.LastIndex(store, "s")], nil + } + return "", "", fmt.Errorf("no \"APFS Physical Store\" line in diskutil info output") +} + +// macOSDiskInfoCmd reads the boot container's details. The Data volume is a +// plain APFS volume in the same container as the sealed system snapshot at /, +// so it is asked first; / is the fallback. +const macOSDiskInfoCmd = `diskutil info /System/Volumes/Data 2>/dev/null || diskutil info /` + +// macOSDiskGrowScript grows the boot container into the space step 1 added. +// It takes the whole disk and the physical store, both validated by +// parseAPFSPhysicalStore, and prints one status line with the root +// filesystem's size before and after. +// +// repairDisk asks before rewriting the partition map, hence `yes |`; its exit +// status is not trusted on its own, because resizeContainer reports plainly +// whether the space is now usable. +func macOSDiskGrowScript(whole, store string) string { + return fmt.Sprintf(`before=$(df -g / | awk 'NR==2{print $2}') +yes | sudo -n diskutil repairDisk %[1]s >/tmp/ephemerd-repairdisk.log 2>&1 +if sudo -n diskutil apfs resizeContainer %[2]s 0 >/tmp/ephemerd-resize.log 2>&1; then r=ok; else r="failed: $(tail -n 3 /tmp/ephemerd-resize.log | tr '\n' ' ')"; fi +after=$(df -g / | awk 'NR==2{print $2}') +echo "before=${before}G after=${after}G resize=${r}" +`, whole, store) +} + +// macOSBrewUntapScript removes the buildkite/buildkite Homebrew tap that the +// Tart base image ships. Nothing on the fleet uses it, and every brew command +// a job runs prints a warning that the tap is not trusted. It runs as the +// guest's admin user (the brew owner on Tart images), never as root, and +// prints one status word. --force untaps even though the tap's formula is +// installed; the installed binary is left alone. +const macOSBrewUntapScript = `B=/opt/homebrew/bin/brew; [ -x "$B" ] || B=/usr/local/bin/brew; [ -x "$B" ] || { echo no-brew; exit 0; } +export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_ANALYTICS=1 HOMEBREW_NO_ENV_HINTS=1 +if "$B" tap 2>/dev/null | grep -qx buildkite/buildkite; then "$B" untap --force buildkite/buildkite >/dev/null 2>&1 && echo untapped || echo untap-failed; else echo absent; fi +` + +// macOSBrewUntapTimeout bounds the untap. `brew tap` is a local listing, so a +// slow one means something is wrong; give up rather than delay the job. +const macOSBrewUntapTimeout = 60 * time.Second + +// SweepStaleMacOSClones deletes leftovers in /vm/macos/jobs — disk +// clones (.img), aux clones (.aux) and virtio-fs share directories +// (/) — whose modification time is older than olderThan. It returns how +// many entries it removed and the paths it failed to remove. +// +// A VM removes its own clones in Stop, but nothing removes them when Stop +// never runs: a daemon restart or crash mid-job, or a stop that wedged inside +// Virtualization.framework and was abandoned (scheduler.abandonVM). The +// guests die with the daemon process, their files do not, and each .img is a +// full-size clone that diverges from base.img as the job wrote to it — one +// from 2026-10-01 had grown to 38 GB on the fleet's Mac before anyone noticed. +// +// Called at startup, before macOS jobs are accepted, so no VM of this process +// can own anything here. The age gate is a second guard on top of that: a +// running guest keeps writing to its .img, so a live clone always looks +// recent. olderThan <= 0 removes everything (for `ephemerd doctor`, which +// only cleans with the daemon stopped). +func SweepStaleMacOSClones(dataDir string, olderThan time.Duration, now time.Time, log *slog.Logger) (removed int, failed []string) { + dir := filepath.Join(dataDir, "vm", "macos", "jobs") + entries, err := os.ReadDir(dir) + if err != nil { + if !os.IsNotExist(err) { + log.Warn("cannot list macOS VM clones for the stale sweep", "dir", dir, "error", err) + } + return 0, nil + } + for _, e := range entries { + info, err := e.Info() + if err != nil { + continue + } + if olderThan > 0 && now.Sub(info.ModTime()) < olderThan { + continue + } + path := filepath.Join(dir, e.Name()) + size := info.Size() + if err := os.RemoveAll(path); err != nil { + log.Warn("could not remove stale macOS VM clone", "path", path, "error", err) + failed = append(failed, path) + continue + } + removed++ + log.Info("removed stale macOS VM clone", "path", path, + "modified", info.ModTime().UTC().Format(time.RFC3339), "apparent_size_gib", fmt.Sprintf("%.1f", float64(size)/gib)) + } + return removed, failed +} diff --git a/pkg/vm/macos_disk_test.go b/pkg/vm/macos_disk_test.go new file mode 100644 index 0000000..b92cb9c --- /dev/null +++ b/pkg/vm/macos_disk_test.go @@ -0,0 +1,202 @@ +package vm + +import ( + "io" + "log/slog" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +func discardLog() *slog.Logger { return slog.New(slog.NewTextHandler(io.Discard, nil)) } + +// baseImageBytes is the fleet's Tart base image: 50,000,000,000 bytes, ~46.6 GiB. +const baseImageBytes = 50_000_000_000 + +// TestMacOSDiskGrowBytes pins the truncate decision: grow only when the +// target is strictly larger than the clone, never shrink. +func TestMacOSDiskGrowBytes(t *testing.T) { + tests := []struct { + name string + cur int64 + wantGB uint64 + size int64 + grow bool + }{ + {"unset keeps the base size", baseImageBytes, 0, baseImageBytes, false}, + {"100 GiB grows", baseImageBytes, 100, 100 * gib, true}, + {"smaller than base never shrinks", baseImageBytes, 40, baseImageBytes, false}, + {"46 GiB is still below 50e9 bytes", baseImageBytes, 46, baseImageBytes, false}, + {"exactly the current size is not a growth", 100 * gib, 100, 100 * gib, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + size, grow := macOSDiskGrowBytes(tt.cur, tt.wantGB) + if size != tt.size || grow != tt.grow { + t.Errorf("macOSDiskGrowBytes(%d, %d) = (%d, %v), want (%d, %v)", tt.cur, tt.wantGB, size, grow, tt.size, tt.grow) + } + }) + } +} + +func TestResolveMacOSDiskSize(t *testing.T) { + base := filepath.Join(t.TempDir(), "base.img") + f, err := os.Create(base) + if err != nil { + t.Fatal(err) + } + // Sparse on every filesystem the tests run on; nothing is written. + if err := f.Truncate(baseImageBytes); err != nil { + t.Fatal(err) + } + _ = f.Close() + + if got := ResolveMacOSDiskSize(base, 0, discardLog()); got != 0 { + t.Errorf("unset: got %d, want 0 (base image size, behaviour unchanged)", got) + } + if got := ResolveMacOSDiskSize(base, 100, discardLog()); got != 100 { + t.Errorf("100 GiB: got %d, want 100", got) + } + if got := ResolveMacOSDiskSize(base, 30, discardLog()); got != 0 { + t.Errorf("smaller than the base image: got %d, want 0 (ignored, never shrunk)", got) + } + if got := ResolveMacOSDiskSize(filepath.Join(t.TempDir(), "missing.img"), 100, discardLog()); got != 0 { + t.Errorf("unreadable base image: got %d, want 0", got) + } +} + +// diskutilInfoData is `diskutil info /System/Volumes/Data` from a guest booted +// off the fleet's base image (layout verified 2026-10-22: disk0s1 ISC, disk0s2 +// the APFS container, then free space). Trimmed to the lines around the one +// the parser needs, in diskutil's real alignment. +const diskutilInfoData = ` Device Identifier: disk3s5 + Device Node: /dev/disk3s5 + Whole: No + Part of Whole: disk3 + + Volume Name: Data + Mounted: Yes + Mount Point: /System/Volumes/Data + + Partition Type: 41504653-0000-11AA-AA11-00306543ECAC + File System Personality: APFS + Type (Bundle): apfs + Name (User Visible): APFS + + APFS Container: disk3 + APFS Physical Store: disk0s2 + Fusion Drive: No + APFS Volume Group: 0C6E6B2A-7D7B-4C59-9E77-2C7D64A8E3E1 + Encrypted: No +` + +func TestParseAPFSPhysicalStore(t *testing.T) { + store, whole, err := parseAPFSPhysicalStore(diskutilInfoData) + if err != nil { + t.Fatalf("parse: %v", err) + } + if store != "disk0s2" || whole != "disk0" { + t.Errorf("got store=%q whole=%q, want disk0s2 / disk0", store, whole) + } + + // Multi-digit identifiers must split on the LAST "s". + store, whole, err = parseAPFSPhysicalStore(" APFS Physical Store: disk12s10\n") + if err != nil || store != "disk12s10" || whole != "disk12" { + t.Errorf("disk12s10: got (%q, %q, %v), want (disk12s10, disk12, nil)", store, whole, err) + } +} + +func TestParseAPFSPhysicalStore_Refuses(t *testing.T) { + for name, info := range map[string]string{ + "no store line": " Device Identifier: disk3s5\n File System Personality: APFS\n", + "whole disk": " APFS Physical Store: disk0\n", + "shell metachars": " APFS Physical Store: disk0s2; rm -rf /\n", + "empty value": " APFS Physical Store:\n", + "not a disk ident": " APFS Physical Store: /dev/disk0s2\n", + } { + t.Run(name, func(t *testing.T) { + if store, _, err := parseAPFSPhysicalStore(info); err == nil { + t.Errorf("accepted %q; anything but a plain diskNsM must be refused", store) + } + }) + } +} + +// TestMacOSDiskGrowScript checks the guest commands target the parsed disk +// and never fail silently: repairDisk is fed its confirmation, sudo is +// non-interactive (a prompt would hang until the deadline), and the status +// line the caller matches on is always printed. +func TestMacOSDiskGrowScript(t *testing.T) { + s := macOSDiskGrowScript("disk0", "disk0s2") + for _, want := range []string{ + "yes | sudo -n diskutil repairDisk disk0 ", + "sudo -n diskutil apfs resizeContainer disk0s2 0 ", + "r=ok", + `echo "before=${before}G after=${after}G resize=${r}"`, + } { + if !strings.Contains(s, want) { + t.Errorf("grow script missing %q:\n%s", want, s) + } + } +} + +// TestSweepStaleMacOSClones: old leftovers (disk clone, aux clone, share +// dir) go; anything recently written — a live guest's clone — stays. +func TestSweepStaleMacOSClones(t *testing.T) { + data := t.TempDir() + jobs := filepath.Join(data, "vm", "macos", "jobs") + if err := os.MkdirAll(filepath.Join(jobs, "110392856702-a9e5c1a4"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(jobs, "119992881741-825ce005"), 0o755); err != nil { + t.Fatal(err) + } + stale := []string{"110392856702-a9e5c1a4.img", "110392856702-a9e5c1a4.aux"} + fresh := []string{"119992881741-825ce005.img", "119992881741-825ce005.aux"} + for _, n := range append(append([]string{}, stale...), fresh...) { + if err := os.WriteFile(filepath.Join(jobs, n), []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + } + now := time.Date(2026, 10, 22, 12, 0, 0, 0, time.UTC) + old := now.Add(-21 * 24 * time.Hour) + recent := now.Add(-time.Hour) + for _, n := range append(stale, "110392856702-a9e5c1a4") { + if err := os.Chtimes(filepath.Join(jobs, n), old, old); err != nil { + t.Fatal(err) + } + } + for _, n := range append(fresh, "119992881741-825ce005") { + if err := os.Chtimes(filepath.Join(jobs, n), recent, recent); err != nil { + t.Fatal(err) + } + } + + removed, failed := SweepStaleMacOSClones(data, 24*time.Hour, now, discardLog()) + if removed != 3 || len(failed) != 0 { + t.Errorf("removed=%d failed=%v, want 3 removed (img, aux, share dir) and no failures", removed, failed) + } + for _, n := range append(stale, "110392856702-a9e5c1a4") { + if _, err := os.Stat(filepath.Join(jobs, n)); !os.IsNotExist(err) { + t.Errorf("stale %s survived the sweep", n) + } + } + for _, n := range append(fresh, "119992881741-825ce005") { + if _, err := os.Stat(filepath.Join(jobs, n)); err != nil { + t.Errorf("recent %s was swept: %v", n, err) + } + } + + // olderThan <= 0 (doctor, daemon stopped) takes everything. + if removed, _ := SweepStaleMacOSClones(data, 0, now, discardLog()); removed != 3 { + t.Errorf("unconditional sweep removed %d, want the 3 remaining", removed) + } +} + +func TestSweepStaleMacOSClones_NoJobsDir(t *testing.T) { + if removed, failed := SweepStaleMacOSClones(t.TempDir(), 24*time.Hour, time.Now(), discardLog()); removed != 0 || failed != nil { + t.Errorf("missing jobs dir: got (%d, %v), want (0, nil)", removed, failed) + } +} diff --git a/pkg/vm/macosvm_darwin.go b/pkg/vm/macosvm_darwin.go index d41fa38..98c88d1 100644 --- a/pkg/vm/macosvm_darwin.go +++ b/pkg/vm/macosvm_darwin.go @@ -34,6 +34,10 @@ type darwinMacOSVM struct { done chan struct{} stopOnce sync.Once // Stop is idempotent: the provisioning watchdog and the // normal teardown path can both call it. + diskGrown bool // the clone was extended before boot; the guest container still has to follow + // guestTuned records that the one-shot guest tweaks (disk grow, brew + // untap) ran. setupRunnerViaSSH is retried on failure; these must not be. + guestTuned bool } // NewMacOSVM creates a new per-job macOS VM. Call Start() to boot it. @@ -102,6 +106,7 @@ func (m *darwinMacOSVM) Start(ctx context.Context) error { if err := apfsCopy(ctx, files.AuxStorage, m.auxPath, m.cfg.Log); err != nil { return fmt.Errorf("copying aux storage: %w", err) } + m.growClone() // Ensure job shared directory exists for JIT config exchange if err := os.MkdirAll(m.jobDir, 0o755); err != nil { @@ -723,6 +728,15 @@ func (m *darwinMacOSVM) setupRunnerViaSSH(ctx context.Context, ip string) error m.cfg.Log.Warn("chown failed", "output", string(out), "error", err) } + // Before the runner starts, so the job sees the full disk from its first step. + if !m.guestTuned { + m.guestTuned = true + if m.diskGrown { + m.growGuestDisk(ctx, client) + } + m.untapBuildkite(ctx, client) + } + // Read JIT config to pass inline via SSH jitData, err := os.ReadFile(filepath.Join(m.jobDir, ".jit_config")) if err != nil { @@ -858,6 +872,70 @@ func (m *darwinMacOSVM) stageProxyEnv(ctx context.Context, client *ssh.Client) { } } +// growClone extends this job's disk clone to the configured size (step 1 in +// macos_disk.go). Sparse, so it is instant and costs no host disk until the +// guest writes. Failure leaves the clone as it was and the job on the base +// image's size. +func (m *darwinMacOSVM) growClone() { + if m.cfg.DiskSizeGB == 0 { + return + } + fi, err := os.Stat(m.clonePath) + if err != nil { + m.cfg.Log.Warn("cannot grow macOS VM disk; job keeps the base image size", "id", m.id, "error", err) + return + } + size, grow := macOSDiskGrowBytes(fi.Size(), m.cfg.DiskSizeGB) + if !grow { + return + } + if err := os.Truncate(m.clonePath, size); err != nil { + m.cfg.Log.Warn("cannot grow macOS VM disk; job keeps the base image size", "id", m.id, "error", err) + return + } + m.diskGrown = true + m.cfg.Log.Info("grew macOS VM disk clone", "id", m.id, "from_bytes", fi.Size(), "to_gib", m.cfg.DiskSizeGB) +} + +// growGuestDisk grows the guest's boot container into the space growClone +// added (step 2 in macos_disk.go). Best-effort: every failure is a WARN and +// the job runs on the smaller disk. +func (m *darwinMacOSVM) growGuestDisk(ctx context.Context, client *ssh.Client) { + info, err := runSSHCommand(ctx, client, macOSDiskInfoCmd, sshCommandTimeout) + if err != nil { + m.cfg.Log.Warn("cannot read the macOS VM's disk layout; job keeps the base image size", "id", m.id, "error", err) + return + } + store, whole, err := parseAPFSPhysicalStore(string(info)) + if err != nil { + m.cfg.Log.Warn("cannot find the macOS VM's boot container; job keeps the base image size", "id", m.id, "error", err) + return + } + out, err := runSSHCommand(ctx, client, macOSDiskGrowScript(whole, store), macOSDiskGrowTimeout) + status := strings.TrimSpace(string(out)) + if err != nil || !strings.Contains(status, "resize=ok") { + m.cfg.Log.Warn("growing the macOS VM's boot container failed; job keeps the base image size", + "id", m.id, "store", store, "status", status, "error", err) + return + } + m.cfg.Log.Info("grew macOS VM boot container", "id", m.id, "store", store, "status", status) +} + +// untapBuildkite drops the base image's untrusted buildkite/buildkite tap +// (see macOSBrewUntapScript). Purely cosmetic for the job, so failures are +// debug-level. +func (m *darwinMacOSVM) untapBuildkite(ctx context.Context, client *ssh.Client) { + out, err := runSSHCommand(ctx, client, macOSBrewUntapScript, macOSBrewUntapTimeout) + status := strings.TrimSpace(string(out)) + if err != nil || status == "untap-failed" { + m.cfg.Log.Debug("could not untap buildkite/buildkite in the macOS VM", "id", m.id, "status", status, "error", err) + return + } + if status == "untapped" { + m.cfg.Log.Info("untapped buildkite/buildkite in the macOS VM", "id", m.id) + } +} + func (m *darwinMacOSVM) Stop() { m.stopOnce.Do(m.stop) } diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index cbac103..c5c60e1 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -120,6 +120,11 @@ type MacOSVMConfig struct { // MemoryMB per macOS VM. Defaults to 8192. MemoryMB uint64 + // DiskSizeGB grows each job's disk clone to this many GiB before boot and + // the guest's boot container with it (see macos_disk.go). 0 keeps the + // base image's size. Pass it through ResolveMacOSDiskSize first. + DiskSizeGB uint64 + // JobEnv is extra KEY=VALUE env for every job's runner: the cache-proxy // env, already rewritten to the host's address on the VM NAT. It is // verified reachable from inside each VM before use; see macos_proxyenv.go. From 9d43a9050230eb981ebbfad3bbe187ce00142b67 Mon Sep 17 00:00:00 2001 From: Luther Monson Date: Thu, 22 Oct 2026 13:19:59 -0700 Subject: [PATCH 2/3] fix(vm): sweep macOS VM clones left behind by dead daemons A macOS VM deletes its .img/.aux clones and share dir in Stop, but nothing removed them when Stop never ran: a daemon restart or crash mid-job, or a stop that wedged in Virtualization.framework and was abandoned. The guests die with the daemon, the files do not, and each .img diverges from base.img as the job writes. One from 2026-10-01 held 38 GB on the fleet's Mac. `ephemerd doctor` only removed the share directories, never the clones. SweepStaleMacOSClones removes jobs/ entries older than 24 h at startup, before macOS jobs are accepted, and logs each one. A live guest keeps writing its clone, so the age gate is a second guard. doctor uses the same sweep with no age gate (it only cleans with the daemon stopped), so the disk clones go too. --- cmd/ephemerd/doctor_darwin.go | 37 ++++++++++++++++------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/cmd/ephemerd/doctor_darwin.go b/cmd/ephemerd/doctor_darwin.go index bb5f8a9..318132b 100644 --- a/cmd/ephemerd/doctor_darwin.go +++ b/cmd/ephemerd/doctor_darwin.go @@ -4,11 +4,16 @@ package main import ( "fmt" + "io" + "log/slog" "os" "os/exec" "runtime" "strings" "syscall" + "time" + + "github.com/ephpm/ephemerd/pkg/vm" ) func platformChecks(pass, warn, fail func(string)) { @@ -125,25 +130,17 @@ func checkEmbeddedAssets(pass, warn, fail func(string)) { } func platformCleanup(dataDir string, pass, warn, fail func(string)) { - // Clean stale VM clone directories - clonesDir := fmt.Sprintf("%s/vm/macos/jobs", dataDir) - if entries, err := os.ReadDir(clonesDir); err == nil { - removed := 0 - for _, e := range entries { - if !e.IsDir() { - continue - } - path := fmt.Sprintf("%s/%s", clonesDir, e.Name()) - if err := os.RemoveAll(path); err != nil { - warn(fmt.Sprintf("could not remove stale VM clone %s: %v", e.Name(), err)) - } else { - removed++ - } - } - if removed > 0 { - pass(fmt.Sprintf("removed %d stale macOS VM clone(s)", removed)) - } else { - pass("no stale macOS VM clones") - } + // Clean stale VM clones: the per-job share directories AND the .img / + // .aux disk clones next to them. Only the directories used to go, + // which left the multi-GB disk clones behind. Doctor only cleans with the + // daemon stopped, so nothing here can belong to a running VM — no age gate. + removed, failed := vm.SweepStaleMacOSClones(dataDir, 0, time.Now(), slog.New(slog.NewTextHandler(io.Discard, nil))) + for _, f := range failed { + warn("could not remove stale VM clone " + f) + } + if removed > 0 { + pass(fmt.Sprintf("removed %d stale macOS VM clone(s)", removed)) + } else if len(failed) == 0 { + pass("no stale macOS VM clones") } } From 10f2b0dcf07483dd247793e584c9e4209a520b0e Mon Sep 17 00:00:00 2001 From: Luther Monson Date: Thu, 22 Oct 2026 13:40:24 -0700 Subject: [PATCH 3/3] fix(vm): remove macOS VM clones on every exit path, sweep leftovers periodically Root cause of the 38 GB clone left on the fleet's Mac from 2026-10-01: a VM deleted its .img/.aux clones and share dir in exactly one place, at the end of Stop, AFTER the Virtualization.framework stop returned. That stop can hang forever (#196). The scheduler then abandons the VM, and the deletion never runs. A daemon restart or crash mid-job leaked the same files: the guest dies with the process and nothing came back for them. Which of the two hit the Oct 1 job can't be told from code alone, so both are closed: - Stop runs the guest stop on its own goroutine and removes the files when it returns OR after 45 s (teardownMacVMFiles), whichever comes first. Unlinking under a wedged guest is safe; the guest keeps its descriptor. Stop itself still blocks until the guest is gone, so the scheduler's abandoned-VM accounting is unchanged. - Start removes the clones itself on any error before the VM exists. - A live-VM registry in pkg/vm (register on NewMacOSVM, unregister when Stop completes) covers VMs that are provisioning or stuck stopping, which s.running does not. - The scheduler sweeps vm/macos/jobs at startup (kicked by SetMacOSVMConfig) and every 30 min. It removes only entries no live VM owns AND unwritten for 2 h, never anything outside jobs/, and logs each removal with its allocated size. Totals appear in status as macos_clones_swept / macos_clone_bytes_swept. - `ephemerd doctor` uses the same sweep with no live set or grace. This replaces the startup-only, mtime-only sweep from the previous commit. --- cmd/ephemerd/doctor_darwin.go | 10 +- cmd/ephemerd/main.go | 5 - pkg/scheduler/macclones.go | 99 +++++++++++++++ pkg/scheduler/macclones_test.go | 75 +++++++++++ pkg/scheduler/scheduler.go | 18 ++- pkg/vm/diskblocks_unix.go | 17 +++ pkg/vm/diskblocks_windows.go | 7 ++ pkg/vm/macos_clones.go | 207 ++++++++++++++++++++++++++++++ pkg/vm/macos_clones_test.go | 215 ++++++++++++++++++++++++++++++++ pkg/vm/macos_disk.go | 49 -------- pkg/vm/macos_disk_test.go | 78 ++++-------- pkg/vm/macosvm_darwin.go | 73 ++++++----- 12 files changed, 712 insertions(+), 141 deletions(-) create mode 100644 pkg/scheduler/macclones.go create mode 100644 pkg/scheduler/macclones_test.go create mode 100644 pkg/vm/diskblocks_unix.go create mode 100644 pkg/vm/diskblocks_windows.go create mode 100644 pkg/vm/macos_clones.go create mode 100644 pkg/vm/macos_clones_test.go diff --git a/cmd/ephemerd/doctor_darwin.go b/cmd/ephemerd/doctor_darwin.go index 318132b..8433fa6 100644 --- a/cmd/ephemerd/doctor_darwin.go +++ b/cmd/ephemerd/doctor_darwin.go @@ -134,13 +134,13 @@ func platformCleanup(dataDir string, pass, warn, fail func(string)) { // .aux disk clones next to them. Only the directories used to go, // which left the multi-GB disk clones behind. Doctor only cleans with the // daemon stopped, so nothing here can belong to a running VM — no age gate. - removed, failed := vm.SweepStaleMacOSClones(dataDir, 0, time.Now(), slog.New(slog.NewTextHandler(io.Discard, nil))) - for _, f := range failed { + res := vm.SweepStaleMacOSClones(dataDir, nil, 0, time.Now(), slog.New(slog.NewTextHandler(io.Discard, nil))) + for _, f := range res.Failed { warn("could not remove stale VM clone " + f) } - if removed > 0 { - pass(fmt.Sprintf("removed %d stale macOS VM clone(s)", removed)) - } else if len(failed) == 0 { + if res.Removed > 0 { + pass(fmt.Sprintf("removed %d stale macOS VM clone(s), %.1f GB", res.Removed, float64(res.FreedBytes)/1e9)) + } else if len(res.Failed) == 0 { pass("no stale macOS VM clones") } } diff --git a/cmd/ephemerd/main.go b/cmd/ephemerd/main.go index a7fea3f..1ed2026 100644 --- a/cmd/ephemerd/main.go +++ b/cmd/ephemerd/main.go @@ -921,11 +921,6 @@ func serve(ctx context.Context, configFile, imagesDirFlag string, containerdTCPP log.Error("macOS VM disk provisioning failed — macOS jobs will be unavailable", "error", err) return } - // No macOS job can be running yet: clean up what earlier daemon - // runs left behind before accepting new ones. - if n, _ := vm.SweepStaleMacOSClones(configDir, 24*time.Hour, time.Now(), log); n > 0 { - log.Info("swept stale macOS VM clones", "removed", n) - } sched.SetMacOSVMConfig(&vm.MacOSVMConfig{ DataDir: configDir, DiskImage: files.DiskImage, diff --git a/pkg/scheduler/macclones.go b/pkg/scheduler/macclones.go new file mode 100644 index 0000000..62f5b51 --- /dev/null +++ b/pkg/scheduler/macclones.go @@ -0,0 +1,99 @@ +package scheduler + +import ( + "context" + "sync" + "time" + + "github.com/ephpm/ephemerd/pkg/vm" +) + +// macCloneSweepInterval is how often the stale macOS clone sweep runs after +// its startup pass. A leaked clone costs what its job wrote — tens of GB for +// a heavy build — so a day of leaks must never pile up, but the pass is a +// single directory read when there is nothing to do. +const macCloneSweepInterval = 30 * time.Minute + +// macCloneSweeper owns the periodic sweep of /vm/macos/jobs (see +// pkg/vm/macos_clones.go for why clones leaked and the layers that now stop +// it) and its running totals for status. +type macCloneSweeper struct { + once sync.Once + kick chan struct{} + + mu sync.Mutex + removed int + freedBytes int64 + + // sweep overrides vm.SweepStaleMacOSClones; only tests set it. + sweep func(dataDir string, live map[string]bool, grace time.Duration, now time.Time) vm.MacOSCloneSweep +} + +func (m *macCloneSweeper) ch() chan struct{} { + m.once.Do(func() { m.kick = make(chan struct{}, 1) }) + return m.kick +} + +// kickNow requests a pass without waiting for the next tick. Non-blocking: a +// pass already pending covers this request. +func (m *macCloneSweeper) kickNow() { + select { + case m.ch() <- struct{}{}: + default: + } +} + +// totals reports how many clones the sweep has removed since start, and the +// disk that gave back. +func (m *macCloneSweeper) totals() (int, int64) { + m.mu.Lock() + defer m.mu.Unlock() + return m.removed, m.freedBytes +} + +// runMacCloneSweeper sweeps on every kick (SetMacOSVMConfig sends the first +// one, which is the startup pass) and every macCloneSweepInterval. A tick +// before macOS support is configured does nothing. +func (s *Scheduler) runMacCloneSweeper(ctx context.Context) { + ticker := time.NewTicker(macCloneSweepInterval) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + case <-s.macClones.ch(): + } + s.sweepMacClones(time.Now()) + } +} + +// sweepMacClones runs one pass. Protected from it: every VM this process +// still owns files for (vm.LiveMacOSVMIDs — provisioning, running, or in a +// wedged stop, which s.running does not cover) and anything written within +// vm.MacOSCloneGrace. +func (s *Scheduler) sweepMacClones(now time.Time) { + s.mu.Lock() + cfg := s.cfg.MacOSVMConfig + s.mu.Unlock() + if cfg == nil || cfg.DataDir == "" { + return + } + sweep := s.macClones.sweep + if sweep == nil { + log := s.cfg.Log.With("component", "macos-clone-sweeper") + sweep = func(dataDir string, live map[string]bool, grace time.Duration, now time.Time) vm.MacOSCloneSweep { + return vm.SweepStaleMacOSClones(dataDir, live, grace, now, log) + } + } + res := sweep(cfg.DataDir, vm.LiveMacOSVMIDs(), vm.MacOSCloneGrace, now) + if res.Removed == 0 { + return + } + s.macClones.mu.Lock() + s.macClones.removed += res.Removed + s.macClones.freedBytes += res.FreedBytes + s.macClones.mu.Unlock() + s.cfg.Log.Info("swept stale macOS VM clones", "component", "macos-clone-sweeper", + "removed", res.Removed, "freed_bytes", res.FreedBytes, "failed", len(res.Failed)) +} diff --git a/pkg/scheduler/macclones_test.go b/pkg/scheduler/macclones_test.go new file mode 100644 index 0000000..e5d8717 --- /dev/null +++ b/pkg/scheduler/macclones_test.go @@ -0,0 +1,75 @@ +package scheduler + +import ( + "context" + "testing" + "time" + + "github.com/ephpm/ephemerd/pkg/vm" +) + +// TestSweepMacClones_IdleWithoutMacOS: a node with no macOS support (every +// Linux and Windows node, and a Mac before its base image is ready) never +// touches the disk. +func TestSweepMacClones_IdleWithoutMacOS(t *testing.T) { + s := New(Config{Log: quietLogger()}) + called := false + s.macClones.sweep = func(string, map[string]bool, time.Duration, time.Time) vm.MacOSCloneSweep { + called = true + return vm.MacOSCloneSweep{} + } + s.sweepMacClones(time.Now()) + if called { + t.Error("swept with no macOS VM config") + } +} + +// TestSweepMacClones_PassesGraceAndTotals: a pass sweeps the configured data +// dir with the production grace and the live set, and its results add up in +// the totals status reports. +func TestSweepMacClones_PassesGraceAndTotals(t *testing.T) { + s := New(Config{Log: quietLogger(), MacOSVMConfig: &vm.MacOSVMConfig{DataDir: "/var/lib/ephemerd"}}) + var gotDir string + var gotGrace time.Duration + s.macClones.sweep = func(dir string, live map[string]bool, grace time.Duration, _ time.Time) vm.MacOSCloneSweep { + gotDir, gotGrace = dir, grace + if live == nil { + t.Error("sweep got a nil live set; it must always get the live VM ids") + } + return vm.MacOSCloneSweep{Removed: 3, FreedBytes: 38 << 30} + } + s.sweepMacClones(time.Now()) + s.sweepMacClones(time.Now()) + if gotDir != "/var/lib/ephemerd" || gotGrace != vm.MacOSCloneGrace { + t.Errorf("swept dir=%q grace=%s, want the data dir and vm.MacOSCloneGrace", gotDir, gotGrace) + } + if n, freed := s.macClones.totals(); n != 6 || freed != 2*(38<<30) { + t.Errorf("totals = (%d, %d), want (6, %d)", n, freed, 2*(38<<30)) + } +} + +// TestMacCloneSweeper_KickRunsAPass: SetMacOSVMConfig is the startup pass — +// it must trigger a sweep promptly, not wait for the 30 minute tick. +func TestMacCloneSweeper_KickRunsAPass(t *testing.T) { + s := New(Config{Log: quietLogger()}) + passes := make(chan struct{}, 4) + s.macClones.sweep = func(string, map[string]bool, time.Duration, time.Time) vm.MacOSCloneSweep { + passes <- struct{}{} + return vm.MacOSCloneSweep{} + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go s.runMacCloneSweeper(ctx) + + s.SetMacOSVMConfig(&vm.MacOSVMConfig{DataDir: t.TempDir()}) + select { + case <-passes: + case <-time.After(2 * time.Second): + t.Fatal("SetMacOSVMConfig did not trigger a sweep") + } + + // Kicks coalesce: a burst never blocks the caller. + for range 10 { + s.macClones.kickNow() + } +} diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go index 1a52c12..f205601 100644 --- a/pkg/scheduler/scheduler.go +++ b/pkg/scheduler/scheduler.go @@ -295,6 +295,10 @@ type Scheduler struct { // default; only tests set it. macAbandonedCap int + // macClones runs the periodic stale-clone sweep for macOS VMs and keeps + // its totals for status. See macclones.go. + macClones macCloneSweeper + // busyProbe overrides the ground-truth "is this runner executing a // job" check that vetoes orphan teardown. Nil (the default) uses // probeRunnerBusy: local container/VM/process introspection, falling @@ -324,8 +328,11 @@ const maxProvisionAttempts = 5 // finishes. func (s *Scheduler) SetMacOSVMConfig(cfg *vm.MacOSVMConfig) { s.mu.Lock() - defer s.mu.Unlock() s.cfg.MacOSVMConfig = cfg + s.mu.Unlock() + // macOS jobs can start any moment now: sweep what earlier daemons left + // behind first. The sweep runs on its own goroutine (see Run). + s.macClones.kickNow() } // failureBackoff tracks per-repo failure counts to compute exponential backoff. @@ -647,6 +654,9 @@ func (s *Scheduler) Run(ctx context.Context) error { go s.retry.Run(ctx) } + // Stale macOS VM clones: idle until macOS support is configured. + go s.runMacCloneSweeper(ctx) + // Clean old job logs on startup, then periodically every hour. // Retention period is configurable via [log] log_retention (default 7d). logDir := filepath.Join(s.cfg.DataDir, "logs") @@ -2418,6 +2428,12 @@ func (s *Scheduler) handleHealthz(w http.ResponseWriter, r *http.Request) { if n := s.AbandonedMacOSVMDetails(); len(n) > 0 { status["abandoned_macos_vm_details"] = n } + // Leaked macOS disk clones the sweeper removed since start. Non-zero means + // some teardown did not clean up after itself — see macclones.go. + if n, freed := s.macClones.totals(); n > 0 { + status["macos_clones_swept"] = n + status["macos_clone_bytes_swept"] = freed + } w.Header().Set("Content-Type", "application/json") if err := json.NewEncoder(w).Encode(status); err != nil { diff --git a/pkg/vm/diskblocks_unix.go b/pkg/vm/diskblocks_unix.go new file mode 100644 index 0000000..8632b8d --- /dev/null +++ b/pkg/vm/diskblocks_unix.go @@ -0,0 +1,17 @@ +//go:build !windows + +package vm + +import ( + "os" + "syscall" +) + +// diskBlocks reports the bytes a file actually occupies. st_blocks is in +// 512-byte units on every unix ephemerd runs on. +func diskBlocks(fi os.FileInfo) int64 { + if st, ok := fi.Sys().(*syscall.Stat_t); ok { + return int64(st.Blocks) * 512 + } + return fi.Size() +} diff --git a/pkg/vm/diskblocks_windows.go b/pkg/vm/diskblocks_windows.go new file mode 100644 index 0000000..47e8693 --- /dev/null +++ b/pkg/vm/diskblocks_windows.go @@ -0,0 +1,7 @@ +package vm + +import "os" + +// diskBlocks falls back to the apparent size. macOS clones never live on a +// Windows host; this only keeps the sweep portable for tests. +func diskBlocks(fi os.FileInfo) int64 { return fi.Size() } diff --git a/pkg/vm/macos_clones.go b/pkg/vm/macos_clones.go new file mode 100644 index 0000000..c1d51cb --- /dev/null +++ b/pkg/vm/macos_clones.go @@ -0,0 +1,207 @@ +package vm + +import ( + "fmt" + "log/slog" + "os" + "path/filepath" + "strings" + "sync" + "time" +) + +// macOS VM clone lifecycle. +// +// Every macOS job owns three things under /vm/macos/jobs, all named +// after its per-attempt instance id: .img (an APFS clone of base.img), +// .aux (a copy of the aux storage) and / (the virtio-fs share). The +// .img starts as a free clone and diverges as the guest writes, so a leaked +// one costs whatever the job wrote — the one found on the fleet's Mac on +// 2026-10-22, from a job on 2026-10-01, held 38 GB. +// +// They used to be removed in exactly one place: at the end of Stop, AFTER the +// Virtualization.framework stop returned. That stop is the call #196 showed +// can hang forever, and when it does the scheduler abandons the VM and moves +// on — so the removal never ran, and nothing ever came back for the files. A +// daemon restart or crash mid-job leaked them the same way: the guest dies +// with the process, its files do not. +// +// Three layers now close that: +// +// 1. Start removes the files itself on any error after it created them, so +// a failed boot leaves nothing even if no one calls Stop. +// 2. Stop removes them unconditionally: the Vz stop runs on its own +// goroutine with a bound (teardownMacVMFiles), and the files go when it +// returns OR when the bound expires. Unlinking a disk under a wedged +// guest is safe — the guest keeps its open descriptor, and the space +// comes back when the descriptor closes. +// 3. SweepStaleMacOSClones, at startup and periodically, removes anything +// left in jobs/ that no live VM of this process owns (LiveMacOSVMIDs) +// and that has not been written for a grace period. That catches what +// 1 and 2 cannot: files from a daemon that died. + +// liveMacVMs is the set of macOS VM instance ids this process has created and +// not yet finished tearing down — provisioning, running, or stuck in a +// wedged stop. It is the ground truth the sweep must not touch, and it lives +// here rather than in the scheduler because the scheduler only learns about a +// VM once it is tracked: a VM still provisioning, or one whose teardown was +// abandoned, is invisible to s.running but very much owns its files. +var liveMacVMs = struct { + sync.Mutex + ids map[string]struct{} +}{ids: map[string]struct{}{}} + +func registerMacVM(id string) { + liveMacVMs.Lock() + liveMacVMs.ids[id] = struct{}{} + liveMacVMs.Unlock() +} + +func unregisterMacVM(id string) { + liveMacVMs.Lock() + delete(liveMacVMs.ids, id) + liveMacVMs.Unlock() +} + +// LiveMacOSVMIDs returns the instance ids of every macOS VM this process +// still owns files for. +func LiveMacOSVMIDs() map[string]bool { + liveMacVMs.Lock() + defer liveMacVMs.Unlock() + out := make(map[string]bool, len(liveMacVMs.ids)) + for id := range liveMacVMs.ids { + out[id] = true + } + return out +} + +// MacOSCloneGrace is how long an unowned entry in jobs/ must have gone +// unwritten before the periodic sweep removes it. The live set is what +// protects running VMs; this is the second guard, for anything the live set +// cannot know about (a second daemon pointed at the same data dir, a clone in +// the instant between apfsCopy and registration). A guest writes to its .img +// continuously, so a live clone never looks two hours old. +const MacOSCloneGrace = 2 * time.Hour + +// macVMStopBound is how long Stop waits for Virtualization.framework before +// it removes the VM's files anyway: the 15 s graceful request plus a force +// stop, with headroom. Past it the stop is wedged (#196) and waiting longer +// only means the files never go. +const macVMStopBound = 45 * time.Second + +// teardownMacVMFiles runs stopVM on its own goroutine, waits up to bound for +// it, then removes files and dirs regardless of whether it returned. It +// reports whether the stop was still running when the files were removed, +// and returns a channel closed when stopVM finally returns (if ever). +func teardownMacVMFiles(stopVM func(), bound time.Duration, files, dirs []string, log *slog.Logger) (wedged bool, stopped <-chan struct{}) { + done := make(chan struct{}) + go func() { + defer close(done) + stopVM() + }() + timer := time.NewTimer(bound) + defer timer.Stop() + select { + case <-done: + case <-timer.C: + wedged = true + log.Warn("macOS VM stop still running after the bound; removing its disk clones anyway", + "bound", bound, "detail", "the guest keeps its open file until it dies; nothing else would ever delete these") + } + removeMacVMFiles(files, dirs, log) + return wedged, done +} + +// removeMacVMFiles deletes a VM's clones and share directory. Idempotent: +// missing paths are not errors, so every exit path can call it. +func removeMacVMFiles(files, dirs []string, log *slog.Logger) { + for _, p := range files { + if p == "" { + continue + } + if err := os.Remove(p); err != nil && !os.IsNotExist(err) { + log.Warn("failed to remove VM clone", "path", p, "error", err) + } + } + for _, d := range dirs { + if d == "" { + continue + } + if err := os.RemoveAll(d); err != nil { + log.Warn("failed to remove job directory", "path", d, "error", err) + } + } +} + +// MacOSCloneSweep is the result of one SweepStaleMacOSClones pass. +type MacOSCloneSweep struct { + Removed int + FreedBytes int64 // allocated bytes, not apparent size: a clone is sparse + Failed []string +} + +// cloneEntryID maps a jobs/ entry name to the VM instance id that owns it. +func cloneEntryID(name string) string { + return strings.TrimSuffix(strings.TrimSuffix(name, ".img"), ".aux") +} + +// SweepStaleMacOSClones removes entries in /vm/macos/jobs that no +// VM in live owns and that have not been modified for grace. It never looks +// outside jobs/, so base.img, aux.bin and the rest of vm/macos are out of +// reach by construction. grace <= 0 with an empty live set removes +// everything — for `ephemerd doctor`, which only cleans with the daemon +// stopped. +func SweepStaleMacOSClones(dataDir string, live map[string]bool, grace time.Duration, now time.Time, log *slog.Logger) MacOSCloneSweep { + var res MacOSCloneSweep + dir := filepath.Join(dataDir, "vm", "macos", "jobs") + entries, err := os.ReadDir(dir) + if err != nil { + if !os.IsNotExist(err) { + log.Warn("cannot list macOS VM clones for the stale sweep", "dir", dir, "error", err) + } + return res + } + for _, e := range entries { + if live[cloneEntryID(e.Name())] { + continue + } + info, err := e.Info() + if err != nil { + continue + } + if grace > 0 && now.Sub(info.ModTime()) < grace { + continue + } + path := filepath.Join(dir, e.Name()) + size := allocatedBytes(path, info) + if err := os.RemoveAll(path); err != nil { + log.Warn("could not remove stale macOS VM clone", "path", path, "error", err) + res.Failed = append(res.Failed, path) + continue + } + res.Removed++ + res.FreedBytes += size + log.Info("removed stale macOS VM clone", "path", path, + "modified", info.ModTime().UTC().Format(time.RFC3339), + "size_gib", fmt.Sprintf("%.1f", float64(size)/gib)) + } + return res +} + +// allocatedBytes is what removing path gives back. For a file that is its +// allocated blocks where the platform reports them (diskBlocks), because a +// clone's apparent size is the whole virtual disk; for a directory it is the +// sum of its files. +func allocatedBytes(path string, info os.FileInfo) int64 { + if !info.IsDir() { + return diskBlocks(info) + } + var total int64 + _ = filepath.Walk(path, func(_ string, fi os.FileInfo, err error) error { + if err == nil && !fi.IsDir() { + total += diskBlocks(fi) + } + return nil + }) + return total +} diff --git a/pkg/vm/macos_clones_test.go b/pkg/vm/macos_clones_test.go new file mode 100644 index 0000000..e9b31f0 --- /dev/null +++ b/pkg/vm/macos_clones_test.go @@ -0,0 +1,215 @@ +package vm + +import ( + "os" + "path/filepath" + "testing" + "time" +) + +// cloneFixture lays out jobs/ the way three VM instances leave it: a disk +// clone, an aux clone and a share dir each, every entry stamped with mtime. +func cloneFixture(t *testing.T, data string, ids map[string]time.Time) string { + t.Helper() + jobs := filepath.Join(data, "vm", "macos", "jobs") + for id, mtime := range ids { + if err := os.MkdirAll(filepath.Join(jobs, id), 0o755); err != nil { + t.Fatal(err) + } + for _, n := range []string{id + ".img", id + ".aux"} { + if err := os.WriteFile(filepath.Join(jobs, n), []byte("clone"), 0o600); err != nil { + t.Fatal(err) + } + } + for _, n := range []string{id, id + ".img", id + ".aux"} { + if err := os.Chtimes(filepath.Join(jobs, n), mtime, mtime); err != nil { + t.Fatal(err) + } + } + } + return jobs +} + +func exists(t *testing.T, p string) bool { + t.Helper() + _, err := os.Stat(p) + return err == nil +} + +// TestSweepStaleMacOSClones_LiveAndGrace: the sweep removes only what is +// both unowned AND past the grace period. An old clone that a live VM owns +// stays (a long job's .img can go quiet), and so does a recent unowned one +// (it may belong to a VM registered a moment from now). +func TestSweepStaleMacOSClones_LiveAndGrace(t *testing.T) { + now := time.Date(2026, 10, 22, 12, 0, 0, 0, time.UTC) + data := t.TempDir() + jobs := cloneFixture(t, data, map[string]time.Time{ + "110392856702-a9e5c1a4": now.Add(-21 * 24 * time.Hour), // the 2026-10-01 leak: unowned, old → removed + "119992881741-825ce005": now.Add(-5 * time.Hour), // old but live (long job) → kept + "120000000001-00000001": now.Add(-30 * time.Minute), // unowned but recent → kept + }) + // Things outside jobs/ that must never be touched. + macos := filepath.Join(data, "vm", "macos") + for _, n := range []string{"base.img", "aux.bin"} { + if err := os.WriteFile(filepath.Join(macos, n), []byte("base"), 0o600); err != nil { + t.Fatal(err) + } + old := now.Add(-100 * 24 * time.Hour) + _ = os.Chtimes(filepath.Join(macos, n), old, old) + } + + live := map[string]bool{"119992881741-825ce005": true} + res := SweepStaleMacOSClones(data, live, MacOSCloneGrace, now, discardLog()) + if res.Removed != 3 || len(res.Failed) != 0 { + t.Fatalf("removed=%d failed=%v, want the leak's 3 entries (img, aux, share) and no failures", res.Removed, res.Failed) + } + if res.FreedBytes <= 0 { + t.Errorf("FreedBytes = %d, want the removed files' size", res.FreedBytes) + } + for _, n := range []string{"110392856702-a9e5c1a4", "110392856702-a9e5c1a4.img", "110392856702-a9e5c1a4.aux"} { + if exists(t, filepath.Join(jobs, n)) { + t.Errorf("stale unowned %s survived", n) + } + } + for _, id := range []string{"119992881741-825ce005", "120000000001-00000001"} { + for _, n := range []string{id, id + ".img", id + ".aux"} { + if !exists(t, filepath.Join(jobs, n)) { + t.Errorf("%s was swept; it is live or inside the grace period", n) + } + } + } + for _, n := range []string{"base.img", "aux.bin"} { + if !exists(t, filepath.Join(macos, n)) { + t.Errorf("%s outside jobs/ was touched", n) + } + } +} + +// TestSweepStaleMacOSClones_DoctorMode: no live set and no grace (daemon +// stopped) takes everything in jobs/, and still nothing outside it. +func TestSweepStaleMacOSClones_DoctorMode(t *testing.T) { + now := time.Now() + data := t.TempDir() + cloneFixture(t, data, map[string]time.Time{"a-1": now, "b-2": now.Add(-time.Hour)}) + base := filepath.Join(data, "vm", "macos", "base.img") + if err := os.WriteFile(base, []byte("base"), 0o600); err != nil { + t.Fatal(err) + } + if res := SweepStaleMacOSClones(data, nil, 0, now, discardLog()); res.Removed != 6 { + t.Errorf("removed %d, want all 6 entries", res.Removed) + } + if !exists(t, base) { + t.Error("base.img was removed") + } +} + +func TestSweepStaleMacOSClones_NoJobsDir(t *testing.T) { + if res := SweepStaleMacOSClones(t.TempDir(), nil, MacOSCloneGrace, time.Now(), discardLog()); res.Removed != 0 || res.Failed != nil { + t.Errorf("missing jobs dir: got %+v, want nothing", res) + } +} + +func TestCloneEntryID(t *testing.T) { + for name, want := range map[string]string{ + "119992881741-825ce005.img": "119992881741-825ce005", + "119992881741-825ce005.aux": "119992881741-825ce005", + "119992881741-825ce005": "119992881741-825ce005", + } { + if got := cloneEntryID(name); got != want { + t.Errorf("cloneEntryID(%q) = %q, want %q", name, got, want) + } + } +} + +// TestLiveMacOSVMRegistry: an id is protected from registration until its +// teardown unregisters it, and the snapshot is a copy. +func TestLiveMacOSVMRegistry(t *testing.T) { + const id = "test-live-registry-1" + registerMacVM(id) + snap := LiveMacOSVMIDs() + if !snap[id] { + t.Fatal("registered id missing from LiveMacOSVMIDs") + } + snap["injected"] = true + if LiveMacOSVMIDs()["injected"] { + t.Error("LiveMacOSVMIDs returned the registry itself, not a copy") + } + unregisterMacVM(id) + if LiveMacOSVMIDs()[id] { + t.Error("unregistered id still live") + } +} + +// TestTeardownMacVMFiles_WedgedStopStillRemovesFiles is the leak itself: a +// Virtualization.framework stop that never returns (#196) used to mean the +// clones were never deleted, because deletion came after it. Now they go at +// the bound, and the caller can still wait for the real stop. +func TestTeardownMacVMFiles_WedgedStopStillRemovesFiles(t *testing.T) { + dir := t.TempDir() + img, aux, share := filepath.Join(dir, "x.img"), filepath.Join(dir, "x.aux"), filepath.Join(dir, "x") + for _, p := range []string{img, aux} { + if err := os.WriteFile(p, []byte("clone"), 0o600); err != nil { + t.Fatal(err) + } + } + if err := os.MkdirAll(filepath.Join(share, "sub"), 0o755); err != nil { + t.Fatal(err) + } + + release := make(chan struct{}) + start := time.Now() + wedged, stopped := teardownMacVMFiles(func() { <-release }, 100*time.Millisecond, []string{img, aux}, []string{share}, discardLog()) + if !wedged { + t.Error("a stop that never returned was not reported as wedged") + } + if took := time.Since(start); took > 2*time.Second { + t.Errorf("teardown waited %s for a wedged stop; it must give up at the bound", took) + } + for _, p := range []string{img, aux, share} { + if exists(t, p) { + t.Errorf("%s survived a wedged stop", p) + } + } + select { + case <-stopped: + t.Fatal("stopped closed while the stop was still running") + default: + } + close(release) + select { + case <-stopped: + case <-time.After(2 * time.Second): + t.Fatal("stopped never closed after the stop returned") + } +} + +// TestMacVMStopBound_OutlastsGracefulShutdown: stopGuest gives the guest 15 s +// to shut down before forcing it. Removing the disk inside that window would +// pull it from under a guest that is still flushing, so the bound must clear +// it with room for the force stop. +func TestMacVMStopBound_OutlastsGracefulShutdown(t *testing.T) { + const gracefulWindow = 15 * time.Second + if macVMStopBound < gracefulWindow+10*time.Second { + t.Errorf("macVMStopBound = %s; it must outlast the %s graceful stop plus a force stop", macVMStopBound, gracefulWindow) + } +} + +func TestTeardownMacVMFiles_CleanStop(t *testing.T) { + dir := t.TempDir() + img := filepath.Join(dir, "y.img") + if err := os.WriteFile(img, []byte("clone"), 0o600); err != nil { + t.Fatal(err) + } + ran := false + // Missing paths and empty strings must be harmless: every exit path + // calls this, some before the files exist. + wedged, stopped := teardownMacVMFiles(func() { ran = true }, time.Minute, + []string{img, "", filepath.Join(dir, "never-created.aux")}, []string{"", filepath.Join(dir, "no-share")}, discardLog()) + <-stopped + if wedged || !ran { + t.Errorf("wedged=%v ran=%v, want a clean stop that ran", wedged, ran) + } + if exists(t, img) { + t.Error("clone survived a clean stop") + } +} diff --git a/pkg/vm/macos_disk.go b/pkg/vm/macos_disk.go index 05fadb7..4f25c9a 100644 --- a/pkg/vm/macos_disk.go +++ b/pkg/vm/macos_disk.go @@ -4,7 +4,6 @@ import ( "fmt" "log/slog" "os" - "path/filepath" "regexp" "strings" "time" @@ -140,51 +139,3 @@ if "$B" tap 2>/dev/null | grep -qx buildkite/buildkite; then "$B" untap --force // macOSBrewUntapTimeout bounds the untap. `brew tap` is a local listing, so a // slow one means something is wrong; give up rather than delay the job. const macOSBrewUntapTimeout = 60 * time.Second - -// SweepStaleMacOSClones deletes leftovers in /vm/macos/jobs — disk -// clones (.img), aux clones (.aux) and virtio-fs share directories -// (/) — whose modification time is older than olderThan. It returns how -// many entries it removed and the paths it failed to remove. -// -// A VM removes its own clones in Stop, but nothing removes them when Stop -// never runs: a daemon restart or crash mid-job, or a stop that wedged inside -// Virtualization.framework and was abandoned (scheduler.abandonVM). The -// guests die with the daemon process, their files do not, and each .img is a -// full-size clone that diverges from base.img as the job wrote to it — one -// from 2026-10-01 had grown to 38 GB on the fleet's Mac before anyone noticed. -// -// Called at startup, before macOS jobs are accepted, so no VM of this process -// can own anything here. The age gate is a second guard on top of that: a -// running guest keeps writing to its .img, so a live clone always looks -// recent. olderThan <= 0 removes everything (for `ephemerd doctor`, which -// only cleans with the daemon stopped). -func SweepStaleMacOSClones(dataDir string, olderThan time.Duration, now time.Time, log *slog.Logger) (removed int, failed []string) { - dir := filepath.Join(dataDir, "vm", "macos", "jobs") - entries, err := os.ReadDir(dir) - if err != nil { - if !os.IsNotExist(err) { - log.Warn("cannot list macOS VM clones for the stale sweep", "dir", dir, "error", err) - } - return 0, nil - } - for _, e := range entries { - info, err := e.Info() - if err != nil { - continue - } - if olderThan > 0 && now.Sub(info.ModTime()) < olderThan { - continue - } - path := filepath.Join(dir, e.Name()) - size := info.Size() - if err := os.RemoveAll(path); err != nil { - log.Warn("could not remove stale macOS VM clone", "path", path, "error", err) - failed = append(failed, path) - continue - } - removed++ - log.Info("removed stale macOS VM clone", "path", path, - "modified", info.ModTime().UTC().Format(time.RFC3339), "apparent_size_gib", fmt.Sprintf("%.1f", float64(size)/gib)) - } - return removed, failed -} diff --git a/pkg/vm/macos_disk_test.go b/pkg/vm/macos_disk_test.go index b92cb9c..ba3211a 100644 --- a/pkg/vm/macos_disk_test.go +++ b/pkg/vm/macos_disk_test.go @@ -142,61 +142,37 @@ func TestMacOSDiskGrowScript(t *testing.T) { } } -// TestSweepStaleMacOSClones: old leftovers (disk clone, aux clone, share -// dir) go; anything recently written — a live guest's clone — stays. -func TestSweepStaleMacOSClones(t *testing.T) { - data := t.TempDir() - jobs := filepath.Join(data, "vm", "macos", "jobs") - if err := os.MkdirAll(filepath.Join(jobs, "110392856702-a9e5c1a4"), 0o755); err != nil { - t.Fatal(err) - } - if err := os.MkdirAll(filepath.Join(jobs, "119992881741-825ce005"), 0o755); err != nil { - t.Fatal(err) - } - stale := []string{"110392856702-a9e5c1a4.img", "110392856702-a9e5c1a4.aux"} - fresh := []string{"119992881741-825ce005.img", "119992881741-825ce005.aux"} - for _, n := range append(append([]string{}, stale...), fresh...) { - if err := os.WriteFile(filepath.Join(jobs, n), []byte("x"), 0o600); err != nil { - t.Fatal(err) - } - } - now := time.Date(2026, 10, 22, 12, 0, 0, 0, time.UTC) - old := now.Add(-21 * 24 * time.Hour) - recent := now.Add(-time.Hour) - for _, n := range append(stale, "110392856702-a9e5c1a4") { - if err := os.Chtimes(filepath.Join(jobs, n), old, old); err != nil { - t.Fatal(err) - } - } - for _, n := range append(fresh, "119992881741-825ce005") { - if err := os.Chtimes(filepath.Join(jobs, n), recent, recent); err != nil { - t.Fatal(err) - } +// TestMacOSGuestTuning_Bounded: the guest-side steps run inside the macOS +// provisioning deadline (5m by default), so a wedged diskutil or brew can +// only cost the job its extra disk or a warning — never the job. +func TestMacOSGuestTuning_Bounded(t *testing.T) { + const provisionDeadline = 5 * time.Minute + // info + grow + untap, all before the runner starts. + if total := 90*time.Second + macOSDiskGrowTimeout + macOSBrewUntapTimeout; total >= provisionDeadline { + t.Errorf("guest tuning can take %s, not inside the %s provisioning deadline", total, provisionDeadline) + } + if !strings.Contains(macOSDiskInfoCmd, "diskutil info /System/Volumes/Data") || !strings.Contains(macOSDiskInfoCmd, "|| diskutil info /") { + t.Errorf("info command must ask the Data volume and fall back to /: %q", macOSDiskInfoCmd) } +} - removed, failed := SweepStaleMacOSClones(data, 24*time.Hour, now, discardLog()) - if removed != 3 || len(failed) != 0 { - t.Errorf("removed=%d failed=%v, want 3 removed (img, aux, share dir) and no failures", removed, failed) - } - for _, n := range append(stale, "110392856702-a9e5c1a4") { - if _, err := os.Stat(filepath.Join(jobs, n)); !os.IsNotExist(err) { - t.Errorf("stale %s survived the sweep", n) - } +// TestMacOSBrewUntapScript: brew runs as the guest's admin user (it refuses +// root, and the tap belongs to admin), never auto-updates (that would make +// every job reach the network for a cosmetic step), and only the buildkite +// tap is touched. +func TestMacOSBrewUntapScript(t *testing.T) { + if strings.Contains(macOSBrewUntapScript, "sudo") { + t.Error("brew must not run as root") } - for _, n := range append(fresh, "119992881741-825ce005") { - if _, err := os.Stat(filepath.Join(jobs, n)); err != nil { - t.Errorf("recent %s was swept: %v", n, err) + for _, want := range []string{ + "HOMEBREW_NO_AUTO_UPDATE=1", + "grep -qx buildkite/buildkite", + `untap --force buildkite/buildkite`, + "echo no-brew; exit 0", + } { + if !strings.Contains(macOSBrewUntapScript, want) { + t.Errorf("untap script missing %q", want) } } - - // olderThan <= 0 (doctor, daemon stopped) takes everything. - if removed, _ := SweepStaleMacOSClones(data, 0, now, discardLog()); removed != 3 { - t.Errorf("unconditional sweep removed %d, want the 3 remaining", removed) - } } -func TestSweepStaleMacOSClones_NoJobsDir(t *testing.T) { - if removed, failed := SweepStaleMacOSClones(t.TempDir(), 24*time.Hour, time.Now(), discardLog()); removed != 0 || failed != nil { - t.Errorf("missing jobs dir: got (%d, %v), want (0, nil)", removed, failed) - } -} diff --git a/pkg/vm/macosvm_darwin.go b/pkg/vm/macosvm_darwin.go index 98c88d1..2d5425e 100644 --- a/pkg/vm/macosvm_darwin.go +++ b/pkg/vm/macosvm_darwin.go @@ -53,6 +53,9 @@ func NewMacOSVM(cfg MacOSVMConfig, jobID string) (MacOSVM, error) { jobDir := filepath.Join(cfg.DataDir, "vm", "macos", "jobs", jobID) + // Owned from here until stop() has removed everything (see macos_clones.go): + // the stale-clone sweep must never touch this id's files in between. + registerMacVM(jobID) return &darwinMacOSVM{ cfg: cfg, id: jobID, @@ -86,7 +89,7 @@ func (m *darwinMacOSVM) WriteJITConfig(encodedJIT string) error { return nil } -func (m *darwinMacOSVM) Start(ctx context.Context) error { +func (m *darwinMacOSVM) Start(ctx context.Context) (err error) { // Clone-on-write copy of the base image for this job. // APFS clones are near-instant — no data is copied until writes occur. cloneDir := filepath.Join(m.cfg.DataDir, "vm", "macos", "jobs") @@ -96,6 +99,15 @@ func (m *darwinMacOSVM) Start(ctx context.Context) error { m.clonePath = filepath.Join(cloneDir, m.id+".img") m.auxPath = filepath.Join(cloneDir, m.id+".aux") + // A boot that fails after the clones exist removes them here, without + // relying on the caller's Stop. Before vm.Start there is no guest that + // could have them open, so this is plain cleanup; stop() repeats it + // harmlessly. + defer func() { + if err != nil && m.vm == nil { + removeMacVMFiles([]string{m.clonePath, m.auxPath}, nil, m.cfg.Log) + } + }() // Use APFS clone (cp -c) for instant copy-on-write if err := apfsCopy(ctx, m.cfg.DiskImage, m.clonePath, m.cfg.Log); err != nil { @@ -943,44 +955,45 @@ func (m *darwinMacOSVM) Stop() { func (m *darwinMacOSVM) stop() { m.cfg.Log.Info("stopping macOS VM", "id", m.id) - if m.vm != nil { - if m.vm.CanRequestStop() { - if _, err := m.vm.RequestStop(); err != nil { - m.cfg.Log.Debug("RequestStop failed", "id", m.id, "error", err) - } - } - - select { - case <-m.done: - case <-time.After(15 * time.Second): - m.cfg.Log.Warn("macOS VM did not stop gracefully, forcing", "id", m.id) - if err := m.vm.Stop(); err != nil { - m.cfg.Log.Warn("force stop failed", "id", m.id, "error", err) - } - } - } + // The clones and the share go whether or not Virtualization.framework + // ever lets go of the guest: when this used to wait for the stop and + // delete afterwards, every wedged stop (#196) leaked a full disk clone + // for good. See macos_clones.go. + // + // Stop itself still returns only once the guest is really stopped. The + // scheduler's abandoned-VM accounting depends on that: a Stop that + // returns is how it learns a guest stopped holding RAM, and returning + // early for a wedged guest would let the cap admit VMs the host cannot + // hold. + _, stopped := teardownMacVMFiles(m.stopGuest, macVMStopBound, + []string{m.clonePath, m.auxPath}, []string{m.jobDir}, m.cfg.Log) + <-stopped if m.cancel != nil { m.cancel() } + unregisterMacVM(m.id) + m.cfg.Log.Info("macOS VM destroyed", "id", m.id) +} - // Delete the clones - for _, path := range []string{m.clonePath, m.auxPath} { - if path != "" { - if err := os.Remove(path); err != nil && !os.IsNotExist(err) { - m.cfg.Log.Warn("failed to remove VM clone", "path", path, "error", err) - } +// stopGuest asks the guest to shut down and forces it after 15 s. +func (m *darwinMacOSVM) stopGuest() { + if m.vm == nil { + return + } + if m.vm.CanRequestStop() { + if _, err := m.vm.RequestStop(); err != nil { + m.cfg.Log.Debug("RequestStop failed", "id", m.id, "error", err) } } - - // Clean up the job shared directory - if m.jobDir != "" { - if err := os.RemoveAll(m.jobDir); err != nil { - m.cfg.Log.Warn("failed to remove job directory", "path", m.jobDir, "error", err) + select { + case <-m.done: + case <-time.After(15 * time.Second): + m.cfg.Log.Warn("macOS VM did not stop gracefully, forcing", "id", m.id) + if err := m.vm.Stop(); err != nil { + m.cfg.Log.Warn("force stop failed", "id", m.id, "error", err) } } - - m.cfg.Log.Info("macOS VM destroyed", "id", m.id) } // apfsCopy uses APFS clone-on-write (cp -c) for instant copies, falling