From 7a026891c62efb6f19f5ebece15c4b704e10170c Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Wed, 7 Oct 2026 15:21:45 +0100 Subject: [PATCH] Keep a low alert default's clear below its trigger Setting a global default to a positive trigger of 5 or less (Machines CPU at 1%) stored agentDefaults.cpu as {trigger: 1, clear: 75}. The thresholds page sends clear max(0, trigger - 5), which is 0 there. NormalizeAgentDefaults then derived trigger - 5, found it non-positive and fell back to the factory clear, and ValidateHysteresisThresholds never checked agent CPU/memory/disk. PBS, node temperature and the persistence normalizer had the same fallback (validation repaired PBS and node temperature on save). Per-type disk fill and disk temperature entries were worse: a clear of 0 reset the whole pair, so a SATA disk temperature trigger of 4 was stored as 55. Alert firing and resolving did not change for these pairs: buildCanonicalMetricSpec drops a clear at or above the trigger and the reducer then clears at the trigger. The wrong level showed in /api/alerts/config and alerts.json, and firing agent alerts reported it as clearThreshold (75 on a 1% CPU alert, 85 on a disk alert still firing at 81.8%). PBS, node temperature, agent and per-type disk defaults now use normalizeThresholdPointer, which Kubernetes, TrueNAS and vSphere already use: a missing clear sits five points under the trigger floored at 0, and a clear at or above the trigger is repaired, so an API-written {50, 80} now gets a real recovery band at 45. A non-positive per-type trigger still resets to the type default. Persistence calls the same normalizer through a new NormalizeHysteresisThreshold instead of its own copies, so configs saved before this load repaired. --- .../v6/internal/subsystems/alerts.md | 36 +++ ...rts_config_normalize_branchcov0716_test.go | 14 +- ...g_normalize_defaults_branchcov0716_test.go | 48 ++-- internal/alerts/config/normalize.go | 118 +++------- .../normalize_low_trigger_clear_test.go | 212 ++++++++++++++++++ internal/alerts/config_facade.go | 4 + internal/alerts/config_validation_test.go | 82 ++++++- internal/alerts/default_normalization_test.go | 14 +- internal/config/persistence.go | 44 +--- internal/config/persistence_test.go | 117 ++++++++++ 10 files changed, 524 insertions(+), 165 deletions(-) create mode 100644 internal/alerts/config/normalize_low_trigger_clear_test.go diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index 9d7946179e..d09b079aba 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -1894,6 +1894,42 @@ controls in `internal/alerts/flapping_threshold_test.go` exercise both sweeps, drained and retained windows, dispatch callbacks and delivery diagnosis. These modeled-time controls are not installed notification-destination acceptance. +### Default thresholds keep a positive trigger's clear below it + +PBS, node temperature, agent, Kubernetes, TrueNAS and vSphere global defaults +go through `normalizeThresholdPointer` in `internal/alerts/config/normalize.go` +from `UpdateConfig`. The shared persistence normalization in +`internal/config/persistence.go` applies the same normalizer, through +`NormalizeHysteresisThreshold`, to the agent, node temperature and storage +pairs. A missing or negative threshold takes the factory default and a zero +trigger is off. A positive trigger always keeps a clear below it: a missing +clear sits five points under the trigger, floored at 0, and a clear at or +above the trigger is repaired. Guest and node usage, storage and Docker +defaults hold the same bound through `EnsureValidHysteresis` in +`ValidateHysteresisThresholds`, `NormalizeStorageDefaults` and +`NormalizeDockerThreshold`. The canonical per-type entries (`nvme`, `sata` and +`hdd` in `DiskFillByType`; `nvme`, `sas` and `sata` in `DiskTempByType`, the +types the thresholds page edits) keep a positive trigger and follow the same +clear rule; a non-positive trigger resets the entry to its type default, +because the page cannot switch a type off on its own. Other per-type keys are +stored as written. + +The thresholds page sends `max(0, trigger - 5)`, so a 1-5% default arrives +with clear 0 and must not fall back to the factory clear. Agent defaults used +to do that, storing `{trigger: 1, clear: 75}` for a 1% Machines CPU default, +and per-type disk entries replaced the whole pair, trigger included. The +evaluator already ignored a clear at or above the trigger +(`buildCanonicalMetricSpec` drops it and the reducer then clears at the +trigger), so those alerts fired and resolved the same way; `/api/alerts/config` +served the factory clear and firing alerts reported it as `clearThreshold`. A +repaired pair above the margin, such as `{50, 80}` written through the API, +now has a real recovery band at 45 where it used to clear at the trigger. +`internal/alerts/config/normalize_low_trigger_clear_test.go` pins every +default family and both per-type maps, `TestUpdateConfigKeepsLowTriggerClearBelowTrigger` in +`internal/alerts/config_validation_test.go` pins the saved config and the +firing alert's clear level, and the low-trigger and stored-clear tests in +`internal/config/persistence_test.go` pin the written file and load repair. + ### Monitor-only delivery is terminal Monitor-only alerts remain visible, but neither a firing nor a recovery diff --git a/internal/alerts/config/alerts_config_normalize_branchcov0716_test.go b/internal/alerts/config/alerts_config_normalize_branchcov0716_test.go index 97fd180691..268fd269e5 100644 --- a/internal/alerts/config/alerts_config_normalize_branchcov0716_test.go +++ b/internal/alerts/config/alerts_config_normalize_branchcov0716_test.go @@ -491,19 +491,23 @@ func TestBranchCovNormalizeDiskFillByType(t *testing.T) { } }) - t.Run("positive trigger with non-positive clear resets to default", func(t *testing.T) { + t.Run("positive trigger with non-positive clear keeps trigger and derives clear", func(t *testing.T) { cfg := &alertconfig.AlertConfig{ DiskFillByType: map[string]alertconfig.HysteresisThreshold{ "sata": {Trigger: 88, Clear: 0}, "hdd": {Trigger: 80, Clear: -3}, + "nvme": {Trigger: 0, Clear: 0}, }, } alertconfig.NormalizeDiskFillByType(cfg) - if sata := cfg.DiskFillByType["sata"]; !htEq(sata, alertconfig.HysteresisThreshold{Trigger: 90, Clear: 85}) { - t.Fatalf("sata = %+v, want default {90 85}", sata) + if sata := cfg.DiskFillByType["sata"]; !htEq(sata, alertconfig.HysteresisThreshold{Trigger: 88, Clear: 83}) { + t.Fatalf("sata = %+v, want {88 83}", sata) } - if hdd := cfg.DiskFillByType["hdd"]; !htEq(hdd, alertconfig.HysteresisThreshold{Trigger: 85, Clear: 80}) { - t.Fatalf("hdd = %+v, want default {85 80}", hdd) + if hdd := cfg.DiskFillByType["hdd"]; !htEq(hdd, alertconfig.HysteresisThreshold{Trigger: 80, Clear: 75}) { + t.Fatalf("hdd = %+v, want {80 75}", hdd) + } + if nvme := cfg.DiskFillByType["nvme"]; !htEq(nvme, alertconfig.HysteresisThreshold{Trigger: 92, Clear: 87}) { + t.Fatalf("nvme = %+v, want default {92 87}", nvme) } }) diff --git a/internal/alerts/config/alerts_config_normalize_defaults_branchcov0716_test.go b/internal/alerts/config/alerts_config_normalize_defaults_branchcov0716_test.go index e283497968..4561b2954b 100644 --- a/internal/alerts/config/alerts_config_normalize_defaults_branchcov0716_test.go +++ b/internal/alerts/config/alerts_config_normalize_defaults_branchcov0716_test.go @@ -178,14 +178,12 @@ func TestBranchCovNormalizePBSDefaults(t *testing.T) { wantMemory: alertconfig.HysteresisThreshold{Trigger: 90, Clear: 85}, }, { - // SUSPECTED SOURCE BUG: derived clear (1-5=-4) is <=0 so the code - // falls back to the FIXED default (75 CPU / 80 Memory), producing - // Clear >> Trigger. Unlike normalizeThresholdPointer (which clamps - // to 0) and with no EnsureValidHysteresis call, this stays invalid. - name: "small trigger with zero clear falls back to fixed default yielding clear>trigger", + // Derived clear (1-5=-4) floors at 0, never the factory clear, + // which would sit above the trigger. + name: "small trigger with zero clear keeps clear below trigger", in: alertconfig.HysteresisThreshold{Trigger: 1, Clear: 0}, - wantCPU: alertconfig.HysteresisThreshold{Trigger: 1, Clear: 75}, - wantMemory: alertconfig.HysteresisThreshold{Trigger: 1, Clear: 80}, + wantCPU: alertconfig.HysteresisThreshold{Trigger: 1, Clear: 0}, + wantMemory: alertconfig.HysteresisThreshold{Trigger: 1, Clear: 0}, }, } for _, tc := range tests { @@ -466,12 +464,10 @@ func TestBranchCovNormalizeNodeDefaults(t *testing.T) { want: alertconfig.HysteresisThreshold{Trigger: 90, Clear: 85}, }, { - // SUSPECTED SOURCE BUG: derived clear (1-5=-4) <=0 falls back to the - // FIXED 75, giving Clear(75) > Trigger(1); NormalizeNodeDefaults has - // no EnsureValidHysteresis call so this stays invalid. - name: "small trigger zero clear falls back to fixed 75 (clear>trigger)", + // Derived clear (1-5=-4) floors at 0, never the factory 75. + name: "small trigger zero clear keeps clear below trigger", in: &alertconfig.HysteresisThreshold{Trigger: 1, Clear: 0}, - want: alertconfig.HysteresisThreshold{Trigger: 1, Clear: 75}, + want: alertconfig.HysteresisThreshold{Trigger: 1, Clear: 0}, }, { name: "valid pair untouched", @@ -588,19 +584,16 @@ func TestBranchCovNormalizeAgentDefaults(t *testing.T) { } }) - // SUSPECTED SOURCE BUG (CPU arm): derived clear (1-5=-4) <=0 falls back to - // the FIXED 75; CPU has NO EnsureValidHysteresis call, so Clear(75) > - // Trigger(1) persists. DiskTemperature (which DOES call EnsureValidHysteresis) - // is repaired - covered by the next two subtests. - t.Run("CPU small trigger zero clear yields clear>trigger (no EnsureValidHysteresis net)", func(t *testing.T) { + // Derived clear (1-5=-4) floors at 0, never the factory 75. + t.Run("CPU small trigger zero clear keeps clear below trigger", func(t *testing.T) { cfg := &alertconfig.AlertConfig{ AgentDefaults: alertconfig.ThresholdConfig{ CPU: &alertconfig.HysteresisThreshold{Trigger: 1, Clear: 0}, }, } alertconfig.NormalizeAgentDefaults(cfg) - if !ptrHtEq(cfg.AgentDefaults.CPU, alertconfig.HysteresisThreshold{Trigger: 1, Clear: 75}) { - t.Fatalf("CPU = %+v, want {1 75} (clear>trigger anomaly)", cfg.AgentDefaults.CPU) + if !ptrHtEq(cfg.AgentDefaults.CPU, alertconfig.HysteresisThreshold{Trigger: 1, Clear: 0}) { + t.Fatalf("CPU = %+v, want {1 0}", cfg.AgentDefaults.CPU) } }) @@ -617,17 +610,16 @@ func TestBranchCovNormalizeAgentDefaults(t *testing.T) { } }) - t.Run("DiskTemperature small trigger: fallback to 50 then repaired to 0 by EnsureValidHysteresis", func(t *testing.T) { + t.Run("DiskTemperature small trigger zero clear keeps clear below trigger", func(t *testing.T) { cfg := &alertconfig.AlertConfig{ AgentDefaults: alertconfig.ThresholdConfig{ DiskTemperature: &alertconfig.HysteresisThreshold{Trigger: 2, Clear: 0}, }, } alertconfig.NormalizeAgentDefaults(cfg) - // derived clear 2-5=-3 <=0 -> fixed fallback 50; EnsureValidHysteresis - // then sees 50>=2 and sets clear=2-5=-3<0 -> 0. End state {2 0}. + // derived clear 2-5=-3 floors at 0. End state {2 0}. if !ptrHtEq(cfg.AgentDefaults.DiskTemperature, alertconfig.HysteresisThreshold{Trigger: 2, Clear: 0}) { - t.Fatalf("DiskTemperature = %+v, want {2 0} (fallback 50 repaired to 0)", cfg.AgentDefaults.DiskTemperature) + t.Fatalf("DiskTemperature = %+v, want {2 0}", cfg.AgentDefaults.DiskTemperature) } }) @@ -987,7 +979,7 @@ func TestBranchCovNormalizeDiskTempByType(t *testing.T) { } }) - t.Run("non-positive trigger or clear resets canonical key to default", func(t *testing.T) { + t.Run("non-positive trigger resets canonical key, non-positive clear derives", func(t *testing.T) { cfg := &alertconfig.AlertConfig{ DiskTempByType: map[string]alertconfig.HysteresisThreshold{ "sata": {Trigger: 88, Clear: 0}, @@ -996,14 +988,14 @@ func TestBranchCovNormalizeDiskTempByType(t *testing.T) { }, } alertconfig.NormalizeDiskTempByType(cfg) - if got := cfg.DiskTempByType["sata"]; !htEq(got, wantDefaults["sata"]) { - t.Fatalf("sata = %+v, want default %+v (clear<=0)", got, wantDefaults["sata"]) + if got := cfg.DiskTempByType["sata"]; !htEq(got, alertconfig.HysteresisThreshold{Trigger: 88, Clear: 83}) { + t.Fatalf("sata = %+v, want {88 83} (clear<=0 derives)", got) } if got := cfg.DiskTempByType["sas"]; !htEq(got, wantDefaults["sas"]) { t.Fatalf("sas = %+v, want default %+v (trigger<=0)", got, wantDefaults["sas"]) } - if got := cfg.DiskTempByType["nvme"]; !htEq(got, wantDefaults["nvme"]) { - t.Fatalf("nvme = %+v, want default %+v (clear<=0)", got, wantDefaults["nvme"]) + if got := cfg.DiskTempByType["nvme"]; !htEq(got, alertconfig.HysteresisThreshold{Trigger: 70, Clear: 65}) { + t.Fatalf("nvme = %+v, want {70 65} (clear<=0 derives)", got) } }) } diff --git a/internal/alerts/config/normalize.go b/internal/alerts/config/normalize.go index e73a527a48..d0268cefb1 100644 --- a/internal/alerts/config/normalize.go +++ b/internal/alerts/config/normalize.go @@ -161,27 +161,8 @@ func NormalizePMGDefaults(config *AlertConfig) { // NormalizePBSDefaults ensures PBS server threshold defaults exist. // Trigger=0 is allowed and means "disable alerting for this metric". func NormalizePBSDefaults(config *AlertConfig) { - if config.PBSDefaults.CPU == nil || config.PBSDefaults.CPU.Trigger < 0 { - config.PBSDefaults.CPU = &HysteresisThreshold{Trigger: 80, Clear: 75} - } else if config.PBSDefaults.CPU.Trigger == 0 { - config.PBSDefaults.CPU.Clear = 0 - } else if config.PBSDefaults.CPU.Clear <= 0 { - config.PBSDefaults.CPU.Clear = config.PBSDefaults.CPU.Trigger - 5 - if config.PBSDefaults.CPU.Clear <= 0 { - config.PBSDefaults.CPU.Clear = 75 - } - } - - if config.PBSDefaults.Memory == nil || config.PBSDefaults.Memory.Trigger < 0 { - config.PBSDefaults.Memory = &HysteresisThreshold{Trigger: 85, Clear: 80} - } else if config.PBSDefaults.Memory.Trigger == 0 { - config.PBSDefaults.Memory.Clear = 0 - } else if config.PBSDefaults.Memory.Clear <= 0 { - config.PBSDefaults.Memory.Clear = config.PBSDefaults.Memory.Trigger - 5 - if config.PBSDefaults.Memory.Clear <= 0 { - config.PBSDefaults.Memory.Clear = 80 - } - } + config.PBSDefaults.CPU = normalizeThresholdPointer(config.PBSDefaults.CPU, 80, 75, "pbs.cpu") + config.PBSDefaults.Memory = normalizeThresholdPointer(config.PBSDefaults.Memory, 85, 80, "pbs.memory") } func NormalizeSnapshotDefaults(config *AlertConfig) { @@ -290,61 +271,14 @@ func NormalizeBackupDefaults(config *AlertConfig) { } func NormalizeNodeDefaults(config *AlertConfig) { - if config.NodeDefaults.Temperature == nil || config.NodeDefaults.Temperature.Trigger < 0 { - config.NodeDefaults.Temperature = &HysteresisThreshold{Trigger: 80, Clear: 75} - } else if config.NodeDefaults.Temperature.Trigger == 0 { - config.NodeDefaults.Temperature.Clear = 0 - } else if config.NodeDefaults.Temperature.Clear <= 0 { - config.NodeDefaults.Temperature.Clear = config.NodeDefaults.Temperature.Trigger - 5 - if config.NodeDefaults.Temperature.Clear <= 0 { - config.NodeDefaults.Temperature.Clear = 75 - } - } + config.NodeDefaults.Temperature = normalizeThresholdPointer(config.NodeDefaults.Temperature, 80, 75, "node.temperature") } func NormalizeAgentDefaults(config *AlertConfig) { - if config.AgentDefaults.CPU == nil || config.AgentDefaults.CPU.Trigger < 0 { - config.AgentDefaults.CPU = &HysteresisThreshold{Trigger: 80, Clear: 75} - } else if config.AgentDefaults.CPU.Trigger == 0 { - config.AgentDefaults.CPU.Clear = 0 - } else if config.AgentDefaults.CPU.Clear <= 0 { - config.AgentDefaults.CPU.Clear = config.AgentDefaults.CPU.Trigger - 5 - if config.AgentDefaults.CPU.Clear <= 0 { - config.AgentDefaults.CPU.Clear = 75 - } - } - if config.AgentDefaults.Memory == nil || config.AgentDefaults.Memory.Trigger < 0 { - config.AgentDefaults.Memory = &HysteresisThreshold{Trigger: 85, Clear: 80} - } else if config.AgentDefaults.Memory.Trigger == 0 { - config.AgentDefaults.Memory.Clear = 0 - } else if config.AgentDefaults.Memory.Clear <= 0 { - config.AgentDefaults.Memory.Clear = config.AgentDefaults.Memory.Trigger - 5 - if config.AgentDefaults.Memory.Clear <= 0 { - config.AgentDefaults.Memory.Clear = 80 - } - } - if config.AgentDefaults.Disk == nil || config.AgentDefaults.Disk.Trigger < 0 { - config.AgentDefaults.Disk = &HysteresisThreshold{Trigger: 90, Clear: 85} - } else if config.AgentDefaults.Disk.Trigger == 0 { - config.AgentDefaults.Disk.Clear = 0 - } else if config.AgentDefaults.Disk.Clear <= 0 { - config.AgentDefaults.Disk.Clear = config.AgentDefaults.Disk.Trigger - 5 - if config.AgentDefaults.Disk.Clear <= 0 { - config.AgentDefaults.Disk.Clear = 85 - } - } - - if config.AgentDefaults.DiskTemperature == nil || config.AgentDefaults.DiskTemperature.Trigger < 0 { - config.AgentDefaults.DiskTemperature = &HysteresisThreshold{Trigger: 55, Clear: 50} - } else if config.AgentDefaults.DiskTemperature.Trigger == 0 { - config.AgentDefaults.DiskTemperature.Clear = 0 - } else if config.AgentDefaults.DiskTemperature.Clear <= 0 { - config.AgentDefaults.DiskTemperature.Clear = config.AgentDefaults.DiskTemperature.Trigger - 5 - if config.AgentDefaults.DiskTemperature.Clear <= 0 { - config.AgentDefaults.DiskTemperature.Clear = 50 - } - } - EnsureValidHysteresis(config.AgentDefaults.DiskTemperature, "agent.diskTemperature") + config.AgentDefaults.CPU = normalizeThresholdPointer(config.AgentDefaults.CPU, 80, 75, "agent.cpu") + config.AgentDefaults.Memory = normalizeThresholdPointer(config.AgentDefaults.Memory, 85, 80, "agent.memory") + config.AgentDefaults.Disk = normalizeThresholdPointer(config.AgentDefaults.Disk, 90, 85, "agent.disk") + config.AgentDefaults.DiskTemperature = normalizeThresholdPointer(config.AgentDefaults.DiskTemperature, 55, 50, "agent.diskTemperature") normalizeSMARTDefaults(&config.AgentDefaults) NormalizeDiskFillByType(config) @@ -396,6 +330,12 @@ func smartIntPtr(value int) *int { return &value } func smartInt64Ptr(value int64) *int64 { return &value } +// normalizeThresholdPointer is the canonical hysteresis normalizer. A missing +// or negative threshold takes the factory default, a zero trigger means off, +// and a positive trigger always keeps a clear below it: a missing clear sits +// 5 points under the trigger, floored at 0 for triggers of 5 or less, never +// the factory clear (which would sit above a low trigger), and a clear at or +// above the trigger is repaired. func normalizeThresholdPointer( current *HysteresisThreshold, defaultTrigger float64, @@ -420,6 +360,12 @@ func normalizeThresholdPointer( return &normalized } +// NormalizeHysteresisThreshold exposes normalizeThresholdPointer for other +// packages (e.g., config persistence). +func NormalizeHysteresisThreshold(current *HysteresisThreshold, defaultTrigger, defaultClear float64, metricName string) *HysteresisThreshold { + return normalizeThresholdPointer(current, defaultTrigger, defaultClear, metricName) +} + func NormalizeKubernetesDefaults(config *AlertConfig) { config.KubernetesDefaults.CPU = normalizeThresholdPointer(config.KubernetesDefaults.CPU, 80, 75, "kubernetes.cpu") config.KubernetesDefaults.Memory = normalizeThresholdPointer(config.KubernetesDefaults.Memory, 85, 80, "kubernetes.memory") @@ -468,8 +414,8 @@ func diskFillByTypeDefaults() map[string]HysteresisThreshold { // NormalizeDiskFillByType ensures AlertConfig.DiskFillByType is seeded with // lowercase nvme/sata/hdd defaults when nil, lowercases any existing keys, -// and resets non-positive trigger or clear values to the default for that -// key. Operator-customized positive values are preserved. +// and resets a non-positive trigger to the default for that key. A positive +// trigger is preserved and its clear follows normalizeThresholdPointer. func NormalizeDiskFillByType(config *AlertConfig) { defaults := diskFillByTypeDefaults() if config.DiskFillByType == nil { @@ -496,16 +442,14 @@ func NormalizeDiskFillByType(config *AlertConfig) { } } - // Ensure all canonical keys are present and have positive trigger/clear values. + // Ensure all canonical keys are present with a positive trigger. for key, defaultVal := range defaults { current, ok := config.DiskFillByType[key] - if !ok { + if !ok || current.Trigger <= 0 { config.DiskFillByType[key] = defaultVal continue } - if current.Trigger <= 0 || current.Clear <= 0 { - config.DiskFillByType[key] = defaultVal - } + config.DiskFillByType[key] = *normalizeThresholdPointer(¤t, defaultVal.Trigger, defaultVal.Clear, "diskFillByType."+key) } } @@ -521,8 +465,8 @@ func diskTempByTypeDefaults() map[string]HysteresisThreshold { // NormalizeDiskTempByType ensures AlertConfig.DiskTempByType is seeded with // lowercase nvme/sas/sata defaults when nil, lowercases any existing keys, -// and resets non-positive trigger or clear values to the default for that key. -// Operator-customized positive values are preserved. +// and resets a non-positive trigger to the default for that key. A positive +// trigger is preserved and its clear follows normalizeThresholdPointer. func NormalizeDiskTempByType(config *AlertConfig) { defaults := diskTempByTypeDefaults() if config.DiskTempByType == nil { @@ -550,13 +494,11 @@ func NormalizeDiskTempByType(config *AlertConfig) { for key, defaultVal := range defaults { current, ok := config.DiskTempByType[key] - if !ok { + if !ok || current.Trigger <= 0 { config.DiskTempByType[key] = defaultVal continue } - if current.Trigger <= 0 || current.Clear <= 0 { - config.DiskTempByType[key] = defaultVal - } + config.DiskTempByType[key] = *normalizeThresholdPointer(¤t, defaultVal.Trigger, defaultVal.Clear, "diskTempByType."+key) } } @@ -644,6 +586,10 @@ func ValidateHysteresisThresholds(config *AlertConfig) { EnsureValidHysteresis(config.VMwareDefaults.Memory, "vmware.memory") EnsureValidHysteresis(config.VMwareDefaults.Disk, "vmware.disk") EnsureValidHysteresis(config.VMwareDefaults.Usage, "vmware.usage") + EnsureValidHysteresis(config.AgentDefaults.CPU, "agent.cpu") + EnsureValidHysteresis(config.AgentDefaults.Memory, "agent.memory") + EnsureValidHysteresis(config.AgentDefaults.Disk, "agent.disk") + EnsureValidHysteresis(config.AgentDefaults.DiskTemperature, "agent.diskTemperature") } func ValidateQuietHoursTimezone(config *AlertConfig) { diff --git a/internal/alerts/config/normalize_low_trigger_clear_test.go b/internal/alerts/config/normalize_low_trigger_clear_test.go new file mode 100644 index 0000000000..cda588282e --- /dev/null +++ b/internal/alerts/config/normalize_low_trigger_clear_test.go @@ -0,0 +1,212 @@ +package config_test + +import ( + "testing" + + alertconfig "github.com/rcourtman/pulse-go-rewrite/internal/alerts/config" +) + +// normalizeLikeUpdateConfig runs the threshold normalizers in the order +// alerts.Manager.UpdateConfig runs them. +func normalizeLikeUpdateConfig(cfg *alertconfig.AlertConfig) { + alertconfig.NormalizeStorageDefaults(cfg) + alertconfig.NormalizeDockerDefaults(cfg) + alertconfig.NormalizePBSDefaults(cfg) + alertconfig.NormalizeNodeDefaults(cfg) + alertconfig.NormalizeAgentDefaults(cfg) + alertconfig.NormalizeKubernetesDefaults(cfg) + alertconfig.NormalizeTrueNASDefaults(cfg) + alertconfig.NormalizeVMwareDefaults(cfg) + alertconfig.ValidateHysteresisThresholds(cfg) +} + +type defaultThresholdSlot struct { + name string + set func(*alertconfig.AlertConfig, alertconfig.HysteresisThreshold) + get func(*alertconfig.AlertConfig) alertconfig.HysteresisThreshold +} + +func pointerSlot(name string, field func(*alertconfig.AlertConfig) **alertconfig.HysteresisThreshold) defaultThresholdSlot { + return defaultThresholdSlot{ + name: name, + set: func(cfg *alertconfig.AlertConfig, threshold alertconfig.HysteresisThreshold) { + *field(cfg) = &threshold + }, + get: func(cfg *alertconfig.AlertConfig) alertconfig.HysteresisThreshold { + if got := *field(cfg); got != nil { + return *got + } + return alertconfig.HysteresisThreshold{Trigger: -1, Clear: -1} + }, + } +} + +func valueSlot(name string, field func(*alertconfig.AlertConfig) *alertconfig.HysteresisThreshold) defaultThresholdSlot { + return defaultThresholdSlot{ + name: name, + set: func(cfg *alertconfig.AlertConfig, threshold alertconfig.HysteresisThreshold) { + *field(cfg) = threshold + }, + get: func(cfg *alertconfig.AlertConfig) alertconfig.HysteresisThreshold { + return *field(cfg) + }, + } +} + +// percentDefaultThresholdSlots lists every global default the thresholds page +// edits as a hysteresis pair that a user can set to a single-digit value. +func percentDefaultThresholdSlots() []defaultThresholdSlot { + type cfg = alertconfig.AlertConfig + type th = alertconfig.HysteresisThreshold + return []defaultThresholdSlot{ + pointerSlot("guest.cpu", func(c *cfg) **th { return &c.GuestDefaults.CPU }), + pointerSlot("guest.memory", func(c *cfg) **th { return &c.GuestDefaults.Memory }), + pointerSlot("guest.disk", func(c *cfg) **th { return &c.GuestDefaults.Disk }), + pointerSlot("node.cpu", func(c *cfg) **th { return &c.NodeDefaults.CPU }), + pointerSlot("node.memory", func(c *cfg) **th { return &c.NodeDefaults.Memory }), + pointerSlot("node.temperature", func(c *cfg) **th { return &c.NodeDefaults.Temperature }), + pointerSlot("pbs.cpu", func(c *cfg) **th { return &c.PBSDefaults.CPU }), + pointerSlot("pbs.memory", func(c *cfg) **th { return &c.PBSDefaults.Memory }), + pointerSlot("agent.cpu", func(c *cfg) **th { return &c.AgentDefaults.CPU }), + pointerSlot("agent.memory", func(c *cfg) **th { return &c.AgentDefaults.Memory }), + pointerSlot("agent.disk", func(c *cfg) **th { return &c.AgentDefaults.Disk }), + pointerSlot("agent.diskTemperature", func(c *cfg) **th { return &c.AgentDefaults.DiskTemperature }), + pointerSlot("kubernetes.cpu", func(c *cfg) **th { return &c.KubernetesDefaults.CPU }), + pointerSlot("kubernetes.memory", func(c *cfg) **th { return &c.KubernetesDefaults.Memory }), + pointerSlot("kubernetes.disk", func(c *cfg) **th { return &c.KubernetesDefaults.Disk }), + pointerSlot("truenas.cpu", func(c *cfg) **th { return &c.TrueNASDefaults.CPU }), + pointerSlot("truenas.memory", func(c *cfg) **th { return &c.TrueNASDefaults.Memory }), + pointerSlot("truenas.usage", func(c *cfg) **th { return &c.TrueNASDefaults.Usage }), + pointerSlot("truenas.temperature", func(c *cfg) **th { return &c.TrueNASDefaults.Temperature }), + pointerSlot("vmware.cpu", func(c *cfg) **th { return &c.VMwareDefaults.CPU }), + pointerSlot("vmware.memory", func(c *cfg) **th { return &c.VMwareDefaults.Memory }), + pointerSlot("vmware.usage", func(c *cfg) **th { return &c.VMwareDefaults.Usage }), + valueSlot("docker.cpu", func(c *cfg) *th { return &c.DockerDefaults.CPU }), + valueSlot("docker.memory", func(c *cfg) *th { return &c.DockerDefaults.Memory }), + valueSlot("docker.disk", func(c *cfg) *th { return &c.DockerDefaults.Disk }), + valueSlot("storage", func(c *cfg) *th { return &c.StorageDefault }), + } +} + +// The thresholds page sends a positive trigger with clear max(0, trigger-5), +// so a trigger at or below the 5-point margin arrives with clear 0. Every +// default family must keep that clear below the trigger. PBS, node +// temperature and agent defaults used to fall back to the factory clear, +// storing {trigger: 1, clear: 75} for a 1% Machines CPU default. +func TestLowTriggerDefaultsKeepClearBelowTrigger(t *testing.T) { + cases := []struct { + in alertconfig.HysteresisThreshold + want alertconfig.HysteresisThreshold + }{ + {alertconfig.HysteresisThreshold{Trigger: 1, Clear: 0}, alertconfig.HysteresisThreshold{Trigger: 1, Clear: 0}}, + {alertconfig.HysteresisThreshold{Trigger: 2.5, Clear: 0}, alertconfig.HysteresisThreshold{Trigger: 2.5, Clear: 0}}, + {alertconfig.HysteresisThreshold{Trigger: 5, Clear: 0}, alertconfig.HysteresisThreshold{Trigger: 5, Clear: 0}}, + // A clear at or above the trigger, such as one stored before this + // fix, is repaired the same way. + {alertconfig.HysteresisThreshold{Trigger: 1, Clear: 75}, alertconfig.HysteresisThreshold{Trigger: 1, Clear: 0}}, + {alertconfig.HysteresisThreshold{Trigger: 50, Clear: 60}, alertconfig.HysteresisThreshold{Trigger: 50, Clear: 45}}, + // Off stays off. + {alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}, alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}}, + } + for _, slot := range percentDefaultThresholdSlots() { + for _, tc := range cases { + cfg := &alertconfig.AlertConfig{} + slot.set(cfg, tc.in) + normalizeLikeUpdateConfig(cfg) + if got := slot.get(cfg); got != tc.want { + t.Errorf("%s %+v normalized to %+v, want %+v", slot.name, tc.in, got, tc.want) + } + } + } +} + +// A clear at or above the trigger that reaches ValidateHysteresisThresholds +// directly is repaired for agent defaults too, as for every other family. +func TestValidateHysteresisThresholdsRepairsAgentDefaults(t *testing.T) { + cfg := &alertconfig.AlertConfig{ + AgentDefaults: alertconfig.ThresholdConfig{ + CPU: &alertconfig.HysteresisThreshold{Trigger: 1, Clear: 75}, + Memory: &alertconfig.HysteresisThreshold{Trigger: 50, Clear: 50}, + Disk: &alertconfig.HysteresisThreshold{Trigger: 90, Clear: 95}, + DiskTemperature: &alertconfig.HysteresisThreshold{Trigger: 55, Clear: 60}, + }, + } + alertconfig.ValidateHysteresisThresholds(cfg) + for name, tc := range map[string]struct { + got *alertconfig.HysteresisThreshold + want alertconfig.HysteresisThreshold + }{ + "cpu": {cfg.AgentDefaults.CPU, alertconfig.HysteresisThreshold{Trigger: 1, Clear: 0}}, + "memory": {cfg.AgentDefaults.Memory, alertconfig.HysteresisThreshold{Trigger: 50, Clear: 45}}, + "disk": {cfg.AgentDefaults.Disk, alertconfig.HysteresisThreshold{Trigger: 90, Clear: 85}}, + "diskTemperature": {cfg.AgentDefaults.DiskTemperature, alertconfig.HysteresisThreshold{Trigger: 55, Clear: 50}}, + } { + if !ptrHtEq(tc.got, tc.want) { + t.Errorf("agent %s = %+v, want %+v", name, tc.got, tc.want) + } + } +} + +// Per-type disk fill and disk temperature entries keep a positive trigger and +// follow the same clear rule. They used to reset to the factory pair whenever +// the clear was not positive, losing a trigger of 1-5 (the thresholds page +// sends those with clear 0) and an API entry such as {88, 0}. A non-positive +// trigger still resets to the default, because a per-type entry cannot be +// switched off on its own. +func TestPerTypeDiskEntriesKeepTriggerAndClearBelowIt(t *testing.T) { + maps := map[string]struct { + set func(*alertconfig.AlertConfig, string, alertconfig.HysteresisThreshold) + get func(*alertconfig.AlertConfig, string) alertconfig.HysteresisThreshold + defaults map[string]alertconfig.HysteresisThreshold + }{ + "diskFillByType": { + set: func(cfg *alertconfig.AlertConfig, key string, th alertconfig.HysteresisThreshold) { + cfg.DiskFillByType = map[string]alertconfig.HysteresisThreshold{key: th} + }, + get: func(cfg *alertconfig.AlertConfig, key string) alertconfig.HysteresisThreshold { + return cfg.DiskFillByType[key] + }, + defaults: map[string]alertconfig.HysteresisThreshold{ + "nvme": {Trigger: 92, Clear: 87}, + "sata": {Trigger: 90, Clear: 85}, + "hdd": {Trigger: 85, Clear: 80}, + }, + }, + "diskTempByType": { + set: func(cfg *alertconfig.AlertConfig, key string, th alertconfig.HysteresisThreshold) { + cfg.DiskTempByType = map[string]alertconfig.HysteresisThreshold{key: th} + }, + get: func(cfg *alertconfig.AlertConfig, key string) alertconfig.HysteresisThreshold { + return cfg.DiskTempByType[key] + }, + defaults: map[string]alertconfig.HysteresisThreshold{ + "nvme": {Trigger: 70, Clear: 65}, + "sas": {Trigger: 65, Clear: 60}, + "sata": {Trigger: 55, Clear: 50}, + }, + }, + } + for mapName, m := range maps { + for key, factory := range m.defaults { + for _, tc := range []struct { + in alertconfig.HysteresisThreshold + want alertconfig.HysteresisThreshold + }{ + {alertconfig.HysteresisThreshold{Trigger: 1, Clear: 0}, alertconfig.HysteresisThreshold{Trigger: 1, Clear: 0}}, + {alertconfig.HysteresisThreshold{Trigger: 5, Clear: 0}, alertconfig.HysteresisThreshold{Trigger: 5, Clear: 0}}, + {alertconfig.HysteresisThreshold{Trigger: 88, Clear: 0}, alertconfig.HysteresisThreshold{Trigger: 88, Clear: 83}}, + {alertconfig.HysteresisThreshold{Trigger: 1, Clear: 75}, alertconfig.HysteresisThreshold{Trigger: 1, Clear: 0}}, + {alertconfig.HysteresisThreshold{Trigger: 60, Clear: 70}, alertconfig.HysteresisThreshold{Trigger: 60, Clear: 55}}, + {alertconfig.HysteresisThreshold{Trigger: 0, Clear: 0}, factory}, + {alertconfig.HysteresisThreshold{Trigger: -1, Clear: 0}, factory}, + } { + cfg := &alertconfig.AlertConfig{} + m.set(cfg, key, tc.in) + normalizeLikeUpdateConfig(cfg) + if got := m.get(cfg, key); got != tc.want { + t.Errorf("%s[%s] %+v normalized to %+v, want %+v", mapName, key, tc.in, got, tc.want) + } + } + } + } +} diff --git a/internal/alerts/config_facade.go b/internal/alerts/config_facade.go index d5352422ed..f7858af005 100644 --- a/internal/alerts/config_facade.go +++ b/internal/alerts/config_facade.go @@ -86,6 +86,10 @@ func NormalizeMetricEvaluationWindows(input map[string]map[string]int) map[strin return alertconfig.NormalizeMetricEvaluationWindows(input) } +func NormalizeHysteresisThreshold(threshold *HysteresisThreshold, defaultTrigger, defaultClear float64, metricName string) *HysteresisThreshold { + return alertconfig.NormalizeHysteresisThreshold(threshold, defaultTrigger, defaultClear, metricName) +} + func NormalizeDockerIgnoredPrefixes(prefixes []string) []string { return alertconfig.NormalizeDockerIgnoredPrefixes(prefixes) } diff --git a/internal/alerts/config_validation_test.go b/internal/alerts/config_validation_test.go index 74a9fc8606..82c579abe3 100644 --- a/internal/alerts/config_validation_test.go +++ b/internal/alerts/config_validation_test.go @@ -1,6 +1,11 @@ package alerts -import "testing" +import ( + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/models" +) func TestUpdateConfigNormalizesBackupIndicatorHours(t *testing.T) { t.Run("non-positive values default", func(t *testing.T) { @@ -83,6 +88,81 @@ func TestUpdateConfigNormalizesFlappingSettings(t *testing.T) { } } +// The thresholds page sends a positive trigger with clear max(0, trigger-5), +// so a 1-5% default arrives with clear 0. Agent defaults used to fall back to +// the factory clear, storing {trigger: 1, clear: 75}; the evaluator ignored +// that clear and cleared at the trigger, but firing alerts reported 75 as +// their clear level. +func TestUpdateConfigKeepsLowTriggerClearBelowTrigger(t *testing.T) { + m := newTestManager(t) + m.UpdateConfig(AlertConfig{ + Enabled: true, + AgentDefaults: ThresholdConfig{ + CPU: &HysteresisThreshold{Trigger: 1, Clear: 0}, + Memory: &HysteresisThreshold{Trigger: 3, Clear: 0}, + Disk: &HysteresisThreshold{Trigger: 5, Clear: 0}, + }, + NodeDefaults: ThresholdConfig{ + Temperature: &HysteresisThreshold{Trigger: 2, Clear: 0}, + }, + PBSDefaults: ThresholdConfig{ + CPU: &HysteresisThreshold{Trigger: 4, Clear: 0}, + Memory: &HysteresisThreshold{Trigger: 5, Clear: 0}, + }, + }) + + cfg := m.GetConfig() + for name, tc := range map[string]struct { + got *HysteresisThreshold + want HysteresisThreshold + }{ + "agent cpu": {cfg.AgentDefaults.CPU, HysteresisThreshold{Trigger: 1, Clear: 0}}, + "agent memory": {cfg.AgentDefaults.Memory, HysteresisThreshold{Trigger: 3, Clear: 0}}, + "agent disk": {cfg.AgentDefaults.Disk, HysteresisThreshold{Trigger: 5, Clear: 0}}, + "node temperature": {cfg.NodeDefaults.Temperature, HysteresisThreshold{Trigger: 2, Clear: 0}}, + "pbs cpu": {cfg.PBSDefaults.CPU, HysteresisThreshold{Trigger: 4, Clear: 0}}, + "pbs memory": {cfg.PBSDefaults.Memory, HysteresisThreshold{Trigger: 5, Clear: 0}}, + } { + if tc.got == nil || *tc.got != tc.want { + t.Errorf("%s = %+v, want %+v", name, tc.got, tc.want) + } + } + + m.mu.Lock() + m.config.TimeThresholds = map[string]int{} + m.config.ActivationState = ActivationActive + m.mu.Unlock() + host := models.Host{ + ID: "host-low-trigger", + DisplayName: "Low Trigger Host", + Hostname: "low-trigger.example", + Memory: models.Memory{Usage: 68.9, Total: 16384, Used: 11288, Free: 5096}, + Status: "online", + IntervalSeconds: 30, + LastSeen: time.Now(), + } + m.CheckHost(host) + memAlert, exists := testLookupActiveAlert(t, m, canonicalMetricStateID(hostResourceID(host.ID), "memory")) + if !exists { + t.Fatalf("expected memory alert at a 3%% trigger") + } + // No band below a 3% trigger: the alert clears at the trigger itself. + if got := memAlert.Metadata["clearThreshold"]; got != 3.0 { + t.Fatalf("memory alert clearThreshold = %v, want 3", got) + } + + // A config saved before the fix is repaired on its next load. + m.UpdateConfig(AlertConfig{ + Enabled: true, + AgentDefaults: ThresholdConfig{ + CPU: &HysteresisThreshold{Trigger: 1, Clear: 75}, + }, + }) + if got := m.GetConfig().AgentDefaults.CPU; got == nil || *got != (HysteresisThreshold{Trigger: 1, Clear: 0}) { + t.Fatalf("stored agent cpu {1 75} normalized to %+v, want {1 0}", got) + } +} + func TestUpdateConfigNormalizesExactEscalationRoutingAndRepeatPolicy(t *testing.T) { m := newTestManager(t) cfg := m.GetConfig() diff --git a/internal/alerts/default_normalization_test.go b/internal/alerts/default_normalization_test.go index 852db656db..eb23e1fb1f 100644 --- a/internal/alerts/default_normalization_test.go +++ b/internal/alerts/default_normalization_test.go @@ -246,7 +246,7 @@ func TestNormalizeDiskFillByType(t *testing.T) { } }) - t.Run("negative trigger resets to default", func(t *testing.T) { + t.Run("negative trigger resets to default, negative clear derives", func(t *testing.T) { cfg := &AlertConfig{ DiskFillByType: map[string]HysteresisThreshold{ "nvme": {Trigger: -1, Clear: 10}, @@ -259,9 +259,10 @@ func TestNormalizeDiskFillByType(t *testing.T) { if nvme.Trigger != 92 || nvme.Clear != 87 { t.Fatalf("nvme = %+v, want default reset {Trigger:92 Clear:87}", nvme) } + // A positive trigger is kept; only its clear is derived. sata := cfg.DiskFillByType["sata"] - if sata.Trigger != 90 || sata.Clear != 85 { - t.Fatalf("sata = %+v, want default reset {Trigger:90 Clear:85}", sata) + if sata.Trigger != 88 || sata.Clear != 83 { + t.Fatalf("sata = %+v, want {Trigger:88 Clear:83}", sata) } }) @@ -356,7 +357,7 @@ func TestNormalizeDiskTempByType(t *testing.T) { } }) - t.Run("negative trigger resets to default", func(t *testing.T) { + t.Run("negative trigger resets to default, negative clear derives", func(t *testing.T) { cfg := &AlertConfig{ DiskTempByType: map[string]HysteresisThreshold{ "nvme": {Trigger: -1, Clear: 10}, @@ -369,9 +370,10 @@ func TestNormalizeDiskTempByType(t *testing.T) { if nvme.Trigger != 70 || nvme.Clear != 65 { t.Fatalf("nvme = %+v, want default reset {Trigger:70 Clear:65}", nvme) } + // A positive trigger is kept; only its clear is derived. sas := cfg.DiskTempByType["sas"] - if sas.Trigger != 65 || sas.Clear != 60 { - t.Fatalf("sas = %+v, want default reset {Trigger:65 Clear:60}", sas) + if sas.Trigger != 64 || sas.Clear != 59 { + t.Fatalf("sas = %+v, want {Trigger:64 Clear:59}", sas) } }) diff --git a/internal/config/persistence.go b/internal/config/persistence.go index c1eefbf941..90a21d72c7 100644 --- a/internal/config/persistence.go +++ b/internal/config/persistence.go @@ -817,40 +817,6 @@ func (c *ConfigPersistence) persistAPITokensLocked(tokens []APITokenRecord) erro return nil } -// normalizeHysteresisThreshold ensures a hysteresis threshold pointer has valid -// trigger/clear values. If the pointer is nil or trigger is negative, it is set -// to the given defaults. Trigger==0 means "disabled" (clear forced to 0). -// Otherwise a non-positive clear is derived from trigger-5 with defaultClear as floor. -func normalizeHysteresisThreshold(t **alerts.HysteresisThreshold, defaultTrigger, defaultClear float64) { - if *t == nil || (*t).Trigger < 0 { - *t = &alerts.HysteresisThreshold{Trigger: defaultTrigger, Clear: defaultClear} - } else if (*t).Trigger == 0 { - (*t).Clear = 0 - } else if (*t).Clear <= 0 { - (*t).Clear = (*t).Trigger - 5 - if (*t).Clear <= 0 { - (*t).Clear = defaultClear - } - } -} - -// normalizeStorageDefault normalizes a non-pointer HysteresisThreshold used for -// storage defaults. Same logic as normalizeHysteresisThreshold but operates on -// a value (not pointer) and uses 0 as the clear floor. -func normalizeStorageDefault(t *alerts.HysteresisThreshold) { - if t.Trigger < 0 { - t.Trigger = 85 - t.Clear = 80 - } else if t.Trigger == 0 { - t.Clear = 0 - } else if t.Clear <= 0 { - t.Clear = t.Trigger - 5 - if t.Clear < 0 { - t.Clear = 0 - } - } -} - // normalizeAlertDefaults applies shared normalization logic to an AlertConfig. // Called by both SaveAlertConfig and LoadAlertConfig to avoid duplicating the // same validation and defaulting code. @@ -858,7 +824,7 @@ func normalizeAlertDefaults(config *alerts.AlertConfig) { alerts.NormalizeAlertConfigAliases(config) // Storage threshold - normalizeStorageDefault(&config.StorageDefault) + config.StorageDefault = *alerts.NormalizeHysteresisThreshold(&config.StorageDefault, 85, 80, "storage") if config.MinimumDelta <= 0 { config.MinimumDelta = 2.0 @@ -871,9 +837,9 @@ func normalizeAlertDefaults(config *alerts.AlertConfig) { } // Agent defaults - normalizeHysteresisThreshold(&config.AgentDefaults.CPU, 80, 75) - normalizeHysteresisThreshold(&config.AgentDefaults.Memory, 85, 80) - normalizeHysteresisThreshold(&config.AgentDefaults.Disk, 90, 85) + config.AgentDefaults.CPU = alerts.NormalizeHysteresisThreshold(config.AgentDefaults.CPU, 80, 75, "agent.cpu") + config.AgentDefaults.Memory = alerts.NormalizeHysteresisThreshold(config.AgentDefaults.Memory, 85, 80, "agent.memory") + config.AgentDefaults.Disk = alerts.NormalizeHysteresisThreshold(config.AgentDefaults.Disk, 90, 85, "agent.disk") // Time thresholds config.MetricTimeThresholds = alerts.NormalizeMetricTimeThresholds(config.MetricTimeThresholds) @@ -1135,7 +1101,7 @@ func (c *ConfigPersistence) LoadAlertConfig() (*alerts.AlertConfig, error) { } // Load-specific: NodeDefaults.Temperature normalization - normalizeHysteresisThreshold(&config.NodeDefaults.Temperature, 80, 75) + config.NodeDefaults.Temperature = alerts.NormalizeHysteresisThreshold(config.NodeDefaults.Temperature, 80, 75, "node.temperature") // Shared normalization (storage, host defaults, time thresholds, snapshots, backups, etc.) normalizeAlertDefaults(&config) diff --git a/internal/config/persistence_test.go b/internal/config/persistence_test.go index 21ef33a6b2..f0e1b5e6c2 100644 --- a/internal/config/persistence_test.go +++ b/internal/config/persistence_test.go @@ -183,6 +183,123 @@ func TestSaveAlertConfig_NormalizesAgentDefaultsClear(t *testing.T) { } } +// A positive trigger at or below the 5-point hysteresis margin arrives from +// the thresholds page with clear 0 (max(0, trigger-5)). It must keep a clear +// below its trigger, never fall back to the factory clear: a 1% Machines CPU +// default used to store {trigger: 1, clear: 75}. +func TestSaveAlertConfig_LowTriggerKeepsClearBelowTrigger(t *testing.T) { + tempDir := t.TempDir() + cp := config.NewConfigPersistence(tempDir) + if err := cp.EnsureConfigDir(); err != nil { + t.Fatalf("EnsureConfigDir: %v", err) + } + + cfg := alerts.AlertConfig{ + Enabled: true, + StorageDefault: alerts.HysteresisThreshold{Trigger: 3, Clear: 0}, + NodeDefaults: alerts.ThresholdConfig{ + Temperature: &alerts.HysteresisThreshold{Trigger: 4, Clear: 0}, + }, + AgentDefaults: alerts.ThresholdConfig{ + CPU: &alerts.HysteresisThreshold{Trigger: 1, Clear: 0}, + Memory: &alerts.HysteresisThreshold{Trigger: 3, Clear: 0}, + Disk: &alerts.HysteresisThreshold{Trigger: 5, Clear: 0}, + }, + } + + if err := cp.SaveAlertConfig(cfg); err != nil { + t.Fatalf("SaveAlertConfig: %v", err) + } + + // The file itself must hold the pairs, not only the loaded config, so a + // load-time repair cannot mask a bad save. + data, err := os.ReadFile(filepath.Join(tempDir, "alerts.json")) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + var saved alerts.AlertConfig + if err := json.Unmarshal(data, &saved); err != nil { + t.Fatalf("Unmarshal saved alerts.json: %v", err) + } + + loaded, err := cp.LoadAlertConfig() + if err != nil { + t.Fatalf("LoadAlertConfig: %v", err) + } + + for _, view := range []struct { + name string + cfg *alerts.AlertConfig + }{{"saved", &saved}, {"loaded", loaded}} { + for name, tc := range map[string]struct { + got *alerts.HysteresisThreshold + want alerts.HysteresisThreshold + }{ + "agent cpu": {view.cfg.AgentDefaults.CPU, alerts.HysteresisThreshold{Trigger: 1, Clear: 0}}, + "agent memory": {view.cfg.AgentDefaults.Memory, alerts.HysteresisThreshold{Trigger: 3, Clear: 0}}, + "agent disk": {view.cfg.AgentDefaults.Disk, alerts.HysteresisThreshold{Trigger: 5, Clear: 0}}, + "storage": {&view.cfg.StorageDefault, alerts.HysteresisThreshold{Trigger: 3, Clear: 0}}, + } { + if tc.got == nil || *tc.got != tc.want { + t.Errorf("%s %s = %+v, want %+v", view.name, name, tc.got, tc.want) + } + } + } + if got := loaded.NodeDefaults.Temperature; got == nil || *got != (alerts.HysteresisThreshold{Trigger: 4, Clear: 0}) { + t.Errorf("loaded node temperature = %+v, want {4 0}", got) + } +} + +// An alerts.json written before the fix holds a clear above its trigger; it +// loads repaired rather than serving the factory clear back to the UI. +func TestLoadAlertConfig_RepairsStoredClearAboveTrigger(t *testing.T) { + tempDir := t.TempDir() + cp := config.NewConfigPersistence(tempDir) + if err := cp.EnsureConfigDir(); err != nil { + t.Fatalf("EnsureConfigDir: %v", err) + } + + raw := alerts.AlertConfig{ + Enabled: true, + StorageDefault: alerts.HysteresisThreshold{Trigger: 1, Clear: 80}, + NodeDefaults: alerts.ThresholdConfig{ + Temperature: &alerts.HysteresisThreshold{Trigger: 2, Clear: 75}, + }, + AgentDefaults: alerts.ThresholdConfig{ + CPU: &alerts.HysteresisThreshold{Trigger: 1, Clear: 75}, + Memory: &alerts.HysteresisThreshold{Trigger: 50, Clear: 80}, + Disk: &alerts.HysteresisThreshold{Trigger: 90, Clear: 85}, + }, + } + data, err := json.Marshal(raw) + if err != nil { + t.Fatalf("Marshal: %v", err) + } + if err := os.WriteFile(filepath.Join(tempDir, "alerts.json"), data, 0600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + loaded, err := cp.LoadAlertConfig() + if err != nil { + t.Fatalf("LoadAlertConfig: %v", err) + } + + for name, tc := range map[string]struct { + got *alerts.HysteresisThreshold + want alerts.HysteresisThreshold + }{ + "agent cpu": {loaded.AgentDefaults.CPU, alerts.HysteresisThreshold{Trigger: 1, Clear: 0}}, + "agent memory": {loaded.AgentDefaults.Memory, alerts.HysteresisThreshold{Trigger: 50, Clear: 45}}, + "agent disk": {loaded.AgentDefaults.Disk, alerts.HysteresisThreshold{Trigger: 90, Clear: 85}}, + "node temperature": {loaded.NodeDefaults.Temperature, alerts.HysteresisThreshold{Trigger: 2, Clear: 0}}, + "storage": {&loaded.StorageDefault, alerts.HysteresisThreshold{Trigger: 1, Clear: 0}}, + } { + if tc.got == nil || *tc.got != tc.want { + t.Errorf("%s = %+v, want %+v", name, tc.got, tc.want) + } + } +} + // TestSaveAlertConfig_AgentDefaultsZeroDisablesAlerting verifies that setting // Host Agent thresholds to 0 is preserved (fixes GitHub issue #864). // Setting a threshold to 0 should disable alerting for that metric.