mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-24 04:33:17 +00:00
Start fresh installs with factory Docker alert thresholds (#2683)
A fresh install has no alerts.json, and LoadAlertConfig answered that
with its own AlertConfig literal. The literal never set DockerDefaults,
a
value struct, so container CPU, memory and disk loaded as trigger 0.
UpdateConfig keeps trigger 0 as a deliberate Off, so container metric
alerts never fired, the thresholds page showed the container Global
Defaults as Custom with every metric Off, and the startup identity
migration wrote those zeros to alerts.json a second after first boot.
The literal had also drifted from the manager's factory config on
flapping detection, alert TTL cleanup and the quiet-hours and
escalation templates.
The missing-file path now returns alerts.DefaultAlertConfig, the factory
constructor (now exported), so persistence keeps no second copy. It
leaves the activation state unset, as the literal did: node updates and
imports reapply LoadAlertConfig, and an explicit pending_review there
would send an active install back to pending review. Stored configs are
untouched, because trigger 0 is how a saved Off looks.
Two factory values would have reached installs for the first time
through this path, so they change with it. AutoAcknowledgeAfterHours
drops from 24 to 0: no settings control shows it, no install ever ran
it, and it would acknowledge still-firing alerts on the user's behalf.
The quiet-hours timezone drops from America/New_York to empty, which
keeps the settings page offering the viewer's browser zone.
Verification:
- Fresh data dir, non-mock and mock: GET /api/alerts/config on
origin/main
returns dockerDefaults cpu/memory/disk {0,0}; this branch returns
80/75, 85/80, 85/80, and alerts.json written at first boot carries them.
- Thresholds page, Docker > Containers on a fresh mock stack:
origin/main
shows Global Defaults "Custom" with CPU/Memory/Disk Off and every
container row Off; this branch shows the factory 80/85/85 with no
Custom badge, at 1440px and 390px.
- TestLoadAlertConfigFreshInstallUsesFactoryConfig fails against
origin/main's persistence.go and passes here;
TestLoadAlertConfigMissingFileKeepsRunningActivation and
TestLoadAlertConfigKeepsStoredDockerDefaultsOff pass on both.
- go test ./internal/alerts/... ./internal/config/... and
./internal/monitoring/ pass.
This commit is contained in:
@@ -1854,8 +1854,8 @@ compatibility aliases and wrapper functions for the leaf config package,
|
||||
`model.go` owns alert runtime data structures and clone semantics,
|
||||
`constants.go` owns package-wide cleanup and storage constants,
|
||||
`metric_hooks.go` owns Prometheus integration callbacks, `manager.go` owns
|
||||
Manager state and construction, `default_config.go` owns the default runtime
|
||||
configuration literal, `lifecycle.go` owns shutdown, and `escalation.go` owns
|
||||
Manager state and construction, `default_config.go` owns the factory runtime
|
||||
configuration (`DefaultAlertConfig`), `lifecycle.go` owns shutdown, and `escalation.go` owns
|
||||
the escalation loop and escalation state mutation. Future changes must extend
|
||||
the owning file rather than reintroducing a central catch-all manager file.
|
||||
Alert notification policy now lives in `internal/alerts/notification_policy.go`.
|
||||
@@ -1865,6 +1865,34 @@ per-alert rate limiting; future notification-gating changes should extend that
|
||||
policy owner rather than burying new checks inside metric or resource-specific
|
||||
evaluators.
|
||||
|
||||
### Fresh installs load the factory alert configuration
|
||||
|
||||
`DefaultAlertConfig` in `internal/alerts/default_config.go` is the only
|
||||
declaration of factory alert settings. A new Manager starts from it, and
|
||||
`LoadAlertConfig` in `internal/config/persistence.go` returns it while
|
||||
`alerts.json` does not exist; persistence must not declare a second copy. The
|
||||
copy it used to return left out `DockerDefaults`, a value struct, so a fresh
|
||||
install loaded container CPU, memory and disk as trigger 0, `UpdateConfig`
|
||||
kept that as a deliberate Off, the thresholds page called the container Global
|
||||
Defaults Custom, and the startup identity migration wrote those zeros to
|
||||
`alerts.json` within a second of first boot. The missing-file config carries
|
||||
no activation state, because activation is a stored decision: node updates and
|
||||
imports reapply `LoadAlertConfig`, and `UpdateConfig` then keeps a running
|
||||
manager's state, while a new manager starts pending review. An existing
|
||||
`alerts.json` is never rewritten to factory values, because a stored trigger 0
|
||||
is how a saved Off looks. The factory leaves `AutoAcknowledgeAfterHours` at 0: no settings
|
||||
control shows it, and the 24 hours it used to declare never reached an
|
||||
install. It leaves the quiet-hours timezone empty so the settings page offers
|
||||
the viewer's browser zone. Flapping detection and alert TTL cleanup are on in
|
||||
the factory and now reach fresh installs; a thresholds-page save still sends
|
||||
neither field, so it turns them off. `TestLoadAlertConfigFreshInstallUsesFactoryConfig`,
|
||||
`TestLoadAlertConfigMissingFileKeepsRunningActivation` and
|
||||
`TestLoadAlertConfigKeepsStoredDockerDefaultsOff` in
|
||||
`internal/config/persistence_alert_fresh_install_test.go`, plus
|
||||
`TestDefaultAlertConfigLeavesFiringAlertsUnacknowledged` and
|
||||
`TestDefaultAlertConfigLeavesQuietHoursTimezoneUnset` in
|
||||
`internal/alerts/alerts_test.go`, pin this.
|
||||
|
||||
### Agent disk temperature alerts clear when their threshold is off
|
||||
|
||||
Host-agent SMART disk temperature alerts (`diskTemperature`, resource
|
||||
|
||||
@@ -20538,8 +20538,8 @@ func TestSetAckRecordUsesCanonicalKeyWithoutMutatingLiveAlert(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestDefaultAlertConfigUsesIndependentBackupAlertOrphanedPointer(t *testing.T) {
|
||||
first := defaultAlertConfig()
|
||||
second := defaultAlertConfig()
|
||||
first := DefaultAlertConfig()
|
||||
second := DefaultAlertConfig()
|
||||
|
||||
if first.BackupDefaults.AlertOrphaned == nil {
|
||||
t.Fatal("first default backup AlertOrphaned is nil")
|
||||
@@ -20558,6 +20558,41 @@ func TestDefaultAlertConfigUsesIndependentBackupAlertOrphanedPointer(t *testing.
|
||||
}
|
||||
}
|
||||
|
||||
// A fresh install runs the factory configuration, and the settings page has no
|
||||
// auto-acknowledge control, so the factory must never acknowledge a firing
|
||||
// alert on the user's behalf.
|
||||
func TestDefaultAlertConfigLeavesFiringAlertsUnacknowledged(t *testing.T) {
|
||||
m := newTestManager(t)
|
||||
|
||||
now := time.Now()
|
||||
m.mu.Lock()
|
||||
m.activeAlerts["firing-alert"] = &Alert{
|
||||
ID: "firing-alert",
|
||||
StartTime: now.Add(-72 * time.Hour),
|
||||
LastSeen: now,
|
||||
}
|
||||
m.mu.Unlock()
|
||||
|
||||
m.Cleanup(time.Hour)
|
||||
|
||||
m.mu.RLock()
|
||||
alert := testRequireActiveAlert(t, m, "firing-alert")
|
||||
m.mu.RUnlock()
|
||||
if alert.Acknowledged {
|
||||
t.Fatalf("factory config acknowledged a firing alert as %q", alert.AckUser)
|
||||
}
|
||||
}
|
||||
|
||||
// The settings page offers the viewer's browser zone only while the stored
|
||||
// quiet-hours timezone is empty; a factory zone would replace it with one
|
||||
// fixed city for every fresh install.
|
||||
func TestDefaultAlertConfigLeavesQuietHoursTimezoneUnset(t *testing.T) {
|
||||
cfg := DefaultAlertConfig()
|
||||
if cfg.Schedule.QuietHours.Timezone != "" {
|
||||
t.Fatalf("factory quiet-hours timezone = %q, want unset", cfg.Schedule.QuietHours.Timezone)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdateConfigNormalizesNotificationDeliveryTargets(t *testing.T) {
|
||||
manager := NewManager()
|
||||
t.Cleanup(manager.Stop)
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
package alerts
|
||||
|
||||
func defaultAlertConfig() AlertConfig {
|
||||
// DefaultAlertConfig returns the factory alert configuration: the one a new
|
||||
// Manager starts with and the one a fresh install loads before alerts.json
|
||||
// exists. Callers that need factory values build them here instead of
|
||||
// declaring their own copy.
|
||||
func DefaultAlertConfig() AlertConfig {
|
||||
alertOrphaned := true
|
||||
return AlertConfig{
|
||||
Enabled: true,
|
||||
@@ -165,7 +169,7 @@ func defaultAlertConfig() AlertConfig {
|
||||
Enabled: false, // OFF - users should opt-in to quiet hours
|
||||
Start: "22:00",
|
||||
End: "08:00",
|
||||
Timezone: "America/New_York",
|
||||
Timezone: "", // Unset: the server's local zone; the settings page offers the browser's zone
|
||||
Days: map[string]bool{
|
||||
"monday": true,
|
||||
"tuesday": true,
|
||||
@@ -197,9 +201,9 @@ func defaultAlertConfig() AlertConfig {
|
||||
},
|
||||
},
|
||||
// Alert TTL defaults
|
||||
MaxAlertAgeDays: 7, // Cleanup unacknowledged alerts inactive for 7 days
|
||||
MaxAcknowledgedAgeDays: 1, // Auto-cleanup acknowledged alerts older than 1 day
|
||||
AutoAcknowledgeAfterHours: 24, // Auto-acknowledge alerts after 24 hours
|
||||
MaxAlertAgeDays: 7, // Cleanup unacknowledged alerts inactive for 7 days
|
||||
MaxAcknowledgedAgeDays: 1, // Auto-cleanup acknowledged alerts older than 1 day
|
||||
AutoAcknowledgeAfterHours: 0, // Off: a firing alert stays unacknowledged until someone acknowledges it
|
||||
// Flapping detection defaults
|
||||
FlappingEnabled: true, // Enable flapping detection
|
||||
FlappingWindowSeconds: 300, // 5 minute window
|
||||
|
||||
@@ -288,9 +288,9 @@ func TestNormalizeDiskFillByType(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestDefaultAlertConfigSeedsDiskFillByType(t *testing.T) {
|
||||
cfg := defaultAlertConfig()
|
||||
cfg := DefaultAlertConfig()
|
||||
if cfg.DiskFillByType == nil {
|
||||
t.Fatal("expected defaultAlertConfig to seed DiskFillByType, got nil")
|
||||
t.Fatal("expected DefaultAlertConfig to seed DiskFillByType, got nil")
|
||||
}
|
||||
if nvme, ok := cfg.DiskFillByType["nvme"]; !ok || nvme.Trigger != 92 || nvme.Clear != 87 {
|
||||
t.Fatalf("nvme = %+v ok=%v, want {Trigger:92 Clear:87}", nvme, ok)
|
||||
@@ -399,9 +399,9 @@ func TestNormalizeDiskTempByType(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestDefaultAlertConfigSeedsDiskTempByType(t *testing.T) {
|
||||
cfg := defaultAlertConfig()
|
||||
cfg := DefaultAlertConfig()
|
||||
if cfg.DiskTempByType == nil {
|
||||
t.Fatal("expected defaultAlertConfig to seed DiskTempByType, got nil")
|
||||
t.Fatal("expected DefaultAlertConfig to seed DiskTempByType, got nil")
|
||||
}
|
||||
if nvme, ok := cfg.DiskTempByType["nvme"]; !ok || nvme.Trigger != 70 || nvme.Clear != 65 {
|
||||
t.Fatalf("nvme = %+v ok=%v, want {Trigger:70 Clear:65}", nvme, ok)
|
||||
|
||||
@@ -1108,7 +1108,7 @@ func (m *Manager) HostDiskTemperatureThreshold(host DiskTemperatureHost, diskTyp
|
||||
// DefaultDiskTemperatureThreshold is DiskTemperatureThreshold under the
|
||||
// factory alert configuration, for callers with no alert manager.
|
||||
func DefaultDiskTemperatureThreshold(diskType string) *HysteresisThreshold {
|
||||
config := defaultAlertConfig()
|
||||
config := DefaultAlertConfig()
|
||||
return cloneThreshold(diskTemperatureThresholdForType(config.DiskTempByType, config.AgentDefaults.DiskTemperature, false, diskType))
|
||||
}
|
||||
|
||||
|
||||
@@ -251,7 +251,7 @@ func NewManagerWithDataDir(dataDir string, options ...ManagerOption) *Manager {
|
||||
nodeDisplayNames: make(map[string]string),
|
||||
instanceNodeDisplayNames: make(map[string]string),
|
||||
now: time.Now,
|
||||
config: defaultAlertConfig(),
|
||||
config: DefaultAlertConfig(),
|
||||
skipPersistedRestore: opts.skipPersistedAlertRestore,
|
||||
}
|
||||
intentClockEpoch := time.Now()
|
||||
|
||||
@@ -979,70 +979,17 @@ func (c *ConfigPersistence) LoadAlertConfig() (*alerts.AlertConfig, error) {
|
||||
c.mu.RLock()
|
||||
defer c.mu.RUnlock()
|
||||
|
||||
alertOrphaned := true
|
||||
data, err := c.fs.ReadFile(c.alertFile)
|
||||
if err != nil {
|
||||
if os.IsNotExist(err) {
|
||||
// Return default config if file doesn't exist
|
||||
return &alerts.AlertConfig{
|
||||
Enabled: true,
|
||||
GuestDefaults: alerts.ThresholdConfig{
|
||||
CPU: &alerts.HysteresisThreshold{Trigger: 80, Clear: 75},
|
||||
Memory: &alerts.HysteresisThreshold{Trigger: 85, Clear: 80},
|
||||
Disk: &alerts.HysteresisThreshold{Trigger: 90, Clear: 85},
|
||||
},
|
||||
NodeDefaults: alerts.ThresholdConfig{
|
||||
CPU: &alerts.HysteresisThreshold{Trigger: 80, Clear: 75},
|
||||
Memory: &alerts.HysteresisThreshold{Trigger: 85, Clear: 80},
|
||||
Disk: &alerts.HysteresisThreshold{Trigger: 90, Clear: 85},
|
||||
Temperature: &alerts.HysteresisThreshold{Trigger: 80, Clear: 75},
|
||||
},
|
||||
AgentDefaults: alerts.ThresholdConfig{
|
||||
CPU: &alerts.HysteresisThreshold{Trigger: 80, Clear: 75},
|
||||
Memory: &alerts.HysteresisThreshold{Trigger: 85, Clear: 80},
|
||||
Disk: &alerts.HysteresisThreshold{Trigger: 90, Clear: 85},
|
||||
},
|
||||
Schedule: alerts.ScheduleConfig{
|
||||
Cooldown: 5,
|
||||
MaxAlertsHour: 10,
|
||||
InitialNotify: "all",
|
||||
NotifyOnResolve: true,
|
||||
Grouping: alerts.GroupingConfig{
|
||||
Enabled: true,
|
||||
Window: 30,
|
||||
ByNode: true,
|
||||
ByGuest: false,
|
||||
},
|
||||
},
|
||||
StorageDefault: alerts.HysteresisThreshold{Trigger: 85, Clear: 80},
|
||||
TimeThresholds: map[string]int{
|
||||
"guest": 5,
|
||||
"node": 5,
|
||||
"agent": 5,
|
||||
"storage": 5,
|
||||
"pbs": 5,
|
||||
},
|
||||
MinimumDelta: 2.0,
|
||||
SuppressionWindow: 5,
|
||||
HysteresisMargin: 5.0,
|
||||
SnapshotDefaults: alerts.SnapshotAlertConfig{
|
||||
Enabled: false,
|
||||
WarningDays: 30,
|
||||
CriticalDays: 45,
|
||||
WarningSizeGiB: 0,
|
||||
CriticalSizeGiB: 0,
|
||||
},
|
||||
BackupDefaults: alerts.BackupAlertConfig{
|
||||
Enabled: false,
|
||||
WarningDays: 7,
|
||||
CriticalDays: 14,
|
||||
FreshHours: 24,
|
||||
StaleHours: 72,
|
||||
AlertOrphaned: &alertOrphaned,
|
||||
IgnoreVMIDs: []string{},
|
||||
},
|
||||
Overrides: make(map[string]alerts.ThresholdConfig),
|
||||
}, nil
|
||||
// A fresh install has no alerts.json yet. It starts from the alert
|
||||
// manager's factory configuration; a second literal here left out
|
||||
// Docker container thresholds, so they loaded as Off. Activation
|
||||
// is a stored decision, not a factory value: leaving it unset lets
|
||||
// UpdateConfig keep a running manager's state.
|
||||
config := alerts.DefaultAlertConfig()
|
||||
config.ActivationState = ""
|
||||
return &config, nil
|
||||
}
|
||||
return nil, err
|
||||
}
|
||||
|
||||
@@ -0,0 +1,82 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/alerts"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// A fresh install has no alerts.json. It must load the alert manager's
|
||||
// factory configuration rather than a copy declared in persistence: the copy
|
||||
// left out Docker container thresholds, so container CPU, memory and disk
|
||||
// alerts started Off and the settings page called them Custom.
|
||||
func TestLoadAlertConfigFreshInstallUsesFactoryConfig(t *testing.T) {
|
||||
persistence := NewConfigPersistence(t.TempDir())
|
||||
|
||||
loaded, err := persistence.LoadAlertConfig()
|
||||
require.NoError(t, err)
|
||||
factory := alerts.DefaultAlertConfig()
|
||||
factory.ActivationState = ""
|
||||
require.Equal(t, factory, *loaded)
|
||||
|
||||
manager := alerts.NewManagerWithDataDir(t.TempDir())
|
||||
defer manager.Stop()
|
||||
manager.UpdateConfig(*loaded)
|
||||
|
||||
applied := manager.GetConfig()
|
||||
require.Equal(t, alerts.ActivationPending, applied.ActivationState)
|
||||
docker := applied.DockerDefaults
|
||||
require.Equal(t, alerts.HysteresisThreshold{Trigger: 80, Clear: 75}, docker.CPU)
|
||||
require.Equal(t, alerts.HysteresisThreshold{Trigger: 85, Clear: 80}, docker.Memory)
|
||||
require.Equal(t, alerts.HysteresisThreshold{Trigger: 85, Clear: 80}, docker.Disk)
|
||||
}
|
||||
|
||||
// Node updates and imports reapply LoadAlertConfig to the running manager. A
|
||||
// missing alerts.json holds no activation decision, so it must not send an
|
||||
// active install back to pending review.
|
||||
func TestLoadAlertConfigMissingFileKeepsRunningActivation(t *testing.T) {
|
||||
persistence := NewConfigPersistence(t.TempDir())
|
||||
|
||||
manager := alerts.NewManagerWithDataDir(t.TempDir())
|
||||
defer manager.Stop()
|
||||
active := manager.GetConfig()
|
||||
activatedAt := time.Now().Add(-time.Hour)
|
||||
active.ActivationState = alerts.ActivationActive
|
||||
active.ActivationTime = &activatedAt
|
||||
manager.UpdateConfig(active)
|
||||
|
||||
loaded, err := persistence.LoadAlertConfig()
|
||||
require.NoError(t, err)
|
||||
manager.UpdateConfig(*loaded)
|
||||
|
||||
applied := manager.GetConfig()
|
||||
require.Equal(t, alerts.ActivationActive, applied.ActivationState)
|
||||
require.NotNil(t, applied.ActivationTime)
|
||||
require.True(t, applied.ActivationTime.Equal(activatedAt))
|
||||
}
|
||||
|
||||
// Trigger 0 is how a saved Off is stored, so a stored all-zero Docker default
|
||||
// stays Off; only the missing-file path takes factory values.
|
||||
func TestLoadAlertConfigKeepsStoredDockerDefaultsOff(t *testing.T) {
|
||||
persistence := NewConfigPersistence(t.TempDir())
|
||||
|
||||
stored := alerts.DefaultAlertConfig()
|
||||
stored.DockerDefaults.CPU = alerts.HysteresisThreshold{}
|
||||
stored.DockerDefaults.Memory = alerts.HysteresisThreshold{}
|
||||
stored.DockerDefaults.Disk = alerts.HysteresisThreshold{}
|
||||
require.NoError(t, persistence.SaveAlertConfig(stored))
|
||||
|
||||
loaded, err := persistence.LoadAlertConfig()
|
||||
require.NoError(t, err)
|
||||
|
||||
manager := alerts.NewManagerWithDataDir(t.TempDir())
|
||||
defer manager.Stop()
|
||||
manager.UpdateConfig(*loaded)
|
||||
|
||||
docker := manager.GetConfig().DockerDefaults
|
||||
require.Equal(t, alerts.HysteresisThreshold{}, docker.CPU)
|
||||
require.Equal(t, alerts.HysteresisThreshold{}, docker.Memory)
|
||||
require.Equal(t, alerts.HysteresisThreshold{}, docker.Disk)
|
||||
}
|
||||
Reference in New Issue
Block a user