diff --git a/spinifex/vm/restore.go b/spinifex/vm/restore.go index 0fa205951..a986e28ba 100644 --- a/spinifex/vm/restore.go +++ b/spinifex/vm/restore.go @@ -175,6 +175,21 @@ func (m *Manager) classifyRestoredInstances() []*VM { } } + // A stop or terminate acknowledged before the restart outranks a + // surviving QEMU: finish what the caller asked for rather than + // re-advertising the instance as running. + if instance.Status == StateStopping || instance.Status == StateShuttingDown { + if isInstanceProcessRunning(instance) { + slog.Warn("QEMU outlived an in-flight transition, killing it to finish", + "instance", instance.ID, "status", instance.Status) + if !killOrphanedQEMU(instance) { + continue + } + } + m.finalizeTransitionalRestore(instance) + continue + } + if isInstanceProcessRunning(instance) { socketsValid := AreVolumeSocketsValid(instance) if !socketsValid && m.backingStoreReady() { @@ -205,14 +220,8 @@ func (m *Manager) classifyRestoredInstances() []*VM { } } - // QEMU is not running -- resolve transitional states from interrupted operations. - switch instance.Status { - case StateStopping, StateShuttingDown: - if m.finalizeTransitionalRestore(instance) { - continue - } - continue - case StateRunning: + // QEMU is not running -- an instance that was up relaunches from scratch. + if instance.Status == StateRunning { instance.Status = StatePending slog.Info("Instance was running but QEMU exited, relaunching", "instance", instance.ID) } @@ -408,6 +417,13 @@ var attachQMPForReconnect = (*Manager).AttachQMP // closes QMP and propagates the error; status is only set to Running after // subscriptions are confirmed live to avoid advertising a broken instance. func (m *Manager) reconnectInstance(instance *VM) error { + // Promoting a transitional instance to Running would silently discard a + // stop or terminate the caller already had acknowledged. Callers resolve + // those before reconnecting, so reaching here is a bug, not a state. + if instance.Status == StateStopping || instance.Status == StateShuttingDown { + return fmt.Errorf("refusing to reconnect instance in transitional state %s", instance.Status) + } + if err := attachQMPForReconnect(m, instance); err != nil { return fmt.Errorf("failed to reconnect QMP: %w", err) } diff --git a/spinifex/vm/restore_test.go b/spinifex/vm/restore_test.go index 10bac5116..0971f0da1 100644 --- a/spinifex/vm/restore_test.go +++ b/spinifex/vm/restore_test.go @@ -248,6 +248,85 @@ func TestClassifyRestoredInstances_ShuttingDownFinalizesToTerminated(t *testing. assert.NotNil(t, store.terminated[v.ID]) } +// spawnLiveChild starts a real child process and returns its PID, so a test can +// exercise the live-QEMU branch. killOrphanedQEMU SIGKILLs whatever PID it finds, +// so this must be a process the test owns and never the test process itself. +func spawnLiveChild(t *testing.T) int { + t.Helper() + cmd := exec.Command("sleep", "300") + require.NoError(t, cmd.Start()) + // Reap in the background. A killed child of this process would otherwise + // linger as a zombie, which still answers signal(0), so the production + // exit-wait would never observe it dying. Real QEMU is reparented to init. + reaped := make(chan struct{}) + go func() { _, _ = cmd.Process.Wait(); close(reaped) }() + t.Cleanup(func() { + select { + case <-reaped: + default: + _ = cmd.Process.Kill() + <-reaped + } + }) + return cmd.Process.Pid +} + +// A terminate acknowledged before a daemon restart must survive the restart. The +// guest can outlive the daemon when it refuses to power down, and reconnecting to +// it silently discarded the terminate and re-advertised the instance as running. +func TestClassifyRestoredInstances_ShuttingDownWithLiveQEMUTerminates(t *testing.T) { + m, store, _ := classifyTestManager(t) + t.Setenv("XDG_RUNTIME_DIR", t.TempDir()) + v := &VM{ID: "i-shutdown-live", Status: StateShuttingDown, InstanceType: "t3.micro"} + require.NoError(t, utils.WritePidFile(v.ID, spawnLiveChild(t))) + m.Replace(map[string]*VM{v.ID: v}) + + toLaunch := m.classifyRestoredInstances() + + assert.Empty(t, toLaunch, "a terminating instance must never be queued for relaunch") + assert.Equal(t, StateTerminated, v.Status, + "a surviving QEMU must not outrank an acknowledged terminate") + assert.NotNil(t, store.terminated[v.ID]) +} + +// The stop half of the same invariant: a stop already acknowledged must finalise +// to Stopped rather than reconnect, so a guest that ignored the power button does +// not come back running. +func TestClassifyRestoredInstances_StoppingWithLiveQEMUStops(t *testing.T) { + m, store, _ := classifyTestManager(t) + t.Setenv("XDG_RUNTIME_DIR", t.TempDir()) + v := &VM{ID: "i-stopping-live", Status: StateStopping, InstanceType: "t3.micro"} + require.NoError(t, utils.WritePidFile(v.ID, spawnLiveChild(t))) + m.Replace(map[string]*VM{v.ID: v}) + + toLaunch := m.classifyRestoredInstances() + + assert.Empty(t, toLaunch) + assert.Equal(t, StateStopped, v.Status, + "a surviving QEMU must not outrank an acknowledged stop") + assert.NotNil(t, store.stopped[v.ID]) +} + +// Belt and braces for the same invariant at its last enforcement point: even if a +// future caller reaches reconnectInstance with a transitional instance, it must +// refuse rather than promote it to Running. +func TestReconnectInstance_RefusesTransitionalState(t *testing.T) { + m, _, _ := classifyTestManager(t) + + // Succeed at every step a reconnect would otherwise fail on, so the refusal + // under test is the guard itself and not a missing QMP socket. + orig := attachQMPForReconnect + t.Cleanup(func() { attachQMPForReconnect = orig }) + attachQMPForReconnect = func(_ *Manager, v *VM) error { fakeAttachQMP(t, v); return nil } + + for _, status := range []InstanceState{StateStopping, StateShuttingDown} { + v := &VM{ID: "i-" + string(status), Status: status, InstanceType: "t3.micro"} + err := m.reconnectInstance(v) + require.Errorf(t, err, "reconnect must refuse an instance in %s", status) + assert.Equal(t, status, v.Status, "a refused reconnect must not change the status") + } +} + // TestClassifyRestoredInstances_RunningWithDeadPidQueuedForRelaunch // covers the no-PID-file path: Status was Running at shutdown but no // QEMU process is alive. classify must reset Status to Pending and