From fcba71018364f10116bf4bba56796119c2cd0562 Mon Sep 17 00:00:00 2001
From: rcourtman <courtmanr@gmail.com>
Date: Wed, 5 Nov 2025 19:26:20 +0000
Subject: [PATCH] Guard PBS backups from failed polls

Related to #613

When all PBS datastore queries fail (e.g., due to network issues or PBS
downtime), the system was clearing all backups and showing an empty list.
This adds the same preservation logic that exists for PVE storage backups.

Changes:
- Add shouldPreservePBSBackups() helper function
- Track datastore query success/failure counts in pollPBSBackups()
- Preserve existing backups when all datastore queries fail
- Add comprehensive unit tests for PBS backup preservation logic

This ensures users can still see their backup history even during
temporary connectivity issues with PBS, matching the behavior already
implemented for PVE storage backups.
---
 internal/monitoring/backup_guard_test.go | 47 ++++++++++++++++++++++++
 internal/monitoring/monitor.go           | 24 ++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/internal/monitoring/backup_guard_test.go b/internal/monitoring/backup_guard_test.go
index 76516cf..40f7d9a 100644
--- a/internal/monitoring/backup_guard_test.go
+++ b/internal/monitoring/backup_guard_test.go
@@ -66,3 +66,50 @@ func TestShouldPreserveBackups(t *testing.T) {
 		})
 	}
 }
+
+func TestShouldPreservePBSBackups(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name             string
+		datastoreCount   int
+		datastoreFetches int
+		want             bool
+	}{
+		{
+			name:             "all datastores failed",
+			datastoreCount:   3,
+			datastoreFetches: 0,
+			want:             true,
+		},
+		{
+			name:             "no datastores skips preservation",
+			datastoreCount:   0,
+			datastoreFetches: 0,
+			want:             false,
+		},
+		{
+			name:             "some datastores succeeded",
+			datastoreCount:   3,
+			datastoreFetches: 2,
+			want:             false,
+		},
+		{
+			name:             "all datastores succeeded",
+			datastoreCount:   3,
+			datastoreFetches: 3,
+			want:             false,
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got := shouldPreservePBSBackups(tt.datastoreCount, tt.datastoreFetches)
+			if got != tt.want {
+				t.Fatalf("shouldPreservePBSBackups() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go
index b5b00bd..5de39dc 100644
--- a/internal/monitoring/monitor.go
+++ b/internal/monitoring/monitor.go
@@ -7928,6 +7928,14 @@ func shouldPreserveBackups(nodeCount int, hadSuccessfulNode bool, storagesWithBa
 	return false
 }
 
+func shouldPreservePBSBackups(datastoreCount, datastoreFetches int) bool {
+	// If there are datastores but all fetches failed, preserve existing backups
+	if datastoreCount > 0 && datastoreFetches == 0 {
+		return true
+	}
+	return false
+}
+
 func buildGuestLookups(snapshot models.StateSnapshot) (map[string]alerts.GuestLookup, map[string]alerts.GuestLookup) {
 	byKey := make(map[string]alerts.GuestLookup)
 	byVMID := make(map[string]alerts.GuestLookup)
@@ -8507,6 +8515,9 @@ func (m *Monitor) pollPBSBackups(ctx context.Context, instanceName string, clien
 	log.Debug().Str("instance", instanceName).Msg("Polling PBS backups")
 
 	var allBackups []models.PBSBackup
+	datastoreCount := len(datastores)       // Number of datastores to query
+	datastoreFetches := 0                   // Number of successful datastore fetches
+	datastoreErrors := 0                    // Number of failed datastore fetches
 
 	// Process each datastore
 	for _, ds := range datastores {
@@ -8530,9 +8541,12 @@ func (m *Monitor) pollPBSBackups(ctx context.Context, instanceName string, clien
 				Str("instance", instanceName).
 				Str("datastore", ds.Name).
 				Msg("Failed to fetch PBS backups")
+			datastoreErrors++
 			continue
 		}
 
+		datastoreFetches++
+
 		// Convert PBS backups to model backups
 		for namespace, snapshots := range backupsMap {
 			for _, snapshot := range snapshots {
@@ -8604,6 +8618,16 @@ func (m *Monitor) pollPBSBackups(ctx context.Context, instanceName string, clien
 		Int("count", len(allBackups)).
 		Msg("PBS backups fetched")
 
+	// Decide whether to keep existing backups when all queries failed
+	if shouldPreservePBSBackups(datastoreCount, datastoreFetches) {
+		log.Warn().
+			Str("instance", instanceName).
+			Int("datastores", datastoreCount).
+			Int("errors", datastoreErrors).
+			Msg("All PBS datastore queries failed; keeping previous backup list")
+		return
+	}
+
 	// Update state
 	m.state.UpdatePBSBackups(instanceName, allBackups)