diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index f1c14e989..03152b682 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -2595,3 +2595,24 @@ The hook and destinations caller regressions in `useAlertDestinationsTabState.test.tsx` pin ordering and loading ownership. `scripts/check-delivery-health-ordering.mjs` exercises the real caller and card in Chromium with scripted API completions; it is not installed delivery proof. + +### Release/v6.4: missing storage connectivity is not recovery + +On candidate `f71542ec2182a5f0fe18a4bf5e4ecc298964f1ee`, the regression +`TestStorageUnknownConnectivityDoesNotRecover` reproduces empty/unknown status +resolving an existing offline incident. This violates the truthful recovery +requirement already being repaired on this patch line; absent observation is +not evidence that storage recovered. + +Backport only the storage implementation and tests from main-work commits +`2e661e075a42e02dd833c7c29fa5741b38dc77b2` and +`9a8ee6a5a7a8bd759f2ab46d864a7b92b0bbd71b` (PR #1931). Preserve the incident +for empty/unknown connectivity while evaluating observed capacity independently. +Normalize connectivity consistently so offline storage cannot emit misleading +capacity alerts. Known inactive/disabled status keeps its existing semantics. +No TrueNAS fixtures or unrelated product work are included. + +Unit reproduction and focused race tests are source-level evidence only, not +installed-provider, restart transport or off-host recipient qualification of +this changed candidate. Any promoted RC must bind the new revision and begin +its own required soak; the former exact-candidate qualification cannot transfer. diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go index 1718ece0c..1763c3f95 100644 --- a/internal/alerts/alerts_test.go +++ b/internal/alerts/alerts_test.go @@ -21041,3 +21041,80 @@ 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") + } + }) + } +} + +func TestStorageNormalizedOfflineSkipsCapacityEvaluation(t *testing.T) { + for _, status := range []string{" OFFLINE ", " UnAvAiLaBlE "} { + t.Run(status, func(t *testing.T) { + m := newTestManager(t) + disableTestTimeThresholds(m) + m.mu.Lock() + m.config.StorageDefault = HysteresisThreshold{Trigger: 80, Clear: 70} + m.mu.Unlock() + + s := models.Storage{ID: "storage-normalized-offline", Name: "backups", Status: status, Usage: 99} + for range 3 { + m.CheckStorage(s) + } + + testRequireActiveAlert(t, m, canonicalConnectivityStateID(s.ID)) + if testHasActiveAlert(t, m, canonicalMetricStateID(s.ID, "usage")) { + t.Fatal("offline storage produced a capacity alert") + } + }) + } +} diff --git a/internal/alerts/storage.go b/internal/alerts/storage.go index 281d4f885..ccbf35a0f 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 @@ -105,7 +108,7 @@ func (m *Manager) CheckStorageWithCapacityTrend(storage models.Storage, trend Ca // confirm an empty store. A default zero from missing capacity telemetry // must not resolve an existing incident. confirmedEmpty := storage.Usage == 0 && storage.Total > 0 && storage.Used == 0 && storage.Free == storage.Total - if storage.Status != "offline" && storage.Status != "unavailable" && (storage.Usage > 0 || confirmedEmpty) { + if connectivityStatus != "offline" && connectivityStatus != "unavailable" && (storage.Usage > 0 || confirmedEmpty) { m.evaluateStorageCapacity(storage, thresholds, trend) }