GL-5: DR bring-up structural bind overrides + 4d real-bind swap

ModeDRGuestLoss now passes restore-time MountOverrides for the two platform-constant structural binds (mp8 parent, mp9 bootstrap) via the shared throwaway-volume format helper - without them a customer-archive restore under the privsep token fails outright ("restoring 'mp8' to bind mount is only possible for root"). New post-restore step 4d swaps the real binds in via the host runner (root pct set, one slot per call), deletes the displaced unusedN volumes (API config PUT; a scoped-token refusal logs the residue loudly instead of widening privileges), and respects the committed/launched rollback envelope. Provision passes nil overrides - byte-identical behavior (regression contract test). Engine grows an optional HostRunner + StateDir seam (DR refuses up front without a runner); selftest bring-up wires the ExecRunner + cleans the scratch mp9 host dir on teardown; proxmox.GuestConfig.Unused() added. 6 new tests incl. C2 mid-swap rollback + C3 older-archive + 403-warn paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6
2026-07-08 08:48:11 +02:00
parent 4c40846769
commit c12b512316
6 changed files with 441 additions and 16 deletions
@@ -2,14 +2,60 @@ package reconcile

 import (
 	"context"
+	"encoding/json"
 	"errors"
+	"io"
+	"os"
+	"path/filepath"
 	"strings"
+	"sync"
 	"testing"
 	"time"

 	"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
 )

+// fakeRunner records the host-root commands the DR structural-bind swap issues (GL-5). failOn — a
+// substring of the joined command — makes that one command fail (the C2 mid-swap failure driver).
+type fakeRunner struct {
+	mu     sync.Mutex
+	cmds   []string
+	failOn string
+}
+
+func (f *fakeRunner) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) {
+	cmd := name + " " + strings.Join(args, " ")
+	f.mu.Lock()
+	f.cmds = append(f.cmds, cmd)
+	f.mu.Unlock()
+	if f.failOn != "" && strings.Contains(cmd, f.failOn) {
+		return nil, []byte("boom"), errors.New("exit status 1")
+	}
+	return nil, nil, nil
+}
+
+func (f *fakeRunner) RunStdin(ctx context.Context, _ io.Reader, name string, args ...string) ([]byte, []byte, error) {
+	return f.Run(ctx, name, args...)
+}
+
+// newDREngine builds an engine wired for ModeDRGuestLoss (GL-5): a recording host runner + an
+// isolated state dir (the 4d swap MkdirAlls the mp9 host dir under it).
+func newDREngine(t *testing.T, api GuestAPI) (*Engine, *fakeRunner, string, *Queue) {
+	t.Helper()
+	jp := filepath.Join(t.TempDir(), "journal.log")
+	j, err := OpenJournal(jp)
+	if err != nil {
+		t.Fatalf("OpenJournal: %v", err)
+	}
+	t.Cleanup(func() { j.Close() })
+	q := NewQueue()
+	t.Cleanup(q.Close)
+	fr := &fakeRunner{}
+	sd := t.TempDir()
+	e := NewEngine(EngineOptions{API: api, Queue: q, Journal: j, Provider: EmptyProvider{}, HostRunner: fr, StateDir: sd})
+	return e, fr, sd, q
+}
+
 // setParamsFor returns the params of the (last) SetConfig call against vmid, or nil.
 func setParamsFor(api *fakeAPI, vmid int) map[string]string {
 	var out map[string]string
@@ -355,7 +401,7 @@ func TestRunBringUp_NoLaunchNoDestroy(t *testing.T) {
 func TestRunBringUp_DRPreservesContinuityIdentity(t *testing.T) {
 	const vmid = 8001
 	api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{vmid: scratchCfg()}}
-	e, _, q := newEngine(t, api, EmptyProvider{})
+	e, _, _, q := newDREngine(t, api)
 	defer q.Close()

 	res := e.RunBringUp(context.Background(), BringUpSpec{
@@ -393,7 +439,7 @@ func TestRunBringUp_DRPreservesContinuityIdentity(t *testing.T) {
 func TestRunBringUp_DRResetMACWhenSourceMayBeLive(t *testing.T) {
 	const vmid = 8002
 	api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{vmid: scratchCfg()}}
-	e, _, q := newEngine(t, api, EmptyProvider{})
+	e, _, _, q := newDREngine(t, api)
 	defer q.Close()

 	res := e.RunBringUp(context.Background(), BringUpSpec{
@@ -649,7 +695,7 @@ func poolAddsFor(api *fakeAPI, vmid int) []poolAddCall {
 func TestRunBringUp_ReassertsPoolMembership(t *testing.T) {
 	const vmid = 8100
 	api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{vmid: scratchCfg()}}
-	e, _, q := newEngine(t, api, EmptyProvider{})
+	e, _, _, q := newDREngine(t, api)
 	defer q.Close()

 	res := e.RunBringUp(context.Background(), BringUpSpec{
@@ -692,7 +738,7 @@ func TestRunBringUp_PoolAddFailure_WarnsButPasses(t *testing.T) {
 		cfg:        map[int]proxmox.GuestConfig{vmid: scratchCfg()},
 		poolAddErr: errors.New("proxmox: PUT /pools/felhom -> HTTP 500: transient"),
 	}
-	e, _, q := newEngine(t, api, EmptyProvider{})
+	e, _, _, q := newDREngine(t, api)
 	defer q.Close()

 	res := e.RunBringUp(context.Background(), BringUpSpec{
@@ -717,3 +763,211 @@ func TestRunBringUp_PoolAddFailure_WarnsButPasses(t *testing.T) {
 		t.Fatalf("pool-add failure must surface as a warning: %+v", res.StartWarnings)
 	}
 }
+
+// ── GL-5: DR structural bind overrides + 4d swap ─────────────────────────────────────────────────
+
+// GL-5 Scenario A: a DR bring-up passes restore-time MountOverrides for EXACTLY the two structural
+// binds (throwaway volumes in bindMountOverrides format), then step 4d swaps the REAL binds in via
+// the root runner (mp9 host dir created first) and deletes the displaced unusedN volumes.
+// COMPANION RED-PROOF: reverting the Part-1 override synthesis fails the MountOverrides asserts
+// (run→fail→revert, recorded in the REPORT).
+func TestRunBringUp_DRStructuralBindOverridesAndSwap(t *testing.T) {
+	const vmid = 8200
+	cfg := scratchCfg()
+	// after the swap PVE parks the two displaced throwaway volumes as unusedN
+	cfg.Extra["unused0"] = json.RawMessage(`"local-lvm:vm-8200-disk-2"`)
+	cfg.Extra["unused1"] = json.RawMessage(`"local-lvm:vm-8200-disk-3"`)
+	api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{vmid: cfg}}
+	e, fr, sd, q := newDREngine(t, api)
+	defer q.Close()
+
+	res := e.RunBringUp(context.Background(), BringUpSpec{
+		Mode: ModeDRGuestLoss, Archive: "local:backup/customer.tar.zst", VMID: vmid,
+		RestoreStorage: "local-lvm", KeepMAC: true,
+	})
+	if res.Err != nil || !res.Pass {
+		t.Fatalf("dr bring-up must pass, got %+v", res)
+	}
+	if len(api.restores) != 1 {
+		t.Fatalf("expected one restore, got %+v", api.restores)
+	}
+	ov := api.restores[0].MountOverrides
+	want := map[string]string{
+		"mp8": "local-lvm:1,mp=/mnt/felhom-drives,backup=0",
+		"mp9": "local-lvm:1,mp=/etc/felhom-bootstrap,backup=0",
+	}
+	if len(ov) != 2 || ov["mp8"] != want["mp8"] || ov["mp9"] != want["mp9"] {
+		t.Fatalf("MountOverrides = %+v, want exactly %+v", ov, want)
+	}
+	// 4d: mp9 host dir created under the engine state dir …
+	bootDir := structuralBootHostDir(sd, vmid)
+	if st, err := os.Stat(bootDir); err != nil || !st.IsDir() {
+		t.Fatalf("mp9 bootstrap host dir not created: %v", err)
+	}
+	// … and the REAL binds set via the root runner, one slot per call, exact backhalf values.
+	wantCmds := []string{
+		"mkdir -p /mnt/felhom-drives",
+		"pct set 8200 -mp8 /mnt/felhom-drives,mp=/mnt/felhom-drives",
+		"pct set 8200 -mp9 " + bootDir + ",mp=/etc/felhom-bootstrap,ro=1",
+	}
+	if len(fr.cmds) != len(wantCmds) {
+		t.Fatalf("runner cmds = %v, want %v", fr.cmds, wantCmds)
+	}
+	for i := range wantCmds {
+		if fr.cmds[i] != wantCmds[i] {
+			t.Fatalf("runner cmd[%d] = %q, want %q", i, fr.cmds[i], wantCmds[i])
+		}
+	}
+	// displaced throwaways deleted in ONE config PUT (deterministic order) — no unusedN residue.
+	del := ""
+	for _, s := range api.sets {
+		if s.vmid == vmid && s.params["delete"] != "" {
+			del = s.params["delete"]
+		}
+	}
+	if del != "unused0,unused1" {
+		t.Fatalf("displaced volumes not deleted: delete=%q sets=%+v", del, api.sets)
+	}
+}
+
+// GL-5 Scenario B (the regression contract): provision passes NO MountOverrides (nil) and never
+// touches the host runner — the golden path is behavior-identical. COMPANION RED-PROOF: making the
+// override synthesis unconditional fails this (run→fail→revert, recorded in the REPORT).
+func TestRunBringUp_ProvisionNoMountOverrides(t *testing.T) {
+	const vmid = 8201
+	api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{vmid: scratchCfg()}}
+	e, fr, _, q := newDREngine(t, api) // runner present but must stay UNUSED
+	defer q.Close()
+
+	res := e.RunBringUp(context.Background(), BringUpSpec{
+		Mode: ModeProvision, Archive: "local:backup/golden.tar.zst", VMID: vmid,
+		RestoreStorage: "local-lvm", Hostname: "h",
+	})
+	if res.Err != nil || !res.Pass {
+		t.Fatalf("provision must pass, got %+v", res)
+	}
+	if api.restores[0].MountOverrides != nil {
+		t.Fatalf("provision must pass NO MountOverrides, got %+v", api.restores[0].MountOverrides)
+	}
+	if len(fr.cmds) != 0 {
+		t.Fatalf("provision must not touch the host runner, got %v", fr.cmds)
+	}
+}
+
+// GL-5 C2: a mid-swap failure (here: the mp9 pct set) fails the bring-up with the exact mpN state
+// named and compensating-rolls-back the guest — never a silent half-wired success.
+func TestRunBringUp_DRSwapFailureRollsBack(t *testing.T) {
+	const vmid = 8202
+	api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{vmid: scratchCfg()}}
+	e, fr, _, q := newDREngine(t, api)
+	defer q.Close()
+	fr.failOn = "-mp9"
+
+	res := e.RunBringUp(context.Background(), BringUpSpec{
+		Mode: ModeDRGuestLoss, Archive: "local:backup/customer.tar.zst", VMID: vmid,
+		RestoreStorage: "local-lvm", KeepMAC: true,
+	})
+	if res.Err == nil || res.Pass {
+		t.Fatalf("mid-swap failure must fail the bring-up, got %+v", res)
+	}
+	if !strings.Contains(res.Err.Error(), "structural-bind swap") ||
+		!strings.Contains(res.Err.Error(), "mp9") || !strings.Contains(res.Err.Error(), "mp8 already landed") {
+		t.Fatalf("error must name the swap + the exact mpN state, got: %v", res.Err)
+	}
+	if len(api.destroys) != 1 || api.destroys[0] != vmid {
+		t.Fatalf("mid-swap failure must compensating-roll-back (destroy %d), got %+v", vmid, api.destroys)
+	}
+	if len(api.starts) != 0 {
+		t.Fatalf("a half-wired guest must never be started, got %+v", api.starts)
+	}
+}
+
+// GL-5 C3: an OLDER archive without mp9 — the overrides are platform constants, not archive-derived,
+// so the restore still names BOTH mpN (PVE simply creates the missing one) and 4d normalizes; the
+// end state is identical, with no unusedN residue (here only ONE displaced volume shows up).
+func TestRunBringUp_DRArchiveWithoutMp9(t *testing.T) {
+	const vmid = 8203
+	cfg := scratchCfg() // no mp9 in Extra — the pre-override archive shape
+	cfg.Extra["unused0"] = json.RawMessage(`"local-lvm:vm-8203-disk-2"`)
+	api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{vmid: cfg}}
+	e, fr, _, q := newDREngine(t, api)
+	defer q.Close()
+
+	res := e.RunBringUp(context.Background(), BringUpSpec{
+		Mode: ModeDRGuestLoss, Archive: "local:backup/old-customer.tar.zst", VMID: vmid,
+		RestoreStorage: "local-lvm", KeepMAC: true,
+	})
+	if res.Err != nil || !res.Pass {
+		t.Fatalf("dr bring-up of an mp9-less archive must pass, got %+v", res)
+	}
+	ov := api.restores[0].MountOverrides
+	if len(ov) != 2 || ov["mp8"] == "" || ov["mp9"] == "" {
+		t.Fatalf("overrides are constants — both mpN must be named regardless of the archive: %+v", ov)
+	}
+	if len(fr.cmds) != 3 {
+		t.Fatalf("4d must run identically (mkdir + 2 pct sets), got %v", fr.cmds)
+	}
+	del := ""
+	for _, s := range api.sets {
+		if s.vmid == vmid && s.params["delete"] != "" {
+			del = s.params["delete"]
+		}
+	}
+	if del != "unused0" {
+		t.Fatalf("the one displaced volume must be deleted: delete=%q", del)
+	}
+}
+
+// GL-5: DR on an API-only engine (no host runner) refuses UP FRONT — before any restore — because
+// the structural-bind swap is a root pct op the API token cannot perform.
+func TestRunBringUp_DRWithoutRunnerRefuses(t *testing.T) {
+	const vmid = 8204
+	api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{vmid: scratchCfg()}}
+	e, _, q := newEngine(t, api, EmptyProvider{}) // no HostRunner
+	defer q.Close()
+
+	res := e.RunBringUp(context.Background(), BringUpSpec{
+		Mode: ModeDRGuestLoss, Archive: "local:backup/customer.tar.zst", VMID: vmid,
+		RestoreStorage: "local-lvm",
+	})
+	if res.Err == nil || !strings.Contains(res.Err.Error(), "host runner") {
+		t.Fatalf("dr without a runner must refuse naming the gap, got %+v", res)
+	}
+	if len(api.restores) != 0 {
+		t.Fatalf("the refusal must fire BEFORE any restore, got %+v", api.restores)
+	}
+}
+
+// GL-5: a refused unusedN delete (the scoped-token-403 class) must NOT fail the correctly-wired
+// guest — the residue is surfaced as a LOUD result warning instead (privileges never widen silently).
+func TestRunBringUp_DRUnusedDeleteFailureWarns(t *testing.T) {
+	const vmid = 8205
+	cfg := scratchCfg()
+	cfg.Extra["unused0"] = json.RawMessage(`"local-lvm:vm-8205-disk-2"`)
+	api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{vmid: cfg}}
+	api.setFunc = func(_ int, params map[string]string) (string, error) {
+		if params["delete"] != "" {
+			return "", errors.New("proxmox: PUT config -> HTTP 403: Permission check failed")
+		}
+		return "", nil
+	}
+	e, _, _, q := newDREngine(t, api)
+	defer q.Close()
+
+	res := e.RunBringUp(context.Background(), BringUpSpec{
+		Mode: ModeDRGuestLoss, Archive: "local:backup/customer.tar.zst", VMID: vmid,
+		RestoreStorage: "local-lvm", KeepMAC: true,
+	})
+	if res.Err != nil || !res.Pass {
+		t.Fatalf("a refused residue delete must NOT fail a wired guest, got %+v", res)
+	}
+	found := false
+	for _, w := range res.StartWarnings {
+		if strings.Contains(w, "displaced volumes not deleted") {
+			found = true
+		}
+	}
+	if !found {
+		t.Fatalf("residue must surface as a warning: %+v", res.StartWarnings)
+	}
+}