Unreleased: host_busy on GET /backup/status (R-921), a missing tunnel container is 'absent' and not-applicable after an OS update (R-1070), the janitor forces a half-removed vzdump snapshot once (R-1010)
gates / gates (push) Successful in 58s
gates / gates (push) Successful in 58s
This commit is contained in:
1 parent
0cbd500154
commit
56aabc729c
13 files changed
+323
-10
No files matched your search
@@ -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)
|
||||
}
|
||||
|
||||
|
||||
@@ -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 = <nil>, 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)
|
||||
}
|
||||
}
|
||||
@@ -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 {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user