diff --git a/frontend-modern/src/pages/Alerts.tsx b/frontend-modern/src/pages/Alerts.tsx index 67b573da8..b6455e862 100644 --- a/frontend-modern/src/pages/Alerts.tsx +++ b/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, + storage: Storage[] = [], +): Record => { + const normalized: Record = {}; + const sharedStorageLegacyKeyMap = new Map(); + + 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 >({}); // 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({ 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) { diff --git a/frontend-modern/src/pages/__tests__/Alerts.helpers.test.ts b/frontend-modern/src/pages/__tests__/Alerts.helpers.test.ts index 1298e5244..a518eb022 100644 --- a/frontend-modern/src/pages/__tests__/Alerts.helpers.test.ts +++ b/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 }, + }, + }); + }); }); diff --git a/internal/alerts/alerts.go b/internal/alerts/alerts.go index 8ac73e284..c60e987d0 100644 --- a/internal/alerts/alerts.go +++ b/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) diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go index cf5d3c2b6..f5223a387 100644 --- a/internal/alerts/alerts_test.go +++ b/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) diff --git a/internal/alerts/threshold_update_test.go b/internal/alerts/threshold_update_test.go index 978ec52b2..8fdc29f8a 100644 --- a/internal/alerts/threshold_update_test.go +++ b/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.