From 62c6e074771572473814eaed099be239ed843e2a Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Sat, 5 Sep 2026 01:05:08 +0100 Subject: [PATCH] fix(truenas): recognise finished replication outcomes (#1892) TrueNAS emits FINISHED for successful replication, but the recovery mapper treated it as unknown. Recognise that provider state while preserving error precedence and the missing-run guard; add regression coverage reproducing the reported outcome. Change-source: pulse-maintainer --- .../internal/subsystems/storage-recovery.md | 7 +++ internal/recovery/mapper/truenas/mapper.go | 2 +- .../recovery/mapper/truenas/mapper_test.go | 35 +++++++++++ internal/recovery/store/store_test.go | 59 +++++++++++++++++++ 4 files changed, 102 insertions(+), 1 deletion(-) diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 32508454e..16e4fbbff 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -21,6 +21,13 @@ ## Purpose +TrueNAS replication state `FINISHED` is successful completion evidence when a +last-run timestamp exists, matching the provider success event. Explicit error +text still takes precedence; a missing last-run timestamp remains unknown. +Persisted recovery rollups must retain this outcome and success time rather +than classify a completed replication as unknown. This does not imply restore +verification or resolve provider INFO alert lifecycle behaviour. + Unraid `StorageMeta.numDisks` is optional source evidence: zero denotes an explicitly empty parity array, not proof that pools are protected or that a recovery point exists. Suppressing the no-parity warning for zero must not erase disk-failure evidence or grant storage/recovery authority. A manifestless PBS snapshot never becomes a successful recovery point and diff --git a/internal/recovery/mapper/truenas/mapper.go b/internal/recovery/mapper/truenas/mapper.go index f3d6abb6d..643cc8b36 100644 --- a/internal/recovery/mapper/truenas/mapper.go +++ b/internal/recovery/mapper/truenas/mapper.go @@ -21,7 +21,7 @@ func outcomeFromTrueNASReplication(state string, errText string) recovery.Outcom } normalized := strings.ToLower(strings.TrimSpace(state)) switch normalized { - case "success", "succeeded", "ok", "complete", "completed": + case "success", "succeeded", "ok", "complete", "completed", "finished": return recovery.OutcomeSuccess case "warning", "partial", "partiallyfailed", "partially_failed": return recovery.OutcomeWarning diff --git a/internal/recovery/mapper/truenas/mapper_test.go b/internal/recovery/mapper/truenas/mapper_test.go index bd07d9187..4f89cf852 100644 --- a/internal/recovery/mapper/truenas/mapper_test.go +++ b/internal/recovery/mapper/truenas/mapper_test.go @@ -198,6 +198,9 @@ func TestOutcomeFromTrueNASReplication(t *testing.T) { {"ok", "ok", "", recovery.OutcomeSuccess}, {"complete", "complete", "", recovery.OutcomeSuccess}, {"completed", "completed", "", recovery.OutcomeSuccess}, + {"finished uppercase", "FINISHED", "", recovery.OutcomeSuccess}, + {"finished normalized", " Finished ", " ", recovery.OutcomeSuccess}, + {"finished error takes precedence", "FINISHED", "replication failed", recovery.OutcomeFailed}, {"warning", "warning", "", recovery.OutcomeWarning}, {"partial", "partial", "", recovery.OutcomeWarning}, {"partiallyfailed", "partiallyfailed", "", recovery.OutcomeWarning}, @@ -259,3 +262,35 @@ func TestSplitSnapshotName(t *testing.T) { func ptrInt64(v int64) *int64 { return &v } + +// FINISHED is emitted by TrueNAS zettarepl for ReplicationTaskSuccess (#1892). +func TestFromTrueNASSnapshot_FinishedReplication(t *testing.T) { + lastRun := time.Date(2026, 9, 4, 5, 45, 0, 0, time.UTC) + for _, tc := range []struct { + name string + lastRun *time.Time + errText string + want recovery.Outcome + }{ + {"completed", &lastRun, "", recovery.OutcomeSuccess}, + {"error retained", &lastRun, "transfer failed", recovery.OutcomeFailed}, + {"missing run remains unknown", nil, "", recovery.OutcomeUnknown}, + } { + t.Run(tc.name, func(t *testing.T) { + points := FromTrueNASSnapshot("conn", &truenas.FixtureSnapshot{ + ReplicationTasks: []truenas.ReplicationTask{{ + ID: "1", LastState: "FINISHED", LastRun: tc.lastRun, LastError: tc.errText, + }}, + }) + if len(points) != 1 { + t.Fatalf("got %d points", len(points)) + } + if points[0].Outcome != tc.want { + t.Errorf("outcome = %v, want %v", points[0].Outcome, tc.want) + } + if points[0].Details["lastState"] != "FINISHED" { + t.Error("provider state lost") + } + }) + } +} diff --git a/internal/recovery/store/store_test.go b/internal/recovery/store/store_test.go index b5820be7a..026804aa7 100644 --- a/internal/recovery/store/store_test.go +++ b/internal/recovery/store/store_test.go @@ -9,6 +9,8 @@ import ( "time" "github.com/rcourtman/pulse-go-rewrite/internal/recovery" + truenasmapper "github.com/rcourtman/pulse-go-rewrite/internal/recovery/mapper/truenas" + "github.com/rcourtman/pulse-go-rewrite/internal/truenas" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" ) @@ -1114,3 +1116,60 @@ func assertRecoveryColumnExists(t *testing.T, dbPath string, column string) { } t.Fatalf("expected recovery_points column %q to exist after migration", column) } + +func TestStore_TrueNASFinishedReplicationRollup(t *testing.T) { + at := time.Date(2026, 9, 4, 12, 0, 0, 0, time.UTC) + for _, tc := range []struct { + name string + lastRun *time.Time + errText string + want recovery.Outcome + }{ + {"finished", &at, "", recovery.OutcomeSuccess}, + {"explicit error", &at, "transfer failed", recovery.OutcomeFailed}, + {"missing run", nil, "", recovery.OutcomeUnknown}, + } { + t.Run(tc.name, func(t *testing.T) { + dbPath := filepath.Join(t.TempDir(), "recovery.db") + db, err := Open(dbPath) + if err != nil { + t.Fatal(err) + } + points := truenasmapper.FromTrueNASSnapshot("nas", &truenas.FixtureSnapshot{ + CollectedAt: at, + ReplicationTasks: []truenas.ReplicationTask{{ + ID: "1", Name: "backup", LastState: "FINISHED", LastRun: tc.lastRun, LastError: tc.errText, + }}, + }) + if err := db.UpsertPoints(context.Background(), points); err != nil { + _ = db.Close() + t.Fatal(err) + } + if err := db.Close(); err != nil { + t.Fatal(err) + } + db, err = Open(dbPath) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = db.Close() }) + rows, total, err := db.ListRollups(context.Background(), recovery.ListPointsOptions{Page: 1, Limit: 50}) + if err != nil { + t.Fatal(err) + } + if total != 1 || len(rows) != 1 { + t.Fatalf("rollups total=%d len=%d, want 1/1", total, len(rows)) + } + if rows[0].LastOutcome != tc.want { + t.Fatalf("outcome=%v, want %v", rows[0].LastOutcome, tc.want) + } + if tc.want == recovery.OutcomeSuccess { + if rows[0].LastSuccessAt == nil || !rows[0].LastSuccessAt.Equal(at) { + t.Fatalf("success time=%v, want %v", rows[0].LastSuccessAt, at) + } + } else if rows[0].LastSuccessAt != nil { + t.Fatalf("unexpected success time: %v", rows[0].LastSuccessAt) + } + }) + } +}