diff --git a/cmd/ephemerd/doctor_darwin.go b/cmd/ephemerd/doctor_darwin.go index bb5f8a9..8433fa6 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. + 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 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 6997429..1ed2026 100644 --- a/cmd/ephemerd/main.go +++ b/cmd/ephemerd/main.go @@ -922,12 +922,13 @@ func serve(ctx context.Context, configFile, imagesDirFlag string, containerdTCPP return } 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/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 new file mode 100644 index 0000000..4f25c9a --- /dev/null +++ b/pkg/vm/macos_disk.go @@ -0,0 +1,141 @@ +package vm + +import ( + "fmt" + "log/slog" + "os" + "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 diff --git a/pkg/vm/macos_disk_test.go b/pkg/vm/macos_disk_test.go new file mode 100644 index 0000000..ba3211a --- /dev/null +++ b/pkg/vm/macos_disk_test.go @@ -0,0 +1,178 @@ +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) + } + } +} + +// 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) + } +} + +// 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 _, 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) + } + } +} + diff --git a/pkg/vm/macosvm_darwin.go b/pkg/vm/macosvm_darwin.go index d41fa38..2d5425e 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. @@ -49,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, @@ -82,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") @@ -92,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 { @@ -102,6 +118,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 +740,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 +884,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) } @@ -865,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 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.