From b3446213df7a286353f9e8fe45141a1cffb09f95 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Wed, 8 Jul 2026 09:10:54 +0200 Subject: [PATCH] GL-5: DR restore passes the FULL archive layout (live finding #2) The live scratch DR exposed the second half of PVE's all-or-nothing explicit-params restore: mountpoints NOT named in the params are silently DROPPED - the DR guest came up without its mp0/mp1 data volumes (boot passed; the customer's world did not ride along). drRestoreOverrides now derives the COMPLETE param set from the archive's extracted config: explicit rootfs, every storage-backed mpN passed through (size + in-guest path + backup flag preserved so vzrestore extracts its content), the two structural binds replaced by 4d-swapped throwaways; unknown bind mpN or unparseable size refuses loudly. Snapshot sections never shadow the current config. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6 --- CHANGELOG.md | 40 +++++++++++++ internal/reconcile/bringup.go | 90 ++++++++++++++++++++++-------- internal/reconcile/bringup_test.go | 57 +++++++++++++++---- 3 files changed, 154 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a3dca6..df3c077 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,43 @@ +## v0.75.0 — DR bring-up structural bind overrides + real-bind swap (GL-5 / go-live G8) (2026-07-08) + +Implements the verdict of `felhom.eu/documentation/audits/SPIKE-dr-bindmount-source-2026-07-07.md`: +`bring-up -mode dr` of a customer archive FAILED outright because the archive carries the two +structural host-bind mountpoints (mp8 parent bind, mp9 bootstrap bind) that a `pct restore` under +the privsep token cannot recreate ("restoring 'mp8' to bind mount is only possible for root") — +bring-up passed no `MountOverrides`. Provision was never affected (the golden has no mp8/mp9; the +back-half adds them) — that asymmetry was the bug. + +- **DR restore overrides** (`internal/reconcile/bringup.go`): ModeDRGuestLoss synthesizes throwaway + 1G-volume overrides for the two PLATFORM-CONSTANT mpN (mirroring backhalf.go's values; the spike's + whole point — no archive parse for the LAYOUT) via the shared `throwawayVolumeOverride` format + helper (extracted from `bindMountOverrides`; the restore-test's is-a-bind FILTER reads live + configs, which DR by definition has none of). ModeProvision passes nil — regression-contract test. +- **LIVE-DISCOVERED PVE constraint (not in the spike — it never ran an override restore):** mpN + restore params without an explicit `rootfs` → HTTP 500 "mount points configured, but 'rootfs' not + set" (same rule restoretest.go:211 documents for the live-config path). Fixed with NEW + `Client.ExtractArchiveConfig` (GET `/nodes/{node}/vzdump/extractconfig` — **answers 200 under the + scoped agent token**, verified live; PBS keys stay server-side, the spike's candidate-1 rejection + holds): the archive's OWN embedded config supplies the rootfs SIZE only; unreadable/unparseable → + clean refusal before any restore. +- **Step 4d — real-bind swap** (DR only, pre-start): mp9 bootstrap host dir created (idempotent — + a same-host guest-loss still has bootstrap.json there, untouched), then mp8/mp9 set to the REAL + binds via the host runner (`pct set` — bind mounts are root@pam-only, hence NOT the API; sudoers + already allowlists both shapes), one slot per call so a failure names the exact mpN and rolls + back per the committed/launched envelope (C2 — never a silent half-wired success). The displaced + throwaway volumes (PVE parks them as `unusedN`) are deleted via one config PUT (`delete=`); + a scoped-token refusal logs the residue LOUDLY + warns in the result instead of widening + privileges. NEW `proxmox.GuestConfig.Unused()`. +- **Engine seam:** `EngineOptions.HostRunner` + `StateDir` (DR refuses up front on an API-only + engine); the bring-up selftest wires the same ExecRunner shape as the back-half and removes the + scratch vmid's mp9 host dir at teardown (never a real drive's bind source). +- Tests (non-hollow): Scenario A exact-override + exact-swap-command + unusedN-delete asserts; + provision-nil regression; C2 mid-swap rollback; C3 older-archive-without-mp9; DR-without-runner + refusal; extract-failure refusal; 403-residue warn; archive rootfs parse (snapshot sections never + shadow). Red-proofs: override synthesis reverted → A FAILS; unconditional overrides → B FAILS + (both run→fail→revert). +- Live validation (campaign-2-precedent scratch DR, §REPORT): see REPORT.md — the exact op that + failed on v0.74.0 now round-trips with correctly-wired binds and no residue. + ## v0.74.0 — pool membership re-asserted after restore-over-existing (campaign-2 R2) (2026-07-07) Closes campaign-2 finding **R2** (`felhom.eu/documentation/tests/CAMPAIGN-2-2026-07-07.md`). Pool diff --git a/internal/reconcile/bringup.go b/internal/reconcile/bringup.go index dc0fc5d..b69325d 100644 --- a/internal/reconcile/bringup.go +++ b/internal/reconcile/bringup.go @@ -82,20 +82,66 @@ 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 { +// archiveCurrentConfig parses the CURRENT-config key/value lines out of an extracted archive +// config (raw pct-conf text). Snapshot sections ("[name]") follow the current config; parsing +// stops at the first one so a snapshot's values can never shadow the live ones. +func archiveCurrentConfig(raw string) map[string]string { + out := map[string]string{} 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)) + k, v, ok := strings.Cut(line, ":") + if !ok { + continue } + out[k] = strings.TrimSpace(v) } - return 0 + return out +} + +// drRestoreOverrides builds the COMPLETE restore param set a DR bring-up must pass, from the +// archive's extracted config (GL-5; both PVE constraints discovered live — see the call site): +// explicit rootfs (sized from the archive), every storage-backed mpN passed through (recreated at +// its archived size + in-guest path + backup flag so vzrestore extracts its content), and the two +// structural bind slots replaced with throwaway volumes for 4d to swap. Unknown bind mpN or an +// unparseable size → error (never silently restore a guest missing a mount). +func drRestoreOverrides(rawArchiveCfg, restoreStorage string) (map[string]string, error) { + cfg := archiveCurrentConfig(rawArchiveCfg) + sz := rootfsSizeGB(cfg["rootfs"]) + if sz <= 0 { + return nil, fmt.Errorf("archive config carries no parseable rootfs size — refusing a mount-override restore without an explicit rootfs") + } + out := map[string]string{"rootfs": fmt.Sprintf("%s:%d", restoreStorage, sz)} + for key, val := range cfg { + if len(key) <= 2 || key[:2] != "mp" || key[2] < '0' || key[2] > '9' { + continue + } + volPart, rest, _ := strings.Cut(val, ",") + if strings.HasPrefix(volPart, "/") { + // a host bind — must be one of the two structural slots (replaced below) + if key != structuralParentSlot && key != structuralBootSlot { + return nil, fmt.Errorf("archive carries an unknown bind mountpoint %s=%q — refusing (only the structural %s/%s binds are known)", key, val, structuralParentSlot, structuralBootSlot) + } + continue + } + msz := rootfsSizeGB(val) + if msz <= 0 { + return nil, fmt.Errorf("archive mountpoint %s=%q carries no parseable size — cannot pass it through the explicit-params restore", key, val) + } + mp := mountPathOf(rest) + if mp == "" { + return nil, fmt.Errorf("archive mountpoint %s=%q carries no mp= path", key, val) + } + v := fmt.Sprintf("%s:%d,mp=%s", restoreStorage, msz, mp) + if strings.Contains(","+rest+",", ",backup=1,") { + v += ",backup=1" + } + out[key] = v + } + out[structuralParentSlot] = throwawayVolumeOverride(restoreStorage, structuralParentDir) + out[structuralBootSlot] = throwawayVolumeOverride(restoreStorage, structuralBootGuestPath) + return out, nil } // configLockMaxAttempts bounds the F4 config-lock retry. configLockBackoff is a package var so @@ -281,26 +327,26 @@ 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. + // PVE's explicit-params restore is ALL-OR-NOTHING (both halves hit live in the GL-5 + // validation): (a) any mpN param without an explicit `rootfs` → 500 "mount points + // configured, but 'rootfs' not set" (the constraint restoretest.go:211 documents); + // (b) mountpoints NOT named in the params are silently DROPPED from the restore — a DR + // guest restored with only the bind overrides came up WITHOUT its mp0/mp1 data volumes. + // So the FULL layout must be specified, from the archive's own embedded config: rootfs + // sized from it, every storage-backed mpN passed through (size+path+backup preserved → + // vzrestore extracts its content), and the two structural binds replaced by throwaways + // that 4d swaps for the real binds. A bind mpN outside the two structural slots means an + // unknown topology — refuse loudly rather than restore a guest missing a mount. 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) + res.Err = fmt.Errorf("reconcile: bring-up dr: extract archive config (the restore params derive from it): %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") + overrides, err = drRestoreOverrides(raw, spec.RestoreStorage) + if err != nil { + res.Err = fmt.Errorf("reconcile: bring-up dr: %w", err) return } - overrides = map[string]string{ - "rootfs": fmt.Sprintf("%s:%d", spec.RestoreStorage, sz), - structuralParentSlot: throwawayVolumeOverride(spec.RestoreStorage, structuralParentDir), - structuralBootSlot: throwawayVolumeOverride(spec.RestoreStorage, structuralBootGuestPath), - } } upid, err := e.api.RestoreLXC(ctx, proxmox.RestoreLXCOptions{ VMID: spec.VMID, Archive: spec.Archive, Storage: spec.RestoreStorage, Pool: spec.Pool, diff --git a/internal/reconcile/bringup_test.go b/internal/reconcile/bringup_test.go index 4c929f5..70b4765 100644 --- a/internal/reconcile/bringup_test.go +++ b/internal/reconcile/bringup_test.go @@ -978,19 +978,54 @@ func TestRunBringUp_DRUnusedDeleteFailureWarns(t *testing.T) { } } -// 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) +// GL-5: drRestoreOverrides builds the COMPLETE explicit-params restore set from the archive's +// extracted config — the live validation proved PVE drops any mountpoint NOT named in the params, +// so mp0/mp1 pass-through is what keeps a DR guest's data volumes. Snapshot sections never shadow; +// unknown binds and unparseable sizes refuse. +func TestDRRestoreOverrides(t *testing.T) { + raw := `hostname: demo +rootfs: local-lvm:vm-9201-disk-0,size=32G +mp0: local-lvm:vm-9201-disk-1,mp=/var/lib/docker,backup=1,size=200G +mp1: local-lvm:vm-9201-disk-2,mp=/mnt/sys_drive,backup=1,size=50G +mp8: /mnt/felhom-drives,mp=/mnt/felhom-drives +mp9: /var/lib/felhom-agent/guests/9201/bootstrap,mp=/etc/felhom-bootstrap,ro=1 +[snap1] +rootfs: local-lvm:vm-9201-disk-9,size=99G +` + ov, err := drRestoreOverrides(raw, "local-lvm") + if err != nil { + t.Fatalf("drRestoreOverrides: %v", err) } - 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) + want := map[string]string{ + "rootfs": "local-lvm:32", + "mp0": "local-lvm:200,mp=/var/lib/docker,backup=1", + "mp1": "local-lvm:50,mp=/mnt/sys_drive,backup=1", + "mp8": "local-lvm:1,mp=/mnt/felhom-drives,backup=0", + "mp9": "local-lvm:1,mp=/etc/felhom-bootstrap,backup=0", } - if got := archiveRootfsSizeGB("hostname: demo\n"); got != 0 { - t.Errorf("no rootfs line: got %d, want 0", got) + if len(ov) != len(want) { + t.Fatalf("overrides = %+v, want %+v", ov, want) + } + for k, v := range want { + if ov[k] != v { + t.Errorf("override[%s] = %q, want %q", k, ov[k], v) + } + } + + // an UNKNOWN bind mpN = unknown topology → refuse (never restore a guest missing a mount) + if _, err := drRestoreOverrides("rootfs: l:d,size=8G\nmp3: /srv/other,mp=/data\n", "local-lvm"); err == nil || + !strings.Contains(err.Error(), "unknown bind mountpoint") { + t.Errorf("unknown bind must refuse, got %v", err) + } + // no parseable rootfs → refuse + if _, err := drRestoreOverrides("hostname: x\n", "local-lvm"); err == nil || + !strings.Contains(err.Error(), "rootfs size") { + t.Errorf("missing rootfs must refuse, got %v", err) + } + // a storage mpN without a size → refuse (cannot pass it through) + if _, err := drRestoreOverrides("rootfs: l:d,size=8G\nmp0: l:d1,mp=/x\n", "local-lvm"); err == nil || + !strings.Contains(err.Error(), "no parseable size") { + t.Errorf("sizeless mpN must refuse, got %v", err) } }