v0.133.0: a restore-test can never fill a box's disk; leftovers retried on a timer (R-672, R-673)
gates / gates (push) Successful in 13s
gates / gates (push) Successful in 13s
Space preflight before anything is created (uncompressed size from the vzdump log / PBS snapshot, x1.2 + 5 GiB, thin metadata, off the tested guest's pool when another storage is eligible, unknown refuses, reported as a non-pass result). Failed scratch teardown and the stale-lock sweep retried every 10 min (the sweep under the heavy-op gate). A thin pool crossing 90% requests an immediate host report. Six red-proofs. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS
This commit is contained in:
@@ -52,7 +52,7 @@ func newDREngine(t *testing.T, api GuestAPI) (*Engine, *fakeRunner, string, *Que
|
||||
t.Cleanup(q.Close)
|
||||
fr := &fakeRunner{}
|
||||
sd := t.TempDir()
|
||||
e := NewEngine(EngineOptions{API: api, Queue: q, Journal: j, Provider: EmptyProvider{}, HostRunner: fr, StateDir: sd})
|
||||
e := NewEngine(EngineOptions{API: api, Queue: q, Journal: j, Provider: EmptyProvider{}, HostRunner: fr, StateDir: sd, RestoreSpace: roomySpace{}})
|
||||
return e, fr, sd, q
|
||||
}
|
||||
|
||||
|
||||
@@ -44,6 +44,17 @@ type Engine struct {
|
||||
|
||||
opSeq uint64 // atomic; makes each op id unique per attempt
|
||||
|
||||
// restoreSpace + spacePolicy are the restore-test's space preflight (R-672). nil space REFUSES
|
||||
// every restore-test (fail-closed) — see restoretest_space.go.
|
||||
restoreSpace RestoreSpace
|
||||
spacePolicy SpacePolicy
|
||||
|
||||
// scratchMu guards activeScratch (the vmids a running restore-test owns — the retry timer never
|
||||
// touches those) and teardownTries (failed timer retries per journal op, R-672 rule 3).
|
||||
scratchMu sync.Mutex
|
||||
activeScratch map[int]bool
|
||||
teardownTries map[string]int
|
||||
|
||||
// lastRes records the most recent successful Reconcile Result (v0.90.0, R-28 fast-tick source).
|
||||
// The fast-tick reads it to decide convergence: actionable drift is Planned − Pending > 0 (a
|
||||
// destructive pending_signature refusal is EXPECTED state, not drift to hammer on). lastOK is
|
||||
@@ -86,6 +97,10 @@ type EngineOptions struct {
|
||||
HostRunner proxmox.Runner
|
||||
// StateDir is the agent state dir ("" → /var/lib/felhom-agent); only the 4d swap reads it.
|
||||
StateDir string
|
||||
// RestoreSpace is the restore-test's space preflight (R-672). nil → every restore-test is REFUSED
|
||||
// with its reason (fail-closed). SpacePolicy zero → DefaultSpacePolicy.
|
||||
RestoreSpace RestoreSpace
|
||||
SpacePolicy SpacePolicy
|
||||
}
|
||||
|
||||
// NewEngine builds an Engine. The Queue is shared (the single §10 choke point); the
|
||||
@@ -124,9 +139,24 @@ func NewEngine(opts EngineOptions) *Engine {
|
||||
logger: logger,
|
||||
hostRun: opts.HostRunner,
|
||||
stateDir: stateDir,
|
||||
|
||||
restoreSpace: opts.RestoreSpace,
|
||||
spacePolicy: policyOrDefault(opts.SpacePolicy),
|
||||
activeScratch: map[int]bool{},
|
||||
teardownTries: map[string]int{},
|
||||
}
|
||||
}
|
||||
|
||||
func policyOrDefault(p SpacePolicy) SpacePolicy {
|
||||
if p.Factor < 1 {
|
||||
p.Factor = DefaultSpacePolicy.Factor
|
||||
}
|
||||
if p.ReserveBytes <= 0 {
|
||||
p.ReserveBytes = DefaultSpacePolicy.ReserveBytes
|
||||
}
|
||||
return p
|
||||
}
|
||||
|
||||
// Result summarizes one Reconcile pass.
|
||||
type Result struct {
|
||||
Planned int
|
||||
|
||||
@@ -213,7 +213,7 @@ func newEngine(t *testing.T, api GuestAPI, provider DesiredProvider) (*Engine, *
|
||||
t.Cleanup(func() { j.Close() })
|
||||
q := NewQueue()
|
||||
t.Cleanup(q.Close)
|
||||
e := NewEngine(EngineOptions{API: api, Queue: q, Journal: j, Provider: provider})
|
||||
e := NewEngine(EngineOptions{API: api, Queue: q, Journal: j, Provider: provider, RestoreSpace: roomySpace{}})
|
||||
return e, j, q
|
||||
}
|
||||
|
||||
|
||||
@@ -50,10 +50,18 @@ type RestoreTestResult struct {
|
||||
ScratchVMID int
|
||||
Pass bool
|
||||
Verified string // "boot+running" this slice
|
||||
Skipped bool // no free scratch VMID in band → test not run
|
||||
Err error
|
||||
StartedAt time.Time
|
||||
Duration time.Duration
|
||||
Skipped bool // test not run: no free scratch VMID in band, or the space preflight refused (R-672)
|
||||
// SkipReason is set when the SPACE PREFLIGHT refused (R-672): the test did not run, and this is
|
||||
// reported to the hub as the test's result (pass=false), never as a pass. Empty for a band skip.
|
||||
SkipReason string
|
||||
// TargetStorage is where the restore went (rule 2 may move it off the tested guest's pool);
|
||||
// RequiredBytes/AvailBytes are rule 1's figures.
|
||||
TargetStorage string
|
||||
RequiredBytes int64
|
||||
AvailBytes int64
|
||||
Err error
|
||||
StartedAt time.Time
|
||||
Duration time.Duration
|
||||
// StartWarnings holds the warning line(s) the guest-start task emitted (e.g. the
|
||||
// systemd-nesting advisory). Populated only when the start exited "WARNINGS: N";
|
||||
// always surfaced, NEVER used to decide pass/fail (the verdict is liveness — waitRunning).
|
||||
@@ -136,6 +144,29 @@ func (e *Engine) RunRestoreTest(ctx context.Context, spec RestoreTestSpec) Resto
|
||||
return res
|
||||
}
|
||||
|
||||
// R-672: the space preflight, BEFORE anything is journaled or created.
|
||||
rawCfg, err := e.api.ExtractArchiveConfig(ctx, spec.Archive)
|
||||
if err != nil {
|
||||
res.Err = fmt.Errorf("reconcile: restore-test extract archive config: %w", err)
|
||||
return res
|
||||
}
|
||||
v := PreflightRestoreSpace(ctx, e.restoreSpace, e.spacePolicy, spec.Archive, rawCfg, spec.RestoreStorage)
|
||||
res.TargetStorage, res.RequiredBytes, res.AvailBytes = v.Storage, v.Required, v.Avail
|
||||
if !v.OK {
|
||||
res.Skipped = true
|
||||
res.SkipReason = "skipped: " + v.Reason
|
||||
e.logger.Warn("restore-test SKIPPED by the space preflight (R-672) — nothing was created",
|
||||
"archive", spec.Archive, "storage", v.Storage, "required_bytes", v.Required, "avail_bytes", v.Avail, "reason", v.Reason)
|
||||
res.Duration = time.Since(now)
|
||||
return res
|
||||
}
|
||||
if v.Storage != spec.RestoreStorage {
|
||||
e.logger.Info("restore-test: restoring OFF the tested guest's own pool (R-672 rule 2)",
|
||||
"configured", spec.RestoreStorage, "avoided", v.Avoided, "target", v.Storage)
|
||||
}
|
||||
e.logger.Info("restore-test: space preflight passed", "storage", v.Storage, "required_bytes", v.Required, "avail_bytes", v.Avail)
|
||||
spec.RestoreStorage = v.Storage
|
||||
|
||||
lxc, err := e.api.ListLXC(ctx)
|
||||
if err != nil {
|
||||
res.Err = fmt.Errorf("reconcile: restore-test list guests: %w", err)
|
||||
@@ -164,7 +195,9 @@ func (e *Engine) RunRestoreTest(ctx context.Context, spec RestoreTestSpec) Resto
|
||||
// Serialize on the scratch VMID's lane (inherits §10), and capture the result.
|
||||
var vmidOccupied bool
|
||||
ch := e.queue.Submit(vmid, func() error {
|
||||
vmidOccupied = e.runScratchTest(ctx, vmid, spec, &res)
|
||||
e.markScratch(vmid, true)
|
||||
defer e.markScratch(vmid, false)
|
||||
vmidOccupied = e.runScratchTest(ctx, vmid, spec, rawCfg, &res)
|
||||
return res.Err
|
||||
})
|
||||
<-ch
|
||||
@@ -182,7 +215,7 @@ func (e *Engine) RunRestoreTest(ctx context.Context, spec RestoreTestSpec) Resto
|
||||
// runScratchTest is the journaled body (runs on vmid's queue lane). The occupied return is true
|
||||
// ONLY when PVE synchronously refused the restore because the vmid already holds a guest (one
|
||||
// the pool-blind band scan couldn't see) — the caller then advances to the next band vmid (F2).
|
||||
func (e *Engine) runScratchTest(ctx context.Context, vmid int, spec RestoreTestSpec, res *RestoreTestResult) (occupied bool) {
|
||||
func (e *Engine) runScratchTest(ctx context.Context, vmid int, spec RestoreTestSpec, rawCfg string, res *RestoreTestResult) (occupied bool) {
|
||||
base := JournalEntry{OpID: e.scratchOpID(vmid), VMID: vmid, Kind: scratchKind, Scratch: true}
|
||||
|
||||
// OWN the scratch guest's cleanup BEFORE any mutation. From here, a crash is recoverable.
|
||||
@@ -215,11 +248,7 @@ func (e *Engine) runScratchTest(ctx context.Context, vmid int, spec RestoreTestS
|
||||
// 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
|
||||
}
|
||||
// The archive's config was read ONCE, by the space preflight (R-672), and is passed in.
|
||||
mountOverrides, err := drRestoreOverrides(rawCfg, spec.RestoreStorage)
|
||||
if err != nil {
|
||||
res.Err = fmt.Errorf("reconcile: restore-test: %w", err)
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
package reconcile
|
||||
|
||||
import "context"
|
||||
|
||||
// ── A failed scratch teardown is retried on a TIMER, not only at agent start (R-672 rule 3) ────────
|
||||
//
|
||||
// MEASURED 2026-09-24 on demo-hp: the scheduled restore-test's teardown failed (`lvremove … contains a
|
||||
// filesystem in use`, a transient hold) and logged "left for Recover" — and Recover runs ONLY at agent
|
||||
// start, so the 22 GiB scratch guest sat in the full pool for 2.5 hours until an agent restart. The
|
||||
// timer calls RetryScratchTeardown every 10 minutes: the SAME resolution as Recover (recoverScratch —
|
||||
// the gate's benign scratch destroy, idempotent when the guest is already gone), restricted to Scratch
|
||||
// entries that carry a launch-proof UPID and that no running restore-test owns. After
|
||||
// MaxTeardownTries failed attempts for one entry the operator is told (the caller reports it); the
|
||||
// timer keeps trying.
|
||||
|
||||
// MaxTeardownTries is how many failed timer retries of one scratch entry happen before the operator
|
||||
// is told.
|
||||
const MaxTeardownTries = 3
|
||||
|
||||
// ScratchRetryResult summarizes one timer pass.
|
||||
type ScratchRetryResult struct {
|
||||
Examined int
|
||||
Destroyed int
|
||||
Clean int // already gone
|
||||
Failed int
|
||||
// GaveUp lists the scratch vmids whose failed tries reached MaxTeardownTries IN THIS PASS — each
|
||||
// is reported exactly once (the caller tells the operator).
|
||||
GaveUp []int
|
||||
}
|
||||
|
||||
func (e *Engine) markScratch(vmid int, active bool) {
|
||||
e.scratchMu.Lock()
|
||||
defer e.scratchMu.Unlock()
|
||||
if active {
|
||||
e.activeScratch[vmid] = true
|
||||
} else {
|
||||
delete(e.activeScratch, vmid)
|
||||
}
|
||||
}
|
||||
|
||||
func (e *Engine) scratchActive(vmid int) bool {
|
||||
e.scratchMu.Lock()
|
||||
defer e.scratchMu.Unlock()
|
||||
return e.activeScratch[vmid]
|
||||
}
|
||||
|
||||
// RetryScratchTeardown is the timer's pass. It never touches a non-Scratch entry (unlike Recover,
|
||||
// which also resolves generic in-flight operations and must therefore run only at start), never an
|
||||
// entry without a launch-proof UPID (nothing was created), and never a vmid a running test owns.
|
||||
func (e *Engine) RetryScratchTeardown(ctx context.Context) ScratchRetryResult {
|
||||
var out ScratchRetryResult
|
||||
if e.journal == nil {
|
||||
return out
|
||||
}
|
||||
for _, entry := range e.journal.InFlight() {
|
||||
if !entry.Scratch || entry.UPID == "" || e.scratchActive(entry.VMID) {
|
||||
continue
|
||||
}
|
||||
out.Examined++
|
||||
var r RecoverResult
|
||||
e.recoverScratch(ctx, entry, &r)
|
||||
switch {
|
||||
case r.ScratchDestroyed > 0:
|
||||
out.Destroyed++
|
||||
e.forgetTries(entry.OpID)
|
||||
case r.ScratchClean > 0:
|
||||
out.Clean++
|
||||
e.forgetTries(entry.OpID)
|
||||
default:
|
||||
out.Failed++
|
||||
n := e.addTry(entry.OpID)
|
||||
e.logger.Warn("restore-test: scratch teardown retry failed (timer)", "vmid", entry.VMID, "op_id", entry.OpID, "try", n)
|
||||
if n == MaxTeardownTries {
|
||||
out.GaveUp = append(out.GaveUp, entry.VMID)
|
||||
e.logger.Error("restore-test: scratch guest still NOT torn down after repeated retries — telling the operator",
|
||||
"vmid", entry.VMID, "tries", n)
|
||||
}
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
func (e *Engine) addTry(op string) int {
|
||||
e.scratchMu.Lock()
|
||||
defer e.scratchMu.Unlock()
|
||||
e.teardownTries[op]++
|
||||
return e.teardownTries[op]
|
||||
}
|
||||
|
||||
func (e *Engine) forgetTries(op string) {
|
||||
e.scratchMu.Lock()
|
||||
defer e.scratchMu.Unlock()
|
||||
delete(e.teardownTries, op)
|
||||
}
|
||||
@@ -0,0 +1,73 @@
|
||||
package reconcile
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
|
||||
)
|
||||
|
||||
// R-672 rule 3 (v0.133.0): a failed scratch teardown is retried on a TIMER. The consequence asserted:
|
||||
// the leaked scratch guest is destroyed by a timer pass (not only by a restart's Recover), the operator
|
||||
// is told exactly once after MaxTeardownTries failures, and a vmid a running test owns is never touched.
|
||||
|
||||
// leakScratch runs a restore-test whose teardown fails, leaving scratch 990000 in-flight — the
|
||||
// 2026-09-24 shape ("lvremove … contains a filesystem in use").
|
||||
func leakScratch(t *testing.T) (*Engine, *fakeAPI, *Journal) {
|
||||
t.Helper()
|
||||
api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{990000: scratchCfg()}, restoreUPID: "UPID:r", destroyErr: errors.New("lvremove: contains a filesystem in use")}
|
||||
e, j := spaceEngine(t, api, roomySpace{})
|
||||
e.RunRestoreTest(context.Background(), RestoreTestSpec{Archive: "local:backup/x.tar.zst", RestoreStorage: "local-lvm", ScratchMin: 990000, ScratchMax: 990009})
|
||||
if len(j.InFlight()) != 1 {
|
||||
t.Fatalf("setup: want the scratch left in-flight after a failed teardown, got %+v", j.InFlight())
|
||||
}
|
||||
api.lxc = []proxmox.Guest{{VMID: 990000}}
|
||||
return e, api, j
|
||||
}
|
||||
|
||||
// COMPANION RED-PROOF (REPORT): make RetryScratchTeardown return without touching the journal (the
|
||||
// v0.132.0 shape — only Recover at start resolved a leak) → "the leaked scratch was not destroyed by
|
||||
// the timer".
|
||||
func TestRetry_TheTimerDestroysALeakedScratch(t *testing.T) {
|
||||
e, api, j := leakScratch(t)
|
||||
api.destroyErr = nil // the transient hold is gone
|
||||
before := len(api.destroys)
|
||||
r := e.RetryScratchTeardown(context.Background())
|
||||
if r.Destroyed != 1 || len(api.destroys) != before+1 || api.destroys[len(api.destroys)-1] != 990000 {
|
||||
t.Fatalf("the leaked scratch was not destroyed by the timer: result=%+v destroys=%v", r, api.destroys)
|
||||
}
|
||||
if len(j.InFlight()) != 0 {
|
||||
t.Fatalf("the entry is still in flight after a successful retry: %+v", j.InFlight())
|
||||
}
|
||||
if r2 := e.RetryScratchTeardown(context.Background()); r2.Examined != 0 {
|
||||
t.Fatalf("a resolved entry was examined again: %+v", r2)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRetry_OperatorToldOnceAfterThreeFailures(t *testing.T) {
|
||||
e, _, _ := leakScratch(t)
|
||||
var gave [][]int
|
||||
for i := 0; i < MaxTeardownTries+2; i++ {
|
||||
gave = append(gave, e.RetryScratchTeardown(context.Background()).GaveUp)
|
||||
}
|
||||
for i, g := range gave {
|
||||
want := 0
|
||||
if i == MaxTeardownTries-1 {
|
||||
want = 1
|
||||
}
|
||||
if len(g) != want {
|
||||
t.Fatalf("pass %d gave up on %v — want the operator told exactly once, on pass %d", i+1, g, MaxTeardownTries)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestRetry_NeverTouchesARunningTest(t *testing.T) {
|
||||
e, api, _ := leakScratch(t)
|
||||
api.destroyErr = nil
|
||||
e.markScratch(990000, true) // a restore-test is (again) working on this vmid
|
||||
before := len(api.destroys)
|
||||
if r := e.RetryScratchTeardown(context.Background()); r.Examined != 0 || len(api.destroys) != before {
|
||||
t.Fatalf("the timer touched a scratch a running test owns: %+v destroys=%v", r, api.destroys)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,176 @@
|
||||
package reconcile
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"sort"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// ── The restore-test's space preflight (R-672, agent v0.133.0) ─────────────────────────────────────
|
||||
//
|
||||
// MEASURED 2026-09-24 on demo-hp: the scheduled restore-test restored 9201's archive into `local-lvm`
|
||||
// — the SAME thin pool that holds 9201 — with no free-space check. The pool reached 100 %
|
||||
// (`out_of_data_space`, `error_if_no_space`), and 9201's rootfs and data volume remounted READ-ONLY.
|
||||
// Evidence: felhom.eu `documentation/audits/night-2026-09-24/C-02…C-07`, `audits/r672-2026-09-24/`.
|
||||
//
|
||||
// THE THREE RULES, all decided here before ANY mutation (before the scratch entry is even journaled):
|
||||
// 1. SPACE FIRST. The target storage must have free data ≥ restored × factor + reserve (defaults 1.2 and
|
||||
// 5 GiB, `backup.restore_test_space_factor` / `backup.restore_test_space_reserve_gib`), and a thin
|
||||
// pool's metadata must have room for the same share. `restored` is the UNCOMPRESSED size — the
|
||||
// archive FILE is the wrong number: 9201's archive was 6.9 GB and its restore wrote 22.6 GB, so
|
||||
// "file × 1.2 + 5 GiB" (14.3 GB) would have let the 2026-09-24 test run into a pool with 23 GB free.
|
||||
// 2. KEEP OFF THE TESTED GUEST'S POOL when another eligible storage (active, takes `rootdir`, and the
|
||||
// agent holds Datastore.AllocateSpace on it) passes rule 1. With only one, rule 1 decides.
|
||||
// 3. UNKNOWN REFUSES. An unreadable size, an unreadable storage or an unknown thin-pool metadata fill is
|
||||
// a skip with its reason, never a guess — the fail-safe direction of every guard in this project.
|
||||
// A refusal is reported to the hub as the test's RESULT ("skipped: …", pass=false), never as a pass.
|
||||
// Pinned by restoretest_space_test.go.
|
||||
|
||||
// RestoreSpace is the preflight's seam onto the host. Production: internal/restorespace.
|
||||
type RestoreSpace interface {
|
||||
// RestoredBytes is how many bytes restoring `archive` will write (uncompressed), and where that
|
||||
// figure came from (for the log and the refusal).
|
||||
RestoredBytes(ctx context.Context, archive string) (bytes int64, source string, err error)
|
||||
// Free reports the storage's free data bytes and, for a thin pool, its metadata-used fraction.
|
||||
Free(ctx context.Context, storage string) (StorageFree, error)
|
||||
// Eligible lists the storages a restore-test may target: active, content `rootdir`, and the agent
|
||||
// holds Datastore.AllocateSpace there.
|
||||
Eligible(ctx context.Context) ([]string, error)
|
||||
}
|
||||
|
||||
// StorageFree is one storage's free space as the preflight judges it.
|
||||
type StorageFree struct {
|
||||
AvailBytes int64
|
||||
UsedBytes int64
|
||||
Thin bool
|
||||
// MetaUsedFraction is the thin pool's metadata use (0..1); MetaKnown false = could not be read.
|
||||
MetaUsedFraction float64
|
||||
MetaKnown bool
|
||||
}
|
||||
|
||||
// SpacePolicy is rule 1's margin.
|
||||
type SpacePolicy struct {
|
||||
Factor float64 // ≥ 1
|
||||
ReserveBytes int64
|
||||
}
|
||||
|
||||
// DefaultSpacePolicy is 1.2 × restored + 5 GiB.
|
||||
var DefaultSpacePolicy = SpacePolicy{Factor: 1.2, ReserveBytes: 5 << 30}
|
||||
|
||||
// SpaceVerdict is the preflight's answer.
|
||||
type SpaceVerdict struct {
|
||||
OK bool
|
||||
Storage string // the storage the restore goes to (when OK) or was judged (when not)
|
||||
Required int64
|
||||
Avail int64
|
||||
Reason string // empty when OK
|
||||
// Avoided is the tested guest's own storage, when rule 2 moved the restore off it.
|
||||
Avoided string
|
||||
}
|
||||
|
||||
// requiredBytes is rule 1's figure.
|
||||
func (p SpacePolicy) requiredBytes(restored int64) int64 {
|
||||
f := p.Factor
|
||||
if f < 1 {
|
||||
f = DefaultSpacePolicy.Factor
|
||||
}
|
||||
return int64(float64(restored)*f) + p.ReserveBytes
|
||||
}
|
||||
|
||||
// fits judges one storage against rule 1 (data AND thin metadata). An unknown metadata fill on a thin
|
||||
// pool refuses (rule 3).
|
||||
func fits(fr StorageFree, required int64) (bool, string) {
|
||||
if fr.AvailBytes < required {
|
||||
return false, fmt.Sprintf("needs %s free, has %s", gib(required), gib(fr.AvailBytes))
|
||||
}
|
||||
if fr.Thin {
|
||||
if !fr.MetaKnown {
|
||||
return false, "thin-pool metadata fill unknown"
|
||||
}
|
||||
// The metadata a restore of `required` bytes needs, in the pool's own proportion of metadata to
|
||||
// data. A pool with no data yet has no proportion to read → only the absolute ceiling applies.
|
||||
need := 0.0
|
||||
if fr.UsedBytes > 0 {
|
||||
need = fr.MetaUsedFraction * float64(required) / float64(fr.UsedBytes)
|
||||
}
|
||||
if fr.MetaUsedFraction+need > 0.9 {
|
||||
return false, fmt.Sprintf("thin-pool metadata would reach %.0f%% (now %.0f%%)", 100*(fr.MetaUsedFraction+need), 100*fr.MetaUsedFraction)
|
||||
}
|
||||
}
|
||||
return true, ""
|
||||
}
|
||||
|
||||
func gib(b int64) string { return fmt.Sprintf("%.1f GiB", float64(b)/(1<<30)) }
|
||||
|
||||
// sourceStorages returns the storage ids that hold the ARCHIVED guest's volumes (rootfs and every mpN
|
||||
// that names a `storage:volume`), read from the archive's own embedded config — the guest under test.
|
||||
// Bind mounts (a leading "/") carry no storage.
|
||||
func sourceStorages(rawCfg string) map[string]bool {
|
||||
out := map[string]bool{}
|
||||
for k, v := range archiveCurrentConfig(rawCfg) {
|
||||
if k != "rootfs" && !(strings.HasPrefix(k, "mp") && len(k) > 2 && k[2] >= '0' && k[2] <= '9') {
|
||||
continue
|
||||
}
|
||||
vol := strings.TrimSpace(strings.SplitN(strings.TrimSpace(v), ",", 2)[0])
|
||||
if vol == "" || strings.HasPrefix(vol, "/") {
|
||||
continue
|
||||
}
|
||||
if st, _, ok := strings.Cut(vol, ":"); ok && st != "" {
|
||||
out[st] = true
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// PreflightRestoreSpace applies the three rules. `configured` is `backup.restore_storage`.
|
||||
func PreflightRestoreSpace(ctx context.Context, space RestoreSpace, policy SpacePolicy, archive, rawCfg, configured string) SpaceVerdict {
|
||||
if space == nil {
|
||||
return SpaceVerdict{Storage: configured, Reason: "no space check is wired — refusing (fail-closed)"}
|
||||
}
|
||||
restored, src, err := space.RestoredBytes(ctx, archive)
|
||||
if err != nil || restored <= 0 {
|
||||
return SpaceVerdict{Storage: configured, Reason: fmt.Sprintf("cannot tell how much the restore writes (%v)", err)}
|
||||
}
|
||||
required := policy.requiredBytes(restored)
|
||||
own := sourceStorages(rawCfg)
|
||||
|
||||
// Rule 2: the configured storage holds the guest under test → try the others first.
|
||||
var order []string
|
||||
avoided := ""
|
||||
if own[configured] {
|
||||
eligible, eerr := space.Eligible(ctx)
|
||||
if eerr == nil {
|
||||
sort.Strings(eligible)
|
||||
for _, s := range eligible {
|
||||
if s != configured && !own[s] {
|
||||
order = append(order, s)
|
||||
}
|
||||
}
|
||||
}
|
||||
if len(order) > 0 {
|
||||
avoided = configured
|
||||
}
|
||||
}
|
||||
order = append(order, configured)
|
||||
|
||||
var last SpaceVerdict
|
||||
for _, s := range order {
|
||||
fr, ferr := space.Free(ctx, s)
|
||||
if ferr != nil {
|
||||
last = SpaceVerdict{Storage: s, Required: required, Reason: fmt.Sprintf("cannot read free space on %s (%v)", s, ferr)}
|
||||
continue
|
||||
}
|
||||
ok, why := fits(fr, required)
|
||||
v := SpaceVerdict{OK: ok, Storage: s, Required: required, Avail: fr.AvailBytes}
|
||||
if ok {
|
||||
if s != configured {
|
||||
v.Avoided = avoided
|
||||
}
|
||||
return v
|
||||
}
|
||||
v.Reason = fmt.Sprintf("not enough space on %s: restoring %s (%s) %s", s, gib(restored), src, why)
|
||||
last = v
|
||||
}
|
||||
return last
|
||||
}
|
||||
@@ -0,0 +1,156 @@
|
||||
package reconcile
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
|
||||
)
|
||||
|
||||
// R-672 (v0.133.0) — the restore-test's space preflight. Every test asserts the CONSEQUENCE: whether the
|
||||
// Proxmox API was asked to restore anything, where to, and what the result says — never only the verdict.
|
||||
|
||||
const gb = int64(1000 * 1000 * 1000)
|
||||
|
||||
// fakeSpace is a configurable RestoreSpace.
|
||||
type fakeSpace struct {
|
||||
restored int64
|
||||
restoredErr error
|
||||
free map[string]StorageFree
|
||||
freeErr map[string]error
|
||||
eligible []string
|
||||
}
|
||||
|
||||
func (f fakeSpace) RestoredBytes(context.Context, string) (int64, string, error) {
|
||||
return f.restored, "fake", f.restoredErr
|
||||
}
|
||||
func (f fakeSpace) Free(_ context.Context, s string) (StorageFree, error) {
|
||||
if err := f.freeErr[s]; err != nil {
|
||||
return StorageFree{}, err
|
||||
}
|
||||
fr, ok := f.free[s]
|
||||
if !ok {
|
||||
return StorageFree{}, errors.New("unknown storage")
|
||||
}
|
||||
return fr, nil
|
||||
}
|
||||
func (f fakeSpace) Eligible(context.Context) ([]string, error) { return f.eligible, nil }
|
||||
|
||||
// thin9201 is demo-hp's local-lvm at 10:29 on 2026-09-24, just before the restore-test that filled it:
|
||||
// 23.2 GB free, 33.3 GB used, metadata 2.65 %.
|
||||
var thin9201 = StorageFree{AvailBytes: 23210892 * 1024, UsedBytes: 33277043 * 1024, Thin: true, MetaUsedFraction: 0.0265, MetaKnown: true}
|
||||
|
||||
// archive9201 is 9201's archive config: both volumes on local-lvm.
|
||||
const archive9201 = "hostname: demo-hp\nrootfs: local-lvm:vm-9201-disk-0,size=32G\nmp0: local-lvm:vm-9201-disk-1,mp=/var/lib/felhom,backup=1,size=70G\nmp8: /mnt/felhom-drives,mp=/mnt/felhom-drives\n"
|
||||
|
||||
func spaceEngine(t *testing.T, api *fakeAPI, sp RestoreSpace) (*Engine, *Journal) {
|
||||
t.Helper()
|
||||
j, err := OpenJournal(filepath.Join(t.TempDir(), "journal.log"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Cleanup(func() { j.Close() })
|
||||
q := NewQueue()
|
||||
t.Cleanup(q.Close)
|
||||
return NewEngine(EngineOptions{API: api, Queue: q, Journal: j, RestoreSpace: sp}), j
|
||||
}
|
||||
|
||||
func run9201(e *Engine) RestoreTestResult {
|
||||
return e.RunRestoreTest(context.Background(), RestoreTestSpec{
|
||||
Archive: "local:backup/vzdump-lxc-9201-2026_09_23-06_55_25.tar.zst", RestoreStorage: "local-lvm",
|
||||
ScratchMin: 990000, ScratchMax: 990009, SourceTier: "local",
|
||||
})
|
||||
}
|
||||
|
||||
// TestSpace_The2026_09_24TestIsRefused replays R-672: 9201's archive restores 22.6 GB (its vzdump log),
|
||||
// the pool has 23.2 GB free. The test must NOT start — no restore call, no journaled scratch — and the
|
||||
// result must say why, as a non-pass.
|
||||
//
|
||||
// COMPANION RED-PROOFS (REPORT): (1) the preflight removed (v0.132.0's shape) → a restore into local-lvm
|
||||
// is issued; (2) `restored` taken from the archive FILE (6.9 GB, the brief's "archive × 1.2 + 5 GiB") →
|
||||
// 6.9×1.2+5.4 = 13.7 GB < 23.2 GB free, so the test starts — the defect the uncompressed size exists for.
|
||||
func TestSpace_The2026_09_24TestIsRefused(t *testing.T) {
|
||||
api := &fakeAPI{extractCfg: archive9201, cfg: map[int]proxmox.GuestConfig{990000: scratchCfg()}}
|
||||
e, j := spaceEngine(t, api, fakeSpace{restored: 22607360000, free: map[string]StorageFree{"local-lvm": thin9201}})
|
||||
res := run9201(e)
|
||||
if len(api.restores) != 0 {
|
||||
t.Fatalf("a restore was issued into a pool that cannot take it: %+v", api.restores)
|
||||
}
|
||||
if len(j.InFlight()) != 0 {
|
||||
t.Fatalf("a scratch entry was journaled for a test that must not start: %+v", j.InFlight())
|
||||
}
|
||||
if res.Pass || !res.Skipped || !strings.Contains(res.SkipReason, "not enough space on local-lvm") {
|
||||
t.Fatalf("result = pass=%v skipped=%v reason=%q — want a non-pass skip naming the storage", res.Pass, res.Skipped, res.SkipReason)
|
||||
}
|
||||
if res.RequiredBytes < 32*gb || res.AvailBytes != thin9201.AvailBytes {
|
||||
t.Fatalf("required=%d avail=%d — want ≥ 32 GB required (22.6 × 1.2 + 5 GiB) against 23.2 GB", res.RequiredBytes, res.AvailBytes)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSpace_KeepsOffTheTestedGuestsPool — rule 2: another eligible storage that fits takes the restore.
|
||||
func TestSpace_KeepsOffTheTestedGuestsPool(t *testing.T) {
|
||||
api := &fakeAPI{extractCfg: archive9201, cfg: map[int]proxmox.GuestConfig{990000: scratchCfg()}}
|
||||
e, _ := spaceEngine(t, api, fakeSpace{restored: 22607360000, eligible: []string{"local-lvm", "big-dir"},
|
||||
free: map[string]StorageFree{"local-lvm": {AvailBytes: 900 * gb, UsedBytes: 10 * gb, Thin: true, MetaKnown: true}, "big-dir": {AvailBytes: 500 * gb}}})
|
||||
res := run9201(e)
|
||||
if len(api.restores) != 1 || api.restores[0].Storage != "big-dir" {
|
||||
t.Fatalf("restores = %+v — want ONE restore onto big-dir, off 9201's own pool", api.restores)
|
||||
}
|
||||
if res.TargetStorage != "big-dir" {
|
||||
t.Fatalf("target=%q", res.TargetStorage)
|
||||
}
|
||||
for k, v := range api.restores[0].MountOverrides {
|
||||
if strings.HasPrefix(v, "local-lvm:") {
|
||||
t.Fatalf("%s still lands on the tested guest's pool: %s", k, v)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestSpace_OnlyOnePool_RuleOneDecides — no other eligible storage: the tested guest's pool is used when it
|
||||
// fits (demo-hp's real shape: nvme-scratch takes rootdir but the agent holds no AllocateSpace there).
|
||||
func TestSpace_OnlyOnePool_RuleOneDecides(t *testing.T) {
|
||||
api := &fakeAPI{extractCfg: archive9201, cfg: map[int]proxmox.GuestConfig{990000: scratchCfg()}}
|
||||
e, _ := spaceEngine(t, api, fakeSpace{restored: 2 * gb, eligible: []string{"local-lvm"}, free: map[string]StorageFree{"local-lvm": thin9201}})
|
||||
res := run9201(e)
|
||||
if len(api.restores) != 1 || api.restores[0].Storage != "local-lvm" || res.Skipped || res.TargetStorage != "local-lvm" {
|
||||
t.Fatalf("restores=%+v skipped=%v — a 2 GB restore fits 23 GB free on the only pool", api.restores, res.Skipped)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSpace_UnknownRefuses — rule 3, one case per unknown. Nothing is restored in any of them.
|
||||
func TestSpace_UnknownRefuses(t *testing.T) {
|
||||
cases := map[string]RestoreSpace{
|
||||
"no space check wired": nil,
|
||||
"restore size unknown": fakeSpace{restoredErr: errors.New("no vzdump log"), free: map[string]StorageFree{"local-lvm": thin9201}},
|
||||
"free space unreadable": fakeSpace{restored: gb, freeErr: map[string]error{"local-lvm": errors.New("api down")}},
|
||||
"thin metadata unknown": fakeSpace{restored: gb, free: map[string]StorageFree{"local-lvm": {AvailBytes: 900 * gb, UsedBytes: gb, Thin: true}}},
|
||||
"metadata would overrun": fakeSpace{restored: 10 * gb, free: map[string]StorageFree{"local-lvm": {AvailBytes: 900 * gb, UsedBytes: 10 * gb, Thin: true, MetaUsedFraction: 0.5, MetaKnown: true}}},
|
||||
}
|
||||
for name, sp := range cases {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
api := &fakeAPI{extractCfg: archive9201, cfg: map[int]proxmox.GuestConfig{990000: scratchCfg()}}
|
||||
var e *Engine
|
||||
if sp == nil {
|
||||
e, _ = spaceEngine(t, api, nil)
|
||||
} else {
|
||||
e, _ = spaceEngine(t, api, sp)
|
||||
}
|
||||
res := run9201(e)
|
||||
if len(api.restores) != 0 || res.Pass || !res.Skipped || res.SkipReason == "" {
|
||||
t.Fatalf("restores=%d pass=%v skipped=%v reason=%q — an unknown must refuse before anything moves",
|
||||
len(api.restores), res.Pass, res.Skipped, res.SkipReason)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestSpace_SourceStorages reads the tested guest's pools from the ARCHIVE's config, binds excluded.
|
||||
func TestSpace_SourceStorages(t *testing.T) {
|
||||
got := sourceStorages(archive9201 + "mp1: other:vm-9201-disk-2,mp=/x,size=1G\n[snap]\nrootfs: snapstore:x\n")
|
||||
if !got["local-lvm"] || !got["other"] || got["snapstore"] || len(got) != 2 {
|
||||
t.Fatalf("sourceStorages = %v — want local-lvm + other, binds and snapshot sections excluded", got)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,16 @@
|
||||
package reconcile
|
||||
|
||||
import "context"
|
||||
|
||||
// roomySpace is the permissive RestoreSpace the pre-R-672 restore-test tests run with: 1 GiB restored,
|
||||
// 1 TiB free, thin metadata known and low. The space rules themselves are pinned in
|
||||
// restoretest_space_test.go.
|
||||
type roomySpace struct{}
|
||||
|
||||
func (roomySpace) RestoredBytes(context.Context, string) (int64, string, error) {
|
||||
return 1 << 30, "test", nil
|
||||
}
|
||||
func (roomySpace) Free(context.Context, string) (StorageFree, error) {
|
||||
return StorageFree{AvailBytes: 1 << 40, UsedBytes: 1 << 30, Thin: true, MetaUsedFraction: 0.01, MetaKnown: true}, nil
|
||||
}
|
||||
func (roomySpace) Eligible(context.Context) ([]string, error) { return nil, nil }
|
||||
Reference in New Issue
Block a user