GL-5: explicit rootfs override for DR restore (live-discovered PVE constraint)

The live validation hit PVE's all-or-nothing restore rule: mpN params without an explicit rootfs -> HTTP 500 "mount points configured, but 'rootfs' not set" (the same constraint restoretest.go:211 documents for the live-config path; the spike never ran an override restore). The lost guest has no live config, so the rootfs SIZE now comes from the archive's own embedded config via NEW Client.ExtractArchiveConfig (GET vzdump/ extractconfig - verified live: answers 200 under the scoped agent token; PBS keys stay server-side, the spike's candidate-1 rejection holds; used for the SIZE ONLY - the bind layout stays the platform constants). Unparseable/unreadable archive config -> clean refusal before any restore. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6
2026-07-08 09:02:36 +02:00
parent c12b512316
commit 3bf0110697
5 changed files with 122 additions and 5 deletions
@@ -82,6 +82,22 @@ func structuralBootHostDir(stateDir string, vmid int) string {
 	return strings.TrimRight(stateDir, "/") + "/guests/" + strconv.Itoa(vmid) + "/bootstrap"
 }

+// archiveRootfsSizeGB parses the current-config "rootfs:" line out of an extracted archive config
+// (raw pct-conf text) and returns its size in whole GB — 0 if absent/unparseable. Snapshot sections
+// ("[name]") follow the current config; parsing stops at the first one so a snapshot's rootfs can
+// never shadow the live value.
+func archiveRootfsSizeGB(raw string) int {
+	for _, line := range strings.Split(raw, "\n") {
+		if strings.HasPrefix(line, "[") {
+			break
+		}
+		if v, ok := strings.CutPrefix(line, "rootfs:"); ok {
+			return rootfsSizeGB(strings.TrimSpace(v))
+		}
+	}
+	return 0
+}
+
 // configLockMaxAttempts bounds the F4 config-lock retry. configLockBackoff is a package var so
 // tests can shrink it (the production value gives PVE time to release its async config lock).
 const configLockMaxAttempts = 5
@@ -265,7 +281,23 @@ func (e *Engine) runBringUp(ctx context.Context, spec BringUpSpec, res *BringUpR
 	//    back-half adds them post-bring-up) — that asymmetry is the whole GL-5 bug.
 	var overrides map[string]string
 	if spec.Mode == ModeDRGuestLoss {
+		// PVE refuses a restore that carries mountpoint params unless `rootfs` is ALSO explicit
+		// ("mount points configured, but 'rootfs' not set" — the same all-or-nothing constraint
+		// restoretest.go:211 documents for the live-config path; hit live in the GL-5 validation).
+		// The lost guest has no live config, so the rootfs SIZE comes from the archive's own
+		// embedded config — used for the SIZE ONLY; the bind LAYOUT stays the platform constants.
+		raw, err := e.api.ExtractArchiveConfig(ctx, spec.Archive)
+		if err != nil {
+			res.Err = fmt.Errorf("reconcile: bring-up dr: extract archive config (for the explicit rootfs size): %w", err)
+			return
+		}
+		sz := archiveRootfsSizeGB(raw)
+		if sz <= 0 {
+			res.Err = fmt.Errorf("reconcile: bring-up dr: archive config carries no parseable rootfs size — refusing a mount-override restore without an explicit rootfs")
+			return
+		}
 		overrides = map[string]string{
+			"rootfs":             fmt.Sprintf("%s:%d", spec.RestoreStorage, sz),
 			structuralParentSlot: throwawayVolumeOverride(spec.RestoreStorage, structuralParentDir),
 			structuralBootSlot:   throwawayVolumeOverride(spec.RestoreStorage, structuralBootGuestPath),
 		}
@@ -792,13 +792,19 @@ func TestRunBringUp_DRStructuralBindOverridesAndSwap(t *testing.T) {
 		t.Fatalf("expected one restore, got %+v", api.restores)
 	}
 	ov := api.restores[0].MountOverrides
+	// rootfs rides along explicitly (PVE refuses mpN params without it — hit live in the GL-5
+	// validation), sized from the ARCHIVE's embedded config (the fake serves size=8G).
 	want := map[string]string{
-		"mp8": "local-lvm:1,mp=/mnt/felhom-drives,backup=0",
-		"mp9": "local-lvm:1,mp=/etc/felhom-bootstrap,backup=0",
+		"rootfs": "local-lvm:8",
+		"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"] {
+	if len(ov) != 3 || ov["rootfs"] != want["rootfs"] || ov["mp8"] != want["mp8"] || ov["mp9"] != want["mp9"] {
 		t.Fatalf("MountOverrides = %+v, want exactly %+v", ov, want)
 	}
+	if len(api.extracts) != 1 || api.extracts[0] != "local:backup/customer.tar.zst" {
+		t.Fatalf("the rootfs size must come from the archive's extracted config: %+v", api.extracts)
+	}
 	// 4d: mp9 host dir created under the engine state dir …
 	bootDir := structuralBootHostDir(sd, vmid)
 	if st, err := os.Stat(bootDir); err != nil || !st.IsDir() {
@@ -901,8 +907,8 @@ func TestRunBringUp_DRArchiveWithoutMp9(t *testing.T) {
 		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(ov) != 3 || ov["mp8"] == "" || ov["mp9"] == "" || ov["rootfs"] == "" {
+		t.Fatalf("overrides are constants (+explicit rootfs) — 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)
@@ -971,3 +977,43 @@ func TestRunBringUp_DRUnusedDeleteFailureWarns(t *testing.T) {
 		t.Fatalf("residue must surface as a warning: %+v", res.StartWarnings)
 	}
 }
+
+// GL-5: the archive-config rootfs parse — current-config line wins, snapshot sections never shadow
+// it, absent/unparseable → 0 (which makes the DR bring-up refuse rather than guess a size).
+func TestArchiveRootfsSizeGB(t *testing.T) {
+	full := "hostname: demo\nrootfs: local-lvm:vm-9201-disk-0,size=32G\nmp0: local-lvm:vm-9201-disk-1,mp=/var/lib/docker,size=200G\n"
+	if got := archiveRootfsSizeGB(full); got != 32 {
+		t.Errorf("full config: got %d, want 32", got)
+	}
+	snap := "hostname: demo\n\n[before-upgrade]\nrootfs: local-lvm:vm-9201-disk-9,size=99G\n"
+	if got := archiveRootfsSizeGB(snap); got != 0 {
+		t.Errorf("a snapshot section's rootfs must NOT shadow an absent current one: got %d, want 0", got)
+	}
+	if got := archiveRootfsSizeGB("hostname: demo\n"); got != 0 {
+		t.Errorf("no rootfs line: got %d, want 0", got)
+	}
+}
+
+// GL-5: an unreadable archive config fails the DR bring-up BEFORE any restore (no rootfs size =
+// the restore would fail anyway; refuse cleanly, nothing to roll back).
+func TestRunBringUp_DRExtractConfigFailureRefuses(t *testing.T) {
+	const vmid = 8206
+	api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{vmid: scratchCfg()}}
+	api.extractErr = errors.New("proxmox: GET extractconfig -> HTTP 500: volume not found")
+	e, _, _, q := newDREngine(t, api)
+	defer q.Close()
+
+	res := e.RunBringUp(context.Background(), BringUpSpec{
+		Mode: ModeDRGuestLoss, Archive: "local:backup/gone.tar.zst", VMID: vmid,
+		RestoreStorage: "local-lvm",
+	})
+	if res.Err == nil || !strings.Contains(res.Err.Error(), "extract archive config") {
+		t.Fatalf("extract failure must refuse naming the step, got %+v", res)
+	}
+	if len(api.restores) != 0 {
+		t.Fatalf("the refusal must fire BEFORE any restore, got %+v", api.restores)
+	}
+	if len(api.destroys) != 0 {
+		t.Fatalf("nothing was created — nothing to roll back, got %+v", api.destroys)
+	}
+}
@@ -53,6 +53,12 @@ type fakeAPI struct {
 	// poolAdds records (pool, vmid) for each PoolAddVMID; poolAddErr backs the failure path.
 	poolAdds   []poolAddCall
 	poolAddErr error
+
+	// extractCfg/extractErr back ExtractArchiveConfig (GL-5 DR rootfs sizing); extracts records
+	// the requested volumes. Empty extractCfg with nil extractErr → a canonical 8G-rootfs config.
+	extractCfg string
+	extractErr error
+	extracts   []string
 }

 type poolAddCall struct {
@@ -73,6 +79,23 @@ type resizeCall struct {
 	disk, size string
 }

+// ExtractArchiveConfig returns extractCfg/extractErr; with neither set it returns a canonical
+// minimal archive config (rootfs size=8G) so DR-mode tests that don't care about the rootfs
+// override don't have to stage one.
+func (f *fakeAPI) ExtractArchiveConfig(_ context.Context, volume string) (string, error) {
+	f.mu.Lock()
+	f.extracts = append(f.extracts, volume)
+	cfg, err := f.extractCfg, f.extractErr
+	f.mu.Unlock()
+	if err != nil {
+		return "", err
+	}
+	if cfg == "" {
+		cfg = "hostname: fake\nrootfs: local-lvm:vm-0-disk-0,size=8G\n"
+	}
+	return cfg, nil
+}
+
 func (f *fakeAPI) RestoreLXC(_ context.Context, opts proxmox.RestoreLXCOptions) (string, error) {
 	if f.restoreHook != nil {
 		f.restoreHook()
@@ -172,6 +172,10 @@ type GuestAPI interface {
 	ResizeLXC(ctx context.Context, vmid int, disk, size string) (string, error)
 	// RestoreLXC restores an archive into a (fresh) vmid — the create path (slice 6). Async → UPID.
 	RestoreLXC(ctx context.Context, opts proxmox.RestoreLXCOptions) (string, error)
+	// ExtractArchiveConfig reads the guest config embedded in a backup archive (raw pct-conf text).
+	// The DR bring-up sizes its explicit rootfs override from it (GL-5) — PVE refuses a restore
+	// carrying mpN params without an explicit rootfs, and the lost guest has no live config.
+	ExtractArchiveConfig(ctx context.Context, volume string) (string, error)
 	// PoolAddVMID re-asserts pool membership after a restore-over-existing (campaign-2 R2). Sync (no
 	// UPID); idempotent. Membership is what lets the pool-scoped token reach the guest next time.
 	PoolAddVMID(ctx context.Context, pool string, vmid int) error