From 918317ebb42f01e6627db603bbe994a064d274f2 Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Wed, 15 Jul 2026 16:46:22 -0400 Subject: [PATCH 1/4] Cut accidental test teardown waits --- .../auto_standby_integration_linux_test.go | 2 +- .../compression_integration_linux_test.go | 4 +- lib/instances/delete.go | 14 ++++++- .../docker_integration_linux_test.go | 2 +- .../egress_proxy_integration_test.go | 4 +- lib/instances/firecracker_test.go | 40 +++++++++---------- lib/instances/fork_speed_darwin_test.go | 2 +- lib/instances/fork_test.go | 14 +++---- lib/instances/manager_darwin_test.go | 4 +- lib/instances/manager_test.go | 17 ++++++++ lib/instances/qemu_test.go | 4 +- lib/instances/snapshot_compression.go | 18 ++++++++- .../snapshot_compression_native_test.go | 2 + .../snapshot_integration_scenario_test.go | 4 +- .../snapshot_running_source_darwin_test.go | 4 +- lib/instances/snapshot_test.go | 4 +- 16 files changed, 93 insertions(+), 46 deletions(-) diff --git a/lib/instances/auto_standby_integration_linux_test.go b/lib/instances/auto_standby_integration_linux_test.go index 7114da3d..96c61627 100644 --- a/lib/instances/auto_standby_integration_linux_test.go +++ b/lib/instances/auto_standby_integration_linux_test.go @@ -128,7 +128,7 @@ func TestAutoStandbyCloudHypervisorActiveInboundTCP(t *testing.T) { t.Cleanup(func() { logInstanceArtifactsOnFailure(t, mgr, instanceID) - _ = mgr.DeleteInstance(context.Background(), instanceID) + _ = deleteTestInstanceNow(context.Background(), mgr, instanceID) }) inst, err = waitForInstanceState(ctx, mgr, instanceID, StateRunning, 30*time.Second) diff --git a/lib/instances/compression_integration_linux_test.go b/lib/instances/compression_integration_linux_test.go index ca92a495..23a04c96 100644 --- a/lib/instances/compression_integration_linux_test.go +++ b/lib/instances/compression_integration_linux_test.go @@ -157,7 +157,7 @@ func runStandbyRestoreCompressionScenarios(t *testing.T, harness compressionInte deleted := false t.Cleanup(func() { if !deleted { - _ = mgr.DeleteInstance(context.Background(), inst.Id) + _ = deleteTestInstanceNow(context.Background(), mgr, inst.Id) } }) @@ -196,7 +196,7 @@ func runStandbyRestoreCompressionScenarios(t *testing.T, harness compressionInte inst = runCompressionCycle(t, ctx, mgr, p, inst, harness.waitHypervisorUp, tc.name, tc.cfg, true) } - require.NoError(t, mgr.DeleteInstance(ctx, inst.Id)) + require.NoError(t, deleteTestInstanceNow(ctx, mgr, inst.Id)) deleted = true } diff --git a/lib/instances/delete.go b/lib/instances/delete.go index 7e175c4f..d4499f69 100644 --- a/lib/instances/delete.go +++ b/lib/instances/delete.go @@ -17,10 +17,22 @@ import ( const deleteGracefulShutdownTimeout = 2 +type deleteInstanceOptions struct { + skipGracefulShutdown bool +} + // deleteInstance stops and deletes an instance func (m *manager) deleteInstance( ctx context.Context, id string, +) (retErr error) { + return m.deleteInstanceWithOptions(ctx, id, deleteInstanceOptions{}) +} + +func (m *manager) deleteInstanceWithOptions( + ctx context.Context, + id string, + options deleteInstanceOptions, ) (retErr error) { log := logger.FromContext(ctx) log.InfoContext(ctx, "deleting instance", "instance_id", id) @@ -76,7 +88,7 @@ func (m *manager) deleteInstance( // 4. If active, try graceful guest shutdown before force kill. gracefulShutdown := false - if inst.State == StateRunning || inst.State == StateInitializing { + if !options.skipGracefulShutdown && (inst.State == StateRunning || inst.State == StateInitializing) { stopTimeout := resolveStopTimeout(stored) if stopTimeout > deleteGracefulShutdownTimeout { stopTimeout = deleteGracefulShutdownTimeout diff --git a/lib/instances/docker_integration_linux_test.go b/lib/instances/docker_integration_linux_test.go index 0f7a9fc0..45face66 100644 --- a/lib/instances/docker_integration_linux_test.go +++ b/lib/instances/docker_integration_linux_test.go @@ -53,7 +53,7 @@ func TestDockerInVMCloudHypervisorWithAttachedVolume(t *testing.T) { t.Logf("dockerd log (exit=%d):\n%s", code, output) } } - _ = manager.DeleteInstance(context.Background(), inst.Id) + _ = deleteTestInstanceNow(context.Background(), manager, inst.Id) } _ = volumeManager.DeleteVolume(context.Background(), vol.Id) }) diff --git a/lib/instances/egress_proxy_integration_test.go b/lib/instances/egress_proxy_integration_test.go index e2b9e125..f866733e 100644 --- a/lib/instances/egress_proxy_integration_test.go +++ b/lib/instances/egress_proxy_integration_test.go @@ -103,7 +103,7 @@ func TestEgressProxyRewritesHTTPSHeaders(t *testing.T) { deleted := false t.Cleanup(func() { if !deleted { - _ = manager.DeleteInstance(context.Background(), inst.Id) + _ = deleteTestInstanceNow(context.Background(), manager, inst.Id) } }) @@ -192,7 +192,7 @@ func TestEgressProxyRewritesHTTPSHeaders(t *testing.T) { require.Contains(t, rotatedOutput, "Bearer rotated-key-456") require.NotContains(t, rotatedOutput, "real-openai-key-123") - require.NoError(t, manager.DeleteInstance(ctx, inst.Id)) + require.NoError(t, deleteTestInstanceNow(ctx, manager, inst.Id)) deleted = true } diff --git a/lib/instances/firecracker_test.go b/lib/instances/firecracker_test.go index 263e683b..a8e67145 100644 --- a/lib/instances/firecracker_test.go +++ b/lib/instances/firecracker_test.go @@ -181,7 +181,7 @@ func TestFirecrackerStandbyAndRestore(t *testing.T) { deleted := false t.Cleanup(func() { if !deleted { - _ = mgr.DeleteInstance(context.Background(), inst.Id) + _ = deleteTestInstanceNow(context.Background(), mgr, inst.Id) } }) @@ -521,7 +521,7 @@ func TestFirecrackerForkFromRunningNetwork(t *testing.T) { source, err = waitForInstanceState(ctx, mgr, source.Id, StateRunning, integrationTestTimeout(20*time.Second)) require.NoError(t, err) sourceID := source.Id - t.Cleanup(func() { _ = mgr.DeleteInstance(context.Background(), sourceID) }) + t.Cleanup(func() { _ = deleteTestInstanceNow(context.Background(), mgr, sourceID) }) assert.NotEmpty(t, source.IP) assert.NotEmpty(t, source.MAC) @@ -540,7 +540,7 @@ func TestFirecrackerForkFromRunningNetwork(t *testing.T) { require.NoError(t, err) require.Equal(t, StateRunning, forked.State) forkID := forked.Id - t.Cleanup(func() { _ = mgr.DeleteInstance(context.Background(), forkID) }) + t.Cleanup(func() { _ = deleteTestInstanceNow(context.Background(), mgr, forkID) }) assert.NotEmpty(t, forked.IP) assert.NotEmpty(t, forked.MAC) assert.Equal(t, mgr.paths.InstanceVsockSocket(forkID), forked.VsockSocket) @@ -610,7 +610,7 @@ func TestFirecrackerWarmForkChain(t *testing.T) { sourceDeleted := false t.Cleanup(func() { if !sourceDeleted { - _ = mgr.DeleteInstance(context.Background(), sourceID) + _ = deleteTestInstanceNow(context.Background(), mgr, sourceID) } }) @@ -625,7 +625,7 @@ func TestFirecrackerWarmForkChain(t *testing.T) { require.NoError(t, err) require.Equal(t, SnapshotKindStandby, snapshot.Kind) - require.NoError(t, mgr.DeleteInstance(ctx, sourceID)) + require.NoError(t, deleteTestInstanceNow(ctx, mgr, sourceID)) sourceDeleted = true warm, err := mgr.ForkSnapshot(ctx, snapshot.Id, ForkSnapshotRequest{ @@ -637,7 +637,7 @@ func TestFirecrackerWarmForkChain(t *testing.T) { warmDeleted := false t.Cleanup(func() { if !warmDeleted { - _ = mgr.DeleteInstance(context.Background(), warmID) + _ = deleteTestInstanceNow(context.Background(), mgr, warmID) } }) warm, err = waitForInstanceState(ctx, mgr, warmID, StateRunning, integrationTestTimeout(20*time.Second)) @@ -652,7 +652,7 @@ func TestFirecrackerWarmForkChain(t *testing.T) { require.NoError(t, err) require.Equal(t, StateStopped, child.State) childID := child.Id - t.Cleanup(func() { _ = mgr.DeleteInstance(context.Background(), childID) }) + t.Cleanup(func() { _ = deleteTestInstanceNow(context.Background(), mgr, childID) }) warm, err = mgr.GetInstance(ctx, warmID) require.NoError(t, err) @@ -663,7 +663,7 @@ func TestFirecrackerWarmForkChain(t *testing.T) { require.Equal(t, StateRunning, warm.State) require.NoError(t, waitForExecAgent(ctx, mgr, warmID, 30*time.Second)) - require.NoError(t, mgr.DeleteInstance(ctx, warmID)) + require.NoError(t, deleteTestInstanceNow(ctx, mgr, warmID)) warmDeleted = true require.NoError(t, mgr.DeleteSnapshot(ctx, snapshot.Id)) } @@ -716,7 +716,7 @@ func TestFCUFFDOneShotLifecycle(t *testing.T) { sourceDeleted := false t.Cleanup(func() { if !sourceDeleted { - _ = mgr.DeleteInstance(context.Background(), sourceID) + _ = deleteTestInstanceNow(context.Background(), mgr, sourceID) } }) @@ -753,7 +753,7 @@ func TestFCUFFDOneShotLifecycle(t *testing.T) { parentDeleted := false t.Cleanup(func() { if !parentDeleted { - _ = mgr.DeleteInstance(context.Background(), parentID) + _ = deleteTestInstanceNow(context.Background(), mgr, parentID) } }) parent = requireRunningSleepInstance(t, ctx, mgr, parentID) @@ -805,7 +805,7 @@ func TestFCUFFDOneShotLifecycle(t *testing.T) { childDeleted := false t.Cleanup(func() { if !childDeleted { - _ = mgr.DeleteInstance(context.Background(), childID) + _ = deleteTestInstanceNow(context.Background(), mgr, childID) } }) @@ -864,11 +864,11 @@ func TestFCUFFDOneShotLifecycle(t *testing.T) { require.NoError(t, err) require.Equal(t, StateStandby, child.State) - require.NoError(t, mgr.DeleteInstance(ctx, childID)) + require.NoError(t, deleteTestInstanceNow(ctx, mgr, childID)) childDeleted = true - require.NoError(t, mgr.DeleteInstance(ctx, parentID)) + require.NoError(t, deleteTestInstanceNow(ctx, mgr, parentID)) parentDeleted = true - require.NoError(t, mgr.DeleteInstance(ctx, sourceID)) + require.NoError(t, deleteTestInstanceNow(ctx, mgr, sourceID)) sourceDeleted = true require.NoError(t, mgr.DeleteSnapshot(ctx, snapshot.Id)) snapshotDeleted = true @@ -922,7 +922,7 @@ func TestFCUFFDGraduationLifecycle(t *testing.T) { sourceDeleted := false t.Cleanup(func() { if !sourceDeleted { - _ = mgr.DeleteInstance(context.Background(), sourceID) + _ = deleteTestInstanceNow(context.Background(), mgr, sourceID) } }) @@ -958,7 +958,7 @@ func TestFCUFFDGraduationLifecycle(t *testing.T) { parentDeleted := false t.Cleanup(func() { if !parentDeleted { - _ = mgr.DeleteInstance(context.Background(), parentID) + _ = deleteTestInstanceNow(context.Background(), mgr, parentID) } }) @@ -1020,9 +1020,9 @@ func TestFCUFFDGraduationLifecycle(t *testing.T) { require.NoError(t, err) require.Empty(t, parentMeta.StoredMetadata.FirecrackerUFFDSessionID, "file-backed restore after graduation should not create a pager session") - require.NoError(t, mgr.DeleteInstance(ctx, parentID)) + require.NoError(t, deleteTestInstanceNow(ctx, mgr, parentID)) parentDeleted = true - require.NoError(t, mgr.DeleteInstance(ctx, sourceID)) + require.NoError(t, deleteTestInstanceNow(ctx, mgr, sourceID)) sourceDeleted = true require.NoError(t, mgr.DeleteSnapshot(ctx, snapshot.Id)) snapshotDeleted = true @@ -1153,7 +1153,7 @@ func TestFirecrackerForkIsolation(t *testing.T) { sourceDeleted := false t.Cleanup(func() { if !sourceDeleted { - _ = mgr.DeleteInstance(context.Background(), sourceID) + _ = deleteTestInstanceNow(context.Background(), mgr, sourceID) } }) @@ -1193,7 +1193,7 @@ func TestFirecrackerForkIsolation(t *testing.T) { forkDeleted := false t.Cleanup(func() { if !forkDeleted { - _ = mgr.DeleteInstance(context.Background(), forkID) + _ = deleteTestInstanceNow(context.Background(), mgr, forkID) } }) require.Equal(t, StateStandby, fork.State) diff --git a/lib/instances/fork_speed_darwin_test.go b/lib/instances/fork_speed_darwin_test.go index 2a91fca9..f1f7bd4a 100644 --- a/lib/instances/fork_speed_darwin_test.go +++ b/lib/instances/fork_speed_darwin_test.go @@ -86,7 +86,7 @@ func TestVZForkSpeed(t *testing.T) { dumpVZShimLogs(t, tmpDir) require.NoError(t, err) } - t.Cleanup(func() { _ = mgr.DeleteInstance(context.Background(), source.Id) }) + t.Cleanup(func() { _ = deleteTestInstanceNow(context.Background(), mgr, source.Id) }) require.NoError(t, waitForExecAgent(ctx, mgr, source.Id, 60*time.Second)) // Fill the overlay with dense (random) data so the fork's disk copy has real diff --git a/lib/instances/fork_test.go b/lib/instances/fork_test.go index 219bc6bb..3f1a8e3b 100644 --- a/lib/instances/fork_test.go +++ b/lib/instances/fork_test.go @@ -892,7 +892,7 @@ func TestForkCloudHypervisorFromRunningNetwork(t *testing.T) { }) require.NoError(t, err) sourceID := source.Id - t.Cleanup(func() { _ = manager.DeleteInstance(context.Background(), sourceID) }) + t.Cleanup(func() { _ = deleteTestInstanceNow(context.Background(), manager, sourceID) }) source, err = waitForInstanceState(ctx, manager, source.Id, StateRunning, integrationTestTimeout(20*time.Second)) require.NoError(t, err) require.NoError(t, waitForVMReady(ctx, source.SocketPath, 5*time.Second)) @@ -918,7 +918,7 @@ func TestForkCloudHypervisorFromRunningNetwork(t *testing.T) { require.NoError(t, err) require.Equal(t, StateRunning, forked.State) forkedID := forked.Id - t.Cleanup(func() { _ = manager.DeleteInstance(context.Background(), forkedID) }) + t.Cleanup(func() { _ = deleteTestInstanceNow(context.Background(), manager, forkedID) }) // Source should be restored and still reachable by its private IP. sourceAfterFork, err := manager.GetInstance(ctx, source.Id) @@ -998,7 +998,7 @@ func runWarmForkChain(t *testing.T, mgr *manager, tmpDir string, cfg warmForkCha sourceDeleted := false t.Cleanup(func() { if !sourceDeleted { - _ = mgr.DeleteInstance(context.Background(), sourceID) + _ = deleteTestInstanceNow(context.Background(), mgr, sourceID) } }) @@ -1019,7 +1019,7 @@ func runWarmForkChain(t *testing.T, mgr *manager, tmpDir string, cfg warmForkCha } }) - require.NoError(t, mgr.DeleteInstance(ctx, sourceID)) + require.NoError(t, deleteTestInstanceNow(ctx, mgr, sourceID)) sourceDeleted = true warm, err := mgr.ForkSnapshot(ctx, snapshot.Id, ForkSnapshotRequest{ @@ -1031,7 +1031,7 @@ func runWarmForkChain(t *testing.T, mgr *manager, tmpDir string, cfg warmForkCha warmDeleted := false t.Cleanup(func() { if !warmDeleted { - _ = mgr.DeleteInstance(context.Background(), warmID) + _ = deleteTestInstanceNow(context.Background(), mgr, warmID) } }) warm, err = waitForInstanceState(ctx, mgr, warmID, StateRunning, readyTimeout) @@ -1046,7 +1046,7 @@ func runWarmForkChain(t *testing.T, mgr *manager, tmpDir string, cfg warmForkCha require.NoError(t, err) require.Equal(t, StateStopped, child.State) childID := child.Id - t.Cleanup(func() { _ = mgr.DeleteInstance(context.Background(), childID) }) + t.Cleanup(func() { _ = deleteTestInstanceNow(context.Background(), mgr, childID) }) warm, err = mgr.GetInstance(ctx, warmID) require.NoError(t, err) @@ -1057,7 +1057,7 @@ func runWarmForkChain(t *testing.T, mgr *manager, tmpDir string, cfg warmForkCha require.Equal(t, StateRunning, warm.State) require.NoError(t, waitForExecAgent(ctx, mgr, warmID, readyTimeout)) - require.NoError(t, mgr.DeleteInstance(ctx, warmID)) + require.NoError(t, deleteTestInstanceNow(ctx, mgr, warmID)) warmDeleted = true require.NoError(t, mgr.DeleteSnapshot(ctx, snapshot.Id)) snapshotDeleted = true diff --git a/lib/instances/manager_darwin_test.go b/lib/instances/manager_darwin_test.go index 2a3859e5..0c22cbf0 100644 --- a/lib/instances/manager_darwin_test.go +++ b/lib/instances/manager_darwin_test.go @@ -604,7 +604,7 @@ func TestVZForkFromRunningNetwork(t *testing.T) { require.NotEmpty(t, source.MAC) sourceID := source.Id - t.Cleanup(func() { _ = mgr.DeleteInstance(context.Background(), sourceID) }) + t.Cleanup(func() { _ = deleteTestInstanceNow(context.Background(), mgr, sourceID) }) err = waitForExecAgent(ctx, mgr, sourceID, 30*time.Second) require.NoError(t, err, "source guest agent should be ready") @@ -633,7 +633,7 @@ func TestVZForkFromRunningNetwork(t *testing.T) { require.Contains(t, []State{StateInitializing, StateRunning}, forked.State) require.NotEqual(t, sourceID, forked.Id) forkID := forked.Id - t.Cleanup(func() { _ = mgr.DeleteInstance(context.Background(), forkID) }) + t.Cleanup(func() { _ = deleteTestInstanceNow(context.Background(), mgr, forkID) }) forked, err = waitForInstanceState(ctx, mgr, forkID, StateRunning, 30*time.Second) require.NoError(t, err) diff --git a/lib/instances/manager_test.go b/lib/instances/manager_test.go index b64ed210..a38ff6cc 100644 --- a/lib/instances/manager_test.go +++ b/lib/instances/manager_test.go @@ -76,6 +76,23 @@ func setupTestManager(t *testing.T) (*manager, string) { return mgr, tmpDir } +// deleteTestInstanceNow performs the same resource and metadata cleanup as +// DeleteInstance, but skips the guest grace period. Cleanup callbacks do not +// need to preserve guest state: the test has already made its assertions and +// is discarding the VM. +func deleteTestInstanceNow(ctx context.Context, mgr *manager, id string) error { + lock := mgr.getInstanceLock(id) + lock.Lock() + defer lock.Unlock() + + err := mgr.deleteInstanceWithOptions(ctx, id, deleteInstanceOptions{skipGracefulShutdown: true}) + if err == nil { + mgr.notifyLifecycleDelete(ctx, id) + mgr.instanceLocks.Delete(id) + } + return err +} + // waitForVMReady polls VM state via VMM API until it's running or times out func waitForVMReady(ctx context.Context, socketPath string, timeout time.Duration) error { deadline := time.Now().Add(timeout) diff --git a/lib/instances/qemu_test.go b/lib/instances/qemu_test.go index 927c7364..8f391994 100644 --- a/lib/instances/qemu_test.go +++ b/lib/instances/qemu_test.go @@ -929,7 +929,7 @@ func TestQEMUForkFromRunningNetwork(t *testing.T) { }) require.NoError(t, err) sourceID := source.Id - t.Cleanup(func() { _ = manager.DeleteInstance(context.Background(), sourceID) }) + t.Cleanup(func() { _ = deleteTestInstanceNow(context.Background(), manager, sourceID) }) // QEMU is the slowest Linux hypervisor under full-suite load on Deft. Give // the host-side Running transition more headroom so we don't fail while the // VM is still legitimately completing boot marker hydration. @@ -953,7 +953,7 @@ func TestQEMUForkFromRunningNetwork(t *testing.T) { require.NoError(t, err) require.Equal(t, StateStandby, forked.State) forkedID := forked.Id - t.Cleanup(func() { _ = manager.DeleteInstance(context.Background(), forkedID) }) + t.Cleanup(func() { _ = deleteTestInstanceNow(context.Background(), manager, forkedID) }) sourceAfterFork, err := manager.GetInstance(ctx, source.Id) require.NoError(t, err) diff --git a/lib/instances/snapshot_compression.go b/lib/instances/snapshot_compression.go index 9f230a63..31752aa5 100644 --- a/lib/instances/snapshot_compression.go +++ b/lib/instances/snapshot_compression.go @@ -97,7 +97,23 @@ func (r nativeCodecRuntime) commandContextFunc() func(context.Context, string, . if r.commandContext != nil { return r.commandContext } - return exec.CommandContext + return newNativeCodecCommand +} + +func newNativeCodecCommand(ctx context.Context, name string, args ...string) *exec.Cmd { + cmd := exec.CommandContext(ctx, name, args...) + // Kill the entire process group on cancellation. Killing only the direct + // child can leave helpers holding stdout/stderr pipes open, which makes + // Cmd.Wait block until those helpers exit on their own. + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + cmd.Cancel = func() error { + err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) + if err == syscall.ESRCH { + return os.ErrProcessDone + } + return err + } + return cmd } func cloneCompressionConfig(cfg *snapshotstore.SnapshotCompressionConfig) *snapshotstore.SnapshotCompressionConfig { diff --git a/lib/instances/snapshot_compression_native_test.go b/lib/instances/snapshot_compression_native_test.go index 2b655897..79285402 100644 --- a/lib/instances/snapshot_compression_native_test.go +++ b/lib/instances/snapshot_compression_native_test.go @@ -322,6 +322,7 @@ func TestCompressSnapshotMemoryFileReturnsContextCanceledWhenNativeProcessIsKill ctx, cancel := context.WithCancel(baseCtx) rawPath, _ := writeRawSnapshotMemoryFile(t) binaryPath := writeExecutableScript(t, "zstd", "#!/bin/sh\nsleep 30\n") + started := time.Now() time.AfterFunc(20*time.Millisecond, cancel) _, _, err := compressSnapshotMemoryFileWithRuntime(ctx, nativeCodecRuntime{ @@ -333,6 +334,7 @@ func TestCompressSnapshotMemoryFileReturnsContextCanceledWhenNativeProcessIsKill Level: intPtr(1), }) require.ErrorIs(t, err, context.Canceled) + require.Less(t, time.Since(started), time.Second, "cancellation must terminate native helper descendants promptly") assert.Empty(t, logs.warnRecords()) var rm metricdata.ResourceMetrics diff --git a/lib/instances/snapshot_integration_scenario_test.go b/lib/instances/snapshot_integration_scenario_test.go index 7a5c3f25..1266a169 100644 --- a/lib/instances/snapshot_integration_scenario_test.go +++ b/lib/instances/snapshot_integration_scenario_test.go @@ -62,7 +62,7 @@ func runStandbySnapshotScenario(t *testing.T, mgr *manager, tmpDir string, cfg s sourceDeleted := false t.Cleanup(func() { if !sourceDeleted { - _ = mgr.DeleteInstance(context.Background(), sourceID) + _ = deleteTestInstanceNow(context.Background(), mgr, sourceID) } }) @@ -103,7 +103,7 @@ func runStandbySnapshotScenario(t *testing.T, mgr *manager, tmpDir string, cfg s require.Equal(t, StateStandby, forked.State) forkID := forked.Id - t.Cleanup(func() { _ = mgr.DeleteInstance(context.Background(), forkID) }) + t.Cleanup(func() { _ = deleteTestInstanceNow(context.Background(), mgr, forkID) }) currentFork, err := mgr.GetInstance(ctx, forkID) requireNoErr(err) require.Equal(t, StateStandby, currentFork.State) diff --git a/lib/instances/snapshot_running_source_darwin_test.go b/lib/instances/snapshot_running_source_darwin_test.go index 3f74288d..1bbdfa74 100644 --- a/lib/instances/snapshot_running_source_darwin_test.go +++ b/lib/instances/snapshot_running_source_darwin_test.go @@ -76,7 +76,7 @@ func TestVZRunningSnapshotRoundTrip(t *testing.T) { require.NoError(t, err) } sourceID := source.Id - t.Cleanup(func() { _ = mgr.DeleteInstance(context.Background(), sourceID) }) + t.Cleanup(func() { _ = deleteTestInstanceNow(context.Background(), mgr, sourceID) }) require.NoError(t, waitForExecAgent(ctx, mgr, sourceID, 60*time.Second), "guest agent should be ready") source, err = waitForInstanceState(ctx, mgr, sourceID, StateRunning, 30*time.Second) @@ -127,7 +127,7 @@ func TestVZRunningSnapshotRoundTrip(t *testing.T) { require.NoError(t, err) } forkID := forked.Id - t.Cleanup(func() { _ = mgr.DeleteInstance(context.Background(), forkID) }) + t.Cleanup(func() { _ = deleteTestInstanceNow(context.Background(), mgr, forkID) }) restored, err := mgr.RestoreInstance(ctx, forkID) if err != nil { diff --git a/lib/instances/snapshot_test.go b/lib/instances/snapshot_test.go index e395bf9b..c2341cc7 100644 --- a/lib/instances/snapshot_test.go +++ b/lib/instances/snapshot_test.go @@ -53,7 +53,7 @@ func TestStoppedSnapshotLifecycleAndForkAfterSourceDeletion(t *testing.T) { require.NoError(t, err) require.Equal(t, StateStopped, forked.State) require.Equal(t, hvType, forked.HypervisorType) - t.Cleanup(func() { _ = mgr.DeleteInstance(context.Background(), forked.Id) }) + t.Cleanup(func() { _ = deleteTestInstanceNow(context.Background(), mgr, forked.Id) }) require.NoError(t, mgr.DeleteSnapshot(ctx, snap.Id)) _, err = mgr.GetSnapshot(ctx, snap.Id) @@ -325,7 +325,7 @@ func TestForkSnapshotFromCompressedSourceCopiesRawMemory(t *testing.T) { TargetState: StateStopped, }) require.NoError(t, err) - t.Cleanup(func() { _ = mgr.DeleteInstance(context.Background(), forked.Id) }) + t.Cleanup(func() { _ = deleteTestInstanceNow(context.Background(), mgr, forked.Id) }) forkSnapshotDir := mgr.paths.InstanceDir(forked.Id) _, ok := findRawSnapshotMemoryFile(forkSnapshotDir) From 1a802179996b31b02c94040745acdcd56c8570f1 Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Wed, 15 Jul 2026 16:49:34 -0400 Subject: [PATCH 2/4] Reuse VM-ready images across tests --- .github/workflows/test.yml | 1 + cmd/test-prewarm/main.go | 21 ++++++++++++- .../auto_standby_integration_linux_test.go | 2 +- .../compression_integration_linux_test.go | 2 +- .../egress_proxy_integration_test.go | 17 ++-------- lib/instances/firecracker_test.go | 31 +++++-------------- lib/snapshot/testsupport/images.go | 6 +++- 7 files changed, 37 insertions(+), 43 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 005a4118..87448884 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -121,6 +121,7 @@ jobs: # cross-filesystem safe. Keeping both on /ci avoids that. export HYPEMAN_TEST_PREWARM_DIR="/ci/prewarm/linux-amd64" mkdir -p "$HYPEMAN_TEST_PREWARM_DIR" + sudo chown -R "$(id -u):$(id -g)" "$HYPEMAN_TEST_PREWARM_DIR" go run ./cmd/test-prewarm - name: Check gofmt diff --git a/cmd/test-prewarm/main.go b/cmd/test-prewarm/main.go index 23601987..02e5b92b 100644 --- a/cmd/test-prewarm/main.go +++ b/cmd/test-prewarm/main.go @@ -106,6 +106,11 @@ func main() { PrewarmDir: prewarmDir, Images: make([]manifestImage, 0, len(imagesToWarm)), } + p := paths.New(prewarmDir) + readyImageManager, err := images.NewManager(p, 1, nil) + if err != nil { + fatalf("create ready image manager: %v", err) + } for _, img := range imagesToWarm { entry, err := ensureMirroredImage(ctx, inspectClient, registry, img) @@ -113,10 +118,24 @@ func main() { fatalf("prewarm image %s: %v", img.Source, err) } fmt.Printf("prewarm image source=%s local=%s digest=%s cache_hit=%t\n", entry.Source, entry.LocalRef, entry.Digest, entry.CacheHit) + created, err := readyImageManager.CreateImage(ctx, images.CreateImageRequest{Name: entry.LocalRef, Platform: img.Platform}) + if err != nil { + fatalf("build ready image %s: %v", entry.LocalRef, err) + } + waitName := created.Name + if created.Digest != "" { + ref, parseErr := images.ParseNormalizedRef(entry.LocalRef) + if parseErr != nil { + fatalf("parse ready image ref %s: %v", entry.LocalRef, parseErr) + } + waitName = ref.Repository() + "@" + created.Digest + } + if err := readyImageManager.WaitForReady(ctx, waitName); err != nil { + fatalf("wait for ready image %s: %v", entry.LocalRef, err) + } manifest.Images = append(manifest.Images, entry) } - p := paths.New(prewarmDir) systemMgr := system.NewManager(p) if err := systemMgr.EnsureSystemFiles(ctx); err != nil { fatalf("prewarm system files: %v", err) diff --git a/lib/instances/auto_standby_integration_linux_test.go b/lib/instances/auto_standby_integration_linux_test.go index 96c61627..2b13a77b 100644 --- a/lib/instances/auto_standby_integration_linux_test.go +++ b/lib/instances/auto_standby_integration_linux_test.go @@ -99,7 +99,7 @@ func TestAutoStandbyCloudHypervisorActiveInboundTCP(t *testing.T) { mgr, _ := setupCompressionTestManagerForHypervisor(t, hypervisor.TypeCloudHypervisor) require.NoError(t, mgr.networkManager.Initialize(ctx, nil)) require.NoError(t, mgr.systemManager.EnsureSystemFiles(ctx)) - createNginxImageAndWait(t, ctx, mgr.imageManager) + createNginxImageAndWait(t, ctx, mgr.paths, mgr.imageManager) connSource := autostandby.NewConntrackSource() if _, err := connSource.ListConnections(ctx); err != nil { diff --git a/lib/instances/compression_integration_linux_test.go b/lib/instances/compression_integration_linux_test.go index 23a04c96..4ee08898 100644 --- a/lib/instances/compression_integration_linux_test.go +++ b/lib/instances/compression_integration_linux_test.go @@ -137,7 +137,7 @@ func runStandbyRestoreCompressionScenarios(t *testing.T, harness compressionInte imageManager, err := images.NewManager(p, 1, nil) require.NoError(t, err) - createNginxImageAndWait(t, ctx, imageManager) + createNginxImageAndWait(t, ctx, p, imageManager) systemManager := system.NewManager(p) require.NoError(t, systemManager.EnsureSystemFiles(ctx)) diff --git a/lib/instances/egress_proxy_integration_test.go b/lib/instances/egress_proxy_integration_test.go index f866733e..e494743a 100644 --- a/lib/instances/egress_proxy_integration_test.go +++ b/lib/instances/egress_proxy_integration_test.go @@ -18,7 +18,7 @@ import ( "time" "github.com/kernel/hypeman/lib/egressproxy" - "github.com/kernel/hypeman/lib/images" + snapshottest "github.com/kernel/hypeman/lib/snapshot/testsupport" "github.com/stretchr/testify/require" ) @@ -49,20 +49,7 @@ func TestEgressProxyRewritesHTTPSHeaders(t *testing.T) { require.NoError(t, err) imageRef := integrationTestImageRef(t, "docker.io/library/nginx:alpine") - t.Logf("Pulling %s image...", imageRef) - created, err := manager.imageManager.CreateImage(ctx, images.CreateImageRequest{Name: imageRef}) - require.NoError(t, err) - - for i := 0; i < 120; i++ { - img, err := manager.imageManager.GetImage(ctx, created.Name) - if err == nil && img.Status == images.StatusReady { - break - } - time.Sleep(1 * time.Second) - } - img, err := manager.imageManager.GetImage(ctx, created.Name) - require.NoError(t, err) - require.Equal(t, images.StatusReady, img.Status) + snapshottest.EnsureImageReady(t, ctx, manager.paths, manager.imageManager, imageRef) require.NoError(t, manager.systemManager.EnsureSystemFiles(ctx)) require.NoError(t, manager.networkManager.Initialize(ctx, nil)) diff --git a/lib/instances/firecracker_test.go b/lib/instances/firecracker_test.go index a8e67145..30804441 100644 --- a/lib/instances/firecracker_test.go +++ b/lib/instances/firecracker_test.go @@ -104,26 +104,9 @@ func requireUserfaultfdIntegrationPrereqs(t *testing.T) { _ = file.Close() } -func createNginxImageAndWait(t *testing.T, ctx context.Context, imageManager images.Manager) { +func createNginxImageAndWait(t *testing.T, ctx context.Context, p *paths.Paths, imageManager images.Manager) { t.Helper() - - nginxImage, err := imageManager.CreateImage(ctx, images.CreateImageRequest{ - Name: integrationTestImageRef(t, "docker.io/library/nginx:alpine"), - }) - require.NoError(t, err) - - for i := 0; i < 60; i++ { - img, err := imageManager.GetImage(ctx, nginxImage.Name) - if err == nil && img.Status == images.StatusReady { - return - } - if err == nil && img.Status == images.StatusFailed { - t.Fatalf("image build failed: %s", *img.Error) - } - time.Sleep(1 * time.Second) - } - - t.Fatalf("timed out waiting for image %q to become ready", nginxImage.Name) + snapshottest.EnsureImageReady(t, ctx, p, imageManager, integrationTestImageRef(t, "docker.io/library/nginx:alpine")) } func startGatewayProbeServer(t *testing.T, gatewayIP string) (string, func()) { @@ -162,7 +145,7 @@ func TestFirecrackerStandbyAndRestore(t *testing.T) { imageManager, err := images.NewManager(p, 1, nil) require.NoError(t, err) - createNginxImageAndWait(t, ctx, imageManager) + createNginxImageAndWait(t, ctx, p, imageManager) systemManager := system.NewManager(p) require.NoError(t, systemManager.EnsureSystemFiles(ctx)) @@ -296,7 +279,7 @@ func TestFirecrackerStopClearsStaleSnapshot(t *testing.T) { imageManager, err := images.NewManager(p, 1, nil) require.NoError(t, err) - createNginxImageAndWait(t, ctx, imageManager) + createNginxImageAndWait(t, ctx, p, imageManager) systemManager := system.NewManager(p) require.NoError(t, systemManager.EnsureSystemFiles(ctx)) @@ -373,7 +356,7 @@ func TestFirecrackerNetworkLifecycle(t *testing.T) { imageManager, err := images.NewManager(p, 1, nil) require.NoError(t, err) - createNginxImageAndWait(t, ctx, imageManager) + createNginxImageAndWait(t, ctx, p, imageManager) systemManager := system.NewManager(p) require.NoError(t, systemManager.EnsureSystemFiles(ctx)) @@ -501,7 +484,7 @@ func TestFirecrackerForkFromRunningNetwork(t *testing.T) { imageManager, err := images.NewManager(p, 1, nil) require.NoError(t, err) - createNginxImageAndWait(t, ctx, imageManager) + createNginxImageAndWait(t, ctx, p, imageManager) systemManager := system.NewManager(p) require.NoError(t, systemManager.EnsureSystemFiles(ctx)) @@ -1132,7 +1115,7 @@ func TestFirecrackerForkIsolation(t *testing.T) { imageManager, err := images.NewManager(p, 1, nil) require.NoError(t, err) - createNginxImageAndWait(t, ctx, imageManager) + createNginxImageAndWait(t, ctx, p, imageManager) systemManager := system.NewManager(p) require.NoError(t, systemManager.EnsureSystemFiles(ctx)) diff --git a/lib/snapshot/testsupport/images.go b/lib/snapshot/testsupport/images.go index f996a6cb..78057a0e 100644 --- a/lib/snapshot/testsupport/images.go +++ b/lib/snapshot/testsupport/images.go @@ -38,7 +38,11 @@ func EnsureImageReady(t *testing.T, ctx context.Context, p *paths.Paths, imageMa ref, err := images.ParseNormalizedRef(image) require.NoError(t, err) - cachePaths := paths.New(filepath.Join(os.TempDir(), "hypeman-snapshot-image-cache")) + cacheDir := strings.TrimSpace(os.Getenv("HYPEMAN_TEST_PREWARM_DIR")) + if cacheDir == "" { + cacheDir = filepath.Join(os.TempDir(), "hypeman-snapshot-image-cache") + } + cachePaths := paths.New(cacheDir) cacheMgr, err := images.NewManager(cachePaths, 1, nil) require.NoError(t, err) From b4d95a42670a83b64d89bbf0ad2bd5151ce04693 Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Wed, 15 Jul 2026 16:56:04 -0400 Subject: [PATCH 3/4] Right-size lifecycle test guests --- lib/instances/compression_integration_linux_test.go | 2 +- lib/instances/firecracker_test.go | 10 +++++----- lib/instances/fork_test.go | 2 +- lib/instances/manager_test.go | 5 +++++ lib/instances/snapshot_integration_scenario_test.go | 2 +- 5 files changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/instances/compression_integration_linux_test.go b/lib/instances/compression_integration_linux_test.go index 4ee08898..a5fad06d 100644 --- a/lib/instances/compression_integration_linux_test.go +++ b/lib/instances/compression_integration_linux_test.go @@ -145,7 +145,7 @@ func runStandbyRestoreCompressionScenarios(t *testing.T, harness compressionInte inst, err := mgr.CreateInstance(ctx, CreateInstanceRequest{ Name: fmt.Sprintf("compression-%s", harness.name), Image: integrationTestImageRef(t, "docker.io/library/nginx:alpine"), - Size: 1024 * 1024 * 1024, + Size: lifecycleTestMemorySize, HotplugSize: 512 * 1024 * 1024, OverlaySize: 10 * 1024 * 1024 * 1024, Vcpus: 1, diff --git a/lib/instances/firecracker_test.go b/lib/instances/firecracker_test.go index 30804441..4e4245b2 100644 --- a/lib/instances/firecracker_test.go +++ b/lib/instances/firecracker_test.go @@ -153,7 +153,7 @@ func TestFirecrackerStandbyAndRestore(t *testing.T) { inst, err := mgr.CreateInstance(ctx, CreateInstanceRequest{ Name: "test-firecracker-standby", Image: integrationTestImageRef(t, "docker.io/library/nginx:alpine"), - Size: 1024 * 1024 * 1024, + Size: lifecycleTestMemorySize, OverlaySize: 10 * 1024 * 1024 * 1024, Vcpus: 1, NetworkEnabled: false, @@ -287,7 +287,7 @@ func TestFirecrackerStopClearsStaleSnapshot(t *testing.T) { inst, err := mgr.CreateInstance(ctx, CreateInstanceRequest{ Name: "fc-stale-snapshot", Image: integrationTestImageRef(t, "docker.io/library/nginx:alpine"), - Size: 1024 * 1024 * 1024, + Size: lifecycleTestMemorySize, OverlaySize: 10 * 1024 * 1024 * 1024, Vcpus: 1, NetworkEnabled: false, @@ -581,7 +581,7 @@ func TestFirecrackerWarmForkChain(t *testing.T) { source, err := mgr.CreateInstance(ctx, CreateInstanceRequest{ Name: "fc-warm-chain-src", Image: imageName, - Size: 1024 * 1024 * 1024, + Size: lifecycleTestMemorySize, OverlaySize: 1024 * 1024 * 1024, Vcpus: 1, NetworkEnabled: false, @@ -687,7 +687,7 @@ func TestFCUFFDOneShotLifecycle(t *testing.T) { source, err := mgr.CreateInstance(ctx, CreateInstanceRequest{ Name: "fc-uffd-oneshot-src", Image: imageName, - Size: 1024 * 1024 * 1024, + Size: lifecycleTestMemorySize, OverlaySize: 1024 * 1024 * 1024, Vcpus: 1, NetworkEnabled: false, @@ -893,7 +893,7 @@ func TestFCUFFDGraduationLifecycle(t *testing.T) { source, err := mgr.CreateInstance(ctx, CreateInstanceRequest{ Name: "fc-uffd-grad-src", Image: imageName, - Size: 1024 * 1024 * 1024, + Size: lifecycleTestMemorySize, OverlaySize: 1024 * 1024 * 1024, Vcpus: 1, NetworkEnabled: false, diff --git a/lib/instances/fork_test.go b/lib/instances/fork_test.go index 3f1a8e3b..2dc63266 100644 --- a/lib/instances/fork_test.go +++ b/lib/instances/fork_test.go @@ -986,7 +986,7 @@ func runWarmForkChain(t *testing.T, mgr *manager, tmpDir string, cfg warmForkCha source, err := mgr.CreateInstance(ctx, CreateInstanceRequest{ Name: cfg.namePrefix + "-warm-chain-src", Image: imageName, - Size: 1024 * 1024 * 1024, + Size: lifecycleTestMemorySize, OverlaySize: 1024 * 1024 * 1024, Vcpus: 1, NetworkEnabled: false, diff --git a/lib/instances/manager_test.go b/lib/instances/manager_test.go index a38ff6cc..2000ae0e 100644 --- a/lib/instances/manager_test.go +++ b/lib/instances/manager_test.go @@ -34,6 +34,11 @@ import ( "github.com/stretchr/testify/require" ) +// lifecycleTestMemorySize is enough to boot the shared test guest while +// avoiding snapshotting memory that lifecycle tests never touch. Dedicated +// memory and resource-limit tests retain their larger sizes. +const lifecycleTestMemorySize = 512 * 1024 * 1024 + // setupTestManager creates a manager and registers cleanup for any orphaned processes func setupTestManager(t *testing.T) (*manager, string) { tmpDir := t.TempDir() diff --git a/lib/instances/snapshot_integration_scenario_test.go b/lib/instances/snapshot_integration_scenario_test.go index 1266a169..788f3ef6 100644 --- a/lib/instances/snapshot_integration_scenario_test.go +++ b/lib/instances/snapshot_integration_scenario_test.go @@ -49,7 +49,7 @@ func runStandbySnapshotScenario(t *testing.T, mgr *manager, tmpDir string, cfg s source, err := mgr.CreateInstance(ctx, CreateInstanceRequest{ Name: cfg.sourceName, Image: integrationTestImageRef(t, "docker.io/library/alpine:latest"), - Size: 1024 * 1024 * 1024, + Size: lifecycleTestMemorySize, OverlaySize: 10 * 1024 * 1024 * 1024, Vcpus: 1, NetworkEnabled: false, From de5fe4fece127e37ab21cd73732f3c79cec01627 Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Thu, 16 Jul 2026 09:55:22 -0400 Subject: [PATCH 4/4] Make codec cancellation regression deterministic Wait for the native helper child before canceling so the regression always exercises process-group termination. Also apply the two clarity cleanups found during self-review. --- lib/instances/delete.go | 2 +- .../snapshot_compression_native_test.go | 35 +++++++++++++------ lib/snapshot/testsupport/images.go | 4 +-- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/lib/instances/delete.go b/lib/instances/delete.go index d4499f69..07c3da1b 100644 --- a/lib/instances/delete.go +++ b/lib/instances/delete.go @@ -25,7 +25,7 @@ type deleteInstanceOptions struct { func (m *manager) deleteInstance( ctx context.Context, id string, -) (retErr error) { +) error { return m.deleteInstanceWithOptions(ctx, id, deleteInstanceOptions{}) } diff --git a/lib/instances/snapshot_compression_native_test.go b/lib/instances/snapshot_compression_native_test.go index 79285402..90fafbe0 100644 --- a/lib/instances/snapshot_compression_native_test.go +++ b/lib/instances/snapshot_compression_native_test.go @@ -320,19 +320,32 @@ func TestCompressSnapshotMemoryFileReturnsContextCanceledWhenNativeProcessIsKill mgr, reader := newCodecRuntimeTestManager(t) baseCtx, logs := newCodecRuntimeContext() ctx, cancel := context.WithCancel(baseCtx) + defer cancel() rawPath, _ := writeRawSnapshotMemoryFile(t) - binaryPath := writeExecutableScript(t, "zstd", "#!/bin/sh\nsleep 30\n") - started := time.Now() - time.AfterFunc(20*time.Millisecond, cancel) + readyPath := filepath.Join(t.TempDir(), "ready") + t.Setenv("HYPEMAN_CODEC_TEST_READY", readyPath) + binaryPath := writeExecutableScript(t, "zstd", "#!/bin/sh\nsleep 30 &\n: > \"$HYPEMAN_CODEC_TEST_READY\"\nwait\n") + + errCh := make(chan error, 1) + go func() { + _, _, err := compressSnapshotMemoryFileWithRuntime(ctx, nativeCodecRuntime{ + manager: mgr, + lookPath: func(string) (string, error) { return binaryPath, nil }, + }, rawPath, snapshotstore.SnapshotCompressionConfig{ + Enabled: true, + Algorithm: snapshotstore.SnapshotCompressionAlgorithmZstd, + Level: intPtr(1), + }) + errCh <- err + }() - _, _, err := compressSnapshotMemoryFileWithRuntime(ctx, nativeCodecRuntime{ - manager: mgr, - lookPath: func(string) (string, error) { return binaryPath, nil }, - }, rawPath, snapshotstore.SnapshotCompressionConfig{ - Enabled: true, - Algorithm: snapshotstore.SnapshotCompressionAlgorithmZstd, - Level: intPtr(1), - }) + require.Eventually(t, func() bool { + _, err := os.Stat(readyPath) + return err == nil + }, time.Second, 5*time.Millisecond, "native helper child must start before cancellation") + started := time.Now() + cancel() + err := <-errCh require.ErrorIs(t, err, context.Canceled) require.Less(t, time.Since(started), time.Second, "cancellation must terminate native helper descendants promptly") assert.Empty(t, logs.warnRecords()) diff --git a/lib/snapshot/testsupport/images.go b/lib/snapshot/testsupport/images.go index 78057a0e..d033cd0f 100644 --- a/lib/snapshot/testsupport/images.go +++ b/lib/snapshot/testsupport/images.go @@ -29,8 +29,8 @@ func imageBuildLock(ref string) *sync.Mutex { return m.(*sync.Mutex) } -// EnsureImageReady pre-warms a shared image cache under /tmp and seeds that -// image into the test data directory so instance integration tests don't need +// EnsureImageReady seeds an image from the configured shared prewarm cache, +// falling back to a cache under /tmp, so instance integration tests don't need // to repull/reconvert from scratch. func EnsureImageReady(t *testing.T, ctx context.Context, p *paths.Paths, imageManager images.Manager, image string) { t.Helper()