Fix shared storage override matching (#1341)

frontend-modern/src/pages/Alerts.tsx

@@ -22,7 +22,7 @@ import { NotificationsAPI, Webhook } from '@/api/notifications';
 import { LicenseAPI, type LicenseFeatureStatus } from '@/api/license';
 import type { EmailConfig, AppriseConfig } from '@/api/notifications';
 import type { HysteresisThreshold } from '@/types/alerts';
-import type { Alert, Incident, IncidentEvent, State, VM, Container, DockerHost, DockerContainer, Host } from '@/types/api';
+import type { Alert, Incident, IncidentEvent, State, VM, Container, DockerHost, DockerContainer, Host, Storage } from '@/types/api';
 import { useNavigate, useLocation } from '@solidjs/router';
 import { useAlertsActivation } from '@/stores/alertsActivation';
 import { logger } from '@/utils/logger';
@@ -483,6 +483,51 @@ export const extractTriggerValues = (
   return result;
 };
 
+export const normalizeRawOverrideConfigKeys = (
+  rawOverrides: Record<string, RawOverrideConfig>,
+  storage: Storage[] = [],
+): Record<string, RawOverrideConfig> => {
+  const normalized: Record<string, RawOverrideConfig> = {};
+  const sharedStorageLegacyKeyMap = new Map<string, string>();
+
+  storage.forEach((entry) => {
+    if (!entry.shared || !entry.id || !entry.name) {
+      return;
+    }
+
+    (entry.nodeIds ?? []).forEach((nodeId) => {
+      const trimmedNodeID = nodeId.trim();
+      if (!trimmedNodeID) {
+        return;
+      }
+      sharedStorageLegacyKeyMap.set(`${trimmedNodeID}-${entry.name}`, entry.id);
+    });
+  });
+
+  for (const [rawKey, value] of Object.entries(rawOverrides || {})) {
+    let key = rawKey;
+
+    const diskMatch = key.match(/^(host:.+\/disk:)(.+)$/);
+    if (diskMatch) {
+      const normalizedDisk = diskMatch[2]
+        .toLowerCase()
+        .replace(/[^a-z0-9]/g, '-')
+        .replace(/-{2,}/g, '-')
+        .replace(/^-|-$/g, '') || 'unknown';
+      key = diskMatch[1] + normalizedDisk;
+    }
+
+    const sharedStorageKey = sharedStorageLegacyKeyMap.get(key);
+    if (sharedStorageKey) {
+      key = sharedStorageKey;
+    }
+
+    normalized[key] = value;
+  }
+
+  return normalized;
+};
+
 const DEFAULT_DELAY_SECONDS = 5;
 
 export function Alerts() {
@@ -642,6 +687,14 @@ export function Alerts() {
     Record<string, RawOverrideConfig>
   >({}); // Store raw config
 
+  createEffect(() => {
+    const currentRawOverrides = rawOverridesConfig();
+    const normalized = normalizeRawOverrideConfigKeys(currentRawOverrides, state.storage || []);
+    if (JSON.stringify(normalized) !== JSON.stringify(currentRawOverrides)) {
+      setRawOverridesConfig(normalized);
+    }
+  });
+
   // Email configuration state moved to parent to persist across tab changes
   const [emailConfig, setEmailConfig] = createSignal<UIEmailConfig>({
     enabled: false,
@@ -1251,21 +1304,7 @@ export function Alerts() {
       setDisableAllPMGOffline(config.disableAllPMGOffline ?? false);
       setDisableAllDockerHostsOffline(config.disableAllDockerHostsOffline ?? false);
 
-      // Clean up any host disk override keys that used old underscore sanitization.
-      // The old frontend incorrectly used '_' but the backend uses '-', so old keys
-      // were never functional for alert evaluation. Drop them silently on load.
-      const rawOverrides = config.overrides || {};
-      const cleanedOverrides: typeof rawOverrides = {};
-      for (const [key, value] of Object.entries(rawOverrides)) {
-        const diskMatch = key.match(/^(host:.+\/disk:)(.+)$/);
-        if (diskMatch) {
-          const normalized = diskMatch[2].toLowerCase().replace(/[^a-z0-9]/g, '-').replace(/-{2,}/g, '-').replace(/^-|-$/g, '') || 'unknown';
-          cleanedOverrides[diskMatch[1] + normalized] = value;
-        } else {
-          cleanedOverrides[key] = value;
-        }
-      }
-      setRawOverridesConfig(cleanedOverrides);
+      setRawOverridesConfig(normalizeRawOverrideConfigKeys(config.overrides || {}, state.storage || []));
 
       if (config.schedule) {
         if (config.schedule.quietHours) {

frontend-modern/src/pages/__tests__/Alerts.helpers.test.ts

@@ -12,11 +12,13 @@ import {
   fallbackMaxAlertsPerHour,
   extractTriggerValues,
   getTriggerValue,
+  normalizeRawOverrideConfigKeys,
   normalizeMetricDelayMap,
   pathForTab,
   tabFromPath,
 } from '../Alerts';
 import type { RawOverrideConfig } from '@/types/alerts';
+import type { Storage } from '@/types/api';
 
 describe('normalizeMetricDelayMap', () => {
   it('returns empty object when input is nullish', () => {
@@ -193,4 +195,36 @@ describe('threshold helper utilities', () => {
     expect(getTriggerValue(true)).toBe(0);
     expect(getTriggerValue(undefined)).toBe(0);
   });
+
+  it('migrates legacy shared storage override keys to the canonical cluster storage id', () => {
+    const storage: Storage[] = [{
+      id: 'Main-cluster-ceph-pool',
+      name: 'ceph-pool',
+      node: 'cluster',
+      instance: 'Main',
+      type: 'rbd',
+      status: 'available',
+      total: 100,
+      used: 20,
+      free: 80,
+      usage: 20,
+      content: 'images',
+      shared: true,
+      enabled: true,
+      active: true,
+      nodes: ['pve1', 'pve2'],
+      nodeIds: ['Main-pve1', 'Main-pve2'],
+      nodeCount: 2,
+    }];
+
+    expect(normalizeRawOverrideConfigKeys({
+      'Main-pve1-ceph-pool': {
+        usage: { trigger: 2, clear: 0 },
+      },
+    }, storage)).toEqual({
+      'Main-cluster-ceph-pool': {
+        usage: { trigger: 2, clear: 0 },
+      },
+    });
+  });
 });

internal/alerts/alerts.go

@@ -1834,7 +1834,7 @@ func (m *Manager) reevaluateActiveAlertsLocked() {
 				thresholds = m.applyThresholdOverride(thresholds, override)
 			}
 			threshold = getThresholdForMetric(thresholds, metricType)
-		} else if threshold == nil && (alert.Instance == "Storage" || strings.Contains(alert.ResourceID, ":storage/")) {
+		} else if threshold == nil && (resourceTypeMeta == "storage" || alert.Instance == "Storage" || strings.Contains(alert.ResourceID, ":storage/")) {
 			// This is a storage alert
 			// Check if all storage alerts are disabled
 			if m.config.DisableAllStorage {
@@ -5124,8 +5124,10 @@ func (m *Manager) CheckStorage(storage models.Storage) {
 		return
 	}
 
-	// Check if there's an override for this storage device
-	override, hasOverride := m.config.Overrides[storage.ID]
+	// Check if there's an override for this storage device. Shared storage used
+	// to be keyed per reporting node before #1049 switched it to a stable
+	// cluster-wide ID, so we still honor legacy per-node override keys.
+	override, hasOverride, _ := findStorageOverride(m.config.Overrides, storage)
 	threshold := m.config.StorageDefault
 
 	// Apply override if it exists for usage threshold
@@ -5204,6 +5206,68 @@ func BuildGuestKey(instance, node string, vmid int) string {
 	return fmt.Sprintf("%s:%s:%d", instance, node, vmid)
 }
 
+func storageOverrideLookupKeys(storage models.Storage) []string {
+	keys := make([]string, 0, 1+len(storage.NodeIDs)+len(storage.Nodes))
+	seen := make(map[string]struct{})
+
+	addKey := func(key string) {
+		key = strings.TrimSpace(key)
+		if key == "" {
+			return
+		}
+		if _, exists := seen[key]; exists {
+			return
+		}
+		seen[key] = struct{}{}
+		keys = append(keys, key)
+	}
+
+	addKey(storage.ID)
+
+	if !storage.Shared {
+		return keys
+	}
+
+	name := strings.TrimSpace(storage.Name)
+	if name == "" {
+		return keys
+	}
+
+	for _, nodeID := range storage.NodeIDs {
+		nodeID = strings.TrimSpace(nodeID)
+		if nodeID == "" {
+			continue
+		}
+		addKey(fmt.Sprintf("%s-%s", nodeID, name))
+	}
+
+	instance := strings.TrimSpace(storage.Instance)
+	for _, node := range storage.Nodes {
+		node = strings.TrimSpace(node)
+		if node == "" || strings.EqualFold(node, "cluster") {
+			continue
+		}
+
+		prefix := node
+		if instance != "" && instance != node {
+			prefix = fmt.Sprintf("%s-%s", instance, node)
+		}
+		addKey(fmt.Sprintf("%s-%s", prefix, name))
+	}
+
+	return keys
+}
+
+func findStorageOverride(overrides map[string]ThresholdConfig, storage models.Storage) (ThresholdConfig, bool, string) {
+	for _, key := range storageOverrideLookupKeys(storage) {
+		override, exists := overrides[key]
+		if exists {
+			return override, true, key
+		}
+	}
+	return ThresholdConfig{}, false, ""
+}
+
 // CheckSnapshotsForInstance evaluates guest snapshots for age-based alerts.
 func (m *Manager) CheckSnapshotsForInstance(instanceName string, snapshots []models.GuestSnapshot, guestNames map[string]string) {
 	m.mu.RLock()
@@ -8553,7 +8617,7 @@ func (m *Manager) checkStorageOffline(storage models.Storage) {
 	defer m.mu.Unlock()
 
 	// Check if storage offline alerts are disabled
-	if override, exists := m.config.Overrides[storage.ID]; exists && override.Disabled {
+	if override, exists, _ := findStorageOverride(m.config.Overrides, storage); exists && override.Disabled {
 		// Storage alerts are disabled, clear any existing alert and return
 		if _, alertExists := m.activeAlerts[alertID]; alertExists {
 			m.clearAlertNoLock(alertID)

internal/alerts/alerts_test.go

@@ -15764,6 +15764,42 @@ func TestCheckStorageComprehensive(t *testing.T) {
 		}
 	})
 
+	t.Run("applies legacy shared storage override threshold", func(t *testing.T) {
+		m := newTestManager(t)
+
+		m.mu.Lock()
+		m.config.TimeThreshold = 0
+		m.config.TimeThresholds = map[string]int{}
+		m.config.StorageDefault = HysteresisThreshold{Trigger: 80.0, Clear: 70.0}
+		overrideThreshold := HysteresisThreshold{Trigger: 10.0, Clear: 5.0}
+		m.config.Overrides = map[string]ThresholdConfig{
+			"Main-pve1-ceph-pool": {Usage: &overrideThreshold},
+		}
+		m.mu.Unlock()
+
+		storage := models.Storage{
+			ID:       "Main-cluster-ceph-pool",
+			Name:     "ceph-pool",
+			Node:     "cluster",
+			Instance: "Main",
+			Status:   "available",
+			Usage:    20.0,
+			Shared:   true,
+			Nodes:    []string{"pve1", "pve2"},
+			NodeIDs:  []string{"Main-pve1", "Main-pve2"},
+		}
+
+		m.CheckStorage(storage)
+
+		m.mu.RLock()
+		alert := m.activeAlerts["Main-cluster-ceph-pool-usage"]
+		m.mu.RUnlock()
+
+		if alert == nil {
+			t.Fatal("expected usage alert when legacy shared-storage override matches canonical storage ID")
+		}
+	})
+
 	t.Run("skips usage check when offline", func(t *testing.T) {
 		// t.Parallel()
 		m := newTestManager(t)

internal/alerts/threshold_update_test.go

@@ -3,6 +3,8 @@ package alerts
 import (
 	"testing"
 	"time"
+
+	"github.com/rcourtman/pulse-go-rewrite/internal/models"
 )
 
 // TestReevaluateActiveAlertsOnThresholdChange tests that active alerts are re-evaluated when thresholds change
@@ -217,6 +219,55 @@ func TestReevaluateActiveAlertsStillAboveThreshold(t *testing.T) {
 	}
 }
 
+func TestReevaluateActiveStorageAlertsOnThresholdChange(t *testing.T) {
+	manager := NewManager()
+
+	manager.mu.Lock()
+	manager.activeAlerts = make(map[string]*Alert)
+	manager.mu.Unlock()
+
+	initialConfig := AlertConfig{
+		Enabled:        true,
+		StorageDefault: HysteresisThreshold{Trigger: 85, Clear: 80},
+		Overrides:      make(map[string]ThresholdConfig),
+	}
+	manager.UpdateConfig(initialConfig)
+	manager.mu.Lock()
+	manager.config.TimeThreshold = 0
+	manager.config.TimeThresholds = map[string]int{}
+	manager.mu.Unlock()
+
+	storage := models.Storage{
+		ID:       "Main-cluster-ceph-pool",
+		Name:     "ceph-pool",
+		Node:     "cluster",
+		Instance: "Main",
+		Status:   "available",
+		Usage:    90,
+	}
+	manager.CheckStorage(storage)
+
+	manager.mu.RLock()
+	_, existsBefore := manager.activeAlerts["Main-cluster-ceph-pool-usage"]
+	manager.mu.RUnlock()
+	if !existsBefore {
+		t.Fatalf("expected storage alert to exist before threshold update")
+	}
+
+	updatedConfig := initialConfig
+	updatedConfig.StorageDefault = HysteresisThreshold{Trigger: 95, Clear: 90}
+	manager.UpdateConfig(updatedConfig)
+
+	time.Sleep(100 * time.Millisecond)
+
+	manager.mu.RLock()
+	_, existsAfter := manager.activeAlerts["Main-cluster-ceph-pool-usage"]
+	manager.mu.RUnlock()
+	if existsAfter {
+		t.Errorf("expected storage alert to be resolved after threshold increase")
+	}
+}
+
 // TestReevaluateActiveAlertsGuestNotMisclassifiedAsNode tests that guest alerts
 // are not misclassified as node alerts when Instance == Node (single-node setups).
 // This is the root cause of GitHub #1145.