mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-24 04:33:17 +00:00
Clear agent disk temperature alerts when their threshold is off
Turning off the agent disk temperature threshold left every open disk temperature alert active indefinitely. CheckHost's disabled branch was an empty placeholder, the host-wide clear paths (all agent alerts disabled, a disabled host override, agent removal, confirmed offline) matched only the /disk: prefix and missed /disk_temp:, and a config save skipped these alerts because diskTemperature was not classed as a threshold metric. CheckHost now clears a host's disk temperature alerts when the effective threshold is off, and the host-wide clears include them. A config save resolves them against the same threshold CheckHost evaluates, including the per-type DiskTempByType refinement. That way an unrelated save does not resolve an alert above its disk type's trigger only for the next report to raise it again. The save path now also matches the evaluator when there is no recovery band. buildCanonicalMetricSpec drops a clear level at or above the trigger, and the reducer keeps firing at the trigger itself, but reevaluateActiveAlertsLocked resolved any reading at or below that clear level. A per-type entry such as 70/75 (normalization keeps it) resolved a 72C alert on save, and the next report raised it again. With no recovery band, a reading exactly at the trigger was resolved the same way.
This commit is contained in:
@@ -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:<host>/disk_temp:<device>`) 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
|
||||
|
||||
@@ -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).
|
||||
|
||||
+63
-15
@@ -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",
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user