diff --git a/cgroup/cgroup_test.go b/cgroup/cgroup_test.go index e7304975..87ebfbd9 100644 --- a/cgroup/cgroup_test.go +++ b/cgroup/cgroup_test.go @@ -292,7 +292,7 @@ func TestPlaceScopeReadGate(t *testing.T) { } } -func TestArmSetsQuotaThenBurst(t *testing.T) { +func TestArmWritesQuotaAndBurst(t *testing.T) { parent := t.TempDir() dir := ScopeDir(parent, "A") if err := os.Mkdir(dir, 0o755); err != nil { diff --git a/cmd/core/metering_test.go b/cmd/core/metering_test.go index 2297ada7..cf13337a 100644 --- a/cmd/core/metering_test.go +++ b/cmd/core/metering_test.go @@ -8,6 +8,7 @@ import ( "github.com/cocoonstack/cocoon/metering" meteringfile "github.com/cocoonstack/cocoon/metering/file" meteringstderr "github.com/cocoonstack/cocoon/metering/stderr" + "github.com/cocoonstack/cocoon/utils" ) func TestBuildRecorderBackends(t *testing.T) { @@ -38,15 +39,19 @@ func TestBuildRecorderBackends(t *testing.T) { func TestBuildFileRecorderCustomPath(t *testing.T) { dir := t.TempDir() + path := dir + "/custom/ledger.jsonl" conf := &config.Config{ RootDir: dir, Metering: config.MeteringConfig{ Backend: config.MeteringFile, - File: config.FileMeteringConfig{Path: dir + "/custom/ledger.jsonl"}, + File: config.FileMeteringConfig{Path: path}, }, } r := buildRecorder(t.Context(), conf) if _, ok := r.(*meteringfile.Recorder); !ok { t.Fatalf("got %T, want *meteringfile.Recorder", r) } + if !utils.FileExists(path) { + t.Errorf("ledger %s not created at the configured path", path) + } } diff --git a/cmd/meta/convert/convert_test.go b/cmd/meta/convert/convert_test.go index 4c5d30a3..7941bd03 100644 --- a/cmd/meta/convert/convert_test.go +++ b/cmd/meta/convert/convert_test.go @@ -108,6 +108,11 @@ func TestResumeClaimsCommittedTarget(t *testing.T) { if err := copyNamespace(ctx, src, dst, spec.Decls[0]); err != nil { t.Fatalf("copy: %v", err) } + if err := src.Update(ctx, meta.Scope{Write: "vms"}, meta.CommitDurable, func(w meta.Writer) error { + return w.PutRaw(ctx, "vms", "records", "late", jsontext.Value(`{}`), false) + }); err != nil { + t.Fatalf("mutate source: %v", err) + } _ = src.Close() _ = dst.Close() diff --git a/cmd/vm/logstream_test.go b/cmd/vm/logstream_test.go index de5057f7..69f25a4a 100644 --- a/cmd/vm/logstream_test.go +++ b/cmd/vm/logstream_test.go @@ -40,13 +40,15 @@ func TestWatchFileDebounce(t *testing.T) { t.Fatal(err) } - ch, err := watchFile(t.Context(), target, 2*time.Second) + const debounce = 200 * time.Millisecond + ch, err := watchFile(t.Context(), target, debounce) if err != nil { t.Fatal(err) } + start := time.Now() for i := range 5 { - if err := utils.AtomicWriteFile(target, []byte{byte('0' + i)}, 0o644, utils.Sync); err != nil { + if err := utils.AtomicWriteFile(target, []byte{byte('0' + i)}, 0o644, utils.NoSync); err != nil { t.Fatal(err) } } @@ -56,11 +58,14 @@ func TestWatchFileDebounce(t *testing.T) { case <-time.After(10 * time.Second): t.Fatal("timed out waiting for debounced signal") } + if elapsed := time.Since(start); elapsed < debounce { + t.Fatalf("signal after %s, want it held for the %s debounce", elapsed, debounce) + } select { case <-ch: t.Fatal("unexpected second signal within debounce window") - case <-time.After(500 * time.Millisecond): + case <-time.After(debounce / 2): } } diff --git a/daemon/cache_test.go b/daemon/cache_test.go index 18bea2dd..008b600f 100644 --- a/daemon/cache_test.go +++ b/daemon/cache_test.go @@ -35,7 +35,10 @@ func TestCacheEmitsNothingForAnUnchangedPass(t *testing.T) { events, release := c.subscribe() defer release() - c.publish([]VMStatus{st}, true, 0, time.Now()) + again := st + again.VerifiedAt = st.VerifiedAt.Add(time.Second) + again.WatchedPID = 99 + c.publish([]VMStatus{again}, true, 0, time.Now()) if got := collect(t, events); len(got) != 0 { t.Errorf("got %+v, want no events when nothing a consumer acts on changed", got) } diff --git a/daemon/reconcile_test.go b/daemon/reconcile_test.go index 5bc3a1ea..8d2bd5f0 100644 --- a/daemon/reconcile_test.go +++ b/daemon/reconcile_test.go @@ -19,7 +19,7 @@ func TestReconcileConvergesDeadRunningRecord(t *testing.T) { f := newFake().put(runningRec("vm1", 7)) newTestDaemon(t, f).reconcile(t.Context()) - if len(f.converged) != 1 || f.converged[0] != (convergeCall{vmID: "vm1", gen: 7}) { + if len(f.converged) != 1 || f.converged[0].vmID != "vm1" || f.converged[0].gen != 7 { t.Fatalf("got %+v, want one convergence fenced on generation 7", f.converged) } } @@ -142,6 +142,9 @@ func TestHandleExitConvergesWatchedGeneration(t *testing.T) { if len(f.converged) != 1 || f.converged[0].gen != 4 { t.Fatalf("got %+v, want one convergence at generation 4", f.converged) } + if !f.converged[0].at.Equal(at) { + t.Errorf("got stop dated %v, want the watcher's observation %v", f.converged[0].at, at) + } } func TestHandleExitDropsStaleGeneration(t *testing.T) { @@ -218,6 +221,7 @@ func TestPassCountsDegradedVMsWithoutGoingUnready(t *testing.T) { type convergeCall struct { vmID string gen uint64 + at time.Time } type fakeSupervisor struct { @@ -283,12 +287,12 @@ func (f *fakeSupervisor) PeekRecord(_ context.Context, vmID string) (*hypervisor return f.records[vmID], nil } -func (f *fakeSupervisor) ConvergeDead(_ context.Context, vmID string, gen uint64, _ time.Time) error { +func (f *fakeSupervisor) ConvergeDead(_ context.Context, vmID string, gen uint64, at time.Time) error { f.mu.Lock() defer f.mu.Unlock() rec := f.records[vmID] if rec != nil && hypervisor.NeedsDeadConvergence(rec) && rec.TransitionGeneration == gen { - f.converged = append(f.converged, convergeCall{vmID: vmID, gen: gen}) + f.converged = append(f.converged, convergeCall{vmID: vmID, gen: gen, at: at}) rec.State = types.VMStateStopped rec.TransitionGeneration++ } diff --git a/docs/daemon.md b/docs/daemon.md index 6c36aaca..a1648af3 100644 --- a/docs/daemon.md +++ b/docs/daemon.md @@ -74,6 +74,8 @@ a rotation, or run the `meta` metering backend instead. The `file` backend only ever appends: cocoon never rotates, truncates, or prunes the ledger, so it grows without bound until an external rotation trims it. +Each entry is written after the state change it reports commits. A SIGKILL between the two loses that entry and a retry does not write it again, and a retried restore can repeat a stop, so downstream must tolerate an unpaired or repeated endpoint. + `--gc-interval` runs the same sweep as a bare `cocoon gc` — orphans and stale records only. It never evicts healthy snapshots by access recency: the LRU policy — including the `--snapshot-size` storage cap — comes from the `gc --snapshot*` diff --git a/docs/known-issues.md b/docs/known-issues.md index b1d4992f..61af075a 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -73,6 +73,12 @@ On x86_64, cocoon uses [rust-hypervisor-firmware](https://github.com/cloud-hyper This is an upstream issue tracked in [rust-hypervisor-firmware#333](https://github.com/cloud-hypervisor/rust-hypervisor-firmware/issues/333) and [cloud-hypervisor#7356](https://github.com/cloud-hypervisor/cloud-hypervisor/issues/7356). As a workaround, use **OCI VM images** for Ubuntu 24.04 — OCI images use direct kernel boot and are not affected. +## Data disk fsck sends an existing VM to the emergency shell + +Cidata from v0.7.6 and earlier gives a mounted data disk fstab pass number 2. If the host's `mke2fs` formats with a feature the guest's `e2fsck` does not know, such as `orphan_file` from a Debian 13 host with an Ubuntu 22.04 guest, the next boot fails the fsck and enters the emergency shell with no network and no sshd, despite `nofail`. New cidata writes pass number 0. + +**Recovery**: log in through `cocoon vm console`, set the last field of the disk's `/etc/fstab` line to 0, then stop and start the VM. + ## Named netns path removed while a VM runs cocoon never removes a running VM's named network namespace. If an operator or a foreign GC runs `ip netns del`, it removes the name under `/var/run/netns`; the running VMM still holds the original namespace and its TAPs. Existing traffic may continue, and `vm list` still shows the recorded IP. NIC hot-add cannot open the missing netns path, while `cocoon vm exec` can still work over vsock. @@ -171,6 +177,8 @@ Cloud Hypervisor uses a GED (Generic Event Device, `ACPI0013`) to deliver power- **Timing caveat**: the Windows ACPI shutdown handler needs ~60 seconds from cold boot to fully initialize (SAC appears at ~30s, handler ready ~30s later). Stopping a Windows VM before the handler is ready triggers the 30s `stop_timeout_seconds` fallback and escalates to force-kill. Clone-restored VMs inherit the ready ACPI state and shut down in ~8-13s immediately. +**Background servicing**: after about 15 minutes of uptime, Windows runs maintenance such as `StartComponentCleanup`, and shutdown waits for `TrustedInstaller` to stop. In testing a cold-booted VM took 20s and a clone 73s to power off, with the power-button event logged at once, so `vm stop` can reach the 30s `stop_timeout_seconds` and force-kill. Raise `stop_timeout_seconds` for a Windows VM that must stop cleanly. + **Previous consequence** (no longer applies with our firmware fork): `cocoon vm stop` always timed out on Windows VMs (default 30s), then fell back to `vm.shutdown` → SIGTERM → SIGKILL. **Previous workaround** (no longer needed with our firmware fork): shut down Windows guests via SSH or WinRM before stopping: diff --git a/docs/vm.md b/docs/vm.md index 7a231a4c..0d0c46a2 100644 --- a/docs/vm.md +++ b/docs/vm.md @@ -157,7 +157,7 @@ cocoon vm run --data-disk size=20G,name=raw,fstype=none ### Snapshot/Clone/Restore -Phase 1 inherits data disks 1:1: snapshot reflinks each `data-.raw` into the snapshot tar, clone re-creates them under the new VM's runDir (and regenerates cidata so cloud-init re-mounts on the new identity), and restore rolls all data disks back to the snapshot timepoint along with the rootfs and memory state. Cloud Hypervisor clones can additionally CREATE fresh data disks at clone time via `--data-disk` (hot-added after restore — the snapshot's device tree itself cannot grow); clone-created disks are hot-added after cidata is regenerated, so they are never auto-mounted — `mount=` has no effect on `vm clone --data-disk`; mount them inside the guest. Removing inherited disks at clone time is not supported, and Firecracker clones accept `--data-disk` only from a `--pci` snapshot (MMIO cannot hot-plug). +Phase 1 inherits data disks 1:1: snapshot reflinks each `data-.raw` into the snapshot tar, clone re-creates them under the new VM's runDir (and regenerates cidata so cloud-init re-mounts on the new identity), and restore rolls all data disks back to the snapshot timepoint along with the rootfs and memory state. Cloud Hypervisor clones can additionally CREATE fresh data disks at clone time via `--data-disk` (hot-added after restore — the snapshot's device tree itself cannot grow); clone-created disks are hot-added after cidata is regenerated, so that clone does not auto-mount them — `mount=` has no effect on it; mount them inside the guest. A clone of that clone inherits the disk like any other: its regenerated cidata lists the mount, which applies when the [post-clone guest setup](snapshots.md#post-clone-guest-setup) commands run. Removing inherited disks at clone time is not supported, and Firecracker clones accept `--data-disk` only from a `--pci` snapshot (MMIO cannot hot-plug). Restore preflight verifies sidecar integrity, file presence (vmstate, memory, COW, every `data-*.raw` — presence, not size; `--from-dir` and `snapshot import` separately check the COW size against the envelope), per-index Path/RO agreement between the sidecar and CH config.json, Role/Serial agreement between the sidecar and the VM record, and on Firecracker that every COW and data disk is recorded at this VM's own path, all **before** killing the running VM, so a malformed or imported snapshot fails fast and leaves the live VM untouched. diff --git a/hypervisor/cgroupgc.go b/hypervisor/cgroupgc.go index 27e677a8..a0a9fbca 100644 --- a/hypervisor/cgroupgc.go +++ b/hypervisor/cgroupgc.go @@ -14,6 +14,8 @@ import ( "github.com/cocoonstack/cocoon/utils" ) +var removeScope = cgroup.RemoveEmpty + // removeCgroupScope reclaims id's scope after its VMM is confirmed dead; failure is left for the next converge/GC pass. func (b *Backend) removeCgroupScope(ctx context.Context, id string) { if err := cgroup.Remove(ctx, b.Conf.CgroupParentDir(), id); err != nil { @@ -42,7 +44,7 @@ func CgroupGCModule(parentDir string, vmInUse network.VMInUse) gc.Module[[]strin continue } // A scope populated since the VM snapshot was read belongs to a launch in flight, not to an orphan - err := cgroup.RemoveEmpty(parentDir, id) + err := removeScope(parentDir, id) if errors.Is(err, syscall.EBUSY) { logger.Infof(ctx, "skipped scope vm-%s reason=populated", id) continue diff --git a/hypervisor/cgroupgc_test.go b/hypervisor/cgroupgc_test.go index b76020b1..b70599c3 100644 --- a/hypervisor/cgroupgc_test.go +++ b/hypervisor/cgroupgc_test.go @@ -2,9 +2,10 @@ package hypervisor import ( "context" + "io/fs" "os" - "path/filepath" "slices" + "syscall" "testing" "github.com/cocoonstack/cocoon/cgroup" @@ -66,19 +67,27 @@ func TestCgroupGCModuleLeavesScopeOfAVMCreatedAfterTheSnapshot(t *testing.T) { } } -func TestCgroupGCModuleLeavesPopulatedScope(t *testing.T) { - parent := t.TempDir() - dir := cgroup.ScopeDir(parent, "BUSY") - if err := os.MkdirAll(filepath.Join(dir, "child"), 0o755); err != nil { - t.Fatalf("setup: %v", err) - } +func TestCgroupGCModuleCollectRemoveFailures(t *testing.T) { + for _, tt := range []struct { + name string + errno syscall.Errno + wantErr bool + }{ + {name: "kernel refuses a populated scope", errno: syscall.EBUSY}, + {name: "any other refusal surfaces", errno: syscall.ENOTEMPTY, wantErr: true}, + } { + t.Run(tt.name, func(t *testing.T) { + orig := removeScope + removeScope = func(string, string) error { + return &fs.PathError{Op: "remove", Path: "vm-BUSY.scope", Err: tt.errno} + } + t.Cleanup(func() { removeScope = orig }) - m := CgroupGCModule(parent, noVMInUse) - if err := m.Collect(t.Context(), []string{"BUSY"}, nil); err == nil { - t.Error("want error for populated scope, got nil") - } - if _, err := os.Stat(dir); err != nil { - t.Errorf("populated scope removed: %v", err) + err := CgroupGCModule(t.TempDir(), noVMInUse).Collect(t.Context(), []string{"BUSY"}, nil) + if (err != nil) != tt.wantErr { + t.Errorf("Collect = %v, wantErr %v", err, tt.wantErr) + } + }) } } diff --git a/hypervisor/cloudhypervisor/restore.go b/hypervisor/cloudhypervisor/restore.go index 3ae1ba87..56dcbf41 100644 --- a/hypervisor/cloudhypervisor/restore.go +++ b/hypervisor/cloudhypervisor/restore.go @@ -130,9 +130,6 @@ func resolveRestoreMode(ctx context.Context, mode string, mem chMemory) string { // vm.restore replays the snapshot's guest MACs verbatim, so drifted NIC identity must fail first. func validateRestoreNICs(chCfg *chVMConfig, rec *hypervisor.VMRecord) error { - if len(chCfg.Nets) != len(rec.NetworkConfigs) { - return fmt.Errorf("snapshot has %d NICs, vm has %d; NIC identity must match for restore", len(chCfg.Nets), len(rec.NetworkConfigs)) - } macs := make([]string, len(chCfg.Nets)) for i, n := range chCfg.Nets { macs[i] = n.MAC diff --git a/hypervisor/cloudhypervisor/snapshot.go b/hypervisor/cloudhypervisor/snapshot.go index d90d73eb..b7f9e145 100644 --- a/hypervisor/cloudhypervisor/snapshot.go +++ b/hypervisor/cloudhypervisor/snapshot.go @@ -46,6 +46,9 @@ func (ch *CloudHypervisor) snapshotSpec(ctx context.Context) hypervisor.Snapshot if err := hotAttachedError(info.Config.Fs, info.Config.Devices, info.Config.Disks); err != nil { return err } + if len(info.Config.Nets) != len(rec.NetworkConfigs) { + return fmt.Errorf("vm %s has %d NICs but its record has %d; rerun vm net first", rec.ID, len(info.Config.Nets), len(rec.NetworkConfigs)) + } if err := waitBalloonSettled(ctx, hc, rec.ID, info); err != nil { return err } diff --git a/hypervisor/cloudhypervisor/snapshot_test.go b/hypervisor/cloudhypervisor/snapshot_test.go index 1af79eb2..209fc2e4 100644 --- a/hypervisor/cloudhypervisor/snapshot_test.go +++ b/hypervisor/cloudhypervisor/snapshot_test.go @@ -14,6 +14,7 @@ import ( "time" "github.com/cocoonstack/cocoon/hypervisor" + "github.com/cocoonstack/cocoon/types" ) func TestSnapshotPauseRefusesHotAttachedBeforePausing(t *testing.T) { @@ -60,6 +61,30 @@ func TestSnapshotPauseRefusesHotAttachedBeforePausing(t *testing.T) { } } +func TestSnapshotPauseRefusesANICCountOffItsRecord(t *testing.T) { + pauses := 0 + mux := http.NewServeMux() + mux.HandleFunc("/api/v1/vm.info", func(w http.ResponseWriter, _ *http.Request) { + _ = json.MarshalWrite(w, chVMInfoResponse{State: chStateRunning, Config: chVMConfig{Nets: []chNet{{ID: "net0"}}}}) + }) + mux.HandleFunc("/api/v1/vm.pause", func(w http.ResponseWriter, _ *http.Request) { + pauses++ + w.WriteHeader(http.StatusNoContent) + }) + hc := &http.Client{Transport: handlerTransport{mux}} + rec := &hypervisor.VMRecord{} + rec.NetworkConfigs = []*types.NetworkConfig{{}, {}} + + err := (&CloudHypervisor{}).snapshotSpec(t.Context()).Pause(rec, hc) + + if err == nil || !strings.Contains(err.Error(), "rerun vm net") { + t.Fatalf("err = %v, want the capture refused until vm net converges", err) + } + if pauses != 0 { + t.Fatalf("pauses = %d, want the VM left running", pauses) + } +} + func TestSnapshotPauseWaitsForBalloonToSettle(t *testing.T) { const ( mem = int64(8 << 30) diff --git a/hypervisor/firecracker/create.go b/hypervisor/firecracker/create.go index ce201632..cb678477 100644 --- a/hypervisor/firecracker/create.go +++ b/hypervisor/firecracker/create.go @@ -52,6 +52,43 @@ func EnsureVmlinuxBoot(boot *types.BootConfig) error { return nil } +// ensureVmlinux decompresses kernelPath if needed and returns the ELF path. +func ensureVmlinux(kernelPath string) (string, error) { + f, err := os.Open(kernelPath) //nolint:gosec + if err != nil { + return "", fmt.Errorf("open kernel: %w", err) + } + var magic [4]byte + _, err = io.ReadFull(f, magic[:]) + _ = f.Close() + if err != nil { + return "", fmt.Errorf("read kernel magic: %w", err) + } + if bytes.Equal(magic[:], elfMagic) { + return kernelPath, nil + } + + vmlinuxPath := filepath.Join(filepath.Dir(kernelPath), "vmlinux") + if utils.FileExists(vmlinuxPath) { + return vmlinuxPath, nil + } + + data, err := os.ReadFile(kernelPath) //nolint:gosec + if err != nil { + return "", fmt.Errorf("read kernel: %w", err) + } + + decompressed, decompErr := decompressKernel(data) + if decompErr != nil { + return "", fmt.Errorf("decompress kernel from %s: %w (FC requires uncompressed ELF kernel)", kernelPath, decompErr) + } + + if err := utils.AtomicWriteFile(vmlinuxPath, decompressed, 0o600, utils.Sync); err != nil { + return "", fmt.Errorf("write vmlinux: %w", err) + } + return vmlinuxPath, nil +} + func decompressKernel(data []byte) ([]byte, error) { type kernelCodec struct { name string @@ -122,40 +159,3 @@ func devPath(idx int) string { } return fmt.Sprintf("/dev/vd%c%c", 'a'+(idx/letters)-1, 'a'+idx%letters) } - -// ensureVmlinux decompresses kernelPath if needed and returns the ELF path. -func ensureVmlinux(kernelPath string) (string, error) { - f, err := os.Open(kernelPath) //nolint:gosec - if err != nil { - return "", fmt.Errorf("open kernel: %w", err) - } - var magic [4]byte - _, err = io.ReadFull(f, magic[:]) - _ = f.Close() - if err != nil { - return "", fmt.Errorf("read kernel magic: %w", err) - } - if bytes.Equal(magic[:], elfMagic) { - return kernelPath, nil - } - - vmlinuxPath := filepath.Join(filepath.Dir(kernelPath), "vmlinux") - if utils.FileExists(vmlinuxPath) { - return vmlinuxPath, nil - } - - data, err := os.ReadFile(kernelPath) //nolint:gosec - if err != nil { - return "", fmt.Errorf("read kernel: %w", err) - } - - decompressed, decompErr := decompressKernel(data) - if decompErr != nil { - return "", fmt.Errorf("decompress kernel from %s: %w (FC requires uncompressed ELF kernel)", kernelPath, decompErr) - } - - if err := utils.AtomicWriteFile(vmlinuxPath, decompressed, 0o600, utils.Sync); err != nil { - return "", fmt.Errorf("write vmlinux: %w", err) - } - return vmlinuxPath, nil -} diff --git a/hypervisor/firecracker/restore_test.go b/hypervisor/firecracker/restore_test.go index 399c6daf..970d5f00 100644 --- a/hypervisor/firecracker/restore_test.go +++ b/hypervisor/firecracker/restore_test.go @@ -73,6 +73,7 @@ func TestPreflightRestoreRefusesDriftedNICs(t *testing.T) { {name: "MAC case differs", rec: recordWith(strings.ToUpper(snapMAC))}, {name: "drifted MAC", rec: recordWith("02:00:00:00:00:02"), wantErr: "nic 0 MAC drifted"}, {name: "NIC removed", rec: recordWith(), wantErr: "NIC identity must match"}, + {name: "NIC added after the snapshot", rec: recordWith(snapMAC, "02:00:00:00:00:03"), wantErr: "NIC identity must match"}, } { t.Run(tt.name, func(t *testing.T) { err := fc.preflightRestore(srcDir, tt.rec) @@ -88,6 +89,17 @@ func TestPreflightRestoreRefusesDriftedNICs(t *testing.T) { }) } + meta.NICMACs = []string{} + if err := snapshot.SaveSnapshotMeta(srcDir, meta); err != nil { + t.Fatalf("write sidecar: %v", err) + } + if err := fc.preflightRestore(srcDir, recordWith()); err != nil { + t.Fatalf("preflight of a zero-NIC snapshot: %v", err) + } + if err := fc.preflightRestore(srcDir, recordWith(snapMAC)); err == nil || !strings.Contains(err.Error(), "NIC identity must match") { + t.Fatalf("err = %v, want the NIC added after a zero-NIC snapshot refused", err) + } + meta.NICMACs = nil if err := snapshot.SaveSnapshotMeta(srcDir, meta); err != nil { t.Fatalf("write sidecar: %v", err) diff --git a/hypervisor/hibernate_test.go b/hypervisor/hibernate_test.go index 88f9e326..044e2285 100644 --- a/hypervisor/hibernate_test.go +++ b/hypervisor/hibernate_test.go @@ -215,10 +215,7 @@ func newHibernateTestVM(t *testing.T) (*Backend, string) { if err := os.WriteFile(sock, nil, 0o600); err != nil { t.Fatalf("touch sock: %v", err) } - cmd := exec.Command("bash", "-c", fmt.Sprintf("exec -a %s tail -f /dev/null %q", b.Conf.BinaryName(), sock)) - if err := cmd.Start(); err != nil { - t.Fatalf("start stub vmm: %v", err) - } + cmd := startFakeVMM(t, b.Conf.BinaryName(), sock) waitDone := make(chan struct{}) go func() { @@ -230,17 +227,30 @@ func newHibernateTestVM(t *testing.T) (*Backend, string) { <-waitDone }) - if err := utils.WaitFor(t.Context(), 2*time.Second, 10*time.Millisecond, func() (bool, error) { - return utils.VerifyProcessCmdline(cmd.Process.Pid, sock), nil - }); err != nil { - t.Fatalf("stub vmm did not exec in time: %v", err) - } if err := os.WriteFile(b.PIDFilePath(runDir), []byte(strconv.Itoa(cmd.Process.Pid)), 0o600); err != nil { t.Fatalf("write pidfile: %v", err) } return b, id } +func startFakeVMM(t *testing.T, argv0, sock string) *exec.Cmd { + t.Helper() + cmd := exec.Command("bash", "-c", fmt.Sprintf("exec -a %s tail -f /dev/null %q", argv0, sock)) + if err := cmd.Start(); err != nil { + t.Fatalf("start fake VMM: %v", err) + } + t.Cleanup(func() { + _ = cmd.Process.Kill() + _ = cmd.Wait() + }) + if err := utils.WaitFor(t.Context(), 5*time.Second, time.Millisecond, func() (bool, error) { + return utils.VerifyProcessCmdline(cmd.Process.Pid, sock), nil + }); err != nil { + t.Fatalf("fake VMM never execed: %v", err) + } + return cmd +} + func hibernateStubSpec(calls *hibernateCalls) HibernateSpec { return HibernateSpec{ Pause: func(*VMRecord, *http.Client) error { return nil }, diff --git a/hypervisor/inspect_linux_test.go b/hypervisor/inspect_linux_test.go index 846cc953..0a2b3fc1 100644 --- a/hypervisor/inspect_linux_test.go +++ b/hypervisor/inspect_linux_test.go @@ -2,12 +2,8 @@ package hypervisor import ( "os" - "os/exec" "strconv" "testing" - "time" - - "github.com/cocoonstack/cocoon/utils" ) func TestToVMDropsARecycledPID(t *testing.T) { @@ -32,20 +28,7 @@ func TestToVMKeepsTheVMMPID(t *testing.T) { t.Fatalf("load record: %v", err) } sock := SocketPath(rec.RunDir) - vmm := exec.Command("/bin/sh", "-c", "sleep 60; :", "sh", sock) - vmm.Args[0] = b.Conf.BinaryName() - if err := vmm.Start(); err != nil { - t.Fatalf("start fake VMM: %v", err) - } - t.Cleanup(func() { - _ = vmm.Process.Kill() - _ = vmm.Wait() - }) - if err := utils.WaitFor(t.Context(), 2*time.Second, 10*time.Millisecond, func() (bool, error) { - return utils.VerifyProcessCmdline(vmm.Process.Pid, sock), nil - }); err != nil { - t.Fatalf("fake VMM did not exec in time: %v", err) - } + vmm := startFakeVMM(t, b.Conf.BinaryName(), sock) writeRunFile(t, b.PIDFilePath(rec.RunDir), strconv.Itoa(vmm.Process.Pid)) if info := b.ToVM(&rec); info.PID != vmm.Process.Pid { diff --git a/hypervisor/netresize_test.go b/hypervisor/netresize_test.go index 9c9e3464..e6f5108e 100644 --- a/hypervisor/netresize_test.go +++ b/hypervisor/netresize_test.go @@ -94,10 +94,7 @@ func TestNetResizeRemoveResumesWithoutLiveDevice(t *testing.T) { if res.After != 0 || len(res.Removed) != 1 { t.Fatalf("res = %+v, want the NIC removed and After=0", res) } - fresh, err := b.PeekRecord(ctx, "vm7") - if err != nil { - t.Fatalf("read record: %v", err) - } + fresh := recordOf(t, b, "vm7") if len(fresh.NetworkConfigs) != 0 { t.Fatalf("record still carries %d NICs, want the stale NIC truncated", len(fresh.NetworkConfigs)) } @@ -146,10 +143,7 @@ func TestNetResizeWithAddsAndPersists(t *testing.T) { if !slices.Equal(plumbing.queues, []int{4, 4}) { t.Fatalf("tap queues = %v, want the device's queue rule applied to every new slot", plumbing.queues) } - fresh, err := b.PeekRecord(ctx, "vm2") - if err != nil { - t.Fatalf("read record: %v", err) - } + fresh := recordOf(t, b, "vm2") if len(fresh.NetworkConfigs) != 3 || fresh.NetworkConfigs[2].MAC != "aa:bb:cc:dd:ee:03" { t.Fatalf("record NICs = %+v, want three with the new MACs persisted", fresh.NetworkConfigs) } @@ -171,10 +165,7 @@ func TestNetResizeAddRollsBackHostSlotOnDeviceFailure(t *testing.T) { if !slices.Equal(plumbing.removed, []int{1}) { t.Fatalf("plumbing.removed = %v, want the fresh host slot 1 rolled back", plumbing.removed) } - fresh, err := b.PeekRecord(ctx, "vm3") - if err != nil { - t.Fatalf("read record: %v", err) - } + fresh := recordOf(t, b, "vm3") if len(fresh.NetworkConfigs) != 1 { t.Fatalf("record NICs = %d, want the failed NIC never persisted", len(fresh.NetworkConfigs)) } @@ -206,10 +197,7 @@ func TestNetResizeWithPersistsTheNetworkOfTheFirstNIC(t *testing.T) { if _, err := b.NetResizeWith(ctx, "vm4", &rec, &fakeNICDevices{}, &stubPlumbing{defaultNetwork: "cni-a"}, 1); err != nil { t.Fatalf("NetResizeWith: %v", err) } - fresh, err := b.PeekRecord(ctx, "vm4") - if err != nil { - t.Fatalf("read record: %v", err) - } + fresh := recordOf(t, b, "vm4") if fresh.Config.Network != tt.want { t.Fatalf("record network = %q, want %q", fresh.Config.Network, tt.want) } diff --git a/hypervisor/placement_test.go b/hypervisor/placement_test.go index 973726f7..9576f60e 100644 --- a/hypervisor/placement_test.go +++ b/hypervisor/placement_test.go @@ -67,10 +67,7 @@ func TestPlaceRecord(t *testing.T) { if placed.CPUSet != tt.wantCPUSet || placed.QueueCPUs != tt.wantQueueCPUs { t.Errorf("got cpuset %q queue_cpus %q, want %q %q", placed.CPUSet, placed.QueueCPUs, tt.wantCPUSet, tt.wantQueueCPUs) } - stored, err := b.PeekRecord(ctx, "self") - if err != nil { - t.Fatalf("PeekRecord: %v", err) - } + stored := recordOf(t, b, "self") if stored.CPUSet != placed.CPUSet || stored.QueueCPUs != placed.QueueCPUs { t.Errorf("stored cpuset %q queue_cpus %q differ from placed", stored.CPUSet, stored.QueueCPUs) } @@ -133,7 +130,9 @@ func TestPlacementRowFollowsTheRecord(t *testing.T) { var got string if err := b.view(ctx, func(tx *vmTx) error { return tx.placements.Scan(ctx, tx.r, func(id string, cpus *string) error { - got += id + "=" + *cpus + ";" + if cpus != nil { + got += id + "=" + *cpus + ";" + } return nil }) }); err != nil { @@ -239,10 +238,7 @@ func TestLeavingRunningClearsPlacement(t *testing.T) { seed("failed", types.VMStateStopped) b.markFailedOperation(ctx, "failed", false) for _, id := range []string{"stopped", "failed"} { - rec, err := b.PeekRecord(ctx, id) - if err != nil { - t.Fatalf("PeekRecord %s: %v", id, err) - } + rec := recordOf(t, b, id) if rec.CPUSet != "" || rec.QueueCPUs != "" { t.Errorf("%s: cpuset %q queue_cpus %q survived leaving running", id, rec.CPUSet, rec.QueueCPUs) } diff --git a/hypervisor/start_linux_test.go b/hypervisor/start_linux_test.go index dbd9c3cc..7add7023 100644 --- a/hypervisor/start_linux_test.go +++ b/hypervisor/start_linux_test.go @@ -2,7 +2,6 @@ package hypervisor import ( "context" - "os/exec" "slices" "testing" @@ -15,11 +14,7 @@ func TestStartSequenceRelaunchesWhenTheScannedVMMDiedBeforeTheLock(t *testing.T) dir := seedStoppedVMWithDirs(t, b, id) sockPath := SocketPath(dir) - vmm := exec.Command("/bin/sh", "-c", "sleep 60; :", "sh", sockPath) - vmm.Args[0] = b.Conf.BinaryName() - if err := vmm.Start(); err != nil { - t.Fatalf("start fake VMM: %v", err) - } + vmm := startFakeVMM(t, b.Conf.BinaryName(), sockPath) scan, err := utils.ScanProcsByBinary(b.Conf.BinaryName()) if err != nil { t.Fatalf("scan: %v", err) diff --git a/hypervisor/state_test.go b/hypervisor/state_test.go index b4597447..27ed3dbf 100644 --- a/hypervisor/state_test.go +++ b/hypervisor/state_test.go @@ -110,7 +110,7 @@ func TestPrepareStartClosesIntervalAfterMarkError(t *testing.T) { if got := rec.Entries(); len(got) != 0 { t.Fatalf("MarkError emitted %d; want 0", len(got)) } - loaded, _ := b.LoadRecord(ctx, "vm1") + loaded := recordOf(t, b, "vm1") if loaded.StoppedAt != nil { t.Errorf("MarkError must not write StoppedAt; got %v", loaded.StoppedAt) } @@ -142,7 +142,7 @@ func TestStopAfterMarkErrorEmitsComputeStop(t *testing.T) { if len(entries) != 1 || entries[0].Kind != metering.KindVMComputeStop || entries[0].Reason != metering.ReasonStopUser { t.Fatalf("got %+v, want one compute.stop reason=user", entries) } - loaded, _ := b.LoadRecord(ctx, "vm1") + loaded := recordOf(t, b, "vm1") if loaded.StoppedAt == nil { t.Error("StoppedAt nil after Stopped transition") } @@ -155,12 +155,12 @@ func TestDeleteForceClosesIntervalAfterMarkError(t *testing.T) { b.MarkError(ctx, "vm1") rec.Reset() - loaded, _ := b.LoadRecord(ctx, "vm1") - if !hasOpenComputeInterval(&loaded) { + loaded := recordOf(t, b, "vm1") + if !hasOpenComputeInterval(loaded) { t.Fatal("interval should still be open after MarkError") } - b.emitDeleteClose(ctx, "vm1", shapeFromConfig(loaded.Config), metering.ReasonStopCrash, hasOpenComputeInterval(&loaded)) + b.emitDeleteClose(ctx, "vm1", shapeFromConfig(loaded.Config), metering.ReasonStopCrash, hasOpenComputeInterval(loaded)) entries := rec.Entries() if len(entries) != 2 { t.Fatalf("got %d entries, want 2 (compute.stop + storage.stop)", len(entries)) @@ -319,23 +319,21 @@ func TestDirectRestoreSequenceEmitsOnlyComputeStopOnPopulateFailure(t *testing.T } func TestStartAllOnlyEmitsForActuallyLaunched(t *testing.T) { - b, rec := newMeteringTestBackend(t) + b, running := newHibernateTestVM(t) + rec := meteringcapture.New() + b.Metering = rec ctx := t.Context() seedVMRecord(t, b, "vm-stopped", 1, 1<<30, 10<<30, false) - seedRunningVM(t, b, "vm-running", 1, 1<<30, 10<<30) seedRunningVM(t, b, "vm-stale", 2, 2<<30, 20<<30) - startOne := func(ctx context.Context, id string, _ *utils.ProcScan) error { - switch id { - case "vm-stopped", "vm-stale": - return b.MarkStarted(ctx, id) - case "vm-running": - return nil - } - return fmt.Errorf("unexpected id: %s", id) + startOne := func(ctx context.Context, id string, scan *utils.ProcScan) error { + return b.StartSequence(ctx, id, scan, StartSpec{ + Launch: func(context.Context, *VMRecord, string) (int, error) { return 4242, nil }, + ConvergeRunning: func(context.Context, *VMRecord, string, int) error { return nil }, + }) } - succeeded, err := b.StartAll(ctx, []string{"vm-stopped", "vm-running", "vm-stale"}, startOne) + succeeded, err := b.StartAll(ctx, []string{"vm-stopped", running, "vm-stale"}, startOne) if err != nil { t.Fatalf("StartAll: %v", err) } @@ -345,14 +343,14 @@ func TestStartAllOnlyEmitsForActuallyLaunched(t *testing.T) { entries := rec.Entries() if len(entries) != 3 { - t.Fatalf("got %d entries, want 3 (vm-stopped: start; vm-stale: stop-crash + start; vm-running: none)", len(entries)) + t.Fatalf("got %d entries, want 3 (vm-stopped: start; vm-stale: stop-crash + start; live VMM: none)", len(entries)) } byVM := map[string][]metering.Entry{} for _, e := range entries { byVM[e.VMID] = append(byVM[e.VMID], e) } - if got := byVM["vm-running"]; len(got) != 0 { - t.Errorf("vm-running emitted %d entries; want 0", len(got)) + if got := byVM[running]; len(got) != 0 { + t.Errorf("%s emitted %d entries; want 0", running, len(got)) } if got := byVM["vm-stopped"]; len(got) != 1 || got[0].Kind != metering.KindVMComputeStart || got[0].Reason != metering.ReasonBoot { t.Errorf("vm-stopped: got %+v, want 1× compute.start reason=boot", got) @@ -452,14 +450,14 @@ func TestReconcileToRunningOrphanLaunchEmitsStart(t *testing.T) { b.ReconcileToRunning(ctx, "vm1") - loaded, _ := b.LoadRecord(ctx, "vm1") + loaded := recordOf(t, b, "vm1") if loaded.State != types.VMStateRunning { t.Errorf("State=%s, want Running", loaded.State) } if !loaded.FirstBooted { t.Error("FirstBooted should be true after orphan reconcile") } - if !hasOpenComputeInterval(&loaded) { + if !hasOpenComputeInterval(loaded) { t.Error("compute interval should be open after orphan reconcile") } entries := rec.Entries() @@ -492,7 +490,7 @@ func TestReconcileToRunningIdempotent(t *testing.T) { if got := rec.Entries(); len(got) != 0 { t.Errorf("idempotent reconcile must not emit; got %d", len(got)) } - loaded, _ := b.LoadRecord(ctx, "vm1") + loaded := recordOf(t, b, "vm1") if loaded.State != types.VMStateRunning { t.Errorf("State=%s, want Running (unchanged)", loaded.State) } diff --git a/hypervisor/stop_linux_test.go b/hypervisor/stop_linux_test.go index 448cbfd0..979d4163 100644 --- a/hypervisor/stop_linux_test.go +++ b/hypervisor/stop_linux_test.go @@ -5,7 +5,6 @@ import ( "errors" "net" "os" - "os/exec" "path/filepath" "slices" "strings" @@ -25,11 +24,7 @@ func TestForceDeleteStopsAVMMStartedUnderAnotherBinaryName(t *testing.T) { dir := seedStoppedVMWithDirs(t, b, id) sockPath := SocketPath(dir) - vmm := exec.Command("/bin/sh", "-c", "sleep 60; :", "sh", sockPath) - vmm.Args[0] = "previous-" + b.Conf.BinaryName() - if err := vmm.Start(); err != nil { - t.Fatalf("start fake VMM: %v", err) - } + vmm := startFakeVMM(t, "previous-"+b.Conf.BinaryName(), sockPath) exited := make(chan struct{}) go func() { _ = vmm.Wait() @@ -39,11 +34,6 @@ func TestForceDeleteStopsAVMMStartedUnderAnotherBinaryName(t *testing.T) { _ = vmm.Process.Kill() <-exited }) - if err := utils.WaitFor(ctx, 5*time.Second, time.Millisecond, func() (bool, error) { - return utils.VerifyProcessCmdline(vmm.Process.Pid, sockPath), nil - }); err != nil { - t.Fatalf("fake VMM never execed: %v", err) - } if err := utils.WritePIDFile(b.PIDFilePath(dir), vmm.Process.Pid); err != nil { t.Fatalf("write pidfile: %v", err) } @@ -90,12 +80,7 @@ func TestDeleteSkipsAScannedVMMThatExitedBeforeTheKill(t *testing.T) { dir := seedStoppedVMWithDirs(t, b, id) sockPath := SocketPath(dir) - child := exec.Command("/bin/sh", "-c", "sleep 60; :", "sh", sockPath) - child.Args[0] = b.Conf.BinaryName() - if err := child.Start(); err != nil { - t.Fatalf("start fake VMM child: %v", err) - } - t.Cleanup(func() { _ = child.Wait() }) + child := startFakeVMM(t, b.Conf.BinaryName(), sockPath) scan, err := utils.ScanProcsByBinary(b.Conf.BinaryName()) if err != nil { t.Fatalf("scan: %v", err) diff --git a/hypervisor/stop_test.go b/hypervisor/stop_test.go index 19f6ca23..0ed3e258 100644 --- a/hypervisor/stop_test.go +++ b/hypervisor/stop_test.go @@ -31,7 +31,7 @@ func TestDeleteAllStoppedVM(t *testing.T) { } deleted, err := b.DeleteAll(ctx, []string{id}, false, func(context.Context, string) error { - t.Fatal("stopLocked must not run for a stopped VM") + t.Error("stopLocked must not run for a stopped VM") return nil }) if err != nil { @@ -148,7 +148,7 @@ func TestDeleteForceTerminatesACreatingOrphan(t *testing.T) { if len(deleted) != 1 || deleted[0] != id { t.Fatalf("deleted = %v, want [%s]", deleted, id) } - if utils.IsProcessAlive(pid) { + if utils.VerifyProcessCmdline(pid, sock) { t.Errorf("orphan vmm pid %d still alive", pid) } if _, err := b.LoadRecord(ctx, id); err == nil { @@ -172,7 +172,7 @@ func TestDeleteCreatingPlaceholderEmitsNoMetering(t *testing.T) { } deleted, err := b.DeleteAll(ctx, []string{id}, false, func(context.Context, string) error { - t.Fatal("stopLocked must not run for a placeholder without a VMM") + t.Error("stopLocked must not run for a placeholder without a VMM") return nil }) if err != nil || len(deleted) != 1 { @@ -345,31 +345,6 @@ func TestDeleteAllRefusesLiveAPISocket(t *testing.T) { } } -func TestDeleteMigratedVMRemovesItsDirs(t *testing.T) { - b, _ := newMeteringTestBackend(t) - ctx := t.Context() - const id = "vm-migrated" - seedVMRecord(t, b, id, 1, 1<<30, 10<<30, true) - runDir, logDir := shortTempDir(t), shortTempDir(t) - if err := b.dbUpdate(ctx, func(idx *VMIndex) error { - idx.VMs[id].State = types.VMStateStopped - idx.VMs[id].RunDir = runDir - idx.VMs[id].LogDir = logDir - return nil - }); err != nil { - t.Fatalf("seed: %v", err) - } - - if _, err := b.DeleteAll(ctx, []string{id}, false, func(context.Context, string) error { return nil }); err != nil { - t.Fatalf("DeleteAll: %v", err) - } - for _, dir := range []string{runDir, logDir} { - if _, err := os.Stat(dir); !os.IsNotExist(err) { - t.Fatalf("migrated dir %s must be removed", dir) - } - } -} - func TestDeleteForceEmitsOneComputeStop(t *testing.T) { b, id := newHibernateTestVM(t) ctx := t.Context() diff --git a/hypervisor/teardown_test.go b/hypervisor/teardown_test.go index cc730bf8..06e11d11 100644 --- a/hypervisor/teardown_test.go +++ b/hypervisor/teardown_test.go @@ -86,8 +86,7 @@ func TestRecoverDeletingClosesMetering(t *testing.T) { rec := seedProtoVM(t, b, "vmproto-meter") if tt.open { if err := b.UpdateRecord(ctx, "vmproto-meter", func(r *VMRecord) error { - now := timeNow() - r.StartedAt = &now + r.StartedAt = new(timeNow()) return nil }); err != nil { t.Fatal(err) diff --git a/hypervisor/utils.go b/hypervisor/utils.go index fb49c84b..b35e99c7 100644 --- a/hypervisor/utils.go +++ b/hypervisor/utils.go @@ -51,7 +51,7 @@ const ( onlineCPUsPath = cgroup.SysCPURoot + "/online" ) -var hostCPUCount = sync.OnceValue(readHostCPUCount) +var hostCPUCount = sync.OnceValue(func() int { return hostCPUCountAt(onlineCPUsPath) }) // SnapshotFileKind classifies a snapshot file for CloneSnapshotFiles. type SnapshotFileKind int @@ -327,7 +327,7 @@ func ValidateResidentPaths(sidecar, rec []*types.StorageConfig) error { } func ValidateRestoreMACs(snapMACs []string, rec *VMRecord) error { - if len(snapMACs) > len(rec.NetworkConfigs) { + if snapMACs != nil && len(snapMACs) != len(rec.NetworkConfigs) { return fmt.Errorf("snapshot has %d NICs, vm has %d; NIC identity must match for restore", len(snapMACs), len(rec.NetworkConfigs)) } for i, mac := range snapMACs { @@ -476,8 +476,8 @@ func snapshotResidentBasename(sc *types.StorageConfig) string { } } -func readHostCPUCount() int { - cpus, err := cgroup.ReadCPUList(onlineCPUsPath) +func hostCPUCountAt(path string) int { + cpus, err := cgroup.ReadCPUList(path) if err != nil || len(cpus) == 0 { return runtime.NumCPU() } diff --git a/hypervisor/utils_test.go b/hypervisor/utils_test.go index 90a75ac1..d58f20cb 100644 --- a/hypervisor/utils_test.go +++ b/hypervisor/utils_test.go @@ -12,10 +12,16 @@ import ( ) func TestHostCPUCountIgnoresAffinityMask(t *testing.T) { - got := HostCPUCount() - t.Logf("HostCPUCount=%d runtime.NumCPU=%d", got, runtime.NumCPU()) - if got < runtime.NumCPU() { - t.Fatalf("HostCPUCount() = %d, below runtime.NumCPU() = %d", got, runtime.NumCPU()) + dir := t.TempDir() + online := filepath.Join(dir, "online") + if err := os.WriteFile(online, []byte("0-1023\n"), 0o600); err != nil { + t.Fatalf("setup: %v", err) + } + if got := hostCPUCountAt(online); got != 1024 { + t.Errorf("hostCPUCountAt(online) = %d, want the 1024 online cpus", got) + } + if got := hostCPUCountAt(filepath.Join(dir, "absent")); got != runtime.NumCPU() { + t.Errorf("hostCPUCountAt(absent) = %d, want the runtime.NumCPU() fallback %d", got, runtime.NumCPU()) } } diff --git a/images/cloudimg/pull_test.go b/images/cloudimg/pull_test.go index 8d9ae1d0..1bb56db7 100644 --- a/images/cloudimg/pull_test.go +++ b/images/cloudimg/pull_test.go @@ -70,8 +70,12 @@ func TestDownloadToFileRejectsNonImageBeforeWriting(t *testing.T) { if _, err := downloadToFile(t.Context(), server.URL, dst, progress.Nop, 4); err == nil || !strings.Contains(err.Error(), "HTML") { t.Fatalf("downloadToFile() = %v, want the HTML rejection", err) } - if st, err := dst.Stat(); err != nil || st.Size() != 0 { - t.Errorf("rejected download wrote %d bytes (err %v), want 0", st.Size(), err) + st, err := dst.Stat() + if err != nil { + t.Fatalf("Stat(): %v", err) + } + if st.Size() != 0 { + t.Errorf("rejected download wrote %d bytes, want 0", st.Size()) } }) } diff --git a/images/oci/uuid.go b/images/oci/uuid.go index 76a7f65a..3caeb52d 100644 --- a/images/oci/uuid.go +++ b/images/oci/uuid.go @@ -2,7 +2,7 @@ package oci import "github.com/google/uuid" -// uuidV5 matches the uuid_v5() bash function in os-image/start.sh. +// uuidV5 returns the RFC 4122 version 5 UUID of name in the URL namespace. func uuidV5(name string) string { return uuid.NewSHA1(uuid.NameSpaceURL, []byte(name)).String() } diff --git a/images/oci/uuid_test.go b/images/oci/uuid_test.go index 8fe220ea..f3b18e82 100644 --- a/images/oci/uuid_test.go +++ b/images/oci/uuid_test.go @@ -1,42 +1,10 @@ package oci -import ( - "strings" - "testing" -) +import "testing" -func TestUUIDv5_Format(t *testing.T) { - id := uuidV5("test-name") - - parts := strings.Split(id, "-") - if len(parts) != 5 { - t.Fatalf("expected 5 parts, got %d: %q", len(parts), id) - } - if len(parts[0]) != 8 || len(parts[1]) != 4 || len(parts[2]) != 4 || len(parts[3]) != 4 || len(parts[4]) != 12 { - t.Errorf("unexpected format: %q", id) - } -} - -func TestUUIDv5_Deterministic(t *testing.T) { - id1 := uuidV5("same-input") - id2 := uuidV5("same-input") - if id1 != id2 { - t.Errorf("not deterministic: %q != %q", id1, id2) - } -} - -func TestUUIDv5_DifferentInputs(t *testing.T) { - id1 := uuidV5("input-a") - id2 := uuidV5("input-b") - if id1 == id2 { - t.Errorf("different inputs produced same UUID: %q", id1) - } -} - -func TestUUIDv5_Version5(t *testing.T) { - id := uuidV5("version-check") - - if id[14] != '5' { - t.Errorf("expected version 5, got %c at position 14 in %q", id[14], id) +func TestUUIDV5Vector(t *testing.T) { + const want = "767acf47-c1e4-5cf8-a34c-45a7096faa2d" + if got := uuidV5("test-name"); got != want { + t.Fatalf("uuidV5(%q) = %s, want %s", "test-name", got, want) } } diff --git a/lock/flock/flock_test.go b/lock/flock/flock_test.go index 7c15e1df..e5fa46c0 100644 --- a/lock/flock/flock_test.go +++ b/lock/flock/flock_test.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "sync" + "sync/atomic" "testing" "time" ) @@ -139,24 +140,24 @@ func TestLockContextCancel(t *testing.T) { } func TestConcurrentLockUnlock(t *testing.T) { - l := New(lockPath(t)) + path := lockPath(t) ctx := t.Context() - var ( - mu sync.Mutex - counter int - ) + var inside, overlaps atomic.Int32 var wg sync.WaitGroup for range 50 { wg.Go(func() { + l := New(path) if err := l.Lock(ctx); err != nil { t.Errorf("Lock: %v", err) return } - mu.Lock() - counter++ - mu.Unlock() + if inside.Add(1) != 1 { + overlaps.Add(1) + } + time.Sleep(time.Millisecond) + inside.Add(-1) if err := l.Unlock(ctx); err != nil { t.Errorf("Unlock: %v", err) } @@ -164,8 +165,8 @@ func TestConcurrentLockUnlock(t *testing.T) { } wg.Wait() - if counter != 50 { - t.Fatalf("counter = %d, want 50", counter) + if n := overlaps.Load(); n != 0 { + t.Fatalf("%d holders overlapped inside the critical section", n) } } diff --git a/meta/contracttest/suite.go b/meta/contracttest/suite.go index 56406f25..0f36e1b9 100644 --- a/meta/contracttest/suite.go +++ b/meta/contracttest/suite.go @@ -142,12 +142,15 @@ func testDetached(t *testing.T, factory Factory) { for i := range raw { raw[i] = 'X' } - return w.ScanRaw(ctx, nsAlpha, "records", func(_ string, raw jsontext.Value) error { + if err := w.ScanRaw(ctx, nsAlpha, "records", func(_ string, raw jsontext.Value) error { for i := range raw { raw[i] = 'Y' } return nil - }) + }); err != nil { + return err + } + return w.PutRaw(ctx, nsAlpha, "records", "dirty", jsontext.Value(`{}`), false) }) if again, err := get1(t, s, c); err != nil || again.Name != nameOrig { t.Fatalf("raw read aliased engine state: %v %v", again, err) @@ -214,7 +217,6 @@ func testForcedRetry(t *testing.T, factory Factory) { s := ForcedRetry(factory(t, []string{nsAlpha})) c := meta.NewCollection[record](nsAlpha, "records") - // The correct pattern: accumulate inside, publish only after nil return. var out []string runs := 0 if err := s.Update(ctx, meta.Scope{Write: nsAlpha}, meta.CommitDurable, func(w meta.Writer) error { @@ -258,18 +260,25 @@ func testCtxVsHeldWriter(t *testing.T, factory Factory) { held := make(chan struct{}) release := make(chan struct{}) - holderDone := make(chan struct{}) + holderErr := make(chan error, 1) var heldOnce sync.Once go func() { - defer close(holderDone) - _ = s.Update(ctx, meta.Scope{Write: nsAlpha}, meta.CommitDurable, func(w meta.Writer) error { + holderErr <- s.Update(ctx, meta.Scope{Write: nsAlpha}, meta.CommitDurable, func(w meta.Writer) error { heldOnce.Do(func() { close(held) }) <-release return c.Insert(ctx, w, "holder", &record{}) }) }() - <-held - defer func() { <-holderDone }() + select { + case <-held: + case err := <-holderErr: + t.Fatalf("holder update ended before holding the writer: %v", err) + } + defer func() { + if err := <-holderErr; err != nil { + t.Errorf("holder update: %v", err) + } + }() defer close(release) dctx, cancel := context.WithTimeout(ctx, 100*time.Millisecond) diff --git a/meta/json/store_test.go b/meta/json/store_test.go index f0a2ae3c..bf3cc055 100644 --- a/meta/json/store_test.go +++ b/meta/json/store_test.go @@ -73,6 +73,9 @@ func TestCommitAtomicityUnderCrash(t *testing.T) { }); err != nil { t.Fatal(err) } + if a == nil { + t.Fatalf("record a missing after crash at %s", step) + } switch { case (*a)["v"] == 1 && b == nil: case (*a)["v"] == 2 && b != nil && (*b)["v"] == 2: @@ -87,20 +90,9 @@ func TestPrevRecovery(t *testing.T) { ctx := t.Context() dir := t.TempDir() s := newStore(t, dir, "alpha") - c := meta.NewCollection[map[string]int]("alpha", "records") path := filepath.Join(dir, "alpha.json") - for gen := 1; gen <= 2; gen++ { - v := map[string]int{"gen": gen} - if err := s.Update(ctx, meta.Scope{Write: "alpha"}, meta.CommitDurable, func(w meta.Writer) error { - if gen == 1 { - return c.Insert(ctx, w, "a", &v) - } - return c.Replace(ctx, w, "a", &v) - }); err != nil { - t.Fatal(err) - } - } + writeTwoGenerations(t, s) if err := os.WriteFile(path, []byte("{torn"), 0o644); err != nil { t.Fatal(err) } @@ -108,7 +100,7 @@ func TestPrevRecovery(t *testing.T) { s2 := newStore(t, dir, "alpha") c2 := meta.NewCollection[map[string]int]("alpha", "records") got, err := mustGet(t, s2, "alpha", "a") - if err != nil || (*got)["gen"] != 1 { + if err != nil || got == nil || (*got)["gen"] != 1 { t.Fatalf("recovered generation: %v, %v", got, err) } @@ -137,8 +129,11 @@ func TestCorruptBothGenerations(t *testing.T) { dir := t.TempDir() s := newStore(t, dir, "alpha") path := filepath.Join(dir, "alpha.json") - if err := os.WriteFile(path, []byte("{torn"), 0o644); err != nil { - t.Fatal(err) + writeTwoGenerations(t, s) + for _, p := range []string{path, path + prevSuffix} { + if err := os.WriteFile(p, []byte("{torn"), 0o644); err != nil { + t.Fatal(err) + } } err := s.View(ctx, []string{"alpha"}, func(meta.Reader) error { return nil }) if !errors.Is(err, meta.ErrCorrupt) { @@ -318,23 +313,11 @@ func TestCleanUpdateSkipsCommit(t *testing.T) { } func TestTrailingDataFallsBackToPrev(t *testing.T) { - ctx := t.Context() dir := t.TempDir() s := newStore(t, dir, "alpha") - c := meta.NewCollection[map[string]int]("alpha", "records") path := filepath.Join(dir, "alpha.json") - for gen := 1; gen <= 2; gen++ { - v := map[string]int{"gen": gen} - if err := s.Update(ctx, meta.Scope{Write: "alpha"}, meta.CommitDurable, func(w meta.Writer) error { - if gen == 1 { - return c.Insert(ctx, w, "a", &v) - } - return c.Replace(ctx, w, "a", &v) - }); err != nil { - t.Fatal(err) - } - } + writeTwoGenerations(t, s) main, err := os.ReadFile(path) if err != nil { @@ -345,7 +328,7 @@ func TestTrailingDataFallsBackToPrev(t *testing.T) { } s2 := newStore(t, dir, "alpha") got, err := mustGet(t, s2, "alpha", "a") - if err != nil || (*got)["gen"] != 1 { + if err != nil || got == nil || (*got)["gen"] != 1 { t.Fatalf("trailing-data main was served instead of .prev: %v, %v", got, err) } } @@ -362,6 +345,20 @@ func (c *crashPoint) hook(step string) error { return nil } +func writeTwoGenerations(t *testing.T, s *Store) { + t.Helper() + ctx := t.Context() + c := meta.NewCollection[map[string]int]("alpha", "records") + for gen := 1; gen <= 2; gen++ { + v := map[string]int{"gen": gen} + if err := s.Update(ctx, meta.Scope{Write: "alpha"}, meta.CommitDurable, func(w meta.Writer) error { + return c.Upsert(ctx, w, "a", &v) + }); err != nil { + t.Fatal(err) + } + } +} + func mustGet(t *testing.T, s *Store, ns, id string) (*map[string]int, error) { t.Helper() c := meta.NewCollection[map[string]int](ns, "records") diff --git a/meta/sqlite/maintenance_test.go b/meta/sqlite/maintenance_test.go index b5e0ec67..d5357745 100644 --- a/meta/sqlite/maintenance_test.go +++ b/meta/sqlite/maintenance_test.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "strings" + "sync" "testing" "testing/synctest" "time" @@ -19,32 +20,50 @@ func TestBackupFidelity(t *testing.T) { ctx := t.Context() dir := t.TempDir() s := newStore(t, dir, "vms") - put := func(id, val string) { - err := s.Update(ctx, meta.Scope{Write: "vms"}, meta.CommitDurable, func(w meta.Writer) error { + write := func(id, val string) error { + return s.Update(ctx, meta.Scope{Write: "vms"}, meta.CommitDurable, func(w meta.Writer) error { return w.PutRaw(ctx, "vms", "records", id, jsontext.Value(val), false) }) - if err != nil { + } + put := func(id, val string) { + if err := write(id, val); err != nil { t.Fatalf("put %s: %v", id, err) } } put("marker", `{"v":1}`) stop := make(chan struct{}) - go func() { + started := make(chan struct{}) + var churnErr error + var wg sync.WaitGroup + wg.Go(func() { for i := 0; ; i++ { select { case <-stop: return default: - put("churn", `{"i":`+string(rune('0'+i%10))+`}`) + } + err := write("churn", `{"i":`+string(rune('0'+i%10))+`}`) + if i == 0 { + close(started) + } + if err != nil { + churnErr = err + return } } - }() + }) + <-started dest := filepath.Join(dir, "backup", "meta.db") - if err := Backup(ctx, filepath.Join(dir, DBFileName), dest); err != nil { - t.Fatalf("backup: %v", err) - } + backupErr := Backup(ctx, filepath.Join(dir, DBFileName), dest) close(stop) + wg.Wait() + if churnErr != nil { + t.Fatalf("churn write: %v", churnErr) + } + if backupErr != nil { + t.Fatalf("backup: %v", backupErr) + } if raw, ok := backupGet(t, dest, "marker"); !ok || raw != `{"v":1}` { t.Fatalf("marker missing from backup: %q ok=%v", raw, ok) } @@ -72,6 +91,7 @@ func TestBackupCrashSteps(t *testing.T) { if err := Backup(ctx, filepath.Join(dir, DBFileName), dest); err != nil { t.Fatalf("publish first backup: %v", err) } + t.Cleanup(func() { testBackupStep = nil }) errCrash := errors.New("injected crash") for _, step := range []string{"vacuumed", "verified", "synced", "renamed"} { @@ -133,14 +153,24 @@ func TestBusyCtxDeadline(t *testing.T) { held := make(chan struct{}) release := make(chan struct{}) + holderErr := make(chan error, 1) go func() { - _ = s1.Update(t.Context(), meta.Scope{Write: "vms"}, meta.CommitDurable, func(meta.Writer) error { + holderErr <- s1.Update(t.Context(), meta.Scope{Write: "vms"}, meta.CommitDurable, func(meta.Writer) error { close(held) <-release return nil }) }() - <-held + select { + case <-held: + case err := <-holderErr: + t.Fatalf("holder update ended before holding the writer: %v", err) + } + defer func() { + if err := <-holderErr; err != nil { + t.Errorf("holder update: %v", err) + } + }() defer close(release) ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond) diff --git a/meta/sqlite/multiprocess_test.go b/meta/sqlite/multiprocess_test.go index 84a1903a..633c7395 100644 --- a/meta/sqlite/multiprocess_test.go +++ b/meta/sqlite/multiprocess_test.go @@ -164,12 +164,13 @@ func TestKillStormAtomicity(t *testing.T) { t.Fatal(err) } sc := bufio.NewScanner(pipe) + want := 3 + round*2 got := 0 for sc.Scan() { if id, ok := strings.CutPrefix(sc.Text(), "ACK "); ok { acked[id] = struct{}{} got++ - if got >= 3+round*2 { + if got >= want { break } } @@ -178,6 +179,9 @@ func TestKillStormAtomicity(t *testing.T) { t.Fatalf("kill worker: %v", err) } _ = cmd.Wait() + if got < want { + t.Fatalf("round %d: worker acked %d transactions, want at least %d", round, got, want) + } } s := newStore(t, dir, "alpha") diff --git a/metadata/metadata.go b/metadata/metadata.go index 4d5be436..58db354f 100644 --- a/metadata/metadata.go +++ b/metadata/metadata.go @@ -44,7 +44,7 @@ runcmd: {{- if .Mounts}} mounts: {{- range .Mounts}} - - ['{{yamlQuote .Device}}', '{{yamlQuote .MountPoint}}', '{{yamlQuote .FSType}}', '{{yamlQuote .Options}}', '0', '2'] + - ['{{yamlQuote .Device}}', '{{yamlQuote .MountPoint}}', '{{yamlQuote .FSType}}', '{{yamlQuote .Options}}', '0', '0'] {{- end}} {{- end}} {{- if .Networks}} diff --git a/metadata/metadata_test.go b/metadata/metadata_test.go index 22101e13..54147382 100644 --- a/metadata/metadata_test.go +++ b/metadata/metadata_test.go @@ -2,8 +2,12 @@ package metadata import ( "bytes" + "encoding/binary" + "maps" + "slices" "strings" "testing" + "unicode/utf16" ) func TestUserData_NonRootPasswordSetOnlyByRuncmd(t *testing.T) { @@ -214,22 +218,24 @@ func TestGenerate_ProducesValidFAT12(t *testing.T) { if err := Generate(&buf, cfg); err != nil { t.Fatal(err) } - if buf.Len() == 0 { - t.Fatal("empty output") - } + label, files := readFAT12(t, buf.Bytes()) - raw := buf.String() - if !strings.Contains(raw, `macaddress: "aa:bb:cc:dd:ee:ff"`) { - t.Error("macaddress matching not found in FAT12 image") + if label != "CIDATA" { + t.Errorf("volume label = %q, want CIDATA", label) + } + if got := slices.Sorted(maps.Keys(files)); !slices.Equal(got, []string{"meta-data", "network-config", "user-data"}) { + t.Fatalf("files in the image = %v", got) } - if !strings.Contains(raw, "10.0.0.2/24") { - t.Error("IP not found in FAT12 image") + if !strings.Contains(string(files["meta-data"]), "test-id") { + t.Errorf("meta-data lacks the instance id: %s", files["meta-data"]) } - if !strings.Contains(raw, "/etc/systemd/network/15-cocoon-id0.network") { - t.Error("fallback .network path not found in FAT12 image") + network := string(files["network-config"]) + if !strings.Contains(network, `macaddress: "aa:bb:cc:dd:ee:ff"`) || !strings.Contains(network, "10.0.0.2/24") { + t.Errorf("network-config content wrong: %s", network) } - if !strings.Contains(raw, "MACAddress=aa:bb:cc:dd:ee:ff") { - t.Error("fallback .network MACAddress not found in FAT12 image") + userData := string(files["user-data"]) + if !strings.Contains(userData, "/etc/systemd/network/15-cocoon-id0.network") || !strings.Contains(userData, "MACAddress=aa:bb:cc:dd:ee:ff") { + t.Errorf("user-data fallback .network missing: %s", userData) } } @@ -244,15 +250,12 @@ func TestGenerate_NoNetworks(t *testing.T) { if err := Generate(&buf, cfg); err != nil { t.Fatal(err) } - if buf.Len() == 0 { - t.Fatal("empty output") - } + _, files := readFAT12(t, buf.Bytes()) - raw := buf.String() - if strings.Contains(raw, "ethernets:") { - t.Error("network-config should not appear without networks") + if got := slices.Sorted(maps.Keys(files)); !slices.Equal(got, []string{"meta-data", "user-data"}) { + t.Fatalf("files in the image = %v, want no network-config", got) } - if strings.Contains(raw, "write_files:") { + if strings.Contains(string(files["user-data"]), "write_files:") { t.Error("write_files should not appear without networks") } } @@ -279,6 +282,9 @@ func TestUserData_Mounts(t *testing.T) { if !strings.Contains(out, "/dev/disk/by-id/virtio-data1") || !strings.Contains(out, "/mnt/data") { t.Errorf("mount entry missing\n%s", out) } + if !strings.Contains(out, "'0', '0']") { + t.Errorf("mount entry must not request a boot fsck\n%s", out) + } } func TestUserData_NoMounts(t *testing.T) { @@ -306,3 +312,73 @@ func TestUserData_MountsYAMLEscape(t *testing.T) { t.Errorf("single quote not escaped: %s", buf.String()) } } + +func readFAT12(t *testing.T, img []byte) (string, map[string][]byte) { + t.Helper() + if len(img) < 512 || img[510] != 0x55 || img[511] != 0xAA { + t.Fatal("boot sector signature 0x55AA missing") + } + if got := string(img[54:62]); got != "FAT12 " { + t.Fatalf("filesystem type = %q, want FAT12", got) + } + bytesPerSector := int(binary.LittleEndian.Uint16(img[11:])) + clusterBytes := int(img[13]) * bytesPerSector + fatOff := int(binary.LittleEndian.Uint16(img[14:])) * bytesPerSector + rootOff := fatOff + int(img[16])*int(binary.LittleEndian.Uint16(img[22:]))*bytesPerSector + dataOff := rootOff + int(binary.LittleEndian.Uint16(img[17:]))*32 + if want := int(binary.LittleEndian.Uint16(img[19:])) * bytesPerSector; len(img) != want { + t.Fatalf("image is %d bytes, boot sector says %d", len(img), want) + } + next := func(cluster int) int { + off := fatOff + cluster + cluster/2 + word := int(img[off]) | int(img[off+1])<<8 + if cluster%2 == 0 { + return word & 0xFFF + } + return word >> 4 + } + + label := strings.TrimRight(string(img[43:54]), " ") + files := map[string][]byte{} + var parts [][]uint16 + for off := rootOff; off < dataOff && img[off] != 0; off += 32 { + entry := img[off : off+32] + switch entry[11] { + case 0x0F: + var chars []uint16 + for _, span := range [][2]int{{1, 11}, {14, 26}, {28, 32}} { + for i := span[0]; i < span[1]; i += 2 { + chars = append(chars, binary.LittleEndian.Uint16(entry[i:])) + } + } + parts = append(parts, chars) + case 0x08: + default: + var chars []uint16 + for _, part := range slices.Backward(parts) { + chars = append(chars, part...) + } + parts = nil + if i := slices.Index(chars, 0); i >= 0 { + chars = chars[:i] + } + name := string(utf16.Decode(chars)) + size := int(binary.LittleEndian.Uint32(entry[28:])) + var data []byte + cluster := int(binary.LittleEndian.Uint16(entry[26:])) + for hops := 0; len(data) < size; hops++ { + if cluster < 2 || cluster >= 0xFF8 || hops > len(img) { + t.Fatalf("%s: broken FAT chain at cluster %d after %d bytes of %d", name, cluster, len(data), size) + } + start := dataOff + (cluster-2)*clusterBytes + data = append(data, img[start:start+clusterBytes]...) + cluster = next(cluster) + } + if size > 0 && cluster < 0xFF8 { + t.Fatalf("%s: FAT chain continues past %d bytes", name, size) + } + files[name] = data[:size] + } + } + return label, files +} diff --git a/metering/metalog/metalog.go b/metering/metalog/metalog.go index 82148097..1cef3dd6 100644 --- a/metering/metalog/metalog.go +++ b/metering/metalog/metalog.go @@ -30,6 +30,7 @@ func New(s meta.Store) *Recorder { } func (r *Recorder) Emit(ctx context.Context, e metering.Entry) { + ctx = context.WithoutCancel(ctx) err := r.store.Update(ctx, meta.Scope{Write: NamespaceName}, meta.CommitRelaxed, func(w meta.Writer) error { _, aerr := r.log.Append(ctx, w, &e, meta.RelaxedOK) return aerr diff --git a/metering/metalog/metalog_test.go b/metering/metalog/metalog_test.go index 7e363861..418cd89f 100644 --- a/metering/metalog/metalog_test.go +++ b/metering/metalog/metalog_test.go @@ -1,6 +1,7 @@ package metalog import ( + "context" "encoding/json/jsontext" "encoding/json/v2" "path/filepath" @@ -12,6 +13,40 @@ import ( "github.com/cocoonstack/cocoon/metering" ) +func TestEmitSurvivesACanceledContext(t *testing.T) { + dir := t.TempDir() + s, err := metajson.Open(metajson.Namespace{ + Name: NamespaceName, + FilePath: filepath.Join(dir, "log.json"), + LockPath: filepath.Join(dir, "log.lock"), + Codec: metajson.GenericCodec{}, + }) + if err != nil { + t.Fatalf("open: %v", err) + } + defer s.Close() + + ctx, cancel := context.WithCancel(t.Context()) + cancel() + New(s).Emit(ctx, metering.Entry{Kind: metering.KindVMStorageStop, VMID: "vm1"}) + + entries := 0 + err = s.View(t.Context(), []string{NamespaceName}, func(rd meta.Reader) error { + return rd.ScanRaw(t.Context(), NamespaceName, TableEntries, func(id string, _ jsontext.Value) error { + if _, err := strconv.ParseUint(id, 10, 64); err == nil { + entries++ + } + return nil + }) + }) + if err != nil { + t.Fatalf("scan: %v", err) + } + if entries != 1 { + t.Fatalf("entries = %d, want the stop event kept after SIGTERM canceled the context", entries) + } +} + func TestEmitAppendsInOrder(t *testing.T) { dir := t.TempDir() s, err := metajson.Open(metajson.Namespace{ diff --git a/network/cni/teardown_test.go b/network/cni/teardown_test.go index 6012f9fa..971de5ba 100644 --- a/network/cni/teardown_test.go +++ b/network/cni/teardown_test.go @@ -15,7 +15,6 @@ import ( "github.com/cocoonstack/cocoon/network" ) -// TestSubsetTeardownRecovery is the design §9 subset gate: crash mid-deleting on a one-NIC `vm net remove`, then recover — the untouched NIC row survives and the netns stays (an aggregate-shaped test would pass while this fails). func TestSubsetTeardownRecovery(t *testing.T) { c, exec := newTestCNIWithStore(t) stubLifecycleSeams(t) diff --git a/os-image/start.sh b/os-image/start.sh index 88e1294d..98ceead2 100644 --- a/os-image/start.sh +++ b/os-image/start.sh @@ -1,7 +1,7 @@ #!/bin/bash set -e -# UUID v5 (RFC 4122, SHA-1 based): deterministic UUID from NAMESPACE_URL + name. +# UUID v5 (RFC 4122, SHA-1 based): deterministic UUID from NAMESPACE_DNS + name. # Pure bash + sha1sum — no Python required. uuid_v5() { local hash diff --git a/snapshot/envelope.go b/snapshot/envelope.go index d9b82162..82280568 100644 --- a/snapshot/envelope.go +++ b/snapshot/envelope.go @@ -29,7 +29,7 @@ var ErrEnvelopeMissing = errors.New("snapshot envelope missing") type SnapshotMeta struct { StorageConfigs []*types.StorageConfig `json:"storage_configs"` BootConfig *types.BootConfig `json:"boot_config,omitzero"` - NICMACs []string `json:"nic_macs,omitempty"` + NICMACs []string `json:"nic_macs,omitzero"` } // ReadSnapshotEnvelope reads /snapshot.json. diff --git a/snapshot/localfile/localfile_test.go b/snapshot/localfile/localfile_test.go index 11d74f8c..d44d5565 100644 --- a/snapshot/localfile/localfile_test.go +++ b/snapshot/localfile/localfile_test.go @@ -516,7 +516,10 @@ func TestDelete(t *testing.T) { t.Errorf("expected ErrNotFound after delete, got %v", err) } - list, _ := lf.List(ctx) + list, err := lf.List(ctx) + if err != nil { + t.Fatalf("List: %v", err) + } if len(list) != 0 { t.Errorf("expected 0 after delete, got %d", len(list)) } diff --git a/snapshot/localfile/meta_shim_test.go b/snapshot/localfile/meta_shim_test.go index 2b88130c..396cbe97 100644 --- a/snapshot/localfile/meta_shim_test.go +++ b/snapshot/localfile/meta_shim_test.go @@ -2,6 +2,7 @@ package localfile import ( "context" + "errors" "maps" "os" "path/filepath" @@ -52,6 +53,9 @@ func TestLegacyDifferentialTrace(t *testing.T) { if err != nil { return err } + if p == nil { + return errors.New("fixture record SNAP333333333333 missing") + } p.Pending = false p.SizeBytes = 8192 p.LastAccessedAt = t2 @@ -62,6 +66,9 @@ func TestLegacyDifferentialTrace(t *testing.T) { if err != nil { return err } + if n == nil { + return errors.New("fixture record SNAP111111111111 missing") + } n.LastAccessedAt = t2 if err := tx.Put("SNAP111111111111", n); err != nil { return err diff --git a/snapshot/localfile/teardown_test.go b/snapshot/localfile/teardown_test.go index a0e72a26..80254642 100644 --- a/snapshot/localfile/teardown_test.go +++ b/snapshot/localfile/teardown_test.go @@ -2,6 +2,7 @@ package localfile import ( "errors" + "fmt" "os" "testing" @@ -110,6 +111,9 @@ func injectSnapTombstone(t *testing.T, lf *LocalFile, id string, phase tombstone if err != nil { return err } + if rec == nil { + return fmt.Errorf("snapshot %s missing", id) + } cl, err := tombstone.MarshalCleanup(snapCleanup{ Name: rec.Name, Hypervisor: rec.Hypervisor, Pending: rec.Pending, }) diff --git a/utils/batch_test.go b/utils/batch_test.go index 17aac440..de6f3346 100644 --- a/utils/batch_test.go +++ b/utils/batch_test.go @@ -58,7 +58,7 @@ func TestForEach_PartialFailure(t *testing.T) { func TestForEach_EmptyIDs(t *testing.T) { result := ForEach(t.Context(), nil, func(_ context.Context, _ string) error { - t.Fatal("should not be called") + t.Error("should not be called") return nil }) @@ -90,12 +90,12 @@ func TestForEach_SingleID(t *testing.T) { } func TestForEach_Concurrent(t *testing.T) { - var peak atomic.Int32 - var cur atomic.Int32 + var peak, cur, ready atomic.Int32 + gate := make(chan struct{}) result := ForEach(t.Context(), []string{"a", "b", "c", "d", "e"}, func(_ context.Context, _ string) error { raisePeak(&peak, cur.Add(1)) - time.Sleep(50 * time.Millisecond) + awaitPair(&ready, gate) cur.Add(-1) return nil }) @@ -109,11 +109,12 @@ func TestForEach_Concurrent(t *testing.T) { } func TestForEach_WithConcurrencyLimit(t *testing.T) { - var peak atomic.Int32 - var cur atomic.Int32 + var peak, cur, ready atomic.Int32 + gate := make(chan struct{}) result := ForEach(t.Context(), []string{"a", "b", "c", "d", "e"}, func(_ context.Context, _ string) error { raisePeak(&peak, cur.Add(1)) + awaitPair(&ready, gate) time.Sleep(50 * time.Millisecond) cur.Add(-1) return nil @@ -182,7 +183,7 @@ func TestMap_FailFast(t *testing.T) { func TestMap_Empty(t *testing.T) { results, err := Map(t.Context(), []int{}, func(_ context.Context, _, _ int) (string, error) { - t.Fatal("should not be called") + t.Error("should not be called") return "", nil }) if err != nil { @@ -254,3 +255,13 @@ func raisePeak(peak *atomic.Int32, n int32) { } } } + +func awaitPair(ready *atomic.Int32, gate chan struct{}) { + if ready.Add(1) == 2 { + close(gate) + } + select { + case <-gate: + case <-time.After(2 * time.Second): + } +} diff --git a/utils/file_test.go b/utils/file_test.go index ec892815..a396deb9 100644 --- a/utils/file_test.go +++ b/utils/file_test.go @@ -95,7 +95,9 @@ func TestScanFileStems_Basic(t *testing.T) { t.Fatal(err) } } - os.Mkdir(filepath.Join(dir, "skip.erofs"), 0o755) + if err := os.Mkdir(filepath.Join(dir, "skip.erofs"), 0o755); err != nil { + t.Fatal(err) + } stems, err := ScanFileStems(dir, ".erofs") if err != nil { diff --git a/utils/poll_test.go b/utils/poll_test.go index fdbcf864..ca110473 100644 --- a/utils/poll_test.go +++ b/utils/poll_test.go @@ -60,8 +60,8 @@ func TestWaitFor_ContextCanceled(t *testing.T) { err := WaitFor(ctx, time.Second, 10*time.Millisecond, func() (bool, error) { return false, nil }) - if err == nil { - t.Fatal("expected error for canceled context") + if !errors.Is(err, context.Canceled) { + t.Fatalf("got %v, want context.Canceled", err) } } diff --git a/utils/process_test.go b/utils/process_test.go index 8f87603a..754e48d0 100644 --- a/utils/process_test.go +++ b/utils/process_test.go @@ -1,7 +1,9 @@ package utils import ( + "bufio" "context" + "errors" "os" "os/exec" "path/filepath" @@ -149,15 +151,7 @@ func TestTerminateProcess_SleepProcess(t *testing.T) { pid := cmd.Process.Pid waitForExec(t, pid, "60") - waitDone := make(chan struct{}) - go func() { - _ = cmd.Wait() - close(waitDone) - }() - defer func() { - _ = cmd.Process.Kill() - <-waitDone - }() + waitDone := reapChild(t, cmd) if !IsProcessAlive(pid) { t.Fatal("sleep process should be alive") @@ -200,28 +194,32 @@ func TestTerminateProcess_InvalidPID(t *testing.T) { } func TestTerminateProcess_SIGTERMIgnored_FallsBackToKill(t *testing.T) { - cmd := exec.Command("bash", "-c", `trap "" TERM; sleep 60; :`, "vmm-sock") + const grace = 200 * time.Millisecond + cmd := exec.Command("bash", "-c", `trap "" TERM; echo ready; sleep 60; :`, "vmm-sock") + stdout, err := cmd.StdoutPipe() + if err != nil { + t.Fatalf("stdout pipe: %v", err) + } if err := cmd.Start(); err != nil { t.Fatalf("start: %v", err) } pid := cmd.Process.Pid + if line, readErr := bufio.NewReader(stdout).ReadString('\n'); readErr != nil || line != "ready\n" { + _ = cmd.Process.Kill() + _ = cmd.Wait() + t.Fatalf("child did not install its TERM trap: %q, %v", line, readErr) + } waitForExec(t, pid, "vmm-sock") - waitDone := make(chan struct{}) - go func() { - _ = cmd.Wait() - close(waitDone) - }() - defer func() { - _ = cmd.Process.Kill() - <-waitDone - }() + waitDone := reapChild(t, cmd) - ctx := t.Context() - err := TerminateProcess(ctx, pid, "vmm-sock", 200*time.Millisecond) - if err != nil { + start := time.Now() + if err := TerminateProcess(t.Context(), pid, "vmm-sock", grace); err != nil { t.Fatalf("TerminateProcess: %v", err) } + if elapsed := time.Since(start); elapsed < grace { + t.Errorf("returned after %s, before the %s grace: SIGTERM was not ignored or SIGKILL came early", elapsed, grace) + } <-waitDone } @@ -268,15 +266,20 @@ func TestTerminateProcess_ContextCancelled(t *testing.T) { } pid := cmd.Process.Pid waitForExec(t, pid, "60") - defer func() { - _ = cmd.Process.Kill() - _ = cmd.Wait() - }() + + waitDone := reapChild(t, cmd) ctx, cancel := context.WithCancel(t.Context()) cancel() - _ = TerminateProcess(ctx, pid, "60", 100*time.Millisecond) + if err := TerminateProcess(ctx, pid, "60", 100*time.Millisecond); err != nil && !errors.Is(err, context.Canceled) { + t.Errorf("TerminateProcess = %v, want nil or context.Canceled", err) + } + select { + case <-waitDone: + case <-time.After(5 * time.Second): + t.Fatal("a canceled context left the process alive") + } } func TestWaitProcessExitReturnsOnExit(t *testing.T) { @@ -313,6 +316,20 @@ func TestWaitProcessExitTimesOutOnALiveProcess(t *testing.T) { } } +func reapChild(t *testing.T, cmd *exec.Cmd) <-chan struct{} { + t.Helper() + done := make(chan struct{}) + go func() { + _ = cmd.Wait() + close(done) + }() + t.Cleanup(func() { + _ = cmd.Process.Kill() + <-done + }) + return done +} + func waitForExec(t *testing.T, pid int, expectArg string) { t.Helper() if err := WaitFor(t.Context(), 5*time.Second, time.Millisecond, func() (bool, error) { diff --git a/utils/sparse_linux_test.go b/utils/sparse_linux_test.go index c3890fca..e66e2a04 100644 --- a/utils/sparse_linux_test.go +++ b/utils/sparse_linux_test.go @@ -83,56 +83,6 @@ func TestSparseCopy_PartialData(t *testing.T) { } } -func TestSparseCopy_NonSparse(t *testing.T) { - dir := t.TempDir() - src := filepath.Join(dir, "src") - dst := filepath.Join(dir, "dst") - - data := bytes.Repeat([]byte("X"), 64*1024) - if err := os.WriteFile(src, data, 0o644); err != nil { - t.Fatal(err) - } - - if err := SparseCopy(dst, src, Sync); err != nil { - t.Fatalf("SparseCopy: %v", err) - } - - dstData := readFull(t, dst) - if !bytes.Equal(data, dstData) { - t.Error("content mismatch") - } -} - -func TestSparseCopy_EmptyFile(t *testing.T) { - dir := t.TempDir() - src := filepath.Join(dir, "src") - dst := filepath.Join(dir, "dst") - - if err := os.WriteFile(src, nil, 0o644); err != nil { - t.Fatal(err) - } - - if err := SparseCopy(dst, src, Sync); err != nil { - t.Fatalf("SparseCopy: %v", err) - } - - info, err := os.Stat(dst) - if err != nil { - t.Fatal(err) - } - if info.Size() != 0 { - t.Errorf("expected 0 size, got %d", info.Size()) - } -} - -func TestSparseCopy_SrcNotExist(t *testing.T) { - dir := t.TempDir() - err := SparseCopy(filepath.Join(dir, "dst"), filepath.Join(dir, "nonexistent"), Sync) - if err == nil { - t.Fatal("expected error for nonexistent src") - } -} - func TestSparseCopy_MultiSegment(t *testing.T) { dir := t.TempDir() src := filepath.Join(dir, "src")