Clear stale storage alerts on inventory changes

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.

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

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)

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