diff --git a/CHANGELOG.md b/CHANGELOG.md index e10da7d..e036d45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,32 @@ -## Unreleased (2026-10-19) — two texts made true after 0.160.0; no behaviour change +## Unreleased (2026-10-19) — three fixes for the next release (R-921, R-1070, R-1010); two texts made true after 0.160.0 + +Not released (the night brief of 2026-10-19: agent code + tests + commit only; the release is the operator's word). No +sudoers, wrapper or config-bundle change. + +- **R-921 — `GET /backup/status` says when the host is busy.** New field `host_busy` (`{busy, what}`): whether the + host-wide heavy-operation gate is held right now (another guest's backup, the restore test, a night OS step, the + stale-lock sweep, a power-off) — the state that makes a `POST /backup` answer 409. Served on every request; absent + when the gate is not wired. Controller ≥ 0.335.0 reads it before it stops the household's apps for a tier, so a + refused tier costs no stop (the measured case of 2026-10-08: the night OS step, all 15 apps stopped for a refusal). + A read, never a reservation. No new route. Test `TestBackupStatus_R921_HostBusy`, red-proved (assignment removed → + `free gate: host_busy = `). +- **R-1070 — a box with no tunnel set up no longer fails its host OS update.** Docker 29 prints „error: no such + object" in lower case, so the tunnel probe's case-sensitive match fell through to `unknown` („could not ask the + guest"), and the host health rule failed every box without a cloudflared container (Tester4, 2026-10-15, + `os_update_health_failed`). Now the match ignores case and a missing container is a new state, **`absent`** („no + tunnel set up on this box"); the host rule reads it as not applicable only when the reading from before the update + (Docker answered) also had no cloudflared container. A tunnel that is down, gone or unreadable still fails. The hub + alarms only on `not_running`, so `absent` never mails. Tests `TestHostHealthVerdict_R1070_NoTunnelSetUp` (7 rows) and + three `TestClassifyTunnel` rows (Docker 29's wording measured on demo-hp), red-proved both ways. +- **R-1010 — the stale-lock janitor clears a half-removed `vzdump` snapshot.** When the delete fails ONLY with + „Failed to find logical volume …_vzdump" (one disk's snapshot already gone — demo-hp 2026-10-14 after a backup ran + out of space), it retries once with Proxmox's own `force=1` on the same API call (`DeleteSnapshotForce`; the same + token privilege, no sudoers line, no root command — `pvesh usage … -command delete` lists `--force`). Any other error + is returned unchanged; a failed forced delete is returned, never a second force. Tests + `TestStaleLock_R1010_ForcesOnceOnlyWhenTheVolumeIsGone` (6 rows) and `TestDeleteSnapshotForce_R1010`, red-proved + (single plain delete → `calls [plain], want [plain force]`). + +### Earlier on 2026-10-19 (texts only) - The ceremony-complete log line now says the package uploads at the claim **or at the typed-back confirm** (R-1151); it read „package uploads at the claim" for both orders (seen in demo-hp's journal, 2026-10-19 01:47 UTC). diff --git a/internal/hub/cloudflared.go b/internal/hub/cloudflared.go index 94c0f99..b976d72 100644 --- a/internal/hub/cloudflared.go +++ b/internal/hub/cloudflared.go @@ -14,6 +14,13 @@ const ( TunnelRunning = "running" // the cloudflared container runs AND its readiness check says CONNECTED TunnelNotRunning = "not_running" // stopped / exited / absent, or running but NOT connected (Detail says which) TunnelUnknown = "unknown" // the probe could not ask (guest down, pct/sudo error, health still starting) + // TunnelAbsent (R-1070, agent Unreleased 2026-10-19): the guest answered and has NO cloudflared container — the + // controller makes none when no tunnel token is configured (a LAN-only or test box). Not a failure by itself: the + // hub never alarms on it, and the OS-update host rule reads it as „not applicable" only when the container was + // also absent before the update (HostHealthVerdict). Docker 29 prints „error: no such object" in lower case, so + // before this state existed the case-sensitive match fell through to `unknown` — which failed the host rule on + // every box without a tunnel (Tester4, 2026-10-15). + TunnelAbsent = "absent" ) // CloudflaredProber reports the box's tunnel. Injectable so tests use a fake and never exec. @@ -53,7 +60,7 @@ func (p GuestTunnelProber) Status(ctx context.Context) (string, string) { return TunnelUnknown, "no running customer guest" } worst, wdetail := "", "" - rank := map[string]int{TunnelRunning: 0, TunnelUnknown: 1, TunnelNotRunning: 2} + rank := map[string]int{TunnelRunning: 0, TunnelAbsent: 1, TunnelUnknown: 2, TunnelNotRunning: 3} for _, v := range vmids { out, errOut, err := p.Runner.Run(ctx, "/usr/sbin/pct", "exec", fmt.Sprint(v), "--", "docker", "inspect", "-f", tunnelInspect, "cloudflared") st, d := ClassifyTunnel(string(out), string(errOut), err) @@ -71,8 +78,8 @@ func (p GuestTunnelProber) Status(ctx context.Context) (string, string) { func ClassifyTunnel(stdout, stderr string, err error) (string, string) { out := strings.TrimSpace(stdout) if err != nil || out == "" { - if strings.Contains(stderr, "No such object") || strings.Contains(stderr, "No such container") { - return TunnelNotRunning, "no cloudflared container in the guest" + if e := strings.ToLower(stderr); strings.Contains(e, "no such object") || strings.Contains(e, "no such container") { + return TunnelAbsent, "no cloudflared container in the guest (no tunnel set up on this box)" } return TunnelUnknown, "could not ask the guest: " + firstLine(stderr, err) } diff --git a/internal/hub/cloudflared_test.go b/internal/hub/cloudflared_test.go index 261f775..4e3f781 100644 --- a/internal/hub/cloudflared_test.go +++ b/internal/hub/cloudflared_test.go @@ -20,7 +20,10 @@ func TestClassifyTunnel(t *testing.T) { {"connected", "running|0|healthy\n", "", nil, TunnelRunning, "connected"}, {"running but not connected", "running|0|unhealthy\n", "", nil, TunnelNotRunning, "NOT connected"}, {"stopped", "exited|137|unhealthy\n", "", nil, TunnelNotRunning, "exit code 137"}, - {"absent", "", "Error: No such object: cloudflared", errors.New("exit status 1"), TunnelNotRunning, "no cloudflared container"}, + {"absent", "", "Error: No such object: cloudflared", errors.New("exit status 1"), TunnelAbsent, "no cloudflared container"}, + // R-1070: Docker 29's wording, measured on demo-hp 2026-10-19 (`docker inspect` of a missing name). + {"absent, docker 29", "", "error: no such object: cloudflared", errors.New("exit status 1"), TunnelAbsent, "no tunnel set up"}, + {"absent, no such container", "", "Error response from daemon: No such container: cloudflared", errors.New("exit status 1"), TunnelAbsent, "no cloudflared container"}, {"still starting", "running|0|starting\n", "", nil, TunnelUnknown, "starting"}, {"no health check (older controller)", "running|0|none\n", "", nil, TunnelRunning, "connection not checked"}, {"guest not running", "", "CT 9201 not running", errors.New("exit status 255"), TunnelUnknown, "could not ask"}, diff --git a/internal/hub/report.go b/internal/hub/report.go index 6235715..8a9258f 100644 --- a/internal/hub/report.go +++ b/internal/hub/report.go @@ -359,7 +359,7 @@ type GuestSpec struct { // Cloudflared is the box's tunnel (R-841, agent v0.141.0): the cloudflared container in the customer guest. type Cloudflared struct { - Status string `json:"status"` // running | not_running | unknown (TunnelRunning …) + Status string `json:"status"` // running | not_running | unknown | absent (TunnelRunning …; absent since R-1070) Detail string `json:"detail,omitempty"` // why not_running / unknown, or "connected" } diff --git a/internal/localapi/server.go b/internal/localapi/server.go index ea72190..da55318 100644 --- a/internal/localapi/server.go +++ b/internal/localapi/server.go @@ -1343,6 +1343,20 @@ type BackupStatusResponse struct { // presence. Served on the UNTARGETED request only; additive, so an older controller reads the // response exactly as before. Tiers []TierBackupState `json:"tiers,omitempty"` + // HostBusy (R-921, agent Unreleased 2026-10-19) says whether the host-wide heavy-operation gate is held RIGHT NOW + // (another guest's backup, the restore test, a night OS step, the stale-lock sweep, a power-off) — a POST /backup + // would then be refused with 409. The controller reads it BEFORE it stops the household's apps, so a refused tier + // costs no stop. Served on every request; ABSENT (nil) = an older agent, or the gate is not wired — the controller + // then keeps its old order (stop, ask, resume at once on 409). A read, never a reservation: the gate can be taken + // between this answer and the POST, and that race keeps the old order. Pinned by TestBackupStatus_R921_HostBusy. + HostBusy *HostBusyState `json:"host_busy,omitempty"` +} + +// HostBusyState is GET /backup/status's host_busy (R-921). What names the holder ("backup:local", "restore-test", +// "os-pve-step" …), empty when Busy is false. +type HostBusyState struct { + Busy bool `json:"busy"` + What string `json:"what,omitempty"` } func (s *Server) handleBackupStatus(w http.ResponseWriter, r *http.Request, vmid int) { @@ -1362,6 +1376,10 @@ func (s *Server) handleBackupStatus(w http.ResponseWriter, r *http.Request, vmid resp.JobID = job.JobID resp.Error = job.Error } + if s.inFlight != nil { + what := s.inFlight.Busy() + resp.HostBusy = &HostBusyState{Busy: what != "", What: what} + } writeOK(w, resp) } diff --git a/internal/localapi/server_test.go b/internal/localapi/server_test.go index 2a77c28..d0f2b4a 100644 --- a/internal/localapi/server_test.go +++ b/internal/localapi/server_test.go @@ -13,6 +13,7 @@ import ( "testing" "time" + "gitea.dooplex.hu/admin/felhom-agent/internal/backup" "gitea.dooplex.hu/admin/felhom-agent/internal/hub" "gitea.dooplex.hu/admin/felhom-agent/internal/proxmox" ) @@ -514,3 +515,47 @@ func TestBackupStatus_FiltersToThisGuest(t *testing.T) { t.Fatalf("status not scoped to this guest / not latest: %s", w.Body.String()) } } + +// R-921: GET /backup/status says when the host-wide heavy-operation gate is held, so the controller can skip the app +// stop for a tier the POST would refuse. Free → busy false; held → busy true with the holder's name; the gate not wired +// → no field at all (the controller's „older agent" reading, never „free"). +// +// COMPANION RED-PROOF (observed 2026-10-19): with the `resp.HostBusy = …` assignment removed, the held case fails with +// +// server_test.go: free gate: host_busy = , want busy=false +// +// Restored. +func TestBackupStatus_R921_HostBusy(t *testing.T) { + read := func(h http.Handler) *HostBusyState { + w := do(t, h, "GET", "/backup/status", "A", "") + var resp struct { + Data BackupStatusResponse `json:"data"` + } + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("status body: %v (%s)", err, w.Body.String()) + } + return resp.Data.HostBusy + } + srv := newTestServerS(t, &fakeGuests{}, &fakeBackups{}, &fakeStore{}, nil) + if got := read(srv.Handler()); got != nil { + t.Fatalf("gate not wired: host_busy = %+v, want absent", got) + } + gate := &backup.InFlight{} + srv.inFlight = gate + h := srv.Handler() + if got := read(h); got == nil || got.Busy || got.What != "" { + t.Fatalf("free gate: host_busy = %+v, want busy=false", got) + } + release, _, ok := gate.TryAcquire("os-pve-step") + if !ok { + t.Fatal("could not take the gate") + } + if got := read(h); got == nil || !got.Busy || got.What != "os-pve-step" { + t.Fatalf("held gate: host_busy = %+v, want busy=true what=\"os-pve-step\"", got) + } + // The read never takes the gate: a POST after the holder lets go is accepted. + release() + if got := read(h); got == nil || got.Busy { + t.Fatalf("released gate: host_busy = %+v, want busy=false", got) + } +} diff --git a/internal/localapi/stalelock.go b/internal/localapi/stalelock.go index 332931e..c1c521b 100644 --- a/internal/localapi/stalelock.go +++ b/internal/localapi/stalelock.go @@ -2,6 +2,7 @@ package localapi import ( "context" + "errors" "fmt" "log/slog" "os" @@ -160,6 +161,7 @@ type staleLockAPI interface { ListSnapshots(ctx context.Context, vmid int) ([]proxmox.Snapshot, error) ListRunningTasks(ctx context.Context) ([]proxmox.TaskStatus, error) DeleteSnapshot(ctx context.Context, vmid int, snapname string) (string, error) + DeleteSnapshotForce(ctx context.Context, vmid int, snapname string) (string, error) Start(ctx context.Context, vmid int) (string, error) WaitTask(ctx context.Context, upid string, opts proxmox.WaitOptions) (proxmox.TaskStatus, error) } @@ -334,8 +336,34 @@ func (c *staleLockController) Unlock(ctx context.Context, vmid int) error { return nil } +// DeleteVzdumpSnapshot removes the dangling `vzdump` snapshot. R-1010: when the delete fails ONLY because one disk's +// snapshot volume is already gone („Failed to find logical volume pve/snap_vm-9201-disk-0_vzdump" — a backup that ran +// out of space, 2026-10-14), Proxmox stops at that volume and keeps the entry and the other volumes' snapshots, and a +// plain retry fails the same way every sweep, forever. Then it retries ONCE with Proxmox's force (the hand fix that +// day was `pct delsnapshot 9201 vzdump --force`): the remaining volumes' snapshots are removed and the entry goes. Any +// other error is returned as it is — never a force on an error that might mean a volume still holds data (a full +// LVM metadata area, a busy volume). Pinned by TestStaleLock_R1010_*. func (c *staleLockController) DeleteVzdumpSnapshot(ctx context.Context, vmid int) error { - upid, err := c.px.DeleteSnapshot(ctx, vmid, vzdumpSnapshotName) + err := c.deleteSnap(ctx, vmid, false) + if err == nil || !snapshotVolumeAlreadyGone(err) { + return err + } + if c.logger != nil { + c.logger.Warn("stale-lock: a disk's vzdump snapshot volume is already gone — retrying the delete once with force (R-1010)", + "vmid", vmid, "err", err) + } + if ferr := c.deleteSnap(ctx, vmid, true); ferr != nil { + return fmt.Errorf("forced snapshot delete after %q also failed: %w", err.Error(), ferr) + } + return nil +} + +func (c *staleLockController) deleteSnap(ctx context.Context, vmid int, force bool) error { + del := c.px.DeleteSnapshot + if force { + del = c.px.DeleteSnapshotForce + } + upid, err := del(ctx, vmid, vzdumpSnapshotName) if err != nil { return err } @@ -346,6 +374,23 @@ func (c *staleLockController) DeleteVzdumpSnapshot(ctx context.Context, vmid int return err } +// snapshotVolumeAlreadyGone: the error is Proxmox's „Failed to find logical volume …_vzdump" from a snapshot delete +// (in the task's exit status, its log tail, or the API error text). Narrow on purpose: the force is safe only when the +// volume it cannot remove does not exist. +func snapshotVolumeAlreadyGone(err error) bool { + text := err.Error() + var te *proxmox.TaskError + if errors.As(err, &te) { + text += "\n" + te.ExitStatus + "\n" + strings.Join(te.LogTail, "\n") + } + for _, l := range strings.Split(text, "\n") { + if strings.Contains(strings.ToLower(l), "failed to find logical volume") && strings.Contains(l, "_"+vzdumpSnapshotName) { + return true + } + } + return false +} + func (c *staleLockController) Start(ctx context.Context, vmid int) error { upid, err := c.px.Start(ctx, vmid) if err != nil { diff --git a/internal/localapi/stalelock_pool_test.go b/internal/localapi/stalelock_pool_test.go index 4ed8096..fbe09e8 100644 --- a/internal/localapi/stalelock_pool_test.go +++ b/internal/localapi/stalelock_pool_test.go @@ -30,6 +30,9 @@ type fakeStaleLockAPI struct { delsnap []int started []int + // R-1010: scripted delete answers, consumed in order (plain and forced share the list); calls records "plain"/"force". + delErrs []error + delCalls []string } func (f *fakeStaleLockAPI) ListLXC(context.Context) ([]proxmox.Guest, error) { return f.lxc, nil } @@ -55,7 +58,22 @@ func (f *fakeStaleLockAPI) ListRunningTasks(context.Context) ([]proxmox.TaskStat func (f *fakeStaleLockAPI) DeleteSnapshot(_ context.Context, vmid int, snapname string) (string, error) { f.delsnap = append(f.delsnap, vmid) - return "", nil // synchronous — no WaitTask + return "", f.nextDelErr("plain") // synchronous — no WaitTask +} + +func (f *fakeStaleLockAPI) DeleteSnapshotForce(_ context.Context, vmid int, snapname string) (string, error) { + f.delsnap = append(f.delsnap, vmid) + return "", f.nextDelErr("force") +} + +func (f *fakeStaleLockAPI) nextDelErr(kind string) error { + f.delCalls = append(f.delCalls, kind) + if len(f.delErrs) == 0 { + return nil + } + e := f.delErrs[0] + f.delErrs = f.delErrs[1:] + return e } func (f *fakeStaleLockAPI) Start(_ context.Context, vmid int) (string, error) { diff --git a/internal/localapi/stalelock_r1010_test.go b/internal/localapi/stalelock_r1010_test.go new file mode 100644 index 0000000..4728fdc --- /dev/null +++ b/internal/localapi/stalelock_r1010_test.go @@ -0,0 +1,59 @@ +package localapi + +import ( + "context" + "errors" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-agent/internal/proxmox" +) + +// R-1010: a half-removed vzdump snapshot. The error text is the one demo-hp's task printed on 2026-10-14 +// (`audits/power-cut-fixes-2026-10-14/C4/hand-fix.txt`). +const r1010MissingLV = `lvremove snapshot 'pve/snap_vm-9201-disk-0_vzdump' error: Failed to find logical volume "pve/snap_vm-9201-disk-0_vzdump"` + +func r1010TaskErr(exit string) error { + return &proxmox.TaskError{UPID: "UPID:demo-hp:0001:0002:0003:vzdelsnapshot:9201:root@pam:", ExitStatus: exit, + LogTail: []string{exit}} +} + +// COMPANION RED-PROOF (observed 2026-10-19): with DeleteVzdumpSnapshot back to the single plain delete, the first case +// fails with +// +// stalelock_r1010_test.go: missing volume: calls [plain], want [plain force]; err = proxmox: task … failed: exitstatus "lvremove snapshot …" +// +// Restored. +func TestStaleLock_R1010_ForcesOnceOnlyWhenTheVolumeIsGone(t *testing.T) { + cases := []struct { + name string + errs []error + wantCalls []string + wantErr string + }{ + {"missing volume", []error{r1010TaskErr(r1010MissingLV)}, []string{"plain", "force"}, ""}, + {"missing volume, named only in the log tail", []error{&proxmox.TaskError{UPID: "x", ExitStatus: "command 'lvremove' failed: exit code 5", + LogTail: []string{r1010MissingLV}}}, []string{"plain", "force"}, ""}, + {"the plain delete works", nil, []string{"plain"}, ""}, + {"another error: no force", []error{r1010TaskErr("lvremove snapshot 'pve/snap_vm-9201-disk-0_vzdump' error: Failed to update pool pve/data.")}, + []string{"plain"}, "Failed to update pool"}, + {"a missing volume of ANOTHER snapshot: no force", []error{r1010TaskErr(`Failed to find logical volume "pve/snap_vm-9201-disk-0_mine"`)}, + []string{"plain"}, "_mine"}, + {"the forced delete fails too: returned, never a second force", []error{r1010TaskErr(r1010MissingLV), errors.New("still broken")}, + []string{"plain", "force"}, "still broken"}, + } + for _, c := range cases { + api := &fakeStaleLockAPI{delErrs: c.errs} + ctrl := NewStaleLockController(api, &recordingRunner{}, "felhom", nil) + err := ctrl.DeleteVzdumpSnapshot(context.Background(), 9201) + if strings.Join(api.delCalls, " ") != strings.Join(c.wantCalls, " ") { + t.Errorf("%s: calls %v, want %v; err = %v", c.name, api.delCalls, c.wantCalls, err) + } + switch { + case c.wantErr == "" && err != nil: + t.Errorf("%s: err = %v, want nil", c.name, err) + case c.wantErr != "" && (err == nil || !strings.Contains(err.Error(), c.wantErr)): + t.Errorf("%s: err = %v, want one naming %q", c.name, err, c.wantErr) + } + } +} diff --git a/internal/osupdate/leg.go b/internal/osupdate/leg.go index 318979b..80116f0 100644 --- a/internal/osupdate/leg.go +++ b/internal/osupdate/leg.go @@ -370,7 +370,8 @@ func HealthVerdict(before, after *Health) (bool, string) { // HostHealthVerdict is THE host health rule (`11` §8.2; pinned by TestHostHealthVerdict): the Proxmox daemons and the // agent are active, the customer guest still runs, the guest's own rule still passes against the start of the pass, -// and the tunnel is `running` (R-841). +// and the tunnel is `running` (R-841) — or `absent` both now and before the update: a box with no tunnel set up +// (R-1070; pinned by TestHostHealthVerdict_R1070_NoTunnelSetUp). func HostHealthVerdict(before, after *Health, tunnel string) (bool, string) { if after == nil { return false, "no health reading" @@ -398,12 +399,28 @@ func HostHealthVerdict(before, after *Health, tunnel string) (bool, string) { if ok, why := HealthVerdict(gb, after.Guest); !ok { return false, "guest: " + why } - if tunnel != hub.TunnelRunning { + switch { + case tunnel == hub.TunnelRunning: + case tunnel == hub.TunnelAbsent && noTunnelBefore(before): + // R-1070: no cloudflared container before the update and none after — the box has no tunnel to check. + case tunnel == hub.TunnelAbsent: + return false, "the tunnel is absent (no cloudflared container now, and the reading from before the update does not show it was absent then)" + default: return false, "the tunnel is " + tunnel } return true, "" } +// noTunnelBefore: the reading from the start of the pass shows the guest's Docker answered and listed NO cloudflared +// container. A missing or failed reading is not „no tunnel" (an empty list from a failed `docker ps` must not pass). +func noTunnelBefore(before *Health) bool { + if before == nil || before.Guest == nil || !before.Guest.DockerOK { + return false + } + _, had := before.Guest.Containers["cloudflared"] + return !had +} + // EngineOf is the engine version `docker version` prints for a docker-ce package version: "5:29.8.2-1~debian.13~trixie" // → "29.8.2" (no epoch, no Debian revision). func EngineOf(pkgVersion string) string { diff --git a/internal/osupdate/leg_test.go b/internal/osupdate/leg_test.go index 6703635..d4b1490 100644 --- a/internal/osupdate/leg_test.go +++ b/internal/osupdate/leg_test.go @@ -568,3 +568,42 @@ func TestDocker_OOMCheckReachesTheHubUnchanged(t *testing.T) { t.Fatalf("no docker report posted: %s", calls(w)) } } + +// R-1070: a box with no tunnel set up (no cloudflared container before the update and none after) passes the host +// rule; a box WITH a tunnel that is down, gone, or unreadable still fails it. +// +// COMPANION RED-PROOF (observed 2026-10-19): with the „not applicable" branch switched off (only `running` passes, the +// pre-fix rule) the first case fails with +// +// leg_test.go: no tunnel before, none after: got false (the tunnel is absent (no cloudflared container now, …)), want true +// +// and with the pre-fix case-sensitive match TestClassifyTunnel's Docker 29 row reads `unknown` („could not ask the +// guest"), the state Tester4 failed on. +// Restored. +func TestHostHealthVerdict_R1070_NoTunnelSetUp(t *testing.T) { + withTunnel := hostOK() + withTunnel.Guest.Containers["cloudflared"] = Container{State: "running", Health: "healthy"} + dockerFailed := hostOK() + dockerFailed.Guest = &Health{DockerOK: false, Containers: map[string]Container{}} + cases := []struct { + name string + before *Health + tunnel string + want bool + why string + }{ + {"no tunnel before, none after", hostOK(), hub.TunnelAbsent, true, ""}, + {"a tunnel before, gone after", withTunnel, hub.TunnelAbsent, false, "cloudflared was running"}, + {"a tunnel before, down after", withTunnel, hub.TunnelNotRunning, false, "cloudflared was running"}, + {"no tunnel before, a stopped one after", hostOK(), hub.TunnelNotRunning, false, "not_running"}, + {"no tunnel before, the probe could not ask after", hostOK(), hub.TunnelUnknown, false, "unknown"}, + {"no reading from before", nil, hub.TunnelAbsent, false, "absent"}, + {"docker failed before (an empty list is not proof of none)", dockerFailed, hub.TunnelAbsent, false, "absent"}, + } + for _, c := range cases { + got, why := HostHealthVerdict(c.before, hostOK(), c.tunnel) + if got != c.want || !strings.Contains(why, c.why) { + t.Errorf("%s: got %v (%s), want %v (…%s…)", c.name, got, why, c.want, c.why) + } + } +} diff --git a/internal/proxmox/mutate.go b/internal/proxmox/mutate.go index d05d6a1..d2f6c18 100644 --- a/internal/proxmox/mutate.go +++ b/internal/proxmox/mutate.go @@ -180,6 +180,18 @@ func (c *Client) DeleteSnapshot(ctx context.Context, vmid int, snapname string) return c.dataString(ctx, http.MethodDelete, path, nil) } +// DeleteSnapshotForce is DeleteSnapshot with Proxmox's own `force=1` („remove from the config file, even if removing +// disk snapshots fails"): the API's flag, so the same token privilege (VM.Snapshot) and no new root command. Used by +// the stale-lock janitor ONLY after a delete failed because one disk's snapshot volume is already gone (R-1010). The +// flag rides the query string, where pveproxy reads a DELETE's parameters. Pinned by TestDeleteSnapshotForce_R1010. +func (c *Client) DeleteSnapshotForce(ctx context.Context, vmid int, snapname string) (string, error) { + if vmid == 0 || snapname == "" { + return "", fmt.Errorf("proxmox: DeleteSnapshotForce needs vmid and snapname") + } + path := fmt.Sprintf("/nodes/%s/lxc/%d/snapshot/%s?force=1", c.node, vmid, url.PathEscape(snapname)) + return c.dataString(ctx, http.MethodDelete, path, nil) +} + // SetConfig applies config changes via PUT /nodes/{node}/lxc/{vmid}/config // (e.g. memory, cores, net0, mpN with a backup flag). PVE may apply this // synchronously (no UPID) — the returned string is empty in that case, and "" is diff --git a/internal/proxmox/scratch_destroy_test.go b/internal/proxmox/scratch_destroy_test.go index 9621123..e94b623 100644 --- a/internal/proxmox/scratch_destroy_test.go +++ b/internal/proxmox/scratch_destroy_test.go @@ -3,6 +3,7 @@ package proxmox import ( "context" "io" + "net/http" "strings" "testing" ) @@ -99,3 +100,24 @@ func TestDestroyScratchLXC_RefusesUnconfiguredBand(t *testing.T) { } } } + +// R-1010: the forced snapshot delete is the API's own flag on the same DELETE (no new root command). +func TestDeleteSnapshotForce_R1010(t *testing.T) { + var got *http.Request + d := &mockDoer{fn: func(r *http.Request) (*http.Response, error) { + got = r + return jsonResp(200, `{"data":"UPID:demo-felhom:0001:0002:0003:vzdelsnapshot:9201:u@pve!t:"}`), nil + }} + upid, err := newTestClient(d).DeleteSnapshotForce(context.Background(), 9201, "vzdump") + if err != nil || upid == "" { + t.Fatalf("DeleteSnapshotForce: upid %q err %v", upid, err) + } + if got.Method != http.MethodDelete || got.URL.Path != "/api2/json/nodes/demo-felhom/lxc/9201/snapshot/vzdump" || + got.URL.Query().Get("force") != "1" { + t.Fatalf("request = %s %s, want DELETE …/lxc/9201/snapshot/vzdump?force=1", got.Method, got.URL.String()) + } + d.fn = func(r *http.Request) (*http.Response, error) { got = r; return jsonResp(200, `{"data":null}`), nil } + if _, err := newTestClient(d).DeleteSnapshot(context.Background(), 9201, "vzdump"); err != nil || got.URL.RawQuery != "" { + t.Fatalf("the plain delete must carry no force: %s (err %v)", got.URL.String(), err) + } +}