From a2f3aeeeff9a99e05100450f2945e2de2cf6b70d Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Wed, 14 Oct 2026 11:29:41 +0200 Subject: [PATCH] After a power cut: remove the dead half-written backup (R-1006); a backup task from before the boot is not in flight (R-1008) --- REPORT.md | 48 ++-- cmd/felhom-agent/main.go | 27 +++ configs/felhom-agent.sudoers | 10 +- configs/felhom-os-apply | 2 + configs/felhom-vzdump-partial-clean | 99 +++++++++ configs/test_felhom_vzdump_partial_clean.py | 198 +++++++++++++++++ internal/backup/r685_space_test.go | 25 +++ internal/backup/runner.go | 15 +- internal/capability/manifest.go | 1 + internal/capability/r861_injection_test.go | 9 + .../localapi/controllersupervisor_test.go | 9 +- internal/localapi/guestbind.go | 2 +- internal/localapi/server.go | 9 +- internal/localapi/stalelock.go | 73 +++++- internal/localapi/stalelock_inflight_test.go | 195 ++++++++++++++++ internal/localapi/vzpartial_sweep.go | 33 +++ internal/vzpartial/vzpartial.go | 208 ++++++++++++++++++ internal/vzpartial/vzpartial_test.go | 206 +++++++++++++++++ 18 files changed, 1129 insertions(+), 40 deletions(-) create mode 100755 configs/felhom-vzdump-partial-clean create mode 100644 configs/test_felhom_vzdump_partial_clean.py create mode 100644 internal/localapi/stalelock_inflight_test.go create mode 100644 internal/localapi/vzpartial_sweep.go create mode 100644 internal/vzpartial/vzpartial.go create mode 100644 internal/vzpartial/vzpartial_test.go diff --git a/REPORT.md b/REPORT.md index 7166bbb..1d0eac2 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,32 +1,30 @@ -# REPORT — v0.155.0 released and delivered: the household can turn the machine off (2026-10-13) +# REPORT — v0.156.0: after a power cut the box clears up (R-1006, R-1008) — built, not yet released (2026-10-14) -Baseline `86adbca` (v0.154.0). Code `3403c87`, `f38b03c`, `c6d9435`, `bdf25f0`; release entry `68a2130`; tag `v0.155.0`. -Task: „The turn the machine off button" (R-998). Design: felhom.eu `documentation/architecture/14-host-power-off.md`. +Evidence: felhom.eu `documentation/audits/power-cut-fixes-2026-10-14/` — the spike (C1 the night's leftover, C2 a killed +worker, C3 a real power cut, C4 a full disk), the 17 red-proofs. -## What it does +## What the spike showed -`POST /host/poweroff` (local API, token-scoped): record → hub told (a report carrying `deliberate_off`, accepted) → -`pct shutdown --timeout 120` → `systemctl poweroff`. Refused in the first 10 min after boot, a second time in a -boot, while any other guest or VM runs, and while any backup/restore/OS/controller-update/drive/escrow job or a vzdump -runs. After acceptance the heavy gate is held; before each step the jobs and the guests are read again (wait ≤ 15 min, -or cancel). Every check fails closed. Sudoers: two exact vectors (`FELHOM_POWEROFF`). +- A power cut mid-backup leaves `vzdump-lxc--.tar.dat` + `.tmp/` (root, in `/var/lib/vz/dump`); Proxmox never + lists it as a backup; nothing removes it; every later backup is skipped „not enough space". +- The dead vzdump task stays in Proxmox's active list ~30–60 s after the boot — long enough for the agent's start-up pass + to call it „genuinely in flight" (R-1008). Short, so R-1008's harm is small. +- A killed worker leaves `tar`/`zstd` writing into the `.tar.dat` for minutes, none named „vzdump" — the reason the root + helper checks open files, not only process names. +- On „No space left" vzdump removes its own partial (both paths): the preflight's sizing stays. But a full host root breaks + LVM's own metadata writes (a half-removed snapshot; the janitor cannot clear it — new row). -## Proof +## What changed -- Tests: `internal/localapi/hostpoweroff_test.go` (11), `internal/poweroff/state_test.go`, `internal/hub/deliberate_off_test.go`, - `internal/capability` (9 injection cases, new `TestEveryAliasIsGranted`). Each guard red-proofed by removing it. -- Three security-review passes the same evening; every finding fixed and red-proofed (other guests, gate not held, - fail-open, jobs and guests starting after acceptance). -- Fixed without a row: an escrow-ceremony reply data race (`go test -race` red before, green 3/3 after). -- Live, real sudo on the N100 after the bundle: the two new vectors ALLOWED; `poweroff --force`, `-ff`, `reboot`, `halt`, - `pct shutdown` with `--forceStop` or without `--timeout` refused. -- Live press on the N100 through the dashboard endpoint: 202 at 18:52:34Z, no ping 18:53:02Z, 0 W from 18:53:07Z. -- Back on: power button 21:48:43Z, clean boot (crash guard `last_boot_unclean: false`); the first report carried no - `deliberate_off`, and the hub ended the hold at 21:49:22Z (`host_deliberate_off_ended`). No crash or recovered mail. +See `CHANGELOG.md` v0.156.0: `vzdumpInFlight` behind the one `BackupRunning`; `internal/vzpartial` at start-up and before +each backup; the root helper `felhom-vzdump-partial-clean` + one sudoers line + the bundle entry. -## Delivered +## Gates -Signed `agent_update` 0.155.0 + `agent_config_update` (bundle `795f2ab4…`) to demo-felhom, demo-hp, Tester 1. N100: -committed 18:37:45Z, bundle installed 18:51:54Z (capability probe 70/70). **Not vouched** — the operator's act. -demo-hp and Tester 1: agent 0.155.0, bundle installed — real `sudo -l` checks there allow the two lines and refuse -`--force`, `--forceStop 1`, `reboot` (`felhom.eu/documentation/audits/poweroff-2026-10-13/B-sudo-live-*.txt`). +`go build ./... && go vet` clean; `go test ./... -count=1` rc 0; `agent_gates.py` all OK; `visudo -cf` parsed OK; the +config bundle tests (55) and the helper's tests (21) pass. 17 mutations, 17 red. + +## Next + +Release (tag, publish, step bundle), the operator's vouch + floor, then the live proof on demo-hp (a cut mid-backup) and +delivery to Tester 1 and the N100. diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index d2b8791..04989e9 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -59,6 +59,7 @@ import ( "gitea.dooplex.hu/admin/felhom-agent/internal/selfupdate" "gitea.dooplex.hu/admin/felhom-agent/internal/signedjobs" "gitea.dooplex.hu/admin/felhom-agent/internal/storage" + "gitea.dooplex.hu/admin/felhom-agent/internal/vzpartial" "gitea.dooplex.hu/admin/felhom-agent/internal/wgtunnel" ) @@ -1532,6 +1533,8 @@ func runDaemon(cfg config.Config, logger *slog.Logger, logRing *applog.Ring) int // the dangling snapshot → start iff onboot). Runs BEFORE the backup loop starts, so a present // backup lock is stale by definition (guarded by a no-vzdump-running check; fail-safe otherwise). localSrv.RecoverStaleLockedGuests(ctx) + // R-1006: then remove a dead half-written backup a power cut left (the lock recovery first: same order as a human). + localSrv.SweepVzdumpPartials(ctx) // F-REBOOT: the startup recovery above only covers a guest left LOCKED by an interrupted // backup. A guest that simply ends up stopped-and-unlocked (a `pct reboot` whose shutdown // half completed and whose start half never fired — Campaign 8 fault 11, 9m47s of total @@ -1892,6 +1895,8 @@ func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.St } apiTiers := make([]localapi.BackupTier, 0, len(backupTiers)) var runner *backup.BackupRunner + var tierRunners []*backup.BackupRunner + var tierTargets []string for _, t := range backupTiers { prune := "" if t.KeepLast > 0 { @@ -1904,6 +1909,8 @@ func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.St if t.Primary { runner = r } + tierRunners = append(tierRunners, r) + tierTargets = append(tierTargets, t.TargetID) apiTiers = append(apiTiers, localapi.BackupTier{ TargetID: t.TargetID, Cadence: t.Cadence, @@ -2032,6 +2039,26 @@ func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.St logger.Warn("daemon: local-api disabled (server build)", "err", err) return nil } + // R-1006: a dead half-written vzdump (a power cut mid-backup) is removed at start-up and before every backup of the + // guest on that target, through the root helper; the in-flight answer is R-1008's (a task from before the boot is dead). + cleaner := &vzpartial.Cleaner{ + API: px, + Runner: &proxmox.ExecRunner{Mode: gaMode, SudoPath: cfg.Privileged.SudoPath}, + InFlight: srv.VzdumpInFlight, + Boot: func() (int64, bool) { + b, err := strconv.ParseInt(poweroff.ProcBtime(), 10, 64) + return b, err == nil && b > 0 + }, + Logger: logger, + } + for _, r := range tierRunners { + r.SetBeforeBackup(func(ctx context.Context, storage string, vmid int) { cleaner.Clean(ctx, storage, vmid) }) + } + srv.SetPartialSweep(func(ctx context.Context, vmid int) { + for _, t := range tierTargets { + cleaner.Clean(ctx, t, vmid) + } + }) return srv } diff --git a/configs/felhom-agent.sudoers b/configs/felhom-agent.sudoers index 8cb6ee1..c3fa523 100644 --- a/configs/felhom-agent.sudoers +++ b/configs/felhom-agent.sudoers @@ -88,6 +88,14 @@ Cmnd_Alias FELHOM_GUESTHOOK = \ Cmnd_Alias FELHOM_POWEROFF = \ /usr/bin/systemctl poweroff +# Dead half-written vzdump removal (R-1006, power-cut-fixes-2026-10-14): a power cut mid-backup leaves +# vzdump-lxc--.tar.dat + .tmp/ on the dir storage; nothing removed them and every later backup was skipped +# „not enough space". ONE root helper (configs/felhom-vzdump-partial-clean, from the config bundle) re-checks everything +# as root — a dir storage from storage.cfg, the exact name of THAT vmid, not in use, dead — and refuses otherwise. The +# arguments here are its exact shape (storage id, vmid, one half-written name); the vmid-in-name match is the helper's. +Cmnd_Alias FELHOM_VZPARTIAL = \ + /usr/local/sbin/felhom-vzdump-partial-clean ^[A-Za-z][A-Za-z0-9_-]{0\,63} [1-9][0-9]{2\,8} vzdump-(lxc|qemu)-[1-9][0-9]{2\,8}-[0-9]{4}_[0-9]{2}_[0-9]{2}-[0-9]{2}_[0-9]{2}_[0-9]{2}\.(tar\.dat|vma\.dat|tmp)$ + # Intermediary mount model (the drive hot-swap re-architecture). The agent keeps a SHARED host parent # /mnt/felhom-drives (self-bind + make-shared) and binds/unbinds each drive's felhom-data namespace UNDERNEATH it so the # change propagates into the running guest live. The boot-persistence script + unit are FIXED files from the config @@ -320,4 +328,4 @@ Cmnd_Alias FELHOM_GUESTNET = \ /usr/sbin/pct ^exec [0-9]+ -- pgrep -x dhclient$, \ /usr/sbin/pct ^exec [0-9]+ -- dhclient -pf /run/dhclient\.eth0\.pid -lf /var/lib/dhcp/dhclient\.eth0\.leases eth0$ -felhom-agent ALL=(root) NOPASSWD: FELHOM_MOUNT, FELHOM_DISK, FELHOM_PROVISION, FELHOM_FORMAT, FELHOM_DNSMASQ, FELHOM_GUESTHOOK, FELHOM_INTERMEDIARY, FELHOM_CONTROLLERSWAP, FELHOM_STALELOCK, FELHOM_FSTRIM, FELHOM_NETMOUNT, FELHOM_WG, FELHOM_SELFUPDATE, FELHOM_SSHD, FELHOM_OOB, FELHOM_PBSDR, FELHOM_BACKUPTARGET, FELHOM_SELFHEAL, FELHOM_ESCROW, FELHOM_GUESTNET, FELHOM_SCRATCH_TEARDOWN, FELHOM_OSAPPLY, FELHOM_POWEROFF +felhom-agent ALL=(root) NOPASSWD: FELHOM_MOUNT, FELHOM_DISK, FELHOM_PROVISION, FELHOM_FORMAT, FELHOM_DNSMASQ, FELHOM_GUESTHOOK, FELHOM_INTERMEDIARY, FELHOM_CONTROLLERSWAP, FELHOM_STALELOCK, FELHOM_FSTRIM, FELHOM_NETMOUNT, FELHOM_WG, FELHOM_SELFUPDATE, FELHOM_SSHD, FELHOM_OOB, FELHOM_PBSDR, FELHOM_BACKUPTARGET, FELHOM_SELFHEAL, FELHOM_ESCROW, FELHOM_GUESTNET, FELHOM_SCRATCH_TEARDOWN, FELHOM_OSAPPLY, FELHOM_POWEROFF, FELHOM_VZPARTIAL diff --git a/configs/felhom-os-apply b/configs/felhom-os-apply index 1ea908c..566df09 100755 --- a/configs/felhom-os-apply +++ b/configs/felhom-os-apply @@ -189,6 +189,8 @@ BUNDLE_FILES = [ ("/usr/local/sbin/felhom-selfupdate-guarded", "felhom-selfupdate-guarded", 0o755, "sh", "replace"), ("/usr/local/sbin/felhom-pbs-apply", "felhom-pbs-apply", 0o755, "bash", "replace"), ("/usr/local/sbin/felhom-backup-target-apply", "felhom-backup-target-apply", 0o755, "bash", "replace"), + # R-1006 (agent v0.156.0): the only delete of backup files — a dead half-written vzdump, re-checked as root. + ("/usr/local/sbin/felhom-vzdump-partial-clean", "felhom-vzdump-partial-clean", 0o755, "bash", "replace"), ("/usr/local/sbin/felhom-os-apply", "felhom-os-apply", 0o755, "python", "replace"), ("/usr/local/sbin/felhom-crash-guard", "felhom-crash-guard", 0o755, "python", "replace"), # R-861 (agent v0.146.0): the content checker for every agent-staged file a root program reads, and the two FIXED diff --git a/configs/felhom-vzdump-partial-clean b/configs/felhom-vzdump-partial-clean new file mode 100755 index 0000000..36f775e --- /dev/null +++ b/configs/felhom-vzdump-partial-clean @@ -0,0 +1,99 @@ +#!/bin/bash +# felhom-vzdump-partial-clean — remove ONE dead, half-written vzdump backup (R-1006). The only delete the agent's +# sudoers allows for backup files. Installed as /usr/local/sbin/felhom-vzdump-partial-clean (config bundle). +# +# felhom-vzdump-partial-clean +# +# A power cut during the night backup leaves `vzdump-lxc--.tar.dat` (+ its `.tmp/` directory) on the +# storage; nothing removes it, every later backup is then skipped „not enough space", and a full host root breaks +# LVM's own metadata writes (power-cut-fixes-2026-10-14 C4). This helper removes such a leftover — and NOTHING else. +# It trusts nothing the agent says; every rule is checked here, as root, and any doubt REFUSES (exit 1): +# 1. the arguments: a storage id, a vmid, and a name that is exactly +# vzdump-(lxc|qemu)--YYYY_MM_DD-HH_MM_SS.(tar.dat|vma.dat|tmp) — never a finished archive +# (.tar.zst, .vma.zst, …), never a .log/.notes, never another vmid; +# 2. the storage: its section in /etc/pve/storage.cfg is `dir: ` with a `path`; the target is +# /dump/, not a symlink, and its real directory IS /dump; +# 3. not in use: no vzdump runs on the host at all (a task `…:vzdump:…` or a `vzdump` command — a job for several +# guests carries no vmid in its task name), and no +# process holds the target (or, for the .tmp directory, anything under it) open or as its working directory +# (C2: after a killed worker, tar and zstd kept writing into the .tar.dat for 4 min 40 s, none named „vzdump"); +# 4. dead: last modified before this boot (`btime` in /proc/stat), or untouched for at least QUIET_S seconds. +# Prints one line on success: „removed B ()". Pinned by configs/test_felhom_vzdump_partial_clean.py. +set -u +STORAGE_CFG=/etc/pve/storage.cfg +PROC=/proc +QUIET_S=120 + +die() { echo "felhom-vzdump-partial-clean: REFUSED: $*" >&2; exit 1; } + +storage_path() { # $1 storage id → prints the dir storage's path, or nothing + awk -v want="$1" ' + /^[a-z]+:[ \t]/ { split($0, h, /:[ \t]+/); type = h[1]; id = h[2]; sub(/[ \t]+$/, "", id); in_s = (id == want); next } + in_s && /^[ \t]+path[ \t]/ { if (type == "dir") { sub(/^[ \t]+path[ \t]+/, ""); sub(/[ \t]+$/, ""); print; exit } } + ' "$STORAGE_CFG" +} + +storage_type() { + awk -v want="$1" '/^[a-z]+:[ \t]/ { split($0, h, /:[ \t]+/); id = h[2]; sub(/[ \t]+$/, "", id); if (id == want) { print h[1]; exit } }' "$STORAGE_CFG" +} + +boot_time() { awk '$1 == "btime" { print $2 }' "$PROC/stat"; } + +newest_mtime() { # a file's mtime, or the newest mtime of anything in a directory (the dir itself included) + if [[ -d "$1" ]]; then find "$1" -xdev -printf '%T@\n' 2>/dev/null | sort -n | tail -1 | cut -d. -f1 + else stat -c %Y "$1"; fi +} + +busy_reason() { # $1 vmid, $2 target → prints why it is in use, or nothing + local vmid=$1 target=$2 p link cmd + for p in "$PROC"/[0-9]*; do + [[ -r "$p/cmdline" ]] || continue + cmd=$(tr '\0' ' ' < "$p/cmdline" 2>/dev/null) + # ANY vzdump on the host refuses: a job for several guests runs as `task UPID:…:vzdump::…` (no vmid in it), and + # its tar may be between this guest's files. Waiting costs one backup run; deleting a live one costs the backup. + if [[ "$cmd" == *":vzdump:"* ]] || [[ "$cmd" =~ (^|/)vzdump([[:space:]]|$) ]]; then + echo "a vzdump runs on this host (pid ${p##*/})"; return + fi + for link in "$p/cwd" "$p"/fd/*; do + local to + to=$(readlink "$link" 2>/dev/null) || continue + if [[ "$to" == "$target" || "$to" == "$target/"* ]]; then + echo "pid ${p##*/} has it open"; return + fi + done + done +} + +main() { + local storage=${1:-} vmid=${2:-} name=${3:-} + [[ $# -eq 3 ]] || die "usage: felhom-vzdump-partial-clean " + [[ "$storage" =~ ^[A-Za-z][A-Za-z0-9_-]{0,63}$ ]] || die "bad storage id" + [[ "$vmid" =~ ^[1-9][0-9]{2,8}$ ]] || die "bad vmid" + [[ "$name" =~ ^vzdump-(lxc|qemu)-${vmid}-[0-9]{4}_[0-9]{2}_[0-9]{2}-[0-9]{2}_[0-9]{2}_[0-9]{2}\.(tar\.dat|vma\.dat|tmp)$ ]] \ + || die "name is not a half-written vzdump of guest $vmid: $name" + [[ "$(storage_type "$storage")" == "dir" ]] || die "storage $storage is not a dir storage" + local path; path=$(storage_path "$storage") + [[ "$path" == /* ]] || die "storage $storage has no path" + local dump="$path/dump" target="$path/dump/$name" + [[ -e "$target" ]] || die "no such entry: $target" + [[ ! -L "$target" ]] || die "a symlink: $target" + [[ "$(realpath -e "$dump")" == "$(realpath -e "$(dirname "$target")")" ]] || die "not directly in $dump" + if [[ "$name" == *.tmp ]]; then [[ -d "$target" ]] || die ".tmp is not a directory" + else [[ -f "$target" ]] || die "not a regular file"; fi + local busy; busy=$(busy_reason "$vmid" "$target") + [[ -z "$busy" ]] || die "in use: $busy" + local bt mt now why + bt=$(boot_time); mt=$(newest_mtime "$target"); now=$(date +%s) + [[ "$bt" =~ ^[0-9]+$ && "$mt" =~ ^[0-9]+$ ]] || die "cannot read the boot time or the modification time" + if (( mt < bt )); then why="written before this boot" + elif (( now - mt >= QUIET_S )); then why="untouched for $(( now - mt )) s, no vzdump for $vmid, nothing holds it" + else die "modified $(( now - mt )) s ago — may still be written"; fi + local size; size=$(du -sb "$target" 2>/dev/null | cut -f1) + if [[ -d "$target" ]]; then rm -rf --one-file-system -- "$target"; else rm -f -- "$target"; fi + [[ ! -e "$target" ]] || die "remove failed: $target" + echo "removed $name ${size:-?} B ($why)" +} + +if [[ "${BASH_SOURCE[0]}" == "$0" ]]; then + main "$@" +fi diff --git a/configs/test_felhom_vzdump_partial_clean.py b/configs/test_felhom_vzdump_partial_clean.py new file mode 100644 index 0000000..c38ed6a --- /dev/null +++ b/configs/test_felhom_vzdump_partial_clean.py @@ -0,0 +1,198 @@ +#!/usr/bin/env python3 +"""Tests for configs/felhom-vzdump-partial-clean (R-1006): it removes a DEAD half-written vzdump backup and refuses +everything else. Each refusal has a positive control: the same setup without the one condition is removed — so a +test cannot pass because the helper refuses everything. A fake storage.cfg and a fake /proc; nothing real is touched. +Run: python3 configs/test_felhom_vzdump_partial_clean.py (also run by internal/backup's Go test) +""" +import os, pathlib, subprocess, tempfile, time, unittest + +HELPER = pathlib.Path(__file__).resolve().parent / "felhom-vzdump-partial-clean" +BOOT = 1_791_967_481 +NAME = "vzdump-lxc-9201-2026_10_14-10_42_25" + + +class Box: + """A temp tree: a dir storage `local` (path vz), an lvmthin `local-lvm`, a dir storage `other`, and a fake /proc.""" + + def __init__(self): + self.t = tempfile.TemporaryDirectory() + r = pathlib.Path(self.t.name) + self.vz, self.other, self.nas, self.proc = r / "vz", r / "other", r / "nas", r / "proc" + for d in (self.vz / "dump", self.other / "dump", self.nas / "dump", self.proc): + d.mkdir(parents=True) + (self.proc / "stat").write_text(f"cpu 1 2 3\nbtime {BOOT}\n") + self.cfg = r / "storage.cfg" + self.cfg.write_text(f"dir: local\n\tpath {self.vz}\n\tcontent iso,backup\n\n" + f"lvmthin: local-lvm\n\tthinpool data\n\tvgname pve\n\n" + f"dir: other\n\tpath {self.other}\n\tcontent backup\n\n" + f"nfs: nas\n\tpath {self.nas}\n\tserver 192.0.2.1\n\texport /x\n\tcontent backup\n") + self.pid = 1000 + + def partial(self, name=NAME + ".tar.dat", mtime=BOOT - 100, where=None, size=4096): + p = (where or self.vz / "dump") / name + if name.endswith(".tmp"): + (p / "etc" / "vzdump").mkdir(parents=True) + (p / "etc" / "vzdump" / "pct.conf").write_text("x") + for q in [p / "etc" / "vzdump" / "pct.conf", p / "etc" / "vzdump", p / "etc", p]: + os.utime(q, (mtime, mtime)) + else: + p.write_bytes(b"\0" * size) + os.utime(p, (mtime, mtime)) + return p + + def process(self, cmdline, fds=(), cwd=None): + self.pid += 1 + d = self.proc / str(self.pid) + (d / "fd").mkdir(parents=True) + (d / "cmdline").write_bytes(b"\0".join(a.encode() for a in cmdline) + b"\0") + for i, f in enumerate(fds): + os.symlink(str(f), d / "fd" / str(i + 3)) + os.symlink(str(cwd or "/"), d / "cwd") + + def run(self, storage, vmid, name, quiet=120): + script = (f'source "{HELPER}"; STORAGE_CFG="{self.cfg}"; PROC="{self.proc}"; QUIET_S={quiet}; ' + f'main "$@"') + r = subprocess.run(["bash", "-c", script, "x", storage, str(vmid), name], capture_output=True, text=True) + return r.returncode, r.stdout + r.stderr + + +class Removes(unittest.TestCase): + def test_dead_tar_dat_before_boot(self): + b = Box(); p = b.partial() + rc, out = b.run("local", 9201, p.name) + self.assertEqual(rc, 0, out); self.assertFalse(p.exists()); self.assertIn("written before this boot", out) + + def test_dead_tmp_dir_before_boot(self): + b = Box(); p = b.partial(NAME + ".tmp") + rc, out = b.run("local", 9201, p.name) + self.assertEqual(rc, 0, out); self.assertFalse(p.exists()) + + def test_quiet_partial_after_boot(self): + b = Box(); p = b.partial(mtime=int(time.time()) - 300) + rc, out = b.run("local", 9201, p.name) + self.assertEqual(rc, 0, out); self.assertFalse(p.exists()); self.assertIn("untouched", out) + + def test_qemu_vma_dat(self): + b = Box(); p = b.partial("vzdump-qemu-341-2026_10_14-10_42_25.vma.dat") + rc, out = b.run("local", 341, p.name) + self.assertEqual(rc, 0, out); self.assertFalse(p.exists()) + + +class Keeps(unittest.TestCase): + """Each case: (setup that must be REFUSED, the same setup that must be REMOVED).""" + + def pair(self, refuse, control, quiet=120): + b = Box(); target, args = refuse(b) + rc, out = b.run(*args, quiet=quiet) + self.assertNotEqual(rc, 0, f"must refuse: {out}") + self.assertIn("REFUSED", out) + self.assertTrue(os.path.lexists(target), "a refused entry must still exist") + c = Box(); ctarget, cargs = control(c) + crc, cout = c.run(*cargs, quiet=quiet) + self.assertEqual(crc, 0, f"positive control must be removed: {cout}") + self.assertFalse(os.path.lexists(ctarget)) + + def test_complete_archive_of_same_vmid(self): + self.pair(lambda b: (b.partial(NAME + ".tar.zst"), ("local", 9201, NAME + ".tar.zst")), + lambda b: (b.partial(), ("local", 9201, NAME + ".tar.dat"))) + + def test_log_and_notes(self): + for suffix in (".log", ".tar.zst.notes"): + self.pair(lambda b, s=suffix: (b.partial(NAME + s), ("local", 9201, NAME + s)), + lambda b: (b.partial(), ("local", 9201, NAME + ".tar.dat"))) + + def test_partial_of_another_vmid(self): + other = "vzdump-lxc-9202-2026_10_14-10_42_25.tar.dat" + self.pair(lambda b: (b.partial(other), ("local", 9201, other)), + lambda b: (b.partial(other), ("local", 9202, other))) + + def test_partial_on_a_non_dir_storage(self): + self.pair(lambda b: (b.partial(), ("local-lvm", 9201, NAME + ".tar.dat")), + lambda b: (b.partial(), ("local", 9201, NAME + ".tar.dat"))) + + def test_partial_on_a_network_storage_with_a_path(self): + # an nfs storage HAS a path: only the „dir only" rule (both its guards) can refuse it + self.pair(lambda b: (b.partial(where=b.nas / "dump"), ("nas", 9201, NAME + ".tar.dat")), + lambda b: (b.partial(where=b.other / "dump"), ("other", 9201, NAME + ".tar.dat"))) + + def test_partial_on_another_storage_than_named(self): + self.pair(lambda b: (b.partial(where=b.other / "dump"), ("local", 9201, NAME + ".tar.dat")), + lambda b: (b.partial(where=b.other / "dump"), ("other", 9201, NAME + ".tar.dat"))) + + def test_unknown_storage(self): + self.pair(lambda b: (b.partial(), ("nosuch", 9201, NAME + ".tar.dat")), + lambda b: (b.partial(), ("local", 9201, NAME + ".tar.dat"))) + + def test_vzdump_task_in_flight(self): + def refuse(b): + p = b.partial(); b.process(["task UPID:demo-hp:001:002:6ACF4071:vzdump:9201:felhom-agent@pve!agent:"]); return p, ("local", 9201, p.name) + def control(b): + p = b.partial(); b.process(["task UPID:demo-hp:001:002:6ACF4071:vzrestore:9202:root@pam:"]); return p, ("local", 9201, p.name) + self.pair(refuse, control) + + def test_any_vzdump_job_even_without_the_vmid(self): + # a job for several guests: `vzdump::` — and a vzdump for ANOTHER guest — both refuse + for cmd in (["task UPID:demo-hp:001:002:6ACF4071:vzdump::root@pam:"], + ["task UPID:demo-hp:001:002:6ACF4071:vzdump:9202:felhom-agent@pve!agent:"]): + def refuse(b, c=cmd): + p = b.partial(); b.process(c); return p, ("local", 9201, p.name) + def control(b): + p = b.partial(); b.process(["task UPID:demo-hp:001:002:6ACF4071:vzstart:9202:root@pam:"]); return p, ("local", 9201, p.name) + self.pair(refuse, control) + + def test_vzdump_command_for_the_vmid(self): + def refuse(b): + p = b.partial(); b.process(["/usr/bin/perl", "/usr/bin/vzdump", "9201", "--storage", "local"]); return p, ("local", 9201, p.name) + def control(b): + p = b.partial(); b.process(["/usr/bin/perl", "/usr/bin/vzdumpfoo", "9201"]); return p, ("local", 9201, p.name) + self.pair(refuse, control) + + def test_file_held_open_by_an_orphan_writer(self): + def refuse(b): + p = b.partial(); b.process(["zstd", "--threads=1"], fds=[p]); return p, ("local", 9201, p.name) + def control(b): + p = b.partial(); b.process(["zstd", "--threads=1"], fds=[b.vz / "dump" / "something-else"]); return p, ("local", 9201, p.name) + self.pair(refuse, control) + + def test_tmp_dir_with_a_process_inside(self): + def refuse(b): + p = b.partial(NAME + ".tmp"); b.process(["tar", "cpf", "-"], cwd=p / "etc"); return p, ("local", 9201, p.name) + def control(b): + p = b.partial(NAME + ".tmp"); b.process(["tar", "cpf", "-"], cwd=b.vz); return p, ("local", 9201, p.name) + self.pair(refuse, control) + + def test_recently_written_after_boot(self): + self.pair(lambda b: (b.partial(mtime=int(time.time()) - 10), ("local", 9201, NAME + ".tar.dat")), + lambda b: (b.partial(mtime=int(time.time()) - 300), ("local", 9201, NAME + ".tar.dat"))) + + def test_symlink(self): + def refuse(b): + real = b.partial(where=b.vz) + link = b.vz / "dump" / (NAME + ".tar.dat") + os.symlink(real, link) + return link, ("local", 9201, link.name) + # quiet=0: the link was made just now, so only the symlink rule may refuse it + self.pair(refuse, lambda b: (b.partial(), ("local", 9201, NAME + ".tar.dat")), quiet=0) + + def test_near_miss_names(self): + for bad in (NAME + ".tar.dat.bak", "vzdump-lxc-92011-2026_10_14-10_42_25.tar.dat", NAME + ".tar.da", + "x" + NAME + ".tar.dat", NAME + ".TAR.DAT", "vzdump-lxc-9201-2026_10_14-10_42.tar.dat"): + self.pair(lambda b, n=bad: (b.partial(n), ("local", 9201, n)), + lambda b: (b.partial(), ("local", 9201, NAME + ".tar.dat"))) + + def test_path_traversal_and_bad_args(self): + b = Box(); p = b.partial() + for args in (("local", 9201, "../dump/" + p.name), ("local", 9201, ""), ("loc al", 9201, p.name), + ("local", "9201x", p.name), ("local", 12, p.name)): + rc, out = b.run(*args) + self.assertNotEqual(rc, 0, f"{args}: {out}") + self.assertTrue(p.exists()) + + def test_wrong_kind(self): + def refuse(b): # a FILE named .tmp + p = b.vz / "dump" / (NAME + ".tmp"); p.write_bytes(b"x"); os.utime(p, (BOOT - 100, BOOT - 100)); return p, ("local", 9201, p.name) + self.pair(refuse, lambda b: (b.partial(NAME + ".tmp"), ("local", 9201, NAME + ".tmp"))) + + +if __name__ == "__main__": + unittest.main() diff --git a/internal/backup/r685_space_test.go b/internal/backup/r685_space_test.go index 16865b1..679fd76 100644 --- a/internal/backup/r685_space_test.go +++ b/internal/backup/r685_space_test.go @@ -81,3 +81,28 @@ func TestR685_FailsOpen(t *testing.T) { } } } + +// TestR1006_LeftoverCleanupRunsBeforeThePreflight — the night of 2026-10-14: a 12.2 GB half-written archive from a power +// cut held the space, so every later backup was skipped. The cleanup step runs BEFORE the preflight reads the free space: +// here it „removes" the leftover (free 4 → 16 GiB) and the backup then fits and runs. +// +// RED-PROOF: drop the r.beforeBackup call from backup() (or move it after spaceFits) → the backup is skipped → fails. +func TestR1006_LeftoverCleanupRunsBeforeThePreflight(t *testing.T) { + api := spaceAPI(4*gib, 8182759056, "dir") + r := NewBackupRunner(api, "local", proxmox.ModeSnapshot, "", "keep-last=1", quiet()) + var got []string + r.SetBeforeBackup(func(_ context.Context, storage string, vmid int) { + got = append(got, storage) + if vmid != 9201 { + t.Errorf("cleanup asked for vmid %d", vmid) + } + api.storages[0].Avail = 16 * gib + }) + _, _ = r.Backup(context.Background(), 9201) + if len(got) != 1 || got[0] != "local" { + t.Fatalf("the cleanup must run once for this target, got %v", got) + } + if len(api.vzdumps) != 1 { + t.Fatalf("after the cleanup freed the space the backup must run, vzdumps=%d", len(api.vzdumps)) + } +} diff --git a/internal/backup/runner.go b/internal/backup/runner.go index 66d5cf9..4726d13 100644 --- a/internal/backup/runner.go +++ b/internal/backup/runner.go @@ -58,8 +58,11 @@ type BackupRunner struct { // tier cannot have that accident — its keep_last defaults to 0 (never prune), so any value // there is a deliberate act. allowPBSPrune bool - logger *slog.Logger - now func() time.Time + // beforeBackup runs just before the space preflight (R-1006): it removes a dead half-written vzdump of this guest on + // this target, so a leftover from a power cut cannot hold the space this backup needs. nil = nothing. + beforeBackup func(ctx context.Context, storage string, vmid int) + logger *slog.Logger + now func() time.Time // rejected remembers the volids already announced by warnRejectedArchiveOnce, so an incomplete // archive is reported ONCE rather than on every 5-minute due-check. Bounded in practice: one // entry per aborted upload, and a process restart clears it. Guarded by rejectedMu because the @@ -102,6 +105,11 @@ func NewBackupRunnerFull(api BackupAPI, target string, mode proxmox.BackupMode, now: func() time.Time { return time.Now().UTC() }} } +// SetBeforeBackup sets the step run just before the space preflight (R-1006: the dead-leftover cleanup). +func (r *BackupRunner) SetBeforeBackup(fn func(ctx context.Context, storage string, vmid int)) { + r.beforeBackup = fn +} + // localPruneSpec returns the `--prune-backups` spec to apply to THIS backup, or "" to skip pruning. It // applies the configured retention ONLY when the target is a non-PBS storage — PBS offsite retention is a // separate lifecycle and must never be pruned by the per-run flag (§9). Fail-safe: if the target's type @@ -183,6 +191,9 @@ func (r *BackupRunner) backup(ctx context.Context, vmid int, onSnapshot func()) // 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 r.beforeBackup != nil { + r.beforeBackup(ctx, r.target, vmid) + } if ok, why := r.spaceFits(ctx, vmid); !ok { rec.Error = BackupSkipNoSpacePrefix + why rec.DurationSeconds = time.Since(start).Seconds() diff --git a/internal/capability/manifest.go b/internal/capability/manifest.go index 3d9e0cc..0e3ac81 100644 --- a/internal/capability/manifest.go +++ b/internal/capability/manifest.go @@ -117,6 +117,7 @@ var manifest = []Capability{ {"guest-reboot", "enroll activate-binds reboot", "/usr/sbin/pct", []string{"reboot", "9201"}, false, ""}, {"guest-shutdown", "household power-off: guest shutdown (R-998)", "/usr/sbin/pct", []string{"shutdown", "9201", "--timeout", "120"}, false, ""}, {"host-poweroff", "household power-off: host power-off (R-998)", "/usr/bin/systemctl", []string{"poweroff"}, false, ""}, + {"vzdump-partial-clean", "dead half-written vzdump removal (R-1006)", "/usr/local/sbin/felhom-vzdump-partial-clean", []string{"local", "9201", "vzdump-lxc-9201-2026_10_14-04_42_18.tar.dat"}, false, ""}, // ---- LAN split-horizon resolver (dnsmasq) ---- {"dnsmasq-install", "dnsmasq package install", "/usr/bin/apt-get", []string{"install", "-y", "-q", "dnsmasq"}, false, ""}, diff --git a/internal/capability/r861_injection_test.go b/internal/capability/r861_injection_test.go index 561c685..a14e94b 100644 --- a/internal/capability/r861_injection_test.go +++ b/internal/capability/r861_injection_test.go @@ -63,6 +63,15 @@ var r861Injections = []string{ "/usr/bin/systemctl reboot", "/usr/bin/systemctl halt", "/usr/bin/systemctl kexec", + // R-1006: the partial-cleanup helper takes exactly one storage, one vmid, one half-written name. + "/usr/local/sbin/felhom-vzdump-partial-clean local 9201 vzdump-lxc-9201-2026_10_14-04_42_18.tar.zst", + "/usr/local/sbin/felhom-vzdump-partial-clean local 9201 vzdump-lxc-9201-2026_10_14-04_42_18.log", + "/usr/local/sbin/felhom-vzdump-partial-clean local 9201 ../../etc/vzdump-lxc-9201-2026_10_14-04_42_18.tar.dat", + "/usr/local/sbin/felhom-vzdump-partial-clean local 9201 vzdump-lxc-9201-2026_10_14-04_42_18.tar.dat /etc/shadow", + "/usr/local/sbin/felhom-vzdump-partial-clean local 9201 vzdump-lxc-9201-2026_10_14-04_42_18.tar.dat;id", + "/usr/local/sbin/felhom-vzdump-partial-clean local;id 9201 vzdump-lxc-9201-2026_10_14-04_42_18.tar.dat", + "/usr/local/sbin/felhom-vzdump-partial-clean local 9201", + "/usr/local/sbin/felhom-vzdump-partial-clean --help", } func TestSudoersRefusesTheR861Injections(t *testing.T) { diff --git a/internal/localapi/controllersupervisor_test.go b/internal/localapi/controllersupervisor_test.go index 48d3fc5..054a21b 100644 --- a/internal/localapi/controllersupervisor_test.go +++ b/internal/localapi/controllersupervisor_test.go @@ -19,10 +19,10 @@ import ( // again", and each guard is pinned by the case where acting would be wrong. type supExec struct { - mu sync.Mutex - status map[int]string // docker .State.Status per vmid; "" = container absent - inspectErr error // non-nil = pct exec itself failed (unknown) - restarts map[int]int + mu sync.Mutex + status map[int]string // docker .State.Status per vmid; "" = container absent + inspectErr error // non-nil = pct exec itself failed (unknown) + restarts map[int]int // onRestart, when set, is the status the container reaches after a restart (a crash-looper // stays "exited"). onRestart string @@ -249,7 +249,6 @@ func TestControllerSupervisorStanza_WireShape(t *testing.T) { } } - // ---- R-539 (operator ruling 3 of 2026-09-16): the SLOW crash loop ---------------------------------- // supKillOnce kills the controller and lets the supervisor restart it (two confirming sweeps), then diff --git a/internal/localapi/guestbind.go b/internal/localapi/guestbind.go index e4fdd4f..d3829cc 100644 --- a/internal/localapi/guestbind.go +++ b/internal/localapi/guestbind.go @@ -1,13 +1,13 @@ package localapi import ( - "time" "context" "fmt" "log/slog" "strconv" "strings" "sync" + "time" "gitea.dooplex.hu/admin/felhom-agent/internal/proxmox" ) diff --git a/internal/localapi/server.go b/internal/localapi/server.go index 73629eb..bc29ff6 100644 --- a/internal/localapi/server.go +++ b/internal/localapi/server.go @@ -313,10 +313,11 @@ type Server struct { // offsite repository password. OPTIONAL — nil (no hub client configured) makes // POST /escrow/recover-offsite-password answer 503 rather than pretending. escrowRecovery EscrowRecoverer - intent IntentRecorder // slice 10 P3 (optional) - guestBinds *GuestBindStore // F9 startup bind re-assert record (optional) - formatJobs *FormatJobStore // F20-BUG3 detached-format job record (optional) - staleLock StaleLockController // F2-b startup stale-lock recovery (optional) + intent IntentRecorder // slice 10 P3 (optional) + guestBinds *GuestBindStore // F9 startup bind re-assert record (optional) + formatJobs *FormatJobStore // F20-BUG3 detached-format job record (optional) + staleLock StaleLockController // F2-b startup stale-lock recovery (optional) + partialSweep func(ctx context.Context, vmid int) // R-1006 start-up sweep of dead vzdump leftovers (optional) // guestPower (F-REBOOT) is per-guest start-attempt state for the guest-power watchdog. // Guarded by guestPowerMu in guestpower.go; in-memory on purpose (see guestPowerState). guestPower map[int]guestPowerState diff --git a/internal/localapi/stalelock.go b/internal/localapi/stalelock.go index 64ed6b1..e3d058a 100644 --- a/internal/localapi/stalelock.go +++ b/internal/localapi/stalelock.go @@ -5,7 +5,9 @@ import ( "fmt" "log/slog" "strconv" + "sync" + "gitea.dooplex.hu/admin/felhom-agent/internal/poweroff" "gitea.dooplex.hu/admin/felhom-agent/internal/proxmox" ) @@ -142,6 +144,11 @@ type staleLockController struct { runner proxmox.Runner pool string // ownership registry: only members of this PVE pool are ever scanned (A1) logger *slog.Logger // the one success-path scan-summary line; nil = silent + // bootTime is the host's boot time (unix seconds) and whether it could be read (R-1008). nil = the + // kernel's btime (poweroff.ProcBtime — the one reader of /proc/stat btime this agent shares). + bootTime func() (int64, bool) + deadMu sync.Mutex + deadSeen map[string]bool // UPIDs of dead tasks already logged (once per task) } // staleLockAPI is the subset of *proxmox.Client the controller uses (kept narrow for clarity/testing). @@ -209,19 +216,81 @@ func (c *staleLockController) Lock(ctx context.Context, vmid int) (string, bool, return cfg.Lock(), cfg.OnBoot(), nil } +// BackupRunning is THE answer to „is a vzdump in flight for vmid" for all four callers (startup stale-lock +// recovery, the guest-power watchdog, the household power-off guard, the controller supervisor): every one of +// them reaches this method through the StaleLockController interface. R-1008: Proxmox keeps a task in +// `tasks?source=active` after a power cut — nobody is left to end it (measured on demo-hp 2026-10-14: still +// listed RUNNING after the boot) — so a task that STARTED BEFORE THIS BOOT is dead, not in flight. func (c *staleLockController) BackupRunning(ctx context.Context, vmid int) (bool, error) { tasks, err := c.px.ListRunningTasks(ctx) if err != nil { return false, err } + bt := c.bootTime + if bt == nil { + bt = procBootTime + } + boot, ok := bt() + running, dead := vzdumpInFlight(tasks, vmid, boot, ok) + for _, t := range dead { + c.logDeadOnce(t, vmid, boot) + } + return running, nil +} + +// vzdumpInFlight decides, from the node's active-task list, whether a vzdump for vmid is in flight. A matching +// task is in flight when it started at or after the host's boot; one that started before the boot is dead +// (returned in dead, for the log). FAIL SAFE on doubt — the old answer: an unreadable boot time, or a task whose +// start time is unknown, counts as in flight. Pure; pinned by stalelock_inflight_test.go. +func vzdumpInFlight(tasks []proxmox.TaskStatus, vmid int, boot int64, bootOK bool) (bool, []proxmox.TaskStatus) { id := strconv.Itoa(vmid) + var dead []proxmox.TaskStatus for _, t := range tasks { // A single-guest vzdump task carries the vmid in its ID field. - if t.Type == "vzdump" && t.ID == id { + if t.Type != "vzdump" || t.ID != id { + continue + } + if !bootOK || boot <= 0 { return true, nil } + start := taskStart(t) + if start <= 0 || start >= boot { + return true, nil + } + dead = append(dead, t) } - return false, nil + return false, dead +} + +// taskStart is the task's start time: the list's starttime field, else the one encoded in its UPID; 0 = unknown. +func taskStart(t proxmox.TaskStatus) int64 { + if t.StartTime > 0 { + return t.StartTime + } + if u, err := proxmox.ParseUPID(t.UPID); err == nil { + return int64(u.StartTime) + } + return 0 +} + +// procBootTime reads the kernel's boot time through poweroff.ProcBtime (no second /proc/stat parser). +func procBootTime() (int64, bool) { + b, err := strconv.ParseInt(poweroff.ProcBtime(), 10, 64) + return b, err == nil && b > 0 +} + +func (c *staleLockController) logDeadOnce(t proxmox.TaskStatus, vmid int, boot int64) { + c.deadMu.Lock() + defer c.deadMu.Unlock() + if c.deadSeen == nil { + c.deadSeen = map[string]bool{} + } + if c.deadSeen[t.UPID] || c.logger == nil { + return + } + c.deadSeen[t.UPID] = true + c.logger.Warn("backup-in-flight: a vzdump task from before this boot is listed as active — it is dead, not running", + "vmid", vmid, "upid", t.UPID, "task_start", taskStart(t), "host_boot", boot) } func (c *staleLockController) HasVzdumpSnapshot(ctx context.Context, vmid int) (bool, error) { diff --git a/internal/localapi/stalelock_inflight_test.go b/internal/localapi/stalelock_inflight_test.go new file mode 100644 index 0000000..1c2074a --- /dev/null +++ b/internal/localapi/stalelock_inflight_test.go @@ -0,0 +1,195 @@ +package localapi + +import ( + "bytes" + "context" + "encoding/json" + "log/slog" + "os" + "path/filepath" + "regexp" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-agent/internal/proxmox" +) + +// R-1008: after a power cut Proxmox keeps the dead vzdump task in `tasks?source=active` (measured on demo-hp +// 2026-10-14: still RUNNING after the boot, started 136 s before it). A task that started before this boot is +// dead, not in flight. These tests pin the helper (vzdumpInFlight), the production controller, and three of the +// four callers end to end; the fourth (the controller supervisor) by the structural pin at the bottom. + +const testBoot = int64(1791967481) // the C3 boot (power-cut-fixes-2026-10-14) + +func vz(vmid string, start int64) proxmox.TaskStatus { + return proxmox.TaskStatus{UPID: "UPID:demo-hp:001E6EEC:00000001:6ACF4071:vzdump:" + vmid + ":felhom-agent@pve!agent:", + Type: "vzdump", ID: vmid, Status: "RUNNING", StartTime: start} +} + +// RED-PROOF: make vzdumpInFlight return true for every matching task (drop the `start >= boot` test) → this fails. +func TestVzdumpInFlight_PreBootTaskIsDead(t *testing.T) { + running, dead := vzdumpInFlight([]proxmox.TaskStatus{vz("9201", testBoot-136)}, 9201, testBoot, true) + if running || len(dead) != 1 { + t.Fatalf("a task from before the boot must be dead: running=%v dead=%d", running, len(dead)) + } +} + +func TestVzdumpInFlight_PostBootTaskRuns(t *testing.T) { + for _, start := range []int64{testBoot, testBoot + 600} { + if running, _ := vzdumpInFlight([]proxmox.TaskStatus{vz("9201", start)}, 9201, testBoot, true); !running { + t.Fatalf("a task started at %d (boot %d) must be in flight", start, testBoot) + } + } +} + +// RED-PROOF: drop the `!bootOK` early return → a pre-boot task with an unreadable boot is called dead → fails. +func TestVzdumpInFlight_UnreadableBootFailsSafe(t *testing.T) { + // the boot value is a real one on purpose: only bootOK=false may make this „running" + if running, _ := vzdumpInFlight([]proxmox.TaskStatus{vz("9201", testBoot-136)}, 9201, testBoot, false); !running { + t.Fatal("an unreadable boot time must count as running (fail safe)") + } +} + +// RED-PROOF: make taskStart ignore the UPID → StartTime 0 counts as unknown → running; the first case fails. +func TestVzdumpInFlight_StartFromUPIDThenUnknownFailsSafe(t *testing.T) { + tk := vz("9201", 0) // no starttime in the list: the UPID's 6ACF4071 = 1791967345 (before the boot) + if running, dead := vzdumpInFlight([]proxmox.TaskStatus{tk}, 9201, testBoot, true); running || len(dead) != 1 { + t.Fatalf("the UPID's start time must be used: running=%v", running) + } + tk.UPID = "garbage" + if running, _ := vzdumpInFlight([]proxmox.TaskStatus{tk}, 9201, testBoot, true); !running { + t.Fatal("an unknown start time must count as running (fail safe)") + } +} + +func TestVzdumpInFlight_OtherGuestOrTypeIgnored(t *testing.T) { + tasks := []proxmox.TaskStatus{vz("9202", testBoot+5), {Type: "vzsnapshot", ID: "9201", StartTime: testBoot + 5}} + if running, dead := vzdumpInFlight(tasks, 9201, testBoot, true); running || len(dead) != 0 { + t.Fatalf("other guests' and other types' tasks are not 9201's backup: running=%v dead=%d", running, len(dead)) + } +} + +func inflightCtrl(api *fakeStaleLockAPI, runner *recordingRunner, logger *slog.Logger) *staleLockController { + c := NewStaleLockController(api, runner, "felhom", logger).(*staleLockController) + c.bootTime = func() (int64, bool) { return testBoot, true } + return c +} + +// The production controller answers through the helper, and logs a dead task ONCE. +// RED-PROOF: make BackupRunning ignore bootTime (the pre-R-1008 body) → returns true → fails. +func TestStaleLockController_BackupRunningIgnoresDeadTask(t *testing.T) { + var buf bytes.Buffer + api := &fakeStaleLockAPI{tasks: []proxmox.TaskStatus{vz("9201", testBoot-136)}} + c := inflightCtrl(api, &recordingRunner{}, slog.New(slog.NewTextHandler(&buf, nil))) + for i := 0; i < 3; i++ { + if running, err := c.BackupRunning(context.Background(), 9201); err != nil || running { + t.Fatalf("dead pre-boot task reported running=%v err=%v", running, err) + } + } + if n := strings.Count(buf.String(), "backup-in-flight: a vzdump task from before this boot"); n != 1 { + t.Fatalf("the dead task must be logged exactly once, got %d", n) + } + api.tasks = []proxmox.TaskStatus{vz("9201", testBoot+30)} + if running, _ := c.BackupRunning(context.Background(), 9201); !running { + t.Fatal("a post-boot task must be running") + } +} + +func poolOf(vmids ...int) proxmox.PoolInfo { + var p proxmox.PoolInfo + raw := `{"members":[` + for i, v := range vmids { + if i > 0 { + raw += "," + } + raw += `{"vmid":` + itoaT(v) + `,"type":"lxc"}` + } + raw += `]}` + _ = json.Unmarshal([]byte(raw), &p) + return p +} + +// Caller 1 — the startup stale-lock recovery clears the lock a power cut left (the night of 2026-10-14 it said +// „genuinely in-flight — leaving the lock"). +func TestCaller_StartupStaleLockClearsAfterPowerCut(t *testing.T) { + for _, tc := range []struct { + start int64 + cleared bool + }{{testBoot - 136, true}, {testBoot + 30, false}} { + api := &fakeStaleLockAPI{tasks: []proxmox.TaskStatus{vz("9201", tc.start)}, + cfg: map[int]proxmox.GuestConfig{9201: staleCfg("backup", true)}} + runner := &recordingRunner{} + s := &Server{staleLock: inflightCtrl(api, runner, nil), logger: slog.New(slog.NewTextHandler(discardW{}, nil))} + s.recoverOneStaleLock(context.Background(), proxmox.Guest{VMID: 9201, Status: "running"}) + if runner.unlocked("9201") != tc.cleared { + t.Fatalf("task start %d: unlocked=%v, want %v", tc.start, runner.unlocked("9201"), tc.cleared) + } + } +} + +// Caller 2 — the guest-power watchdog starts a stopped guest despite a dead pre-boot vzdump task. +func TestCaller_GuestPowerStartsGuestAfterPowerCut(t *testing.T) { + for _, tc := range []struct { + start int64 + started bool + }{{testBoot - 136, true}, {testBoot + 30, false}} { + api := &fakeStaleLockAPI{lxc: []proxmox.Guest{{VMID: 9201, Status: "stopped"}}, pool: poolOf(9201), + tasks: []proxmox.TaskStatus{vz("9201", tc.start)}, cfg: map[int]proxmox.GuestConfig{9201: staleCfg("", true)}} + s := gpServer(t, inflightCtrl(api, &recordingRunner{}, nil), nil) + s.GuestPowerTick(context.Background()) + if (len(api.started) == 1) != tc.started { + t.Fatalf("task start %d: started=%v, want %v", tc.start, api.started, tc.started) + } + } +} + +// Caller 3 — the household power-off guard does not refuse „backup running" for a dead task. +func TestCaller_PowerOffJobBlockerIgnoresDeadTask(t *testing.T) { + for _, tc := range []struct { + start int64 + want string + }{{testBoot - 136, ""}, {testBoot + 30, PowerOffBackup}} { + rig := newPowerRig(t, 0) + rig.srv.staleLock = inflightCtrl(&fakeStaleLockAPI{tasks: []proxmox.TaskStatus{vz("9201", tc.start)}}, &recordingRunner{}, nil) + if code, _ := rig.srv.jobBlocker(context.Background(), 9201); code != tc.want { + t.Fatalf("task start %d: jobBlocker=%q, want %q", tc.start, code, tc.want) + } + } +} + +// Caller 4 (the supervisor), the R-1006 cleanup (VzdumpInFlight) and the rest — structural pin: every non-test BackupRunning call in this package goes through +// s.staleLock (whose one production implementation is staleLockController, which uses vzdumpInFlight), and no +// other production type implements BackupRunning. If someone adds a second path, this fails. +func TestBackupRunning_AllCallersUseTheOneHelper(t *testing.T) { + files, _ := filepath.Glob("*.go") + call := regexp.MustCompile(`([A-Za-z_.]+)\.BackupRunning\(`) + impl := regexp.MustCompile(`func \([a-z]+ \*?([A-Za-z]+)\) BackupRunning\(`) + callers := 0 + for _, f := range files { + if strings.HasSuffix(f, "_test.go") { + continue + } + b, err := os.ReadFile(f) + if err != nil { + t.Fatal(err) + } + for _, m := range call.FindAllStringSubmatch(string(b), -1) { + if m[1] != "s.staleLock" { + t.Errorf("%s: BackupRunning called on %q, not s.staleLock", f, m[1]) + } + callers++ + } + for _, m := range impl.FindAllStringSubmatch(string(b), -1) { + if m[1] != "staleLockController" { + t.Errorf("%s: a second production BackupRunning on %s", f, m[1]) + } + } + } + if callers != 5 { + t.Errorf("expected the five known callers (stale-lock, guest-power, power-off, supervisor, the R-1006 cleanup), found %d", callers) + } + b, _ := os.ReadFile("stalelock.go") + if !strings.Contains(string(b), "vzdumpInFlight(tasks, vmid, boot, ok)") { + t.Error("staleLockController.BackupRunning no longer answers through vzdumpInFlight") + } +} diff --git a/internal/localapi/vzpartial_sweep.go b/internal/localapi/vzpartial_sweep.go new file mode 100644 index 0000000..3f5b392 --- /dev/null +++ b/internal/localapi/vzpartial_sweep.go @@ -0,0 +1,33 @@ +package localapi + +import "context" + +// R-1006: the half-written vzdump cleanup's two needs from the Server, and its start-up sweep. The cleaner itself is +// internal/vzpartial; the root helper is configs/felhom-vzdump-partial-clean. + +// VzdumpInFlight is the one in-flight answer (staleLockController.BackupRunning, R-1008) for the cleanup. No stale-lock +// controller wired → true: fail safe, nothing is removed. +func (s *Server) VzdumpInFlight(ctx context.Context, vmid int) (bool, error) { + if s.staleLock == nil { + return true, nil + } + return s.staleLock.BackupRunning(ctx, vmid) +} + +// SetPartialSweep sets the start-up sweep: called with every owned guest (the felhom pool, ownership proven). +func (s *Server) SetPartialSweep(fn func(ctx context.Context, vmid int)) { s.partialSweep = fn } + +// SweepVzdumpPartials runs the sweep once for every owned guest. No-op when unwired. +func (s *Server) SweepVzdumpPartials(ctx context.Context) { + if s.partialSweep == nil || s.staleLock == nil { + return + } + guests, err := s.staleLock.Guests(ctx) + if err != nil { + s.logger.Warn("vzdump-partial: guest list unavailable — skipping the start-up sweep", "err", err) + return + } + for _, g := range guests { + s.partialSweep(ctx, g.VMID) + } +} diff --git a/internal/vzpartial/vzpartial.go b/internal/vzpartial/vzpartial.go new file mode 100644 index 0000000..bea2d23 --- /dev/null +++ b/internal/vzpartial/vzpartial.go @@ -0,0 +1,208 @@ +// Package vzpartial removes a dead, half-written vzdump backup of a guest from a local dir storage (R-1006). +// +// A power cut during the night's whole-guest backup leaves `vzdump-lxc--.tar.dat` and its `.tmp/` directory +// on the target (demo-hp 2026-10-14: 12.2 GB on the host root). Nothing removes them: Proxmox does not list them as +// backups, every later backup is skipped „not enough space", and a host root that fills up breaks LVM's own metadata +// writes (audits/power-cut-fixes-2026-10-14 C4). This package finds such a leftover and asks the root helper +// /usr/local/sbin/felhom-vzdump-partial-clean to remove it; the helper re-checks every rule as root. +// +// Rules here (the helper repeats them, and adds the open-file check the agent cannot do): +// - only a `dir` storage; only names of the exact vzdump pattern for that vmid (NamePattern) — never a finished +// archive, a log, another guest; +// - only when no vzdump for that vmid is in flight (the R-1008 answer: a task from before the boot is not in flight); +// - only when dead: last modified before this boot, or untouched for QuietFor. +// +// Any doubt leaves the entry and logs why. Pinned by vzpartial_test.go. +package vzpartial + +import ( + "context" + "fmt" + "log/slog" + "os" + "path/filepath" + "regexp" + "strconv" + "strings" + "time" + + "gitea.dooplex.hu/admin/felhom-agent/internal/proxmox" +) + +// Helper is the root helper's installed path (configs/felhom-vzdump-partial-clean, delivered by the config bundle). +const Helper = "/usr/local/sbin/felhom-vzdump-partial-clean" + +// QuietFor is how long an entry written in THIS boot must be untouched before it counts as dead (the helper's QUIET_S). +const QuietFor = 120 * time.Second + +// NamePattern matches the half-written entries of one guest: the archive being written (.tar.dat / .vma.dat) and the +// temporary directory (.tmp). The timestamp is vzdump's own (host local time). +func NamePattern(vmid int) *regexp.Regexp { + return regexp.MustCompile(`^vzdump-(lxc|qemu)-` + strconv.Itoa(vmid) + + `-[0-9]{4}_[0-9]{2}_[0-9]{2}-[0-9]{2}_[0-9]{2}_[0-9]{2}\.(tar\.dat|vma\.dat|tmp)$`) +} + +// Entry is one leftover found in the dump directory. +type Entry struct { + Name string + Dir bool + ModTime time.Time + Size int64 +} + +// Candidates lists the entries of dumpDir that match vmid's half-written pattern. A missing directory is no error. +func Candidates(dumpDir string, vmid int) ([]Entry, error) { + ents, err := os.ReadDir(dumpDir) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, err + } + re := NamePattern(vmid) + var out []Entry + for _, e := range ents { + if !re.MatchString(e.Name()) || e.Type()&os.ModeSymlink != 0 { + continue + } + info, err := e.Info() + if err != nil { + continue + } + if strings.HasSuffix(e.Name(), ".tmp") != info.IsDir() { + continue // a .tmp that is not a directory, or an archive name that is one: not vzdump's + } + mt := info.ModTime() + if info.IsDir() { + mt = newestUnder(filepath.Join(dumpDir, e.Name()), mt) + } + out = append(out, Entry{Name: e.Name(), Dir: info.IsDir(), ModTime: mt, Size: info.Size()}) + } + return out, nil +} + +func newestUnder(dir string, mt time.Time) time.Time { + _ = filepath.WalkDir(dir, func(_ string, d os.DirEntry, err error) error { + if err != nil { + return nil + } + if i, e := d.Info(); e == nil && i.ModTime().After(mt) { + mt = i.ModTime() + } + return nil + }) + return mt +} + +// Dead decides whether an entry is dead: written before this boot, or untouched for QuietFor. bootOK=false (the boot +// time could not be read) leaves only the quiet rule. +func Dead(e Entry, boot time.Time, bootOK bool, now time.Time) (bool, string) { + if bootOK && e.ModTime.Before(boot) { + return true, "written before this boot" + } + if age := now.Sub(e.ModTime); age >= QuietFor { + return true, "untouched for " + age.Round(time.Second).String() + } + return false, "modified " + now.Sub(e.ModTime).Round(time.Second).String() + " ago — may still be written" +} + +// StorageAPI is the one Proxmox read the cleaner needs (the storage's type and path). +type StorageAPI interface { + ListStorage(ctx context.Context) ([]proxmox.Storage, error) +} + +// Cleaner removes dead leftovers. All fields but Logger and Now are required; a nil Cleaner is a no-op. +type Cleaner struct { + API StorageAPI + Runner proxmox.Runner // the fenced sudo runner + InFlight func(ctx context.Context, vmid int) (bool, error) + Boot func() (int64, bool) + Logger *slog.Logger + Now func() time.Time + // dumpDir maps a storage path to its dump directory (tests); nil = /dump. + dumpDir func(path string) string +} + +// Result is one removal or one entry left in place. +type Result struct { + Entry Entry + Removed bool + Why string +} + +// Clean removes the dead leftovers of vmid on storage. It never returns an error for „nothing to do"; every entry it +// finds is logged with what was done and why. +func (c *Cleaner) Clean(ctx context.Context, storage string, vmid int) []Result { + if c == nil || c.API == nil || c.Runner == nil || c.InFlight == nil || c.Boot == nil { + return nil + } + log := c.Logger + if log == nil { + log = slog.Default() + } + now := time.Now + if c.Now != nil { + now = c.Now + } + stores, err := c.API.ListStorage(ctx) + if err != nil { + log.Warn("vzdump-partial: storage list unreadable — not looking for leftovers", "storage", storage, "err", err) + return nil + } + var st *proxmox.Storage + for i := range stores { + if stores[i].Storage == storage { + st = &stores[i] + break + } + } + if st == nil || st.Type != "dir" || st.Path == "" { + return nil // PBS, LVM, NFS …: vzdump does not leave a .tar.dat there that this agent may remove + } + dump := filepath.Join(st.Path, "dump") + if c.dumpDir != nil { + dump = c.dumpDir(st.Path) + } + ents, err := Candidates(dump, vmid) + if err != nil { + log.Warn("vzdump-partial: dump directory unreadable", "dir", dump, "err", err) + return nil + } + if len(ents) == 0 { + return nil + } + var out []Result + inFlight, err := c.InFlight(ctx, vmid) + if err != nil || inFlight { + why := "a backup of this guest is in flight" + if err != nil { + why = "cannot tell whether a backup is in flight: " + err.Error() + } + for _, e := range ents { + log.Info("vzdump-partial: leaving a half-written backup", "storage", storage, "vmid", vmid, "name", e.Name, "why", why) + out = append(out, Result{Entry: e, Why: why}) + } + return out + } + b, bootOK := c.Boot() + boot := time.Unix(b, 0) + for _, e := range ents { + dead, why := Dead(e, boot, bootOK, now()) + if !dead { + log.Info("vzdump-partial: leaving a half-written backup", "storage", storage, "vmid", vmid, "name", e.Name, "why", why) + out = append(out, Result{Entry: e, Why: why}) + continue + } + stdout, stderr, err := c.Runner.Run(ctx, Helper, storage, strconv.Itoa(vmid), e.Name) + if err != nil { + why = fmt.Sprintf("the root helper refused: %s", strings.TrimSpace(string(stderr))) + log.Warn("vzdump-partial: leaving a half-written backup", "storage", storage, "vmid", vmid, "name", e.Name, "why", why, "err", err) + out = append(out, Result{Entry: e, Why: why}) + continue + } + log.Warn("vzdump-partial: removed a dead half-written backup", "storage", storage, "vmid", vmid, "name", e.Name, + "bytes", e.Size, "why", why, "helper", strings.TrimSpace(string(stdout))) + out = append(out, Result{Entry: e, Removed: true, Why: why}) + } + return out +} diff --git a/internal/vzpartial/vzpartial_test.go b/internal/vzpartial/vzpartial_test.go new file mode 100644 index 0000000..aef41fe --- /dev/null +++ b/internal/vzpartial/vzpartial_test.go @@ -0,0 +1,206 @@ +package vzpartial + +import ( + "context" + "errors" + "io" + "os" + "os/exec" + "path/filepath" + "runtime" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-agent/internal/proxmox" +) + +const vmid = 9201 +const name = "vzdump-lxc-9201-2026_10_14-10_42_25" + +var boot = time.Unix(1791967481, 0) + +type fakeAPI struct{ stores []proxmox.Storage } + +func (f fakeAPI) ListStorage(context.Context) ([]proxmox.Storage, error) { return f.stores, nil } + +type fakeRunner struct { + calls [][]string + err error +} + +func (r *fakeRunner) Run(_ context.Context, n string, a ...string) ([]byte, []byte, error) { + r.calls = append(r.calls, append([]string{n}, a...)) + if r.err != nil { + return nil, []byte("REFUSED: in use"), r.err + } + return []byte("removed"), nil, nil +} +func (r *fakeRunner) RunStdin(ctx context.Context, _ io.Reader, n string, a ...string) ([]byte, []byte, error) { + return r.Run(ctx, n, a...) +} + +type rig struct { + dump string + runner *fakeRunner + c *Cleaner +} + +func newRig(t *testing.T, storeType string, inFlight bool, inErr error) *rig { + t.Helper() + root := t.TempDir() + dump := filepath.Join(root, "dump") + if err := os.MkdirAll(dump, 0o755); err != nil { + t.Fatal(err) + } + r := &rig{dump: dump, runner: &fakeRunner{}} + r.c = &Cleaner{ + API: fakeAPI{stores: []proxmox.Storage{{Storage: "local", Type: storeType, Path: root}}}, + Runner: r.runner, + InFlight: func(context.Context, int) (bool, error) { return inFlight, inErr }, + Boot: func() (int64, bool) { return boot.Unix(), true }, + Now: func() time.Time { return boot.Add(5 * time.Minute) }, + } + return r +} + +func (r *rig) put(t *testing.T, n string, dir bool, mt time.Time) { + t.Helper() + p := filepath.Join(r.dump, n) + if dir { + if err := os.MkdirAll(filepath.Join(p, "etc"), 0o755); err != nil { + t.Fatal(err) + } + _ = os.Chtimes(filepath.Join(p, "etc"), mt, mt) + } else if err := os.WriteFile(p, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + _ = os.Chtimes(p, mt, mt) +} + +func (r *rig) asked() []string { + var out []string + for _, c := range r.runner.calls { + if len(c) == 4 && c[0] == Helper && c[1] == "local" && c[2] == "9201" { + out = append(out, c[3]) + } + } + return out +} + +// RED-PROOF: remove the `bootOK && e.ModTime.Before(boot)` rule in Dead → a pre-boot entry modified 1 min before the +// boot is still „untouched for 6m" — so the test uses a 1-min-old-at-boot entry AND a Now 30 s after the boot. +func TestClean_RemovesDeadPartialAndTmp(t *testing.T) { + r := newRig(t, "dir", false, nil) + r.c.Now = func() time.Time { return boot.Add(30 * time.Second) } + r.put(t, name+".tar.dat", false, boot.Add(-time.Minute)) + r.put(t, name+".tmp", true, boot.Add(-time.Minute)) + res := r.c.Clean(context.Background(), "local", vmid) + if got := r.asked(); len(got) != 2 { + t.Fatalf("both dead entries must go to the helper, got %v", got) + } + for _, x := range res { + if !x.Removed || x.Why != "written before this boot" { + t.Fatalf("%+v", x) + } + } +} + +func TestClean_RemovesQuietPartialOfThisBoot(t *testing.T) { + r := newRig(t, "dir", false, nil) + r.put(t, name+".tar.dat", false, boot.Add(time.Minute)) // 4 min before Now + r.c.Clean(context.Background(), "local", vmid) + if len(r.asked()) != 1 { + t.Fatal("a partial untouched for QuietFor must be removed") + } +} + +// Each keep has its positive control (the dead pre-boot partial in the same rig IS asked for). +func TestClean_Keeps(t *testing.T) { + cases := map[string]func(t *testing.T) (*rig, string){ + // RED-PROOF: drop the InFlight check in Clean → the in-flight partial is asked for → fails. + "in flight": func(t *testing.T) (*rig, string) { return newRig(t, "dir", true, nil), name + ".tar.dat" }, + "in-flight unknown": func(t *testing.T) (*rig, string) { + return newRig(t, "dir", false, errors.New("api down")), name + ".tar.dat" + }, + // RED-PROOF: drop the `st.Type != "dir"` test → an lvmthin target is cleaned → fails. + "not a dir storage": func(t *testing.T) (*rig, string) { return newRig(t, "lvmthin", false, nil), name + ".tar.dat" }, + } + for what, mk := range cases { + r, n := mk(t) + r.put(t, n, false, boot.Add(-time.Hour)) + r.c.Clean(context.Background(), "local", vmid) + if len(r.asked()) != 0 { + t.Fatalf("%s: must keep, helper asked for %v", what, r.asked()) + } + ctl := newRig(t, "dir", false, nil) + ctl.put(t, n, false, boot.Add(-time.Hour)) + ctl.c.Clean(context.Background(), "local", vmid) + if len(ctl.asked()) != 1 { + t.Fatalf("%s: positive control not removed", what) + } + } +} + +// RED-PROOF: weaken NamePattern to `vzdump-.*` → the finished archive / log / other guest's partial are asked for. +func TestClean_OnlyThisGuestsHalfWrittenNames(t *testing.T) { + r := newRig(t, "dir", false, nil) + old := boot.Add(-time.Hour) + for _, n := range []string{name + ".tar.zst", name + ".log", name + ".tar.zst.notes", "vzdump-lxc-9202-2026_10_14-10_42_25.tar.dat", + "vzdump-lxc-92011-2026_10_14-10_42_25.tar.dat", name + ".tar.dat.bak", name + ".tar.da", "x" + name + ".tar.dat"} { + r.put(t, n, false, old) + } + r.put(t, name+".tar.dat", false, old) // the positive control in the same directory + r.c.Clean(context.Background(), "local", vmid) + if got := r.asked(); len(got) != 1 || got[0] != name+".tar.dat" { + t.Fatalf("only %s.tar.dat may be asked for, got %v", name, got) + } +} + +func TestClean_KeepsRecentAndWrongKindAndSymlink(t *testing.T) { + r := newRig(t, "dir", false, nil) + r.put(t, name+".tar.dat", false, boot.Add(4*time.Minute)) // 1 min before Now: may still be written + r.put(t, "vzdump-lxc-9201-2026_10_14-11_00_00.tmp", false, boot.Add(-time.Hour)) // a FILE named .tmp + r.put(t, "vzdump-lxc-9201-2026_10_14-13_00_00.tar.dat", false, boot.Add(-time.Hour)) // positive control + r.c.Clean(context.Background(), "local", vmid) + if got := r.asked(); len(got) != 1 || got[0] != "vzdump-lxc-9201-2026_10_14-13_00_00.tar.dat" { + t.Fatalf("only the positive control may be asked for, got %v", got) + } +} + +// RED-PROOF: drop the ModeSymlink skip in Candidates → the symlink (quiet: Now is an hour ahead) is asked for → fails. +func TestClean_KeepsSymlink(t *testing.T) { + r := newRig(t, "dir", false, nil) + r.c.Now = func() time.Time { return time.Now().Add(time.Hour) } + target := filepath.Join(r.dump, "elsewhere") + _ = os.WriteFile(target, []byte("x"), 0o644) + _ = os.Symlink(target, filepath.Join(r.dump, "vzdump-lxc-9201-2026_10_14-12_00_00.tar.dat")) + r.put(t, "vzdump-lxc-9201-2026_10_14-13_00_00.tar.dat", false, boot.Add(-time.Hour)) // positive control + r.c.Clean(context.Background(), "local", vmid) + if got := r.asked(); len(got) != 1 || got[0] != "vzdump-lxc-9201-2026_10_14-13_00_00.tar.dat" { + t.Fatalf("only the positive control may be asked for, got %v", got) + } +} + +func TestClean_HelperRefusalIsNotARemoval(t *testing.T) { + r := newRig(t, "dir", false, nil) + r.runner.err = errors.New("exit status 1") + r.put(t, name+".tar.dat", false, boot.Add(-time.Hour)) + res := r.c.Clean(context.Background(), "local", vmid) + if len(res) != 1 || res[0].Removed { + t.Fatalf("a refused helper call must not count as removed: %+v", res) + } +} + +// The root helper's own tests (configs/test_felhom_vzdump_partial_clean.py) run with the Go suite. +func TestRootHelperPythonSuite(t *testing.T) { + py, err := exec.LookPath("python3") + if err != nil { + t.Skip("python3 not found") + } + _, file, _, _ := runtime.Caller(0) + script := filepath.Join(filepath.Dir(file), "..", "..", "configs", "test_felhom_vzdump_partial_clean.py") + out, err := exec.Command(py, script).CombinedOutput() + if err != nil { + t.Fatalf("helper suite failed: %v\n%s", err, out) + } +}