mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
fix(alerts): preserve storage incidents when connectivity is unknown
Do not count empty or unknown storage status as recovery evidence. Normalise status spelling for connectivity checks while leaving capacity evaluation independent and preserving existing inactive/disabled storage behaviour. Change-source: pulse-maintainer
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user