From 7c8e9099a1c08d925142613807cbbd358536afb3 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Tue, 6 Oct 2026 16:39:29 +0100 Subject: [PATCH] Clear agent disk temperature alerts when the disk leaves A host-agent SMART disk temperature alert stayed active for a day once its disk left the agent's report (removed, replaced, or a renamed device path). CheckHost only evaluated disks still listed and node cleanup exempts agent: alerts, so only the 24-hour stale sweep resolved the alert or dropped a pending threshold run. CheckHost now tracks which disks a non-empty SMART list reports. Once a disk with an alert or a pending threshold run has been missing from three consecutive non-empty reports, the alert clears and the pending run is dropped. The Windows, FreeBSD and controller-multiplexed Linux collectors drop a disk whose probe fails, so a single omission does not count as the disk leaving, and the count restarts when the disk is reported again. An empty SMART list holds every alert, because the agent omits the list when collection fails or is unsupported. A listed disk in standby or without a temperature also holds its alert. Neither shows the disk cooled, and SMART health alerts already hold through standby. Turning the threshold off or removing the agent now drops pending runs and absence counts as well as alerts. Cleanup matches the disk_temp resource prefix alone, which also catches alerts raised under the original disk_temperature type. --- .../v6/internal/subsystems/alerts.md | 29 ++++ internal/alerts/host.go | 85 +++++++++- internal/alerts/host_unraid_lifecycle_test.go | 153 ++++++++++++++++++ internal/alerts/manager.go | 2 + 4 files changed, 265 insertions(+), 4 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index 1c5fa9f1ff..c0b0274c87 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -1632,6 +1632,35 @@ an alert the next evaluation raises again. `TestConfigSaveKeepsAlertTheEvaluatorKeepsWithoutRecoveryBand` in `internal/alerts/host_unraid_lifecycle_test.go` pin these paths. +### Agent disk temperature alerts clear when their disk leaves the report + +`CheckHost` also clears a disk temperature alert, and drops its pending +threshold run, once its device is missing from three consecutive non-empty +SMART lists (`cleanupHostDiskTemperatureAlerts`, a seen map keyed by resource +ID like `cleanupHostCustomSensorAlerts`). A removed, replaced or renamed disk +never sends the reading that would resolve its alert, and +`shouldPreserveAlertOutsideNodeCleanup` exempts `agent:` alerts from node +cleanup, so before this only the 24-hour stale-alert sweep in +`cleanupStaleMaps` resolved it. One omission is not proof the disk left: the +Windows, FreeBSD and controller-multiplexed Linux collectors drop a disk whose +probe fails, so the count restarts whenever the disk is reported again. Three +reports in a row of failed probes on such a disk still clear its alert. + +An empty SMART list skips this cleanup and does not count toward the three, +because the agent omits the list when SMART collection fails or is +unsupported. A listed disk in standby or without a temperature also holds its +alert, since neither shows the disk cooled: standby reports avoid waking the +disk, and most failed Linux probes still list the disk with identity only. +SMART health alerts hold through standby the same way. A held alert resolves +through the normal recovery path, including any configured recovery delay, +once the disk reports cool readings again; with no evaluation for 24 hours, +the stale-alert sweep resolves it. Turning the threshold off, or removing the +agent, drops the host's pending disk temperature runs and absence counts as +well as its alerts. +`TestCheckHostClearsDiskTemperatureAlertWhenDiskLeavesSMARTReport` in +`internal/alerts/host_unraid_lifecycle_test.go` pins the clear, the holds, the +restarted count and the pending-run and count cleanup. + ### Configured flapping thresholds remain reachable Every accepted positive `FlappingThreshold`, including values above ten, must diff --git a/internal/alerts/host.go b/internal/alerts/host.go index 5fb2692605..04ead920c3 100644 --- a/internal/alerts/host.go +++ b/internal/alerts/host.go @@ -355,15 +355,21 @@ func (m *Manager) CheckHost(host models.Host) { } if thresholds.DiskTemperature != nil && thresholds.DiskTemperature.Trigger > 0 { + // An empty SMART list means collection failed or is unsupported for + // this report, not that every disk left, so existing alerts are held. if len(host.Sensors.SMART) > 0 { + seenDiskTemps := make(map[string]struct{}, len(host.Sensors.SMART)) for _, disk := range host.Sensors.SMART { + // A listed disk in standby, or without a temperature after a + // failed probe, is still present. Its alert holds until a fresh + // reading resolves it instead of clearing and re-raising. + tempResourceID := hostDiskTemperatureResourceID(host.ID, disk.Device) + seenDiskTemps[tempResourceID] = struct{}{} if disk.Temperature > 0 && !disk.Standby { m.mu.RLock() effectiveTempThreshold := m.hostDiskTemperatureThresholdNoLock(thresholds.DiskTemperature, diskTempOverridden, disk.Type) m.mu.RUnlock() - // Use specific resource ID for the disk: hostID/disk-temp:device - tempResourceID := hostDiskTemperatureResourcePrefix(host.ID) + sanitizeHostComponent(disk.Device) tempResourceName := fmt.Sprintf("%s (%s Temp)", hostDisplayName(host), disk.Device) diskTempMetadata := cloneMetadata(baseMetadata) @@ -386,6 +392,9 @@ func (m *Manager) CheckHost(host models.Host) { m.checkMetricWithCanonicalSpec(spec, tempResourceName, nodeName, disk.Device, "agent", float64(disk.Temperature), effectiveTempThreshold, &metricOptions{Metadata: diskTempMetadata}) } } + // A disk missing from consecutive non-empty reports was removed, + // replaced or renamed, so no later reading will resolve its alert. + m.cleanupHostDiskTemperatureAlerts(host.ID, seenDiskTemps) } } else { // Disk temperature alerting is off for this host, so no later reading @@ -634,6 +643,9 @@ func (m *Manager) HandleHostRemoved(host models.Host) { m.clearHostRAIDAlerts(host.ID) m.clearHostUnraidAlerts(host.ID) m.clearHostCustomSensorAlerts(host.ID) + // No later report will close the host's pending disk temperature runs or + // reset its disk absence counts. + m.clearHostDiskTemperatureAlerts(host.ID) } // HandleHostTelemetryExpired re-evaluates transient storage-operation evidence @@ -924,6 +936,12 @@ func hostDiskTemperatureResourcePrefix(hostID string) string { return hostResourceID(hostID) + "/disk_temp:" } +// hostDiskTemperatureResourceID is the resource ID of the SMART disk +// temperature alert for one device on a host. +func hostDiskTemperatureResourceID(hostID, device string) string { + return hostDiskTemperatureResourcePrefix(hostID) + sanitizeHostComponent(device) +} + // hostDiskTemperatureThresholdNoLock returns the threshold for one SMART disk // given the host's resolved disk temperature threshold. An enabled threshold // gives way to the disk type's DiskTempByType entry unless an explicit host or @@ -961,6 +979,20 @@ func (m *Manager) lowestHostDiskTemperatureThresholdNoLock(hostThreshold *Hyster // clearHostDiskTemperatureAlerts resolves every SMART disk temperature alert // for a host. func (m *Manager) clearHostDiskTemperatureAlerts(hostID string) { + m.cleanupHostDiskTemperatureAlerts(hostID, nil) +} + +// hostDiskTemperatureAbsenceConfirmations is how many consecutive non-empty +// SMART reports must omit a disk before its temperature alert clears. The +// Windows, FreeBSD and controller-multiplexed Linux collectors drop a disk +// whose probe fails, so one omission is not evidence the disk left. +const hostDiskTemperatureAbsenceConfirmations = 3 + +// cleanupHostDiskTemperatureAlerts resolves a host's SMART disk temperature +// alerts, and drops their pending threshold runs, once their resource ID has +// been missing from seen for hostDiskTemperatureAbsenceConfirmations calls in +// a row. A nil seen resolves all of them at once. +func (m *Manager) cleanupHostDiskTemperatureAlerts(hostID string, seen map[string]struct{}) { if hostID == "" { return } @@ -970,11 +1002,56 @@ func (m *Manager) clearHostDiskTemperatureAlerts(hostID string) { m.mu.Lock() defer m.mu.Unlock() + // Absent resources map to the storage keys of their active alerts; a + // resource with only a pending run maps to none. + absent := make(map[string][]string) for storageKey, alert := range m.activeAlerts { - if alert == nil || alert.Type != "diskTemperature" { + // Only CheckHost raises alerts under this prefix. Matching on it + // alone also catches alerts raised before the type became + // diskTemperature ("disk_temperature", January 2026). + if alert == nil || !strings.HasPrefix(alert.ResourceID, resourcePrefix) { continue } - if strings.HasPrefix(alert.ResourceID, resourcePrefix) { + if _, exists := seen[alert.ResourceID]; exists { + continue + } + absent[alert.ResourceID] = append(absent[alert.ResourceID], storageKey) + } + for _, resourceID := range m.core.PendingResourceIDs() { + if !strings.HasPrefix(resourceID, resourcePrefix) { + continue + } + if _, exists := seen[resourceID]; exists { + continue + } + if _, exists := absent[resourceID]; !exists { + absent[resourceID] = nil + } + } + + if m.hostDiskTempAbsences == nil { + m.hostDiskTempAbsences = make(map[string]int) + } + // A disk seen again, or whose state is gone, restarts its count. + for resourceID := range m.hostDiskTempAbsences { + if !strings.HasPrefix(resourceID, resourcePrefix) { + continue + } + if _, exists := absent[resourceID]; !exists { + delete(m.hostDiskTempAbsences, resourceID) + } + } + for resourceID, storageKeys := range absent { + if seen != nil { + m.hostDiskTempAbsences[resourceID]++ + if m.hostDiskTempAbsences[resourceID] < hostDiskTemperatureAbsenceConfirmations { + continue + } + } + delete(m.hostDiskTempAbsences, resourceID) + // A departed disk never sends the reading that would close its run. + m.core.DropPendingForResource(resourceID) + for _, storageKey := range storageKeys { m.clearAlertNoLock(storageKey) } } diff --git a/internal/alerts/host_unraid_lifecycle_test.go b/internal/alerts/host_unraid_lifecycle_test.go index afb88c6c8c..ae5d09a3f7 100644 --- a/internal/alerts/host_unraid_lifecycle_test.go +++ b/internal/alerts/host_unraid_lifecycle_test.go @@ -294,6 +294,159 @@ func TestHostDiskTemperatureAlertsClearWhenAgentLeaves(t *testing.T) { }) } +func TestCheckHostClearsDiskTemperatureAlertWhenDiskLeavesSMARTReport(t *testing.T) { + const hostID = "host-temp-absent" + smartDisk := func(device string, temperature int) models.HostDiskSMART { + return models.HostDiskSMART{Device: device, Model: "test-disk", Type: "sata", Temperature: temperature} + } + alertIDFor := func(device string) string { + return canonicalMetricStateID(hostDiskTemperatureResourceID(hostID, device), "diskTemperature") + } + sdaActive := func(m *Manager) bool { + _, exists := testLookupActiveAlert(t, m, alertIDFor("/dev/sda")) + return exists + } + // sda runs hot (sata trigger 55, clear 50); sdb stays cool. + hotHost := func(t *testing.T) (*Manager, models.Host) { + m := configureDiskTempTypeHostManager(t) + host := hostWithSMARTDiskTemp(hostID, "sata", 60) + host.Sensors.SMART = []models.HostDiskSMART{smartDisk("/dev/sda", 60), smartDisk("/dev/sdb", 40)} + m.CheckHost(host) + if !sdaActive(m) { + t.Fatalf("expected sda disk temperature alert at 60C, active: %v", alertKeys(m)) + } + return m, host + } + + cases := []struct { + name string + next []models.HostDiskSMART + wantHeld bool + wantRaised string + }{ + {name: "disk removed", next: []models.HostDiskSMART{smartDisk("/dev/sdb", 40)}}, + {name: "device renamed", next: []models.HostDiskSMART{smartDisk("/dev/sdc", 60), smartDisk("/dev/sdb", 40)}, wantRaised: "/dev/sdc"}, + // No fresh reading says the disk cooled or left, so the alert holds. + {name: "SMART collection empty", next: nil, wantHeld: true}, + {name: "disk in standby", next: []models.HostDiskSMART{{Device: "/dev/sda", Type: "sata", Temperature: 45, Standby: true}, smartDisk("/dev/sdb", 40)}, wantHeld: true}, + {name: "probe returned no temperature", next: []models.HostDiskSMART{{Device: "/dev/sda", Type: "sata"}, smartDisk("/dev/sdb", 40)}, wantHeld: true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + m, host := hotHost(t) + + host.Sensors.SMART = tc.next + for report := 1; report <= hostDiskTemperatureAbsenceConfirmations; report++ { + m.CheckHost(host) + wantActive := tc.wantHeld || report < hostDiskTemperatureAbsenceConfirmations + if active := sdaActive(m); active != wantActive { + t.Fatalf("report %d: sda disk temperature alert active = %v, want %v; active: %v", report, active, wantActive, alertKeys(m)) + } + } + if tc.wantRaised != "" { + if _, exists := testLookupActiveAlert(t, m, alertIDFor(tc.wantRaised)); !exists { + t.Fatalf("expected %s disk temperature alert, active: %v", tc.wantRaised, alertKeys(m)) + } + } + if !tc.wantHeld { + return + } + + // A held alert still resolves on the disk's next cool reading. + host.Sensors.SMART = []models.HostDiskSMART{smartDisk("/dev/sda", 45), smartDisk("/dev/sdb", 40)} + m.CheckHost(host) + if sdaActive(m) { + t.Fatalf("held sda disk temperature alert outlived a 45C reading, active: %v", alertKeys(m)) + } + }) + } + + t.Run("intermittently dropped disk keeps its alert", func(t *testing.T) { + m, host := hotHost(t) + present := host.Sensors.SMART + dropped := []models.HostDiskSMART{smartDisk("/dev/sdb", 40)} + + // The run of omissions restarts whenever the disk is reported again. + for _, smart := range [][]models.HostDiskSMART{dropped, dropped, present, dropped, dropped} { + host.Sensors.SMART = smart + m.CheckHost(host) + if !sdaActive(m) { + t.Fatalf("sda disk temperature alert cleared by an omission run shorter than %d reports, active: %v", hostDiskTemperatureAbsenceConfirmations, alertKeys(m)) + } + } + }) + + // pendingHost leaves a hot sda inside a 300s alert delay, so sda holds a + // pending run and no alert. + pendingHost := func(t *testing.T) (*Manager, models.Host, func() bool) { + m := configureDiskTempTypeHostManager(t) + m.mu.Lock() + m.config.TimeThresholds = map[string]int{"agent": 300} + m.mu.Unlock() + sdaResourceID := hostDiskTemperatureResourceID(hostID, "/dev/sda") + sdaPending := func() bool { + m.mu.RLock() + defer m.mu.RUnlock() + for _, resourceID := range m.core.PendingResourceIDs() { + if resourceID == sdaResourceID { + return true + } + } + return false + } + + host := hostWithSMARTDiskTemp(hostID, "sata", 60) + host.Sensors.SMART = []models.HostDiskSMART{smartDisk("/dev/sda", 60), smartDisk("/dev/sdb", 40)} + m.CheckHost(host) + if sdaActive(m) || !sdaPending() { + t.Fatalf("expected a pending sda run inside the 300s delay, active: %v", alertKeys(m)) + } + return m, host, sdaPending + } + + t.Run("departed disk drops its pending run", func(t *testing.T) { + m, host, sdaPending := pendingHost(t) + + host.Sensors.SMART = []models.HostDiskSMART{smartDisk("/dev/sdb", 40)} + for report := 1; report <= hostDiskTemperatureAbsenceConfirmations; report++ { + m.CheckHost(host) + wantPending := report < hostDiskTemperatureAbsenceConfirmations + if pending := sdaPending(); pending != wantPending { + t.Fatalf("report %d: pending sda run = %v, want %v", report, pending, wantPending) + } + } + }) + + t.Run("threshold off drops pending runs", func(t *testing.T) { + m, host, sdaPending := pendingHost(t) + + m.mu.Lock() + m.config.AgentDefaults.DiskTemperature = &HysteresisThreshold{Trigger: 0, Clear: 0} + m.mu.Unlock() + m.CheckHost(host) + if sdaPending() { + t.Fatal("pending sda run survived its threshold being turned off; re-enabling would inherit its old start time") + } + }) + + t.Run("removed host leaves no pending run or absence count", func(t *testing.T) { + m, host, sdaPending := pendingHost(t) + + host.Sensors.SMART = []models.HostDiskSMART{smartDisk("/dev/sdb", 40)} + m.CheckHost(host) + m.HandleHostRemoved(host) + if sdaPending() { + t.Fatal("pending sda run outlived its removed host") + } + m.mu.RLock() + defer m.mu.RUnlock() + if len(m.hostDiskTempAbsences) != 0 { + t.Fatalf("absence counts outlived their removed host: %v", m.hostDiskTempAbsences) + } + }) +} + func TestConfigSaveResolvesDiskTemperatureAlertAgainstItsDiskTypeThreshold(t *testing.T) { m := configureDiskTempTypeHostManager(t) diff --git a/internal/alerts/manager.go b/internal/alerts/manager.go index 53aa04e84f..64b7093c4d 100644 --- a/internal/alerts/manager.go +++ b/internal/alerts/manager.go @@ -78,6 +78,7 @@ type Manager struct { unifiedIncidentConfirmations map[string]int // Track consecutive provider-incident observations before activation unifiedIncidentFirstSeen map[string]time.Time // Preserve the first confirmed observation as lifecycle start unifiedIncidentRecoveries map[string]int // Track consecutive healthy observations before provider-incident recovery + hostDiskTempAbsences map[string]int // Track consecutive SMART reports missing a disk that holds temperature alert state dockerRestartTracking map[string]*dockerRestartRecord // Track restart counts and times for restart loop detection dockerUpdateFirstSeen map[string]time.Time // Track when image updates were first detected for alert delay // Stable identity tracking prevents update-delay resets when host IDs churn. @@ -233,6 +234,7 @@ func NewManagerWithDataDir(dataDir string, options ...ManagerOption) *Manager { unifiedIncidentConfirmations: make(map[string]int), unifiedIncidentFirstSeen: make(map[string]time.Time), unifiedIncidentRecoveries: make(map[string]int), + hostDiskTempAbsences: make(map[string]int), dockerRestartTracking: make(map[string]*dockerRestartRecord), dockerUpdateFirstSeen: make(map[string]time.Time), dockerUpdateFirstSeenByIdentity: make(map[string]time.Time),