fix: reap orphaned nbdkit processes recovery cannot adopt (#783)

* fix: reap orphaned nbdkit processes recovery cannot adopt

recoverMountedVolumes already rebuilds MountedVolumes from surviving
nbdkit processes, but an unadoptable backend (buildVB fails, most
commonly a deleted volume) or a duplicate corroborated process for an
already-adopted volume was logged and left running forever, with no
later pass reconsidering it.

Reap both classes, but only when a scan of every other process's
/proc/<pid>/cmdline on the host finds no reference to the backend's
NBD endpoint -- its socket path, or its 127.0.0.1:<port> pair, checked
both joined and split across server.host=/server.port=. A referencing
process, or a scan that could not complete, both leave the process
running and log loudly instead, since signalling the wrong process
destroys a running guest's data path.

* fix: treat an unreadable /proc candidate as inconclusive, not absent

endpointReferencer skipped any os.ReadFile failure on a candidate's
cmdline, which silently counted a read it could not perform as
evidence the candidate does not reference the endpoint. Under a /proc
mounted with hidepid, a guest QEMU owned by another uid is invisible
this way, so an in-use backend could be reaped.

Only os.IsNotExist (the candidate exited between ReadDir and the read)
continues the scan. Any other error now returns non-nil from
endpointReferencer, which reapOrphanedNbdkit already treats as an
inconclusive scan: leave the process running, log at WARN.

* fix: stop orphan nbdkit reaping from killing live backends under hidepid=invisible

hidepid=invisible hides another uid's /proc/<pid> directory entry outright,
not just its contents, so a live QEMU referencer (owned by spinifex-daemon)
never appeared in viperblockd's scan at all -- a clean "no referencer" result
that was actually just a hidden one. reapOrphanedNbdkit now detects the
hazardous mount via /proc/self/mountinfo and declines to reap rather than
trust a scan that could be blind to a live guest.

spinifex-viperblock.service drops ProtectProc=invisible (now default, same
tradeoff already accepted by spinifex-qmp-collector.service) so the scan can
actually see spinifex-daemon's processes again once it's safe to trust.
This commit is contained in:
Josh Siviour
2026-08-13 12:28:50 +10:00
committed by GitHub
parent 29d4a6221e
commit eb53dc3396
5 changed files with 530 additions and 9 deletions
+3 -2
View File
@@ -1,4 +1,4 @@
# spinifex-unit-version: 1
# spinifex-unit-version: 2
[Unit]
Description=Spinifex Viperblock (Block Storage)
PartOf=spinifex.target
@@ -51,7 +51,8 @@ ProtectKernelTunables=yes
ProtectKernelModules=yes
ProtectKernelLogs=yes
ProtectControlGroups=yes
ProtectProc=invisible
# ProtectProc must stay default: hidepid=invisible hides spinifex-daemon's
# QEMU /proc entries (different uid) orphan nbdkit reaping must see.
SystemCallArchitectures=native
MemoryDenyWriteExecute=yes
RestrictAddressFamilies=AF_INET AF_INET6 AF_UNIX AF_NETLINK
@@ -2,7 +2,8 @@ package viperblockd
// Rebuilds cfg.MountedVolumes from nbdkit processes that survived a
// viperblockd restart, so findMountedVolume can find them again (see
// orphan_scan.go for the equivalent problem/fix on the QEMU side).
// orphan_scan.go for the equivalent problem/fix on the QEMU side), and reaps
// the two classes of surviving nbdkit that recovery itself cannot adopt.
import (
"bytes"
@@ -11,8 +12,10 @@ import (
"log/slog"
"os"
"path/filepath"
"slices"
"strconv"
"strings"
"syscall"
"github.com/mulgadc/spinifex/spinifex/utils"
"github.com/nats-io/nats.go"
@@ -170,10 +173,11 @@ func recoverMountedVolumes(ctx context.Context, cfg *Config, nc *nats.Conn, proc
}
// Two live nbdkits for one volume is the double-mount hazard itself:
// adopt the first and leave the second visible in the log.
// adopt the first and reap the rest if nothing is using them.
if _, ok := findMountedVolume(cfg, disc.Volume); ok {
slog.Warn("recovery: volume already recovered from another nbdkit process, skipping",
"pid", disc.PID, "volume", disc.Volume)
reapOrphanedNbdkit(procRoot, disc, reapReasonDuplicate)
continue
}
@@ -181,6 +185,7 @@ func recoverMountedVolumes(ctx context.Context, cfg *Config, nc *nats.Conn, proc
if err != nil {
slog.Error("recovery: failed to rebuild mounted volume registry entry, skipping",
"pid", disc.PID, "volume", disc.Volume, "err", err)
reapOrphanedNbdkit(procRoot, disc, reapReasonUnadoptable)
continue
}
@@ -192,3 +197,164 @@ func recoverMountedVolumes(ctx context.Context, cfg *Config, nc *nats.Conn, proc
"pid", mv.PID, "volume", mv.Name, "socket", mv.Socket, "port", mv.Port)
}
}
// reapReason names which of recovery's two failure-to-adopt paths produced
// an unclaimed nbdkit process, carried only for logging.
type reapReason string
const (
reapReasonUnadoptable reapReason = "unadoptable"
reapReasonDuplicate reapReason = "duplicate"
)
// reapOrphanedNbdkit decides whether disc -- a corroborated nbdkit process
// recovery could not fold into MountedVolumes -- may be safely signalled.
// Reaping the wrong process destroys a running guest's data path, so disc is
// only ever signalled once a scan of every other process on the host finds no
// reference to its NBD endpoint. A referencing process, or a scan that could
// not complete, both leave it running: the caller gets a loud log either way.
func reapOrphanedNbdkit(procRoot string, disc discoveredNbdkit, reason reapReason) {
if hidden, herr := procHidepidInvisible(procRoot); hidden {
slog.Warn("recovery: proc mount hides other users' process entries; a live referencer could be invisible to this scan, leaving unclaimed nbdkit running",
"pid", disc.PID, "volume", disc.Volume, "reason", reason, "err", herr)
return
}
referencingPID, referenced, err := endpointReferencer(procRoot, disc)
if err != nil {
slog.Warn("recovery: could not scan for processes using an unclaimed nbdkit's endpoint, leaving it running",
"pid", disc.PID, "volume", disc.Volume, "reason", reason, "err", err)
return
}
if referenced {
slog.Error("recovery: unclaimed nbdkit process is still referenced by a live process; left running deliberately, needs operator attention",
"pid", disc.PID, "volume", disc.Volume, "reason", reason, "referencing_pid", referencingPID)
return
}
proc, err := os.FindProcess(disc.PID)
if err != nil {
slog.Error("recovery: failed to locate unclaimed nbdkit process to reap",
"pid", disc.PID, "volume", disc.Volume, "reason", reason, "err", err)
return
}
if err := proc.Signal(syscall.SIGTERM); err != nil {
slog.Error("recovery: failed to signal unclaimed nbdkit process",
"pid", disc.PID, "volume", disc.Volume, "reason", reason, "err", err)
return
}
if disc.Socket != "" {
if err := os.Remove(disc.Socket); err != nil && !os.IsNotExist(err) {
slog.Warn("recovery: failed to remove socket file for reaped nbdkit process",
"pid", disc.PID, "volume", disc.Volume, "socket", disc.Socket, "err", err)
}
}
slog.Info("recovery: reaped unclaimed nbdkit process with no live referencer",
"pid", disc.PID, "volume", disc.Volume, "reason", reason)
}
// procHidepidInvisible reports whether procRoot's mount hides another uid's
// /proc/<pid> entry outright (hidepid=invisible/2), so a referencer never
// reaches os.ReadDir. Missing mountinfo reports false; any other read error cannot be ruled out.
func procHidepidInvisible(procRoot string) (bool, error) {
data, err := os.ReadFile(filepath.Join(procRoot, "self", "mountinfo"))
if err != nil {
if os.IsNotExist(err) {
return false, nil
}
return true, err
}
for line := range strings.SplitSeq(string(data), "\n") {
fields := strings.Fields(line)
sep := slices.Index(fields, "-")
if sep < 0 || sep+3 >= len(fields) || len(fields) < 6 {
continue
}
if fields[4] != "/proc" || fields[sep+1] != "proc" {
continue
}
opts := fields[5] + "," + fields[sep+3]
if strings.Contains(opts, "hidepid=invisible") || strings.Contains(opts, "hidepid=2") {
return true, nil
}
}
return false, nil
}
// endpointReferencer reports whether any process other than disc's own PID
// has disc's NBD endpoint on its own /proc/<pid>/cmdline: the socket path for
// a unix mount, or the 127.0.0.1:<port> pair for a TCP one. That is how a
// guest's QEMU process is handed its drive backend at launch, and it is the
// only positive evidence available that an unclaimed nbdkit still serves a
// live guest. A non-nil error means the scan itself could not be completed,
// not that nothing was found -- callers must treat that as unknown, not safe.
func endpointReferencer(procRoot string, disc discoveredNbdkit) (pid int, found bool, err error) {
entries, err := os.ReadDir(procRoot)
if err != nil {
return 0, false, err
}
for _, entry := range entries {
candidate, cerr := strconv.Atoi(entry.Name())
if cerr != nil || candidate <= 0 || candidate == disc.PID {
continue
}
cmdline, rerr := os.ReadFile(filepath.Join(procRoot, entry.Name(), "cmdline"))
if rerr != nil {
// The process exited between ReadDir and this read: genuinely
// not a referencer. Any other error (EACCES under a hidepid
// /proc, most importantly) means the scan cannot rule this PID
// out, so it must not be silently read as absence of evidence.
if os.IsNotExist(rerr) {
continue
}
return 0, false, rerr
}
if cmdlineReferencesEndpoint(cmdline, disc) {
return candidate, true, nil
}
}
return 0, false, nil
}
// cmdlineReferencesEndpoint checks a NUL-separated /proc/<pid>/cmdline for
// disc's endpoint. A socket mount's path appears verbatim in whichever
// argument named it (QEMU's nbd+unix:///?socket=<path> file= or its
// server.path=<path> -blockdev option). A TCP mount's 127.0.0.1:<port> pair
// may appear joined in one argument (a boot/EFI drive's nbd://host:port) or
// split across server.host=/server.port= within the same comma-joined
// -blockdev argument, so both shapes are checked.
func cmdlineReferencesEndpoint(cmdline []byte, disc discoveredNbdkit) bool {
args := bytes.Split(bytes.TrimRight(cmdline, "\x00"), []byte{0})
if disc.Socket != "" {
needle := []byte(disc.Socket)
for _, arg := range args {
if bytes.Contains(arg, needle) {
return true
}
}
return false
}
if disc.Port <= 0 {
return false
}
joined := fmt.Appendf(nil, "127.0.0.1:%d", disc.Port)
serverPort := fmt.Appendf(nil, "server.port=%d", disc.Port)
serverHost := []byte("server.host=127.0.0.1")
for _, arg := range args {
if bytes.Contains(arg, joined) {
return true
}
if bytes.Contains(arg, serverPort) && bytes.Contains(arg, serverHost) {
return true
}
}
return false
}
@@ -10,6 +10,7 @@ import (
"fmt"
"net"
"os"
"os/exec"
"path/filepath"
"strconv"
"strings"
@@ -17,6 +18,7 @@ import (
"time"
"github.com/mulgadc/spinifex/spinifex/ebsprovider"
"github.com/mulgadc/spinifex/spinifex/utils"
"github.com/mulgadc/viperblock/viperblock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -66,6 +68,34 @@ func writeProcFixture(t *testing.T, procRoot string, pid int, comm string, argv
require.NoError(t, os.WriteFile(filepath.Join(dir, "cmdline"), []byte(strings.Join(argv, "\x00")+"\x00"), 0644))
}
// spawnLongRunningProcess starts a real, short-lived-by-signal process so
// reap tests can send it a genuine SIGTERM and observe a genuine exit,
// instead of pretending a fabricated /proc entry is a live process (SIGTERM
// only ever reaches the real kernel process table, never procRoot).
func spawnLongRunningProcess(t *testing.T) int {
t.Helper()
cmd := exec.Command("sleep", "60")
require.NoError(t, cmd.Start())
pid := cmd.Process.Pid
done := make(chan struct{})
go func() { _ = cmd.Wait(); close(done) }()
t.Cleanup(func() {
_ = cmd.Process.Kill()
<-done
})
return pid
}
// writeReferencerFixture writes a /proc/<pid>/{comm,cmdline} pair for a
// process whose argv carries volumeSocket's endpoint the way QEMU's -blockdev
// does, so endpointReferencer has something positive to find. comm is
// cosmetic only -- endpointReferencer does not filter by it, matching the
// "any process on the host may be a legitimate referencer" fail-safe design.
func writeReferencerFixture(t *testing.T, procRoot string, pid int, blockdevArg string) {
t.Helper()
writeProcFixture(t, procRoot, pid, "qemu-system-x86_64", []string{"qemu-system-x86_64", "-blockdev", blockdevArg})
}
// --- argv parsing ---
func TestParseNbdkitCmdline_SocketTransport(t *testing.T) {
@@ -330,3 +360,324 @@ func TestRecoverMountedVolumes_DuplicateVolumeAdoptedOnce(t *testing.T) {
}
assert.Equal(t, 1, count, "two nbdkits for one volume must produce one registry entry, not two")
}
// --- orphan reaping ---
// TestReapOrphanedNbdkit_NoReferencer_SignalsAndRemovesSocket proves the
// baseline case the bead asks for: an unclaimed nbdkit with nothing on the
// host referencing its endpoint is signalled and its socket cleaned up.
func TestReapOrphanedNbdkit_NoReferencer_SignalsAndRemovesSocket(t *testing.T) {
pid := spawnLongRunningProcess(t)
socket := filepath.Join(t.TempDir(), "vol-reap-unreferenced.sock")
listenUnix(t, socket)
procRoot := t.TempDir() // no other processes at all: nothing to find
logs := captureLogs(t)
disc := discoveredNbdkit{PID: pid, Volume: "vol-reap-unreferenced1", Socket: socket}
reapOrphanedNbdkit(procRoot, disc, reapReasonUnadoptable)
require.Eventually(t, func() bool { return !utils.ProcessAlive(pid) }, 2*time.Second, 20*time.Millisecond,
"an unreferenced orphan must be signalled and exit")
_, err := os.Stat(socket)
assert.True(t, os.IsNotExist(err), "the reaped orphan's socket file must be removed")
assert.Contains(t, logs.String(), "reaped unclaimed nbdkit process")
assert.Contains(t, logs.String(), fmt.Sprintf("pid=%d", pid))
assert.Contains(t, logs.String(), "volume=vol-reap-unreferenced1")
assert.Contains(t, logs.String(), "reason=unadoptable")
}
// TestReapOrphanedNbdkit_LiveReferencer_LeftRunningAndLogged proves the
// safety rule: an unclaimed nbdkit whose endpoint is still named on another
// process's cmdline must never be signalled, however unadoptable it looked.
func TestReapOrphanedNbdkit_LiveReferencer_LeftRunningAndLogged(t *testing.T) {
pid := spawnLongRunningProcess(t)
socket := filepath.Join(t.TempDir(), "vol-reap-referenced.sock")
listenUnix(t, socket)
procRoot := t.TempDir()
const referencerPID = 909090
writeReferencerFixture(t, procRoot, referencerPID,
fmt.Sprintf("driver=nbd,node-name=vol0,server.type=unix,server.path=%s,export=", socket))
logs := captureLogs(t)
disc := discoveredNbdkit{PID: pid, Volume: "vol-reap-referenced1", Socket: socket}
reapOrphanedNbdkit(procRoot, disc, reapReasonDuplicate)
// Give a real signal every chance to have landed before asserting it did not.
time.Sleep(100 * time.Millisecond)
assert.True(t, utils.ProcessAlive(pid), "a referenced orphan must never be signalled")
_, err := os.Stat(socket)
assert.NoError(t, err, "a live orphan's socket must not be removed")
assert.Contains(t, logs.String(), "left running deliberately")
assert.Contains(t, logs.String(), fmt.Sprintf("pid=%d", pid))
assert.Contains(t, logs.String(), "volume=vol-reap-referenced1")
assert.Contains(t, logs.String(), fmt.Sprintf("referencing_pid=%d", referencerPID))
}
// TestReapOrphanedNbdkit_TCPEndpointReferencedAcrossSplitBlockdevArgs proves
// endpointReferencer recognizes a TCP endpoint even when QEMU's -blockdev
// splits host and port into server.host=/server.port= within one argument,
// not just the joined host:port form a boot/EFI drive uses.
func TestReapOrphanedNbdkit_TCPEndpointReferencedAcrossSplitBlockdevArgs(t *testing.T) {
pid := spawnLongRunningProcess(t)
procRoot := t.TempDir()
const referencerPID = 909091
writeReferencerFixture(t, procRoot, referencerPID,
"driver=nbd,node-name=vol0,server.type=inet,server.host=127.0.0.1,server.port=10899,export=")
disc := discoveredNbdkit{PID: pid, Volume: "vol-reap-tcp1", Port: 10899}
reapOrphanedNbdkit(procRoot, disc, reapReasonUnadoptable)
time.Sleep(100 * time.Millisecond)
assert.True(t, utils.ProcessAlive(pid), "a TCP endpoint split across server.host=/server.port= must still be recognized as referenced")
}
// unreadableProcDir creates procRoot/<pid> with a cmdline file inside, then
// strips all permissions from the directory itself, so os.ReadFile on the
// cmdline path inside it fails with a permission error rather than
// os.ErrNotExist -- standing in for an /proc mounted with hidepid, where a
// guest owned by another uid is invisible to viperblockd entirely. Skips the
// calling test when running as root, since root ignores directory permission
// bits and the failure this simulates would not occur.
func unreadableProcDir(t *testing.T, procRoot string, pid int) {
t.Helper()
if os.Geteuid() == 0 {
t.Skip("cannot simulate a permission-denied /proc entry while running as root")
}
dir := filepath.Join(procRoot, strconv.Itoa(pid))
require.NoError(t, os.MkdirAll(dir, 0755))
require.NoError(t, os.WriteFile(filepath.Join(dir, "cmdline"), []byte("x\x00"), 0644))
require.NoError(t, os.Chmod(dir, 0000))
t.Cleanup(func() { _ = os.Chmod(dir, 0755) }) // let t.TempDir() clean up
}
// TestEndpointReferencer_MissingCandidateCmdline_NotAnError proves a
// candidate that exited between ReadDir and the cmdline read (ENOENT) is
// correctly read as "not a referencer", not as an inconclusive scan.
func TestEndpointReferencer_MissingCandidateCmdline_NotAnError(t *testing.T) {
procRoot := t.TempDir()
// A numbered entry with no cmdline file at all: the process vanished
// between the directory listing and this read.
require.NoError(t, os.MkdirAll(filepath.Join(procRoot, "424243"), 0755))
disc := discoveredNbdkit{PID: 424242, Volume: "vol-enoent1", Socket: filepath.Join(t.TempDir(), "x.sock")}
pid, found, err := endpointReferencer(procRoot, disc)
require.NoError(t, err, "a vanished candidate must not make the scan inconclusive")
assert.False(t, found)
assert.Zero(t, pid)
}
// TestEndpointReferencer_UnreadableCandidate_ReturnsError proves the hole the
// safety argument depends on is closed: a candidate this scan could not read
// for any reason other than ENOENT must surface as an error, not as "no
// referencer found", since a read failure is not evidence of absence.
func TestEndpointReferencer_UnreadableCandidate_ReturnsError(t *testing.T) {
procRoot := t.TempDir()
unreadableProcDir(t, procRoot, 555555)
disc := discoveredNbdkit{PID: 424242, Volume: "vol-eacces1", Socket: filepath.Join(t.TempDir(), "x.sock")}
_, found, err := endpointReferencer(procRoot, disc)
require.Error(t, err, "a permission-denied candidate must make the scan inconclusive, not silently pass")
assert.False(t, os.IsNotExist(err), "the error must not be mistaken for ENOENT")
assert.False(t, found)
}
// TestReapOrphanedNbdkit_UnreadableCandidate_LeftRunningAndLogged is the
// end-to-end version of the fix: an unclaimed nbdkit is left running, not
// reaped, when the referencer scan itself could not be completed -- the same
// fail-safe outcome as an unreadable procRoot, just triggered by one
// unreadable candidate entry among otherwise-readable ones.
func TestReapOrphanedNbdkit_UnreadableCandidate_LeftRunningAndLogged(t *testing.T) {
pid := spawnLongRunningProcess(t)
socket := filepath.Join(t.TempDir(), "vol-reap-eacces.sock")
listenUnix(t, socket)
procRoot := t.TempDir()
unreadableProcDir(t, procRoot, 555556)
logs := captureLogs(t)
disc := discoveredNbdkit{PID: pid, Volume: "vol-reap-eacces1", Socket: socket}
reapOrphanedNbdkit(procRoot, disc, reapReasonUnadoptable)
time.Sleep(100 * time.Millisecond)
assert.True(t, utils.ProcessAlive(pid), "an inconclusive scan must never result in a signal")
_, err := os.Stat(socket)
assert.NoError(t, err, "an orphan left running because the scan was inconclusive must keep its socket")
assert.Contains(t, logs.String(), "could not scan for processes")
assert.Contains(t, logs.String(), fmt.Sprintf("pid=%d", pid))
assert.Contains(t, logs.String(), "volume=vol-reap-eacces1")
}
// writeMountinfoFixture writes procRoot/self/mountinfo with a single "/proc"
// entry carrying opts, standing in for the real /proc/self/mountinfo that
// procHidepidInvisible parses on a live node.
func writeMountinfoFixture(t *testing.T, procRoot, opts string) {
t.Helper()
dir := filepath.Join(procRoot, "self")
require.NoError(t, os.MkdirAll(dir, 0755))
line := fmt.Sprintf("36 35 0:29 / /proc %s shared:1 - proc proc rw\n", opts)
require.NoError(t, os.WriteFile(filepath.Join(dir, "mountinfo"), []byte(line), 0644))
}
// TestProcHidepidInvisible_DetectsNamedAndLegacyValues proves the mountinfo
// parser recognizes both the named hidepid=invisible option and its legacy
// numeric equivalent, and does not false-positive on an unrelated mount.
func TestProcHidepidInvisible_DetectsNamedAndLegacyValues(t *testing.T) {
for _, tc := range []struct {
name string
opts string
want bool
}{
{"named invisible", "rw,nosuid,nodev,noexec,relatime,hidepid=invisible", true},
{"legacy numeric 2", "rw,nosuid,nodev,noexec,relatime,hidepid=2", true},
{"hidepid off", "rw,nosuid,nodev,noexec,relatime,hidepid=off", false},
{"hidepid noaccess (EACCES-style, already handled)", "rw,nosuid,nodev,noexec,relatime,hidepid=1", false},
{"no hidepid option at all", "rw,nosuid,nodev,noexec,relatime", false},
} {
t.Run(tc.name, func(t *testing.T) {
procRoot := t.TempDir()
writeMountinfoFixture(t, procRoot, tc.opts)
got, err := procHidepidInvisible(procRoot)
require.NoError(t, err)
assert.Equal(t, tc.want, got)
})
}
}
// TestProcHidepidInvisible_MissingMountinfoIsNotEvidence proves a fabricated
// procRoot with no self/mountinfo at all (every other test's fixture shape)
// is read as "not detected", not as an error -- so it never regresses tests
// written before this fail-safe existed.
func TestProcHidepidInvisible_MissingMountinfoIsNotEvidence(t *testing.T) {
got, err := procHidepidInvisible(t.TempDir())
require.NoError(t, err)
assert.False(t, got)
}
// TestReapOrphanedNbdkit_HidepidInvisible_DeclinesEvenOnACleanScan is the
// bug this fix closes: under hidepid=invisible a live referencer's
// /proc/<pid> directory entry is simply absent, not unreadable, so
// endpointReferencer would return a clean "not found" with no error at all.
// reapOrphanedNbdkit must detect the hazardous mount itself and decline
// before trusting that clean result, exactly as it declines on EACCES.
func TestReapOrphanedNbdkit_HidepidInvisible_DeclinesEvenOnACleanScan(t *testing.T) {
pid := spawnLongRunningProcess(t)
socket := filepath.Join(t.TempDir(), "vol-reap-hidepid.sock")
listenUnix(t, socket)
procRoot := t.TempDir() // no referencer entries at all: a clean scan
writeMountinfoFixture(t, procRoot, "rw,nosuid,nodev,noexec,relatime,hidepid=invisible")
logs := captureLogs(t)
disc := discoveredNbdkit{PID: pid, Volume: "vol-reap-hidepid1", Socket: socket}
reapOrphanedNbdkit(procRoot, disc, reapReasonUnadoptable)
time.Sleep(100 * time.Millisecond)
assert.True(t, utils.ProcessAlive(pid), "hidepid=invisible must never be read as a clean 'no referencer' scan")
_, err := os.Stat(socket)
assert.NoError(t, err, "an orphan left running because entries could be hidden must keep its socket")
assert.Contains(t, logs.String(), "hides other users' process entries")
assert.Contains(t, logs.String(), fmt.Sprintf("pid=%d", pid))
assert.Contains(t, logs.String(), "volume=vol-reap-hidepid1")
}
// TestRecoverMountedVolumes_DuplicateVolume_UnusedDuplicateReaped extends
// TestRecoverMountedVolumes_DuplicateVolumeAdoptedOnce: the duplicate
// recovery declines to adopt must not just be skipped but reaped, while the
// adopted process is never touched, matching the bead's acceptance that no
// volume is ever served by two processes.
func TestRecoverMountedVolumes_DuplicateVolume_UnusedDuplicateReaped(t *testing.T) {
_, natsURL := setupEmbeddedNATS(t)
cfg := setupTestConfig(t, natsURL)
cfg.constructVB = fileBackedConstructVB(t)
nc := startProviderSubjects(t, cfg, natsURL)
const volumeName = "vol-duplicatereap1"
procRoot := t.TempDir()
socketDir := t.TempDir()
// Both candidates are real processes: scanNbdkitProcs' adoption order
// follows os.ReadDir's lexical sort of the PID directory names, which is
// not something this test controls, so either could end up "adopted".
// Using two real spawned PIDs means whichever one recovery declines to
// adopt is always safe to actually SIGTERM.
pids := map[int]string{}
for _, name := range []string{"a", "b"} {
pid := spawnLongRunningProcess(t)
socket := filepath.Join(socketDir, name+".sock")
listenUnix(t, socket)
writeProcFixture(t, procRoot, pid, "nbdkit", nbdkitArgv(cfg.BaseDir, socket, 0, volumeName))
pids[pid] = socket
}
recoverMountedVolumes(context.Background(), cfg, nc, procRoot)
mv, ok := findMountedVolume(cfg, volumeName)
require.True(t, ok)
t.Cleanup(func() { mv.VB.StopWALSyncer() })
require.Contains(t, pids, mv.PID, "the adopted PID must be one of the two candidates")
var duplicatePID int
var duplicateSocket string
for pid, socket := range pids {
if pid != mv.PID {
duplicatePID, duplicateSocket = pid, socket
}
}
require.Eventually(t, func() bool { return !utils.ProcessAlive(duplicatePID) }, 2*time.Second, 20*time.Millisecond,
"the unused duplicate must be reaped")
assert.True(t, utils.ProcessAlive(mv.PID), "the adopted process must never be signalled")
_, err := os.Stat(duplicateSocket)
assert.True(t, os.IsNotExist(err), "the reaped duplicate's socket file must be removed")
}
// TestRecoverMountedVolumes_UnadoptableReapedAcrossRestarts covers the
// restart-then-stop sequence the bead asks for, not a single mount/unmount
// lifetime: a backend recovery cannot adopt (its volume no longer exists) is
// reaped on the first restart's recovery pass, and a second pass -- standing
// in for the field case's further stop/start cycles -- finds nothing left to
// reap or log, proving the orphan does not survive the way it did before
// this fix.
func TestRecoverMountedVolumes_UnadoptableReapedAcrossRestarts(t *testing.T) {
_, natsURL := setupEmbeddedNATS(t)
cfg := setupTestConfig(t, natsURL)
cfg.S3Host = fastFailingS3Host(t) // forces buildVB to fail: the volume is unadoptable
nc := startProviderSubjects(t, cfg, natsURL)
const volumeName = "vol-unadoptable-restart1"
procRoot := t.TempDir()
pid := spawnLongRunningProcess(t)
socket := filepath.Join(t.TempDir(), "o.sock") // short: unix socket paths cap at ~108 bytes
listenUnix(t, socket)
writeProcFixture(t, procRoot, pid, "nbdkit", nbdkitArgv(cfg.BaseDir, socket, 0, volumeName))
// First restart's recovery pass: the volume cannot be rebuilt, and
// nothing references its endpoint, so it must be reaped.
logs := captureLogs(t)
recoverMountedVolumes(context.Background(), cfg, nc, procRoot)
require.Eventually(t, func() bool { return !utils.ProcessAlive(pid) }, 2*time.Second, 20*time.Millisecond,
"an unadoptable backend with no referencer must be reaped on its first recovery pass")
assert.Contains(t, logs.String(), "reaped unclaimed nbdkit process")
_, ok := findMountedVolume(cfg, volumeName)
assert.False(t, ok, "a backend recovery could not rebuild must never appear in MountedVolumes")
// The kernel's real /proc would no longer list a PID that has exited;
// mirror that so the second pass sees exactly what a real restart would.
require.NoError(t, os.RemoveAll(filepath.Join(procRoot, strconv.Itoa(pid))))
// Second restart's recovery pass (standing in for further stop/start
// cycles): nothing survived, so there is nothing left to discover, reap
// or log -- unlike the field case, which was still there after six.
logs2 := captureLogs(t)
recoverMountedVolumes(context.Background(), cfg, nc, procRoot)
assert.NotContains(t, logs2.String(), "volume="+volumeName,
"a reaped orphan must not be rediscovered or re-logged on a later restart")
_, ok = findMountedVolume(cfg, volumeName)
assert.False(t, ok)
}
+1 -1
View File
@@ -20,7 +20,7 @@ var Units = map[string]string{
"spinifex-shutdown.service": "# spinifex-unit-version: 1\n[Unit]\nDescription=Spinifex graceful guest drain\nPartOf=spinifex.target\n# After= all services so it stops FIRST on target/host shutdown: the drain runs\n# while nats/predastore/viperblock/daemon are still up (QMP, unmount, WAL flush).\nAfter=spinifex-nats.service spinifex-predastore.service spinifex-viperblock.service spinifex-daemon.service\n\n[Service]\nType=oneshot\nRemainAfterExit=yes\nExecStart=/bin/true\n# ExecStop fires on ANY target stop, including the stop half of `systemctl\n# restart spinifex.target` (a rolling deploy) -- PartOf propagates regardless\n# of whether the host is actually going down. --unless-restarting drains on a\n# real shutdown/reboot and on a plain `systemctl stop spinifex.target`; it\n# skips only when a restart of the stack is already queued, so QEMU/nbdkit\n# survive (KillMode=process) and the daemon reconnects on the way back up.\nExecStop=/usr/local/bin/spx admin node drain --local --timeout=120s --unless-restarting\nTimeoutStopSec=180\n\n[Install]\nWantedBy=spinifex.target\n",
"spinifex-system.slice": "# spinifex-unit-version: 1\n[Unit]\nDescription=Spinifex control/infra tier (NATS, predastore, viperblock, daemon, awsgw, vpcd, ui)\nBefore=slices.target\n\n[Slice]\n# RG-3: child of spinifex.slice (spinifex-system -> spinifex), inherits the\n# parent ceiling, no extra limit. Holds the 7 control/infra services. Guest QEMU\n# moves to the sibling spinifex-guests.slice (per-instance placement lands in\n# 0002 RG-7); until then the daemon-forked guests stay under spinifex-daemon.service.\n",
"spinifex-ui.service": "# spinifex-unit-version: 1\n[Unit]\nDescription=Spinifex Web UI\nPartOf=spinifex.target\nAfter=spinifex-awsgw.service\n\n[Service]\nEnvironmentFile=-/etc/spinifex/telemetry.env\nType=simple\nSlice=spinifex-system.slice\nUser=spinifex-ui\nGroup=spinifex\nExecStart=/usr/local/bin/spx service spinifex-ui start\nRestart=on-failure\nRestartSec=5\n# server.Shutdown() uses an internal 30s context (WriteTimeout=120s covers\n# long streaming proxy responses, e.g. console passthrough); add margin above it.\nTimeoutStopSec=40\nOOMScoreAdjust=-500\n# Per-service ceiling bounds a leak in this service alone rather than\n# tripping the slice-wide cap, which would pick an arbitrary victim.\nMemoryHigh=10%\nMemoryMax=20%\nEnvironment=SPINIFEX_UI_TLS_CERT=/etc/spinifex/server.pem\nEnvironment=SPINIFEX_UI_TLS_KEY=/etc/spinifex/server.key\nEnvironment=SPINIFEX_UI_BASE_DIR=/var/lib/spinifex/spinifex-ui/\n\n# Filesystem access\nNoNewPrivileges=yes\nProtectSystem=strict\nPrivateDevices=yes\nReadOnlyPaths=/etc/spinifex/server.pem /etc/spinifex/server.key\nReadWritePaths=/var/lib/spinifex/spinifex-ui /var/log/spinifex\n\n# Hardening\nProtectHome=yes\nPrivateTmp=yes\nRestrictSUIDSGID=yes\nLockPersonality=yes\nRestrictRealtime=yes\nRestrictNamespaces=yes\nProtectClock=yes\nProtectKernelTunables=yes\nProtectKernelModules=yes\nProtectKernelLogs=yes\nProtectControlGroups=yes\nProtectProc=invisible\nSystemCallArchitectures=native\nMemoryDenyWriteExecute=yes\nRestrictAddressFamilies=AF_INET AF_INET6 AF_UNIX AF_NETLINK\nCapabilityBoundingSet=\nProtectHostname=yes\nKeyringMode=private\nRemoveIPC=yes\nLimitCORE=0\nLimitNOFILE=65536\nLimitNPROC=4096\n\n[Install]\nWantedBy=spinifex.target\n",
"spinifex-viperblock.service": "# spinifex-unit-version: 1\n[Unit]\nDescription=Spinifex Viperblock (Block Storage)\nPartOf=spinifex.target\nAfter=spinifex-nats.service spinifex-predastore.service\nWants=spinifex-nats.service\n\n[Service]\nEnvironmentFile=-/etc/spinifex/telemetry.env\nType=simple\nSlice=spinifex-system.slice\nUser=spinifex-viperblock\nGroup=spinifex\nExecStartPre=/var/lib/spinifex/wait-for-nats.sh\nExecStart=/usr/local/bin/spx service viperblock start\nRestart=on-failure\nRestartSec=5\nOOMScoreAdjust=-500\n# Per-service ceiling bounds a leak in this service alone rather than\n# tripping the slice-wide cap, which would pick an arbitrary victim.\nMemoryHigh=15%\nMemoryMax=25%\n# KillMode=process: nbdkit still serving a guest survives a restart here, the\n# way guest QEMU survives a daemon restart. Tearing one out mid-write corrupts\n# that guest's filesystem; idle nbdkit is reaped explicitly on shutdown.\nKillMode=process\n# One worst-case nbdkit reap: killProcessGracePeriod (120s, utils.KillProcess)\n# plus its own shutdownDrainTimeout (20s, viperblock/nbd) and headroom — must\n# strictly exceed 120s now shutdownVolumes reaps concurrently, not serially.\nTimeoutStopSec=150\nEnvironmentFile=/etc/spinifex/systemd.env\nEnvironment=SPINIFEX_CONFIG_PATH=/etc/spinifex/spinifex.toml\nEnvironment=SPINIFEX_BASE_DIR=/var/lib/spinifex/viperblock/\n\n# Filesystem access\nNoNewPrivileges=yes\nProtectSystem=strict\nPrivateDevices=yes\nReadOnlyPaths=/etc/spinifex/spinifex.toml /etc/spinifex/systemd.env /etc/spinifex/ca.pem\nReadWritePaths=/var/lib/spinifex/viperblock /var/log/spinifex /run/spinifex/nbd\n\n# Hardening\nProtectHome=yes\nPrivateTmp=yes\nRestrictSUIDSGID=yes\nLockPersonality=yes\nRestrictRealtime=yes\nRestrictNamespaces=yes\nProtectClock=yes\nProtectKernelTunables=yes\nProtectKernelModules=yes\nProtectKernelLogs=yes\nProtectControlGroups=yes\nProtectProc=invisible\nSystemCallArchitectures=native\nMemoryDenyWriteExecute=yes\nRestrictAddressFamilies=AF_INET AF_INET6 AF_UNIX AF_NETLINK\nCapabilityBoundingSet=\nProtectHostname=yes\nKeyringMode=private\nRemoveIPC=yes\nLimitCORE=0\nLimitNOFILE=65536\nLimitNPROC=4096\n\n[Install]\nWantedBy=spinifex.target\n",
"spinifex-viperblock.service": "# spinifex-unit-version: 2\n[Unit]\nDescription=Spinifex Viperblock (Block Storage)\nPartOf=spinifex.target\nAfter=spinifex-nats.service spinifex-predastore.service\nWants=spinifex-nats.service\n\n[Service]\nEnvironmentFile=-/etc/spinifex/telemetry.env\nType=simple\nSlice=spinifex-system.slice\nUser=spinifex-viperblock\nGroup=spinifex\nExecStartPre=/var/lib/spinifex/wait-for-nats.sh\nExecStart=/usr/local/bin/spx service viperblock start\nRestart=on-failure\nRestartSec=5\nOOMScoreAdjust=-500\n# Per-service ceiling bounds a leak in this service alone rather than\n# tripping the slice-wide cap, which would pick an arbitrary victim.\nMemoryHigh=15%\nMemoryMax=25%\n# KillMode=process: nbdkit still serving a guest survives a restart here, the\n# way guest QEMU survives a daemon restart. Tearing one out mid-write corrupts\n# that guest's filesystem; idle nbdkit is reaped explicitly on shutdown.\nKillMode=process\n# One worst-case nbdkit reap: killProcessGracePeriod (120s, utils.KillProcess)\n# plus its own shutdownDrainTimeout (20s, viperblock/nbd) and headroom — must\n# strictly exceed 120s now shutdownVolumes reaps concurrently, not serially.\nTimeoutStopSec=150\nEnvironmentFile=/etc/spinifex/systemd.env\nEnvironment=SPINIFEX_CONFIG_PATH=/etc/spinifex/spinifex.toml\nEnvironment=SPINIFEX_BASE_DIR=/var/lib/spinifex/viperblock/\n\n# Filesystem access\nNoNewPrivileges=yes\nProtectSystem=strict\nPrivateDevices=yes\nReadOnlyPaths=/etc/spinifex/spinifex.toml /etc/spinifex/systemd.env /etc/spinifex/ca.pem\nReadWritePaths=/var/lib/spinifex/viperblock /var/log/spinifex /run/spinifex/nbd\n\n# Hardening\nProtectHome=yes\nPrivateTmp=yes\nRestrictSUIDSGID=yes\nLockPersonality=yes\nRestrictRealtime=yes\nRestrictNamespaces=yes\nProtectClock=yes\nProtectKernelTunables=yes\nProtectKernelModules=yes\nProtectKernelLogs=yes\nProtectControlGroups=yes\n# ProtectProc must stay default: hidepid=invisible hides spinifex-daemon's\n# QEMU /proc entries (different uid) orphan nbdkit reaping must see.\nSystemCallArchitectures=native\nMemoryDenyWriteExecute=yes\nRestrictAddressFamilies=AF_INET AF_INET6 AF_UNIX AF_NETLINK\nCapabilityBoundingSet=\nProtectHostname=yes\nKeyringMode=private\nRemoveIPC=yes\nLimitCORE=0\nLimitNOFILE=65536\nLimitNPROC=4096\n\n[Install]\nWantedBy=spinifex.target\n",
"spinifex-vpcd.service": "# spinifex-unit-version: 1\n[Unit]\nDescription=Spinifex VPC Daemon (OVN Networking)\nPartOf=spinifex.target\nAfter=spinifex-nats.service spinifex-daemon.service systemd-networkd.service openvswitch-switch.service ovn-controller.service\nWants=spinifex-nats.service ovn-controller.service\n\n[Service]\nEnvironmentFile=-/etc/spinifex/telemetry.env\nType=simple\nSlice=spinifex-system.slice\nUser=spinifex-vpcd\nGroup=spinifex\nExecStartPre=/var/lib/spinifex/wait-for-nats.sh\nExecStart=/usr/local/bin/spx service vpcd start\nRestart=on-failure\nRestartSec=5\n# Waits for an in-flight OVN drift-reconcile pass to return before exiting;\n# no sub-timeout bounds that call today, so this covers a normal pass.\nTimeoutStopSec=20\nOOMScoreAdjust=-500\n# Per-service ceiling: bounds a leak here instead of the slice-wide cap.\nMemoryHigh=10%\nMemoryMax=20%\nEnvironment=SPINIFEX_CONFIG_PATH=/etc/spinifex/spinifex.toml\nEnvironment=SPINIFEX_BASE_DIR=/var/lib/spinifex/vpcd/\n\n# RG-9: network tier. Per-tap IMDS removed the in-process setns, so vpcd no\n# longer enters a per-VPC netns: CAP_SYS_ADMIN is dropped and the filesystem\n# sandbox is restored.\n#\n# RG-10: these are ambient so ip/iptables/sysctl/arping inherit them on exec and\n# vpcd holds no sudoers rules at all. NoNewPrivileges stays off pending the ADR\n# update that retires the last escalation path.\nAmbientCapabilities=CAP_NET_ADMIN CAP_NET_RAW CAP_NET_BIND_SERVICE CAP_SETUID CAP_SETGID\nCapabilityBoundingSet=CAP_NET_ADMIN CAP_NET_RAW CAP_NET_BIND_SERVICE CAP_SETUID CAP_SETGID\nNoNewPrivileges=no\nProtectSystem=full\nPrivateDevices=yes\nReadOnlyPaths=/etc/spinifex/spinifex.toml /etc/spinifex/server.pem /etc/spinifex/server.key /etc/spinifex/ca.pem\nReadWritePaths=/var/lib/spinifex/vpcd /var/log/spinifex\n\n# RG-9: hardening baseline\nProtectHome=yes\nPrivateTmp=yes\nRestrictSUIDSGID=yes\nLockPersonality=yes\nRestrictRealtime=yes\nRestrictNamespaces=yes\nProtectClock=yes\nProtectKernelTunables=yes\nProtectKernelModules=yes\nProtectKernelLogs=yes\nProtectControlGroups=yes\nProtectProc=invisible\nSystemCallArchitectures=native\nMemoryDenyWriteExecute=yes\n# AF_PACKET: raw L2 BOOTP for the in-process DHCP client before the iface has an IP\nRestrictAddressFamilies=AF_INET AF_INET6 AF_UNIX AF_NETLINK AF_PACKET\nProtectHostname=yes\nKeyringMode=private\nRemoveIPC=yes\nLimitCORE=0\nLimitNOFILE=65536\nLimitNPROC=4096\n\n[Install]\nWantedBy=spinifex.target\n",
"spinifex.slice": "# spinifex-unit-version: 1\n[Unit]\nDescription=Spinifex resource-bounded slice (host ceiling)\nBefore=slices.target\n\n[Slice]\n# RG-1: no CPUWeight — slice roots are siblings; capping below the default would\n# let user.slice out-prioritize the VM fleet. Operator CPU priority lives on\n# system.slice (system.slice.d/spinifex-reserve.conf).\n# RG-1/RG-5: throttle+reclaim at High, hard wall at Max (% of physical RAM).\nMemoryHigh=80%\nMemoryMax=90%\n# RG-5: spill to the full swap device before the MemoryMax wall; matched to swap size.\nMemorySwapMax=8G\n",
"spinifex.target": "# spinifex-unit-version: 1\n[Unit]\nDescription=Spinifex Infrastructure Platform\nAfter=network-online.target\nWants=network-online.target\nWants=spinifex-nats.service spinifex-predastore.service spinifex-northstar.service spinifex-viperblock.service spinifex-daemon.service spinifex-awsgw.service spinifex-vpcd.service spinifex-ui.service spinifex-qmp-collector.service spinifex-shutdown.service spinifex-nats-watchdog.timer openvswitch-switch.service ovn-controller.service\n\n[Install]\nWantedBy=multi-user.target\n",
+7 -4
View File
@@ -431,10 +431,13 @@ var unitBodyHashes = map[string]map[int]string{
"spinifex-shutdown.service": {1: "bcdc455916f35aa7494b2fe25e691339e8f1e22f031dfd9fd95203a9aa4bdaa4"},
"spinifex-system.slice": {1: "ca450c2b28a8b13dd767957fa9469bd74bd222d7abed79945e83d564d5ce16dd"},
"spinifex-ui.service": {1: "9d5ec3785bf730405f23ec9df79675bf65ecbca9df47c15f5a4b4639393afbc5"},
"spinifex-viperblock.service": {1: "5c8cdd2004abf8e5725cbce0200565b9b29be7ab3210a4e0a2cdfe37ec5facb9"},
"spinifex-vpcd.service": {1: "1b722640310145767cd34e87f4804852e63f46d2145ed20f8cc5f5400ebc5965"},
"spinifex.slice": {1: "f73d9343e0e1bedd647835c8bb0c80fb3a3bd66474661234ecac23a4caafc24f"},
"spinifex.target": {1: "0ffba9faee5a477f8ff7466a6bccb4dc7e04f5cf92a405553c242e6548402078"},
"spinifex-viperblock.service": {
1: "5c8cdd2004abf8e5725cbce0200565b9b29be7ab3210a4e0a2cdfe37ec5facb9",
2: "5d691bf5ce4a5a636d5dcc07f7bfa36d0da9eb0d7fb26079f89358ca0e5440cd",
},
"spinifex-vpcd.service": {1: "1b722640310145767cd34e87f4804852e63f46d2145ed20f8cc5f5400ebc5965"},
"spinifex.slice": {1: "f73d9343e0e1bedd647835c8bb0c80fb3a3bd66474661234ecac23a4caafc24f"},
"spinifex.target": {1: "0ffba9faee5a477f8ff7466a6bccb4dc7e04f5cf92a405553c242e6548402078"},
}
// TestUnitBodyPinnedToVersion asserts each unit's body hash matches the