mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 17:11:30 +00:00
fix: explicit stop timeouts, graceful-shutdown exit code, and QEMU orphan classification (#640)
* fix(systemd): give every unit an explicit TimeoutStopSec 9 of 12 spinifex systemd units had no TimeoutStopSec and inherited systemd's 90s default, which is either far more or far less time than their own SIGTERM handling needs. Size each one to what its shutdown path actually does: predastore matches viperblock's 30s WAL-drain budget since viperblock flushes through it (After= already sequences viperblock's stop first; this is the backstop if that's ever bypassed), spinifex-ui covers its 30s internal shutdown context plus margin for long-lived streaming proxy connections, nats covers its own JetStream-shutdown/raft-stepdown/client-drain signal handler, and services with no SIGTERM handler at all (awsgw) get a short bound since they exit immediately regardless. Extends the existing unit-file invariant test with a check that every .service file declares TimeoutStopSec, so a newly added unit that forgets one fails CI instead of silently falling back to 90s. * fix(vm): reconcile QEMU processes with no instance record on restore classifyRestoredInstances only walks Manager.Snapshot(), so a qemu-system process with no instance record at all -- a crash mid-launch, a lost KV write, a stale process surviving a wiped state file -- was never handed to killOrphanedQEMU and stayed permanently invisible to recovery. Add a host-process scan that diffs live qemu-system PIDs (matched via /proc/<pid>/comm) against the PIDs claimed by known instances' own -pidfile files. It only reports what it finds, never kills: this daemon cannot distinguish "record not loaded yet" or "owned by another daemon/tenant on this host" from a genuine orphan, and killing a running customer VM to fix an invisibility gap is the wrong trade. Runs at the end of Restore, once every known instance has been classified and relaunched, so a live process still unaccounted for at that point genuinely has no record anywhere. * fix(vm): distinguish recordless QEMU orphans by pidfile ownership A recordless qemu-system process with a stale spinifex pidfile is positively this daemon's own orphan; one with no pidfile at all could belong to another tenant on a shared host. Log the two cases distinctly instead of one blanket warning, so an operator can tell them apart. Also sandbox XDG_RUNTIME_DIR in the shared daemon test harness so Restore()-driven tests never read this box's real runtime directory. * fix(spinifexui): treat ErrServerClosed as a clean shutdown, not a failure Serve returns http.ErrServerClosed after Shutdown is called deliberately (the SIGTERM handler above does exactly that), but Start propagated it as an error, making systemd log a graceful stop as exit code 1 and Result: exit-code -- indistinguishable from a real crash.
This commit is contained in:
@@ -7,6 +7,9 @@ Before=ssh.service
|
||||
Type=oneshot
|
||||
ExecStart=/usr/bin/ssh-keygen -A
|
||||
RemainAfterExit=yes
|
||||
# No ExecStop: the keygen process has already exited by the time this unit
|
||||
# is stopped, so there is nothing to signal.
|
||||
TimeoutStopSec=5
|
||||
|
||||
[Install]
|
||||
WantedBy=multi-user.target
|
||||
|
||||
@@ -14,6 +14,9 @@ ExecStartPre=/var/lib/spinifex/wait-for-nats.sh
|
||||
ExecStart=/usr/local/bin/spx service awsgw start
|
||||
Restart=on-failure
|
||||
RestartSec=5
|
||||
# No SIGTERM handler in this process: it exits immediately on signal, so this
|
||||
# only bounds an unexpectedly wedged process before SIGKILL.
|
||||
TimeoutStopSec=10
|
||||
|
||||
OOMScoreAdjust=-500
|
||||
Environment=SPINIFEX_CONFIG_PATH=/etc/spinifex/spinifex.toml
|
||||
|
||||
@@ -9,6 +9,9 @@ Type=oneshot
|
||||
# remedy is `systemctl try-restart spinifex-nats` when JetStream is disabled while
|
||||
# the process stays alive (which Restart=on-failure cannot catch).
|
||||
ExecStart=/var/lib/spinifex/nats-js-watchdog.sh
|
||||
# Sized for the probe loop's own worst case (3 samples x 5s curl + 2 gaps x 2s
|
||||
# ~= 19s): a stop mid-cycle lets the script finish rather than cutting it off.
|
||||
TimeoutStopSec=20
|
||||
|
||||
# Hardening (kept compatible with talking to PID1 over /run/systemd).
|
||||
NoNewPrivileges=yes
|
||||
|
||||
@@ -12,6 +12,10 @@ Group=spinifex
|
||||
ExecStart=/usr/local/bin/spx service nats start
|
||||
Restart=on-failure
|
||||
RestartSec=5
|
||||
# nats-server installs its own SIGTERM handler (JetStream store shutdown, raft
|
||||
# stepdown, client drain) with no internal deadline; every dependent service
|
||||
# stops before nats does, so size this for nats's own graceful-stop needs.
|
||||
TimeoutStopSec=20
|
||||
OOMScoreAdjust=-500
|
||||
Environment=SPINIFEX_CONFIG_PATH=/etc/spinifex/nats/nats.conf
|
||||
|
||||
|
||||
@@ -13,6 +13,9 @@ Group=spinifex
|
||||
ExecStart=/usr/local/bin/spx service northstar start
|
||||
Restart=on-failure
|
||||
RestartSec=5
|
||||
# server.Shutdown() is called with an internal 5s context; margin covers the
|
||||
# NATS unsubscribe/close and process exit that follow it.
|
||||
TimeoutStopSec=10
|
||||
OOMScoreAdjust=-500
|
||||
Environment=SPINIFEX_CONFIG_PATH=/etc/spinifex/spinifex.toml
|
||||
Environment=SPINIFEX_BASE_DIR=/var/lib/spinifex/northstar/
|
||||
|
||||
@@ -17,6 +17,10 @@ ExecStartPre=/var/lib/spinifex/wait-for-nats.sh
|
||||
ExecStart=/var/lib/spinifex/predastore-start.sh
|
||||
Restart=on-failure
|
||||
RestartSec=5
|
||||
# Matches viperblock's drain budget: viperblock's WAL flush writes into
|
||||
# predastore, and After= already stops viperblock first, but this is the
|
||||
# backstop if that ordering is ever bypassed (direct stop, future edit).
|
||||
TimeoutStopSec=30
|
||||
OOMScoreAdjust=-500
|
||||
Environment=SPINIFEX_PREDASTORE_CONFIG_PATH=/etc/spinifex/predastore/predastore.toml
|
||||
Environment=SPINIFEX_PREDASTORE_BASE_PATH=/var/lib/spinifex/predastore/
|
||||
|
||||
@@ -16,6 +16,9 @@ ExecStartPre=/var/lib/spinifex/wait-for-nats.sh
|
||||
ExecStart=/usr/local/bin/spx service qmp-collector start
|
||||
Restart=on-failure
|
||||
RestartSec=5
|
||||
# Poll loop selects on ctx.Done() directly and closes NATS locally; shutdown
|
||||
# is near-instant, so this only bounds an unexpectedly wedged process.
|
||||
TimeoutStopSec=10
|
||||
Environment=SPINIFEX_CONFIG_PATH=/etc/spinifex/spinifex.toml
|
||||
Environment=SPINIFEX_BASE_DIR=/var/lib/spinifex/spinifex/
|
||||
# Same runtime dir as the daemon: telemetry sockets + metadata live there
|
||||
|
||||
@@ -12,6 +12,9 @@ Group=spinifex
|
||||
ExecStart=/usr/local/bin/spx service spinifex-ui start
|
||||
Restart=on-failure
|
||||
RestartSec=5
|
||||
# server.Shutdown() uses an internal 30s context (WriteTimeout=120s covers
|
||||
# long streaming proxy responses, e.g. console passthrough); add margin above it.
|
||||
TimeoutStopSec=40
|
||||
OOMScoreAdjust=-500
|
||||
Environment=SPINIFEX_UI_TLS_CERT=/etc/spinifex/server.pem
|
||||
Environment=SPINIFEX_UI_TLS_KEY=/etc/spinifex/server.key
|
||||
|
||||
@@ -14,6 +14,9 @@ ExecStartPre=/var/lib/spinifex/wait-for-nats.sh
|
||||
ExecStart=/usr/local/bin/spx service vpcd start
|
||||
Restart=on-failure
|
||||
RestartSec=5
|
||||
# Waits for an in-flight OVN drift-reconcile pass to return before exiting;
|
||||
# no sub-timeout bounds that call today, so this covers a normal pass.
|
||||
TimeoutStopSec=20
|
||||
OOMScoreAdjust=-500
|
||||
Environment=SPINIFEX_CONFIG_PATH=/etc/spinifex/spinifex.toml
|
||||
Environment=SPINIFEX_BASE_DIR=/var/lib/spinifex/vpcd/
|
||||
|
||||
@@ -26,6 +26,9 @@ func createDaemonWithJetStream(t *testing.T) *Daemon {
|
||||
_, nc, _ := testutil.StartTestJetStream(t)
|
||||
|
||||
tmpDir := t.TempDir()
|
||||
// Restore's orphan reconciliation reads pidfiles from the runtime dir;
|
||||
// sandbox it so tests never touch this box's real /run/spinifex.
|
||||
t.Setenv("XDG_RUNTIME_DIR", tmpDir)
|
||||
|
||||
clusterCfg := &config.ClusterConfig{
|
||||
Node: "node-1",
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"crypto/tls"
|
||||
"embed"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io/fs"
|
||||
"log/slog"
|
||||
@@ -279,7 +280,13 @@ func (svc *Service) launchService() error {
|
||||
}
|
||||
|
||||
slog.Info("Starting spinifex-ui service with HTTPS (auto-redirect HTTP)", "addr", addr)
|
||||
return server.Serve(splitLn)
|
||||
// ErrServerClosed is Serve's documented return value after Shutdown was
|
||||
// called deliberately (see the SIGTERM handler above) -- success, not a
|
||||
// failure to report or exit non-zero for.
|
||||
if err := server.Serve(splitLn); err != nil && !errors.Is(err, http.ErrServerClosed) {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// Content-Security-Policy header. All API requests are proxied through the
|
||||
|
||||
@@ -2,9 +2,15 @@ package spinifexui
|
||||
|
||||
import (
|
||||
"compress/gzip"
|
||||
"crypto/ecdsa"
|
||||
"crypto/elliptic"
|
||||
"crypto/rand"
|
||||
"crypto/tls"
|
||||
"crypto/x509"
|
||||
"crypto/x509/pkix"
|
||||
"encoding/pem"
|
||||
"io"
|
||||
"math/big"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
@@ -12,6 +18,7 @@ import (
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/go-chi/chi/v5/middleware"
|
||||
"github.com/stretchr/testify/assert"
|
||||
@@ -448,3 +455,88 @@ func TestShutdown_WithServer(t *testing.T) {
|
||||
err = svc.Shutdown()
|
||||
assert.NoError(t, err)
|
||||
}
|
||||
|
||||
// writeTestTLSFiles generates a self-signed cert/key pair on disk for a
|
||||
// launchService test, which needs real file paths (tls.LoadX509KeyPair),
|
||||
// unlike loadTLSCert's in-memory tls.Certificate used by the split-listener tests.
|
||||
func writeTestTLSFiles(t *testing.T) (certPath, keyPath string) {
|
||||
t.Helper()
|
||||
|
||||
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
|
||||
require.NoError(t, err)
|
||||
|
||||
template := &x509.Certificate{
|
||||
SerialNumber: big.NewInt(1),
|
||||
Subject: pkix.Name{CommonName: "spinifexui-test"},
|
||||
NotBefore: time.Now(),
|
||||
NotAfter: time.Now().Add(time.Hour),
|
||||
IPAddresses: []net.IP{net.ParseIP("127.0.0.1")},
|
||||
}
|
||||
certDER, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key)
|
||||
require.NoError(t, err)
|
||||
keyDER, err := x509.MarshalECPrivateKey(key)
|
||||
require.NoError(t, err)
|
||||
|
||||
certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER})
|
||||
|
||||
dir := t.TempDir()
|
||||
certPath = filepath.Join(dir, "server.pem")
|
||||
keyPath = filepath.Join(dir, "server.key")
|
||||
require.NoError(t, os.WriteFile(certPath, certPEM, 0o600))
|
||||
require.NoError(t, os.WriteFile(keyPath,
|
||||
pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyDER}), 0o600))
|
||||
// newProxyTransport reads <TLSCert dir>/ca.pem; the self-signed leaf
|
||||
// itself is a valid enough PEM cert to satisfy that read in a test.
|
||||
require.NoError(t, os.WriteFile(filepath.Join(dir, "ca.pem"), certPEM, 0o600))
|
||||
return certPath, keyPath
|
||||
}
|
||||
|
||||
// freePort binds an ephemeral port, closes it immediately, and returns the
|
||||
// number so Start can bind the same port without hardcoding 3000 (which may
|
||||
// already be in use by a real spinifex-ui on a shared box).
|
||||
func freePort(t *testing.T) int {
|
||||
t.Helper()
|
||||
ln, err := net.Listen("tcp", "127.0.0.1:0")
|
||||
require.NoError(t, err)
|
||||
port := ln.Addr().(*net.TCPAddr).Port
|
||||
require.NoError(t, ln.Close())
|
||||
return port
|
||||
}
|
||||
|
||||
// TestStart_GracefulShutdownIsNotAnError guards against Start propagating
|
||||
// http.ErrServerClosed -- Serve's documented return value after a deliberate
|
||||
// Shutdown -- as a failure. Before the fix this made a clean stop look like
|
||||
// a crash (exit code 1) to systemd.
|
||||
func TestStart_GracefulShutdownIsNotAnError(t *testing.T) {
|
||||
t.Setenv("XDG_RUNTIME_DIR", t.TempDir())
|
||||
certPath, keyPath := writeTestTLSFiles(t)
|
||||
|
||||
svc, err := New(&Config{
|
||||
Port: freePort(t),
|
||||
Host: "127.0.0.1",
|
||||
TLSCert: certPath,
|
||||
TLSKey: keyPath,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
startErrCh := make(chan error, 1)
|
||||
go func() {
|
||||
_, startErr := svc.Start()
|
||||
startErrCh <- startErr
|
||||
}()
|
||||
|
||||
require.Eventually(t, func() bool {
|
||||
svc.mu.Lock()
|
||||
defer svc.mu.Unlock()
|
||||
return svc.server != nil
|
||||
}, 2*time.Second, 10*time.Millisecond, "server never registered before shutdown")
|
||||
|
||||
require.NoError(t, svc.Shutdown())
|
||||
|
||||
select {
|
||||
case startErr := <-startErrCh:
|
||||
assert.NoError(t, startErr, "Start must treat a deliberate Shutdown's ErrServerClosed as success")
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("Start did not return after Shutdown")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -262,6 +262,30 @@ func TestRG11_LeanUnits(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestAllServiceUnitsDeclareTimeoutStopSec asserts every .service unit pins an
|
||||
// explicit TimeoutStopSec, so a newly added unit that forgets one falls back to
|
||||
// systemd's blanket 90s default instead of a value chosen for that service's
|
||||
// own SIGTERM handling.
|
||||
func TestAllServiceUnitsDeclareTimeoutStopSec(t *testing.T) {
|
||||
dir := unitsDir(t)
|
||||
for _, name := range unitFiles(t, dir) {
|
||||
if !strings.HasSuffix(name, ".service") {
|
||||
continue
|
||||
}
|
||||
u := readUnit(t, dir, name)
|
||||
found := false
|
||||
for l := range strings.SplitSeq(u, "\n") {
|
||||
if strings.HasPrefix(strings.TrimSpace(l), "TimeoutStopSec=") {
|
||||
found = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
t.Errorf("%s must declare an explicit TimeoutStopSec=", name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestApplicationUnitsExportTelemetry asserts every unit that runs an spx
|
||||
// application service sources the telemetry drop-in. That file carries
|
||||
// OTEL_EXPORTER_OTLP_ENDPOINT, MULGA_ENV and MULGA_SOURCE, so a unit missing it
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
package vm
|
||||
|
||||
import (
|
||||
"log/slog"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"github.com/mulgadc/spinifex/spinifex/utils"
|
||||
)
|
||||
|
||||
// qemuProcessPrefix matches both qemu-system-x86_64 and qemu-system-aarch64;
|
||||
// /proc/<pid>/comm truncates at 15 bytes but this prefix is only 11.
|
||||
const qemuProcessPrefix = "qemu-system"
|
||||
|
||||
// qemuProcRoot is the /proc mount scanned for live qemu-system processes.
|
||||
// Overridden in tests to avoid depending on the real host process table.
|
||||
var qemuProcRoot = "/proc"
|
||||
|
||||
// scanLiveQEMUPIDs walks procRoot for processes whose comm starts with
|
||||
// qemuProcessPrefix. procRoot is a parameter so tests point it at a
|
||||
// fabricated directory instead of spawning a real qemu-system binary.
|
||||
func scanLiveQEMUPIDs(procRoot string) ([]int, error) {
|
||||
entries, err := os.ReadDir(procRoot)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
var pids []int
|
||||
for _, entry := range entries {
|
||||
pid, err := strconv.Atoi(entry.Name())
|
||||
if err != nil || pid <= 0 {
|
||||
continue
|
||||
}
|
||||
comm, err := os.ReadFile(filepath.Join(procRoot, entry.Name(), "comm"))
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
if strings.HasPrefix(strings.TrimSpace(string(comm)), qemuProcessPrefix) {
|
||||
pids = append(pids, pid)
|
||||
}
|
||||
}
|
||||
return pids, nil
|
||||
}
|
||||
|
||||
// pidFileOwners reads every "<instance-id>.pid" file in runtimeDir and
|
||||
// returns the PID -> instance ID it names, regardless of whether that
|
||||
// instance still exists in Snapshot(). QEMU itself writes these via
|
||||
// -pidfile, so an entry here is a claim this daemon made at some point --
|
||||
// not necessarily one it still stands behind.
|
||||
func pidFileOwners(runtimeDir string) map[int]string {
|
||||
owners := make(map[int]string)
|
||||
entries, err := os.ReadDir(runtimeDir)
|
||||
if err != nil {
|
||||
return owners
|
||||
}
|
||||
for _, entry := range entries {
|
||||
name := entry.Name()
|
||||
if !strings.HasSuffix(name, ".pid") {
|
||||
continue
|
||||
}
|
||||
id := strings.TrimSuffix(name, ".pid")
|
||||
if pid, err := utils.ReadPidFileFrom(runtimeDir, id); err == nil {
|
||||
owners[pid] = id
|
||||
}
|
||||
}
|
||||
return owners
|
||||
}
|
||||
|
||||
// qemuOrphan is a live qemu-system PID with no matching instance in
|
||||
// Snapshot(). hasPidFile distinguishes two very different situations: a
|
||||
// pidfile this daemon itself wrote (instanceID is that pidfile's claim,
|
||||
// positively this daemon's process) versus no pidfile at all (could be
|
||||
// another tenant's process on a shared host).
|
||||
type qemuOrphan struct {
|
||||
pid int
|
||||
instanceID string
|
||||
hasPidFile bool
|
||||
}
|
||||
|
||||
// classifyQEMUOrphans returns every live qemu-system PID not vouched for by
|
||||
// a pidfile naming a known instance. This is the gap classifyRestoredInstances
|
||||
// cannot see, since it only walks Snapshot().
|
||||
func classifyQEMUOrphans(procRoot, runtimeDir string, known map[string]bool) ([]qemuOrphan, error) {
|
||||
live, err := scanLiveQEMUPIDs(procRoot)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if len(live) == 0 {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
owners := pidFileOwners(runtimeDir)
|
||||
var orphans []qemuOrphan
|
||||
for _, pid := range live {
|
||||
id, hasPidFile := owners[pid]
|
||||
if hasPidFile && known[id] {
|
||||
continue // legitimate, tracked instance
|
||||
}
|
||||
orphans = append(orphans, qemuOrphan{pid: pid, instanceID: id, hasPidFile: hasPidFile})
|
||||
}
|
||||
return orphans, nil
|
||||
}
|
||||
|
||||
// reportRecordlessQEMUOrphans logs, but never kills, a qemu-system process
|
||||
// with no instance record. A pidfile in runtimeDir naming an unknown
|
||||
// instance positively identifies the process as this daemon's own stale
|
||||
// orphan (logged distinctly, safe for an operator to reap by hand); no
|
||||
// pidfile at all could belong to another tenant/daemon on a shared host, so
|
||||
// it is logged as unowned. Neither case signals the process automatically:
|
||||
// a false positive here would destroy a running customer VM. Call only
|
||||
// after Restore has finished loading and relaunching every known instance.
|
||||
func (m *Manager) reportRecordlessQEMUOrphans() {
|
||||
known := make(map[string]bool)
|
||||
for _, instance := range m.Snapshot() {
|
||||
known[instance.ID] = true
|
||||
}
|
||||
|
||||
orphans, err := classifyQEMUOrphans(qemuProcRoot, utils.RuntimeDir(), known)
|
||||
if err != nil {
|
||||
slog.Warn("recordless QEMU orphan scan failed", "error", err)
|
||||
return
|
||||
}
|
||||
for _, o := range orphans {
|
||||
if o.hasPidFile {
|
||||
slog.Warn("qemu-system process has a stale spinifex pidfile with no matching instance record; not killing, safe to reap manually",
|
||||
"pid", o.pid, "instanceId", o.instanceID)
|
||||
continue
|
||||
}
|
||||
slog.Warn("qemu-system process has no spinifex pidfile at all; not killing, ownership unconfirmed",
|
||||
"pid", o.pid)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,157 @@
|
||||
package vm
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// fakeProcEntry creates procRoot/<pid>/comm containing name, mimicking the
|
||||
// /proc layout scanLiveQEMUPIDs reads, without spawning a real process.
|
||||
func fakeProcEntry(t *testing.T, procRoot string, pid int, name string) {
|
||||
t.Helper()
|
||||
dir := filepath.Join(procRoot, strconv.Itoa(pid))
|
||||
require.NoError(t, os.MkdirAll(dir, 0o755))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(dir, "comm"), []byte(name+"\n"), 0o644))
|
||||
}
|
||||
|
||||
func TestScanLiveQEMUPIDs(t *testing.T) {
|
||||
procRoot := t.TempDir()
|
||||
fakeProcEntry(t, procRoot, 111, "qemu-system-x86_64")
|
||||
fakeProcEntry(t, procRoot, 222, "qemu-system-aarch64")
|
||||
fakeProcEntry(t, procRoot, 333, "bash")
|
||||
// Non-numeric /proc entries (self, curproc, etc.) must not crash the scan.
|
||||
require.NoError(t, os.MkdirAll(filepath.Join(procRoot, "self"), 0o755))
|
||||
|
||||
pids, err := scanLiveQEMUPIDs(procRoot)
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.ElementsMatch(t, []int{111, 222}, pids, "only qemu-system* processes are returned")
|
||||
}
|
||||
|
||||
func TestScanLiveQEMUPIDs_MissingProcRoot(t *testing.T) {
|
||||
_, err := scanLiveQEMUPIDs(filepath.Join(t.TempDir(), "does-not-exist"))
|
||||
assert.Error(t, err, "a missing proc root must surface an error, not a silent empty result")
|
||||
}
|
||||
|
||||
func TestPidFileOwners(t *testing.T) {
|
||||
runtimeDir := t.TempDir()
|
||||
require.NoError(t, os.WriteFile(filepath.Join(runtimeDir, "i-known.pid"), []byte("111"), 0o600))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(runtimeDir, "i-gone.pid"), []byte("999"), 0o600))
|
||||
|
||||
owners := pidFileOwners(runtimeDir)
|
||||
|
||||
assert.Equal(t, "i-known", owners[111], "a pidfile's PID maps to the instance ID it names")
|
||||
assert.Equal(t, "i-gone", owners[999],
|
||||
"pidFileOwners is not filtered by known -- it returns every claim on disk, known or not")
|
||||
}
|
||||
|
||||
func TestClassifyQEMUOrphans(t *testing.T) {
|
||||
t.Run("known instance's QEMU is left alone", func(t *testing.T) {
|
||||
procRoot, runtimeDir := t.TempDir(), t.TempDir()
|
||||
fakeProcEntry(t, procRoot, 111, "qemu-system-x86_64")
|
||||
require.NoError(t, os.WriteFile(filepath.Join(runtimeDir, "i-known.pid"), []byte("111"), 0o600))
|
||||
|
||||
orphans, err := classifyQEMUOrphans(procRoot, runtimeDir, map[string]bool{"i-known": true})
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.Empty(t, orphans, "a QEMU process claimed by a known instance's pidfile is not an orphan")
|
||||
})
|
||||
|
||||
t.Run("recordless QEMU process with no pidfile at all", func(t *testing.T) {
|
||||
procRoot, runtimeDir := t.TempDir(), t.TempDir()
|
||||
fakeProcEntry(t, procRoot, 222, "qemu-system-x86_64")
|
||||
// No pidfile at all ties PID 222 to any instance.
|
||||
|
||||
orphans, err := classifyQEMUOrphans(procRoot, runtimeDir, map[string]bool{})
|
||||
|
||||
require.NoError(t, err)
|
||||
require.Len(t, orphans, 1)
|
||||
assert.Equal(t, 222, orphans[0].pid)
|
||||
assert.False(t, orphans[0].hasPidFile, "no pidfile claims this PID -- ownership is unconfirmed")
|
||||
})
|
||||
|
||||
t.Run("stale pidfile naming an unknown instance is classified as this daemon's own", func(t *testing.T) {
|
||||
procRoot, runtimeDir := t.TempDir(), t.TempDir()
|
||||
fakeProcEntry(t, procRoot, 333, "qemu-system-aarch64")
|
||||
// Pidfile exists but names an instance this manager has no record of.
|
||||
require.NoError(t, os.WriteFile(filepath.Join(runtimeDir, "i-gone.pid"), []byte("333"), 0o600))
|
||||
|
||||
orphans, err := classifyQEMUOrphans(procRoot, runtimeDir, map[string]bool{})
|
||||
|
||||
require.NoError(t, err)
|
||||
require.Len(t, orphans, 1)
|
||||
assert.Equal(t, 333, orphans[0].pid)
|
||||
assert.True(t, orphans[0].hasPidFile, "a pidfile this daemon wrote positively identifies the process")
|
||||
assert.Equal(t, "i-gone", orphans[0].instanceID)
|
||||
})
|
||||
|
||||
t.Run("mixed: known left alone, recordless-with-pidfile and recordless-without kept separate", func(t *testing.T) {
|
||||
procRoot, runtimeDir := t.TempDir(), t.TempDir()
|
||||
fakeProcEntry(t, procRoot, 111, "qemu-system-x86_64") // known
|
||||
fakeProcEntry(t, procRoot, 222, "qemu-system-x86_64") // stale pidfile
|
||||
fakeProcEntry(t, procRoot, 333, "qemu-system-x86_64") // no pidfile
|
||||
require.NoError(t, os.WriteFile(filepath.Join(runtimeDir, "i-known.pid"), []byte("111"), 0o600))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(runtimeDir, "i-gone.pid"), []byte("222"), 0o600))
|
||||
|
||||
orphans, err := classifyQEMUOrphans(procRoot, runtimeDir, map[string]bool{"i-known": true})
|
||||
|
||||
require.NoError(t, err)
|
||||
require.Len(t, orphans, 2)
|
||||
byPID := map[int]qemuOrphan{}
|
||||
for _, o := range orphans {
|
||||
byPID[o.pid] = o
|
||||
}
|
||||
assert.True(t, byPID[222].hasPidFile)
|
||||
assert.Equal(t, "i-gone", byPID[222].instanceID)
|
||||
assert.False(t, byPID[333].hasPidFile)
|
||||
})
|
||||
|
||||
t.Run("no live qemu-system processes returns no orphans", func(t *testing.T) {
|
||||
procRoot, runtimeDir := t.TempDir(), t.TempDir()
|
||||
fakeProcEntry(t, procRoot, 444, "bash")
|
||||
|
||||
orphans, err := classifyQEMUOrphans(procRoot, runtimeDir, map[string]bool{})
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.Empty(t, orphans)
|
||||
})
|
||||
}
|
||||
|
||||
// TestReportRecordlessQEMUOrphans covers the Manager-level entry point used
|
||||
// by Restore: a known instance's QEMU must not be reported, and the two
|
||||
// orphan classes (own stale pidfile vs no pidfile at all) must be logged
|
||||
// distinctly so an operator can tell them apart. Neither is signalled --
|
||||
// there is no kill path in this function to assert against.
|
||||
func TestReportRecordlessQEMUOrphans(t *testing.T) {
|
||||
procRoot := t.TempDir()
|
||||
runtimeDir := t.TempDir()
|
||||
t.Setenv("XDG_RUNTIME_DIR", runtimeDir)
|
||||
|
||||
origProcRoot := qemuProcRoot
|
||||
qemuProcRoot = procRoot
|
||||
t.Cleanup(func() { qemuProcRoot = origProcRoot })
|
||||
|
||||
fakeProcEntry(t, procRoot, 111, "qemu-system-x86_64") // claimed by i-known
|
||||
fakeProcEntry(t, procRoot, 222, "qemu-system-x86_64") // stale pidfile, unknown instance
|
||||
fakeProcEntry(t, procRoot, 333, "qemu-system-x86_64") // no pidfile at all
|
||||
require.NoError(t, os.WriteFile(filepath.Join(runtimeDir, "i-known.pid"), []byte("111"), 0o600))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(runtimeDir, "i-gone.pid"), []byte("222"), 0o600))
|
||||
|
||||
m := NewManager()
|
||||
m.Replace(map[string]*VM{"i-known": {ID: "i-known", Status: StateRunning}})
|
||||
|
||||
buf := captureSlogRestore(t)
|
||||
m.reportRecordlessQEMUOrphans()
|
||||
output := buf.String()
|
||||
|
||||
assert.NotContains(t, output, "pid=111", "the known instance's QEMU PID must not be reported as an orphan")
|
||||
assert.Contains(t, output, "pid=222", "the stale-pidfile orphan must be reported")
|
||||
assert.Contains(t, output, "i-gone", "the stale pidfile's instance ID must be logged for the operator")
|
||||
assert.Contains(t, output, "pid=333", "the no-pidfile orphan must be reported")
|
||||
assert.Contains(t, output, "not killing", "neither orphan class is signalled")
|
||||
}
|
||||
@@ -49,6 +49,10 @@ func (m *Manager) Restore() {
|
||||
if err := m.writeRunningState(); err != nil {
|
||||
slog.Error("Failed to persist state after restore", "error", err)
|
||||
}
|
||||
|
||||
// Every known instance is classified and relaunched by this point, so a
|
||||
// live qemu-system process still unaccounted for genuinely has no record.
|
||||
m.reportRecordlessQEMUOrphans()
|
||||
}
|
||||
|
||||
// loadRunningState replaces the manager's running set from the persisted snapshot.
|
||||
|
||||
Reference in New Issue
Block a user