diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a90b16..62f98af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,44 @@ All notable changes to **felhom-agent** are recorded here. Update on every code change that gets pushed. +## v0.39.0 — DR recipe completion: live PBS coord + drop the two unfillable drive fields (2026-06-16) + +**DR-recipe agent-half completion.** A live eyeball of the demo recipe (v0.38.0) found three host-half +problems; all three are resolved here. No behavior change outside the recipe path. + +- **PBS coord now resolved LIVE each collect.** New `internal/pbs/live_reporter.go` — + `LiveSnapshotReporter` implements `hub.PBSReporter` by doing the cheap `Client.Snapshots()` list + itself, with **last-known-good fallback**, instead of reading only the verify-loop's `SnapshotStore`. + Previously the recipe's `pbs` block was omitted whenever the store was empty — which a one-shot + collect (`--selftest=hub`) and the first ~6 h window of every daemon after a restart always saw (the + verify loop populates the store on its own 6 h cadence). The restore SOURCE must not depend on a + maintenance cadence. Per-datastore: a live error/timeout → that datastore's last-known-good; a + successful (even empty) response is authoritative and updates the shared store. Targets-resolution + failure → the full LKG aggregate. Bounded by `DefaultLiveSnapshotTimeout` (8 s) so a hung PBS never + stalls the heartbeat. List only — it never triggers a `Verify`. The verify loop keeps Recording into + the SAME store (shared last-known-good); both use one hoisted `pbsTargets` closure. + - `SnapshotStore.Get(datastore)` added (per-datastore LKG copy) — the only `SnapshotStore` change. + - Wired into the collector in BOTH `runDaemon` and `runSelftestHub` (the selftest built its own + collector with a `nil` reporter — that is why the live `--selftest=hub` showed `pbs_snapshots:[]`). + - Intended side effect: `report.pbs_snapshots` is now live too (fresher hub PBS view). +- **`drives[].role` DROPPED from the v1 host-half shape.** A drive's purpose is a hub/operator-owned + manifest concept, not cleanly derivable host-side (both demo externals are `content=backup`, yet one + is the primary data drive and the other holds no apps). Deferred until the hub/operator stamps it. +- **`drives[].restic_repo_coord` DROPPED from the v1 host-half shape.** It named a backup tier that does + not exist — cross-drive backup is rsync to the SAME internal SSD; there is no offsite/second-failure- + domain bulk copy. RESERVED for a future tier (see the BACKLOG note in REPORT). v1 drive shape is now + `{durable_id, mount_path, intent, fs_type?, total_bytes}` — identifiers/intent/size only. +- The hub reads drives as `json.RawMessage`, so dropping fields needs NO hub struct change — only + golden + test sync. Cross-repo golden (`host-report.golden.json` here + the hub's copy) re-pinned and + verified **byte-identical** (sha256 `57f2a5e7…18b2f2b5` — manual checksum-diff discipline): the hub copy + previously lacked the `dr_recipe` section entirely; it is now a verbatim copy of the agent golden. +- Tests: new `internal/pbs/live_reporter_test.go` (T1 coord-present-without-prior-verify [load-bearing] + + inline bare-store companion, T2 error→LKG fallback, T3 success-warms-store, T4 targets-error→aggregate, + T5 bounded-by-timeout, T6 empty-success-authoritative); `TestDRRecipeHostHalf_V1DriveShape` (drive + object carries neither `role` nor `restic_repo_coord`); `TestBuildDRRecipeHostHalf` / + `TestHostReport_ContractMatchesGolden` updated to the v1 drive shape. Each companion was demonstrated + to FAIL on the pre-fix/mutated code, then reverted (see REPORT). + ## v0.38.0 — DR recipe: emit the secret-free storage/guest/PBS half in the host-report (2026-06-16) **DR recipe slice (agent half).** Additive `dr_recipe` section on the host-report — the agent half of the diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index 0804ca3..8c5066b 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -44,7 +44,7 @@ import ( // version is the agent version. Overridable at build time with // -ldflags "-X main.version="; defaults to the in-repo CHANGELOG version. -var version = "0.38.0" +var version = "0.39.0" // runGuestHook is the PVE pre-start hook body (`felhom-agent guest-hook `). On the // pre-start phase it creates placeholder dirs for any absent bind-mount source so the guest always boots @@ -289,10 +289,17 @@ func runDaemon(cfg config.Config, logger *slog.Logger) int { // latest restore-test result; the collector reads it via the BackupReporter / // RestoreTestReporter seams; the cadence scheduler writes it. backupStore := backup.NewStore() - // PBS snapshot inventory + verify-state (slice 6 Phase B): the verify loop writes it; the - // collector reads it via the PBSReporter seam. + // PBS snapshot inventory + verify-state (slice 6 Phase B): the verify loop writes the shared + // store; the collector reads it via the PBSReporter seam. DR-recipe completion (v0.39.0): the + // collector now reads through a LiveSnapshotReporter that lists snapshots LIVE each collect + // (cheap GET, last-known-good fallback) so the host-report's pbs coord is present whenever PBS is + // reachable — even seconds after a restart, before the 6 h verify loop has run. The verify loop + // keeps Recording into the SAME store (shared last-known-good); targets are resolved once and + // shared by both. pbsTargets is hoisted so the reporter and the verify loop use one closure. pbsStore := pbs.NewSnapshotStore() - collector := hub.NewCollector(px, hub.SystemctlProber{}, observer, backupStore, backupStore, pbsStore, cfg.Hub.HostID, version, logger) + pbsTargets := pbsTargetsFromPVE(cfg, px, logger) + pbsReporter := pbs.NewLiveSnapshotReporter(pbsTargets, pbsStore, pbs.DefaultLiveSnapshotTimeout, logger) + collector := hub.NewCollector(px, hub.SystemctlProber{}, observer, backupStore, backupStore, pbsReporter, cfg.Hub.HostID, version, logger) loop := hub.NewLoop(collector, client, time.Duration(hcfg.PollSeconds)*time.Second, logger) interval := time.Duration(hcfg.PollSeconds) * time.Second @@ -430,7 +437,7 @@ func runDaemon(cfg config.Config, logger *slog.Logger) int { // verify-state. It is maintenance/reporting (NOT gated/journaled). Auto-discovers pbs // storages from the PVE config each cycle; disabled cleanly (cadence<0) without crashing. pbsLoop := pbs.NewVerifyLoop(pbs.VerifyLoopOptions{ - Targets: pbsTargetsFromPVE(cfg, px, logger), + Targets: pbsTargets, // the same closure the live reporter uses (shared store, one resolver) Store: pbsStore, Cadence: cfg.Backup.PBSVerifyCadence(), Logger: logger, @@ -807,7 +814,12 @@ func runSelftestHub(ctx context.Context, cfg config.Config, logger *slog.Logger) return 1 } observer := storage.NewObserver(px, storage.NewProcHostReader(), newHostOps(cfg, logger), logger) - collector := hub.NewCollector(px, hub.SystemctlProber{}, observer, nil, nil, nil, cfg.Hub.HostID, version, logger) + // Wire the LIVE PBS reporter here too (v0.39.0): selftest=hub is a separate one-shot process — no + // verify loop runs — so a nil reporter previously yielded pbs_snapshots:[] and an absent dr_recipe + // pbs coord. The live reporter lists snapshots directly (fresh store, last-known-good fallback) so + // the selftest reflects exactly what a freshly-restarted daemon's first collect emits. + pbsReporter := pbs.NewLiveSnapshotReporter(pbsTargetsFromPVE(cfg, px, logger), pbs.NewSnapshotStore(), pbs.DefaultLiveSnapshotTimeout, logger) + collector := hub.NewCollector(px, hub.SystemctlProber{}, observer, nil, nil, pbsReporter, cfg.Hub.HostID, version, logger) ctx, cancel := context.WithTimeout(ctx, 60*time.Second) defer cancel() diff --git a/internal/hub/dr_recipe.go b/internal/hub/dr_recipe.go index 6b8b8c3..1820ec5 100644 --- a/internal/hub/dr_recipe.go +++ b/internal/hub/dr_recipe.go @@ -3,9 +3,9 @@ package hub import "sort" // DR recipe — the agent (storage/guest/PBS) HALF of the secret-free reconstruction recipe -// (SPIKE-dr-recipe-2026-06-16). The recipe complements escrow (keys) + PBS/restic (bytes): it is +// (SPIKE-dr-recipe-2026-06-16). The recipe complements escrow (keys) + PBS (bytes): it is // the non-secret SCAFFOLDING an operator must rebuild before the PBS bytes can land — guest sizing, -// drive inventory (durable-id → role → mount → intent), PVE storage defs, and PBS coordinates. +// drive inventory (durable-id → mount → intent → size), PVE storage defs, and PBS coordinates. // // BOUNDARY (non-negotiable, the Phase-1 lesson): every field here is an identifier, intent, size, or // coordinate — NEVER a key, password, token, hash, or ENC: value. Secrets live in the PBS whole-CT @@ -13,6 +13,17 @@ import "sort" // asserts no field name matches the secret regex. The hub assembles this half with the controller's // app half into one customer recipe. // +// v1 host-half drive shape (v0.39.0) = identifiers/intent/size ONLY: {durable_id, mount_path, intent, +// fs_type?, total_bytes}. Two fields were deliberately DROPPED from v1: +// - role — a drive's purpose (primary/bulk-data/…) is a hub/operator-owned manifest concept, not +// cleanly derivable host-side (both demo externals are content=backup, yet one is the primary +// data drive and the other holds no apps). Deferred until the hub/operator stamps it. +// - restic_repo_coord — RESERVED for a future offsite bulk-volume backup tier. None exists today: +// external-drive data has no offsite/second-failure-domain copy (cross-drive backup is rsync to +// the SAME internal SSD), so the field named nothing real. Re-add when that tier ships. +// The pbs coord, by contrast, is resolved LIVE each collect (LiveSnapshotReporter) so the restore +// SOURCE is present whenever PBS is reachable — not gated on the 6 h verify cadence. +// // recipe_version=1. The wire shape is byte-pinned in the cross-repo golden (host-report.golden.json // here + the hub's copy) — see the manual checksum-diff discipline in CHANGELOG. Read is // ignore-unknown (encoding/json default) for forward-compat, mirroring storage_manifest. @@ -44,17 +55,15 @@ type DRPBSCoord struct { LatestSnapshotID string `json:"latest_snapshot_id"` // most-recent snapshot's backup_id (a coordinate) } -// DRDrive is one user-data drive: identifiers + intent + size. The restic_repo_coord NAMES where the -// bulk-volume backup lives (PBS excludes external drives — the UncoveredVolumes gap); the restic -// PASSWORD stays in escrow, never here. +// DRDrive is one user-data drive: identifiers + intent + size. v1 carries ONLY these fields (role + +// restic_repo_coord were dropped — see the file header for why). Every field is an identifier, intent, +// or size; none is a credential. type DRDrive struct { - DurableID string `json:"durable_id"` // uuid: — a hardware identifier, not a credential - Role string `json:"role"` - MountPath string `json:"mount_path"` - Intent string `json:"intent"` // enrolled | ejected | decommissioned - FSType string `json:"fs_type,omitempty"` - TotalBytes int64 `json:"total_bytes"` - ResticRepoCoord string `json:"restic_repo_coord,omitempty"` // bulk-backup location coord (password in escrow) + DurableID string `json:"durable_id"` // uuid: — a hardware identifier, not a credential + MountPath string `json:"mount_path"` + Intent string `json:"intent"` // enrolled | ejected | decommissioned + FSType string `json:"fs_type,omitempty"` + TotalBytes int64 `json:"total_bytes"` } // DRPVEStorage is a PVE storage definition (to rebuild /etc/pve/storage.cfg scaffolding) — no auth. @@ -102,7 +111,6 @@ func BuildDRRecipeHostHalf(guests []Guest, targets []StorageTarget, pbs []PBSSna if isUserDataDrive(t) { h.Drives = append(h.Drives, DRDrive{ DurableID: t.DurableID, - Role: t.Role, MountPath: t.MountPath, Intent: driveIntentEnrolled, TotalBytes: t.TotalBytes, diff --git a/internal/hub/dr_recipe_test.go b/internal/hub/dr_recipe_test.go index 0b16016..2be9870 100644 --- a/internal/hub/dr_recipe_test.go +++ b/internal/hub/dr_recipe_test.go @@ -54,7 +54,7 @@ func TestBuildDRRecipeHostHalf(t *testing.T) { t.Errorf("drive %s intent=%q, want enrolled", d.DurableID, d.Intent) } } - if d, ok := byDur["uuid:da9e7089"]; !ok || d.Role != "bulk-data" || d.MountPath != "/mnt/felhom-usb" || d.TotalBytes != 931<<30 { + if d, ok := byDur["uuid:da9e7089"]; !ok || d.MountPath != "/mnt/felhom-usb" || d.TotalBytes != 931<<30 { t.Errorf("felhom-usb drive wrong: %+v", d) } if _, ok := byDur["uuid:81a26531"]; !ok { @@ -77,6 +77,42 @@ func TestBuildDRRecipeHostHalf_NoPBS(t *testing.T) { } } +// TestDRRecipeHostHalf_V1DriveShape pins the v1 host-half drive shape: a drive object carries ONLY +// {durable_id, mount_path, intent, total_bytes} (fs_type is omitempty) — and specifically NEITHER the +// dropped "role" NOR "restic_repo_coord" keys. Re-adding either field to DRDrive makes this fail +// (the companion: `Role string \`json:"role"\`` reintroduces the "role" key → caught here). +func TestDRRecipeHostHalf_V1DriveShape(t *testing.T) { + h := BuildDRRecipeHostHalf( + nil, + []StorageTarget{ + {Name: "felhom-usb", Type: StorageTypeUSB, DurableID: "uuid:da9e7089", Role: "bulk-data", + MountPath: "/mnt/felhom-usb", TotalBytes: 931 << 30}, + }, + nil, + ) + if len(h.Drives) != 1 { + t.Fatalf("want 1 drive, got %d", len(h.Drives)) + } + b, err := json.Marshal(h.Drives[0]) + if err != nil { + t.Fatal(err) + } + var keys map[string]json.RawMessage + if err := json.Unmarshal(b, &keys); err != nil { + t.Fatal(err) + } + for _, banned := range []string{"role", "restic_repo_coord"} { + if _, ok := keys[banned]; ok { + t.Errorf("v1 drive must NOT carry %q key (it was dropped); got %s", banned, b) + } + } + for _, want := range []string{"durable_id", "mount_path", "intent", "total_bytes"} { + if _, ok := keys[want]; !ok { + t.Errorf("v1 drive missing required key %q; got %s", want, b) + } + } +} + // TestDRRecipeHostHalf_NoSecrets is the agent-side boundary assertion (the lighter mirror of the // controller's load-bearing boundary test): a fully-populated host-half must carry NO field whose // name smells like a credential. If a future field leaks a key/token/hash in, this fails. @@ -97,8 +133,8 @@ func TestDRRecipeHostHalf_NoSecrets(t *testing.T) { } // assertNoSecretKeys walks decoded JSON and fails on any object key matching secretNameRe. Shared by -// the agent boundary assertions. (durable_id/repo_id/latest_snapshot_id/restic_repo_coord are -// identifiers/coordinates — none match the credential regex.) +// the agent boundary assertions. (durable_id/repo_id/latest_snapshot_id are identifiers/coordinates — +// none match the credential regex.) func assertNoSecretKeys(t *testing.T, jsonBytes []byte) { t.Helper() var v any diff --git a/internal/hub/testdata/host-report.golden.json b/internal/hub/testdata/host-report.golden.json index 9462d95..16832c1 100644 --- a/internal/hub/testdata/host-report.golden.json +++ b/internal/hub/testdata/host-report.golden.json @@ -145,7 +145,6 @@ "drives": [ { "durable_id": "uuid:0fc63daf-8483-4772-8e79-3d69d8477de4", - "role": "", "mount_path": "/mnt/usb-backup", "intent": "enrolled", "total_bytes": 2000000000000 diff --git a/internal/pbs/live_reporter.go b/internal/pbs/live_reporter.go new file mode 100644 index 0000000..0bd9b20 --- /dev/null +++ b/internal/pbs/live_reporter.go @@ -0,0 +1,97 @@ +package pbs + +import ( + "context" + "log/slog" + "time" + + "gitea.dooplex.hu/admin/felhom-agent/internal/hub" +) + +// DefaultLiveSnapshotTimeout bounds the per-collect live PBS reads so a slow/hung PBS can never +// stall the host-report (and thus the heartbeat). A few seconds is ample for the cheap list GET on +// the LAN; on overrun the reporter falls back to last-known-good rather than blocking. +const DefaultLiveSnapshotTimeout = 8 * time.Second + +// LiveSnapshotReporter resolves the PBS snapshot inventory LIVE on each collect (the cheap +// Snapshots() list), so the host-report — and the DR recipe's pbs coord derived from it — is present +// whenever PBS is reachable, INDEPENDENT of the 6 h verify cadence. This closes the gap where a +// one-shot collect (selftest=hub) and the first window of every daemon after a restart saw the +// verify-loop's SnapshotStore still empty (→ no pbs coord, the restore SOURCE missing). +// +// On a per-datastore live error/timeout it falls back to the store's last-known-good; a successful +// (even empty) response is authoritative and updates the store. If targets cannot be resolved at all +// it returns the store's full aggregate. It shares the same *SnapshotStore the verify loop Records +// into, so the two keep each other's last-known-good warm. It NEVER triggers a verify — list only. +type LiveSnapshotReporter struct { + targets Targets // pbsTargetsFromPVE(...) closure — re-resolved each collect + store *SnapshotStore // shared last-known-good cache (verify loop writes it too) + timeout time.Duration // per-collect bound on the live reads + log *slog.Logger + + // listSnapshots is the production→PBS seam, overridable in tests so no live PBS is needed. + // Default = liveListSnapshots (one Snapshots() GET, converted via Snapshot.ToHub()). + listSnapshots func(ctx context.Context, t Target) ([]hub.PBSSnapshot, error) +} + +// NewLiveSnapshotReporter builds a live reporter sharing store with the verify loop. A zero timeout +// falls back to DefaultLiveSnapshotTimeout; a nil logger to slog.Default. +func NewLiveSnapshotReporter(targets Targets, store *SnapshotStore, timeout time.Duration, log *slog.Logger) *LiveSnapshotReporter { + if timeout <= 0 { + timeout = DefaultLiveSnapshotTimeout + } + if log == nil { + log = slog.Default() + } + return &LiveSnapshotReporter{ + targets: targets, + store: store, + timeout: timeout, + log: log, + listSnapshots: liveListSnapshots, + } +} + +// liveListSnapshots does ONE cheap snapshot list for a target and converts each record to the hub +// wire shape (reusing Snapshot.ToHub — RFC3339 backup_time, verify_state). List only; no verify. +func liveListSnapshots(ctx context.Context, t Target) ([]hub.PBSSnapshot, error) { + snaps, err := t.Client.Snapshots(ctx, t.Datastore) + if err != nil { + return nil, err + } + out := make([]hub.PBSSnapshot, 0, len(snaps)) + for _, s := range snaps { + out = append(out, s.ToHub()) + } + return out, nil +} + +// PBSSnapshots implements hub.PBSReporter with a single bounded, live pass (last-known-good +// fallback). Non-nil result so it marshals as []. +func (r *LiveSnapshotReporter) PBSSnapshots(ctx context.Context) []hub.PBSSnapshot { + childCtx, cancel := context.WithTimeout(ctx, r.timeout) + defer cancel() + + targets, err := r.targets(childCtx) + if err != nil { + // Cannot even enumerate datastores → fall back to the full last-known-good aggregate. + r.log.Debug("pbs: live targets resolution failed; using last-known-good aggregate", "err", err) + return r.store.PBSSnapshots(ctx) + } + + out := []hub.PBSSnapshot{} + for _, t := range targets { + snaps, err := r.listSnapshots(childCtx, t) + if err != nil { + // Per-datastore live failure → that datastore's last-known-good (does NOT clobber it). + r.log.Debug("pbs: live list failed; using last-known-good", "datastore", t.Datastore, "err", err) + out = append(out, r.store.Get(t.Datastore)...) + continue + } + // A successful response (INCLUDING empty) is authoritative — record it as the new + // last-known-good so a later error reuses fresh truth, not a stale set. + r.store.Record(t.Datastore, snaps) + out = append(out, snaps...) + } + return out +} diff --git a/internal/pbs/live_reporter_test.go b/internal/pbs/live_reporter_test.go new file mode 100644 index 0000000..fcc52a0 --- /dev/null +++ b/internal/pbs/live_reporter_test.go @@ -0,0 +1,186 @@ +package pbs + +import ( + "context" + "errors" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-agent/internal/hub" +) + +// fakeReporter builds a LiveSnapshotReporter with an injected snapshot-lister seam and a targets +// closure, so no live PBS is needed. listFn is keyed by datastore; targetsErr forces a targets- +// resolution failure. The Target.Client is a throwaway &Client{} the seam never dereferences. +func fakeReporter(store *SnapshotStore, datastores []string, targetsErr error, + listFn func(ctx context.Context, ds string) ([]hub.PBSSnapshot, error)) *LiveSnapshotReporter { + targets := func(ctx context.Context) ([]Target, error) { + if targetsErr != nil { + return nil, targetsErr + } + out := make([]Target, 0, len(datastores)) + for _, ds := range datastores { + out = append(out, Target{Datastore: ds, Client: &Client{}}) + } + return out, nil + } + r := NewLiveSnapshotReporter(targets, store, 2*time.Second, nil) + r.listSnapshots = func(ctx context.Context, t Target) ([]hub.PBSSnapshot, error) { + return listFn(ctx, t.Datastore) + } + return r +} + +func snap(ds, backupID, backupTime string) hub.PBSSnapshot { + return hub.PBSSnapshot{Namespace: "root", BackupType: "ct", BackupID: backupID, BackupTime: backupTime} +} + +// T1 (load-bearing): coord present with NO prior verify. A fresh store (verify never ran) + a live +// lister returning 2 snapshots → PBSSnapshots returns both, and feeding them through +// BuildDRRecipeHostHalf yields a non-nil pbs whose latest_snapshot_id is the LATER snapshot. +func TestLiveReporter_CoordPresentWithoutPriorVerify(t *testing.T) { + store := NewSnapshotStore() // empty — simulates a just-restarted daemon, no verify yet + r := fakeReporter(store, []string{"felhom-spike"}, nil, + func(ctx context.Context, ds string) ([]hub.PBSSnapshot, error) { + return []hub.PBSSnapshot{ + snap(ds, "9201", "2026-06-13T20:00:00Z"), + snap(ds, "9201", "2026-06-16T17:00:00Z"), // latest + }, nil + }) + + got := r.PBSSnapshots(context.Background()) + if len(got) != 2 { + t.Fatalf("want 2 live snapshots, got %d", len(got)) + } + + // The whole point: a recipe built from the live read carries the pbs coord. + h := hub.BuildDRRecipeHostHalf(nil, + []hub.StorageTarget{{Name: "felhom-pbs", Type: hub.StorageTypePBS, Content: "backup"}}, got) + if h.PBS == nil { + t.Fatal("pbs coord absent despite a reachable PBS — the gap this fixes") + } + if h.PBS.RepoID != "felhom-pbs" || h.PBS.Namespace != "root" || h.PBS.LatestSnapshotID != "9201" { + t.Errorf("pbs coord = %+v, want felhom-pbs/root/9201", h.PBS) + } + + // COMPANION (pre-fix): the bare SnapshotStore (no live read) with an empty store omits pbs. + bare := NewSnapshotStore() + h2 := hub.BuildDRRecipeHostHalf(nil, + []hub.StorageTarget{{Name: "felhom-pbs", Type: hub.StorageTypePBS, Content: "backup"}}, + bare.PBSSnapshots(context.Background())) + if h2.PBS != nil { + t.Fatal("companion sanity: the bare store should yield NO pbs coord (proves the live read is load-bearing)") + } +} + +// T2: live error → last-known-good fallback (and the store is NOT clobbered). +func TestLiveReporter_ErrorFallsBackToLastKnownGood(t *testing.T) { + store := NewSnapshotStore() + store.Record("D", []hub.PBSSnapshot{snap("D", "9001", "2026-06-10T00:00:00Z")}) + + r := fakeReporter(store, []string{"D"}, nil, + func(ctx context.Context, ds string) ([]hub.PBSSnapshot, error) { + return nil, errors.New("pbs unreachable") + }) + + got := r.PBSSnapshots(context.Background()) + if len(got) != 1 || got[0].BackupID != "9001" { + t.Fatalf("want the last-known-good snapshot on live error, got %+v", got) + } + // The store must still hold the prior set (a failed live read must not wipe it). + if lkg := store.Get("D"); len(lkg) != 1 || lkg[0].BackupID != "9001" { + t.Errorf("live error clobbered the store: %+v", lkg) + } + + // COMPANION (pre-fix): without the fallback branch (return empty on error) the result is empty. + noFallback := func(ctx context.Context) []hub.PBSSnapshot { + targets, _ := r.targets(ctx) + out := []hub.PBSSnapshot{} + for _, tg := range targets { + if _, err := r.listSnapshots(ctx, tg); err != nil { + continue // the mutation: drop the LKG-append branch + } + } + return out + } + if len(noFallback(context.Background())) != 0 { + t.Fatal("companion sanity: the no-fallback variant should return empty") + } +} + +// T3: a successful response updates the store (last-known-good warm for a later error). +func TestLiveReporter_SuccessUpdatesStore(t *testing.T) { + store := NewSnapshotStore() // empty + r := fakeReporter(store, []string{"D"}, nil, + func(ctx context.Context, ds string) ([]hub.PBSSnapshot, error) { + return []hub.PBSSnapshot{snap("D", "9201", "2026-06-16T00:00:00Z")}, nil + }) + _ = r.PBSSnapshots(context.Background()) + if lkg := store.Get("D"); len(lkg) != 1 || lkg[0].BackupID != "9201" { + t.Errorf("store not warmed by a successful live read: %+v", lkg) + } +} + +// T4: targets-resolution error → the full last-known-good aggregate (not empty). +func TestLiveReporter_TargetsErrorReturnsAggregate(t *testing.T) { + store := NewSnapshotStore() + store.Record("D1", []hub.PBSSnapshot{snap("D1", "1", "2026-06-10T00:00:00Z")}) + store.Record("D2", []hub.PBSSnapshot{snap("D2", "2", "2026-06-11T00:00:00Z")}) + + r := fakeReporter(store, nil, errors.New("ListStorage failed"), + func(ctx context.Context, ds string) ([]hub.PBSSnapshot, error) { + t.Fatal("listSnapshots must not be called when targets fail to resolve") + return nil, nil + }) + got := r.PBSSnapshots(context.Background()) + if len(got) != 2 { + t.Fatalf("want the 2-snapshot aggregate on a targets-resolution error, got %d (%+v)", len(got), got) + } +} + +// T5: bounded. A lister that blocks until ctx is done must return within ~timeout (the child-ctx +// deadline) and fall back, not hang. A short timeout keeps the test fast + deterministic. +func TestLiveReporter_BoundedByTimeout(t *testing.T) { + store := NewSnapshotStore() + store.Record("D", []hub.PBSSnapshot{snap("D", "9001", "2026-06-10T00:00:00Z")}) + + targets := func(ctx context.Context) ([]Target, error) { + return []Target{{Datastore: "D", Client: &Client{}}}, nil + } + r := NewLiveSnapshotReporter(targets, store, 100*time.Millisecond, nil) + r.listSnapshots = func(ctx context.Context, t Target) ([]hub.PBSSnapshot, error) { + <-ctx.Done() // block until the child-ctx deadline fires + return nil, ctx.Err() + } + + done := make(chan []hub.PBSSnapshot, 1) + go func() { done <- r.PBSSnapshots(context.Background()) }() + select { + case got := <-done: + // Fell back to last-known-good rather than hanging. + if len(got) != 1 || got[0].BackupID != "9001" { + t.Errorf("want last-known-good after the deadline, got %+v", got) + } + case <-time.After(2 * time.Second): + t.Fatal("PBSSnapshots hung past the timeout — not bounded by the child ctx") + } +} + +// T6: an empty-but-successful response is authoritative — it OVERWRITES a prior non-empty set. +// (Documents the chosen semantics: a real empty datastore must be reflected, not masked by stale LKG.) +func TestLiveReporter_EmptySuccessIsAuthoritative(t *testing.T) { + store := NewSnapshotStore() + store.Record("D", []hub.PBSSnapshot{snap("D", "9001", "2026-06-10T00:00:00Z")}) + + r := fakeReporter(store, []string{"D"}, nil, + func(ctx context.Context, ds string) ([]hub.PBSSnapshot, error) { + return []hub.PBSSnapshot{}, nil // success, but the datastore is now empty + }) + got := r.PBSSnapshots(context.Background()) + if len(got) != 0 { + t.Errorf("empty-but-successful should yield an empty result, got %+v", got) + } + if lkg := store.Get("D"); len(lkg) != 0 { + t.Errorf("empty success should overwrite the store to empty, got %+v", lkg) + } +} diff --git a/internal/pbs/report.go b/internal/pbs/report.go index dee998e..9641b94 100644 --- a/internal/pbs/report.go +++ b/internal/pbs/report.go @@ -72,6 +72,21 @@ func (s *SnapshotStore) Record(datastore string, snaps []hub.PBSSnapshot) { s.byDatastore[datastore] = snaps } +// Get returns a copy of the last-known-good snapshot set for ONE datastore (nil when absent or +// empty). It is the per-datastore fallback the live reporter reaches for when a live list fails — +// distinct from PBSSnapshots, which aggregates every datastore. +func (s *SnapshotStore) Get(datastore string) []hub.PBSSnapshot { + s.mu.Lock() + defer s.mu.Unlock() + src := s.byDatastore[datastore] + if len(src) == 0 { + return nil + } + out := make([]hub.PBSSnapshot, len(src)) + copy(out, src) + return out +} + // PBSSnapshots implements hub.PBSReporter — all known snapshots across datastores. func (s *SnapshotStore) PBSSnapshots(context.Context) []hub.PBSSnapshot { s.mu.Lock()