diff --git a/internal/alerts/alerts.go b/internal/alerts/alerts.go index c02f4ba7f..ac68416da 100644 --- a/internal/alerts/alerts.go +++ b/internal/alerts/alerts.go @@ -5329,6 +5329,79 @@ func (m *Manager) CheckStorage(storage models.Storage) { } } +// SyncStorageAlertsForInstance clears storage-related alerts that no longer map +// to the current storage inventory for an instance. This handles cases where a +// storage disappears or changes identity between polls; relying on the generic +// 24h stale-alert cleanup keeps the alert visible far too long. +func (m *Manager) SyncStorageAlertsForInstance(instanceName string, storages []models.Storage) { + instanceName = strings.TrimSpace(instanceName) + if instanceName == "" { + return + } + + validAlertIDs := make(map[string]struct{}, len(storages)*4) + for _, storage := range storages { + if strings.TrimSpace(storage.Instance) != instanceName { + continue + } + + validAlertIDs[fmt.Sprintf("%s-usage", storage.ID)] = struct{}{} + validAlertIDs[fmt.Sprintf("storage-offline-%s", storage.ID)] = struct{}{} + + if storage.ZFSPool == nil { + continue + } + + validAlertIDs[fmt.Sprintf("zfs-pool-state-%s", storage.ID)] = struct{}{} + validAlertIDs[fmt.Sprintf("zfs-pool-errors-%s", storage.ID)] = struct{}{} + for _, device := range storage.ZFSPool.Devices { + validAlertIDs[fmt.Sprintf("zfs-device-%s-%s", storage.ID, device.Name)] = struct{}{} + } + } + + m.mu.Lock() + defer m.mu.Unlock() + + for alertID, alert := range m.activeAlerts { + if alert == nil { + continue + } + if strings.TrimSpace(alert.Instance) != instanceName { + continue + } + if !isStorageInventoryAlert(alertID, alert) { + continue + } + if _, exists := validAlertIDs[alertID]; exists { + continue + } + m.clearAlertNoLock(alertID) + } +} + +func isStorageInventoryAlert(alertID string, alert *Alert) bool { + if alert == nil { + return false + } + + switch alert.Type { + case "usage", "zfs-pool-state", "zfs-pool-errors", "zfs-device": + return true + } + + if strings.HasPrefix(alertID, "storage-offline-") { + return true + } + + if alert.Metadata != nil { + if resourceType, ok := alert.Metadata["resourceType"].(string); ok && strings.EqualFold(strings.TrimSpace(resourceType), "storage") { + return true + } + } + + return false +} + // BuildGuestKey constructs a unique key for a guest from instance, node, and VMID. // Uses the canonical format: instance:node:vmid // This matches the format used by makeGuestID in the monitoring package. diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go index 5a19711bf..5efa81205 100644 --- a/internal/alerts/alerts_test.go +++ b/internal/alerts/alerts_test.go @@ -16060,6 +16060,124 @@ func TestCheckStorageComprehensive(t *testing.T) { }) } +func TestSyncStorageAlertsForInstance(t *testing.T) { + t.Run("clears missing storage alerts while preserving other alert types", func(t *testing.T) { + m := newTestManager(t) + + m.mu.Lock() + m.activeAlerts["inst1-node1-old-usage"] = &Alert{ + ID: "inst1-node1-old-usage", + Type: "usage", + ResourceID: "inst1-node1-old", + Instance: "inst1", + Metadata: map[string]interface{}{ + "resourceType": "Storage", + }, + } + m.activeAlerts["storage-offline-inst1-node1-old"] = &Alert{ + ID: "storage-offline-inst1-node1-old", + Type: "connectivity", + ResourceID: "inst1-node1-old", + Instance: "inst1", + } + m.activeAlerts["zfs-device-inst1-node1-old-sda"] = &Alert{ + ID: "zfs-device-inst1-node1-old-sda", + Type: "zfs-device", + ResourceID: "inst1-node1-old", + Instance: "inst1", + } + m.activeAlerts["inst1:vm:100-cpu"] = &Alert{ + ID: "inst1:vm:100-cpu", + Type: "cpu", + ResourceID: "inst1:vm:100", + Instance: "inst1", + Metadata: map[string]interface{}{ + "resourceType": "VM", + }, + } + m.mu.Unlock() + + m.SyncStorageAlertsForInstance("inst1", []models.Storage{ + { + ID: "inst1-node1-new", + Name: "new-storage", + Instance: "inst1", + }, + }) + + m.mu.RLock() + defer m.mu.RUnlock() + + if _, exists := m.activeAlerts["inst1-node1-old-usage"]; exists { + t.Fatal("expected stale storage usage alert to be cleared") + } + if _, exists := m.activeAlerts["storage-offline-inst1-node1-old"]; exists { + t.Fatal("expected stale storage offline alert to be cleared") + } + if _, exists := m.activeAlerts["zfs-device-inst1-node1-old-sda"]; exists { + t.Fatal("expected stale zfs device alert to be cleared") + } + if _, exists := m.activeAlerts["inst1:vm:100-cpu"]; !exists { + t.Fatal("expected non-storage alert to be preserved") + } + }) + + t.Run("preserves current storage and zfs device alerts", func(t *testing.T) { + m := newTestManager(t) + + m.mu.Lock() + m.activeAlerts["inst1-node1-rpool-usage"] = &Alert{ + ID: "inst1-node1-rpool-usage", + Type: "usage", + ResourceID: "inst1-node1-rpool", + Instance: "inst1", + Metadata: map[string]interface{}{ + "resourceType": "Storage", + }, + } + m.activeAlerts["storage-offline-inst1-node1-rpool"] = &Alert{ + ID: "storage-offline-inst1-node1-rpool", + Type: "connectivity", + ResourceID: "inst1-node1-rpool", + Instance: "inst1", + } + m.activeAlerts["zfs-device-inst1-node1-rpool-sda"] = &Alert{ + ID: "zfs-device-inst1-node1-rpool-sda", + Type: "zfs-device", + ResourceID: "inst1-node1-rpool", + Instance: "inst1", + } + m.mu.Unlock() + + m.SyncStorageAlertsForInstance("inst1", []models.Storage{ + { + ID: "inst1-node1-rpool", + Name: "rpool", + Instance: "inst1", + ZFSPool: &models.ZFSPool{ + Name: "rpool", + Devices: []models.ZFSDevice{ + {Name: "sda", State: "ONLINE"}, + }, + }, + }, + }) + + m.mu.RLock() + defer m.mu.RUnlock() + + if _, exists := m.activeAlerts["inst1-node1-rpool-usage"]; !exists { + t.Fatal("expected current storage usage alert to remain active") + } + if _, exists := m.activeAlerts["storage-offline-inst1-node1-rpool"]; !exists { + t.Fatal("expected current storage offline alert to remain active") + } + if _, exists := m.activeAlerts["zfs-device-inst1-node1-rpool-sda"]; !exists { + t.Fatal("expected current zfs device alert to remain active") + } + }) +} + func TestDispatchAlert(t *testing.T) { // t.Parallel() diff --git a/internal/monitoring/monitor_polling.go b/internal/monitoring/monitor_polling.go index 124464958..79e71dc74 100644 --- a/internal/monitoring/monitor_polling.go +++ b/internal/monitoring/monitor_polling.go @@ -2011,6 +2011,9 @@ func (m *Monitor) pollStorageWithNodes(ctx context.Context, instanceName string, m.alertManager.CheckStorage(storage) } } + if m.alertManager != nil { + m.alertManager.SyncStorageAlertsForInstance(storageInstanceName, allStorage) + } // Update state with all storage m.state.UpdateStorageForInstance(storageInstanceName, allStorage) diff --git a/internal/monitoring/monitor_storage_test.go b/internal/monitoring/monitor_storage_test.go index 53fec92b9..590da1a3b 100644 --- a/internal/monitoring/monitor_storage_test.go +++ b/internal/monitoring/monitor_storage_test.go @@ -541,6 +541,98 @@ func TestPollStorageWithNodesOptimizedHydratesSharedCephStorageFromDF(t *testing } } +func TestPollStorageWithNodesOptimizedClearsStaleStorageAlertsWhenIdentityChanges(t *testing.T) { + t.Setenv("PULSE_DATA_DIR", t.TempDir()) + + monitor := &Monitor{ + state: &models.State{}, + metricsHistory: NewMetricsHistory(16, time.Hour), + alertManager: alerts.NewManager(), + } + t.Cleanup(func() { + monitor.alertManager.Stop() + }) + + cfg := monitor.alertManager.GetConfig() + cfg.MinimumDelta = 0 + if cfg.TimeThresholds == nil { + cfg.TimeThresholds = make(map[string]int) + } + cfg.TimeThresholds["storage"] = 0 + cfg.StorageDefault = alerts.HysteresisThreshold{Trigger: 80, Clear: 70} + monitor.alertManager.UpdateConfig(cfg) + + nodes := []proxmox.Node{{Node: "node1", Status: "online"}} + + oldStorage := proxmox.Storage{ + Storage: "local", + Type: "dir", + Content: "images", + Active: 1, + Enabled: 1, + Shared: 0, + Total: 1000, + Used: 900, + Available: 100, + } + oldClient := &fakeStorageClient{ + allStorage: []proxmox.Storage{oldStorage}, + storageByNode: map[string][]proxmox.Storage{ + "node1": {oldStorage}, + }, + } + + monitor.pollStorageWithNodes(context.Background(), "inst1", oldClient, nodes) + + foundOldAlert := false + for _, alert := range monitor.alertManager.GetActiveAlerts() { + if alert.ID == "inst1-node1-local-usage" { + foundOldAlert = true + break + } + } + if !foundOldAlert { + t.Fatal("expected initial storage usage alert to be active") + } + + newStorage := proxmox.Storage{ + Storage: "rootfs", + Type: "dir", + Content: "images", + Active: 1, + Enabled: 1, + Shared: 0, + Total: 1000, + Used: 200, + Available: 800, + } + newClient := &fakeStorageClient{ + allStorage: []proxmox.Storage{newStorage}, + storageByNode: map[string][]proxmox.Storage{ + "node1": {newStorage}, + }, + } + + monitor.pollStorageWithNodes(context.Background(), "inst1", newClient, nodes) + + foundOldAlert = false + foundNewAlert := false + for _, alert := range monitor.alertManager.GetActiveAlerts() { + switch alert.ID { + case "inst1-node1-local-usage": + foundOldAlert = true + case "inst1-node1-rootfs-usage": + foundNewAlert = true + } + } + if foundOldAlert { + t.Fatal("expected stale storage usage alert to be cleared after storage identity changed") + } + if foundNewAlert { + t.Fatal("expected no usage alert for replacement storage below threshold") + } +} + func TestPollStorageWithNodesOptimizedAttachesZFSPoolForDirStorageOnDatasetPath(t *testing.T) { t.Setenv("PULSE_DATA_DIR", t.TempDir())