Expose RunnerPID and terminate stale runners before cleanup

Add VM.RunnerPID() convenience method that returns the runner process
ID directly, complementing the existing VM.ID() string accessor.

Update WithCleanDataDir to terminate orphaned runner processes before
wiping the data directory. Reads the persisted state file, checks if
the recorded PID is still alive, sends SIGTERM with a 5s grace period,
then SIGKILL if needed. Prevents orphaned runners from holding KVM
file descriptors and virtiofs mounts after a parent process crash.

Closes #3
Closes #5

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: JAORMX

options.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"os"
 	"path/filepath"
+	"syscall"
 
 	"github.com/stacklok/propolis/hypervisor"
 	"github.com/stacklok/propolis/image"
@@ -81,6 +82,8 @@ type config struct {
 	cleanDataDir          bool
 	removeAll             func(string) error
 	stat                  func(string) (os.FileInfo, error)
+	killProcess           func(pid int, sig syscall.Signal) error
+	processAlive          func(pid int) bool
 }
 
 func defaultConfig() *config {
@@ -96,6 +99,14 @@ func defaultConfig() *config {
 		dataDir:     dataDir,
 		removeAll:   forceRemoveAll,
 		stat:        os.Stat,
+		killProcess: func(pid int, sig syscall.Signal) error { return syscall.Kill(pid, sig) },
+		processAlive: func(pid int) bool {
+			proc, err := os.FindProcess(pid)
+			if err != nil {
+				return false
+			}
+			return proc.Signal(syscall.Signal(0)) == nil
+		},
 	}
 }
 

propolis.go

@@ -24,6 +24,8 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"syscall"
+	"time"
 
 	"github.com/stacklok/propolis/hypervisor"
 	"github.com/stacklok/propolis/hypervisor/libkrun"
@@ -226,6 +228,15 @@ func Run(ctx context.Context, imageRef string, opts ...Option) (*VM, error) {
 	return vm, nil
 }
 
+const (
+	// staleTermTimeout is the maximum time to wait for a stale runner to
+	// exit after SIGTERM before sending SIGKILL.
+	staleTermTimeout = 5 * time.Second
+	// staleTermPoll is the interval between liveness checks during stale
+	// runner termination.
+	staleTermPoll = 250 * time.Millisecond
+)
+
 func cleanDataDir(cfg *config) error {
 	if cfg.dataDir == "" {
 		return nil
@@ -238,6 +249,9 @@ func cleanDataDir(cfg *config) error {
 		return fmt.Errorf("check data dir: %w", err)
 	}
 
+	// Best-effort: terminate any stale runner process before wiping.
+	terminateStaleRunner(cfg)
+
 	var keep []string
 	cache := cacheDir(cfg)
 	if cache != "" && isWithin(cfg.dataDir, cache) {
@@ -259,6 +273,49 @@ func cleanDataDir(cfg *config) error {
 	return nil
 }
 
+// terminateStaleRunner checks the state file in the data directory for a
+// previously-running runner process. If the PID is alive, it sends SIGTERM
+// and waits up to staleTermTimeout before sending SIGKILL. This prevents
+// orphaned runner processes from holding KVM file descriptors and virtiofs
+// mounts when the parent process was hard-killed.
+func terminateStaleRunner(cfg *config) {
+	mgr := state.NewManager(cfg.dataDir)
+	st, err := mgr.Load()
+	if err != nil {
+		slog.Debug("could not load state for stale runner check", "error", err)
+		return
+	}
+	if st.PID <= 0 {
+		return
+	}
+	if !cfg.processAlive(st.PID) {
+		slog.Debug("stale runner already dead", "pid", st.PID)
+		return
+	}
+
+	slog.Warn("terminating stale runner process", "pid", st.PID)
+	if err := cfg.killProcess(st.PID, syscall.SIGTERM); err != nil {
+		slog.Warn("failed to send SIGTERM to stale runner", "pid", st.PID, "error", err)
+		return
+	}
+
+	// Poll until the process exits or the timeout expires.
+	deadline := time.Now().Add(staleTermTimeout)
+	for time.Now().Before(deadline) {
+		time.Sleep(staleTermPoll)
+		if !cfg.processAlive(st.PID) {
+			slog.Info("stale runner terminated gracefully", "pid", st.PID)
+			return
+		}
+	}
+
+	// Force kill.
+	slog.Warn("stale runner did not exit after SIGTERM, sending SIGKILL", "pid", st.PID)
+	if err := cfg.killProcess(st.PID, syscall.SIGKILL); err != nil {
+		slog.Warn("failed to send SIGKILL to stale runner", "pid", st.PID, "error", err)
+	}
+}
+
 // forceRemoveAll removes the given path, handling read-only directory trees
 // such as Go module caches (whose entries are set to 0444/0555). It first
 // attempts a plain os.RemoveAll; on failure it walks the tree making every

propolis_test.go

@@ -8,6 +8,8 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"sync"
+	"syscall"
 	"testing"
 
 	v1 "github.com/google/go-containerregistry/pkg/v1"
@@ -726,3 +728,191 @@ func TestWithBackend(t *testing.T) {
 	WithBackend(backend).apply(cfg)
 	assert.Equal(t, backend, cfg.backend)
 }
+
+// --- terminateStaleRunner tests ---
+
+func TestTerminateStaleRunner_NoStateFile(t *testing.T) {
+	t.Parallel()
+
+	dataDir := t.TempDir()
+	cfg := defaultConfig()
+	cfg.dataDir = dataDir
+
+	// Should not panic or error when no state file exists.
+	terminateStaleRunner(cfg)
+}
+
+func TestTerminateStaleRunner_DeadProcess(t *testing.T) {
+	t.Parallel()
+
+	dataDir := t.TempDir()
+
+	// Write state with a PID that doesn't exist.
+	mgr := state.NewManager(dataDir)
+	ls, err := mgr.LoadAndLock(context.Background())
+	require.NoError(t, err)
+	ls.State.Active = true
+	ls.State.PID = 2147483647 // max PID, almost certainly dead
+	require.NoError(t, ls.Save())
+	ls.Release()
+
+	cfg := defaultConfig()
+	cfg.dataDir = dataDir
+
+	var killCalled bool
+	cfg.killProcess = func(_ int, _ syscall.Signal) error {
+		killCalled = true
+		return nil
+	}
+	cfg.processAlive = func(_ int) bool { return false }
+
+	terminateStaleRunner(cfg)
+	assert.False(t, killCalled, "should not attempt to kill a dead process")
+}
+
+func TestTerminateStaleRunner_AliveProcess_GracefulExit(t *testing.T) {
+	t.Parallel()
+
+	dataDir := t.TempDir()
+
+	mgr := state.NewManager(dataDir)
+	ls, err := mgr.LoadAndLock(context.Background())
+	require.NoError(t, err)
+	ls.State.Active = true
+	ls.State.PID = 99999
+	require.NoError(t, ls.Save())
+	ls.Release()
+
+	cfg := defaultConfig()
+	cfg.dataDir = dataDir
+
+	var mu sync.Mutex
+	var signals []syscall.Signal
+	aliveCount := 0
+
+	cfg.killProcess = func(pid int, sig syscall.Signal) error {
+		assert.Equal(t, 99999, pid)
+		mu.Lock()
+		signals = append(signals, sig)
+		mu.Unlock()
+		return nil
+	}
+	cfg.processAlive = func(_ int) bool {
+		mu.Lock()
+		defer mu.Unlock()
+		aliveCount++
+		// Process is alive on first check (before SIGTERM), dead on second
+		// (after SIGTERM + first poll).
+		return aliveCount <= 1
+	}
+
+	terminateStaleRunner(cfg)
+
+	mu.Lock()
+	defer mu.Unlock()
+	require.Len(t, signals, 1, "should only send SIGTERM")
+	assert.Equal(t, syscall.SIGTERM, signals[0])
+}
+
+func TestTerminateStaleRunner_AliveProcess_RequiresKill(t *testing.T) {
+	t.Parallel()
+
+	dataDir := t.TempDir()
+
+	mgr := state.NewManager(dataDir)
+	ls, err := mgr.LoadAndLock(context.Background())
+	require.NoError(t, err)
+	ls.State.Active = true
+	ls.State.PID = 99999
+	require.NoError(t, ls.Save())
+	ls.Release()
+
+	cfg := defaultConfig()
+	cfg.dataDir = dataDir
+
+	var mu sync.Mutex
+	var signals []syscall.Signal
+
+	cfg.killProcess = func(pid int, sig syscall.Signal) error {
+		assert.Equal(t, 99999, pid)
+		mu.Lock()
+		signals = append(signals, sig)
+		mu.Unlock()
+		return nil
+	}
+	// Process never exits on its own.
+	cfg.processAlive = func(_ int) bool { return true }
+
+	terminateStaleRunner(cfg)
+
+	mu.Lock()
+	defer mu.Unlock()
+	require.Len(t, signals, 2, "should send SIGTERM then SIGKILL")
+	assert.Equal(t, syscall.SIGTERM, signals[0])
+	assert.Equal(t, syscall.SIGKILL, signals[1])
+}
+
+func TestTerminateStaleRunner_ZeroPID(t *testing.T) {
+	t.Parallel()
+
+	dataDir := t.TempDir()
+
+	// Write state with PID=0 (clean shutdown).
+	mgr := state.NewManager(dataDir)
+	ls, err := mgr.LoadAndLock(context.Background())
+	require.NoError(t, err)
+	ls.State.Active = false
+	ls.State.PID = 0
+	require.NoError(t, ls.Save())
+	ls.Release()
+
+	cfg := defaultConfig()
+	cfg.dataDir = dataDir
+
+	var killCalled bool
+	cfg.killProcess = func(_ int, _ syscall.Signal) error {
+		killCalled = true
+		return nil
+	}
+
+	terminateStaleRunner(cfg)
+	assert.False(t, killCalled, "should not attempt to kill PID 0")
+}
+
+func TestRun_WithCleanDataDir_TerminatesStaleRunner(t *testing.T) {
+	t.Parallel()
+
+	dataDir := t.TempDir()
+
+	// Pre-populate state as if a previous runner crashed.
+	mgr := state.NewManager(dataDir)
+	ls, err := mgr.LoadAndLock(context.Background())
+	require.NoError(t, err)
+	ls.State.Active = true
+	ls.State.PID = 2147483647 // dead PID
+	require.NoError(t, ls.Save())
+	ls.Release()
+
+	rootfsDir := filepath.Join(dataDir, "rootfs")
+	require.NoError(t, os.MkdirAll(rootfsDir, 0o755))
+
+	handle := &mockVMHandle{id: "1234", alive: true}
+	netProv := &mockNetProvider{sockPath: "/tmp/fake.sock"}
+
+	vm, err := Run(context.Background(), "test:latest",
+		WithDataDir(dataDir),
+		WithCleanDataDir(),
+		WithPreflightChecker(preflight.NewEmpty()),
+		WithRootFSPath(rootfsDir),
+		WithNetProvider(netProv),
+		WithBackend(&mockBackend{startHandle: handle}),
+	)
+	require.NoError(t, err)
+	require.NotNil(t, vm)
+
+	// The new state should reflect the new VM, not the stale one.
+	loaded, loadErr := mgr.Load()
+	require.NoError(t, loadErr)
+	assert.True(t, loaded.Active)
+	assert.Equal(t, 1234, loaded.PID)
+}

vm.go

@@ -125,6 +125,16 @@ func (vm *VM) Remove(ctx context.Context) error {
 	return nil
 }
 
+// RunnerPID returns the runner process ID, or 0 if the ID cannot be parsed
+// as a PID (e.g. non-process-based backends).
+func (vm *VM) RunnerPID() int {
+	pid, err := pidFromID(vm.handle.ID())
+	if err != nil {
+		return 0
+	}
+	return pid
+}
+
 // Name returns the VM name.
 func (vm *VM) Name() string { return vm.name }
 

vm_test.go

@@ -226,6 +226,24 @@ func TestVM_Remove_RemovesRootfsOutsideCache(t *testing.T) {
 	require.NoError(t, err)
 }
 
+func TestVM_RunnerPID_Valid(t *testing.T) {
+	t.Parallel()
+
+	handle := &mockVMHandle{id: "1234", alive: true}
+	vm := &VM{name: "test-vm", handle: handle}
+
+	assert.Equal(t, 1234, vm.RunnerPID())
+}
+
+func TestVM_RunnerPID_NonNumericID(t *testing.T) {
+	t.Parallel()
+
+	handle := &mockVMHandle{id: "abc-123", alive: true}
+	vm := &VM{name: "test-vm", handle: handle}
+
+	assert.Equal(t, 0, vm.RunnerPID())
+}
+
 func TestVM_Accessors(t *testing.T) {
 	t.Parallel()