diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index a951911e5f..bbfbba386d 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -1592,6 +1592,41 @@ per-alert rate limiting; future notification-gating changes should extend that policy owner rather than burying new checks inside metric or resource-specific evaluators. +### Agent disk temperature alerts clear when their threshold is off + +Host-agent SMART disk temperature alerts (`diskTemperature`, resource +`agent:/disk_temp:`) follow one effective threshold: the +resolved agent `DiskTemperature`, refined by the disk type's `DiskTempByType` +entry unless an explicit host or linked-resource override set it +(`hostDiskTemperatureThresholdNoLock` in `internal/alerts/host.go`). When that +threshold is off, `CheckHost` clears the host's disk temperature alerts. The +all-agents switch, a disabled host override, agent removal and confirmed +agent offline clear them with the host's other disk alerts. Before this, +nothing cleared them once the agent-level threshold was off. + +A config save judges an open disk temperature alert against the same +threshold (`resolveHostAlertThresholdsNoLock` refines it by the alert's +`diskType` metadata, and `metric_runtime.go` classifies `diskTemperature` as a +threshold metric), so turning the threshold off resolves the alert +immediately. Every firing evaluation refreshes `diskType`, so an alert at or +above its trigger always carries it. Only an alert held in its recovery band +since before that metadata existed (May 2026) falls back to the agent +threshold. Its reading is already below its trigger, so the fallback can +resolve it early or leave it to the evaluator, but it cannot resolve an alert +the next report raises again. + +For every threshold metric, a config save resolves an alert whose reading is +below the new trigger. That includes readings inside a valid recovery band, +which the evaluator would hold. A clear level at or above the trigger is no +recovery band (`buildCanonicalMetricSpec` drops it), and a reading at the +trigger stays firing, as it does in the reducer, so a save no longer resolves +an alert the next evaluation raises again. +`TestCheckHostClearsDiskTemperatureAlertsWhenThresholdTurnsOff`, +`TestHostDiskTemperatureAlertsClearWhenAgentLeaves`, +`TestConfigSaveResolvesDiskTemperatureAlertAgainstItsDiskTypeThreshold` and +`TestConfigSaveKeepsAlertTheEvaluatorKeepsWithoutRecoveryBand` in +`internal/alerts/host_unraid_lifecycle_test.go` pin these paths. + ### Configured flapping thresholds remain reachable Every accepted positive `FlappingThreshold`, including values above ten, must diff --git a/internal/alerts/config_runtime.go b/internal/alerts/config_runtime.go index 97260eca4e..67c46b52c3 100644 --- a/internal/alerts/config_runtime.go +++ b/internal/alerts/config_runtime.go @@ -579,12 +579,14 @@ func (m *Manager) reevaluateActiveAlertsLocked() { continue } + // buildCanonicalMetricSpec keeps a clear level only below the trigger, + // and without one the evaluator keeps firing at the trigger itself. + // A clear level at or above the trigger must not resolve a reading + // the next evaluation raises again. clearThreshold := threshold.Clear - if clearThreshold <= 0 { - clearThreshold = threshold.Trigger - } + hasRecoveryBand := clearThreshold > 0 && clearThreshold < threshold.Trigger - if alert.Value <= clearThreshold { + if hasRecoveryBand && alert.Value <= clearThreshold { alertsToResolve = append(alertsToResolve, alertID) log.Info(). Str("alertID", alertID). diff --git a/internal/alerts/host.go b/internal/alerts/host.go index 8e0e8c5516..6084b5c158 100644 --- a/internal/alerts/host.go +++ b/internal/alerts/host.go @@ -136,7 +136,16 @@ func (m *Manager) resolveHostAlertThresholdsNoLock(alert *Alert, resourceID stri linkedContainerID = metadataStringValue(alert.Metadata, "linkedContainerId") } - return m.resolveHostThresholdsNoLock(hostID, linkedNodeID, linkedVMID, linkedContainerID) + thresholds := m.resolveHostThresholdsNoLock(hostID, linkedNodeID, linkedVMID, linkedContainerID) + // A disk temperature alert is judged against the per-type threshold + // CheckHost evaluates for that disk, so a config save does not resolve an + // alert the next report would raise again. + if alert != nil && alert.Type == "diskTemperature" { + override, exists := m.hostThresholdOverrideNoLock(hostID, linkedNodeID, linkedVMID, linkedContainerID) + overridden := exists && override.DiskTemperature != nil + thresholds.DiskTemperature = m.hostDiskTemperatureThresholdNoLock(thresholds.DiskTemperature, overridden, metadataStringValue(alert.Metadata, "diskType")) + } + return thresholds } func sanitizeHostComponent(value string) string { @@ -343,18 +352,12 @@ func (m *Manager) CheckHost(host models.Host) { if len(host.Sensors.SMART) > 0 { for _, disk := range host.Sensors.SMART { if disk.Temperature > 0 && !disk.Standby { - effectiveTempThreshold := thresholds.DiskTemperature - if diskType := strings.ToLower(strings.TrimSpace(disk.Type)); diskType != "" && !diskTempOverridden { - m.mu.RLock() - if th, ok := m.config.DiskTempByType[diskType]; ok { - t := th - effectiveTempThreshold = &t - } - m.mu.RUnlock() - } + 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 := fmt.Sprintf("%s/disk_temp:%s", hostResourceID(host.ID), sanitizeHostComponent(disk.Device)) + tempResourceID := hostDiskTemperatureResourcePrefix(host.ID) + sanitizeHostComponent(disk.Device) tempResourceName := fmt.Sprintf("%s (%s Temp)", hostDisplayName(host), disk.Device) diskTempMetadata := cloneMetadata(baseMetadata) @@ -379,10 +382,9 @@ func (m *Manager) CheckHost(host models.Host) { } } } else { - // We can't easily clear all disk temp alerts without tracking them, - // but checkMetric logic handles auto-resolution if value drops. - // If feature is disabled, ideally we should clear existing alerts. - // For now simple implementation. + // Disk temperature alerting is off for this host, so no later reading + // will resolve an alert it raised while it was on. + m.clearHostDiskTemperatureAlerts(host.ID) } seenDisks := make(map[string]struct{}, len(host.Disks)) @@ -828,6 +830,7 @@ func (m *Manager) HandleHostOfflineWithCorrelation(host models.Host, correlation diskResourcePrefixes := []string{ fmt.Sprintf("%s/disk:", resourceKey), + hostDiskTemperatureResourcePrefix(host.ID), } raidAlertPrefix := fmt.Sprintf("host-%s-raid-", host.ID) var alertsToClear []string @@ -884,6 +887,7 @@ func (m *Manager) clearHostDiskAlerts(hostID string) { prefixes := []string{ fmt.Sprintf("%s/disk:", hostResourceID(hostID)), + hostDiskTemperatureResourcePrefix(hostID), } m.mu.Lock() @@ -908,6 +912,50 @@ func (m *Manager) clearHostDiskAlerts(hostID string) { } } +// hostDiskTemperatureResourcePrefix is the resource ID prefix of every SMART +// disk temperature alert CheckHost raises for a host. +func hostDiskTemperatureResourcePrefix(hostID string) string { + return hostResourceID(hostID) + "/disk_temp:" +} + +// 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 +// linked-resource override set it. Callers must hold m.mu. +func (m *Manager) hostDiskTemperatureThresholdNoLock(hostThreshold *HysteresisThreshold, overridden bool, diskType string) *HysteresisThreshold { + if hostThreshold == nil || hostThreshold.Trigger <= 0 || overridden { + return hostThreshold + } + if diskType = strings.ToLower(strings.TrimSpace(diskType)); diskType != "" { + if th, ok := m.config.DiskTempByType[diskType]; ok { + return &th + } + } + return hostThreshold +} + +// clearHostDiskTemperatureAlerts resolves every SMART disk temperature alert +// for a host. +func (m *Manager) clearHostDiskTemperatureAlerts(hostID string) { + if hostID == "" { + return + } + + resourcePrefix := hostDiskTemperatureResourcePrefix(hostID) + + m.mu.Lock() + defer m.mu.Unlock() + + for storageKey, alert := range m.activeAlerts { + if alert == nil || alert.Type != "diskTemperature" { + continue + } + if strings.HasPrefix(alert.ResourceID, resourcePrefix) { + m.clearAlertNoLock(storageKey) + } + } +} + var customSensorAssessmentCodes = []string{ "custom_sensor_warning", "custom_sensor_critical", diff --git a/internal/alerts/host_unraid_lifecycle_test.go b/internal/alerts/host_unraid_lifecycle_test.go index 3388516e35..300f35b26f 100644 --- a/internal/alerts/host_unraid_lifecycle_test.go +++ b/internal/alerts/host_unraid_lifecycle_test.go @@ -1,6 +1,7 @@ package alerts import ( + "fmt" "strings" "testing" "time" @@ -199,6 +200,164 @@ func TestHandleHostTelemetryExpiredPreservesStaticStorageRisk(t *testing.T) { }) } +func TestCheckHostClearsDiskTemperatureAlertsWhenThresholdTurnsOff(t *testing.T) { + cases := []struct { + name string + turnOff func(m *Manager, hostID string) + }{ + { + name: "agent default disabled", + turnOff: func(m *Manager, hostID string) { + m.config.AgentDefaults.DiskTemperature = &HysteresisThreshold{Trigger: 0, Clear: 0} + }, + }, + { + name: "host override disables disk temperature", + turnOff: func(m *Manager, hostID string) { + m.config.Overrides[hostID] = ThresholdConfig{DiskTemperature: &HysteresisThreshold{Trigger: 0, Clear: 0}} + }, + }, + { + name: "host override disables the agent", + turnOff: func(m *Manager, hostID string) { + m.config.Overrides[hostID] = ThresholdConfig{Disabled: true} + }, + }, + { + name: "all agent alerts disabled", + turnOff: func(m *Manager, hostID string) { + m.config.DisableAllAgents = true + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + m := configureDiskTempTypeHostManager(t) + host := hostWithSMARTDiskTemp("host-temp-off", "sata", 60) + alertID := hostDiskTempAlertID(host) + + m.CheckHost(host) + alert, exists := testLookupActiveAlert(t, m, alertID) + if !exists { + t.Fatalf("expected disk temperature alert at 60C (sata trigger 55), active: %v", alertKeys(m)) + } + if alert.Type != "diskTemperature" || !strings.HasPrefix(alert.ResourceID, hostDiskTemperatureResourcePrefix(host.ID)) { + t.Fatalf("disk temperature alert type/resource = %q/%q", alert.Type, alert.ResourceID) + } + + // Change config directly so only CheckHost can resolve the alert. + m.mu.Lock() + tc.turnOff(m, host.ID) + m.mu.Unlock() + + m.CheckHost(host) + if _, exists := testLookupActiveAlert(t, m, alertID); exists { + t.Fatalf("disk temperature alert stayed active after its threshold was turned off, active: %v", alertKeys(m)) + } + }) + } +} + +func TestHostDiskTemperatureAlertsClearWhenAgentLeaves(t *testing.T) { + t.Run("agent removed", func(t *testing.T) { + m := configureDiskTempTypeHostManager(t) + host := hostWithSMARTDiskTemp("host-temp-removed", "sata", 60) + m.CheckHost(host) + if _, exists := testLookupActiveAlert(t, m, hostDiskTempAlertID(host)); !exists { + t.Fatalf("expected disk temperature alert, active: %v", alertKeys(m)) + } + + m.HandleHostRemoved(host) + if _, exists := testLookupActiveAlert(t, m, hostDiskTempAlertID(host)); exists { + t.Fatalf("disk temperature alert outlived its removed agent, active: %v", alertKeys(m)) + } + }) + + t.Run("agent confirmed offline", func(t *testing.T) { + m := configureDiskTempTypeHostManager(t) + host := hostWithSMARTDiskTemp("host-temp-offline", "sata", 60) + m.CheckHost(host) + if _, exists := testLookupActiveAlert(t, m, hostDiskTempAlertID(host)); !exists { + t.Fatalf("expected disk temperature alert, active: %v", alertKeys(m)) + } + + for i := 0; i < 3; i++ { + m.HandleHostOffline(host) + } + if _, exists := testLookupActiveAlert(t, m, canonicalConnectivityStateID(hostResourceID(host.ID))); !exists { + t.Fatalf("expected confirmed offline alert, active: %v", alertKeys(m)) + } + if _, exists := testLookupActiveAlert(t, m, hostDiskTempAlertID(host)); exists { + t.Fatalf("disk temperature alert stayed beside the offline alert, active: %v", alertKeys(m)) + } + }) +} + +func TestConfigSaveResolvesDiskTemperatureAlertAgainstItsDiskTypeThreshold(t *testing.T) { + m := configureDiskTempTypeHostManager(t) + + m.mu.Lock() + m.config.AgentDefaults.DiskTemperature = &HysteresisThreshold{Trigger: 75, Clear: 70} + m.mu.Unlock() + + // 72C is above the nvme trigger (70) but below the agent default (75). + host := hostWithSMARTDiskTemp("host-temp-save", "nvme", 72) + alertID := hostDiskTempAlertID(host) + m.CheckHost(host) + if _, exists := testLookupActiveAlert(t, m, alertID); !exists { + t.Fatalf("expected nvme disk temperature alert at 72C, active: %v", alertKeys(m)) + } + + // An unrelated save keeps the alert CheckHost would raise again. + cfg := m.GetConfig() + cfg.AgentDefaults.CPU = &HysteresisThreshold{Trigger: 85, Clear: 80} + m.UpdateConfig(cfg) + if _, exists := testLookupActiveAlert(t, m, alertID); !exists { + t.Fatalf("unrelated config save resolved a disk temperature alert above its nvme trigger, active: %v", alertKeys(m)) + } + + // Turning disk temperature off resolves it on save, before any report. + cfg = m.GetConfig() + cfg.AgentDefaults.DiskTemperature = &HysteresisThreshold{Trigger: 0, Clear: 0} + m.UpdateConfig(cfg) + if _, exists := testLookupActiveAlert(t, m, alertID); exists { + t.Fatalf("disk temperature alert stayed active after the threshold was turned off, active: %v", alertKeys(m)) + } +} + +func TestConfigSaveKeepsAlertTheEvaluatorKeepsWithoutRecoveryBand(t *testing.T) { + // Normalization keeps a 70/75 entry, and the evaluator treats a clear + // level at or above the trigger as no recovery band, firing from 70C. + for _, reading := range []int{72, 70} { + t.Run(fmt.Sprintf("%dC", reading), func(t *testing.T) { + m := configureDiskTempTypeHostManager(t) + m.mu.Lock() + m.config.DiskTempByType["nvme"] = HysteresisThreshold{Trigger: 70, Clear: 75} + m.mu.Unlock() + + host := hostWithSMARTDiskTemp("host-temp-no-band", "nvme", reading) + alertID := hostDiskTempAlertID(host) + m.CheckHost(host) + if _, exists := testLookupActiveAlert(t, m, alertID); !exists { + t.Fatalf("expected nvme disk temperature alert at %dC, active: %v", reading, alertKeys(m)) + } + + cfg := m.GetConfig() + cfg.AgentDefaults.CPU = &HysteresisThreshold{Trigger: 85, Clear: 80} + m.UpdateConfig(cfg) + if _, exists := testLookupActiveAlert(t, m, alertID); !exists { + t.Fatalf("config save resolved an alert the evaluator keeps firing at %dC, active: %v", reading, alertKeys(m)) + } + + m.CheckHost(host) + if _, exists := testLookupActiveAlert(t, m, alertID); !exists { + t.Fatalf("expected the next report to keep the alert, active: %v", alertKeys(m)) + } + }) + } +} + func hasAlertType(alerts []Alert, alertType string) bool { for _, alert := range alerts { if alert.Type == alertType { diff --git a/internal/alerts/metric_runtime.go b/internal/alerts/metric_runtime.go index 6d1ad3cba2..bfe670920d 100644 --- a/internal/alerts/metric_runtime.go +++ b/internal/alerts/metric_runtime.go @@ -51,7 +51,7 @@ func metricStabilityFor(metricType string, delaySeconds int) metricStabilityPoli func isMetricThresholdAlertType(metricType string) bool { switch metricType { - case "cpu", "memory", "disk", "diskRead", "diskWrite", "networkIn", "networkOut", "temperature", "usage": + case "cpu", "memory", "disk", "diskRead", "diskWrite", "networkIn", "networkOut", "temperature", "diskTemperature", "usage": return true default: return false @@ -77,6 +77,8 @@ func getThresholdForMetric(config ThresholdConfig, metricType string) *Hysteresi return config.NetworkOut case "temperature": return config.Temperature + case "diskTemperature": + return config.DiskTemperature case "usage": return config.Usage default: