diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index 17ac19400..1549f436a 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -1390,6 +1390,16 @@ health checks, and storage offline lifecycle handling; future storage alert behavior should extend that resource checker owner while shared storage-health assessment helpers remain package-level until host and storage health paths are separated cleanly. +Storage connectivity status is trimmed and case-normalised before evaluation. +Empty or `unknown` status is absent connectivity evidence: it must not advance +recovery or replace an existing incident's identity/start time. Capacity and +pool-health evaluation remain independent and may still use valid observations. +The existing non-failing treatment of available, online, active, inactive and +disabled statuses remains unchanged; explicit connectivity-disable policy still +clears the incident. `TestStorageUnknownConnectivityDoesNotRecover` and +`TestStorageKnownConnectivityRecoveryCompatibility` in +`internal/alerts/alerts_test.go` pin unknown-observation preservation, independent +capacity activation, normalised offline activation and confirmed recovery. Proxmox node alert evaluation now lives in `internal/alerts/node.go`. That file owns node metric and temperature projection, node offline lifecycle handling, host-agent deduplication bookkeeping, and instance-scoped node display-name diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go index 1718ece0c..8e1bc091e 100644 --- a/internal/alerts/alerts_test.go +++ b/internal/alerts/alerts_test.go @@ -21041,3 +21041,58 @@ func TestStorageEmptyCapacityRecovery(t *testing.T) { }) } } + +func TestStorageUnknownConnectivityDoesNotRecover(t *testing.T) { + for _, status := range []string{"", "unknown", " UNKNOWN "} { + t.Run(status, func(t *testing.T) { + m := newTestManager(t) + disableTestTimeThresholds(m) + s := models.Storage{ID: "storage-observation", Name: "backups", Status: "unavailable"} + id := canonicalConnectivityStateID(s.ID) + for range 3 { + m.CheckStorage(s) + } + original := *testRequireActiveAlert(t, m, id) + s.Status = status + s.Total, s.Used, s.Free, s.Usage = 1000, 990, 10, 99 + for range 5 { + m.CheckStorage(s) + } + active := testRequireActiveAlert(t, m, id) + if !active.StartTime.Equal(original.StartTime) || m.GetResolvedAlert(id) != nil { + t.Fatal("unknown connectivity changed the incident") + } + testRequireActiveAlert(t, m, canonicalMetricStateID(s.ID, "usage")) + s.Status = "available" + for range offlineRecoveryConfirmationsStorage { + m.CheckStorage(s) + } + if testHasActiveAlert(t, m, id) || m.GetResolvedAlert(id) == nil { + t.Fatal("confirmed available storage did not recover") + } + }) + } +} + +func TestStorageKnownConnectivityRecoveryCompatibility(t *testing.T) { + // Inactive shared storage and disabled storage are intentionally not + // connectivity failures; retain the existing treatment of these statuses. + for _, status := range []string{"available", "online", "active", "inactive", "disabled"} { + t.Run(status, func(t *testing.T) { + m := newTestManager(t) + s := models.Storage{ID: "storage-known", Name: "backups", Status: " OFFLINE "} + id := canonicalConnectivityStateID(s.ID) + for range 3 { + m.CheckStorage(s) + } + testRequireActiveAlert(t, m, id) + s.Status = status + for range offlineRecoveryConfirmationsStorage { + m.CheckStorage(s) + } + if testHasActiveAlert(t, m, id) || m.GetResolvedAlert(id) == nil { + t.Fatal("known non-failing status did not recover") + } + }) + } +} diff --git a/internal/alerts/storage.go b/internal/alerts/storage.go index 281d4f885..29ef3bf09 100644 --- a/internal/alerts/storage.go +++ b/internal/alerts/storage.go @@ -73,6 +73,7 @@ func (m *Manager) CheckStorageWithCapacityTrend(storage models.Storage, trend Ca // Check if storage is truly offline/unavailable (not just inactive from other nodes) // Note: In a cluster, local storage from other nodes shows as inactive which is normal + connectivityStatus := strings.ToLower(strings.TrimSpace(storage.Status)) if thresholds.DisableConnectivity { m.mu.Lock() for _, resourceID := range resourceIDs { @@ -82,12 +83,14 @@ func (m *Manager) CheckStorageWithCapacityTrend(storage models.Storage, trend Ca for _, resourceID := range resourceIDs { m.clearAlert(canonicalConnectivityStateID(resourceID)) } - } else if storage.Status == "offline" || storage.Status == "unavailable" { + } else if connectivityStatus == "offline" || connectivityStatus == "unavailable" { m.checkStorageOffline(storage) - } else { + } else if connectivityStatus != "" && connectivityStatus != "unknown" { // Clear any existing offline alert if storage is back online m.clearStorageOfflineAlert(storage) } + // Missing connectivity evidence must not count as a healthy observation. + // Capacity and pool health below remain independently observable. // Check usage if storage has valid data (even if not currently active on this node) // In clusters, storage may show as inactive on nodes where it's not currently mounted