Merge commit 'e106c04d2ed08d0846b3a5e5f91c669971583c33' into release/v6.4

Change-source: pulse-maintainer

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 CRITICAL, ALERT and EMERGENCY native levels project as canonical critical incidents. EMERGENCY must not be discarded as an unknown level: repeated observations retain the active incident rather than supplying false recovery evidence. Provider projection tests cover every documented native level and normalized input.
 
 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.

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
 	}

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")
+			}
+		})
+	}
+}

internal/monitoring/pbs_backup_cache_terminal_test.go

@@ -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")
-	}
-}

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
 }
 

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)
 		}
 	}