diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go index 8a9cae0db..6b8274794 100644 --- a/internal/alerts/alerts_test.go +++ b/internal/alerts/alerts_test.go @@ -17162,6 +17162,164 @@ 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: "offline", + ResourceID: "inst1-node1-old", + Instance: "inst1", + Metadata: map[string]interface{}{ + "resourceType": "storage", + }, + } + 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: "offline", + ResourceID: "inst1-node1-rpool", + Instance: "inst1", + Metadata: map[string]interface{}{ + "resourceType": "storage", + }, + } + 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") + } + }) + + t.Run("preserves alerts keyed under ceph alias ids", func(t *testing.T) { + m := newTestManager(t) + + m.mu.Lock() + m.activeAlerts["agent:pve1-ceph-pool-data-usage"] = &Alert{ + ID: "agent:pve1-ceph-pool-data-usage", + Type: "usage", + ResourceID: "agent:pve1-ceph-pool-data", + Instance: "pve1", + Metadata: map[string]interface{}{ + "resourceType": "Storage", + }, + } + m.mu.Unlock() + + // The current inventory lists the pool under its canonical id but + // carries the agent-sourced id as an alias. + m.SyncStorageAlertsForInstance("pve1", []models.Storage{ + { + ID: "pve1-ceph-pool-data", + Name: "ceph-pool-data", + Instance: "pve1", + AliasIDs: []string{"agent:pve1-ceph-pool-data"}, + }, + }) + + m.mu.RLock() + defer m.mu.RUnlock() + + if _, exists := m.activeAlerts["agent:pve1-ceph-pool-data-usage"]; !exists { + t.Fatal("expected alias-keyed ceph storage alert to be preserved") + } + }) +} + func TestDispatchAlert(t *testing.T) { // t.Parallel() diff --git a/internal/alerts/storage.go b/internal/alerts/storage.go index 9dbdf6223..f00d525b2 100644 --- a/internal/alerts/storage.go +++ b/internal/alerts/storage.go @@ -110,6 +110,81 @@ func (m *Manager) CheckStorage(storage models.Storage) { } } +// SyncStorageAlertsForInstance clears storage-related alerts whose underlying +// storage is no longer in the current inventory for an instance. This handles a +// storage disappearing or changing identity between polls; without it the alert +// stays visible until the generic multi-day stale-alert cleanup fires. +// +// Matching is by alert ResourceID (every v6 storage alert sets ResourceID = +// storage.ID, regardless of the composite alert-state key) so it is robust to +// the alert-key format. The sweep is conservative: it only touches alerts the +// instance owns that are recognised as storage-inventory alerts, and a live +// storage keeps its alert because its ResourceID is in the valid set. Ceph pool +// alias IDs are treated as valid so a deduped multi-source pool keeps its alert. +func (m *Manager) SyncStorageAlertsForInstance(instanceName string, storages []models.Storage) { + instanceName = strings.TrimSpace(instanceName) + if instanceName == "" { + return + } + + validResourceIDs := make(map[string]struct{}, len(storages)*2) + for _, storage := range storages { + if strings.TrimSpace(storage.Instance) != instanceName { + continue + } + if id := strings.TrimSpace(storage.ID); id != "" { + validResourceIDs[id] = struct{}{} + } + for _, aliasID := range storage.AliasIDs { + if id := strings.TrimSpace(aliasID); id != "" { + validResourceIDs[id] = 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(alert) { + continue + } + if _, exists := validResourceIDs[strings.TrimSpace(alert.ResourceID)]; exists { + continue + } + m.clearAlertNoLock(alertID) + } +} + +// isStorageInventoryAlert reports whether an alert belongs to the storage +// inventory, so it should be cleared when its storage leaves the inventory. +// Gated on the storage resourceType metadata that v6 stamps on storage alerts +// plus the ZFS pool/offline alert types; anything else is left untouched. +func isStorageInventoryAlert(alert *Alert) bool { + if alert == nil { + return false + } + + if alert.Metadata != nil { + if resourceType, ok := alert.Metadata["resourceType"].(string); ok && strings.EqualFold(strings.TrimSpace(resourceType), "storage") { + return true + } + } + + switch alert.Type { + case "storage", "usage", "zfs-pool-state", "zfs-pool-errors", "zfs-device", "offline": + return true + } + + return false +} + func (m *Manager) clearStorageAliasAlerts(storage models.Storage) { for i, resourceID := range storageAlertResourceIDs(storage) { if i == 0 { diff --git a/internal/monitoring/monitor_polling_storage.go b/internal/monitoring/monitor_polling_storage.go index 219da082c..d17b89a45 100644 --- a/internal/monitoring/monitor_polling_storage.go +++ b/internal/monitoring/monitor_polling_storage.go @@ -652,6 +652,10 @@ func (m *Monitor) pollStorageWithNodes(ctx context.Context, instanceName string, } } + if m.alertManager != nil { + m.alertManager.SyncStorageAlertsForInstance(storageInstanceName, allStorage) + } + if !cephDetected { for _, storage := range allStorage { if isCephStorageType(storage.Type) { diff --git a/internal/monitoring/monitor_storage_test.go b/internal/monitoring/monitor_storage_test.go index 1c74c996f..0d7530652 100644 --- a/internal/monitoring/monitor_storage_test.go +++ b/internal/monitoring/monitor_storage_test.go @@ -235,6 +235,73 @@ func TestPollStorageWithNodesOptimizedRecordsMetricsAndAlerts(t *testing.T) { } } +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"}} + + hasStorageAlert := func(resourceID string) bool { + for _, alert := range monitor.alertManager.GetActiveAlerts() { + if alert.ResourceID == resourceID { + return true + } + } + return false + } + + 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) + + if !hasStorageAlert("inst1-node1-local") { + t.Fatal("expected initial storage usage alert to be active") + } + + // Next poll: "local" is gone and replaced by a below-threshold "rootfs". + 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) + + if hasStorageAlert("inst1-node1-local") { + t.Fatal("expected stale storage usage alert to be cleared after storage identity changed") + } + if hasStorageAlert("inst1-node1-rootfs") { + t.Fatal("expected no usage alert for replacement storage below threshold") + } +} + func TestPollStorageWithNodesSynthesizesSharedClusterOnlyStorage(t *testing.T) { monitor := &Monitor{ state: models.NewState(),