diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d19540..1acf377 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,51 @@ +## v0.104.0-dev — R-85 Phase 2: tier rotation, persisted state, one heavy op at a time (2026-07-26) + +The scheduler could only ever see `cfg.Backup.BackupTarget()`, so the offsite tier's archives were +never candidates. That is why demo-hp's DR tier reported `applied` with **zero snapshots for five +days** and nobody noticed. + +### Selection — oldest-first (operator ruling 2026-07-26, Option 1) +The tier whose last **successful** restore-test is oldest goes first; **never-proven sorts first of +all**, which is exactly where the offsite tier starts. Self-balancing, no new config knob, and each +tier is covered every ~2 cadences — comfortably inside the 2-week offsite retention, so a tier is +never proven against an archive that is about to be pruned. Ties break on target id, because two +tiers proven in the same second would otherwise rotate by Go's randomised map order: untestable, and +occasionally starving. + +Rotation credit is given **only on success**. A tier that fails every time must keep sorting first — +otherwise a permanently broken tier would look freshly proven and quietly stop being retried. + +### Added +- **`backup.RestoreTestState`** — last successful restore-test per tier, persisted (atomic + tmp+rename). **This one genuinely needs persistence, unlike R-84**, and the difference is worth + stating because they look alike: R-84 had a GROUND TRUTH to consult (the archive is still on the + storage), so it never persisted anything. A restore-test destroys its scratch as its final act and + leaves no artifact — "did we prove this tier restores?" exists only as remembered state. A corrupt + or missing file degrades to "nothing proven", which is the correct starting point. +- **`backup.InFlight`** — the host-wide one-heavy-operation gate, shared by the restore-test + scheduler and the local-API backup path. Not a lock concern (the scratch VMID never touches the + live guest's vzdump lock) but a **LINK** concern: an offsite restore PULLS multi-GB over the same + tunnel an offsite backup PUSHES one. At the ~33 MB/min measured upstream, running both drives each + toward its timeout — which is how a healthy tier ends up recorded as failed. A caller that cannot + acquire **defers**; it never cancels what is already running. +- **`BackupRunner.PickRestoreCandidateOn`** — newest archive on a NAMED tier. `""` + nil error when + that tier holds none: **a tier with nothing to restore is not an error**, or every fresh box would + look broken for its first week. + +### Changed +- A tier with no archive is **skipped and the next tier tried**, not left to burn the cadence. It + cannot starve either — an empty tier is still the least recently proven, so it still sorts first + the moment it has an archive. +- `POST /backup` now also joins the gate: a **409** naming the holder when a restore-test is running. + +### Tests ++12. Red-proofs observed: +- **A** — the single-target picker yields `both tiers must be exercised across 4 cadences; got [local:… local:… local:… local:…]`. +- **E** — losing the state file yields `after a restart the OTHER tier must be next; got … twice (rotation state was lost)`. +- **F** — removing the gate yields `the restore-test must DEFER while a backup holds the gate; concurrent operations = 2`. **The count is the assertion** — "both completed" would pass against a fully concurrent implementation. + +Full suite green (29 packages, `rc=0`, vet unpiped). + ## v0.104.0-dev — R-85 Phase 1: the restore-test spec is built PER RUN (2026-07-26) Prerequisite for scheduling the offsite tier at all. Shipped on its own because it is independently diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index ec1469a..180f1d1 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -649,7 +649,11 @@ func runDaemon(cfg config.Config, logger *slog.Logger, logRing *applog.Ring) int // Self-restore-test scheduler (slice 6): the fourth daemon goroutine. Runs the restore- // test on the configured cadence (default 24h). Disabled cleanly when the cadence is off // OR the scratch band / restore storage is misconfigured — the daemon still runs. - scheduler := buildRestoreTestScheduler(cfg, px, engine, backupStore, logger) + // R-85: persisted per-tier restore-test state + the host-wide one-heavy-op gate, both shared + // with the local API so a backup and a restore-test can never run together. + rtState := backup.NewRestoreTestState(filepath.Join(cfg.OOB.WithDefaults().StateDir, "restore-test-state.json")) + heavyOps := &backup.InFlight{} + scheduler := buildRestoreTestScheduler(cfg, px, engine, backupStore, rtState, heavyOps, logger) // PBS verify loop (slice 6 Phase B): the fifth daemon goroutine. Cheap, key-free, // ciphertext-level integrity check on its own cadence (default 6h), reporting per-snapshot @@ -757,7 +761,7 @@ func runDaemon(cfg config.Config, logger *slog.Logger, logRing *applog.Ring) int return false }, } - localSrv := buildLocalAPIServer(cfg, px, backupStore, observer, driveKnown, hostOps, gate, collector, intentRec, guestBindStore, formatJobStore, logRing, escrowCeremonyCfg, logger, &localTokens) + localSrv := buildLocalAPIServer(cfg, px, backupStore, heavyOps, observer, driveKnown, hostOps, gate, collector, intentRec, guestBindStore, formatJobStore, logRing, escrowCeremonyCfg, logger, &localTokens) if localTokens != nil { defer localTokens.Close() } @@ -1232,7 +1236,7 @@ func readTrimmed(path string) (string, error) { // disables the cadence (returns a scheduler that just waits) when the cadence is off or the // scratch band / restore storage is invalid — a misconfig must not crash the daemon, and the // machinery still works on-demand via --selftest=restore-test. -func buildRestoreTestScheduler(cfg config.Config, px *proxmox.Client, engine *reconcile.Engine, store *backup.Store, logger *slog.Logger) *backup.Scheduler { +func buildRestoreTestScheduler(cfg config.Config, px *proxmox.Client, engine *reconcile.Engine, store *backup.Store, rtState *backup.RestoreTestState, inFlight *backup.InFlight, logger *slog.Logger) *backup.Scheduler { cadence := cfg.Backup.RestoreTestCadence() if cadence > 0 { if err := cfg.Backup.ValidateForRestoreTest(); err != nil { @@ -1243,6 +1247,12 @@ func buildRestoreTestScheduler(cfg config.Config, px *proxmox.Client, engine *re min, max := cfg.Backup.ScratchBand() target := cfg.Backup.BackupTarget() runner := backup.NewBackupRunner(px, target, "", "felhom restore-test", "", logger) + // Every configured tier is a rotation candidate, not just the primary. + cfgTiers, _ := cfg.Backup.BackupTiers() // warnings already logged where the tiers are armed + tierIDs := make([]string, 0, len(cfgTiers)) + for _, t := range cfgTiers { + tierIDs = append(tierIDs, t.TargetID) + } return backup.NewScheduler(backup.SchedulerOptions{ Runner: engine, Pick: runner.PickRestoreCandidate, @@ -1270,6 +1280,14 @@ func buildRestoreTestScheduler(cfg config.Config, px *proxmox.Client, engine *re }, Cadence: cadence, Logger: logger, + + // R-85: rotate across EVERY configured tier, oldest-proven first (operator ruling, Option 1). + // Before this the scheduler only ever saw cfg.Backup.BackupTarget(), so the offsite tier's + // archives were never candidates and the DR tier went unproven for its whole existence. + Tiers: tierIDs, + TierPick: runner.PickRestoreCandidateOn, + State: rtState, + InFlight: inFlight, }) } @@ -1278,7 +1296,7 @@ func buildRestoreTestScheduler(cfg config.Config, px *proxmox.Client, engine *re // leaf (stable fingerprint). Any failure DISABLES the server (returns nil) WITHOUT crashing the // daemon — the host still reports/reconciles; only the controller channel is unavailable until // fixed. The opened token store is returned via outTokens so the caller can Close it. -func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.Store, observer *storage.Observer, driveTargets storage.KnownTargets, hostOps *storage.SudoHostOps, gate *reconcile.Gate, collector *hub.Collector, intent localapi.IntentRecorder, guestBinds *localapi.GuestBindStore, formatJobs *localapi.FormatJobStore, logRing *applog.Ring, escrowCeremony *localapi.EscrowCeremonyConfig, logger *slog.Logger, outTokens **localapi.TokenStore) *localapi.Server { +func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.Store, inFlight *backup.InFlight, observer *storage.Observer, driveTargets storage.KnownTargets, hostOps *storage.SudoHostOps, gate *reconcile.Gate, collector *hub.Collector, intent localapi.IntentRecorder, guestBinds *localapi.GuestBindStore, formatJobs *localapi.FormatJobStore, logRing *applog.Ring, escrowCeremony *localapi.EscrowCeremonyConfig, logger *slog.Logger, outTokens **localapi.TokenStore) *localapi.Server { if !cfg.LocalAPI.Enabled() { return nil } @@ -1357,6 +1375,7 @@ func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.St Guests: px, Backups: runner, BackupTiers: apiTiers, // R-82: primary first; untargeted endpoints act on the primary + InFlight: inFlight, // R-85: shared with the restore-test scheduler (Scenario F) Store: store, Storage: observer, DriveTargets: driveTargets, // Impl-2a: registry+units drives for the /disks view (union w/ Observe storages) @@ -1846,7 +1865,7 @@ func runSelftestBringUp(ctx context.Context, cfg config.Config, logger *slog.Log Cores: sizing.Cores, MemoryMB: sizing.MemoryMB, RootfsGrowGB: sizing.RootfsGrowGB, DataVolGrowGB: sizing.DataVolGrowGB, DataVolMount: sizing.DataVolMount, SysDataGrowGB: sizing.SysDataGrowGB, SysDataMount: sizing.SysDataMount, - IslandBridge: cfg.LocalAPI.IslandBridge, IslandGuestAddr: cfg.LocalAPI.IslandGuestAddr, // R-50 island NIC (both empty = pre-R-50) + IslandBridge: cfg.LocalAPI.IslandBridge, IslandGuestAddr: cfg.LocalAPI.IslandGuestAddr, // R-50 island NIC (both empty = pre-R-50) } fmt.Printf(" bringing up %s → vmid %d on %s …\n", archive, vmid, cfg.Backup.RestoreStorage) res := engine.RunBringUp(ctx, spec) @@ -2010,7 +2029,7 @@ func runSelftestProvision(ctx context.Context, cfg config.Config, logger *slog.L Cores: a.sizing.Cores, MemoryMB: a.sizing.MemoryMB, RootfsGrowGB: a.sizing.RootfsGrowGB, DataVolGrowGB: a.sizing.DataVolGrowGB, DataVolMount: a.sizing.DataVolMount, SysDataGrowGB: a.sizing.SysDataGrowGB, SysDataMount: a.sizing.SysDataMount, - IslandBridge: cfg.LocalAPI.IslandBridge, IslandGuestAddr: cfg.LocalAPI.IslandGuestAddr, // R-50 island NIC (both empty = pre-R-50) + IslandBridge: cfg.LocalAPI.IslandBridge, IslandGuestAddr: cfg.LocalAPI.IslandGuestAddr, // R-50 island NIC (both empty = pre-R-50) }) if res.Err != nil || !res.Pass { fmt.Fprintf(os.Stderr, " [FAIL] front-half bring-up (vmid %d): %v\n", a.vmid, res.Err) diff --git a/internal/backup/inflight.go b/internal/backup/inflight.go new file mode 100644 index 0000000..af543f0 --- /dev/null +++ b/internal/backup/inflight.go @@ -0,0 +1,57 @@ +package backup + +import "sync" + +// InFlight is the host-wide "one heavy guest operation at a time" gate. +// +// R-85 (Scenario F). The operator's R-82 ruling was "one backup at a time per guest"; a restore-test +// must JOIN that single-flight rather than sit outside it. It is not a lock-contention concern — +// a restore-test uses a scratch VMID, so it never touches the live guest's vzdump lock. It is a +// LINK concern: an offsite restore PULLS a multi-GB archive while an offsite backup PUSHES one, over +// the same WireGuard tunnel. On the demo fleet that link runs at ~33 MB/min upstream; running both +// at once makes each slower and pushes both toward their timeouts, which is how a healthy tier ends +// up recorded as failed. +// +// It is deliberately host-wide and coarse rather than per-guest: these boxes carry one customer +// guest, and the resource being protected (the uplink) is shared by everything on the host anyway. +// +// The gate is ADVISORY in one direction only — it never cancels anything already running. A caller +// that cannot acquire DEFERS to its next cadence. Deferring a restore-test costs a few hours of +// coverage; cancelling a running backup costs the backup. +type InFlight struct { + mu sync.Mutex + what string // "" = idle +} + +// TryAcquire claims the gate for `what`. ok=false means something else holds it, and `busy` names +// it — the name matters, because "deferred" with no reason is indistinguishable from "broken". +func (g *InFlight) TryAcquire(what string) (release func(), busy string, ok bool) { + if g == nil { + // Not wired (older call sites, tests) → no gating, previous behaviour. + return func() {}, "", true + } + g.mu.Lock() + defer g.mu.Unlock() + if g.what != "" { + return nil, g.what, false + } + g.what = what + var once sync.Once + return func() { + once.Do(func() { + g.mu.Lock() + g.what = "" + g.mu.Unlock() + }) + }, "", true +} + +// Busy reports what currently holds the gate ("" = idle). +func (g *InFlight) Busy() string { + if g == nil { + return "" + } + g.mu.Lock() + defer g.mu.Unlock() + return g.what +} diff --git a/internal/backup/restoretest_state.go b/internal/backup/restoretest_state.go new file mode 100644 index 0000000..e078521 --- /dev/null +++ b/internal/backup/restoretest_state.go @@ -0,0 +1,139 @@ +package backup + +import ( + "encoding/json" + "os" + "path/filepath" + "sort" + "sync" + "time" +) + +// RestoreTestState persists the last SUCCESSFUL restore-test per backup tier. +// +// R-85 (1.4). This one genuinely needs PERSISTENCE, unlike R-84 — and the difference is worth +// stating, because the two look like the same problem and are not: +// +// - R-84 (backup freshness) had a GROUND TRUTH to consult: the archive is still on the storage, +// so the agent could ask "when did a backup last land?" and never persist anything. That is +// strictly better, because a pruned archive correctly stops counting. +// - A restore-test leaves NO artifact — the scratch guest is destroyed as its final act. There is +// nothing to query. "Did we prove this tier restores?" exists only as remembered state, so it +// must be written down or it is lost. +// +// Why it must survive a restart: rotation is oldest-first (the operator ruling), so an in-memory map +// would reset every tier to "never tested" on each restart. Ordering would then depend on map +// iteration order, and one tier could be starved indefinitely while the other is re-tested — with +// agent deploys as routine as they are, that is not a corner case. +// +// Only SUCCESS is recorded. A failed run must not satisfy rotation, or a tier that fails every time +// would look freshly proven and stop being retried — the same "a failure satisfies the cadence" +// trap the backup due-check avoids. +type RestoreTestState struct { + path string + mu sync.Mutex + last map[string]time.Time // target id → last SUCCESSFUL restore-test (UTC) +} + +// NewRestoreTestState opens (or creates) the state at path. A missing or unreadable file is NOT an +// error: it degrades to "nothing proven yet", which is the correct starting point and keeps a +// corrupt file from wedging the daemon. +func NewRestoreTestState(path string) *RestoreTestState { + s := &RestoreTestState{path: path, last: map[string]time.Time{}} + data, err := os.ReadFile(path) + if err != nil { + return s + } + var raw map[string]string + if json.Unmarshal(data, &raw) != nil { + return s + } + for target, ts := range raw { + if t, perr := time.Parse(time.RFC3339, ts); perr == nil { + s.last[target] = t.UTC() + } + } + return s +} + +// RecordSuccess stamps a tier as proven at t. Only call this for a PASSING restore-test. +func (s *RestoreTestState) RecordSuccess(target string, t time.Time) error { + if target == "" { + return nil + } + s.mu.Lock() + defer s.mu.Unlock() + s.last[target] = t.UTC() + return s.saveLocked() +} + +// LastSuccess returns when this tier was last proven (ok=false = never). +func (s *RestoreTestState) LastSuccess(target string) (time.Time, bool) { + s.mu.Lock() + defer s.mu.Unlock() + t, ok := s.last[target] + return t, ok +} + +// Snapshot returns a copy of the whole map — for the host-report gauge. +func (s *RestoreTestState) Snapshot() map[string]time.Time { + s.mu.Lock() + defer s.mu.Unlock() + out := make(map[string]time.Time, len(s.last)) + for k, v := range s.last { + out[k] = v + } + return out +} + +// OldestFirst orders targets by "least recently proven first"; never-proven sorts FIRST. +// +// This is the operator's 2026-07-26 ruling (Option 1): self-balancing, no new config knob, and it +// naturally prioritises a tier that has never been restore-tested at all — which on this fleet was +// the offsite tier, unproven for its entire existence. +// +// Ties break on target id so the order is deterministic; without that, two tiers proven in the same +// second would rotate by map iteration order, which is randomised in Go and would make the +// behaviour untestable and occasionally starving. +func (s *RestoreTestState) OldestFirst(targets []string) []string { + s.mu.Lock() + defer s.mu.Unlock() + out := append([]string(nil), targets...) + sort.SliceStable(out, func(i, j int) bool { + ti, oki := s.last[out[i]] + tj, okj := s.last[out[j]] + switch { + case !oki && !okj: + return out[i] < out[j] // both never proven → deterministic + case !oki: + return true // never proven wins + case !okj: + return false + case !ti.Equal(tj): + return ti.Before(tj) + default: + return out[i] < out[j] + } + }) + return out +} + +func (s *RestoreTestState) saveLocked() error { + raw := make(map[string]string, len(s.last)) + for target, t := range s.last { + raw[target] = t.UTC().Format(time.RFC3339) + } + data, err := json.MarshalIndent(raw, "", " ") + if err != nil { + return err + } + if err := os.MkdirAll(filepath.Dir(s.path), 0o755); err != nil { + return err + } + tmp := s.path + ".tmp" + if err := os.WriteFile(tmp, data, 0o600); err != nil { + os.Remove(tmp) + return err + } + return os.Rename(tmp, s.path) +} diff --git a/internal/backup/rotation_test.go b/internal/backup/rotation_test.go new file mode 100644 index 0000000..e10a177 --- /dev/null +++ b/internal/backup/rotation_test.go @@ -0,0 +1,335 @@ +package backup + +import ( + "context" + "os" + "path/filepath" + "sync" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-agent/internal/reconcile" +) + +// R-85 Phase 2 — tier rotation, persisted state, and the one-heavy-operation gate. +// +// The failure this prevents is not hypothetical: demo-hp's DR tier reported `applied` with ZERO +// snapshots for five days and nobody noticed, because the scheduler could only ever see the primary +// tier. Rotation is what makes the offsite tier testable at all. + +// rotRunner records which archives it was asked to restore. +type rotRunner struct { + mu sync.Mutex + archives []string + pass bool +} + +func (r *rotRunner) RunRestoreTest(_ context.Context, spec reconcile.RestoreTestSpec) reconcile.RestoreTestResult { + r.mu.Lock() + defer r.mu.Unlock() + r.archives = append(r.archives, spec.Archive) + return reconcile.RestoreTestResult{ + Archive: spec.Archive, SourceTier: spec.SourceTier, + Pass: r.pass, Verified: "boot+running", + } +} +func (r *rotRunner) seen() []string { + r.mu.Lock() + defer r.mu.Unlock() + return append([]string(nil), r.archives...) +} + +// archiveFor is a TierPicker over a fixed map: target → archive ("" = that tier holds none). +func archiveFor(m map[string]string) TierPicker { + return func(_ context.Context, target string) (string, error) { + return m[target], nil + } +} + +func rotScheduler(t *testing.T, rr *rotRunner, st *RestoreTestState, pick TierPicker, gate *InFlight) *Scheduler { + t.Helper() + return NewScheduler(SchedulerOptions{ + Runner: rr, + Store: NewStore(), + Spec: func(_ context.Context, archive string) reconcile.RestoreTestSpec { + return reconcile.RestoreTestSpec{RestoreStorage: "local-lvm", ScratchMin: 990000, ScratchMax: 990009} + }, + Cadence: time.Hour, + Logger: quiet(), + Tiers: []string{"local", "felhom-pbs"}, + TierPick: pick, + State: st, + InFlight: gate, + }) +} + +// ── SCENARIO A — both tiers get tested across consecutive cadences ─────────────────────────── +// +// COMPANION RED-PROOF (observed): restore the single-target picker — set `Tiers`/`TierPick` to nil +// so `pickForThisRun` falls back to `s.pick` on the primary runner — and this fails with +// "both tiers must be exercised across 4 cadences; got [local:… local:… local:… local:…]", +// i.e. the offsite tier never appears. That is today's behaviour, and it is why demo-hp's DR tier +// went unproven for its entire existence. +func TestRotation_BothTiersExercisedAcrossCadences(t *testing.T) { + rr := &rotRunner{pass: true} + st := NewRestoreTestState(filepath.Join(t.TempDir(), "rt.json")) + s := rotScheduler(t, rr, st, archiveFor(map[string]string{ + "local": "local:backup/vzdump-lxc-9201-x.tar.zst", + "felhom-pbs": "felhom-pbs:backup/ct/9201/2026-07-26T15:42:42Z", + }), &InFlight{}) + s.now = func() time.Time { return time.Now().UTC() } + + for i := 0; i < 4; i++ { + s.tick(context.Background()) + } + + got := rr.seen() + var sawLocal, sawPBS bool + for _, a := range got { + if len(a) >= 5 && a[:5] == "local" { + sawLocal = true + } + if len(a) >= 10 && a[:10] == "felhom-pbs" { + sawPBS = true + } + } + if !sawLocal || !sawPBS { + t.Fatalf("both tiers must be exercised across 4 cadences; got %v", got) + } + // Oldest-first must ALTERNATE, not clump — otherwise one tier is starved between visits. + if len(got) != 4 { + t.Fatalf("want 4 runs, got %d: %v", len(got), got) + } + if got[0] == got[1] { + t.Fatalf("consecutive runs hit the same tier — oldest-first is not rotating: %v", got) + } +} + +// A tier with NO archive is skipped, not failed, and the other tier still runs. A brand-new offsite +// tier legitimately has nothing to restore; turning that into a failure would make every fresh box +// look broken for its first week. +func TestRotation_EmptyTierSkippedNotFailed(t *testing.T) { + rr := &rotRunner{pass: true} + st := NewRestoreTestState(filepath.Join(t.TempDir(), "rt.json")) + s := rotScheduler(t, rr, st, archiveFor(map[string]string{ + "local": "local:backup/vzdump-lxc-9201-x.tar.zst", + "felhom-pbs": "", // provisioned but empty + }), &InFlight{}) + + s.tick(context.Background()) + got := rr.seen() + if len(got) != 1 || got[0][:5] != "local" { + t.Fatalf("an empty tier must be skipped and the testable one still run; got %v", got) + } +} + +// Nothing testable anywhere → a clean no-op, not an error and not a run. +func TestRotation_NoArchivesAnywhereIsANoOp(t *testing.T) { + rr := &rotRunner{pass: true} + s := rotScheduler(t, rr, NewRestoreTestState(filepath.Join(t.TempDir(), "rt.json")), + archiveFor(map[string]string{}), &InFlight{}) + s.tick(context.Background()) + if got := rr.seen(); len(got) != 0 { + t.Fatalf("no archives anywhere → no run; got %v", got) + } +} + +// A FAILED restore-test must NOT earn rotation credit, or a tier that fails every time would look +// freshly proven and quietly stop being retried. +func TestRotation_FailureEarnsNoCredit(t *testing.T) { + rr := &rotRunner{pass: false} + st := NewRestoreTestState(filepath.Join(t.TempDir(), "rt.json")) + s := rotScheduler(t, rr, st, archiveFor(map[string]string{ + "local": "local:backup/x.tar.zst", + "felhom-pbs": "felhom-pbs:backup/ct/9201/y", + }), &InFlight{}) + s.tick(context.Background()) + if _, ok := st.LastSuccess("local"); ok { + t.Fatal("a FAILED restore-test must not stamp the tier as proven") + } + if _, ok := st.LastSuccess("felhom-pbs"); ok { + t.Fatal("a FAILED restore-test must not stamp the tier as proven") + } +} + +// ── SCENARIO E — rotation survives a restart ───────────────────────────────────────────────── +// +// COMPANION RED-PROOF (observed): make the state in-memory (construct a fresh +// `NewRestoreTestState` on a DIFFERENT path for the second scheduler, i.e. lose the file) and this +// fails with "after a restart the OTHER tier must be next; got felhom-pbs" — the same tier repeats +// and the other is starved indefinitely, which with agent deploys as routine as they are is not a +// corner case. +func TestRotation_SurvivesRestart(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "rt.json") + picks := archiveFor(map[string]string{ + "local": "local:backup/x.tar.zst", + "felhom-pbs": "felhom-pbs:backup/ct/9201/y", + }) + + // First process: the OFFSITE tier is tested (never-proven sorts first). + rr1 := &rotRunner{pass: true} + st1 := NewRestoreTestState(path) + s1 := rotScheduler(t, rr1, st1, picks, &InFlight{}) + s1.tick(context.Background()) + first := rr1.seen() + if len(first) != 1 { + t.Fatalf("want one run, got %v", first) + } + + // --- restart: brand-new state object reading the SAME file --- + rr2 := &rotRunner{pass: true} + st2 := NewRestoreTestState(path) + s2 := rotScheduler(t, rr2, st2, picks, &InFlight{}) + s2.tick(context.Background()) + second := rr2.seen() + if len(second) != 1 { + t.Fatalf("want one run after restart, got %v", second) + } + + if second[0] == first[0] { + t.Fatalf("after a restart the OTHER tier must be next; got %s twice (rotation state was lost)", second[0]) + } +} + +// ── SCENARIO F — no collision with a backup ────────────────────────────────────────────────── +// +// COMPANION RED-PROOF (observed): drop the TryAcquire guard from `tick` and this fails with +// "the restore-test must DEFER while a backup holds the gate; concurrent operations = 2" — the +// count is the assertion, since "both completed" would pass against a fully concurrent +// implementation. +func TestRotation_DefersWhileABackupHoldsTheGate(t *testing.T) { + gate := &InFlight{} + release, _, ok := gate.TryAcquire("backup:felhom-pbs") + if !ok { + t.Fatal("precondition: the gate should have been free") + } + defer release() + + rr := &rotRunner{pass: true} + s := rotScheduler(t, rr, NewRestoreTestState(filepath.Join(t.TempDir(), "rt.json")), + archiveFor(map[string]string{"local": "local:backup/x.tar.zst"}), gate) + + s.tick(context.Background()) + + concurrent := 1 + len(rr.seen()) // the backup holding the gate, plus anything the tick started + if concurrent != 1 { + t.Fatalf("the restore-test must DEFER while a backup holds the gate; concurrent operations = %d", concurrent) + } +} + +// Once the backup releases, the next cadence proceeds — deferral must not be permanent. +func TestRotation_ResumesAfterTheGateFrees(t *testing.T) { + gate := &InFlight{} + release, _, _ := gate.TryAcquire("backup:local") + + rr := &rotRunner{pass: true} + s := rotScheduler(t, rr, NewRestoreTestState(filepath.Join(t.TempDir(), "rt.json")), + archiveFor(map[string]string{"local": "local:backup/x.tar.zst"}), gate) + + s.tick(context.Background()) + if len(rr.seen()) != 0 { + t.Fatal("should have deferred while held") + } + release() + s.tick(context.Background()) + if len(rr.seen()) != 1 { + t.Fatalf("must resume once the gate frees; got %v", rr.seen()) + } +} + +// The gate itself: one holder at a time, named, and release is idempotent. +func TestInFlight_Semantics(t *testing.T) { + g := &InFlight{} + rel, busy, ok := g.TryAcquire("backup:local") + if !ok || busy != "" { + t.Fatalf("first acquire must succeed; ok=%v busy=%q", ok, busy) + } + if _, busy2, ok2 := g.TryAcquire("restore-test"); ok2 || busy2 != "backup:local" { + t.Fatalf("second acquire must fail and NAME the holder; ok=%v busy=%q", ok2, busy2) + } + rel() + rel() // idempotent — a double release must not free someone else's later claim + if g.Busy() != "" { + t.Fatalf("gate should be idle after release; busy=%q", g.Busy()) + } + if _, _, ok3 := g.TryAcquire("restore-test"); !ok3 { + t.Fatal("gate must be reusable after release") + } +} + +// A nil gate means "not wired" → no gating, pre-R-85 behaviour. Keeps every existing caller working. +func TestInFlight_NilIsUngated(t *testing.T) { + var g *InFlight + rel, _, ok := g.TryAcquire("x") + if !ok { + t.Fatal("a nil gate must not block") + } + rel() + if g.Busy() != "" { + t.Fatal("a nil gate is never busy") + } +} + +// ── oldest-first ordering ──────────────────────────────────────────────────────────────────── + +func TestOldestFirst_Ordering(t *testing.T) { + st := NewRestoreTestState(filepath.Join(t.TempDir(), "rt.json")) + now := time.Now().UTC() + + // Never-proven sorts FIRST — the case that matters, since the offsite tier starts there. + if got := st.OldestFirst([]string{"local", "felhom-pbs"}); got[0] != "felhom-pbs" { + // both never proven → deterministic tie-break by id + if got[0] != "felhom-pbs" && got[0] != "local" { + t.Fatalf("unexpected: %v", got) + } + } + _ = st.RecordSuccess("local", now) + if got := st.OldestFirst([]string{"local", "felhom-pbs"}); got[0] != "felhom-pbs" { + t.Fatalf("a never-proven tier must sort before a proven one; got %v", got) + } + _ = st.RecordSuccess("felhom-pbs", now.Add(time.Hour)) + if got := st.OldestFirst([]string{"local", "felhom-pbs"}); got[0] != "local" { + t.Fatalf("the least recently proven must sort first; got %v", got) + } +} + +// Ordering must be DETERMINISTIC for equal timestamps, or two tiers proven in the same second would +// rotate by Go's randomised map iteration — untestable, and occasionally starving. +func TestOldestFirst_DeterministicOnTies(t *testing.T) { + st := NewRestoreTestState(filepath.Join(t.TempDir(), "rt.json")) + now := time.Now().UTC() + _ = st.RecordSuccess("b-tier", now) + _ = st.RecordSuccess("a-tier", now) + for i := 0; i < 20; i++ { + if got := st.OldestFirst([]string{"b-tier", "a-tier"}); got[0] != "a-tier" { + t.Fatalf("tie-break must be deterministic; iteration %d gave %v", i, got) + } + } +} + +// The state file round-trips, and a corrupt file degrades to "nothing proven" rather than wedging. +func TestRestoreTestState_PersistenceAndCorruption(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "rt.json") + now := time.Now().UTC().Truncate(time.Second) + + st := NewRestoreTestState(path) + if err := st.RecordSuccess("felhom-pbs", now); err != nil { + t.Fatal(err) + } + reopened := NewRestoreTestState(path) + got, ok := reopened.LastSuccess("felhom-pbs") + if !ok || !got.Equal(now) { + t.Fatalf("state must round-trip; got %v ok=%v want %v", got, ok, now) + } + + bad := filepath.Join(dir, "corrupt.json") + if err := os.WriteFile(bad, []byte("{{{not json"), 0o600); err != nil { + t.Fatal(err) + } + c := NewRestoreTestState(bad) + if _, ok := c.LastSuccess("felhom-pbs"); ok { + t.Fatal("a corrupt state file must degrade to 'nothing proven', not invent a timestamp") + } +} diff --git a/internal/backup/runner.go b/internal/backup/runner.go index 5b83a36..63da28d 100644 --- a/internal/backup/runner.go +++ b/internal/backup/runner.go @@ -243,7 +243,20 @@ func (r *BackupRunner) watchForSnapshot(ctx context.Context, upid string, onSnap // PickRestoreCandidate returns the newest backup archive on the target (any guest), or "" // when there is none — the restore-test then no-ops cleanly. func (r *BackupRunner) PickRestoreCandidate(ctx context.Context) (string, error) { - contents, err := r.api.StorageContent(ctx, r.target) + return r.PickRestoreCandidateOn(ctx, r.target) +} + +// PickRestoreCandidateOn is PickRestoreCandidate for an ARBITRARY tier's storage (R-85 1.2), so the +// scheduler can rotate across tiers instead of only ever seeing this runner's own target. +// +// Contract preserved: "" + nil error when the storage holds no archive. **A tier with nothing to +// restore is not an error** — a brand-new offsite tier legitimately has nothing yet, and turning +// that into a failure would make every fresh box look broken for its first week. +func (r *BackupRunner) PickRestoreCandidateOn(ctx context.Context, target string) (string, error) { + if target == "" { + return "", nil + } + contents, err := r.api.StorageContent(ctx, target) if err != nil { return "", err } diff --git a/internal/backup/schedule.go b/internal/backup/schedule.go index 5fd3e3f..28dcbbd 100644 --- a/internal/backup/schedule.go +++ b/internal/backup/schedule.go @@ -32,6 +32,11 @@ type CandidatePicker func(ctx context.Context) (string, error) // PBS archive was classified "local" and got the 10-minute local wait. type SpecBuilder func(ctx context.Context, archive string) reconcile.RestoreTestSpec +// TierPicker resolves the newest archive on a NAMED tier, or "" when that tier holds none. +// (*BackupRunner).PickRestoreCandidateOn satisfies it. "" must NOT be an error — a brand-new +// offsite tier legitimately has nothing to restore yet. +type TierPicker func(ctx context.Context, target string) (string, error) + // Scheduler runs the self-restore-test on an agent-internal cadence. It is the fourth daemon // goroutine; it does real restore→boot→destroy, so it only runs when the cadence is enabled // AND a valid scratch band is configured (validated by the caller before construction). @@ -43,6 +48,13 @@ type Scheduler struct { cadence time.Duration logger *slog.Logger now func() time.Time + + // R-85 tier rotation. All optional: without them the scheduler behaves exactly as before + // (single tier via `pick`), which keeps every existing caller and test working untouched. + tiers []string // configured tier target ids, primary first + tierPick TierPicker // newest archive on a named tier + rtState *RestoreTestState // persisted last-successful-per-tier (drives oldest-first) + inFlight *InFlight // shared with the backup path — Scenario F } // SchedulerOptions configures a Scheduler. @@ -55,6 +67,14 @@ type SchedulerOptions struct { Spec SpecBuilder Cadence time.Duration // 0 → disabled Logger *slog.Logger + + // R-85 (all optional — omit for the pre-R-85 single-tier behaviour): + // Tiers are the configured tier target ids (primary first); TierPick resolves an archive on a + // named tier; State persists last-successful-per-tier; InFlight is the shared one-heavy-op gate. + Tiers []string + TierPick TierPicker + State *RestoreTestState + InFlight *InFlight } // NewScheduler builds a Scheduler. @@ -64,13 +84,17 @@ func NewScheduler(opts SchedulerOptions) *Scheduler { logger = slog.Default() } return &Scheduler{ - runner: opts.Runner, - pick: opts.Pick, - store: opts.Store, - spec: opts.Spec, - cadence: opts.Cadence, - logger: logger, - now: func() time.Time { return time.Now().UTC() }, + runner: opts.Runner, + pick: opts.Pick, + store: opts.Store, + spec: opts.Spec, + cadence: opts.Cadence, + logger: logger, + now: func() time.Time { return time.Now().UTC() }, + tiers: append([]string(nil), opts.Tiers...), + tierPick: opts.TierPick, + rtState: opts.State, + inFlight: opts.InFlight, } } @@ -79,7 +103,7 @@ func NewScheduler(opts SchedulerOptions) *Scheduler { // is heavy; the first runs one interval in) — on-demand runs use the selftest harness. // Returns nil on ctx cancellation. func (s *Scheduler) Run(ctx context.Context) error { - if s.cadence <= 0 || s.runner == nil || s.pick == nil || s.spec == nil { + if s.cadence <= 0 || s.runner == nil || s.spec == nil || (s.pick == nil && !s.rotating()) { s.logger.Info("backup: restore-test cadence disabled") <-ctx.Done() return nil @@ -108,7 +132,19 @@ func (s *Scheduler) tick(ctx context.Context) { s.logger.Error("backup: restore-test has no spec builder — skipping (this is a wiring bug)") return } - archive, err := s.pick(ctx) + // Scenario F: join the one-heavy-operation-at-a-time gate. A restore-test PULLS a multi-GB + // archive over the same tunnel an offsite backup PUSHES one; running both saturates the link and + // drives each toward its timeout, which is how a healthy tier gets recorded as failed. DEFER — + // never cancel what is already running: a deferred restore-test costs hours of coverage, a + // cancelled backup costs the backup. + release, busy, ok := s.inFlight.TryAcquire("restore-test") + if !ok { + s.logger.Info("backup: restore-test deferred — a heavy operation is already in flight", "busy", busy) + return + } + defer release() + + archive, target, err := s.pickForThisRun(ctx) if err != nil { s.logger.Warn("backup: restore-test could not pick a candidate; skipping", "err", err) return @@ -126,6 +162,13 @@ func (s *Scheduler) tick(ctx context.Context) { } rt := ToHubRestoreTest(res, s.now()) s.store.RecordRestoreTest(rt) + // Rotation credit is given ONLY on success. A failing tier must keep sorting first, or a tier + // that fails every time would look freshly proven and quietly stop being retried. + if rt.Pass && s.rtState != nil && target != "" { + if err := s.rtState.RecordSuccess(target, s.now()); err != nil { + s.logger.Warn("backup: could not persist the restore-test rotation state", "target", target, "err", err) + } + } switch { case !rt.Pass: // A failing restore-test is the loudest DR signal there is. @@ -142,3 +185,53 @@ func (s *Scheduler) tick(ctx context.Context) { "archive", rt.SourceArchive, "duration_s", rt.DurationSeconds, "warnings", res.StartWarnings) } } + +// rotating reports whether multi-tier rotation is wired. +func (s *Scheduler) rotating() bool { return len(s.tiers) > 0 && s.tierPick != nil } + +// pickForThisRun chooses the tier and its newest archive. +// +// OLDEST-FIRST (operator ruling 2026-07-26, Option 1): the tier whose last SUCCESSFUL restore-test +// is oldest goes first, never-proven first of all. Self-balancing, no config knob, and it naturally +// prioritises a tier that has never been proven — which on this fleet was the offsite tier, unproven +// for its entire existence while reporting `applied`. +// +// A tier with no archives is SKIPPED, not failed, and the next tier is tried. Skipping to a testable +// tier is strictly better than burning the whole cadence: a brand-new offsite tier has nothing to +// restore yet, and that is normal, not broken. It cannot starve the empty tier either — as soon as +// it has an archive it still sorts first, because it is still the least recently proven. +// +// Returns ("", "", nil) when nothing anywhere is testable. +func (s *Scheduler) pickForThisRun(ctx context.Context) (archive, target string, err error) { + if !s.rotating() { + a, perr := s.pick(ctx) + return a, "", perr // pre-R-85 single-tier path; no rotation credit to record + } + order := s.tiers + if s.rtState != nil { + order = s.rtState.OldestFirst(s.tiers) + } + var firstErr error + for _, t := range order { + a, perr := s.tierPick(ctx, t) + if perr != nil { + // One tier's storage being unreadable must not block the others. + s.logger.Warn("backup: restore-test candidate lookup failed for a tier; trying the next", + "target", t, "err", perr) + if firstErr == nil { + firstErr = perr + } + continue + } + if a == "" { + s.logger.Debug("backup: restore-test tier has no archive yet; trying the next", "target", t) + continue + } + s.logger.Info("backup: restore-test tier selected (oldest-proven first)", "target", t, "archive", a) + return a, t, nil + } + if firstErr != nil { + return "", "", firstErr + } + return "", "", nil +} diff --git a/internal/localapi/server.go b/internal/localapi/server.go index 360961d..5c0d511 100644 --- a/internal/localapi/server.go +++ b/internal/localapi/server.go @@ -16,6 +16,7 @@ import ( "sync" "time" + "gitea.dooplex.hu/admin/felhom-agent/internal/backup" "gitea.dooplex.hu/admin/felhom-agent/internal/escrow" "gitea.dooplex.hu/admin/felhom-agent/internal/hub" applog "gitea.dooplex.hu/admin/felhom-agent/internal/log" @@ -127,6 +128,9 @@ type Options struct { // BackupTiers (R-82) is the resolved multi-tier policy, primary first. OPTIONAL: nil → one tier // synthesized from Backups + BackupCadence, i.e. exactly the pre-R-82 behaviour. BackupTiers []BackupTier + // InFlight (R-85) is the host-wide one-heavy-operation gate shared with the restore-test + // scheduler. OPTIONAL: nil → no cross-gating (pre-R-85 behaviour). See backup.InFlight. + InFlight *backup.InFlight // Disk management (slice 8C) — OPTIONAL. When Disks + DiskGate are set, the /disks endpoints // are served; otherwise they report "not configured". DiskGate authorizes the destructive // (data-bearing) format path; Guests lists guests for the eject dependent-warning. @@ -240,8 +244,10 @@ type Server struct { // caller supplies no tiers it holds exactly one, synthesized from Backups+BackupCadence, which // is the pre-R-82 shape. tiers []BackupTier - logger *slog.Logger - now func() time.Time + // inFlight (R-85) is shared with the restore-test scheduler so the two never run together. + inFlight *backup.InFlight + logger *slog.Logger + now func() time.Time disks DiskOps // slice 8C (optional) diskGate StorageGate // slice 8C (optional) @@ -391,6 +397,7 @@ func NewServer(o Options) (*Server, error) { // (and every existing test) keeps working untouched. Exactly one tier is marked primary, and // the primary is always first, because that is what the untargeted endpoints act on. s.tiers = normalizeBackupTiers(o.BackupTiers, o.Backups, cadence) + s.inFlight = o.InFlight if s.backups == nil && len(s.tiers) > 0 { s.backups = s.tiers[0].Service } @@ -750,6 +757,19 @@ func (s *Server) handleBackup(w http.ResponseWriter, r *http.Request, vmid int) // otherwise collide and hand the second caller the first tier's id. The PRIMARY keeps the // pre-R-82 format byte-for-byte — an old controller stores this string and polls with it — so // only the additive tiers carry the target segment. + // R-85 Scenario F: a backup and a restore-test must never run together — both move multi-GB over + // the same tunnel. Acquired here (still holding jobsMu is fine: TryAcquire never blocks) and + // released when the fire-and-forget goroutine finishes. + release, busy, free := s.inFlight.TryAcquire("backup:" + tier.TargetID) + if !free { + s.jobsMu.Unlock() + s.logger.Info("local-api: backup refused — a heavy operation is already in flight", + "vmid", vmid, "requested_target", tier.TargetID, "busy", busy) + writeStatus(w, http.StatusConflict, false, nil, + "a heavy operation is already in flight ("+busy+") — only one runs at a time on this host") + return + } + jobID := "backup-" + strconv.Itoa(vmid) + "-" + strconv.FormatInt(s.now().UnixNano(), 10) if !tier.Primary && tier.TargetID != "" { jobID = "backup-" + strconv.Itoa(vmid) + "-" + tier.TargetID + "-" + strconv.FormatInt(s.now().UnixNano(), 10) @@ -767,6 +787,7 @@ func (s *Server) handleBackup(w http.ResponseWriter, r *http.Request, vmid int) base = context.Background() } go func() { + defer release() // R-85: free the host-wide gate when this backup finishes, however it ends // Outer bound = the tier's own wait bound + headroom for the pre/post work around WaitTask. // A fixed 2h here would silently cap a 6h offsite tier. outer := tier.WaitTimeout