mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-23 20:25:56 +00:00
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:
@@ -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
|
||||
|
||||
@@ -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
@@ -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
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user