agent v0.33.0: C1 net — pre-start self-heal hook + decommission mp-delete
Pre-start PVE hookscript (internal/guesthook) creates host-root placeholders for absent bind-mount sources so the guest always boots (fail-closed); decommission now pct set --delete's the dead mp (GuestBinder.DetachBind) so a missing source can't brick the next reboot (B3 C1 bug). Non-hollow tests + companions. Installed + registered per-guest by the provision back-half. Transitional ahead of the intermediary-mount re-architecture which makes C1 structural. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,135 @@
|
||||
// Package guesthook is the LXC guest pre-start self-heal (C1 net, transitional).
|
||||
//
|
||||
// THE BUG IT FIXES (C1, B3 audit): in the per-drive bind model an external data drive is bound into the
|
||||
// guest as `pct set <vmid> -mpN <drive>/felhom-data,mp=/mnt/<name>`. When that drive is ABSENT at guest
|
||||
// boot, the bind SOURCE `<drive>/felhom-data` does not exist, `pct start` fails the mount, and the guest
|
||||
// BRICKS (pre-start exit 255 — ALL apps down). Today nothing recovers it.
|
||||
//
|
||||
// THE FIX: a PVE `pre-start` hookscript runs this code; for every BIND mountpoint whose source path is
|
||||
// missing it CREATES an empty, host-root-owned placeholder directory so the mount succeeds and the guest
|
||||
// boots. It is fail-closed: the placeholder is owned by host root (uid 0), which is UNMAPPED in the
|
||||
// unprivileged-LXC user namespace, so the in-guest controller/apps (even as guest-root) cannot write to
|
||||
// it — and a returning drive simply shadows it (the agent mounts over it).
|
||||
//
|
||||
// WHY CREATE, NOT DELETE: removing the dead mp would need `pct set <vmid> --delete mpN`, which takes the
|
||||
// per-guest config lock the start task ALREADY holds → it dead-times-out (~10s) and the guest still
|
||||
// bricks. So in pre-start we NEUTRALISE (placeholder) rather than mutate config. Proper mp removal runs
|
||||
// OUTSIDE the start lock — at decommission (handleDiskDecommission → DetachBind) and the startup
|
||||
// reconcile. The intermediary-mount re-architecture later makes C1 STRUCTURAL (the only bind source is
|
||||
// the permanent, always-present /mnt/felhom-drives parent), after which this hook is pure defense-in-depth.
|
||||
package guesthook
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"sort"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// PhasePreStart is the PVE hook phase at which we self-heal (before the container mounts are set up).
|
||||
const PhasePreStart = "pre-start"
|
||||
|
||||
// placeholderMode is the mode for a created bind-source placeholder. Host-root-owned + this mode =
|
||||
// fail-closed against the unprivileged guest (host uid 0 is unmapped in the guest userns).
|
||||
const placeholderMode = 0o755
|
||||
|
||||
// ParseConfMounts parses an LXC config file body (/etc/pve/lxc/<vmid>.conf) and returns each mount key
|
||||
// (`mp0`..`mp255`, plus `rootfs`) mapped to its SOURCE — the first comma-field of the value, before any
|
||||
// `mp=`/`size=`/`backup=` options. A BIND mount has an absolute-path source (`/mnt/...`); a storage
|
||||
// volume has a `<storage>:<volid>` source (no leading slash). Lines that aren't a mountpoint/rootfs key
|
||||
// are ignored.
|
||||
func ParseConfMounts(conf string) map[string]string {
|
||||
out := map[string]string{}
|
||||
for _, line := range strings.Split(conf, "\n") {
|
||||
line = strings.TrimSpace(line)
|
||||
colon := strings.IndexByte(line, ':')
|
||||
if colon <= 0 {
|
||||
continue
|
||||
}
|
||||
key := line[:colon]
|
||||
if key != "rootfs" && !(strings.HasPrefix(key, "mp") && isAllDigits(strings.TrimPrefix(key, "mp"))) {
|
||||
continue
|
||||
}
|
||||
val := strings.TrimSpace(line[colon+1:])
|
||||
if val == "" {
|
||||
continue
|
||||
}
|
||||
src := val
|
||||
if c := strings.IndexByte(val, ','); c >= 0 {
|
||||
src = val[:c]
|
||||
}
|
||||
out[key] = strings.TrimSpace(src)
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// isBindSource reports whether an mp source is a host-path BIND (an absolute path) rather than a PVE
|
||||
// storage volume (`<storage>:<volid>`, never absolute). On the Linux host a bind source is `/mnt/...`
|
||||
// (leading slash); the filepath.IsAbs arm additionally recognises an OS-absolute path so the real-IO
|
||||
// tests pass under a Windows temp dir too — on Linux both arms agree and a storage volid matches neither.
|
||||
func isBindSource(src string) bool {
|
||||
return strings.HasPrefix(src, "/") || filepath.IsAbs(src)
|
||||
}
|
||||
|
||||
func isAllDigits(s string) bool {
|
||||
if s == "" {
|
||||
return false
|
||||
}
|
||||
for _, c := range s {
|
||||
if c < '0' || c > '9' {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
// MissingBindSources returns the BIND-mount source paths (absolute host paths) that do NOT exist, sorted
|
||||
// and de-duplicated. Storage-volume sources (`<storage>:<volid>`, no leading '/') are NEVER returned —
|
||||
// only a real host-path bind can have a vanished source we must heal; a storage volume that's missing is
|
||||
// PVE's own concern, not ours to mkdir. `exists` reports whether a path is present (injected for tests).
|
||||
func MissingBindSources(mounts map[string]string, exists func(string) bool) []string {
|
||||
seen := map[string]bool{}
|
||||
var miss []string
|
||||
for _, src := range mounts {
|
||||
if !isBindSource(src) { // storage volume (<storage>:<volid>), not a host-path bind — never touch
|
||||
continue
|
||||
}
|
||||
if seen[src] || exists(src) {
|
||||
continue
|
||||
}
|
||||
seen[src] = true
|
||||
miss = append(miss, src)
|
||||
}
|
||||
sort.Strings(miss)
|
||||
return miss
|
||||
}
|
||||
|
||||
// Heal reads the LXC config at confPath and creates a placeholder directory for every bind-mount source
|
||||
// that is missing, returning the list of paths it created. It never returns a fatal error for an
|
||||
// unreadable/empty config (a guest with no config simply has nothing to heal) — the hook must NEVER block
|
||||
// a start. A mkdir failure on one path is collected into err but the others still proceed.
|
||||
func Heal(confPath string) (created []string, err error) {
|
||||
data, readErr := os.ReadFile(confPath)
|
||||
if readErr != nil {
|
||||
// No config = nothing to heal. Never block the start over a read error.
|
||||
return nil, nil
|
||||
}
|
||||
mounts := ParseConfMounts(string(data))
|
||||
miss := MissingBindSources(mounts, func(p string) bool {
|
||||
_, statErr := os.Stat(p)
|
||||
return statErr == nil
|
||||
})
|
||||
var errs []string
|
||||
for _, p := range miss {
|
||||
if mkErr := os.MkdirAll(p, placeholderMode); mkErr != nil {
|
||||
errs = append(errs, fmt.Sprintf("%s: %v", p, mkErr))
|
||||
continue
|
||||
}
|
||||
created = append(created, p)
|
||||
}
|
||||
if len(errs) > 0 {
|
||||
return created, fmt.Errorf("guesthook: placeholder creation failed for: %s", strings.Join(errs, "; "))
|
||||
}
|
||||
return created, nil
|
||||
}
|
||||
@@ -0,0 +1,118 @@
|
||||
package guesthook
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"reflect"
|
||||
"testing"
|
||||
)
|
||||
|
||||
const sampleConf = `arch: amd64
|
||||
cores: 2
|
||||
hostname: demo-felhom
|
||||
memory: 12288
|
||||
mp0: local-lvm:vm-9201-disk-1,mp=/var/lib/docker,backup=1,size=256G
|
||||
mp1: /mnt/felhom-usb/felhom-data,mp=/mnt/felhom-usb
|
||||
mp2: /mnt/felhom-flash/felhom-data,mp=/mnt/felhom-flash
|
||||
mp9: /var/lib/felhom-agent/guests/9201/bootstrap,mp=/etc/felhom-bootstrap,ro=1
|
||||
net0: name=eth0,bridge=vmbr0
|
||||
rootfs: local-lvm:vm-9201-disk-0,size=32G
|
||||
swap: 4096
|
||||
unprivileged: 1
|
||||
`
|
||||
|
||||
func TestParseConfMounts(t *testing.T) {
|
||||
got := ParseConfMounts(sampleConf)
|
||||
want := map[string]string{
|
||||
"mp0": "local-lvm:vm-9201-disk-1",
|
||||
"mp1": "/mnt/felhom-usb/felhom-data",
|
||||
"mp2": "/mnt/felhom-flash/felhom-data",
|
||||
"mp9": "/var/lib/felhom-agent/guests/9201/bootstrap",
|
||||
"rootfs": "local-lvm:vm-9201-disk-0",
|
||||
}
|
||||
if !reflect.DeepEqual(got, want) {
|
||||
t.Fatalf("ParseConfMounts mismatch:\n got=%v\nwant=%v", got, want)
|
||||
}
|
||||
// net0/arch/etc. (non-mount keys) must NOT leak in.
|
||||
if _, bad := got["net0"]; bad {
|
||||
t.Fatalf("net0 was parsed as a mount source")
|
||||
}
|
||||
}
|
||||
|
||||
// TestMissingBindSources is the load-bearing selector test. The flash drive is absent (its felhom-data
|
||||
// source missing); usb is present; mp0/rootfs are STORAGE volumes (must never be selected); mp9's
|
||||
// bootstrap source is present. Only the flash source may be returned.
|
||||
//
|
||||
// COMPANION GUARD — this test FAILS on the two trivial impls the spec warns about:
|
||||
// - "return nothing" (the pre-fix no-op hook) → flash not selected → guest still bricks → FAIL.
|
||||
// - "return every source" (mkdir everything) → would include the present usb bind AND the
|
||||
// local-lvm storage volumes (creating bogus dirs that shadow real data) → FAIL.
|
||||
func TestMissingBindSources(t *testing.T) {
|
||||
mounts := ParseConfMounts(sampleConf)
|
||||
present := map[string]bool{
|
||||
"/mnt/felhom-usb/felhom-data": true, // usb attached
|
||||
"/var/lib/felhom-agent/guests/9201/bootstrap": true, // bootstrap always present
|
||||
// "/mnt/felhom-flash/felhom-data" is ABSENT (drive unplugged)
|
||||
}
|
||||
got := MissingBindSources(mounts, func(p string) bool { return present[p] })
|
||||
want := []string{"/mnt/felhom-flash/felhom-data"}
|
||||
if !reflect.DeepEqual(got, want) {
|
||||
t.Fatalf("MissingBindSources mismatch:\n got=%v\nwant=%v", got, want)
|
||||
}
|
||||
|
||||
// Explicit companion assertions (pin both failure directions independently of want):
|
||||
for _, p := range got {
|
||||
if p == "/mnt/felhom-usb/felhom-data" {
|
||||
t.Fatalf("selected a PRESENT bind source — over-eager (would shadow live data)")
|
||||
}
|
||||
if p == "local-lvm:vm-9201-disk-1" || p == "local-lvm:vm-9201-disk-0" {
|
||||
t.Fatalf("selected a STORAGE VOLUME source — must only heal host-path binds")
|
||||
}
|
||||
}
|
||||
if len(got) == 0 {
|
||||
t.Fatalf("selected nothing — the absent flash bind would brick the guest (pre-fix no-op)")
|
||||
}
|
||||
}
|
||||
|
||||
// TestHealCreatesOnlyMissingBind drives the real filesystem path in a temp dir: a present bind source is
|
||||
// left untouched, an absent one is created (so the guest boots), a storage volume is never created.
|
||||
func TestHealCreatesOnlyMissingBind(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
presentSrc := filepath.Join(root, "usb", "felhom-data")
|
||||
absentSrc := filepath.Join(root, "flash", "felhom-data")
|
||||
if err := os.MkdirAll(presentSrc, 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
conf := "" +
|
||||
"mp0: local-lvm:vm-9-disk-0,mp=/var/lib/docker,backup=1\n" +
|
||||
"mp1: " + presentSrc + ",mp=/mnt/usb\n" +
|
||||
"mp2: " + absentSrc + ",mp=/mnt/flash\n" +
|
||||
"rootfs: local-lvm:vm-9-disk-1,size=32G\n"
|
||||
confPath := filepath.Join(root, "9.conf")
|
||||
if err := os.WriteFile(confPath, []byte(conf), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
created, err := Heal(confPath)
|
||||
if err != nil {
|
||||
t.Fatalf("Heal: %v", err)
|
||||
}
|
||||
if !reflect.DeepEqual(created, []string{absentSrc}) {
|
||||
t.Fatalf("Heal created %v, want [%s]", created, absentSrc)
|
||||
}
|
||||
if _, err := os.Stat(absentSrc); err != nil {
|
||||
t.Fatalf("absent bind source not created — guest would still brick: %v", err)
|
||||
}
|
||||
// A storage-volume mp must never produce a bogus host directory.
|
||||
if _, err := os.Stat(filepath.Join(root, "local-lvm:vm-9-disk-0")); err == nil {
|
||||
t.Fatalf("a storage volume source was materialised as a directory")
|
||||
}
|
||||
}
|
||||
|
||||
// TestHealMissingConfNeverErrors — a hook must never block a start, even for an unreadable config.
|
||||
func TestHealMissingConfNeverErrors(t *testing.T) {
|
||||
created, err := Heal(filepath.Join(t.TempDir(), "does-not-exist.conf"))
|
||||
if err != nil || created != nil {
|
||||
t.Fatalf("Heal on missing conf: created=%v err=%v (want nil,nil)", created, err)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,59 @@
|
||||
package guesthook
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
|
||||
"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
|
||||
)
|
||||
|
||||
// Install/registration of the pre-start hook. The wrapper lives in a PVE `snippets`-enabled storage dir
|
||||
// (the `local` storage maps to /var/lib/vz/snippets) and is referenced per-guest by its volid.
|
||||
const (
|
||||
// SnippetDir is the local-storage snippets directory PVE serves hookscripts from.
|
||||
SnippetDir = "/var/lib/vz/snippets"
|
||||
// SnippetName is the wrapper filename.
|
||||
SnippetName = "felhom-guest-hook.sh"
|
||||
// HookVolID is the volid form `pct set --hookscript` expects.
|
||||
HookVolID = "local:snippets/" + SnippetName
|
||||
// AgentBin is the installed agent binary the wrapper delegates to.
|
||||
AgentBin = "/usr/local/bin/felhom-agent"
|
||||
)
|
||||
|
||||
// SnippetPath is the absolute path of the installed wrapper.
|
||||
var SnippetPath = filepath.Join(SnippetDir, SnippetName)
|
||||
|
||||
// snippetBody is the tiny wrapper PVE execs as `<script> <vmid> <phase>`. It delegates to the agent
|
||||
// binary so the heal LOGIC is the unit-tested Go, never duplicated (divergence-proof) shell. Executable.
|
||||
const snippetBody = `#!/bin/sh
|
||||
# felhom-agent guest pre-start self-heal hook (C1 net). PVE calls: <script> <vmid> <phase>.
|
||||
exec ` + AgentBin + ` guest-hook "$1" "$2"
|
||||
`
|
||||
|
||||
// InstallSnippet writes the pre-start hook wrapper into the PVE snippets dir (idempotent, root-owned,
|
||||
// executable). The agent runs as a non-root service user, so it writes an agent-writable temp file then
|
||||
// `install`s it host-root (same pattern as the bootstrap mount + dnsmasq drop-ins). Safe to call repeatedly.
|
||||
func InstallSnippet(ctx context.Context, runner proxmox.Runner) error {
|
||||
tmp := filepath.Join(os.TempDir(), "felhom-guest-hook.sh")
|
||||
if err := os.WriteFile(tmp, []byte(snippetBody), 0o755); err != nil {
|
||||
return fmt.Errorf("guesthook: write temp snippet: %w", err)
|
||||
}
|
||||
defer os.Remove(tmp)
|
||||
if _, stderr, err := runner.Run(ctx, "install", "-m", "0755", "--", tmp, SnippetPath); err != nil {
|
||||
return fmt.Errorf("guesthook: install snippet to %s: %w: %s", SnippetPath, err, string(stderr))
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// Register points a guest at the pre-start hook (`pct set <vmid> --hookscript <volid>`). Idempotent —
|
||||
// re-setting the same hookscript is a no-op. Safe on a running guest (a config edit, not a start, so no
|
||||
// start-lock contention).
|
||||
func Register(ctx context.Context, runner proxmox.Runner, vmid int) error {
|
||||
if _, stderr, err := runner.Run(ctx, "pct", "set", strconv.Itoa(vmid), "--hookscript", HookVolID); err != nil {
|
||||
return fmt.Errorf("guesthook: register hookscript on %d: %w: %s", vmid, err, string(stderr))
|
||||
}
|
||||
return nil
|
||||
}
|
||||
Reference in New Issue
Block a user