From 6bb147c1bb0d5bdab54aaa841fbef4aa379e0a55 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Fri, 26 Jun 2026 00:22:59 +0100 Subject: [PATCH] Suppress powered-off alerts for guests not configured to autostart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The alert engine fired powered-off alerts for every stopped VM and container regardless of whether the guest was configured to autostart. This meant intentionally-stopped guests (onboot=0) generated alarm fatigue — in the live lab, 7 of 12 alerts were noise from stopped VMs that are deliberately off. Now the alert engine checks the Proxmox onboot setting before firing: - onboot=true → stopped is unexpected, alert (preserved behavior) - onboot=false → stopped is expected, suppress - onboot=nil → unknown, alert (preserved behavior) For containers, onboot is parsed from the already-fetched container config in enrichContainerMetadata (no new API call). For VMs, the config is fetched only for stopped VMs via GetVMConfig (one extra call per stopped VM, running VMs are unaffected). Models: add OnBoot *bool to VM and Container. Alerts: add onboot-aware suppression branch in CheckGuest. Monitoring: add parseProxmoxOnBoot helper and fetchVMOnBoot. --- internal/alerts/guest.go | 8 +++ internal/alerts/guest_snapshot.go | 7 ++- .../alerts/migration_characterization_test.go | 50 +++++++++++++++++++ internal/models/models.go | 2 + internal/monitoring/container_parsing.go | 26 ++++++++++ internal/monitoring/container_parsing_test.go | 42 ++++++++++++++++ internal/monitoring/monitor_pve.go | 5 ++ .../monitoring/monitor_pve_guest_builders.go | 26 ++++++++++ 8 files changed, 164 insertions(+), 2 deletions(-) diff --git a/internal/alerts/guest.go b/internal/alerts/guest.go index 64764f7be..31e2acab2 100644 --- a/internal/alerts/guest.go +++ b/internal/alerts/guest.go @@ -221,6 +221,14 @@ func (m *Manager) CheckGuest(guest any, instanceName string) { delete(m.offlineConfirmations, guestID) m.mu.Unlock() m.clearAlert(canonicalPoweredStateStateID(guestID)) + } else if snapshot.OnBoot != nil && !*snapshot.OnBoot { + // Guest is explicitly not configured to autostart (onboot=0). + // Being stopped is the expected state — suppress the alert. + m.mu.Lock() + delete(m.offlineConfirmations, guestID) + m.mu.Unlock() + m.clearAlert(canonicalPoweredStateStateID(guestID)) + m.clearGuestPoweredOffAlert(guestID, name) } else { m.mu.RLock() thresholds := m.getGuestThresholds(guest, guestID) diff --git a/internal/alerts/guest_snapshot.go b/internal/alerts/guest_snapshot.go index 05ab3bed7..91cc88f6c 100644 --- a/internal/alerts/guest_snapshot.go +++ b/internal/alerts/guest_snapshot.go @@ -32,8 +32,9 @@ type guestSnapshot struct { NetworkIn int64 NetworkOut int64 - Disks []models.Disk - Tags []string + Disks []models.Disk + Tags []string + OnBoot *bool } func emptyGuestSnapshot() guestSnapshot { @@ -106,6 +107,7 @@ func guestSnapshotFromVM(vm models.VM) guestSnapshot { NetworkOut: vm.NetworkOut, Disks: append([]models.Disk(nil), vm.Disks...), Tags: append([]string(nil), vm.Tags...), + OnBoot: vm.OnBoot, }.normalizeCollections() } @@ -127,6 +129,7 @@ func guestSnapshotFromContainer(container models.Container) guestSnapshot { NetworkOut: container.NetworkOut, Disks: append([]models.Disk(nil), container.Disks...), Tags: append([]string(nil), container.Tags...), + OnBoot: container.OnBoot, }.normalizeCollections() } diff --git a/internal/alerts/migration_characterization_test.go b/internal/alerts/migration_characterization_test.go index f0dd96d6e..845359979 100644 --- a/internal/alerts/migration_characterization_test.go +++ b/internal/alerts/migration_characterization_test.go @@ -373,6 +373,56 @@ func TestAlertCharacterizationDisableConnectivitySuppressesPoweredOffButNotMetri assertAlertPresent(t, m, canonicalMetricStateID(resourceID, "cpu")) } +func TestAlertCharacterizationOnBootFalseSuppressesPoweredOffAlert(t *testing.T) { + resourceID := BuildGuestKey("pve1", "node1", 101) + cfg := characterizationBaseConfig() + m := newCharacterizationManager(t, cfg) + + off := false + stopped := testVM(resourceID, 101, "app01", "node1", "pve1", "stopped", 0) + stopped.OnBoot = &off + + m.CheckGuest(stopped, "pve1") + m.CheckGuest(stopped, "pve1") + + assertAlertMissing(t, m, "guest-powered-off-"+resourceID) + + m.mu.RLock() + _, hasConfirmations := m.offlineConfirmations[resourceID] + m.mu.RUnlock() + if hasConfirmations { + t.Fatalf("expected no powered-off tracking for guest with onboot=false") + } +} + +func TestAlertCharacterizationOnBootTrueStillGeneratesPoweredOffAlert(t *testing.T) { + resourceID := BuildGuestKey("pve1", "node1", 101) + cfg := characterizationBaseConfig() + m := newCharacterizationManager(t, cfg) + + on := true + stopped := testVM(resourceID, 101, "app01", "node1", "pve1", "stopped", 0) + stopped.OnBoot = &on + + m.CheckGuest(stopped, "pve1") + m.CheckGuest(stopped, "pve1") + + assertAlertPresent(t, m, "guest-powered-off-"+resourceID) +} + +func TestAlertCharacterizationOnBootNilPreservesCurrentBehavior(t *testing.T) { + resourceID := BuildGuestKey("pve1", "node1", 101) + cfg := characterizationBaseConfig() + m := newCharacterizationManager(t, cfg) + + stopped := testVM(resourceID, 101, "app01", "node1", "pve1", "stopped", 0) + + m.CheckGuest(stopped, "pve1") + m.CheckGuest(stopped, "pve1") + + assertAlertPresent(t, m, "guest-powered-off-"+resourceID) +} + func TestAlertCharacterizationReevaluatesAlertsWhenConfigChanges(t *testing.T) { resourceID := BuildGuestKey("pve1", "node1", 101) alertID := canonicalMetricStateID(resourceID, "cpu") diff --git a/internal/models/models.go b/internal/models/models.go index 5d4eca6f1..85f80703d 100644 --- a/internal/models/models.go +++ b/internal/models/models.go @@ -160,6 +160,7 @@ type VM struct { DiskWrite int64 `json:"diskWrite"` Uptime int64 `json:"uptime"` Template bool `json:"template"` + OnBoot *bool `json:"onBoot,omitempty"` LastBackup time.Time `json:"lastBackup,omitempty"` Tags []string `json:"tags,omitempty"` Lock string `json:"lock,omitempty"` @@ -206,6 +207,7 @@ type Container struct { DiskWrite int64 `json:"diskWrite"` Uptime int64 `json:"uptime"` Template bool `json:"template"` + OnBoot *bool `json:"onBoot,omitempty"` LastBackup time.Time `json:"lastBackup,omitempty"` Tags []string `json:"tags,omitempty"` Lock string `json:"lock,omitempty"` diff --git a/internal/monitoring/container_parsing.go b/internal/monitoring/container_parsing.go index cadb35844..ac231709e 100644 --- a/internal/monitoring/container_parsing.go +++ b/internal/monitoring/container_parsing.go @@ -491,6 +491,32 @@ func extractContainerOSType(config map[string]interface{}) string { return ostype } +// parseProxmoxOnBoot extracts the onboot (autostart) setting from a Proxmox +// guest config map. Returns nil when the key is absent or unrecognised so +// callers can distinguish "explicitly off" from "unknown". +func parseProxmoxOnBoot(config map[string]interface{}) *bool { + if len(config) == 0 { + return nil + } + raw, ok := config["onboot"] + if !ok || raw == nil { + return nil + } + s := strings.TrimSpace(fmt.Sprint(raw)) + if s == "" { + return nil + } + if s == "1" || strings.EqualFold(s, "yes") || strings.EqualFold(s, "true") { + v := true + return &v + } + if s == "0" || strings.EqualFold(s, "no") || strings.EqualFold(s, "false") { + v := false + return &v + } + return nil +} + // extractContainerOSTemplate extracts the ostemplate value from container config. // This is the template used to create the container, which may be an LXC template // or an OCI image reference (Proxmox VE 9.1+). diff --git a/internal/monitoring/container_parsing_test.go b/internal/monitoring/container_parsing_test.go index a621ef05c..d872f3820 100644 --- a/internal/monitoring/container_parsing_test.go +++ b/internal/monitoring/container_parsing_test.go @@ -1953,3 +1953,45 @@ func TestIsOCITemplate(t *testing.T) { }) } } + +func TestParseProxmoxOnBoot(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + config map[string]interface{} + wantNil bool + wantVal bool + }{ + {name: "nil config", config: nil, wantNil: true}, + {name: "empty config", config: map[string]interface{}{}, wantNil: true}, + {name: "missing key", config: map[string]interface{}{"ostype": "debian"}, wantNil: true}, + {name: "onboot 1 (string)", config: map[string]interface{}{"onboot": "1"}, wantVal: true}, + {name: "onboot 0 (string)", config: map[string]interface{}{"onboot": "0"}, wantVal: false}, + {name: "onboot true", config: map[string]interface{}{"onboot": "true"}, wantVal: true}, + {name: "onboot false", config: map[string]interface{}{"onboot": "false"}, wantVal: false}, + {name: "onboot yes", config: map[string]interface{}{"onboot": "yes"}, wantVal: true}, + {name: "onboot no", config: map[string]interface{}{"onboot": "no"}, wantVal: false}, + {name: "onboot unrecognised", config: map[string]interface{}{"onboot": "maybe"}, wantNil: true}, + {name: "onboot empty string", config: map[string]interface{}{"onboot": ""}, wantNil: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := parseProxmoxOnBoot(tt.config) + if tt.wantNil { + if got != nil { + t.Errorf("parseProxmoxOnBoot() = %v, want nil", *got) + } + return + } + if got == nil { + t.Fatalf("parseProxmoxOnBoot() = nil, want %v", tt.wantVal) + } + if *got != tt.wantVal { + t.Errorf("parseProxmoxOnBoot() = %v, want %v", *got, tt.wantVal) + } + }) + } +} diff --git a/internal/monitoring/monitor_pve.go b/internal/monitoring/monitor_pve.go index 565b1fcc7..310ea2638 100644 --- a/internal/monitoring/monitor_pve.go +++ b/internal/monitoring/monitor_pve.go @@ -205,6 +205,11 @@ func (m *Monitor) enrichContainerMetadata(ctx context.Context, client PVEClientI if osName := extractContainerOSType(configData); osName != "" { container.OSName = osName } + // Extract onboot (autostart) setting so the alert engine can + // suppress powered-off alerts for guests not configured to autostart. + if onBoot := parseProxmoxOnBoot(configData); onBoot != nil { + container.OnBoot = onBoot + } // Detect OCI containers (Proxmox VE 9.1+) // Method 1: Check ostemplate for OCI registry patterns if osTemplate := extractContainerOSTemplate(configData); osTemplate != "" { diff --git a/internal/monitoring/monitor_pve_guest_builders.go b/internal/monitoring/monitor_pve_guest_builders.go index cb300b13b..b9c80c854 100644 --- a/internal/monitoring/monitor_pve_guest_builders.go +++ b/internal/monitoring/monitor_pve_guest_builders.go @@ -34,6 +34,7 @@ type vmBuildState struct { osVersion string agentVersion string detailedStatus *proxmox.VMStatus + onBoot *bool } func (m *Monitor) applyVMStatusDetails( @@ -285,6 +286,7 @@ func (m *Monitor) buildVMFromClusterResource( if res.Status != "running" { state.memorySource = "powered-off" state.memUsed = 0 + state.onBoot = m.fetchVMOnBoot(ctx, client, res.Node, res.VMID) } memFree := uint64(0) @@ -370,6 +372,7 @@ func (m *Monitor) buildVMFromClusterResource( DiskWrite: max(0, int64(diskWriteRate)), Uptime: int64(res.Uptime), Template: res.Template == 1, + OnBoot: state.onBoot, LastSeen: sampleTime, } @@ -393,6 +396,29 @@ func (m *Monitor) buildVMFromClusterResource( return vm, state.guestRaw, state.memorySource, snapshotNotes, sampleTime, true } +// fetchVMOnBoot retrieves the onboot (autostart) setting for a stopped VM by +// fetching its config. Returns nil when the config is unavailable or the +// onboot key is absent, so callers can distinguish "explicitly off" from +// "unknown". Uses a type assertion because PVEClientInterface does not include +// GetVMConfig (it is only on the concrete client, matching the pattern in +// guest_config.go). +func (m *Monitor) fetchVMOnBoot(ctx context.Context, client PVEClientInterface, node string, vmid int) *bool { + type vmConfigClient interface { + GetVMConfig(ctx context.Context, node string, vmid int) (map[string]interface{}, error) + } + vmClient, ok := client.(vmConfigClient) + if !ok { + return nil + } + configCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + configData, err := vmClient.GetVMConfig(configCtx, node, vmid) + if err != nil || len(configData) == 0 { + return nil + } + return parseProxmoxOnBoot(configData) +} + type vmFSInfoSummary struct { totalBytes uint64 usedBytes uint64