DR bring-up refuses beside a live original (R-834): source guest present, drives bind, or unreadable config

The DR route keeps onboot 1, binds the real drives and starts the guest: right on a replaced host, a second box on the same drives beside a live original. The restore-test's no-host-bind half is now pinned too (measured safe live on demo-hp 2026-10-04). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS
2026-10-04 08:52:35 +02:00
parent d766666ff8
commit 0e03d36d0c
2 changed files with 195 additions and 0 deletions
@@ -302,6 +302,19 @@ func (e *Engine) runBringUp(ctx context.Context, spec BringUpSpec, res *BringUpR
 		}
 	}

+	// R-834: DR keeps the archive's `onboot: 1`, binds the host's REAL drives (4d) and STARTS the guest
+	// — right on a replaced host, where the original is gone. Beside a LIVE original it would be a
+	// second controller for the same household on the same drives. So DR refuses when this host
+	// still carries the original (the archive's source VMID) or any guest that binds the drives.
+	// A copy beside the original is the restore-test's job (onboot=0, throwaway stand-ins, torn
+	// down) or the runbook's beside-restore. Pinned by TestRunBringUp_DRRefusesBesideALiveOriginal.
+	if spec.Mode == ModeDRGuestLoss {
+		if why := e.liveOriginalBeside(ctx, lxc, spec.Archive); why != "" {
+			res.Err = fmt.Errorf("reconcile: dr bring-up refused: %s — a DR restore beside a live original would run two boxes on the same drives (R-834)", why)
+			return
+		}
+	}
+
 	base := JournalEntry{OpID: e.bringUpOpID(spec.VMID), VMID: spec.VMID, Kind: bringUpKind, Rollback: true}

 	// OWN the rollback BEFORE any mutation. From here a crash leaves an in-flight Rollback
@@ -734,3 +747,47 @@ func net0MAC(cfg proxmox.GuestConfig) string {
 	}
 	return ""
 }
+
+// archiveSourceVMID reads the source guest's VMID from a backup volid: a vzdump file
+// (`…/vzdump-lxc-<vmid>-<date>.tar.zst`) or a PBS snapshot (`…:backup/ct/<vmid>/<time>`). 0 = unknown.
+func archiveSourceVMID(archive string) int {
+	if i := strings.Index(archive, "vzdump-lxc-"); i >= 0 {
+		rest := archive[i+len("vzdump-lxc-"):]
+		if j := strings.Index(rest, "-"); j > 0 {
+			if n, err := strconv.Atoi(rest[:j]); err == nil {
+				return n
+			}
+		}
+	}
+	if i := strings.Index(archive, "ct/"); i >= 0 {
+		rest := archive[i+len("ct/"):]
+		if j := strings.Index(rest, "/"); j > 0 {
+			if n, err := strconv.Atoi(rest[:j]); err == nil {
+				return n
+			}
+		}
+	}
+	return 0
+}
+
+// liveOriginalBeside says why a DR bring-up would land beside a live original ("" = it would not):
+// the archive's source guest still exists here, or a guest binds the drives parent. Fails CLOSED: a
+// guest whose config cannot be read cannot be ruled out.
+func (e *Engine) liveOriginalBeside(ctx context.Context, lxc []proxmox.Guest, archive string) string {
+	src := archiveSourceVMID(archive)
+	for _, g := range lxc {
+		if src > 0 && g.VMID == src {
+			return fmt.Sprintf("the archive's source guest %d still exists on this host (status %s)", g.VMID, g.Status)
+		}
+		cfg, err := e.api.GuestConfig(ctx, g.VMID)
+		if err != nil {
+			return fmt.Sprintf("guest %d's config could not be read to rule out a live original: %v", g.VMID, err)
+		}
+		for slot, v := range cfg.MountPoints() {
+			if source, _, _ := strings.Cut(v, ","); source == structuralParentDir {
+				return fmt.Sprintf("guest %d binds the household drives (%s %s)", g.VMID, slot, structuralParentDir)
+			}
+		}
+	}
+	return ""
+}
@@ -0,0 +1,138 @@
+package reconcile
+
+import (
+	"context"
+	"encoding/json"
+	"strings"
+	"testing"
+
+	"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
+)
+
+// R-834: a whole-guest restore BESIDE a live original must never come up as a second box on the same
+// drives. The DR route keeps `onboot: 1`, binds the real drives and starts the guest, so it refuses
+// when the original (or any guest binding the drives) is still on this host; on a replaced host it
+// proceeds and keeps its binds. Red-proof: make liveOriginalBeside return "" and the refusals pass
+// the restore through (the "refused" sub-tests fail).
+func TestRunBringUp_DRRefusesBesideALiveOriginal(t *testing.T) {
+	const target = 9299
+	drivesBind := proxmox.GuestConfig{Extra: map[string]json.RawMessage{
+		"mp8": json.RawMessage(`"/mnt/felhom-drives,mp=/mnt/felhom-drives"`),
+	}}
+	cases := []struct {
+		name    string
+		archive string
+		lxc     []proxmox.Guest
+		cfg     map[int]proxmox.GuestConfig
+		refuse  string // substring of the refusal; "" = must proceed
+	}{
+		{"the source guest still exists", "local:backup/vzdump-lxc-9201-2026_10_04-04_34_55.tar.zst",
+			[]proxmox.Guest{{VMID: 9201, Status: "running"}}, map[int]proxmox.GuestConfig{9201: scratchCfg()}, "source guest 9201"},
+		{"another guest binds the drives (PBS archive)", "felhom-pbs:backup/ct/9201/2026-10-04T02:34:55Z",
+			[]proxmox.Guest{{VMID: 9300, Status: "stopped"}}, map[int]proxmox.GuestConfig{9300: drivesBind}, "binds the household drives"},
+		{"a guest whose config cannot be read", "local:backup/vzdump-lxc-9201-x.tar.zst",
+			[]proxmox.Guest{{VMID: 9400, Status: "running"}}, map[int]proxmox.GuestConfig{}, "could not be read"},
+		{"replaced host: only an unrelated scratch guest", "local:backup/vzdump-lxc-9201-x.tar.zst",
+			[]proxmox.Guest{{VMID: 9202, Status: "running"}}, map[int]proxmox.GuestConfig{9202: scratchCfg()}, ""},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			cfg := c.cfg
+			cfg[target] = scratchCfg()
+			api := &fakeAPI{lxc: c.lxc, cfg: cfg}
+			e, fr, _, q := newDREngine(t, api)
+			defer q.Close()
+			res := e.RunBringUp(context.Background(), BringUpSpec{
+				Mode: ModeDRGuestLoss, Archive: c.archive, VMID: target, RestoreStorage: "local-lvm", KeepMAC: true,
+			})
+			if c.refuse != "" {
+				if res.Err == nil || !strings.Contains(res.Err.Error(), c.refuse) || !strings.Contains(res.Err.Error(), "R-834") {
+					t.Fatalf("want a refusal naming %q, got %+v", c.refuse, res)
+				}
+				if len(api.restores) != 0 || len(api.starts) != 0 || len(fr.cmds) != 0 {
+					t.Fatalf("a refused DR touched the host: restores=%d starts=%v cmds=%v", len(api.restores), api.starts, fr.cmds)
+				}
+				return
+			}
+			if res.Err != nil || !res.Pass {
+				t.Fatalf("a DR on a replaced host must proceed, got %+v", res)
+			}
+			// … and there it keeps the REAL drives bind (the right binds on a replaced host).
+			joined := strings.Join(fr.cmds, "\n")
+			if !strings.Contains(joined, "-mp8 /mnt/felhom-drives,mp=/mnt/felhom-drives") {
+				t.Fatalf("the DR guest lost its drives bind: %v", fr.cmds)
+			}
+		})
+	}
+}
+
+// Provisioning restores the GOLDEN (no drives, onboot set by the back-half on purpose): a drives-
+// binding guest on the host does not block it — the rule is DR's alone.
+func TestRunBringUp_ProvisionNotBlockedByADrivesBind(t *testing.T) {
+	api := &fakeAPI{
+		lxc: []proxmox.Guest{{VMID: 9201, Status: "running"}},
+		cfg: map[int]proxmox.GuestConfig{
+			9201: {Extra: map[string]json.RawMessage{"mp8": json.RawMessage(`"/mnt/felhom-drives,mp=/mnt/felhom-drives"`)}},
+			9203: scratchCfg(),
+		},
+	}
+	e, _, q := newEngine(t, api, EmptyProvider{})
+	defer q.Close()
+	res := e.RunBringUp(context.Background(), BringUpSpec{Mode: ModeProvision, Archive: "local:vztmpl/felhom-golden.tar.zst", VMID: 9203, RestoreStorage: "local-lvm"})
+	if res.Err != nil || !res.Pass {
+		t.Fatalf("provision must proceed, got %+v", res)
+	}
+}
+
+func TestArchiveSourceVMID(t *testing.T) {
+	for in, want := range map[string]int{
+		"local:backup/vzdump-lxc-9201-2026_10_04-04_34_55.tar.zst": 9201,
+		"felhom-pbs:backup/ct/9201/2026-10-04T02:34:55Z":           9201,
+		"tmp-dooplex-copy:backup/ct/9201/2026-10-03T19:00:00Z":     9201,
+		"local:vztmpl/felhom-golden.tar.zst":                       0,
+		"vol":                                                      0,
+	} {
+		if got := archiveSourceVMID(in); got != want {
+			t.Errorf("archiveSourceVMID(%q) = %d, want %d", in, got, want)
+		}
+	}
+}
+
+// R-834, the restore-test route: its scratch guest sits BESIDE the live original by design, so it must
+// carry no host-path bind — the archive's mp8 (the household's drives) and mp9 (the original's
+// bootstrap) are replaced by throwaway volumes AT restore time. Measured live 2026-10-04 on demo-hp
+// (`audits/backup-close-2026-10-04/partA/`): onboot 0 and no host bind on every poll. Red-proof:
+// make drRestoreOverrides return the archive's own mp8 value and this fails.
+func TestRestoreTest_NoHostPathBindBesideTheOriginal(t *testing.T) {
+	api := &fakeAPI{
+		cfg: map[int]proxmox.GuestConfig{990000: scratchCfg()},
+		extractCfg: "hostname: demo-hp\nonboot: 1\nrootfs: local-lvm:vm-9201-disk-0,size=16G\n" +
+			"mp0: local-lvm:vm-9201-disk-1,mp=/var/lib/felhom,backup=1,size=70G\n" +
+			"mp8: /mnt/felhom-drives,mp=/mnt/felhom-drives\n" +
+			"mp9: /var/lib/felhom-agent/guests/9201/bootstrap,mp=/etc/felhom-bootstrap,ro=1\n",
+	}
+	e, _, q := newEngine(t, api, EmptyProvider{})
+	defer q.Close()
+	_ = e.RunRestoreTest(context.Background(), RestoreTestSpec{
+		Archive: "local:backup/vzdump-lxc-9201-x.tar.zst", RestoreStorage: "local-lvm",
+		ScratchMin: 990000, ScratchMax: 990009, SourceTier: "local",
+	})
+	if len(api.restores) != 1 {
+		t.Fatalf("want one restore, got %+v", api.restores)
+	}
+	r := api.restores[0]
+	for _, slot := range []string{"mp8", "mp9"} {
+		v, ok := r.MountOverrides[slot]
+		if !ok || strings.HasPrefix(v, "/") {
+			t.Fatalf("%s = %q (present=%v): the scratch beside the original must get a throwaway volume, never the host path", slot, v, ok)
+		}
+	}
+	for slot, v := range r.MountOverrides {
+		if strings.HasPrefix(v, "/") {
+			t.Fatalf("%s carries a host path %q into the scratch guest", slot, v)
+		}
+	}
+	if r.ConfigOverrides["onboot"] != "0" {
+		t.Fatalf("onboot = %q, want 0", r.ConfigOverrides["onboot"])
+	}
+}