diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index 05087d4..2397890 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -8245,12 +8245,14 @@ func (m *Monitor) GetConfigPersistence() *config.ConfigPersistence { func (m *Monitor) pollStorageBackupsWithNodes(ctx context.Context, instanceName string, client PVEClientInterface, nodes []proxmox.Node, nodeEffectiveStatus map[string]string) { var allBackups []models.StorageBackup - seenVolids := make(map[string]bool) // Track seen volume IDs to avoid duplicates - hadSuccessfulNode := false // Track if at least one node responded successfully - storagesWithBackup := 0 // Number of storages that should contain backups - contentSuccess := 0 // Number of successful storage content fetches - contentFailures := 0 // Number of failed storage content fetches - storageQueryErrors := 0 // Number of nodes where storage list could not be queried + seenVolids := make(map[string]bool) // Track seen volume IDs to avoid duplicates + hadSuccessfulNode := false // Track if at least one node responded successfully + storagesWithBackup := 0 // Number of storages that should contain backups + contentSuccess := 0 // Number of successful storage content fetches + contentFailures := 0 // Number of failed storage content fetches + storageQueryErrors := 0 // Number of nodes where storage list could not be queried + storagePreserveNeeded := map[string]struct{}{} + storageSuccess := map[string]struct{}{} // Build guest lookup map to find actual node for each VMID snapshot := m.state.GetSnapshot() @@ -8269,6 +8271,9 @@ func (m *Monitor) pollStorageBackupsWithNodes(ctx context.Context, instanceName // For each node, get storage and check content for _, node := range nodes { if nodeEffectiveStatus[node.Node] != "online" { + for _, storageName := range storageNamesForNode(instanceName, node.Node, snapshot) { + storagePreserveNeeded[storageName] = struct{}{} + } continue } @@ -8301,6 +8306,9 @@ func (m *Monitor) pollStorageBackupsWithNodes(ctx context.Context, instanceName if err != nil { monErr := errors.NewMonitorError(errors.ErrorTypeAPI, "get_storage_for_backups", instanceName, err).WithNode(node.Node) log.Warn().Err(monErr).Str("node", node.Node).Msg("Failed to get storage for backups - skipping node") + for _, storageName := range storageNamesForNode(instanceName, node.Node, snapshot) { + storagePreserveNeeded[storageName] = struct{}{} + } storageQueryErrors++ continue } @@ -8324,11 +8332,16 @@ func (m *Monitor) pollStorageBackupsWithNodes(ctx context.Context, instanceName Str("node", node.Node). Str("storage", storage.Storage). Msg("Failed to get storage content") + if _, ok := storageSuccess[storage.Storage]; !ok { + storagePreserveNeeded[storage.Storage] = struct{}{} + } contentFailures++ continue } contentSuccess++ + storageSuccess[storage.Storage] = struct{}{} + delete(storagePreserveNeeded, storage.Storage) // Convert to models for _, content := range contents { @@ -8411,6 +8424,14 @@ func (m *Monitor) pollStorageBackupsWithNodes(ctx context.Context, instanceName } } + allBackups, preservedStorages := preserveFailedStorageBackups(instanceName, snapshot, storagePreserveNeeded, allBackups) + if len(preservedStorages) > 0 { + log.Warn(). + Str("instance", instanceName). + Strs("storages", preservedStorages). + Msg("Preserving previous storage backup data due to partial failures") + } + // Decide whether to keep existing backups when every query failed if shouldPreserveBackups(len(nodes), hadSuccessfulNode, storagesWithBackup, contentSuccess) { if len(nodes) > 0 && !hadSuccessfulNode { @@ -8474,6 +8495,75 @@ func shouldPreservePBSBackups(datastoreCount, datastoreFetches int) bool { return false } +func storageNamesForNode(instanceName, nodeName string, snapshot models.StateSnapshot) []string { + if nodeName == "" { + return nil + } + + var storages []string + for _, storage := range snapshot.Storage { + if storage.Instance != instanceName { + continue + } + if storage.Name == "" { + continue + } + if !strings.Contains(storage.Content, "backup") { + continue + } + if storage.Node == nodeName { + storages = append(storages, storage.Name) + continue + } + for _, node := range storage.Nodes { + if node == nodeName { + storages = append(storages, storage.Name) + break + } + } + } + + return storages +} + +func preserveFailedStorageBackups(instanceName string, snapshot models.StateSnapshot, storagesToPreserve map[string]struct{}, current []models.StorageBackup) ([]models.StorageBackup, []string) { + if len(storagesToPreserve) == 0 { + return current, nil + } + + existing := make(map[string]struct{}, len(current)) + for _, backup := range current { + existing[backup.ID] = struct{}{} + } + + preserved := make(map[string]struct{}) + for _, backup := range snapshot.PVEBackups.StorageBackups { + if backup.Instance != instanceName { + continue + } + if _, ok := storagesToPreserve[backup.Storage]; !ok { + continue + } + if _, duplicate := existing[backup.ID]; duplicate { + continue + } + current = append(current, backup) + existing[backup.ID] = struct{}{} + preserved[backup.Storage] = struct{}{} + } + + if len(preserved) == 0 { + return current, nil + } + + storages := make([]string, 0, len(preserved)) + for storage := range preserved { + storages = append(storages, storage) + } + sort.Strings(storages) + return current, storages +} + func buildGuestLookups(snapshot models.StateSnapshot, metadataStore *config.GuestMetadataStore) (map[string]alerts.GuestLookup, map[string][]alerts.GuestLookup) { byKey := make(map[string]alerts.GuestLookup) byVMID := make(map[string][]alerts.GuestLookup) diff --git a/internal/monitoring/storage_backup_preserve_test.go b/internal/monitoring/storage_backup_preserve_test.go new file mode 100644 index 0000000..85cd66d --- /dev/null +++ b/internal/monitoring/storage_backup_preserve_test.go @@ -0,0 +1,129 @@ +package monitoring + +import ( + "slices" + "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/models" +) + +func TestPreserveFailedStorageBackups(t *testing.T) { + instance := "pve01" + current := []models.StorageBackup{ + { + ID: "pve01-volid-new", + Instance: instance, + Storage: "local-lvm", + Volid: "local-lvm:backup/new.vma.zst", + }, + } + snapshot := models.StateSnapshot{ + PVEBackups: models.PVEBackups{ + StorageBackups: []models.StorageBackup{ + { + ID: "pve01-volid-old", + Instance: instance, + Storage: "nas-share", + Volid: "nas-share:backup/old.vma.zst", + }, + { + ID: "pve01-volid-new", + Instance: instance, + Storage: "local-lvm", + Volid: "local-lvm:backup/new.vma.zst", + }, + }, + }, + } + toPreserve := map[string]struct{}{ + "nas-share": {}, + } + + merged, storages := preserveFailedStorageBackups(instance, snapshot, toPreserve, current) + + if len(merged) != 2 { + t.Fatalf("expected 2 backups after merge, got %d", len(merged)) + } + + if !slices.Contains(storages, "nas-share") { + t.Fatalf("expected nas-share to be reported as preserved, got %v", storages) + } + + if !slices.ContainsFunc(merged, func(b models.StorageBackup) bool { + return b.Storage == "nas-share" + }) { + t.Fatalf("expected preserved backup for nas-share to be present") + } +} + +func TestPreserveFailedStorageBackupsSkipsDuplicates(t *testing.T) { + instance := "pve01" + current := []models.StorageBackup{ + { + ID: "pve01-volid-old", + Instance: instance, + Storage: "nas-share", + Volid: "nas-share:backup/old.vma.zst", + }, + } + snapshot := models.StateSnapshot{ + PVEBackups: models.PVEBackups{ + StorageBackups: []models.StorageBackup{ + { + ID: "pve01-volid-old", + Instance: instance, + Storage: "nas-share", + Volid: "nas-share:backup/old.vma.zst", + }, + }, + }, + } + toPreserve := map[string]struct{}{ + "nas-share": {}, + } + + merged, storages := preserveFailedStorageBackups(instance, snapshot, toPreserve, current) + + if len(merged) != 1 { + t.Fatalf("expected duplicate backup to be ignored, got %d entries", len(merged)) + } + if len(storages) != 0 { + t.Fatalf("expected no storages to be reported because nothing new was preserved, got %v", storages) + } +} + +func TestStorageNamesForNode(t *testing.T) { + instance := "pve01" + snapshot := models.StateSnapshot{ + Storage: []models.Storage{ + { + Name: "local-zfs", + Instance: instance, + Node: "pve-node1", + Content: "images,rootdir,backup", + }, + { + Name: "nas-share", + Instance: instance, + Nodes: []string{"pve-node2", "pve-node3"}, + Content: "backup,iso", + }, + { + Name: "other-instance", + Instance: "pve02", + Node: "pve-node1", + Content: "backup", + }, + }, + } + + found := storageNamesForNode(instance, "pve-node2", snapshot) + if !slices.Equal(found, []string{"nas-share"}) { + t.Fatalf("unexpected storages for node2: %v", found) + } + + found = storageNamesForNode(instance, "pve-node1", snapshot) + if !slices.Equal(found, []string{"local-zfs"}) { + t.Fatalf("unexpected storages for node1: %v", found) + } +}