diff --git a/CHANGELOG.md b/CHANGELOG.md index e8272cb..dbe0f89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,76 @@ +## v0.97.0 — R-82 Slice A: per-target backup tiers (local daily + PBS weekly) (2026-07-26) + +Additive; **MinAgent floor rises** for the multi-tier contract (a controller that wants per-tier +backups needs this agent — but see the compatibility rule: an OLD controller is unaffected). + +Slice A of R-82. `BackupTarget()` returned ONE string and `BackupCadence()` ONE 24h window, so +"local daily **and** PBS weekly" was not expressible at all — which is why the DR promise is +currently unbacked (demo-felhom holds one PBS snapshot from 2026-07-18, demo-hp zero, ever). +Phase-0 gate results: `felhom.eu/documentation/audits/SPIKE-r82-phase0-2026-07-26.md`. + +**This slice ships the mechanism only. No box's behaviour changes until a `backup_targets` entry is +added to its config (Slice D).** An untouched config resolves to exactly one tier and behaves +byte-identically to v0.96.0. + +### The compatibility rule (load-bearing) + +The agent and controller deploy independently, so the untargeted local-API contract is **frozen**: + +- `GET /backup/due` with **no** `?target=` → the PRIMARY tier, same cadence, same response **bytes**. + `BackupDueResponse.Target` is `omitempty` and left EMPTY for untargeted requests, so an old + controller cannot tell this agent from the old one. Pinned by a red-proofed test. +- Same for `POST /backup` and `GET /backup/status`. +- The primary tier's **job-id format is unchanged**; only additive tiers carry a target segment. + +### Added +- **`config.BackupTargetConfig` + `BackupConfig.ExtraTargets`** (`backup_targets`) — each tier + carries its OWN cadence and its OWN retention. Those are semantically different per tier: + `keep-last=3` is three DAYS on a daily tier and three WEEKS on a weekly one, so sharing one knob + guarantees one of them is wrong. +- **`BackupConfig.BackupTiers() ([]BackupTier, []string)`** — resolves the tier list, primary first, + plus warnings the caller MUST log. A tier is **rejected, not defaulted**, when its cadence is + missing: silently defaulting a weekly DR tier to the 24h local default would fill the 37.2 GB + datastore. Empty/duplicate targets are rejected too. `main.go` logs every rejection at **ERROR** — + a silently dropped backup tier is the "applied and empty" fault this task exists to fix. +- **`GET /backup/tiers`** — advertises the tiers, primary first. This is the controller's capability + probe: a **404 means a pre-R-82 agent**, and Slice B falls back to single-tier on it. +- **`localapi.BackupTier` + `normalizeBackupTiers`** — nil tiers synthesize the legacy single tier, + so every existing caller and test hits the pre-R-82 path untouched. A tier with a nil runner is + DROPPED rather than advertised (advertising one would be an applied-and-empty tier). + +### Changed +- **`/backup/due?target=` judges that tier against ITS OWN newest successful backup** + (`latestSuccessfulBackupForTarget`). Without this filter a fresh daily local backup would satisfy + the weekly PBS cadence and the DR tier would never run — today's bug, re-created in code. The + store was already keyed by target, so this is a lookup change, not a data-model change. +- **Backup jobs are keyed by (vmid, target), not vmid.** Single-flight is now PER TIER, which is + what lets the weekly night run both backups inside ONE quiesce window (Slice B). Keying by vmid + alone handed the second caller the first tier's job id — **caught by its own test**, and it would + have made the controller believe a PBS backup ran when only the local one had. +- **Job ids are unique per tier by construction**, not by clock luck (two tiers can start in the + same nanosecond). The primary keeps the old format; additive tiers carry the target segment. +- **One runner per tier** (`main.go`). The runner holds target/mode/notes/retention as immutable + construction state and `localPruneSpec` reads that retention — parameterising a single runner by + target would risk pairing tier A's target with tier B's retention. +- An unknown `?target=` is a **400, never a silent fallback to the primary**. A controller asking + about a tier this agent does not serve must find out, not act on another tier's freshness. + +### NOT changed (deliberate) +- **The local tier.** Same target, same 24h cadence, same keep-last=3 clamp. It is the only honest + tier today and this slice does not touch it. +- **PBS is still never pruned by the per-run `--prune-backups` flag** (`localPruneSpec`). Per-tier + retention is plumbed and a tier's `keep_last` defaults to 0 = never prune, but turning on + automatic pruning of the DR datastore is irreversible and needs an operator ruling — R-82 Phase 0 + already flagged retention as needing one. Recorded, not decided here. +- The fail-safe-toward-due rule on an unparseable timestamp. A spurious backup is cheap; a skipped + one is not. + +### Tests +768 (was 748), +20 across `internal/localapi/backup_tiers_test.go` and +`internal/config/backup_tiers_test.go`. Red-proof #1 (old controller ↔ new agent) observed: +removing the untargeted compat branch makes the untargeted body carry `"target":"local"` and the +test fails on it. Restored. + ## v0.96.0 — R-50 island NIC: provision attaches the guest's island net1 (2026-07-25) Additive; **MinAgent unchanged** (no controller coupling — the controller dials whatever `bootstrap.json` diff --git a/REUSE.md b/REUSE.md index a06e01c..8ab70e4 100644 --- a/REUSE.md +++ b/REUSE.md @@ -144,6 +144,7 @@ | `storage.HostReader` | internal/storage/hostread.go | `*ProcHostReader` | `fakeHostReader` internal/localapi/disks_test.go; internal/storage/role_test.go. v0.87.0: `BlockSlaves(name)` lists `/sys/block//slaves` (root-free) — backs the `SystemDisks` dm/md walk (`physicalDisksOf`/`walkSlaves`, role.go); per-branch conservatism: an unresolvable slave fails the WHOLE walk → all-system fail-safe. NEVER weaken the signature test `TestSystemDisks_WalkTopologies` (root-backing disk always in the system set). | | `localapi.DiskOps` / `StorageGate` / `GuestAttacher` / `GuestLister` | internal/localapi/disks.go | `*storage.SudoHostOps`; `storageGateAdapter` (cmd/felhom-agent/main.go); `*GuestBinder`; `*proxmox.Client` | `fakeDiskOps`/`fakeGate`/`fakeGuestAttacher`/`fakeGuestList` internal/localapi/disks_test.go | | `localapi.GuestAPI` / `BackupService` / `BackupStore` / `TokenAuthority` | internal/localapi/server.go | `*proxmox.Client`, `*backup.BackupRunner`, `*backup.Store`, `*TokenStore` | `fakeGuests`/`fakeBackups`/`fakeStore` internal/localapi/server_test.go | +| `localapi.BackupTier` + `normalizeBackupTiers` / `config.BackupConfig.BackupTiers` | internal/localapi/backup_tiers.go, internal/config/config.go | `normalizeBackupTiers(tiers, legacy, cadence) []BackupTier`; `BackupTiers() ([]BackupTier, []string)` | THE R-82 multi-tier resolution — one runner per tier, primary first | **The untargeted local-API contract is FROZEN**: no `?target=` ⇒ primary tier ⇒ pre-R-82 response BYTES (Target is `omitempty` and stays empty). Never default a missing cadence — reject it and log the warning at ERROR. Never share one retention knob between tiers. Jobs are keyed by (vmid,target). | | `localapi.StaleLockController` | internal/localapi/stalelock.go | `*staleLockController` (Client + Runner + pool) | `fakeStaleLock` (Server-level) stalelock_test.go; `fakeStaleLockAPI` (controller-level, tests the A1 pool intersect) stalelock_pool_test.go | | `localapi.GuestExecutor` | internal/localapi/controllerswap.go | `*GuestBinder` (pct exec) | `fakeGuestExec` internal/localapi/controllerswap_test.go | | `guestnet.Runner` / `guestnet.GuestSource` (R-54, v0.92.0) | internal/guestnet/{probe,watchdog}.go | `*proxmox.ExecRunner`; the POOL-VERIFIED `localapi.StaleLockController.Guests` (ListLXC ∩ felhom pool, audit A1) | `scriptedRunner` + `fakeGuests` internal/guestnet/watchdog_test.go. **Never wire a bare `ListLXC` here** — under a broad token that would run dhclient inside a co-tenant's container. Every assertion is an exec COUNT, and the load-bearing ones are the negatives: a static guest, an unprobeable guest, a boot-race guest and an unproven guest list must record **zero** heal calls | diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index 8a6a08f..3612a66 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -1269,7 +1269,36 @@ func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.St } // v0.48.0: ride the served leaf fp on every host report so the hub can detect a re-key fleet-wide. collector.SetLeafFingerprint(fp) - runner := backup.NewBackupRunner(px, cfg.Backup.BackupTarget(), "", "felhom local-api", cfg.Backup.PruneBackupsSpec(), logger) + // R-82: ONE RUNNER PER TIER. The runner holds its target, mode, notes and retention as + // immutable construction state, and localPruneSpec reads that retention — so parameterising a + // single runner by target would risk a call pairing tier A's target with tier B's retention. + // One runner per tier keeps each tier's policy structurally inseparable from its target. + backupTiers, tierWarnings := cfg.Backup.BackupTiers() + for _, wmsg := range tierWarnings { + // LOUD on purpose: a silently dropped backup tier is an "applied and empty" DR tier, which + // is the exact fault R-82 exists to fix. Never downgrade this to DEBUG. + logger.Error("backup tier REJECTED — this tier will never run", "detail", wmsg) + } + apiTiers := make([]localapi.BackupTier, 0, len(backupTiers)) + var runner *backup.BackupRunner + for _, t := range backupTiers { + prune := "" + if t.KeepLast > 0 { + prune = fmt.Sprintf("keep-last=%d", t.KeepLast) + } + r := backup.NewBackupRunner(px, t.TargetID, "", "felhom local-api", prune, logger) + if t.Primary { + runner = r + } + apiTiers = append(apiTiers, localapi.BackupTier{ + TargetID: t.TargetID, + Cadence: t.Cadence, + Primary: t.Primary, + Service: r, + }) + logger.Info("backup tier armed", "target", t.TargetID, "cadence", t.Cadence.String(), + "keep_last", t.KeepLast, "primary", t.Primary) + } // Guest data-drive passthrough (slice 10 P2): a root-CLI runner for the `pct set` bind + chown // (same fenced ExecRunner the host-storage + provision back-half use). gaMode := proxmox.RunnerMode(cfg.Privileged.Mode) @@ -1283,6 +1312,7 @@ func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.St AgentVersion: version, // v0.82.0: the X-Felhom-Agent-Version capability channel Guests: px, Backups: runner, + BackupTiers: apiTiers, // R-82: primary first; untargeted endpoints act on the primary Store: store, Storage: observer, DriveTargets: driveTargets, // Impl-2a: registry+units drives for the /disks view (union w/ Observe storages) diff --git a/internal/config/backup_tiers_test.go b/internal/config/backup_tiers_test.go new file mode 100644 index 0000000..0876ff8 --- /dev/null +++ b/internal/config/backup_tiers_test.go @@ -0,0 +1,150 @@ +package config + +import ( + "encoding/json" + "strings" + "testing" + "time" +) + +// R-82 Slice A.1 — per-target cadence + retention resolution. +// +// The load-bearing property is ADDITIVITY: every config that exists on a live box today must +// resolve to exactly one tier that behaves as it does now. The second property is that a +// mis-configured tier is REJECTED LOUDLY rather than defaulted — a weekly DR tier silently running +// daily would fill the datastore, and a silently dropped tier is the "applied and empty" fault +// R-82 exists to fix. + +func TestBackupTiers_LegacyConfigIsUnchanged(t *testing.T) { + // Exactly the shape live on demo-felhom today. + var b BackupConfig + raw := `{"local_backup_target":"local","local_backup_retention":3,"backup_cadence_seconds":0}` + if err := json.Unmarshal([]byte(raw), &b); err != nil { + t.Fatal(err) + } + tiers, warnings := b.BackupTiers() + if len(warnings) != 0 { + t.Fatalf("a legacy config must produce NO warnings; got %v", warnings) + } + if len(tiers) != 1 { + t.Fatalf("a config with no backup_targets must resolve to exactly ONE tier; got %+v", tiers) + } + got := tiers[0] + if got.TargetID != "local" || got.Cadence != 24*time.Hour || got.KeepLast != 3 || !got.Primary { + t.Fatalf("legacy tier changed: %+v", got) + } +} + +// An empty BackupConfig still resolves — to the felhom-pbs default target, 24h, keep-last 3. +// (Unchanged pre-R-82 behaviour; pinned so the default target can't drift unnoticed.) +func TestBackupTiers_ZeroConfigKeepsDefaults(t *testing.T) { + tiers, warnings := BackupConfig{}.BackupTiers() + if len(warnings) != 0 || len(tiers) != 1 { + t.Fatalf("zero config: tiers=%+v warnings=%v", tiers, warnings) + } + if tiers[0].TargetID != defaultBackupTarget || tiers[0].Cadence != 24*time.Hour || tiers[0].KeepLast != 3 { + t.Fatalf("zero-config defaults changed: %+v", tiers[0]) + } +} + +// The whole point: local daily + PBS weekly, each with its OWN retention. +func TestBackupTiers_LocalDailyPlusPBSWeekly(t *testing.T) { + var b BackupConfig + raw := `{ + "local_backup_target":"local", + "local_backup_retention":3, + "backup_cadence_seconds":86400, + "backup_targets":[{"target_id":"felhom-pbs","cadence_seconds":604800,"keep_last":2}] + }` + if err := json.Unmarshal([]byte(raw), &b); err != nil { + t.Fatal(err) + } + tiers, warnings := b.BackupTiers() + if len(warnings) != 0 { + t.Fatalf("unexpected warnings: %v", warnings) + } + if len(tiers) != 2 { + t.Fatalf("want 2 tiers, got %+v", tiers) + } + if !tiers[0].Primary || tiers[0].TargetID != "local" || tiers[0].Cadence != 24*time.Hour || tiers[0].KeepLast != 3 { + t.Fatalf("primary tier wrong: %+v", tiers[0]) + } + if tiers[1].Primary || tiers[1].TargetID != "felhom-pbs" || tiers[1].Cadence != 7*24*time.Hour || tiers[1].KeepLast != 2 { + t.Fatalf("PBS tier wrong: %+v", tiers[1]) + } + // THE knob-sharing check: the two retentions are independent values, not one shared number. + if tiers[0].KeepLast == tiers[1].KeepLast { + t.Fatalf("this fixture sets 3 and 2 deliberately — equal values mean the knob is shared: %+v", tiers) + } +} + +// A tier with no cadence is REJECTED, not defaulted. Defaulting would turn a weekly DR tier into a +// daily one and fill the 37.2 GB datastore (R-82 Phase 0, P0.3). +func TestBackupTiers_MissingCadenceIsRejectedLoudly(t *testing.T) { + b := BackupConfig{ + LocalBackupTarget: "local", + ExtraTargets: []BackupTargetConfig{{TargetID: "felhom-pbs", KeepLast: 2}}, + } + tiers, warnings := b.BackupTiers() + if len(tiers) != 1 { + t.Fatalf("a cadence-less tier must NOT be armed; got %+v", tiers) + } + if len(warnings) != 1 || !strings.Contains(warnings[0], "cadence_seconds must be > 0") { + t.Fatalf("rejection must be reported so the caller can log it loudly; got %v", warnings) + } + if !strings.Contains(warnings[0], "felhom-pbs") { + t.Fatalf("the warning must name the tier it dropped; got %q", warnings[0]) + } +} + +func TestBackupTiers_RejectsEmptyAndDuplicateTargets(t *testing.T) { + b := BackupConfig{ + LocalBackupTarget: "local", + ExtraTargets: []BackupTargetConfig{ + {TargetID: "", CadenceSeconds: 3600}, + {TargetID: "local", CadenceSeconds: 3600}, // repeats the primary + {TargetID: "felhom-pbs", CadenceSeconds: 604800}, // good + {TargetID: "felhom-pbs", CadenceSeconds: 99}, // duplicate + }, + } + tiers, warnings := b.BackupTiers() + if len(tiers) != 2 || tiers[1].TargetID != "felhom-pbs" || tiers[1].Cadence != 7*24*time.Hour { + t.Fatalf("want primary + one PBS tier at the FIRST definition; got %+v", tiers) + } + if len(warnings) != 3 { + t.Fatalf("want 3 rejections (empty, duplicate-of-primary, duplicate); got %v", warnings) + } +} + +// keep_last unset means DO NOT PRUNE. That is the fail-safe: a DR tier must never start pruning +// itself because someone forgot a field. +func TestBackupTiers_UnsetKeepLastMeansNoPrune(t *testing.T) { + b := BackupConfig{ + LocalBackupTarget: "local", + ExtraTargets: []BackupTargetConfig{{TargetID: "felhom-pbs", CadenceSeconds: 604800}}, + } + tiers, _ := b.BackupTiers() + if len(tiers) != 2 { + t.Fatalf("got %+v", tiers) + } + if tiers[1].KeepLast != 0 { + t.Fatalf("an unset keep_last must resolve to 0 = never prune; got %d", tiers[1].KeepLast) + } + // And a negative is clamped to the same fail-safe rather than becoming a prune spec. + b.ExtraTargets[0].KeepLast = -5 + tiers, _ = b.BackupTiers() + if tiers[1].KeepLast != 0 { + t.Fatalf("a negative keep_last must clamp to 0 (never prune); got %d", tiers[1].KeepLast) + } +} + +// The primary's retention still comes from the legacy knob with its legacy clamp — untouched. +func TestBackupTiers_PrimaryRetentionClampUnchanged(t *testing.T) { + for _, tc := range []struct{ in, want int }{{0, 3}, {-1, 3}, {1, 1}, {7, 7}} { + b := BackupConfig{LocalBackupTarget: "local", LocalBackupRetention: tc.in} + tiers, _ := b.BackupTiers() + if tiers[0].KeepLast != tc.want { + t.Fatalf("LocalBackupRetention=%d → KeepLast=%d, want %d", tc.in, tiers[0].KeepLast, tc.want) + } + } +} diff --git a/internal/config/config.go b/internal/config/config.go index d08e79a..34fd9fc 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -366,6 +366,95 @@ type BackupConfig struct { // default 3; ALWAYS clamped to ≥1 by KeepLast() so a mis-config can never prune the fresh backup. // NEVER applied to a PBS target (offsite retention is a separate lifecycle). LocalBackupRetention int `json:"local_backup_retention"` + + // ExtraTargets (R-82) are ADDITIONAL backup tiers beyond the primary one above — the shape that + // makes "local daily + PBS weekly" expressible at all. Each carries its OWN cadence and its OWN + // retention, because those are semantically different per tier: keep-last=3 on a daily tier is + // three DAYS of restore points; on a weekly tier it is three WEEKS. Sharing one knob between + // tiers silently means one of them is wrong. + // + // ADDITIVE BY CONSTRUCTION: an existing config with no `backup_targets` key resolves to exactly + // one tier — the primary — and behaves byte-identically to pre-R-82. Nothing here changes the + // local tier. + ExtraTargets []BackupTargetConfig `json:"backup_targets"` +} + +// BackupTargetConfig is ONE additional backup tier: a vzdump storage plus its own cadence and +// retention. A tier with no cadence is not a tier — see BackupTiers for why that is rejected loudly +// rather than defaulted. +type BackupTargetConfig struct { + // TargetID is the Proxmox storage id (content=backup), e.g. "felhom-pbs". + TargetID string `json:"target_id"` + // CadenceSeconds is THIS tier's /backup/due window. REQUIRED (>0) — see BackupTiers. + CadenceSeconds int `json:"cadence_seconds"` + // KeepLast is THIS tier's per-run `--prune-backups` keep-last. 0/unset → NEVER prune this tier + // (the fail-safe default, and the current behaviour for every PBS target). A PBS tier is never + // pruned by the per-run flag regardless — see BackupRunner.localPruneSpec. + KeepLast int `json:"keep_last"` +} + +// BackupTier is a RESOLVED backup tier: one target, its own cadence, its own retention. The agent +// builds one runner per tier from these. +type BackupTier struct { + TargetID string + Cadence time.Duration + // KeepLast is the per-run prune keep-last; 0 means DO NOT PRUNE this tier. + KeepLast int + // Primary marks the tier that the UNTARGETED local-API endpoints act on — the pre-R-82 tier. + // Exactly one tier is primary, and it is always first. + Primary bool +} + +// BackupTiers resolves the effective tier list, primary first, plus any warnings the caller MUST +// log (they describe tiers that were REJECTED, and a silently-dropped backup tier is precisely the +// "applied and empty" fault R-82 exists to fix). +// +// Rules: +// - Tier 0 is always the primary, built from BackupTarget()/BackupCadence()/KeepLast() — so a +// config with no `backup_targets` is byte-identical to pre-R-82. +// - An extra with an empty target_id is rejected. +// - An extra with cadence_seconds <= 0 is REJECTED, not defaulted. Defaulting a PBS tier to the +// 24h local default would quietly turn a weekly tier into a daily one and fill the DR datastore; +// a tier whose cadence you did not state is not a tier. +// - An extra repeating the primary's target is rejected (one policy per target, or the two +// cadences race and neither is the truth). +// - Duplicate extras are rejected after the first. +func (b BackupConfig) BackupTiers() ([]BackupTier, []string) { + primary := BackupTier{ + TargetID: b.BackupTarget(), + Cadence: b.BackupCadence(), + KeepLast: b.KeepLast(), + Primary: true, + } + tiers := []BackupTier{primary} + var warnings []string + + seen := map[string]bool{primary.TargetID: true} + for i, t := range b.ExtraTargets { + id := strings.TrimSpace(t.TargetID) + switch { + case id == "": + warnings = append(warnings, fmt.Sprintf("backup_targets[%d]: empty target_id — tier ignored", i)) + continue + case seen[id]: + warnings = append(warnings, fmt.Sprintf("backup_targets[%d]: target %q already configured — duplicate tier ignored", i, id)) + continue + case t.CadenceSeconds <= 0: + warnings = append(warnings, fmt.Sprintf("backup_targets[%d] (%s): cadence_seconds must be > 0 — tier ignored (a cadence is NOT defaulted: a weekly tier silently running daily would fill the DR datastore)", i, id)) + continue + } + seen[id] = true + keep := t.KeepLast + if keep < 0 { + keep = 0 + } + tiers = append(tiers, BackupTier{ + TargetID: id, + Cadence: time.Duration(t.CadenceSeconds) * time.Second, + KeepLast: keep, + }) + } + return tiers, warnings } // defaultLocalBackupKeepLast is the local vzdump retention default (newest N restore points kept). diff --git a/internal/localapi/backup_tiers.go b/internal/localapi/backup_tiers.go new file mode 100644 index 0000000..69e0c5e --- /dev/null +++ b/internal/localapi/backup_tiers.go @@ -0,0 +1,56 @@ +package localapi + +import "time" + +// normalizeBackupTiers resolves the tier list the Server serves. +// +// Contract (R-82), and the reason this is a named function rather than inline setup: the UNTARGETED +// local-API endpoints must keep behaving exactly as they did before multi-tier existed, forever. +// That property lives here. +// +// - tiers == nil → synthesize ONE tier from the legacy (Backups, BackupCadence) pair and mark it +// primary. This is the pre-R-82 shape; every existing caller and test hits this path. +// - tiers supplied → keep order but hoist the primary to the front; if none is marked primary, +// the FIRST becomes primary (a tier list with no primary would leave untargeted requests with +// nothing to act on, which would silently stop backups). +// - tiers with a nil Service are dropped: a tier with no runner cannot back anything up, and +// advertising it would be an "applied and empty" tier — the exact fault R-82 exists to fix. +func normalizeBackupTiers(tiers []BackupTier, legacy BackupService, cadence time.Duration) []BackupTier { + usable := make([]BackupTier, 0, len(tiers)) + for _, t := range tiers { + if t.Service == nil || t.TargetID == "" { + continue + } + if t.Cadence <= 0 { + t.Cadence = cadence + } + usable = append(usable, t) + } + if len(usable) == 0 { + if legacy == nil { + return nil + } + return []BackupTier{{TargetID: "", Cadence: cadence, Primary: true, Service: legacy}} + } + primary := -1 + for i, t := range usable { + if t.Primary { + primary = i + break + } + } + if primary < 0 { + primary = 0 + } + out := make([]BackupTier, 0, len(usable)) + usable[primary].Primary = true + out = append(out, usable[primary]) + for i, t := range usable { + if i == primary { + continue + } + t.Primary = false + out = append(out, t) + } + return out +} diff --git a/internal/localapi/backup_tiers_test.go b/internal/localapi/backup_tiers_test.go new file mode 100644 index 0000000..bdecc5f --- /dev/null +++ b/internal/localapi/backup_tiers_test.go @@ -0,0 +1,390 @@ +package localapi + +import ( + "context" + "encoding/json" + "io" + "log/slog" + "net/http" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-agent/internal/hub" +) + +// R-82 Slice A — per-target cadence, due and runner. +// +// The agent and the controller deploy INDEPENDENTLY. A new agent will serve old controllers for as +// long as it takes the fleet to catch up, so the untargeted contract is frozen, not merely +// "probably fine". These tests pin that freeze; the multi-tier behaviour is additive on top. + +// tieredServer builds a two-tier server: primary "local" (24h) + "felhom-pbs" (7d), each with its +// own runner, exactly as main.go wires it. +func tieredServer(t *testing.T, st *fakeStore, localSvc, pbsSvc *fakeBackups) *Server { + t.Helper() + srv, err := NewServer(Options{ + ListenAddr: "127.0.0.1:0", + Guests: &fakeGuests{}, + Backups: localSvc, + Store: st, + Storage: fakeStorage{}, + Tokens: staticTokens{"A": 8200, "B": 9300}, + BackupTiers: []BackupTier{ + {TargetID: "local", Cadence: 24 * time.Hour, Primary: true, Service: localSvc}, + {TargetID: "felhom-pbs", Cadence: 7 * 24 * time.Hour, Service: pbsSvc}, + }, + Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + }) + if err != nil { + t.Fatalf("new server: %v", err) + } + srv.baseCtx = context.Background() + srv.now = func() time.Time { return testNow } + return srv +} + +// seen returns the vmids this fake runner was invoked for (mutex-guarded — the backup runs on a +// goroutine). +func (f *fakeBackups) seen() []int { + f.mu.Lock() + defer f.mu.Unlock() + return append([]int(nil), f.vmids...) +} + +// waitFor polls cond for up to 2s. POST /backup is fire-and-forget, so the assertion has to wait +// for the goroutine rather than assume it has run. +func waitFor(t *testing.T, cond func() bool) { + t.Helper() + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + if cond() { + return + } + time.Sleep(5 * time.Millisecond) + } + t.Fatalf("condition not met within 2s") +} + +func backupAt(target string, vmid int, ago time.Duration, ok bool) hub.Backup { + return hub.Backup{ + TargetID: target, + VMID: vmid, + Success: ok, + StartedAt: testNow.Add(-ago).Format(time.RFC3339), + } +} + +// ── RED-PROOF 1 — old controller ↔ new agent ──────────────────────────────────────────────── +// +// An old controller sends `GET /backup/due` with no query string and parses the pre-R-82 response. +// The response must be BYTE-IDENTICAL — not merely semantically similar. A stray `"target":"local"` +// key is harmless to a tolerant JSON decoder and fatal to a strict one, and we do not get to choose +// which the deployed fleet has. +// +// COMPANION RED-PROOF (observed): drop the `omitempty` from BackupDueResponse.Target and have +// tierFromRequest echo the primary's id for an untargeted request → this test fails with the +// observed body carrying `"target":"local"`. Restored. +func TestBackupDue_Untargeted_ResponseBytesUnchanged(t *testing.T) { + st := &fakeStore{} + st.RecordBackup(backupAt("local", 8200, 2*time.Hour, true)) + h := tieredServer(t, st, &fakeBackups{}, &fakeBackups{}).Handler() + + rr := do(t, h, "GET", "/backup/due", "A", "") + if rr.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", rr.Code) + } + var got map[string]any + if err := json.Unmarshal(rr.Body.Bytes(), &got); err != nil { + t.Fatalf("unmarshal: %v (body %s)", err, rr.Body.String()) + } + data, _ := got["data"].(map[string]any) + if data == nil { + t.Fatalf("no data object in %s", rr.Body.String()) + } + if _, present := data["target"]; present { + t.Fatalf("UNTARGETED response MUST NOT carry a target key — an old controller sees a changed contract; body: %s", rr.Body.String()) + } + if data["due"] != false { + t.Fatalf("2h-old local backup under a 24h cadence must not be due; body: %s", rr.Body.String()) + } + if data["reason"] != "within cadence window" { + t.Fatalf("reason string changed: %v", data["reason"]) + } +} + +// The untargeted verdict must come from the PRIMARY tier's cadence, not from whichever tier +// happens to be freshest. With a stale local and a fresh PBS backup, untargeted must say DUE. +func TestBackupDue_Untargeted_UsesPrimaryTierOnly(t *testing.T) { + st := &fakeStore{} + st.RecordBackup(backupAt("local", 8200, 30*time.Hour, true)) // stale for 24h cadence + st.RecordBackup(backupAt("felhom-pbs", 8200, 1*time.Hour, true)) // fresh, different tier + h := tieredServer(t, st, &fakeBackups{}, &fakeBackups{}).Handler() + + var resp struct { + Data BackupDueResponse `json:"data"` + } + rr := do(t, h, "GET", "/backup/due", "A", "") + if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil { + t.Fatal(err) + } + if !resp.Data.Due { + t.Fatalf("a fresh backup on ANOTHER tier must not satisfy the primary's cadence; got %+v", resp.Data) + } +} + +// ── Per-tier due-ness ──────────────────────────────────────────────────────────────────────── + +// THE POINT OF THE WHOLE SLICE: a fresh daily local backup must NOT satisfy the weekly PBS tier. +// Without the per-target filter in latestSuccessfulBackupForTarget the DR tier would never run — +// which is exactly today's "applied and empty" state, re-created in code. +func TestBackupDue_PerTier_LocalFreshDoesNotSatisfyPBS(t *testing.T) { + st := &fakeStore{} + st.RecordBackup(backupAt("local", 8200, 2*time.Hour, true)) // fresh daily + st.RecordBackup(backupAt("felhom-pbs", 8200, 8*24*time.Hour, true)) // 8d — past the 7d weekly + h := tieredServer(t, st, &fakeBackups{}, &fakeBackups{}).Handler() + + var local, pbs struct { + Data BackupDueResponse `json:"data"` + } + if err := json.Unmarshal(do(t, h, "GET", "/backup/due?target=local", "A", "").Body.Bytes(), &local); err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(do(t, h, "GET", "/backup/due?target=felhom-pbs", "A", "").Body.Bytes(), &pbs); err != nil { + t.Fatal(err) + } + if local.Data.Due { + t.Fatalf("local tier: 2h old under 24h cadence must NOT be due; got %+v", local.Data) + } + if !pbs.Data.Due { + t.Fatalf("PBS tier: 8d old under a 7d cadence MUST be due; got %+v", pbs.Data) + } + if pbs.Data.Target != "felhom-pbs" || local.Data.Target != "local" { + t.Fatalf("a targeted response must echo its tier; got local=%q pbs=%q", local.Data.Target, pbs.Data.Target) + } +} + +// A 6-day-old PBS snapshot is INSIDE the weekly window — it must not be due. The mirror of the +// hub-side threshold test in Slice C, asserted here at the source of truth. +func TestBackupDue_PerTier_PBSWithinWeeklyWindow(t *testing.T) { + st := &fakeStore{} + st.RecordBackup(backupAt("felhom-pbs", 8200, 6*24*time.Hour, true)) + h := tieredServer(t, st, &fakeBackups{}, &fakeBackups{}).Handler() + + var pbs struct { + Data BackupDueResponse `json:"data"` + } + if err := json.Unmarshal(do(t, h, "GET", "/backup/due?target=felhom-pbs", "A", "").Body.Bytes(), &pbs); err != nil { + t.Fatal(err) + } + if pbs.Data.Due { + t.Fatalf("6d old under a 7d cadence must NOT be due; got %+v", pbs.Data) + } +} + +// A failed backup must not satisfy any tier's cadence (pre-existing rule, re-asserted per-tier). +func TestBackupDue_PerTier_FailedBackupDoesNotCount(t *testing.T) { + st := &fakeStore{} + st.RecordBackup(backupAt("felhom-pbs", 8200, time.Hour, false)) + h := tieredServer(t, st, &fakeBackups{}, &fakeBackups{}).Handler() + + var pbs struct { + Data BackupDueResponse `json:"data"` + } + if err := json.Unmarshal(do(t, h, "GET", "/backup/due?target=felhom-pbs", "A", "").Body.Bytes(), &pbs); err != nil { + t.Fatal(err) + } + if !pbs.Data.Due { + t.Fatalf("a FAILED backup must not satisfy the cadence; got %+v", pbs.Data) + } +} + +// The fail-safe-toward-due rule survives per-tier: an unparseable timestamp yields DUE. +// A spurious backup is cheap; a skipped one is not. +func TestBackupDue_PerTier_UnparseableTimestampIsDue(t *testing.T) { + st := &fakeStore{} + st.RecordBackup(hub.Backup{TargetID: "felhom-pbs", VMID: 8200, Success: true, StartedAt: "not-a-time"}) + h := tieredServer(t, st, &fakeBackups{}, &fakeBackups{}).Handler() + + var pbs struct { + Data BackupDueResponse `json:"data"` + } + if err := json.Unmarshal(do(t, h, "GET", "/backup/due?target=felhom-pbs", "A", "").Body.Bytes(), &pbs); err != nil { + t.Fatal(err) + } + if !pbs.Data.Due { + t.Fatalf("unparseable timestamp must fail SAFE toward due; got %+v", pbs.Data) + } +} + +// An unknown target is a 400 — never a silent fallback to the primary. A controller asking about a +// tier this agent does not serve must find out, not be handed a different tier's freshness and act +// on it. +func TestBackupDue_UnknownTarget_IsAnErrorNotAFallback(t *testing.T) { + st := &fakeStore{} + st.RecordBackup(backupAt("local", 8200, 2*time.Hour, true)) + h := tieredServer(t, st, &fakeBackups{}, &fakeBackups{}).Handler() + + rr := do(t, h, "GET", "/backup/due?target=does-not-exist", "A", "") + if rr.Code != http.StatusBadRequest { + t.Fatalf("unknown target must be 400 (got %d, body %s)", rr.Code, rr.Body.String()) + } +} + +// ── Tier advertisement ─────────────────────────────────────────────────────────────────────── + +// GET /backup/tiers is the controller's capability probe. Primary must be first and flagged, so a +// controller can tell which tier the untargeted endpoints act on. +func TestBackupTiers_AdvertisesPrimaryFirst(t *testing.T) { + h := tieredServer(t, &fakeStore{}, &fakeBackups{}, &fakeBackups{}).Handler() + var resp struct { + Data BackupTiersResponse `json:"data"` + } + rr := do(t, h, "GET", "/backup/tiers", "A", "") + if rr.Code != http.StatusOK { + t.Fatalf("status = %d", rr.Code) + } + if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil { + t.Fatal(err) + } + if len(resp.Data.Tiers) != 2 { + t.Fatalf("want 2 tiers, got %+v", resp.Data.Tiers) + } + if !resp.Data.Tiers[0].Primary || resp.Data.Tiers[0].Target != "local" { + t.Fatalf("primary must be first and flagged; got %+v", resp.Data.Tiers) + } + if resp.Data.Tiers[1].Target != "felhom-pbs" || resp.Data.Tiers[1].CadenceSeconds != int64((7 * 24 * time.Hour).Seconds()) { + t.Fatalf("PBS tier mis-advertised: %+v", resp.Data.Tiers[1]) + } +} + +// ── POST /backup routing + per-tier single-flight ──────────────────────────────────────────── + +// A targeted POST must run THAT tier's runner. Routing both tiers to one runner would silently +// write every "PBS" backup to local — a DR tier that reports success and stores nothing. +func TestBackupPost_RoutesToTheTargetsOwnRunner(t *testing.T) { + local, pbs := &fakeBackups{}, &fakeBackups{} + srv := tieredServer(t, &fakeStore{}, local, pbs) + h := srv.Handler() + + if rr := do(t, h, "POST", "/backup?target=felhom-pbs", "A", ""); rr.Code != http.StatusAccepted { + t.Fatalf("status = %d, body %s", rr.Code, rr.Body.String()) + } + waitFor(t, func() bool { return len(pbs.seen()) == 1 }) + + if got := len(local.seen()); got != 0 { + t.Fatalf("the LOCAL runner must not have run for a PBS-targeted request (ran %d times)", got) + } +} + +// Untargeted POST routes to the primary — the old controller's path. +func TestBackupPost_UntargetedRoutesToPrimary(t *testing.T) { + local, pbs := &fakeBackups{}, &fakeBackups{} + h := tieredServer(t, &fakeStore{}, local, pbs).Handler() + + if rr := do(t, h, "POST", "/backup", "A", ""); rr.Code != http.StatusAccepted { + t.Fatalf("status = %d", rr.Code) + } + waitFor(t, func() bool { return len(local.seen()) == 1 }) + if got := len(pbs.seen()); got != 0 { + t.Fatalf("untargeted POST must not touch a non-primary tier (ran %d times)", got) + } +} + +// Single-flight is PER TIER. A PBS backup starting while the local one is still running must get +// its OWN job id — this is what lets Slice B run both inside one quiesce window. Keying jobs by +// vmid alone would hand the second call the first job's id and the controller would believe the +// PBS backup finished when only the local one had. +func TestBackupPost_SingleFlightIsPerTierNotPerGuest(t *testing.T) { + localGate := make(chan struct{}) + local := &fakeBackups{gate: localGate} + pbs := &fakeBackups{} + h := tieredServer(t, &fakeStore{}, local, pbs).Handler() + + var first, second BackupResponse + rr1 := do(t, h, "POST", "/backup", "A", "") // local; blocks on the gate + if err := json.Unmarshal(rr1.Body.Bytes(), &struct { + Data *BackupResponse `json:"data"` + }{Data: &first}); err != nil { + t.Fatal(err) + } + rr2 := do(t, h, "POST", "/backup?target=felhom-pbs", "A", "") // different tier → new job + if err := json.Unmarshal(rr2.Body.Bytes(), &struct { + Data *BackupResponse `json:"data"` + }{Data: &second}); err != nil { + t.Fatal(err) + } + if first.JobID == "" || second.JobID == "" { + t.Fatalf("both tiers must get a job id; got %q / %q", first.JobID, second.JobID) + } + if first.JobID == second.JobID { + t.Fatalf("PER-TIER single-flight violated: the PBS request was handed the LOCAL job id %q — the controller would believe the PBS backup ran", first.JobID) + } + waitFor(t, func() bool { return len(pbs.seen()) == 1 }) + close(localGate) +} + +// Same tier, still single-flight: a second POST to a running tier returns the SAME job. +func TestBackupPost_SameTierStillSingleFlight(t *testing.T) { + gate := make(chan struct{}) + local := &fakeBackups{gate: gate} + h := tieredServer(t, &fakeStore{}, local, &fakeBackups{}).Handler() + + var a, b BackupResponse + if err := json.Unmarshal(do(t, h, "POST", "/backup", "A", "").Body.Bytes(), &struct { + Data *BackupResponse `json:"data"` + }{Data: &a}); err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(do(t, h, "POST", "/backup", "A", "").Body.Bytes(), &struct { + Data *BackupResponse `json:"data"` + }{Data: &b}); err != nil { + t.Fatal(err) + } + if a.JobID != b.JobID { + t.Fatalf("same tier must single-flight: %q vs %q", a.JobID, b.JobID) + } + close(gate) +} + +// ── normalizeBackupTiers — the compatibility core ──────────────────────────────────────────── + +func TestNormalizeBackupTiers(t *testing.T) { + svc := &fakeBackups{} + + t.Run("nil tiers synthesize the legacy single tier", func(t *testing.T) { + got := normalizeBackupTiers(nil, svc, 24*time.Hour) + if len(got) != 1 || !got[0].Primary || got[0].Cadence != 24*time.Hour { + t.Fatalf("legacy synthesis broken: %+v", got) + } + }) + + t.Run("primary is hoisted to the front", func(t *testing.T) { + got := normalizeBackupTiers([]BackupTier{ + {TargetID: "felhom-pbs", Cadence: time.Hour, Service: svc}, + {TargetID: "local", Cadence: time.Hour, Primary: true, Service: svc}, + }, svc, 24*time.Hour) + if len(got) != 2 || got[0].TargetID != "local" || !got[0].Primary || got[1].Primary { + t.Fatalf("primary not hoisted / uniqueness broken: %+v", got) + } + }) + + t.Run("no primary marked → first becomes primary", func(t *testing.T) { + got := normalizeBackupTiers([]BackupTier{ + {TargetID: "a", Cadence: time.Hour, Service: svc}, + {TargetID: "b", Cadence: time.Hour, Service: svc}, + }, svc, 24*time.Hour) + if len(got) != 2 || got[0].TargetID != "a" || !got[0].Primary { + t.Fatalf("want first-as-primary, got %+v", got) + } + }) + + t.Run("a tier with no runner is DROPPED, not advertised", func(t *testing.T) { + got := normalizeBackupTiers([]BackupTier{ + {TargetID: "local", Cadence: time.Hour, Primary: true, Service: svc}, + {TargetID: "felhom-pbs", Cadence: time.Hour, Service: nil}, + }, svc, 24*time.Hour) + if len(got) != 1 || got[0].TargetID != "local" { + t.Fatalf("a serviceless tier must not be advertised (it could never run): %+v", got) + } + }) +} diff --git a/internal/localapi/server.go b/internal/localapi/server.go index 101252a..6e03bf8 100644 --- a/internal/localapi/server.go +++ b/internal/localapi/server.go @@ -41,6 +41,20 @@ type BackupService interface { BackupWithSnapshotHook(ctx context.Context, vmid int, onSnapshot func()) (hub.Backup, error) } +// BackupTier (R-82) binds ONE backup tier's runner to its own policy. The agent builds one per +// resolved config tier; the local API serves each independently so "local daily + PBS weekly" is +// expressible over the wire, not just in config. +// +// COMPATIBILITY CONTRACT (load-bearing — the agent and controller deploy independently): +// the tier whose Primary is true is what EVERY untargeted endpoint acts on. An old controller +// never sends `?target=`, so it sees exactly the pre-R-82 behaviour and response bytes. +type BackupTier struct { + TargetID string + Cadence time.Duration + Primary bool + Service BackupService +} + // BackupStore records + reads the latest backup/restore-test state. Satisfied by *backup.Store. type BackupStore interface { RecordBackup(hub.Backup) @@ -92,7 +106,11 @@ type Options struct { // BackupCadence is the per-guest backup interval driving GET /backup/due (slice 8B). A guest // is "due" when no successful backup is recorded OR the newest one is older than this. 0 → a // safe default (24h). The hub-served policy is slice 10; this is the agent-local cadence. + // When BackupTiers is supplied this is IGNORED (the primary tier carries its own cadence). BackupCadence time.Duration + // 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 // 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. @@ -183,6 +201,12 @@ type backupJob struct { Error string } +// backupJobKey identifies one guest's job on ONE tier (R-82). +type backupJobKey struct { + vmid int + target string +} + // Server is the per-guest local API (doc 03 §6). It serves the agent's pinned self-signed leaf // and authorizes every request against the token's guest only. type Server struct { @@ -196,6 +220,10 @@ type Server struct { smart SmartReader // v0.95.0 Fix B: SMART for the /disks union path (optional) tokens TokenAuthority cadence time.Duration + // tiers (R-82) is the resolved backup-tier list, PRIMARY FIRST. Always non-empty: when the + // 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 @@ -247,7 +275,10 @@ type Server struct { boundCheck func(string) bool jobsMu sync.Mutex - jobs map[int]*backupJob // per-guest backup job state (slice 8B) + // jobs is per-guest-PER-TARGET backup job state (slice 8B; keyed by target too since R-82). + // Keying by vmid alone would let a PBS backup started inside the same quiesce window collide + // with the local one's single-flight and hand the caller the WRONG job id. + jobs map[backupJobKey]*backupJob // agentic controller update (Phase 1): the swapper + per-guest single-flight gate. swap *ControllerSwapper @@ -336,9 +367,17 @@ func NewServer(o Options) (*Server, error) { hostID: o.HostID, agentVersion: o.AgentVersion, logRing: o.LogRing, - jobs: map[int]*backupJob{}, + jobs: map[backupJobKey]*backupJob{}, swapInFlight: map[int]bool{}, } + // R-82 tier resolution. Options.BackupTiers is authoritative when supplied; otherwise ONE tier + // is synthesized from Backups + BackupCadence — the pre-R-82 shape, so every existing caller + // (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) + if s.backups == nil && len(s.tiers) > 0 { + s.backups = s.tiers[0].Service + } if s.escrowStagePath == "" { s.escrowStagePath = escrow.StagedResticPasswordPath() } @@ -372,6 +411,7 @@ func (s *Server) Handler() http.Handler { mux.HandleFunc("POST /rollback", s.withGuest(s.handleRollback)) mux.HandleFunc("POST /backup", s.withGuest(s.handleBackup)) mux.HandleFunc("GET /backup/due", s.withGuest(s.handleBackupDue)) + mux.HandleFunc("GET /backup/tiers", s.withGuest(s.handleBackupTiers)) mux.HandleFunc("GET /backup/status", s.withGuest(s.handleBackupStatus)) mux.HandleFunc("GET /restore-test/status", s.withGuest(s.handleRestoreTestStatus)) // Host metrics (slice 9): host-wide health + per-storage capacity for the customer's monitoring @@ -640,6 +680,8 @@ type BackupResponse struct { VMID int `json:"vmid"` JobID string `json:"job_id"` Phase string `json:"phase"` + // Target (R-82) echoes the tier; empty + omitted for an untargeted request (pre-R-82 bytes). + Target string `json:"target,omitempty"` } func (s *Server) handleBackup(w http.ResponseWriter, r *http.Request, vmid int) { @@ -653,17 +695,33 @@ func (s *Server) handleBackup(w http.ResponseWriter, r *http.Request, vmid int) return } } - // Single-flight per guest: if a backup is already running for this guest, return that job - // (don't start a second concurrent vzdump). The controller polls /backup/status on it. - s.jobsMu.Lock() - if cur := s.jobs[vmid]; cur != nil && cur.Phase == PhaseRunning { - job := *cur - s.jobsMu.Unlock() - writeStatus(w, http.StatusAccepted, true, BackupResponse{VMID: vmid, JobID: job.JobID, Phase: job.Phase}, "") + tier, echo, ok := s.tierFromRequest(w, r) + if !ok { return } + key := backupJobKey{vmid: vmid, target: tier.TargetID} + + // Single-flight per guest PER TIER: if a backup is already running for this guest ON THIS + // TIER, return that job (don't start a second concurrent vzdump to the same target). A + // DIFFERENT tier is a different job — that is what lets the weekly night run both backups + // inside one quiesce window without the second call being handed the first one's id. + s.jobsMu.Lock() + if cur := s.jobs[key]; cur != nil && cur.Phase == PhaseRunning { + job := *cur + s.jobsMu.Unlock() + writeStatus(w, http.StatusAccepted, true, BackupResponse{VMID: vmid, JobID: job.JobID, Phase: job.Phase, Target: echo}, "") + return + } + // Job ids must be unique PER TIER, and by construction rather than by clock luck: two tiers + // started inside the same nanosecond (the weekly both-due night, or any injected clock) would + // 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. jobID := "backup-" + strconv.Itoa(vmid) + "-" + strconv.FormatInt(s.now().UnixNano(), 10) - s.jobs[vmid] = &backupJob{JobID: jobID, Phase: PhaseRunning, StartedAt: s.now()} + if !tier.Primary && tier.TargetID != "" { + jobID = "backup-" + strconv.Itoa(vmid) + "-" + tier.TargetID + "-" + strconv.FormatInt(s.now().UnixNano(), 10) + } + s.jobs[key] = &backupJob{JobID: jobID, Phase: PhaseRunning, StartedAt: s.now()} s.jobsMu.Unlock() // Enqueue: a vzdump runs for minutes, so we do not block the request. The backup runs on the @@ -680,42 +738,47 @@ func (s *Server) handleBackup(w http.ResponseWriter, r *http.Request, vmid int) defer cancel() // 8B.2: flip the job to `snapshotted` when the storage snapshot is taken, so the // controller resumes its app early (snapshot mode only; in stop mode this never fires). - b, err := s.backups.BackupWithSnapshotHook(bctx, vmid, func() { s.markSnapshotted(vmid, jobID) }) + b, err := tier.Service.BackupWithSnapshotHook(bctx, vmid, func() { s.markSnapshotted(key, jobID) }) if err != nil { b.VMID = vmid b.Success = false if b.Error == "" { b.Error = err.Error() } - s.logger.Error("local-api: backup job failed", "vmid", vmid, "job", jobID, "err", err) + // TargetID is what the hub attributes the record to; a failed run must still say which + // tier failed, and the runner may not have set it on the error path. + if b.TargetID == "" { + b.TargetID = tier.TargetID + } + s.logger.Error("local-api: backup job failed", "vmid", vmid, "target", tier.TargetID, "job", jobID, "err", err) } else { - s.logger.Info("local-api: backup job complete", "vmid", vmid, "job", jobID, "archive", b.Archive) + s.logger.Info("local-api: backup job complete", "vmid", vmid, "target", tier.TargetID, "job", jobID, "archive", b.Archive) } s.store.RecordBackup(b) - s.finishJob(vmid, jobID, b) + s.finishJob(key, jobID, b) }() writeStatus(w, http.StatusAccepted, true, BackupResponse{VMID: vmid, JobID: jobID, Phase: PhaseRunning}, "") } // markSnapshotted flips the guest's running job to the `snapshotted` phase (8B.2) — only if it is // still the current job and still running (don't regress done/failed, and don't touch a newer job). -func (s *Server) markSnapshotted(vmid int, jobID string) { +func (s *Server) markSnapshotted(key backupJobKey, jobID string) { s.jobsMu.Lock() defer s.jobsMu.Unlock() - cur := s.jobs[vmid] + cur := s.jobs[key] if cur == nil || cur.JobID != jobID || cur.Phase != PhaseRunning { return } cur.Phase = PhaseSnapshotted - s.logger.Info("local-api: backup reached snapshotted (app may resume)", "vmid", vmid, "job", jobID) + s.logger.Info("local-api: backup reached snapshotted (app may resume)", "vmid", key.vmid, "target", key.target, "job", jobID) } // finishJob transitions the guest's job to done/failed (only if it is still the current job — a // later job started after a single-flight gap must not be overwritten by an older one's result). -func (s *Server) finishJob(vmid int, jobID string, b hub.Backup) { +func (s *Server) finishJob(key backupJobKey, jobID string, b hub.Backup) { s.jobsMu.Lock() defer s.jobsMu.Unlock() - cur := s.jobs[vmid] + cur := s.jobs[key] if cur == nil || cur.JobID != jobID { return } @@ -730,10 +793,10 @@ func (s *Server) finishJob(vmid int, jobID string, b hub.Backup) { } // jobSnapshot returns a copy of the guest's current job (ok=false if none). -func (s *Server) jobSnapshot(vmid int) (backupJob, bool) { +func (s *Server) jobSnapshot(key backupJobKey) (backupJob, bool) { s.jobsMu.Lock() defer s.jobsMu.Unlock() - if j := s.jobs[vmid]; j != nil { + if j := s.jobs[key]; j != nil { return *j, true } return backupJob{}, false @@ -748,26 +811,96 @@ type BackupDueResponse struct { Due bool `json:"due"` Reason string `json:"reason"` AgeSecs *int64 `json:"age_seconds,omitempty"` // age of the newest successful backup; null if none + // Target (R-82) echoes the tier this verdict is about. EMPTY (and omitted) for an untargeted + // request, which is what keeps the pre-R-82 response bytes identical for old controllers. + Target string `json:"target,omitempty"` } func (s *Server) handleBackupDue(w http.ResponseWriter, r *http.Request, vmid int) { - latest := s.latestSuccessfulBackupFor(r.Context(), vmid) - if latest == nil { - writeOK(w, BackupDueResponse{VMID: vmid, Due: true, Reason: "no successful backup recorded yet"}) + tier, echo, ok := s.tierFromRequest(w, r) + if !ok { return } - age, ok := backupAge(latest.StartedAt, s.now()) - if !ok { + latest := s.latestSuccessfulBackupForTarget(r.Context(), vmid, tier.TargetID) + if latest == nil { + writeOK(w, BackupDueResponse{VMID: vmid, Due: true, Reason: "no successful backup recorded yet", Target: echo}) + return + } + age, ok2 := backupAge(latest.StartedAt, s.now()) + if !ok2 { // Unparseable timestamp: fail safe toward "due" so a backup still happens. - writeOK(w, BackupDueResponse{VMID: vmid, Due: true, Reason: "last backup time unparseable — treating as due"}) + writeOK(w, BackupDueResponse{VMID: vmid, Due: true, Reason: "last backup time unparseable — treating as due", Target: echo}) return } ageSecs := int64(age.Seconds()) - if age >= s.cadence { - writeOK(w, BackupDueResponse{VMID: vmid, Due: true, Reason: "older than cadence", AgeSecs: &ageSecs}) + if age >= tier.Cadence { + writeOK(w, BackupDueResponse{VMID: vmid, Due: true, Reason: "older than cadence", AgeSecs: &ageSecs, Target: echo}) return } - writeOK(w, BackupDueResponse{VMID: vmid, Due: false, Reason: "within cadence window", AgeSecs: &ageSecs}) + writeOK(w, BackupDueResponse{VMID: vmid, Due: false, Reason: "within cadence window", AgeSecs: &ageSecs, Target: echo}) +} + +// BackupTiersResponse is GET /backup/tiers (R-82): the tiers this agent serves, primary first. +// A controller that gets 404 here is talking to a PRE-R-82 agent and must fall back to the single +// untargeted tier — that 404 is the designed capability probe. +type BackupTiersResponse struct { + VMID int `json:"vmid"` + Tiers []BackupTierInfo `json:"tiers"` +} + +// BackupTierInfo is one tier as advertised to the controller. +type BackupTierInfo struct { + Target string `json:"target"` + CadenceSeconds int64 `json:"cadence_seconds"` + Primary bool `json:"primary"` +} + +func (s *Server) handleBackupTiers(w http.ResponseWriter, r *http.Request, vmid int) { + resp := BackupTiersResponse{VMID: vmid, Tiers: make([]BackupTierInfo, 0, len(s.tiers))} + for _, t := range s.tiers { + resp.Tiers = append(resp.Tiers, BackupTierInfo{ + Target: t.TargetID, + CadenceSeconds: int64(t.Cadence.Seconds()), + Primary: t.Primary, + }) + } + writeOK(w, resp) +} + +// tierFromRequest resolves the `?target=` query parameter to a tier. +// +// THE COMPATIBILITY RULE (§4): NO target parameter → the PRIMARY tier, and the echoed target is +// EMPTY so the response marshals byte-identically to pre-R-82 (BackupDueResponse.Target is +// omitempty). An old controller cannot tell this agent from the old one. +// +// An UNKNOWN target is a 400, never a silent fallback to the primary: a controller asking about a +// tier this agent does not serve must find out, not be told about a different tier's freshness. +func (s *Server) tierFromRequest(w http.ResponseWriter, r *http.Request) (BackupTier, string, bool) { + want := strings.TrimSpace(r.URL.Query().Get("target")) + if want == "" { + return s.primaryTier(), "", true + } + for _, t := range s.tiers { + if t.TargetID == want { + return t, t.TargetID, true + } + } + writeStatus(w, http.StatusBadRequest, false, nil, "unknown backup target: "+want) + return BackupTier{}, "", false +} + +// primaryTier returns the tier every untargeted endpoint acts on. tiers is never empty (New +// synthesizes one), but this stays defensive: a zero tier would silently disable backups. +func (s *Server) primaryTier() BackupTier { + for _, t := range s.tiers { + if t.Primary { + return t + } + } + if len(s.tiers) > 0 { + return s.tiers[0] + } + return BackupTier{TargetID: "", Cadence: defaultBackupCadence, Service: s.backups} } // BackupStatusResponse is GET /backup/status (slice 8B): the current/last job phase + the latest @@ -778,11 +911,20 @@ type BackupStatusResponse struct { JobID string `json:"job_id,omitempty"` Error string `json:"error,omitempty"` Backup *hub.Backup `json:"backup,omitempty"` // latest recorded backup for this guest + // Target (R-82) echoes the tier; empty + omitted when untargeted (pre-R-82 bytes). + Target string `json:"target,omitempty"` } func (s *Server) handleBackupStatus(w http.ResponseWriter, r *http.Request, vmid int) { - resp := BackupStatusResponse{VMID: vmid, Phase: PhaseIdle, Backup: s.latestBackupFor(r.Context(), vmid)} - if job, ok := s.jobSnapshot(vmid); ok { + tier, echo, ok0 := s.tierFromRequest(w, r) + if !ok0 { + return + } + // Untargeted keeps the pre-R-82 meaning EXACTLY: the primary tier's job, and the newest backup + // across ANY target (echo == "" → pickLatestBackup's match-any path). + resp := BackupStatusResponse{VMID: vmid, Phase: PhaseIdle, Target: echo, + Backup: s.pickLatestBackup(r.Context(), vmid, false, echo)} + if job, ok := s.jobSnapshot(backupJobKey{vmid: vmid, target: tier.TargetID}); ok { resp.Phase = job.Phase resp.JobID = job.JobID resp.Error = job.Error @@ -805,21 +947,34 @@ func (s *Server) handleRestoreTestStatus(w http.ResponseWriter, r *http.Request, // latestBackupFor returns this guest's most recent backup from the store (nil if none). func (s *Server) latestBackupFor(ctx context.Context, vmid int) *hub.Backup { - return s.pickLatestBackup(ctx, vmid, false) + return s.pickLatestBackup(ctx, vmid, false, "") } // latestSuccessfulBackupFor returns this guest's most recent SUCCESSFUL backup (nil if none) — // the basis for /backup/due (a failed backup must not satisfy the cadence). func (s *Server) latestSuccessfulBackupFor(ctx context.Context, vmid int) *hub.Backup { - return s.pickLatestBackup(ctx, vmid, true) + return s.pickLatestBackup(ctx, vmid, true, "") } -func (s *Server) pickLatestBackup(ctx context.Context, vmid int, successOnly bool) *hub.Backup { +// latestSuccessfulBackupForTarget is the R-82 per-tier twin: a tier's due-ness must be judged +// against ITS OWN newest successful backup. The store is already keyed by target, so this is a +// filter, not a data-model change — but WITHOUT it a fresh local backup would satisfy the PBS +// tier's cadence and the DR tier would never run. +func (s *Server) latestSuccessfulBackupForTarget(ctx context.Context, vmid int, target string) *hub.Backup { + return s.pickLatestBackup(ctx, vmid, true, target) +} + +// pickLatestBackup returns the newest matching record. target "" matches ANY target (the pre-R-82 +// behaviour, kept for the untargeted status endpoint). +func (s *Server) pickLatestBackup(ctx context.Context, vmid int, successOnly bool, target string) *hub.Backup { var latest *hub.Backup for _, b := range s.store.Backups(ctx) { if b.VMID != vmid || (successOnly && !b.Success) { continue } + if target != "" && b.TargetID != target { + continue + } bb := b if latest == nil || bb.StartedAt > latest.StartedAt { // RFC3339 sorts lexically latest = &bb