fix: a terminate or stop survives a daemon restart that finds QEMU alive (#1043)

Recovery tested whether the QEMU process was still running before it looked at the instance's state, so a live process short-circuited the transitional-state handling and reconnectInstance then set the instance back to running. A stop or terminate the caller had already been told was under way was silently discarded.

Check for StateStopping and StateShuttingDown first, kill a QEMU that outlived the transition, and finalize. reconnectInstance now refuses a transitional instance outright rather than promoting it.

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)
 	}

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