agent: a whole-box backup that cannot fit its local target is skipped with a reason before anything starts (R-685)
gates / gates (push) Successful in 13s
gates / gates (push) Successful in 13s
Free space is read from GET /nodes/<node>/storage — GET /storage carries no usage (found live, before release). 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:
@@ -24,11 +24,13 @@ type fakeBackupAPI struct {
|
|||||||
cfgErr error
|
cfgErr error
|
||||||
content []proxmox.StorageContent
|
content []proxmox.StorageContent
|
||||||
contentErr error
|
contentErr error
|
||||||
storages []proxmox.Storage // returned by ListStorage (the local-prune scope gate)
|
storages []proxmox.Storage // returned by ListStorage (the local-prune scope gate) — DEFINITIONS: like
|
||||||
storageErr error
|
// production's GET /storage, it never carries usage; ListStorage strips Avail/Used (R-685's live lesson)
|
||||||
vzdumps []proxmox.VzdumpOptions
|
nodeStorages []proxmox.Storage // returned by NodeStorage (GET /nodes/{node}/storage — WITH usage)
|
||||||
logLines []string // returned by TaskLogTail (e.g. "INFO: backup mode: stop")
|
storageErr error
|
||||||
waitGate chan struct{} // if non-nil, WaitTask blocks until closed (8B.2 watcher timing)
|
vzdumps []proxmox.VzdumpOptions
|
||||||
|
logLines []string // returned by TaskLogTail (e.g. "INFO: backup mode: stop")
|
||||||
|
waitGate chan struct{} // if non-nil, WaitTask blocks until closed (8B.2 watcher timing)
|
||||||
}
|
}
|
||||||
|
|
||||||
func (f *fakeBackupAPI) Vzdump(_ context.Context, o proxmox.VzdumpOptions) (string, error) {
|
func (f *fakeBackupAPI) Vzdump(_ context.Context, o proxmox.VzdumpOptions) (string, error) {
|
||||||
@@ -48,6 +50,17 @@ func (f *fakeBackupAPI) StorageContent(_ context.Context, _ string) ([]proxmox.S
|
|||||||
return f.content, f.contentErr
|
return f.content, f.contentErr
|
||||||
}
|
}
|
||||||
func (f *fakeBackupAPI) ListStorage(_ context.Context) ([]proxmox.Storage, error) {
|
func (f *fakeBackupAPI) ListStorage(_ context.Context) ([]proxmox.Storage, error) {
|
||||||
|
out := make([]proxmox.Storage, len(f.storages))
|
||||||
|
for i, s := range f.storages {
|
||||||
|
s.Avail, s.Used, s.Total = 0, 0, 0 // GET /storage has no usage — a fake that had it hid R-685's defect
|
||||||
|
out[i] = s
|
||||||
|
}
|
||||||
|
return out, f.storageErr
|
||||||
|
}
|
||||||
|
func (f *fakeBackupAPI) NodeStorage(_ context.Context) ([]proxmox.Storage, error) {
|
||||||
|
if f.nodeStorages != nil {
|
||||||
|
return f.nodeStorages, f.storageErr
|
||||||
|
}
|
||||||
return f.storages, f.storageErr
|
return f.storages, f.storageErr
|
||||||
}
|
}
|
||||||
func (f *fakeBackupAPI) TaskLogTail(_ context.Context, _ string, _ int) ([]string, error) {
|
func (f *fakeBackupAPI) TaskLogTail(_ context.Context, _ string, _ int) ([]string, error) {
|
||||||
|
|||||||
@@ -0,0 +1,83 @@
|
|||||||
|
package backup
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
|
||||||
|
)
|
||||||
|
|
||||||
|
// R-685 (v0.134.0) — a whole-box backup that cannot fit its LOCAL target is a named skip BEFORE anything
|
||||||
|
// starts, with the numbers in the record's Error, never a vzdump that fills the disk and fails.
|
||||||
|
|
||||||
|
const gib = int64(1) << 30
|
||||||
|
|
||||||
|
func spaceAPI(avail int64, lastArchive int64, typ string) *fakeBackupAPI {
|
||||||
|
api := &fakeBackupAPI{vzdumpUPID: "UPID:vzdump:1",
|
||||||
|
storages: []proxmox.Storage{{Storage: "local", Type: typ, Content: "backup", Avail: avail}}}
|
||||||
|
if lastArchive > 0 {
|
||||||
|
api.content = []proxmox.StorageContent{{VolID: "local:backup/vzdump-lxc-9201-2026_09_24-21_59_25.tar.zst",
|
||||||
|
Content: "backup", VMID: 9201, Size: lastArchive, CTime: 1790280000}}
|
||||||
|
}
|
||||||
|
return api
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestR685_BackupThatCannotFitIsSkipped — demo-hp's shape: an 8.2 GB archive, 4 GiB free. No vzdump is
|
||||||
|
// started; the record says why, with the numbers, under a stable prefix.
|
||||||
|
//
|
||||||
|
// COMPANION RED-PROOF (REPORT.md): drop the spaceFits call from backup() — this test fails at "a vzdump
|
||||||
|
// was started on a target that cannot hold it".
|
||||||
|
func TestR685_BackupThatCannotFitIsSkipped(t *testing.T) {
|
||||||
|
api := spaceAPI(4*gib, 8182759056, "dir")
|
||||||
|
r := NewBackupRunner(api, "local", proxmox.ModeSnapshot, "", "keep-last=1", quiet())
|
||||||
|
rec, err := r.Backup(context.Background(), 9201)
|
||||||
|
if len(api.vzdumps) != 0 {
|
||||||
|
t.Fatalf("a vzdump was started on a target that cannot hold it: %+v", api.vzdumps)
|
||||||
|
}
|
||||||
|
if err == nil || rec.Success || !strings.HasPrefix(rec.Error, BackupSkipNoSpacePrefix) {
|
||||||
|
t.Fatalf("want a named skip, got err=%v rec=%+v", err, rec)
|
||||||
|
}
|
||||||
|
for _, want := range []string{"4.0 GiB free", "7.6 GiB", "10.5 GiB"} {
|
||||||
|
if !strings.Contains(rec.Error, want) {
|
||||||
|
t.Errorf("the reason must carry the numbers (%q missing): %s", want, rec.Error)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestR685_BackupThatFitsRuns — the same archive with 16 GiB free (demo-hp after tonight's prune) runs.
|
||||||
|
func TestR685_BackupThatFitsRuns(t *testing.T) {
|
||||||
|
api := spaceAPI(16*gib, 8182759056, "dir")
|
||||||
|
r := NewBackupRunner(api, "local", proxmox.ModeSnapshot, "", "keep-last=1", quiet())
|
||||||
|
_, _ = r.Backup(context.Background(), 9201)
|
||||||
|
if len(api.vzdumps) != 1 {
|
||||||
|
t.Fatalf("a backup that fits must run, vzdumps=%d", len(api.vzdumps))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestR685_FailsOpen — never refuse on what is not KNOWN: a PBS target, a first backup (no archive to
|
||||||
|
// size from), an unknown free figure, or a storage list that cannot be read.
|
||||||
|
func TestR685_FailsOpen(t *testing.T) {
|
||||||
|
cases := map[string]*fakeBackupAPI{
|
||||||
|
"pbs target": spaceAPI(1*gib, 8*gib, "pbs"),
|
||||||
|
"first backup": spaceAPI(1*gib, 0, "dir"),
|
||||||
|
"avail unknown": spaceAPI(0, 8*gib, "dir"),
|
||||||
|
"storage list error": func() *fakeBackupAPI {
|
||||||
|
a := spaceAPI(1*gib, 8*gib, "dir")
|
||||||
|
a.storageErr = context.DeadlineExceeded
|
||||||
|
return a
|
||||||
|
}(),
|
||||||
|
}
|
||||||
|
for name, api := range cases {
|
||||||
|
target := "local"
|
||||||
|
if name == "pbs target" {
|
||||||
|
api.storages[0].Storage = "felhom-pbs"
|
||||||
|
target = "felhom-pbs"
|
||||||
|
}
|
||||||
|
r := NewBackupRunner(api, target, proxmox.ModeSnapshot, "", "", quiet())
|
||||||
|
_, _ = r.Backup(context.Background(), 9201)
|
||||||
|
if len(api.vzdumps) != 1 {
|
||||||
|
t.Errorf("%s: the preflight must fail OPEN, but no vzdump ran", name)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -22,6 +22,10 @@ type BackupAPI interface {
|
|||||||
StorageContent(ctx context.Context, store string) ([]proxmox.StorageContent, error)
|
StorageContent(ctx context.Context, store string) ([]proxmox.StorageContent, error)
|
||||||
// ListStorage enumerates storages (name+type) — used to scope local-only retention (never prune PBS).
|
// ListStorage enumerates storages (name+type) — used to scope local-only retention (never prune PBS).
|
||||||
ListStorage(ctx context.Context) ([]proxmox.Storage, error)
|
ListStorage(ctx context.Context) ([]proxmox.Storage, error)
|
||||||
|
// NodeStorage is GET /nodes/{node}/storage — the storages WITH live usage (avail/used). R-685's space
|
||||||
|
// preflight reads free space HERE: ListStorage (GET /storage) is the cluster DEFINITIONS and carries no
|
||||||
|
// usage at all — measured live 2026-09-24 on demo-hp, where reading it let a backup through.
|
||||||
|
NodeStorage(ctx context.Context) ([]proxmox.Storage, error)
|
||||||
// TaskLogTail reads trailing task-log lines — used to read the ACTUAL vzdump mode
|
// TaskLogTail reads trailing task-log lines — used to read the ACTUAL vzdump mode
|
||||||
// (PVE may downgrade a requested snapshot to stop for a stopped guest — spike B1).
|
// (PVE may downgrade a requested snapshot to stop for a stopped guest — spike B1).
|
||||||
TaskLogTail(ctx context.Context, upid string, limit int) ([]string, error)
|
TaskLogTail(ctx context.Context, upid string, limit int) ([]string, error)
|
||||||
@@ -170,6 +174,16 @@ func (r *BackupRunner) backup(ctx context.Context, vmid int, onSnapshot func())
|
|||||||
rec.UncoveredVolumes = []string{}
|
rec.UncoveredVolumes = []string{}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// R-685 (v0.134.0): will the new archive FIT on a local target? Asked before anything runs, so a
|
||||||
|
// target that cannot hold it is a named SKIP with the numbers, not a nightly "No space left on
|
||||||
|
// device" that only the vzdump log explains (demo-hp, every night from 2026-09-23 — R-684).
|
||||||
|
if ok, why := r.spaceFits(ctx, vmid); !ok {
|
||||||
|
rec.Error = BackupSkipNoSpacePrefix + why
|
||||||
|
rec.DurationSeconds = time.Since(start).Seconds()
|
||||||
|
r.logger.Warn("backup SKIPPED by the space preflight (R-685) — nothing was started", "vmid", vmid, "target", r.target, "reason", why)
|
||||||
|
return rec, fmt.Errorf("backup: %s", rec.Error)
|
||||||
|
}
|
||||||
|
|
||||||
upid, err := r.api.Vzdump(ctx, proxmox.VzdumpOptions{
|
upid, err := r.api.Vzdump(ctx, proxmox.VzdumpOptions{
|
||||||
VMID: vmid, Storage: r.target, Mode: r.mode, Notes: r.notes,
|
VMID: vmid, Storage: r.target, Mode: r.mode, Notes: r.notes,
|
||||||
PruneBackups: r.localPruneSpec(ctx), // local target → keep-last=N; PBS/unknown → "" (no prune)
|
PruneBackups: r.localPruneSpec(ctx), // local target → keep-last=N; PBS/unknown → "" (no prune)
|
||||||
@@ -217,6 +231,56 @@ func (r *BackupRunner) backup(ctx context.Context, vmid int, onSnapshot func())
|
|||||||
return rec, nil
|
return rec, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// BackupSkipNoSpacePrefix starts a backup record's Error when the space preflight refused (R-685) — a
|
||||||
|
// stable prefix the controller's page and the hub can key on.
|
||||||
|
const BackupSkipNoSpacePrefix = "skipped: not enough space: "
|
||||||
|
|
||||||
|
// Space preflight margins (R-685): the new archive is predicted as the newest archive of this guest on the
|
||||||
|
// target × backupSpaceGrowth, plus backupSpaceFloorBytes of headroom for the host. MEASURED 2026-09-24:
|
||||||
|
// demo-hp 9201's archives grew 5.8 → 6.2 → 6.9 → 7.6 GB in four nights (+10 % a night at worst), so 1.25
|
||||||
|
// covers two nights' growth. PVE prunes old archives only AFTER a successful backup, so the free space
|
||||||
|
// must hold the new archive while every kept one still exists.
|
||||||
|
const (
|
||||||
|
backupSpaceGrowth = 1.25
|
||||||
|
backupSpaceFloorBytes = int64(1) << 30
|
||||||
|
)
|
||||||
|
|
||||||
|
// spaceFits answers whether a new archive of vmid fits on a LOCAL (non-PBS) target. It FAILS OPEN — a
|
||||||
|
// backup is the thing being protected, so an unreadable storage, an unknown type or a first backup (no
|
||||||
|
// previous archive to size from) proceeds and says so; only a POSITIVE "it does not fit" refuses.
|
||||||
|
func (r *BackupRunner) spaceFits(ctx context.Context, vmid int) (bool, string) {
|
||||||
|
// NodeStorage, never ListStorage: only the node view carries avail (see BackupAPI.NodeStorage).
|
||||||
|
stores, err := r.api.NodeStorage(ctx)
|
||||||
|
if err != nil {
|
||||||
|
r.logger.Warn("backup: space preflight could not read storage usage — proceeding (fail-open)", "target", r.target, "err", err)
|
||||||
|
return true, ""
|
||||||
|
}
|
||||||
|
var st *proxmox.Storage
|
||||||
|
for i := range stores {
|
||||||
|
if stores[i].Storage == r.target {
|
||||||
|
st = &stores[i]
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if st == nil || st.Type == "pbs" || st.Avail <= 0 {
|
||||||
|
return true, "" // PBS dedups and has its own lifecycle; an unknown avail never refuses
|
||||||
|
}
|
||||||
|
_, last, err := r.latestArchive(ctx, vmid)
|
||||||
|
if err != nil || last <= 0 {
|
||||||
|
r.logger.Info("backup: space preflight has no previous archive to size from — proceeding", "vmid", vmid, "target", r.target)
|
||||||
|
return true, ""
|
||||||
|
}
|
||||||
|
need := int64(float64(last)*backupSpaceGrowth) + backupSpaceFloorBytes
|
||||||
|
if st.Avail >= need {
|
||||||
|
r.logger.Info("backup: space preflight passed", "vmid", vmid, "target", r.target, "last_archive_bytes", last, "need_bytes", need, "avail_bytes", st.Avail)
|
||||||
|
return true, ""
|
||||||
|
}
|
||||||
|
return false, fmt.Sprintf("%s has %s free; the last archive of guest %d was %s, so a new one needs about %s (old archives are removed only after a successful backup)",
|
||||||
|
r.target, humanGiB(st.Avail), vmid, humanGiB(last), humanGiB(need))
|
||||||
|
}
|
||||||
|
|
||||||
|
func humanGiB(b int64) string { return fmt.Sprintf("%.1f GiB", float64(b)/(1<<30)) }
|
||||||
|
|
||||||
// watchForSnapshot polls the running backup's task log until it sees the storage-snapshot marker
|
// watchForSnapshot polls the running backup's task log until it sees the storage-snapshot marker
|
||||||
// (→ onSnapshot once) or the requested mode is reported as `stop` (→ downgraded; the marker will
|
// (→ onSnapshot once) or the requested mode is reported as `stop` (→ downgraded; the marker will
|
||||||
// never come, so stop watching) or ctx is cancelled (backup finished). Best-effort: a log-read
|
// never come, so stop watching) or ctx is cancelled (backup finished). Best-effort: a log-read
|
||||||
|
|||||||
Reference in New Issue
Block a user