From 1654f98e185af47e5790e3ec3338e8dfd8c7c770 Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Tue, 9 Jun 2026 22:56:46 +1000 Subject: [PATCH] =?UTF-8?q?feat(resource-governance):=20RG-3/4/6/8/9/11=20?= =?UTF-8?q?=E2=80=94=20admission=20charge,=20OOM=20ladder,=20tier=20confin?= =?UTF-8?q?ement=20(#327)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(daemon): live MemAvailable admission gate + fixed-services reserve scope Admission previously charged only guest -m against a flat reserve, blind to the co-located storage VMs, nbdkit, and daemon stack — so it greenlit launches that overcommit real RAM and OOM-kill guests on memory-tight hosts. Add a second admission gate that clamps the accounting-derived launch count by live /proc/meminfo MemAvailable (which already nets reclaimable cache and sees every untracked consumer): admitted = min(accounting, (MemAvailable-reserve)/ guestMem). Reads live per admission, fails open on a /proc read error so a transient fault can't wedge launches, and is disabled by SPINIFEX_ADMISSION_LIVE_MEM=0. Re-scope the host reserve to the fixed infrastructure tier only; per-instance nbdkit is no longer expected to fit inside it (explicit per-instance charge is a follow-up; the live gate covers it implicitly in the meantime). Pure helpers (liveMemCount, parseMemAvailableKB, liveMemAdmissionEnabled) are unit-tested; liveMemGate covered via an injected reader. * feat(daemon): charge nbdkit per-instance in admission accounting An instance's admission memory charge is now its guest -m plus the nbdkit processes backing its volumes (one per attached volume), not guest -m alone. The static reserve no longer has to absorb per-instance nbdkit, which was unsizable and the root of the host-memory-overcommit OOM. Per-volume charge is measured per class (main 768 MiB: 128 cache + plugin working set + in-flight I/O; aux 96 MiB: process floor, 0 cache), env-tunable via SPINIFEX_NBDKIT_MAIN_MIB / SPINIFEX_NBDKIT_AUX_MIB. Default layout is one main + two aux volumes (root + cloudinit + efi) ~= 0.94 GB/instance. Applied at every accounting site through instanceMemChargeMiB so charge and release never drift; the live MemAvailable gate backstops block-device-mapping volumes the static count misses. * feat(vm): three-tier guest OOM ladder + spinifex-guests.slice Replace the flat oom_score_adj=500 on every guest QEMU with a ManagedBy-derived ladder: user guests (ManagedBy=='') stay +500 and are reaped first under host memory pressure; system instances (ELBv2 LB-VMs, EKS control-plane nodes, ManagedBy!='') score 0, ranking above user guests but below the infra tier (-500). Killing a system instance degrades the data path or cluster control plane for many user VMs, so it must outlive them. Add spinifex-guests.slice as a sibling of spinifex-system under spinifex.slice — the parent tier for the per-instance leaves that land with RG-7. Guest QEMU placement into the slice is deferred to the per-instance work. * refactor(systemd): lean unit comments to RG-n tags + RG-9/RG-11 invariants Strip the multi-paragraph rationale novels from spinifex-daemon.service (48->7 comment lines), spinifex-vpcd.service (29->8) and spinifex.slice (15->5), replacing them with settings + terse # RG-n clause tags. Remove plan-doc, bead and CI-run references from comments (project policy). Directives are byte-for-byte unchanged — comment-only. Add RG-9/RG-11 invariant tests asserting per-tier confinement (locked-down storage/control tier drops all caps + NoNewPrivileges=yes; daemon keeps exactly CAP_SYS_ADMIN+CAP_DAC_OVERRIDE and the four-entry DeviceAllow; vpcd is the documented network-tier exception) and the lean-comment budget + no-plan-ref rule. --- build/systemd/spinifex-daemon.service | 55 +------ build/systemd/spinifex-guests.slice | 8 + build/systemd/spinifex-system.slice | 9 +- build/systemd/spinifex-vpcd.service | 38 +---- build/systemd/spinifex.slice | 20 +-- spinifex/daemon/daemon.go | 79 ++++++++-- spinifex/daemon/daemon_test.go | 29 ++-- spinifex/daemon/resource.go | 143 ++++++++++++++++- spinifex/daemon/resource_test.go | 181 ++++++++++++++++++++++ spinifex/systemd/units_invariants_test.go | 158 +++++++++++++++++++ spinifex/vm/lifecycle.go | 24 ++- spinifex/vm/lifecycle_test.go | 16 ++ 12 files changed, 629 insertions(+), 131 deletions(-) create mode 100644 build/systemd/spinifex-guests.slice create mode 100644 spinifex/systemd/units_invariants_test.go diff --git a/build/systemd/spinifex-daemon.service b/build/systemd/spinifex-daemon.service index ef1cf94a6..202410762 100644 --- a/build/systemd/spinifex-daemon.service +++ b/build/systemd/spinifex-daemon.service @@ -2,67 +2,29 @@ Description=Spinifex Daemon (VM Orchestration) PartOf=spinifex.target After=spinifex-nats.service spinifex-viperblock.service openvswitch-switch.service ovn-controller.service openvswitch-ipsec.service -# Wants=openvswitch-ipsec.service so systemd brings the IPsec userspace -# daemon up before spinifex flips other_config:ipsec_encapsulation=true. -# Without that daemon, OVS holds the cert pointers unused and strongSwan -# never gets IKE config — tunnel traffic is encrypted with no SAs and -# silently dropped. Wants= (not Requires=) keeps daemon bootable on -# single-node / lab hosts where the package is intentionally absent. +# Wants (not Requires): stay bootable when openvswitch-ipsec is absent Wants=openvswitch-ipsec.service -# No Wants=spinifex-nats.service: Wants= reactivates a stopped unit during -# `systemctl restart spinifex-daemon`, which would mask the standalone-startup -# path the Tier 1 DDIL Scenario B exercises. spinifex.target Wants= covers -# cold boot. [Service] Type=simple Slice=spinifex-system.slice User=spinifex-daemon Group=spinifex -# DDIL Tier 1 (siv-17): no ExecStartPre wait-for-nats. The daemon starts in -# standalone mode and reconnects to NATS in the background — process-exiting -# on a missing NATS would kill the local VM management plane. -# -# Opt-in strict-startup (§1d-strict, siv-44): set SPINIFEX_REQUIRE_NATS=1 to -# restore pre-DDIL fail-fast UX. First NATS connect uses a ~30s bounded retry -# and the daemon aborts on timeout. Intended for dev/test/single-node deploys -# where silent NATS misconfig is more confusing than helpful. Do NOT set this -# in the shipped unit — it re-introduces the SPOF that Tier 1 removed. ExecStart=/usr/local/bin/spx service spinifex start Restart=on-failure RestartSec=5 -# DDIL Tier 1: 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: guest QEMU survives daemon restart; the new daemon reattaches (DDIL) 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 +# RG-4: infra tier — reaped only after every guest OOMScoreAdjust=-500 Environment=SPINIFEX_CONFIG_PATH=/etc/spinifex/spinifex.toml Environment=SPINIFEX_BASE_DIR=/var/lib/spinifex/spinifex/ Environment=SPINIFEX_WAL_DIR=/var/lib/spinifex/spinifex/ -# DDIL Tier 1: QEMU pidfiles, QMP sockets, and the -# shared qemu.log all resolve via utils.pidPath() — which falls back -# to os.TempDir() (=/tmp) when $HOME/spinifex doesn't exist. With -# PrivateTmp=yes, each daemon start gets a fresh /tmp namespace, so -# pidfiles written by QEMU (running in the OLD daemon's namespace) -# are invisible to the new daemon. The /local/instances handler then -# reports PID=0 and Scenario B fails its "live QEMU PID" assertion. -# Pin XDG_RUNTIME_DIR to the host-wide tmpfs at /run/spinifex (created -# by tmpfiles.d, already in ReadWritePaths) so pidPath() returns a -# stable, namespace-independent path that both daemon incarnations and -# QEMU children resolve identically. +# Pin to host tmpfs: QEMU pidfiles/QMP sockets must survive PrivateTmp namespace churn (DDIL) Environment=XDG_RUNTIME_DIR=/run/spinifex -# Filesystem access — NoNewPrivileges omitted (daemon uses sudo for ip, ovs-vsctl, dhcpcd) -# CAP_SYS_ADMIN: required for GPU passthrough sysfs operations (vfio group management). -# CAP_DAC_OVERRIDE: sysfs driver bind/unbind paths (/sys/bus/pci/drivers/vfio-pci/bind, -# unbind) are root:root 0200; non-root processes need CAP_DAC_OVERRIDE to open them. +# RG-9: daemon tier — privileged by necessity (GPU vfio). NoNewPrivileges omitted (RG-10: sudo ip/ovs-vsctl/dhcpcd) AmbientCapabilities=CAP_SYS_ADMIN CAP_DAC_OVERRIDE ProtectSystem=full PrivateDevices=no @@ -74,7 +36,7 @@ DeviceAllow=/dev/vfio/vfio rw ReadOnlyPaths=/etc/spinifex/spinifex.toml /etc/spinifex/server.pem /etc/spinifex/server.key /etc/spinifex/ca.pem ReadWritePaths=/var/lib/spinifex/spinifex /var/log/spinifex /run/spinifex /run/spinifex/nbd -# Hardening +# RG-9: hardening baseline ProtectHome=yes PrivateTmp=yes RestrictSUIDSGID=yes @@ -89,8 +51,6 @@ ProtectControlGroups=yes ProtectProc=invisible SystemCallArchitectures=native MemoryDenyWriteExecute=yes -# DHCP lease acquisition moved to spinifex-vpcd (see dhcp-allocate-error.md); -# the daemon no longer needs AF_PACKET or the softened kernel protections. RestrictAddressFamilies=AF_INET AF_INET6 AF_UNIX AF_NETLINK ProtectHostname=yes KeyringMode=private @@ -98,8 +58,7 @@ RemoveIPC=yes LimitCORE=0 LimitNOFILE=65536 LimitNPROC=4096 -# VFIO GPU passthrough requires locking all guest RAM into physical memory. -# Without this, QEMU fails with RLIMIT_MEMLOCK exceeded when pinning guest pages. +# LimitMEMLOCK=infinity: VFIO pins all guest RAM LimitMEMLOCK=infinity [Install] diff --git a/build/systemd/spinifex-guests.slice b/build/systemd/spinifex-guests.slice new file mode 100644 index 000000000..515b52e28 --- /dev/null +++ b/build/systemd/spinifex-guests.slice @@ -0,0 +1,8 @@ +[Unit] +Description=Spinifex guest VM tier +Before=slices.target + +[Slice] +# RG-3: child of spinifex.slice (spinifex-guests -> spinifex), sibling of +# spinifex-system. Parent for the per-instance spinifex-instance-.slice +# leaves (0002 RG-7). Inherits the spinifex.slice ceiling; no extra limit here. diff --git a/build/systemd/spinifex-system.slice b/build/systemd/spinifex-system.slice index 1c52d8282..bf02a0771 100644 --- a/build/systemd/spinifex-system.slice +++ b/build/systemd/spinifex-system.slice @@ -3,8 +3,7 @@ Description=Spinifex control/infra tier (NATS, predastore, viperblock, daemon, a Before=slices.target [Slice] -# Child of spinifex.slice by name (spinifex-system -> spinifex) — inherits the -# parent ceiling, no extra limit. Holds the 7 control/infra services (and, until -# the guest sub-slice lands, the daemon-forked QEMU guests under -# spinifex-daemon.service). Exists now so a future spinifex-guests.slice can -# become a sibling without re-editing the service units. +# RG-3: child of spinifex.slice (spinifex-system -> spinifex), inherits the +# parent ceiling, no extra limit. Holds the 7 control/infra services. Guest QEMU +# moves to the sibling spinifex-guests.slice (per-instance placement lands in +# 0002 RG-7); until then the daemon-forked guests stay under spinifex-daemon.service. diff --git a/build/systemd/spinifex-vpcd.service b/build/systemd/spinifex-vpcd.service index 9cefa87d7..61c262d07 100644 --- a/build/systemd/spinifex-vpcd.service +++ b/build/systemd/spinifex-vpcd.service @@ -17,45 +17,23 @@ OOMScoreAdjust=-500 Environment=SPINIFEX_CONFIG_PATH=/etc/spinifex/spinifex.toml Environment=SPINIFEX_BASE_DIR=/var/lib/spinifex/vpcd/ -# vpcd MUST share the HOST mount namespace. IMDS binds one listener per VPC inside -# a per-VPC netns entered in-process via setns(CLONE_NEWNET); the /run/netns/ -# bind-mount that `ip netns add` creates must be host-visible (for diagnostics) and -# must survive a vpcd restart (so the listener can rebind). ANY filesystem-sandbox -# directive — ProtectSystem / ProtectHome / PrivateTmp / ProtectKernel* / -# ProtectControlGroups / ProtectProc / Read{Only,Write}Paths — forks a PRIVATE -# mount namespace that traps that bind-mount inside vpcd, leaving the host a stale -# handle that fails setns(2) with EINVAL (CI runs 26920490454, 26922587471). Those -# directives are therefore omitted by design, and so is the MountFlags=shared / -# spinifex-netns.service propagation workaround they required. -# -# Boundary that REMAINS: DAC (User=spinifex-vpcd) plus the seccomp / capability / -# namespace / address-family restrictions below. The durable fix that restores -# filesystem sandboxing AND drops CAP_SYS_ADMIN is the process-per-netns model -# (`ip netns exec` helper instead of in-process setns) — see -# docs/development/improvements/imds-l2-datapath.md Phase 5. +# RG-9: network tier. vpcd MUST share the host mount namespace — IMDS enters +# per-VPC netns via setns(), and any filesystem-sandbox directive (ProtectSystem/ +# Home, PrivateTmp, ProtectKernel*, ProtectControlGroups/Proc, Read*Paths) forks a +# private mount ns that traps the /run/netns bind-mount and breaks setns(2). Those, +# plus MemoryDenyWriteExecute, are omitted by design — not regressions. PrivateDevices=no - -# Hardening that does NOT fork a mount namespace (kept) RestrictSUIDSGID=no LockPersonality=yes RestrictRealtime=yes ProtectClock=yes -# IMDS enters the per-VPC netns via setns(CLONE_NEWNET); the `ip -n ` / -# `ip netns exec` plumbing also unshares a mount namespace (CLONE_NEWNS, to give -# the netns its own /sys), so `mnt` must be allowlisted alongside `net` or the -# first in-netns command fails with EPERM. +# mnt alongside net: in-netns `ip` unshares a mount ns for its own /sys RestrictNamespaces=net mnt SystemCallArchitectures=native MemoryDenyWriteExecute=no -# AF_PACKET: required by the in-process DHCP client (github.com/insomniacslk/dhcp -# / nclient4) for raw L2 BOOTP on br-wan before an IP is configured on the -# interface. See docs/development/bugs/dhcp-allocate-error.md. +# AF_PACKET: raw L2 BOOTP for the in-process DHCP client before the iface has an IP RestrictAddressFamilies=AF_INET AF_INET6 AF_UNIX AF_NETLINK AF_PACKET -# CAP_NET_RAW: open AF_PACKET sockets. -# CAP_NET_BIND_SERVICE: covers unicast RENEW to UDP :68. -# CAP_NET_ADMIN: already used for ovs-vsctl / ip link wiring on the bridge. -# CAP_SYS_ADMIN: setns(CLONE_NEWNET) into per-VPC IMDS netns to bind the listener. -# Note, disabled for now, sudo issue with ovs +# Caps: NET_ADMIN (ovs/ip), NET_RAW (AF_PACKET), NET_BIND_SERVICE (:68), SETUID/SETGID, SYS_ADMIN (setns into VPC netns) AmbientCapabilities=CAP_NET_ADMIN CAP_NET_RAW CAP_NET_BIND_SERVICE CAP_SETUID CAP_SETGID CAP_SYS_ADMIN CapabilityBoundingSet=CAP_NET_ADMIN CAP_NET_RAW CAP_NET_BIND_SERVICE CAP_SETUID CAP_SETGID CAP_SYS_ADMIN NoNewPrivileges=no diff --git a/build/systemd/spinifex.slice b/build/systemd/spinifex.slice index 859095f70..42edceba2 100644 --- a/build/systemd/spinifex.slice +++ b/build/systemd/spinifex.slice @@ -3,21 +3,11 @@ Description=Spinifex resource-bounded slice (host ceiling) Before=slices.target [Slice] -# No CPUWeight — inherits the default 100. The slice roots -# (system/user/spinifex/init.scope) are siblings; capping spinifex below the -# default would let a stray user.slice job out-prioritize the VM fleet under -# contention, and CPUWeight only bites under contention anyway (idle host => -# spinifex still gets 100% of cores). Operator/sshd CPU priority is bought on -# system.slice instead (see system.slice.d/spinifex-reserve.conf). -# -# Memory: throttle+reclaim at High, hard wall at Max. Percentages are of -# physical RAM (systemd native, auto-scales with host size). Override per host -# via a spinifex.slice.d/*.conf drop-in + `systemctl daemon-reload`. +# RG-1: no CPUWeight — slice roots are siblings; capping below the default would +# let user.slice out-prioritize the VM fleet. Operator CPU priority lives on +# system.slice (system.slice.d/spinifex-reserve.conf). +# RG-1/RG-5: throttle+reclaim at High, hard wall at Max (% of physical RAM). MemoryHigh=80% MemoryMax=90% -# Host swap is an 8G swapfile (provisioned by setup.sh). Let spinifex spill to -# the full swap device for graceful degradation before the MemoryMax wall. With -# vm.swappiness=10 the kernel only swaps under real pressure, so this is a safety -# buffer, not a routine path. Matched to the swap device size; raise both -# together if the swapfile grows. +# RG-5: spill to the full swap device before the MemoryMax wall; matched to swap size. MemorySwapMax=8G diff --git a/spinifex/daemon/daemon.go b/spinifex/daemon/daemon.go index fa8081b1a..c2cc762d4 100644 --- a/spinifex/daemon/daemon.go +++ b/spinifex/daemon/daemon.go @@ -97,9 +97,23 @@ type ResourceManager struct { reservedMem float64 allocatedVCPU int allocatedMem float64 + // nbdkitMainMiB / nbdkitAuxMiB are the per-volume nbdkit memory charges + // (main vs aux), added to every instance's guest -m at admission so the + // nbdkit processes backing its volumes are accounted, not absorbed by the + // fixed reserve. See resolveNbdkitCharge / instanceMemChargeMiB. RG-6. + nbdkitMainMiB int + nbdkitAuxMiB int instanceTypes map[string]*ec2.InstanceTypeInfo gpuManager *gpu.Manager // nil if GPU passthrough is disabled or no GPUs present + // readMemAvailableGB is the live second admission gate. The static accounting + // above counts only guest -m, so co-located storage VMs, nbdkit (grows with + // guest disk I/O), the daemon stack and page cache are invisible to it. + // MemAvailable sees all of them and already nets reclaimable cache, catching + // real overcommit the accounting would greenlight. nil disables the gate + // (SPINIFEX_ADMISSION_LIVE_MEM=0); a read failure fails open. + readMemAvailableGB func() (float64, bool) + // Dynamic instance-type subscription management subsMu sync.Mutex natsConn *nats.Conn @@ -515,13 +529,25 @@ func NewResourceManager(gpuModels []instancetypes.GPUModel, migProfiles []instan "schedulableVCPU", hostVCPU-reservedVCPU, "schedulableMemGB", totalMemGB-reservedMem, "instanceTypes", len(instanceTypes)) + // Live-memory admission gate: enabled by default, reads /proc/meminfo + // MemAvailable at each admission. nil when disabled so liveMemGate is a no-op. + var memReader func() (float64, bool) + if liveMemAdmissionEnabled(os.Getenv) { + memReader = readMemAvailableGB + } + + nbdkitMainMiB, nbdkitAuxMiB := resolveNbdkitCharge(os.Getenv) + return &ResourceManager{ - hostVCPU: hostVCPU, - hostMemGB: totalMemGB, - reservedVCPU: reservedVCPU, - reservedMem: reservedMem, - instanceTypes: instanceTypes, - gpuManager: gpuMgr, + hostVCPU: hostVCPU, + hostMemGB: totalMemGB, + reservedVCPU: reservedVCPU, + reservedMem: reservedMem, + nbdkitMainMiB: nbdkitMainMiB, + nbdkitAuxMiB: nbdkitAuxMiB, + instanceTypes: instanceTypes, + gpuManager: gpuMgr, + readMemAvailableGB: memReader, }, nil } @@ -541,6 +567,16 @@ func instanceTypeMemoryMiB(it *ec2.InstanceTypeInfo) int64 { return 0 } +// instanceMemChargeMiB is the full memory admission charges one instance of it: +// its guest -m plus the nbdkit processes backing its volumes (RG-6). Every +// accounting gate (both canAllocateCount sites, liveMemGate, allocate, +// deallocate) divides remaining memory by — or moves the running total by — +// this value, so the charge and release can never drift. +func (rm *ResourceManager) instanceMemChargeMiB(it *ec2.InstanceTypeInfo) int64 { + return instanceTypeMemoryMiB(it) + + nbdkitChargeMiB(defaultMainVolumes, defaultAuxVolumes, rm.nbdkitMainMiB, rm.nbdkitAuxMiB) +} + // GetInstanceTypeInfos returns all instance types as ec2.InstanceTypeInfo for AWS API compatibility func (rm *ResourceManager) GetInstanceTypeInfos() []*ec2.InstanceTypeInfo { rm.mu.RLock() @@ -602,7 +638,7 @@ func (rm *ResourceManager) GetAvailableInstanceTypeInfos(showCapacity bool) []*e count := canAllocateCount( rm.hostVCPU-rm.reservedVCPU, rm.allocatedVCPU, rm.hostMemGB-rm.reservedMem, rm.allocatedMem, - vCPUs, memMiB, + vCPUs, rm.instanceMemChargeMiB(it), 1<<30, // effectively unlimited — let resources be the constraint 0, false, ) @@ -2124,14 +2160,35 @@ func (rm *ResourceManager) canAllocateLocked(instanceType *ec2.InstanceTypeInfo, return count } - return canAllocateCount( + n := canAllocateCount( rm.hostVCPU-rm.reservedVCPU, rm.allocatedVCPU, rm.hostMemGB-rm.reservedMem, rm.allocatedMem, instanceTypeVCPUs(instanceType), - instanceTypeMemoryMiB(instanceType), + rm.instanceMemChargeMiB(instanceType), count, 0, false, ) + return rm.liveMemGate(n, instanceType) +} + +// liveMemGate clamps an accounting-derived launch count by the host's current +// MemAvailable, so admission also refuses launches that fit the static -m budget +// but would overcommit real memory (untracked storage VMs, nbdkit, daemon stack, +// cache). Returns n unchanged when the gate is disabled (readMemAvailableGB is +// nil) or the live read fails — failing open keeps a transient /proc/meminfo +// error from wedging all launches; the accounting gate still applies. The bound +// holds at least reservedMem physically available after the launch, read live on +// each call so back-to-back launches see headroom shrink as guests fault in. +func (rm *ResourceManager) liveMemGate(n int, instanceType *ec2.InstanceTypeInfo) int { + if rm.readMemAvailableGB == nil || n <= 0 { + return n + } + availGB, ok := rm.readMemAvailableGB() + if !ok { + return n + } + memGB := float64(rm.instanceMemChargeMiB(instanceType)) / 1024.0 + return liveMemCount(n, availGB, rm.reservedMem, memGB) } // allocate reserves resources for one instance and updates NATS subscriptions. @@ -2150,7 +2207,7 @@ func (rm *ResourceManager) allocate(instanceType *ec2.InstanceTypeInfo) error { return fmt.Errorf("insufficient resources for instance type %s", instanceTypeName) } vCPUs := instanceTypeVCPUs(instanceType) - memoryGB := float64(instanceTypeMemoryMiB(instanceType)) / 1024.0 + memoryGB := float64(rm.instanceMemChargeMiB(instanceType)) / 1024.0 rm.allocatedVCPU += int(vCPUs) rm.allocatedMem += memoryGB rm.mu.Unlock() @@ -2163,7 +2220,7 @@ func (rm *ResourceManager) allocate(instanceType *ec2.InstanceTypeInfo) error { func (rm *ResourceManager) deallocate(instanceType *ec2.InstanceTypeInfo) { rm.mu.Lock() vCPUs := instanceTypeVCPUs(instanceType) - memoryGB := float64(instanceTypeMemoryMiB(instanceType)) / 1024.0 + memoryGB := float64(rm.instanceMemChargeMiB(instanceType)) / 1024.0 rm.allocatedVCPU -= int(vCPUs) rm.allocatedMem -= memoryGB rm.mu.Unlock() diff --git a/spinifex/daemon/daemon_test.go b/spinifex/daemon/daemon_test.go index 56e2af77b..ab40f7ed9 100644 --- a/spinifex/daemon/daemon_test.go +++ b/spinifex/daemon/daemon_test.go @@ -216,12 +216,9 @@ func TestResourceManager(t *testing.T) { if instanceType.VCpuInfo != nil && instanceType.VCpuInfo.DefaultVCpus != nil { vCPUs = *instanceType.VCpuInfo.DefaultVCpus } - memoryGB := float64(0) - if instanceType.MemoryInfo != nil && instanceType.MemoryInfo.SizeInMiB != nil { - memoryGB = float64(*instanceType.MemoryInfo.SizeInMiB) / 1024.0 - } + expectedMem := float64(rm.instanceMemChargeMiB(instanceType)) / 1024.0 // guest -m + nbdkit (RG-6) assert.Equal(t, int(vCPUs), rm.allocatedVCPU) - assert.Equal(t, memoryGB, rm.allocatedMem) + assert.Equal(t, expectedMem, rm.allocatedMem) // Deallocate rm.deallocate(instanceType) @@ -233,6 +230,7 @@ func TestResourceManager(t *testing.T) { // Fresh resource manager for predictable testing rm, err := NewResourceManager(nil, nil, nil) require.NoError(t, err) + rm.readMemAvailableGB = nil // accounting test: decrement must track allocations, not live /proc // Find a .micro instance type var microType *ec2.InstanceTypeInfo @@ -631,16 +629,16 @@ func TestHandleEC2DescribeInstanceTypes(t *testing.T) { err = json.Unmarshal(reply.Data, &output) require.NoError(t, err) - // With 2 vCPUs and 16GB, every type with 2 vCPUs and <=16GB memory should have 1 slot. - // Calculate expected by counting fitting types directly. + // With 2 vCPUs and 16GB, every type whose full charge (guest -m + nbdkit, + // RG-6) fits 2 vCPUs / 16GB should have 1 slot. Calculate expected directly. expectedSlots := 0 for name, it := range daemon.resourceMgr.instanceTypes { if instancetypes.IsSystemType(name) { continue } vcpus := *it.VCpuInfo.DefaultVCpus - memGB := float64(*it.MemoryInfo.SizeInMiB) / 1024.0 - if vcpus <= 2 && memGB <= 16.0 { + chargeGB := float64(daemon.resourceMgr.instanceMemChargeMiB(it)) / 1024.0 + if vcpus <= 2 && chargeGB <= 16.0 { expectedSlots++ } } @@ -752,7 +750,7 @@ func TestDaemon_BootAllocation(t *testing.T) { // Verify only i-running was allocated instanceType := daemon.resourceMgr.instanceTypes[vms["i-running"].InstanceType] expectedVCPU := int(*instanceType.VCpuInfo.DefaultVCpus) - expectedMem := float64(*instanceType.MemoryInfo.SizeInMiB) / 1024.0 + expectedMem := float64(daemon.resourceMgr.instanceMemChargeMiB(instanceType)) / 1024.0 // guest -m + nbdkit (RG-6) assert.Equal(t, expectedVCPU, daemon.resourceMgr.allocatedVCPU) assert.Equal(t, expectedMem, daemon.resourceMgr.allocatedMem) @@ -858,6 +856,7 @@ func TestCanAllocate_CountEdgeCases(t *testing.T) { rm.hostMemGB = 32.0 rm.reservedVCPU = 0 rm.reservedMem = 0 + rm.readMemAvailableGB = nil // accounting test: pin to synthetic host, not live /proc rm.mu.Unlock() initial := rm.canAllocate(microType, 100) @@ -963,7 +962,9 @@ func TestAllocate_NoOvercommitUnderContention(t *testing.T) { require.NotNil(t, microType, "test requires a .micro instance type") vCPUs := int(instanceTypeVCPUs(microType)) - memGB := float64(instanceTypeMemoryMiB(microType)) / 1024.0 + // Size the pool on the full per-instance charge (guest -m + nbdkit, RG-6), + // not bare guest -m, so capacityFor slots still fit exactly after RG-6. + memGB := float64(rm.instanceMemChargeMiB(microType)) / 1024.0 require.Greater(t, vCPUs, 0, "micro type must report vCPU count") require.Greater(t, memGB, float64(0), "micro type must report memory") @@ -976,6 +977,7 @@ func TestAllocate_NoOvercommitUnderContention(t *testing.T) { rm.reservedMem = 0 rm.allocatedVCPU = 0 rm.allocatedMem = 0 + rm.readMemAvailableGB = nil // accounting test: pin to synthetic host, not live /proc rm.mu.Unlock() require.Equal(t, capacityFor, rm.canAllocate(microType, goroutines), @@ -1396,10 +1398,11 @@ func TestInstanceTypeSubscriptions(t *testing.T) { handler := func(msg *nats.Msg) {} rm.initSubscriptions(nc, handler, nil, "test-node") - // Leave only 2 vCPUs and 1 GB schedulable — enough for nano/micro but not larger types. + // Leave only 2 vCPUs and 2.5 GB schedulable — enough for a nano/micro plus + // its nbdkit charge (RG-6), but not larger types. rm.mu.Lock() rm.allocatedVCPU = (rm.hostVCPU - rm.reservedVCPU) - 2 - rm.allocatedMem = (rm.hostMemGB - rm.reservedMem) - 1.0 + rm.allocatedMem = (rm.hostMemGB - rm.reservedMem) - 2.5 rm.mu.Unlock() rm.updateInstanceSubscriptions() diff --git a/spinifex/daemon/resource.go b/spinifex/daemon/resource.go index a16a08240..919a6812b 100644 --- a/spinifex/daemon/resource.go +++ b/spinifex/daemon/resource.go @@ -4,7 +4,10 @@ import ( "errors" "fmt" "log/slog" + "os" + "runtime" "strconv" + "strings" "github.com/aws/aws-sdk-go/service/ec2" "github.com/mulgadc/spinifex/spinifex/types" @@ -14,18 +17,24 @@ import ( // cannot be satisfied. var errInsufficientCapacity = errors.New("insufficient capacity to satisfy MinCount") -// hostReserve is the fixed amount of host CPU and RAM held back from guest -// scheduling so the spinifex daemon and co-located services (NATS, -// predastore, viperblock, vpcd, awsgw, ui) cannot be starved by guest VMs -// at maximum density. A future `capacity` command will lift this into -// operator-tunable config. +// hostReserve is the host CPU and RAM held back from guest scheduling for the +// always-on infrastructure tier only — the daemon and co-located fixed services +// (NATS, predastore, viperblock, vpcd, awsgw, ui). It deliberately does NOT +// cover per-instance consumers such as the nbdkit process backing a guest's +// volumes: those scale with instance count and disk I/O, so folding them into a +// fixed reserve makes it unsizable. Per-instance cost is charged to the instance +// and bounded by the live-memory admission gate (see liveMemGate). Tunable via +// SPINIFEX_RESERVED_VCPU / SPINIFEX_RESERVED_MEM_GB until a `capacity` command +// lifts it into config. type hostReserve struct { vCPU int memGB float64 } -// defaultHostReserve is sized to cover predastore + viperblock under load; -// the daemon itself needs little. +// defaultHostReserve covers the fixed infrastructure tier (predastore + +// viperblock storage VMs under load, plus the daemon/NATS/awsgw/vpcd/ui stack). +// Size it from the measured fixed-tier footprint of the target host via +// SPINIFEX_RESERVED_MEM_GB; the default is a conservative floor, not a ceiling. var defaultHostReserve = hostReserve{vCPU: 2, memGB: 2.0} // resolveHostReserve returns defaultHostReserve, with vCPU/memGB overridden @@ -141,6 +150,126 @@ func canAllocateCount(availVCPU, allocVCPU int, availMem, allocMem float64, return max(result, 0) } +// liveMemCount clamps an accounting-derived launch count n by live host memory: +// how many guests of memGB fit in (availMemGB − reservedMemGB) right now, never +// more than n. Pure function — the live read and instance-type lookup happen in +// liveMemGate. A negative headroom (host already below the reserve) yields 0, so +// admission refuses rather than admitting into OOM territory. +func liveMemCount(n int, availMemGB, reservedMemGB, memGB float64) int { + if memGB <= 0 { + return n + } + byLive := int((availMemGB - reservedMemGB) / memGB) + return max(0, min(n, byLive)) +} + +// liveMemAdmissionEnabled reports whether the live-memory admission gate is on. +// Default on; SPINIFEX_ADMISSION_LIVE_MEM set to a false-y value ("0", "false", +// "off", "no") disables it, falling back to pure accounting — a rollback hatch +// if the live gate proves too conservative on a busy host. +func liveMemAdmissionEnabled(getenv func(string) string) bool { + switch strings.ToLower(strings.TrimSpace(getenv("SPINIFEX_ADMISSION_LIVE_MEM"))) { + case "0", "false", "off", "no": + return false + default: + return true + } +} + +// readMemAvailableGB returns the host's current /proc/meminfo MemAvailable in +// GB. ok=false on non-Linux hosts or any read/parse failure, which disables the +// live gate for that call (fail open). MemAvailable is the kernel's own estimate +// of memory available for new workloads without swapping, already netting +// reclaimable page cache — the figure admission should gate on, and one that +// already reflects co-located storage VMs, nbdkit, and the daemon stack that the +// static -m accounting never sees. +func readMemAvailableGB() (float64, bool) { + if runtime.GOOS != "linux" { + return 0, false + } + data, err := os.ReadFile("/proc/meminfo") + if err != nil { + slog.Warn("live-mem admission gate: read /proc/meminfo failed, falling back to accounting", "err", err) + return 0, false + } + kb, ok := parseMemAvailableKB(data) + if !ok { + slog.Warn("live-mem admission gate: MemAvailable absent from /proc/meminfo, falling back to accounting") + return 0, false + } + return float64(kb) / (1024 * 1024), true +} + +// parseMemAvailableKB extracts the MemAvailable value (in kB) from /proc/meminfo +// contents. Returns ok=false when the field is absent or unparseable. Pure — +// split out for unit-testability without touching the filesystem. +func parseMemAvailableKB(data []byte) (int64, bool) { + for line := range strings.SplitSeq(string(data), "\n") { + key, val, ok := strings.Cut(line, ":") + if !ok || strings.TrimSpace(key) != "MemAvailable" { + continue + } + fields := strings.Fields(val) // " 12345 kB" + if len(fields) < 1 { + return 0, false + } + kb, err := strconv.ParseInt(fields[0], 10, 64) + if err != nil { + return 0, false + } + return kb, true + } + return 0, false +} + +// nbdkit runs one process per attached volume (spinifex/nbd). Measured on a live +// host: a main volume's nbdkit climbs to ~0.75 GB under guest I/O (128 MiB cache +// + plugin working set + in-flight buffers); aux volumes (-cloudinit, -efi; 0 +// cache) stay flat ~32-68 MB. Charged per instance so a busy nbdkit can't +// overcommit memory the static -m budget never saw. Env-tunable; RG-6. +const ( + defaultNbdkitMainMiB = 768 + defaultNbdkitAuxMiB = 96 +) + +// defaultMainVolumes / defaultAuxVolumes is the volume layout every instance +// gets: one main root volume plus -cloudinit and -efi aux volumes. Block-device +// mappings add main volumes the static charge can't see at admission; the live +// MemAvailable gate (RG-8) is the backstop for those. RG-6. +const ( + defaultMainVolumes = 1 + defaultAuxVolumes = 2 +) + +// resolveNbdkitCharge returns the per-volume nbdkit memory charge in MiB, with +// main/aux overridable via SPINIFEX_NBDKIT_MAIN_MIB / SPINIFEX_NBDKIT_AUX_MIB so +// an operator can retune from their own nbdkit RSS characterisation without a +// rebuild. Invalid or negative values are logged and ignored. +func resolveNbdkitCharge(getenv func(string) string) (mainMiB, auxMiB int) { + mainMiB, auxMiB = defaultNbdkitMainMiB, defaultNbdkitAuxMiB + if v := getenv("SPINIFEX_NBDKIT_MAIN_MIB"); v != "" { + if n, err := strconv.Atoi(v); err == nil && n >= 0 { + mainMiB = n + } else { + slog.Warn("ignoring SPINIFEX_NBDKIT_MAIN_MIB", "value", v, "err", err) + } + } + if v := getenv("SPINIFEX_NBDKIT_AUX_MIB"); v != "" { + if n, err := strconv.Atoi(v); err == nil && n >= 0 { + auxMiB = n + } else { + slog.Warn("ignoring SPINIFEX_NBDKIT_AUX_MIB", "value", v, "err", err) + } + } + return mainMiB, auxMiB +} + +// nbdkitChargeMiB is the per-instance nbdkit charge — one process per volume over +// the default layout (mainVols main + auxVols aux). Pure; RG-6. +func nbdkitChargeMiB(mainVols, auxVols, mainMiB, auxMiB int) int64 { + return int64(mainVols*mainMiB + auxVols*auxMiB) +} + // resourceStatsForType computes the InstanceTypeCap for a single instance type // given the remaining host resources. Pure function — no locks or side effects. // Callers are responsible for alarming on negative remainVCPU/remainMem diff --git a/spinifex/daemon/resource_test.go b/spinifex/daemon/resource_test.go index 8ae5e9a94..e29c38ced 100644 --- a/spinifex/daemon/resource_test.go +++ b/spinifex/daemon/resource_test.go @@ -6,6 +6,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestCanAllocateCount(t *testing.T) { @@ -463,3 +464,183 @@ func TestResolveHostVCPU(t *testing.T) { }) } } + +func TestLiveMemCount(t *testing.T) { + tests := []struct { + name string + n int + availMemGB, reservedMemGB, memGB float64 + want int + }{ + {"live looser than accounting keeps n", 3, 20, 2, 4, 3}, + {"live tighter clamps below n", 5, 10, 2, 4, 2}, // (10-2)/4 = 2 + {"live exactly meets n", 2, 10, 2, 4, 2}, // (10-2)/4 = 2 + {"headroom below one guest yields 0", 3, 5, 2, 4, 0}, // (5-2)/4 = 0 + {"available below reserve yields 0", 3, 1, 2, 4, 0}, // negative headroom + {"zero count stays zero", 0, 100, 2, 4, 0}, + {"zero memGB is a no-op (returns n)", 3, 1, 2, 0, 3}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := liveMemCount(tc.n, tc.availMemGB, tc.reservedMemGB, tc.memGB) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestParseMemAvailableKB(t *testing.T) { + meminfo := "MemTotal: 16384000 kB\n" + + "MemFree: 1048576 kB\n" + + "MemAvailable: 8388608 kB\n" + + "Buffers: 204800 kB\n" + tests := []struct { + name string + data string + wantKB int64 + wantOK bool + }{ + {"present", meminfo, 8388608, true}, + {"absent", "MemTotal: 16384000 kB\nMemFree: 1048576 kB\n", 0, false}, + {"malformed value", "MemAvailable: notanumber kB\n", 0, false}, + {"empty", "", 0, false}, + {"first match wins", "MemAvailable: 100 kB\nMemAvailable: 200 kB\n", 100, true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + kb, ok := parseMemAvailableKB([]byte(tc.data)) + assert.Equal(t, tc.wantOK, ok) + assert.Equal(t, tc.wantKB, kb) + }) + } +} + +func TestLiveMemAdmissionEnabled(t *testing.T) { + tests := []struct { + env string + want bool + }{ + {"", true}, + {"1", true}, + {"true", true}, + {"anything", true}, + {"0", false}, + {"false", false}, + {"off", false}, + {"no", false}, + {" OFF ", false}, + {"FALSE", false}, + } + for _, tc := range tests { + t.Run("env="+tc.env, func(t *testing.T) { + got := liveMemAdmissionEnabled(func(string) string { return tc.env }) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestLiveMemGate(t *testing.T) { + it := &ec2.InstanceTypeInfo{ + InstanceType: aws.String("t3.medium"), + MemoryInfo: &ec2.MemoryInfo{SizeInMiB: aws.Int64(4096)}, // 4 GB + } + + t.Run("nil reader is a no-op", func(t *testing.T) { + rm := &ResourceManager{reservedMem: 2} + assert.Equal(t, 5, rm.liveMemGate(5, it)) + }) + + t.Run("read failure fails open", func(t *testing.T) { + rm := &ResourceManager{reservedMem: 2, readMemAvailableGB: func() (float64, bool) { return 0, false }} + assert.Equal(t, 5, rm.liveMemGate(5, it)) + }) + + t.Run("live availability clamps below accounting", func(t *testing.T) { + // MemAvailable 11 GB, reserve 2 -> 9 GB headroom / 4 GB = 2 guests. + rm := &ResourceManager{reservedMem: 2, readMemAvailableGB: func() (float64, bool) { return 11, true }} + assert.Equal(t, 2, rm.liveMemGate(5, it)) + }) + + t.Run("host below reserve refuses all", func(t *testing.T) { + rm := &ResourceManager{reservedMem: 2, readMemAvailableGB: func() (float64, bool) { return 1, true }} + assert.Equal(t, 0, rm.liveMemGate(5, it)) + }) + + t.Run("zero count short-circuits before read", func(t *testing.T) { + called := false + rm := &ResourceManager{reservedMem: 2, readMemAvailableGB: func() (float64, bool) { called = true; return 100, true }} + assert.Equal(t, 0, rm.liveMemGate(0, it)) + assert.False(t, called, "reader must not be called when n<=0") + }) +} + +func TestResolveNbdkitCharge(t *testing.T) { + t.Run("defaults when unset", func(t *testing.T) { + main, aux := resolveNbdkitCharge(func(string) string { return "" }) + assert.Equal(t, defaultNbdkitMainMiB, main) + assert.Equal(t, defaultNbdkitAuxMiB, aux) + }) + + t.Run("env overrides both", func(t *testing.T) { + env := map[string]string{"SPINIFEX_NBDKIT_MAIN_MIB": "1024", "SPINIFEX_NBDKIT_AUX_MIB": "0"} + main, aux := resolveNbdkitCharge(func(k string) string { return env[k] }) + assert.Equal(t, 1024, main) + assert.Equal(t, 0, aux) + }) + + t.Run("invalid and negative values ignored", func(t *testing.T) { + env := map[string]string{"SPINIFEX_NBDKIT_MAIN_MIB": "nope", "SPINIFEX_NBDKIT_AUX_MIB": "-50"} + main, aux := resolveNbdkitCharge(func(k string) string { return env[k] }) + assert.Equal(t, defaultNbdkitMainMiB, main) + assert.Equal(t, defaultNbdkitAuxMiB, aux) + }) +} + +func TestNbdkitChargeMiB(t *testing.T) { + // Default layout: 1 main + 2 aux at the measured per-class figures. + assert.Equal(t, int64(768+2*96), nbdkitChargeMiB(1, 2, 768, 96)) + // Aux-only / main-only decompose linearly. + assert.Equal(t, int64(192), nbdkitChargeMiB(0, 2, 768, 96)) + assert.Equal(t, int64(768), nbdkitChargeMiB(1, 0, 768, 96)) + // Zeroed charges (struct-literal RM in other tests) cost nothing. + assert.Equal(t, int64(0), nbdkitChargeMiB(1, 2, 0, 0)) +} + +// TestRG6_FullCostCharge asserts RG-6: an instance's admission charge is its +// guest -m PLUS one nbdkit process per volume (main 768 MiB, aux 96 MiB over the +// default 1-main+2-aux layout), and an allocate/deallocate round-trip restores +// the exact baseline with no drift. +func TestRG6_FullCostCharge(t *testing.T) { + rm, err := NewResourceManager(nil, nil, nil) + require.NoError(t, err) + + require.Equal(t, defaultNbdkitMainMiB, rm.nbdkitMainMiB) + require.Equal(t, defaultNbdkitAuxMiB, rm.nbdkitAuxMiB) + + it := &ec2.InstanceTypeInfo{ + InstanceType: aws.String("t3.medium"), + VCpuInfo: &ec2.VCpuInfo{DefaultVCpus: aws.Int64(2)}, + MemoryInfo: &ec2.MemoryInfo{SizeInMiB: aws.Int64(4096)}, + } + + wantCharge := int64(4096) + nbdkitChargeMiB(defaultMainVolumes, defaultAuxVolumes, rm.nbdkitMainMiB, rm.nbdkitAuxMiB) + assert.Equal(t, wantCharge, rm.instanceMemChargeMiB(it), + "RG-6: charge must be guest -m + nbdkit per volume, not guest -m alone") + assert.Greater(t, rm.instanceMemChargeMiB(it), instanceTypeMemoryMiB(it), + "RG-6: nbdkit must add to the charge — bare guest -m undercounts and overcommits") + + // Round-trip: allocate then deallocate restores the baseline exactly. + rm.mu.Lock() + rm.hostVCPU, rm.hostMemGB = 64, 256 + rm.reservedVCPU, rm.reservedMem = 0, 0 + rm.allocatedVCPU, rm.allocatedMem = 0, 0 + rm.readMemAvailableGB = nil // isolate accounting from the live gate + rm.mu.Unlock() + + require.NoError(t, rm.allocate(it)) + assert.InDelta(t, float64(wantCharge)/1024.0, rm.allocatedMem, 1e-9, + "RG-6: allocate must charge guest + nbdkit") + + rm.deallocate(it) + assert.InDelta(t, 0.0, rm.allocatedMem, 1e-9, + "RG-6: deallocate must restore the baseline — no accounting drift") +} diff --git a/spinifex/systemd/units_invariants_test.go b/spinifex/systemd/units_invariants_test.go new file mode 100644 index 000000000..cfa55f676 --- /dev/null +++ b/spinifex/systemd/units_invariants_test.go @@ -0,0 +1,158 @@ +package systemd + +import ( + "os" + "path/filepath" + "regexp" + "runtime" + "strings" + "testing" +) + +// unitsDir locates build/systemd by walking up from this test file. +func unitsDir(t *testing.T) string { + t.Helper() + _, self, _, ok := runtime.Caller(0) + if !ok { + t.Fatal("runtime.Caller failed") + } + dir := filepath.Dir(self) + for range 8 { + cand := filepath.Join(dir, "build", "systemd") + if st, err := os.Stat(cand); err == nil && st.IsDir() { + return cand + } + dir = filepath.Dir(dir) + } + t.Fatalf("build/systemd not found above %s", self) + return "" +} + +func readUnit(t *testing.T, dir, name string) string { + t.Helper() + b, err := os.ReadFile(filepath.Join(dir, name)) + if err != nil { + t.Fatalf("read %s: %v", name, err) + } + return string(b) +} + +func unitFiles(t *testing.T, dir string) []string { + t.Helper() + ents, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + var out []string + for _, e := range ents { + if n := e.Name(); strings.HasSuffix(n, ".service") || strings.HasSuffix(n, ".slice") { + out = append(out, n) + } + } + return out +} + +// hasDirective reports whether unit carries an exact directive line (trimmed). +func hasDirective(unit, line string) bool { + for l := range strings.SplitSeq(unit, "\n") { + if strings.TrimSpace(l) == line { + return true + } + } + return false +} + +// TestRG9_TierConfinement asserts the RG-9 per-tier least-privilege contract: +// the locked-down storage/control tier drops all capabilities; the daemon and +// vpcd are the two privileged exceptions, each confined to exactly what its role +// needs and no broader. Adding a capability/device to any tier — or dropping the +// baseline from the locked-down tier — without updating this test fails CI. +func TestRG9_TierConfinement(t *testing.T) { + dir := unitsDir(t) + + // Storage/control tier: near-zero privilege. + lockedDown := []string{ + "spinifex-nats.service", + "spinifex-predastore.service", + "spinifex-viperblock.service", + "spinifex-awsgw.service", + "spinifex-ui.service", + } + for _, name := range lockedDown { + u := readUnit(t, dir, name) + for _, want := range []string{ + "CapabilityBoundingSet=", // empty — all caps dropped + "NoNewPrivileges=yes", + "ProtectSystem=strict", + "MemoryDenyWriteExecute=yes", + "SystemCallArchitectures=native", + "RestrictNamespaces=yes", + } { + if !hasDirective(u, want) { + t.Errorf("RG-9: %s (locked-down tier) must carry %q", name, want) + } + } + } + + // Daemon tier: privileged by necessity (GPU vfio), no broader. + daemon := readUnit(t, dir, "spinifex-daemon.service") + if !hasDirective(daemon, "AmbientCapabilities=CAP_SYS_ADMIN CAP_DAC_OVERRIDE") { + t.Error("RG-9: daemon must carry exactly CAP_SYS_ADMIN CAP_DAC_OVERRIDE — no broader") + } + for _, dev := range []string{ + "DeviceAllow=/dev/kvm rw", + "DeviceAllow=/dev/net/tun rw", + "DeviceAllow=char-vfio rw", + "DeviceAllow=/dev/vfio/vfio rw", + } { + if !hasDirective(daemon, dev) { + t.Errorf("RG-9: daemon must carry the explicit device allowlist entry %q", dev) + } + } + if hasDirective(daemon, "NoNewPrivileges=yes") { + t.Error("RG-9/RG-10: daemon must NOT set NoNewPrivileges=yes while it shells out to sudo (tracked RG-10 gap)") + } + for _, want := range []string{"MemoryDenyWriteExecute=yes", "SystemCallArchitectures=native"} { + if !hasDirective(daemon, want) { + t.Errorf("RG-9: daemon must keep hardening baseline %q", want) + } + } + + // Network tier (vpcd): privileged for in-process netns; a documented exception, + // not the locked-down tier. It deliberately runs NoNewPrivileges=no. + vpcd := readUnit(t, dir, "spinifex-vpcd.service") + if !hasDirective(vpcd, "NoNewPrivileges=no") { + t.Error("RG-9: vpcd (network tier) is the documented NoNewPrivileges=no exception") + } + if !hasDirective(vpcd, "SystemCallArchitectures=native") { + t.Error("RG-9: vpcd must keep SystemCallArchitectures=native") + } +} + +// TestRG11_LeanUnits asserts the RG-11 contract: unit/slice files carry settings +// plus terse # RG-n references, not paragraphs of rationale, and never reference +// a plan doc, bead, or CI run (project policy — reasoning lives in the ADR). +func TestRG11_LeanUnits(t *testing.T) { + dir := unitsDir(t) + // Plan/bead/doc/CI-run references that must not appear in a unit comment. + planRef := regexp.MustCompile(`(?i)siv-[0-9]+|mulga-[a-z0-9-]+|[a-z0-9_-]+\.md|\b[0-9]{9,}\b`) + const maxComments = 12 + + for _, name := range unitFiles(t, dir) { + u := readUnit(t, dir, name) + comments := 0 + for l := range strings.SplitSeq(u, "\n") { + ls := strings.TrimSpace(l) + if !strings.HasPrefix(ls, "#") { + continue + } + comments++ + if m := planRef.FindString(ls); m != "" { + t.Errorf("RG-11: %s comment references a plan/bead/CI artifact (%q); rationale belongs in the ADR: %s", name, m, ls) + } + } + if comments > maxComments { + t.Errorf("RG-11: %s has %d comment lines (budget %d) — strip the rationale novel, keep settings + a terse # RG-n tag", name, comments, maxComments) + } + } +} diff --git a/spinifex/vm/lifecycle.go b/spinifex/vm/lifecycle.go index a339f4bc6..65c22dfd2 100644 --- a/spinifex/vm/lifecycle.go +++ b/spinifex/vm/lifecycle.go @@ -21,6 +21,25 @@ import ( "github.com/mulgadc/viperblock/viperblock" ) +// RG-4 guest OOM tiers. Under host memory pressure the kernel reaps a user +// guest QEMU first; system instances (ELBv2 LB-VMs, EKS control-plane nodes) +// back many user VMs, so they rank above user guests but below the infra tier +// (units carry OOMScoreAdjust=-500). The host system.slice is reserved below all. +const ( + oomScoreUserGuest = 500 // ManagedBy == "" — customer instance, reaped first + oomScoreSystemInstance = 0 // ManagedBy != "" — elbv2 / eks, protected above user guests +) + +// guestOOMScore returns the oom_score_adj for a guest QEMU per the RG-4 ladder, +// keyed on whether the instance is platform-managed (ManagedBy set). Pure — +// split out for unit-testability of the tiering. +func guestOOMScore(managedBy string) int { + if managedBy == "" { + return oomScoreUserGuest + } + return oomScoreSystemInstance +} + // Run launches a VM through the manager's lifecycle pipeline: validate state, // mount volumes, exec QEMU, attach QMP, transition to Running, fire // OnInstanceUp. The instance need not be in the manager's map yet — Run @@ -386,8 +405,9 @@ func (m *Manager) startQEMU(instance *VM) error { slog.Info("VM started successfully", "pid", cmd.Process.Pid) - if err := utils.SetOOMScore(cmd.Process.Pid, 500); err != nil { - slog.Warn("Failed to set QEMU OOM score", "pid", cmd.Process.Pid, "err", err) + oomScore := guestOOMScore(instance.ManagedBy) + if err := utils.SetOOMScore(cmd.Process.Pid, oomScore); err != nil { + slog.Warn("Failed to set QEMU OOM score", "pid", cmd.Process.Pid, "score", oomScore, "err", err) } go func() { diff --git a/spinifex/vm/lifecycle_test.go b/spinifex/vm/lifecycle_test.go index 0c215ef2f..480045d4c 100644 --- a/spinifex/vm/lifecycle_test.go +++ b/spinifex/vm/lifecycle_test.go @@ -849,3 +849,19 @@ func TestStartupTimeouts(t *testing.T) { assert.Equal(t, 5*time.Second, qemuStartupTimeout) assert.Equal(t, 5*time.Second, nbdReadyTimeout) } + +// TestRG4_GuestOOMTier asserts the RG-4 OOM ladder: a customer guest QEMU +// (ManagedBy == "") is scored +500 so the kernel reaps it first under host +// memory pressure, while a platform-managed system instance (ManagedBy set — +// ELBv2 LB-VM, EKS control-plane node) is scored 0, ranking above user guests +// but below the infra tier. Regressing either tier widens the OOM blast radius. +func TestRG4_GuestOOMTier(t *testing.T) { + assert.Equal(t, 500, guestOOMScore(""), + "RG-4: user guest (no ManagedBy) must be reaped first (+500)") + assert.Equal(t, 0, guestOOMScore("elbv2"), + "RG-4: ELBv2 system instance must rank above user guests (0)") + assert.Equal(t, 0, guestOOMScore("eks"), + "RG-4: EKS control-plane node must rank above user guests (0)") + assert.Greater(t, guestOOMScore(""), guestOOMScore("elbv2"), + "RG-4: a user guest must always be killed before a system instance") +}