diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index efe6f0ad0..8777dc780 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -17,6 +17,22 @@ ## Purpose +Direct PBS backup polling classifies typed API and authentication failures by +the response status exposed by `pbs.HTTPStatus`, including wrapped errors. +A 5xx gateway or server response remains transient even when its body quotes +an upstream “API error 403” or “API error 404”; that text must not erase the +last known backup inventory. Genuine 4xx responses remain terminal under the +existing cache policy. The legacy untyped-error fallback is unchanged. +Retaining cached inventory does not establish a successful poll or fresh +backup evidence. + +Verification: `TestPollPBSBackups_PreservesCacheOnTransientDatastoreError` and +`TestPollPBSBackups_DropsStaleCacheOnTerminalDatastoreError` in +`internal/monitoring/monitor_backups_readstate_test.go` exercise actual HTTP +fixtures for 500, 502 quoting 403, 503 quoting 404, and genuine 401/403/404. +These tests prove cache retention/removal, not installed PBS wake, service +restart, or notification receipt. + TrueNAS native alert projection preserves the trimmed, uppercase provider level in ResourceIncident.NativeSeverity. INFO and NOTICE retain the same canonical monitor risk; consumers must not lose their distinct actionability when projecting provider evidence. Native CRITICAL, ALERT, and EMERGENCY all project to canonical critical severity; EMERGENCY must not be discarded as unknown or make a still-active condition appear recovered. WARNING remains warning, and INFO and NOTICE remain informational at this projection boundary. Verification: `TestIncidentProjectionPreservesNativeSeverity` in `internal/truenas/provider_pool_health_contract_test.go` covers all seven native levels and case/whitespace normalization. `TestTrueNASNativeSeverityDispatch` in `internal/alerts/truenas_native_dispatch_test.go` verifies downstream INFO suppression, NOTICE preservation, notification severity, duplicate-poll retention, and confirmed recovery callback identity. The TrueNAS lifecycle tests in `internal/alerts/unified_incidents_test.go` require repeated EMERGENCY evidence to interrupt recovery confirmation. These are fixture-based projection and manager checks, not appliance ingestion or external notification-provider receipt proof. diff --git a/internal/monitoring/backup_guard.go b/internal/monitoring/backup_guard.go index 388768b02..f6e1b298d 100644 --- a/internal/monitoring/backup_guard.go +++ b/internal/monitoring/backup_guard.go @@ -1,6 +1,10 @@ package monitoring -import "strings" +import ( + "strings" + + "github.com/rcourtman/pulse-go-rewrite/pkg/pbs" +) // shouldPreservePBSBackupsWithTerminal preserves stale PBS backups only when all // datastore fetches failed and at least one failure was non-terminal. @@ -17,6 +21,10 @@ func shouldReuseCachedPBSBackups(err error) bool { if err == nil { return false } + if status, ok := pbs.HTTPStatus(err); ok { + return status < 400 || status >= 500 + } + // Retain compatibility with untyped errors from older callers. if strings.Contains(strings.ToLower(err.Error()), "api error 4") { return false } diff --git a/internal/monitoring/monitor_backups_readstate_test.go b/internal/monitoring/monitor_backups_readstate_test.go index 1baf09269..bd2b881d6 100644 --- a/internal/monitoring/monitor_backups_readstate_test.go +++ b/internal/monitoring/monitor_backups_readstate_test.go @@ -995,3 +995,117 @@ func TestRetirePVEInstanceRuntimeClearsPBSGuestConfirmations(t *testing.T) { } } } + +func TestPollPBSBackups_DropsStaleCacheOnTerminalDatastoreError(t *testing.T) { + t.Parallel() + + for _, status := range []int{http.StatusUnauthorized, http.StatusForbidden, http.StatusNotFound} { + t.Run(http.StatusText(status), func(t *testing.T) { + t.Parallel() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/admin/datastore/archive/groups") { + http.Error(w, `{"errors":"datastore does not exist"}`, status) + return + } + http.NotFound(w, r) + })) + defer server.Close() + + client, err := pbs.NewClient(pbs.ClientConfig{ + Host: server.URL, + TokenName: "root@pam!token", + TokenValue: "secret", + }) + if err != nil { + t.Fatalf("failed to create PBS client: %v", err) + } + + m := &Monitor{state: models.NewState()} + m.state.UpdatePBSBackups("pbs1", []models.PBSBackup{ + { + ID: "pbs-pbs1-archive--vm-100-1700000000", + Instance: "pbs1", + Datastore: "archive", + Namespace: "", + BackupType: "vm", + VMID: "100", + BackupTime: time.Unix(1700000000, 0), + }, + }) + + m.pollPBSBackups(context.Background(), "pbs1", client, []models.PBSDatastore{ + {Name: "archive"}, + }) + + snapshot := m.state.GetSnapshot() + for _, backup := range snapshot.PBSBackups { + if backup.Instance == "pbs1" { + t.Fatalf("expected stale backups to be removed after terminal error, found: %+v", backup) + } + } + }) + } +} + +func TestPollPBSBackups_PreservesCacheOnTransientDatastoreError(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + status int + body string + }{ + {"server error", http.StatusInternalServerError, "temporary server issue"}, + {"gateway quoting forbidden", http.StatusBadGateway, "upstream API error 403: permission denied"}, + {"unavailable quoting missing datastore", http.StatusServiceUnavailable, "upstream API error 404: datastore does not exist"}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/admin/datastore/archive/groups") { + http.Error(w, tc.body, tc.status) + return + } + http.NotFound(w, r) + })) + defer server.Close() + + client, err := pbs.NewClient(pbs.ClientConfig{ + Host: server.URL, + TokenName: "root@pam!token", + TokenValue: "secret", + }) + if err != nil { + t.Fatalf("failed to create PBS client: %v", err) + } + + m := &Monitor{state: models.NewState()} + original := models.PBSBackup{ + ID: "pbs-pbs1-archive--vm-100-1700000000", + Instance: "pbs1", + Datastore: "archive", + Namespace: "", + BackupType: "vm", + VMID: "100", + BackupTime: time.Unix(1700000000, 0), + } + m.state.UpdatePBSBackups("pbs1", []models.PBSBackup{original}) + + m.pollPBSBackups(context.Background(), "pbs1", client, []models.PBSDatastore{ + {Name: "archive"}, + }) + + snapshot := m.state.GetSnapshot() + var found bool + for _, backup := range snapshot.PBSBackups { + if backup.Instance == "pbs1" && backup.ID == original.ID { + found = true + break + } + } + if !found { + t.Fatal("expected cached backup to be preserved on transient error") + } + }) + } +} diff --git a/internal/monitoring/pbs_backup_cache_terminal_test.go b/internal/monitoring/pbs_backup_cache_terminal_test.go deleted file mode 100644 index 8b01baca6..000000000 --- a/internal/monitoring/pbs_backup_cache_terminal_test.go +++ /dev/null @@ -1,109 +0,0 @@ -package monitoring - -import ( - "context" - "net/http" - "net/http/httptest" - "strings" - "testing" - "time" - - "github.com/rcourtman/pulse-go-rewrite/internal/models" - "github.com/rcourtman/pulse-go-rewrite/pkg/pbs" -) - -func TestPollPBSBackups_DropsStaleCacheOnTerminalDatastoreError(t *testing.T) { - t.Parallel() - - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if strings.Contains(r.URL.Path, "/admin/datastore/archive/groups") { - http.Error(w, `{"errors":"datastore does not exist"}`, http.StatusNotFound) - return - } - http.NotFound(w, r) - })) - defer server.Close() - - client, err := pbs.NewClient(pbs.ClientConfig{ - Host: server.URL, - TokenName: "root@pam!token", - TokenValue: "secret", - }) - if err != nil { - t.Fatalf("failed to create PBS client: %v", err) - } - - m := &Monitor{state: models.NewState()} - m.state.UpdatePBSBackups("pbs1", []models.PBSBackup{ - { - ID: "pbs-pbs1-archive--vm-100-1700000000", - Instance: "pbs1", - Datastore: "archive", - Namespace: "", - BackupType: "vm", - VMID: "100", - BackupTime: time.Unix(1700000000, 0), - }, - }) - - m.pollPBSBackups(context.Background(), "pbs1", client, []models.PBSDatastore{ - {Name: "archive"}, - }) - - snapshot := m.state.GetSnapshot() - for _, backup := range snapshot.PBSBackups { - if backup.Instance == "pbs1" { - t.Fatalf("expected stale backups to be removed after terminal error, found: %+v", backup) - } - } -} - -func TestPollPBSBackups_PreservesCacheOnTransientDatastoreError(t *testing.T) { - t.Parallel() - - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if strings.Contains(r.URL.Path, "/admin/datastore/archive/groups") { - http.Error(w, `{"errors":"temporary server issue"}`, http.StatusInternalServerError) - return - } - http.NotFound(w, r) - })) - defer server.Close() - - client, err := pbs.NewClient(pbs.ClientConfig{ - Host: server.URL, - TokenName: "root@pam!token", - TokenValue: "secret", - }) - if err != nil { - t.Fatalf("failed to create PBS client: %v", err) - } - - m := &Monitor{state: models.NewState()} - original := models.PBSBackup{ - ID: "pbs-pbs1-archive--vm-100-1700000000", - Instance: "pbs1", - Datastore: "archive", - Namespace: "", - BackupType: "vm", - VMID: "100", - BackupTime: time.Unix(1700000000, 0), - } - m.state.UpdatePBSBackups("pbs1", []models.PBSBackup{original}) - - m.pollPBSBackups(context.Background(), "pbs1", client, []models.PBSDatastore{ - {Name: "archive"}, - }) - - snapshot := m.state.GetSnapshot() - var found bool - for _, backup := range snapshot.PBSBackups { - if backup.Instance == "pbs1" && backup.ID == original.ID { - found = true - break - } - } - if !found { - t.Fatal("expected cached backup to be preserved on transient error") - } -} diff --git a/pkg/pbs/client.go b/pkg/pbs/client.go index 6e1844465..b45f705b3 100644 --- a/pkg/pbs/client.go +++ b/pkg/pbs/client.go @@ -331,7 +331,9 @@ func (e *apiHTTPError) Error() string { return message } -func pbsHTTPStatus(err error) (int, bool) { +// HTTPStatus returns the response status from a PBS API or authentication error, +// including wrapped errors. The response body is never used for classification. +func HTTPStatus(err error) (int, bool) { var apiErr *apiHTTPError if errors.As(err, &apiErr) { return apiErr.status, true @@ -346,12 +348,12 @@ func pbsHTTPStatus(err error) (int, bool) { } func isPBSPermissionError(err error) bool { - status, ok := pbsHTTPStatus(err) + status, ok := HTTPStatus(err) return ok && (status == http.StatusUnauthorized || status == http.StatusForbidden) } func isPBSNotFoundError(err error) bool { - status, ok := pbsHTTPStatus(err) + status, ok := HTTPStatus(err) return ok && status == http.StatusNotFound } diff --git a/pkg/pbs/client_http_test.go b/pkg/pbs/client_http_test.go index f313c9b7d..9225dd118 100644 --- a/pkg/pbs/client_http_test.go +++ b/pkg/pbs/client_http_test.go @@ -580,7 +580,7 @@ func TestClient_GetNodeName_SuperuserPermissionFailureRetries(t *testing.T) { if firstErr == nil { t.Fatal("first GetNodeName: expected error") } - if got, ok := pbsHTTPStatus(firstErr); !ok || got != status { + if got, ok := HTTPStatus(firstErr); !ok || got != status { t.Fatalf("first GetNodeName status = (%d, %v), want (%d, true): %v", got, ok, status, firstErr) } name, err := client.GetNodeName(context.Background()) @@ -632,7 +632,7 @@ func TestClient_GetNodeName_TransientHTTPFailuresRetryAndRecover(t *testing.T) { if firstErr == nil { t.Fatal("first GetNodeName: expected error") } - if got, ok := pbsHTTPStatus(firstErr); !ok || got != tc.status { + if got, ok := HTTPStatus(firstErr); !ok || got != tc.status { t.Fatalf("first GetNodeName status = (%d, %v), want (%d, true): %v", got, ok, tc.status, firstErr) } name, err := client.GetNodeName(context.Background()) @@ -795,7 +795,7 @@ func TestClient_GetNodeName_ConcurrentTransientFailureIsSingleFlight(t *testing. if err == nil { t.Fatal("concurrent GetNodeName: expected transient error") } - if got, ok := pbsHTTPStatus(err); !ok || got != http.StatusServiceUnavailable { + if got, ok := HTTPStatus(err); !ok || got != http.StatusServiceUnavailable { t.Fatalf("concurrent GetNodeName status = (%d, %v), want (503, true): %v", got, ok, err) } } @@ -856,7 +856,7 @@ func TestClient_GetNodeStatus_PermissionOutageRecovery(t *testing.T) { if status != nil || err == nil { t.Fatalf("outage %d = (%+v, %v), want nil status and error", code, status, err) } - if got, ok := pbsHTTPStatus(err); !ok || got != code { + if got, ok := HTTPStatus(err); !ok || got != code { t.Fatalf("outage status = (%d, %v), want %d", got, ok, code) } }