Judge filesystem alerts by their own threshold on config save

A config save re-judges every active alert. Agent filesystem alerts
were re-judged by the host disk threshold, ignoring the filesystem's
own override and the per-type DiskFillByType threshold CheckHost uses.
Guest filesystem alerts were resolved by their "<guestID>-disk-<key>"
resource ID, which matches no guest-disk override and names no guest,
so the guest's own override was missed too. A filesystem override of
70 raised an alert at 80 under a default of 90; saving any alert
setting resolved it and sent a recovery, and the next poll raised it
again as a new alert.

CheckHost and CheckGuest now resolve a filesystem's threshold through
hostDiskUsageThresholdNoLock and guestDiskUsageThresholdNoLock, and
reevaluation calls the same helpers: resolveHostAlertThresholdsNoLock
for agent filesystem alerts, and a new resolveGuestAlertThresholdsNoLock
that resolves a guest filesystem alert's guest by the ID
parseGuestAlertIdentity reads from its resource ID. The helpers return
override and per-type thresholds as owned copies, so evaluation still
never normalizes the live config.

TestConfigSaveJudgesFilesystemAlertsByTheirOwnThreshold covers an
agent filesystem override, an NVMe fill threshold, a guest filesystem
override and a guest override. With host.go, guest.go and
config_runtime.go overlaid by their origin/main or #2696 copies, all
four fail: the unchanged save resolves the alert.

Stacked on #2696.
This commit is contained in:
rcourtman
2026-10-07 17:36:34 +01:00
parent 71a470129f
commit 539eadfab2
5 changed files with 234 additions and 47 deletions
@@ -3163,6 +3163,34 @@ in place, is gone. `internal/alerts/alerts_test.go`
evaluation against `GetConfig()` and `UpdateConfig(GetConfig())` under
`-race`.
### Config-save reevaluation judges a filesystem alert by its own threshold
`UpdateConfig` re-judges every active alert, and a filesystem usage alert's
threshold resolves through the same per-filesystem helper its evaluator uses.
`hostDiskUsageThresholdNoLock` resolves an agent filesystem
(`agent:<host>/disk:<label>`): its own override, then the `DiskFillByType`
threshold for a hardware type inferred from the device while the host's disk
alerting is on, then the host's disk threshold. `guestDiskUsageThresholdNoLock`
resolves a guest filesystem (`<guestID>-disk-<key>`): its `guest-disk:`
override, then the guest's disk threshold. `CheckHost` and `CheckGuest` call
them under the read lock, and `resolveHostAlertThresholdsNoLock` and
`resolveGuestAlertThresholdsNoLock` call them for reevaluation. Both return an
override or per-type threshold as an owned copy. A guest filesystem alert
resolves its guest's threshold, override and identity-based custom rules by the
canonical `instance:node:vmid` guest ID that `parseGuestAlertIdentity` reads
from its resource ID. Custom rules that filter on live metrics still see no
readings there, as for every reevaluated guest alert. Reevaluation used to
judge agent filesystems by the host threshold, and guest filesystems without
their `guest-disk:` override or the guest's own override, because the
filesystem's resource ID names no guest. Any settings save resolved an alert a
per-disk threshold of 70 raised at 80 under a default of 90, notifying
recovery, and the next poll raised it again as a new alert.
`internal/alerts/threshold_resolution_shared_test.go`
`TestConfigSaveJudgesFilesystemAlertsByTheirOwnThreshold` covers an agent
filesystem override, an NVMe fill threshold, a guest filesystem override and a
guest override: an unchanged save must keep the alert, and a save lifting the
trigger above the reading must resolve it.
### Versioned alert-intent policy
The alerts runtime owns one versioned alert-intent document and its durable
+1 -1
View File
@@ -545,7 +545,7 @@ func (m *Manager) reevaluateActiveAlertsLocked() {
continue
}
guestThresholds := m.getGuestThresholds(guestSnapshotFromAlert(alert, resourceID), resourceID)
guestThresholds := m.resolveGuestAlertThresholdsNoLock(alert, resourceID)
if guestThresholds.Disabled {
alertsToResolve = append(alertsToResolve, alertID)
continue
+43 -13
View File
@@ -327,21 +327,12 @@ func (m *Manager) CheckGuest(guest any, instanceName string) {
seenDiskResources[perDiskResourceID] = struct{}{}
message := fmt.Sprintf("%s disk (%s) at %.1f%%", guestType, label, disk.Usage)
effectiveDiskThreshold := thresholds.Disk
// Own the override's Disk before ensureHysteresisThreshold fills
// Clear: the returned copy still points into the live config.
m.mu.RLock()
diskOverride, hasDiskOverride := lookupGuestDiskOverride(m.config.Overrides, guest, guestID, keySource)
diskOverride.Disk = cloneThreshold(diskOverride.Disk)
effectiveDiskThreshold, diskDisabled := m.guestDiskUsageThresholdNoLock(guest, guestID, keySource, thresholds.Disk)
m.mu.RUnlock()
if hasDiskOverride {
if diskOverride.Disabled {
m.clearAlert(canonicalMetricStateID(perDiskResourceID, "disk"))
continue
}
if diskOverride.Disk != nil {
effectiveDiskThreshold = ensureHysteresisThreshold(diskOverride.Disk)
}
if diskDisabled {
m.clearAlert(canonicalMetricStateID(perDiskResourceID, "disk"))
continue
}
if snapshot.DiskUnavailable {
if effectiveDiskThreshold == nil || effectiveDiskThreshold.Trigger <= 0 {
@@ -426,6 +417,45 @@ func guestDiskIdentity(disk models.Disk, idx int) (label, keySource, sanitizedKe
return label, keySource, sanitizedKey
}
// guestDiskUsageThresholdNoLock resolves the usage threshold one guest
// filesystem is judged by: the filesystem's own override, else the guest's disk
// threshold. disabled reports an override that switches the filesystem's
// alerts off, and then the threshold is nil. CheckGuest and config-save
// reevaluation both resolve through it. An override threshold is returned as
// an owned copy, so normalizing it never writes into the live config. Callers
// must hold m.mu.
func (m *Manager) guestDiskUsageThresholdNoLock(guest any, guestID, diskKey string, guestDisk *HysteresisThreshold) (threshold *HysteresisThreshold, disabled bool) {
if override, ok := lookupGuestDiskOverride(m.config.Overrides, guest, guestID, diskKey); ok {
if override.Disabled {
return nil, true
}
if override.Disk != nil {
return ensureHysteresisThreshold(cloneThreshold(override.Disk)), false
}
}
return guestDisk, false
}
// resolveGuestAlertThresholdsNoLock resolves thresholds for a persisted guest
// alert. A filesystem alert ("<guestID>-disk-<key>") resolves its guest's
// thresholds by the guest ID, and its Disk by the filesystem's own override, as
// CheckGuest does. Resolved by the filesystem's resource ID, which names no
// guest, it missed both overrides, and a config save resolved an alert the next
// poll raised again. Callers must hold m.mu.
func (m *Manager) resolveGuestAlertThresholdsNoLock(alert *Alert, resourceID string) ThresholdConfig {
resourceID = strings.TrimSpace(resourceID)
if identity, ok := parseGuestAlertIdentity(resourceID); ok {
if diskKey, isFilesystem := strings.CutPrefix(identity.resourceSuffix, "-disk-"); isFilesystem {
guestID := strings.TrimSuffix(resourceID, identity.resourceSuffix)
guest := guestSnapshotFromAlert(alert, guestID)
thresholds := m.getGuestThresholds(guest, guestID)
thresholds.Disk, _ = m.guestDiskUsageThresholdNoLock(guest, guestID, diskKey, thresholds.Disk)
return thresholds
}
}
return m.getGuestThresholds(guestSnapshotFromAlert(alert, resourceID), resourceID)
}
func guestIORateMetrics(snapshot guestSnapshot) (diskRead, diskWrite, networkIn, networkOut *UnifiedResourceMetric) {
validity := snapshot.IORateValidity.EffectiveForRates(
snapshot.DiskRead,
+45 -33
View File
@@ -172,9 +172,50 @@ func (m *Manager) resolveHostAlertThresholdsNoLock(alert *Alert, resourceID stri
thresholds.DiskTemperature = m.lowestHostDiskTemperatureThresholdNoLock(thresholds.DiskTemperature)
}
}
// A filesystem alert is judged by the threshold CheckHost evaluates that
// filesystem against, so a config save does not resolve an alert its
// override or per-type fill threshold still holds. A disabled filesystem
// resolves with no threshold, as CheckHost clears it.
if alert != nil && alert.Type == "disk" && isHostFilesystemResourceID(resourceID) {
thresholds.Disk, _ = m.hostDiskUsageThresholdNoLock(strings.TrimSpace(resourceID), metadataStringValue(alert.Metadata, "device"), thresholds.Disk)
}
return thresholds
}
// isHostFilesystemResourceID reports whether resourceID names one agent
// filesystem ("agent:<host>/disk:<label>") rather than the host itself.
func isHostFilesystemResourceID(resourceID string) bool {
_, child, ok := strings.Cut(stripHostResourcePrefix(resourceID), "/")
return ok && strings.HasPrefix(child, "disk:")
}
// hostDiskUsageThresholdNoLock resolves the usage threshold one agent
// filesystem is judged by: the filesystem's own override, then the fill
// threshold for its hardware type while the host's disk alerting is on, then
// the host's disk threshold. disabled reports an override that switches the
// filesystem's alerts off, and then the threshold is nil. CheckHost and
// config-save reevaluation both resolve through it. An override or per-type
// threshold is returned as an owned copy, so normalizing it never writes into
// the live config. Callers must hold m.mu.
func (m *Manager) hostDiskUsageThresholdNoLock(diskResourceID, device string, hostDisk *HysteresisThreshold) (threshold *HysteresisThreshold, disabled bool) {
if override, ok := m.config.Overrides[diskResourceID]; ok {
if override.Disabled {
return nil, true
}
if override.Disk != nil {
return ensureHysteresisThreshold(cloneThreshold(override.Disk)), false
}
}
if hostDisk != nil && hostDisk.Trigger > 0 {
if hwType := inferDiskHardwareType(device); hwType != "" {
if byType, ok := m.config.DiskFillByType[hwType]; ok {
return &byType, false
}
}
}
return hostDisk, false
}
func sanitizeHostComponent(value string) string {
value = strings.TrimSpace(strings.ToLower(value))
if value == "" {
@@ -450,41 +491,12 @@ func (m *Manager) CheckHost(host models.Host) {
diskResourceID, diskName := hostDiskResourceID(host, disk)
seenDisks[diskResourceID] = struct{}{}
// Check for disk-specific override. The copy's Disk still points into
// the live config, so own it before ensureHysteresisThreshold fills Clear.
m.mu.RLock()
diskOverride, hasDiskOverride := m.config.Overrides[diskResourceID]
diskOverride.Disk = cloneThreshold(diskOverride.Disk)
effectiveDiskThreshold, diskDisabled := m.hostDiskUsageThresholdNoLock(diskResourceID, disk.Device, thresholds.Disk)
m.mu.RUnlock()
// Determine the effective disk threshold
var effectiveDiskThreshold *HysteresisThreshold
if hasDiskOverride {
// If disk is disabled via override, skip alerting
if diskOverride.Disabled {
m.clearAlert(canonicalMetricStateID(diskResourceID, "disk"))
continue
}
// Use disk-specific threshold if set
if diskOverride.Disk != nil {
effectiveDiskThreshold = ensureHysteresisThreshold(diskOverride.Disk)
}
}
// Per-type override: consult DiskFillByType if hardware type is inferable
// from the device path and no disk-specific override applied above.
if effectiveDiskThreshold == nil && thresholds.Disk != nil && thresholds.Disk.Trigger > 0 {
if hwType := inferDiskHardwareType(disk.Device); hwType != "" {
m.mu.RLock()
if th, ok := m.config.DiskFillByType[hwType]; ok {
t := th
effectiveDiskThreshold = &t
}
m.mu.RUnlock()
}
}
// Fall back to host-level threshold
if effectiveDiskThreshold == nil {
effectiveDiskThreshold = thresholds.Disk
if diskDisabled {
m.clearAlert(canonicalMetricStateID(diskResourceID, "disk"))
continue
}
// Skip if no threshold configured (nil)
@@ -1350,6 +1350,123 @@ func TestGuestFilesystemEvidenceSuppressesDuplicateAggregateAlert(t *testing.T)
}
}
// A config save re-judges every active alert. A filesystem alert must be judged
// by the threshold its evaluator used for that filesystem, not the host or guest
// default: saving any unrelated setting used to resolve an alert a per-disk
// threshold of 70 raised at 80 under a default of 90, notifying recovery, and
// the next poll raised it again as a new alert.
func TestConfigSaveJudgesFilesystemAlertsByTheirOwnThreshold(t *testing.T) {
perDisk := func() *HysteresisThreshold { return &HysteresisThreshold{Trigger: 70, Clear: 65} }
abovePerDisk := func() *HysteresisThreshold { return &HysteresisThreshold{Trigger: 85, Clear: 80} }
agentHost := func(device string) models.Host {
return models.Host{
ID: "host-fs", Hostname: "host-fs", Platform: "linux", Status: "online",
IntervalSeconds: 30, LastSeen: time.Now(),
Disks: []models.Disk{{Mountpoint: "/data", Device: device, Usage: 80, Total: 100, Used: 80, Free: 20}},
}
}
guestID := BuildGuestKey("pve1", "node1", 101)
guestVM := models.VM{
ID: guestID, VMID: 101, Name: "db", Node: "node1", Instance: "pve1", Status: "running",
Disks: []models.Disk{{Mountpoint: "/data", Device: "/dev/vdb1", Usage: 80, Total: 100, Used: 80, Free: 20}},
}
guestAlertID := canonicalMetricStateID(guestID+"-disk-data-dev-vdb1", "disk")
type diskCase struct {
name string
alertID string
configure func(cfg *AlertConfig)
evaluate func(m *Manager)
// raise lifts the threshold that judges this filesystem above its reading.
raise func(cfg *AlertConfig)
}
agentDiskID, _ := hostDiskResourceID(agentHost("/dev/sdb1"), agentHost("/dev/sdb1").Disks[0])
guestDiskOverrideID := guestDiskOverrideKey(guestVM, guestID, "/data-/dev/vdb1")
guestOverrideID := guestOverridePrimaryKey(guestVM, guestID)
cases := []diskCase{
{
name: "agent filesystem override",
alertID: canonicalMetricStateID(agentDiskID, "disk"),
configure: func(cfg *AlertConfig) {
cfg.Overrides = map[string]ThresholdConfig{agentDiskID: {Disk: perDisk()}}
},
evaluate: func(m *Manager) { m.CheckHost(agentHost("/dev/sdb1")) },
raise: func(cfg *AlertConfig) {
cfg.Overrides = map[string]ThresholdConfig{agentDiskID: {Disk: abovePerDisk()}}
},
},
{
name: "agent disk fill by hardware type",
alertID: canonicalMetricStateID(agentDiskID, "disk"),
configure: func(cfg *AlertConfig) {
cfg.DiskFillByType["nvme"] = *perDisk()
},
evaluate: func(m *Manager) { m.CheckHost(agentHost("/dev/nvme0n1p1")) },
raise: func(cfg *AlertConfig) {
cfg.DiskFillByType["nvme"] = *abovePerDisk()
},
},
{
name: "guest filesystem override",
alertID: guestAlertID,
configure: func(cfg *AlertConfig) {
cfg.Overrides = map[string]ThresholdConfig{guestDiskOverrideID: {Disk: perDisk()}}
},
evaluate: func(m *Manager) { m.CheckGuest(guestVM, "pve1") },
raise: func(cfg *AlertConfig) {
cfg.Overrides = map[string]ThresholdConfig{guestDiskOverrideID: {Disk: abovePerDisk()}}
},
},
{
name: "guest override on a filesystem alert",
alertID: guestAlertID,
configure: func(cfg *AlertConfig) {
cfg.Overrides = map[string]ThresholdConfig{guestOverrideID: {Disk: perDisk()}}
},
evaluate: func(m *Manager) { m.CheckGuest(guestVM, "pve1") },
raise: func(cfg *AlertConfig) {
cfg.Overrides = map[string]ThresholdConfig{guestOverrideID: {Disk: abovePerDisk()}}
},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
m := newTestManager(t)
cfg := m.GetConfig()
cfg.Enabled = true
// UpdateConfig fills missing delays with 5s; zero fires at once.
cfg.TimeThresholds = map[string]int{"agent": 0, "guest": 0}
cfg.AgentDefaults.Disk = &HysteresisThreshold{Trigger: 90, Clear: 85}
cfg.GuestDefaults.Disk = &HysteresisThreshold{Trigger: 90, Clear: 85}
tc.configure(&cfg)
m.UpdateConfig(cfg)
tc.evaluate(m)
startedAt := testRequireActiveAlert(t, m, tc.alertID).StartTime
m.UpdateConfig(m.GetConfig())
if !testHasActiveAlert(t, m, tc.alertID) {
t.Fatalf("an unchanged config save resolved %q, which its own threshold of 70 still holds at 80", tc.alertID)
}
tc.evaluate(m)
if got := testRequireActiveAlert(t, m, tc.alertID).StartTime; !got.Equal(startedAt) {
t.Fatalf("alert %q was raised again after the save: started %v, want %v", tc.alertID, got, startedAt)
}
cfg = m.GetConfig()
tc.raise(&cfg)
m.UpdateConfig(cfg)
if testHasActiveAlert(t, m, tc.alertID) {
t.Fatalf("a save lifting %q's own trigger above its reading kept the alert", tc.alertID)
}
})
}
}
// Regression: checkMetric stores canonical-identity alerts under the
// canonical state key, so hysteresis resolution must not remove only the
// unregistered legacy "<resourceID>-<metric>" ID.