fix(daemon): SIGTERM preserves VMs + KillMode=process on the unit

DDIL Scenario B asserts that `systemctl restart spinifex-daemon`
preserves every guest VM so the new daemon can reattach via the
persisted local state file. The previous setupShutdown SIGTERM handler
called vmMgr.StopAll(), gracefully QMP-powering-down every guest before
the daemon exited — ~90s wall-clock and zero surviving QEMU on the
other side. CI run 25666369604 surfaced this as
`recovered instance i-... has no live QEMU PID`.

Two changes restore the Tier 1 contract:

- setupShutdown no longer calls StopAll. It still flips d.shuttingDown
  so crash/restart handlers bail out, persists state, unsubscribes from
  NATS, and exits. VMs are only stopped when coordinated cluster
  shutdown drains them — handleShutdownDrain still calls StopAll and
  flips the flag before the SIGTERM path runs. Operator-visible
  behaviour change: `systemctl stop spinifex-daemon` leaves guest VMs
  running. To stop VMs, use the cluster-shutdown topics or terminate
  via the AWS API.

- The systemd unit gains `KillMode=process` so systemd only signals the
  daemon's main PID. With the default control-group mode, QEMU
  children were killed when systemd tore down the daemon's cgroup —
  defeating the SIGTERM change above. `TimeoutStopSec=15` bounds the
  new fast-exit grace window.

Also nil-guards natsConn.Close in the shutdown path: when a daemon
restarts while NATS is unreachable (Scenario B), natsConn is nil and
the close would panic.

Mulga-siv-58 (reopened).
This commit is contained in:
joshsiv-mulga
2026-05-11 21:38:17 +10:00
parent c04ab230aa
commit 49393d4c9a
2 changed files with 25 additions and 8 deletions
+11
View File
@@ -23,6 +23,17 @@ Group=spinifex
ExecStart=/usr/local/bin/spx service spinifex start
Restart=on-failure
RestartSec=5
# DDIL Tier 1 (mulga-siv-58): KillMode=process so systemd only signals the
# daemon's main PID, not the cgroup. QEMU children stay alive across
# `systemctl restart spinifex-daemon` and the new daemon reattaches to them
# via the persisted local state file + pidfile + QMP socket. With the
# default control-group mode, restart would terminate every guest VM —
# defeating Scenario B's "daemon restart preserves running instances".
KillMode=process
# 15s upper bound on the SIGTERM grace window. The new SIGTERM handler
# skips VM teardown (it now just persists state and exits), so 15s is
# plenty of slack for the cluster-manager + NATS unsubscribe path.
TimeoutStopSec=15
OOMScoreAdjust=-500
Environment=SPINIFEX_CONFIG_PATH=/etc/spinifex/spinifex.toml
Environment=SPINIFEX_BASE_DIR=/var/lib/spinifex/spinifex/
+14 -8
View File
@@ -1744,16 +1744,18 @@ func (d *Daemon) setupShutdown() {
// Cancel context to stop heartbeat and other goroutines
d.cancel()
// If coordinated shutdown already handled VMs (DRAIN phase), skip the
// per-instance teardown. Otherwise, set the flag now so crash handlers
// and restart schedulers know to bail out during SIGTERM-based shutdown.
// DDIL Tier 1 (mulga-siv-58): SIGTERM alone never stops local VMs.
// `systemctl restart spinifex-daemon` must preserve every QEMU child so
// the new daemon can reattach via the persisted local state file
// (Scenario B). VMs are only stopped when coordinated cluster shutdown
// has explicitly drained them — handleShutdownDrain sets shuttingDown
// after calling StopAll, and crash/restart handlers also gate on this
// flag to bail out.
if d.shuttingDown.Load() {
slog.Info("Coordinated shutdown in progress, skipping VM stop (already handled by DRAIN phase)")
} else {
slog.Info("SIGTERM with no coordinated drain — leaving local VMs running for restart recovery")
d.shuttingDown.Store(true)
if err := d.vmMgr.StopAll(); err != nil {
slog.Error("Failed to stop instances during shutdown", "err", err)
}
}
// Stop ELBv2 background goroutines
@@ -1783,8 +1785,12 @@ func (d *Daemon) setupShutdown() {
slog.Error("Failed to write state", "err", err)
}
// Close NATS connection
d.natsConn.Close()
// Close NATS connection. natsConn is nil when the daemon was started
// with NATS unreachable and never managed an initial connect — that
// is the DDIL Scenario B path (daemon restart while NATS is down).
if d.natsConn != nil {
d.natsConn.Close()
}
// Shutdown cluster manager
if d.clusterServer != nil {