GL-5b: restore-test full-fidelity verification (v0.76.0)
The restore-test had GL-5 finding #2's mirror image: its live-source-config bind-override path tripped PVE's drop-unlisted-mountpoints rule, so scratch guests boot-verified WITHOUT their storage mpN - weaker verification than claimed. Params now derive from the ARCHIVE's own embedded config via ExtractArchiveConfig + drRestoreOverrides (the object under test; full layout, content genuinely extracted - the added runtime IS the verification); unreadable/unknown-topology archives refuse up front. NEW mount-parity assert (2b, pre-start): restored mpN set vs the archive's - missing/mispathed/undersized/extra mpN fail the test naming the delta, so constraint (b) can never regress into a green light. MountParity + MountInventory ride the result + hub wire record (additive). Dead bindMountOverrides/archiveVMID path deleted with its tests (no reachable lookalike). DR bring-up untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6
This commit is contained in:
@@ -4,7 +4,7 @@ import (
|
||||
"context"
|
||||
"fmt"
|
||||
"math"
|
||||
"regexp"
|
||||
"sort"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
@@ -63,6 +63,14 @@ type RestoreTestResult struct {
|
||||
// wrong/stale recognizer can at worst over-notice a benign warning, never false-fail and
|
||||
// never hide a real one. Empty StartWarnings ⇒ trivially recognized (N/A).
|
||||
WarningsRecognized bool
|
||||
// MountParity is "ok" when the restored scratch's mpN set matches the archive's (GL-5b: the
|
||||
// assert that keeps PVE's drop-unlisted-mountpoints rule from ever regressing into a green
|
||||
// light); "mismatch" fails the test with Err naming the delta. "" on runs that never reached
|
||||
// the assert (restore failed earlier).
|
||||
MountParity string
|
||||
// MountInventory lists the parity-verified mountpoints ("mpN=<path> (<N>G)" / throwaway
|
||||
// stand-ins) — the record's proof of WHAT was verified, not just that something booted.
|
||||
MountInventory []string
|
||||
}
|
||||
|
||||
// benignWarningAnchor is a deliberately version-FREE substring of the systemd-nesting start
|
||||
@@ -198,36 +206,27 @@ func (e *Engine) runScratchTest(ctx context.Context, vmid int, spec RestoreTestS
|
||||
|
||||
// 1. Restore into the fresh scratch VMID (benign create path). The UPID is for error
|
||||
// detection only — it does NOT make the Scratch entry terminal (teardown does).
|
||||
// A source guest with a host BIND-mount mountpoint (slice-10 data drive) can't be
|
||||
// vzrestore'd by the privsep token ("restoring 'mpN' to bind mount is only possible for
|
||||
// root"). The restore-test only needs the guest to BOOT — the data drives are irrelevant
|
||||
// (their host paths would also collide). So neutralize each source bind-mount mpN to a
|
||||
// throwaway volume on the restore storage (needs no root). Best-effort: if the source
|
||||
// config can't be read, restore as-is (a bind-mount guest then fails as before, in the verdict).
|
||||
var mountOverrides map[string]string
|
||||
if srcVMID, ok := archiveVMID(spec.Archive); ok {
|
||||
if srcCfg, cerr := e.api.GuestConfig(ctx, srcVMID); cerr == nil {
|
||||
if binds := bindMountOverrides(srcCfg.MountPoints(), spec.RestoreStorage); len(binds) > 0 {
|
||||
// PVE refuses a restore that carries mountpoint params unless `rootfs` is also set
|
||||
// ("mount points configured, but 'rootfs' not set"). Size the rootfs override from the
|
||||
// SOURCE rootfs (restore needs target >= the archive's volume). Without a parseable
|
||||
// size we can't safely override, so restore as-is (the bind-mount restore then fails
|
||||
// in the verdict rather than risking a wrong rootfs size).
|
||||
if sz := rootfsSizeGB(srcCfg.RootFS); sz > 0 {
|
||||
binds["rootfs"] = fmt.Sprintf("%s:%d", spec.RestoreStorage, sz)
|
||||
mountOverrides = binds
|
||||
e.logger.Info("restore-test: neutralizing source bind-mount mountpoints for scratch restore",
|
||||
"source_vmid", srcVMID, "scratch", vmid, "bind_mounts", len(binds)-1, "rootfs_gb", sz)
|
||||
} else {
|
||||
e.logger.Warn("restore-test: source has bind mounts but rootfs size unparseable — restoring as-is",
|
||||
"source_vmid", srcVMID, "rootfs", srcCfg.RootFS)
|
||||
}
|
||||
}
|
||||
} else {
|
||||
e.logger.Warn("restore-test: could not read source config for mp overrides (restoring as-is)",
|
||||
"source_vmid", srcVMID, "err", cerr)
|
||||
}
|
||||
// The restore params derive from the ARCHIVE's own embedded config — the object under
|
||||
// test — exactly like DR bring-up (GL-5b closes GL-5 finding #2's mirror image): PVE's
|
||||
// explicit-params restore REQUIRES an explicit rootfs AND silently DROPS unlisted
|
||||
// mountpoints, so the pre-v0.76.0 live-source-config bind-override path boot-verified
|
||||
// scratch guests WITHOUT their storage mpN — weaker verification than it claimed. The FULL
|
||||
// layout now rides: rootfs explicit, every storage mpN passed through (its content is
|
||||
// genuinely EXTRACTED — full fidelity; the added runtime IS the verification), the two
|
||||
// structural binds → throwaway stand-ins. An unreadable archive config or an unknown
|
||||
// topology REFUSES up front — never restore a partial guest to "verify" it.
|
||||
rawCfg, err := e.api.ExtractArchiveConfig(ctx, spec.Archive)
|
||||
if err != nil {
|
||||
res.Err = fmt.Errorf("reconcile: restore-test extract archive config: %w", err)
|
||||
return false
|
||||
}
|
||||
mountOverrides, err := drRestoreOverrides(rawCfg, spec.RestoreStorage)
|
||||
if err != nil {
|
||||
res.Err = fmt.Errorf("reconcile: restore-test: %w", err)
|
||||
return false
|
||||
}
|
||||
e.logger.Info("restore-test: full-fidelity restore params derived from the archive config",
|
||||
"scratch", vmid, "params", len(mountOverrides))
|
||||
upid, err := e.api.RestoreLXC(ctx, proxmox.RestoreLXCOptions{
|
||||
// Pool=DefaultPool so the scratch guest is created INTO the felhom pool — else a pool-scoped
|
||||
// token 403s on the scratch guest's config/start/destroy (SPIKE residual #2).
|
||||
@@ -262,6 +261,21 @@ func (e *Engine) runScratchTest(ctx context.Context, vmid int, spec RestoreTestS
|
||||
res.Err = fmt.Errorf("reconcile: restore-test read scratch config: %w", err)
|
||||
return
|
||||
}
|
||||
|
||||
// 2b. Mount-parity assert (GL-5b, the non-hollow core): the restored guest's mpN set must
|
||||
// match the archive's — storage mpN as real volumes at their archived path+size, the two
|
||||
// structural binds as their throwaway stand-ins. This makes GL-5's PVE rule (b) —
|
||||
// unlisted mountpoints silently dropped — impossible to regress into a green light: a
|
||||
// boot-only verify cannot see a missing data volume; this can.
|
||||
inventory, delta := mountParity(archiveCurrentConfig(rawCfg), cfg.MountPoints())
|
||||
res.MountInventory = inventory
|
||||
if len(delta) > 0 {
|
||||
res.MountParity = "mismatch"
|
||||
res.Err = fmt.Errorf("reconcile: restore-test mount parity FAILED (restored scratch does not match the archive): %s", strings.Join(delta, "; "))
|
||||
return
|
||||
}
|
||||
res.MountParity = "ok"
|
||||
|
||||
for key, val := range cfg.Nets() {
|
||||
if _, err := e.api.SetConfig(ctx, vmid, map[string]string{key: withLinkDown(val)}); err != nil {
|
||||
res.Err = fmt.Errorf("reconcile: restore-test net link-down %s: %w", key, err)
|
||||
@@ -307,58 +321,72 @@ func (e *Engine) runScratchTest(ctx context.Context, vmid int, spec RestoreTestS
|
||||
return false
|
||||
}
|
||||
|
||||
// archiveVMID extracts the source VMID from a backup archive volid. Handles PBS volids
|
||||
// ("<store>:backup/ct/<vmid>/<ts>" and the /vm/<vmid>/ form) and vzdump file volids
|
||||
// ("<store>:backup/vzdump-(lxc|qemu)-<vmid>-..."). Returns false when no vmid is found.
|
||||
func archiveVMID(volid string) (int, bool) {
|
||||
for _, re := range []*regexp.Regexp{pbsArchiveRE, vzdumpArchiveRE} {
|
||||
if m := re.FindStringSubmatch(volid); m != nil {
|
||||
if n, err := strconv.Atoi(m[1]); err == nil {
|
||||
return n, true
|
||||
}
|
||||
}
|
||||
}
|
||||
return 0, false
|
||||
}
|
||||
|
||||
var (
|
||||
pbsArchiveRE = regexp.MustCompile(`/(?:ct|vm)/(\d+)/`)
|
||||
vzdumpArchiveRE = regexp.MustCompile(`vzdump-(?:lxc|qemu)-(\d+)-`)
|
||||
)
|
||||
|
||||
// bindMountOverrides maps each BIND-mount mpN (the volume part is an absolute host PATH, not a
|
||||
// "storage:volume") to a throwaway 1G volume override on restoreStorage, preserving the in-guest
|
||||
// mount path. Storage-backed mpN are left to restore normally (returns nil when there are none).
|
||||
// This is what lets the restore-test boot-verify a slice-10 enrolled guest whose data drive is a
|
||||
// host bind mount the privsep token can't otherwise restore.
|
||||
func bindMountOverrides(mps map[string]string, restoreStorage string) map[string]string {
|
||||
out := map[string]string{}
|
||||
for key, val := range mps {
|
||||
volPart, rest, _ := strings.Cut(val, ",")
|
||||
if !strings.HasPrefix(volPart, "/") {
|
||||
continue // "storage:volume" → a real volume, not a host bind mount
|
||||
}
|
||||
mp := mountPathOf(rest)
|
||||
if mp == "" {
|
||||
mp = volPart // fall back to the host path if no explicit in-guest mp=
|
||||
}
|
||||
out[key] = throwawayVolumeOverride(restoreStorage, mp)
|
||||
}
|
||||
if len(out) == 0 {
|
||||
return nil
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// throwawayVolumeOverride is the ONE source of the restore-override value format: a throwaway 1G
|
||||
// volume on restoreStorage at the given in-guest path, excluded from backup. Used by the
|
||||
// restore-test's source-config-derived overrides (above) and by the DR bring-up's synthesized
|
||||
// structural-bind overrides (GL-5, bringup.go — its mpN are platform constants, so it skips this
|
||||
// file's is-a-bind filtering and calls the format directly).
|
||||
// volume on restoreStorage at the given in-guest path, excluded from backup. Used by
|
||||
// drRestoreOverrides (bringup.go) for the structural binds — which both the DR bring-up and the
|
||||
// restore-test (GL-5b) derive their restore params from. (The old live-source-config
|
||||
// bindMountOverrides/archiveVMID path was deleted in v0.76.0 when the restore-test switched to
|
||||
// archive-derived params: it verified the wrong object AND tripped PVE's drop-unlisted-mountpoints
|
||||
// rule; keeping the dead lookalike reachable is how the next bug happens.)
|
||||
func throwawayVolumeOverride(restoreStorage, guestPath string) string {
|
||||
return fmt.Sprintf("%s:1,mp=%s,backup=0", restoreStorage, guestPath)
|
||||
}
|
||||
|
||||
// mountParity compares the restored scratch guest's mpN set against the archive's current config
|
||||
// (GL-5b). Per archive mpN: a storage-backed one must exist restored at the same in-guest path
|
||||
// with at least the archived size (the pass-through recreates it at exactly that size); a
|
||||
// structural host bind must exist as its throwaway stand-in at the same path. Restored mpN slots
|
||||
// the archive doesn't carry are a delta too. Returns the verified inventory + the mismatch list
|
||||
// (empty delta = parity). Deterministic order (sorted slots).
|
||||
func mountParity(archiveCfg, restored map[string]string) (inventory, delta []string) {
|
||||
isMP := func(k string) bool {
|
||||
return len(k) > 2 && k[:2] == "mp" && k[2] >= '0' && k[2] <= '9'
|
||||
}
|
||||
slots := make([]string, 0, len(archiveCfg))
|
||||
for k := range archiveCfg {
|
||||
if isMP(k) {
|
||||
slots = append(slots, k)
|
||||
}
|
||||
}
|
||||
sort.Strings(slots)
|
||||
for _, k := range slots {
|
||||
aval := archiveCfg[k]
|
||||
avol, arest, _ := strings.Cut(aval, ",")
|
||||
apath := mountPathOf(arest)
|
||||
if apath == "" && strings.HasPrefix(avol, "/") {
|
||||
apath = avol // a bind without an explicit mp= mounts at its host path
|
||||
}
|
||||
rval, ok := restored[k]
|
||||
if !ok {
|
||||
delta = append(delta, fmt.Sprintf("%s MISSING from the restored guest (archive: %s)", k, aval))
|
||||
continue
|
||||
}
|
||||
_, rrest, _ := strings.Cut(rval, ",")
|
||||
if rpath := mountPathOf(rrest); rpath != apath {
|
||||
delta = append(delta, fmt.Sprintf("%s path %q != archive %q", k, rpath, apath))
|
||||
continue
|
||||
}
|
||||
if strings.HasPrefix(avol, "/") {
|
||||
// structural bind → its throwaway stand-in (a real volume at the same path)
|
||||
inventory = append(inventory, fmt.Sprintf("%s=%s (throwaway for the archived bind)", k, apath))
|
||||
continue
|
||||
}
|
||||
asz, rsz := rootfsSizeGB(aval), rootfsSizeGB(rval)
|
||||
if asz > 0 && rsz < asz {
|
||||
delta = append(delta, fmt.Sprintf("%s size %dG < archive %dG", k, rsz, asz))
|
||||
continue
|
||||
}
|
||||
inventory = append(inventory, fmt.Sprintf("%s=%s (%dG)", k, apath, rsz))
|
||||
}
|
||||
for k, v := range restored {
|
||||
if isMP(k) && archiveCfg[k] == "" {
|
||||
delta = append(delta, fmt.Sprintf("%s present on the restored guest but not in the archive: %s", k, v))
|
||||
}
|
||||
}
|
||||
sort.Strings(delta)
|
||||
return inventory, delta
|
||||
}
|
||||
|
||||
// mountPathOf returns the mp= field from an mpN value's trailing options ("" if absent).
|
||||
func mountPathOf(opts string) string {
|
||||
for _, kv := range strings.Split(opts, ",") {
|
||||
|
||||
Reference in New Issue
Block a user