Add host alert deduplication with tests
- Modified alerts.go for improved host alert handling - Added host_dedup_test.go for deduplication test coverage
This commit is contained in:
rcourtman
parent
721f973271
commit
190e02fbbc
2 changed files with 259 additions and 10 deletions
|
|
@ -516,6 +516,10 @@ type Manager struct {
|
|||
flappingActive map[string]bool // Track which alerts are currently in flapping state
|
||||
// Cleanup control
|
||||
cleanupStop chan struct{} // Signal to stop cleanup goroutine
|
||||
// Host agent deduplication: track hostnames of active host agents
|
||||
// When a host agent is running on a Proxmox node, we prefer the host agent
|
||||
// alerts and suppress the node alerts to avoid duplicate monitoring.
|
||||
hostAgentHostnames map[string]struct{} // Normalized hostnames (lowercase)
|
||||
}
|
||||
|
||||
type ackRecord struct {
|
||||
|
|
@ -555,6 +559,7 @@ func NewManager() *Manager {
|
|||
flappingHistory: make(map[string][]time.Time),
|
||||
flappingActive: make(map[string]bool),
|
||||
cleanupStop: make(chan struct{}),
|
||||
hostAgentHostnames: make(map[string]struct{}),
|
||||
config: AlertConfig{
|
||||
Enabled: true,
|
||||
ActivationState: ActivationPending,
|
||||
|
|
@ -2345,22 +2350,77 @@ func (m *Manager) CheckNode(node models.Node) {
|
|||
|
||||
// Check each metric (only if node is online) - checkMetric will skip if threshold is nil or <= 0
|
||||
if node.Status != "offline" {
|
||||
m.checkMetric(node.ID, node.Name, node.Name, node.Instance, "Node", "cpu", node.CPU*100, thresholds.CPU, nil)
|
||||
m.checkMetric(node.ID, node.Name, node.Name, node.Instance, "Node", "memory", node.Memory.Usage, thresholds.Memory, nil)
|
||||
m.checkMetric(node.ID, node.Name, node.Name, node.Instance, "Node", "disk", node.Disk.Usage, thresholds.Disk, nil)
|
||||
// Check for host agent deduplication: if a host agent is running on this node,
|
||||
// prefer the host agent alerts and skip node metric alerts to avoid duplicates.
|
||||
if m.hasHostAgentForNode(node.Name) {
|
||||
log.Debug().
|
||||
Str("node", node.Name).
|
||||
Msg("Skipping node metric alerts - host agent is monitoring this machine")
|
||||
} else {
|
||||
m.checkMetric(node.ID, node.Name, node.Name, node.Instance, "Node", "cpu", node.CPU*100, thresholds.CPU, nil)
|
||||
m.checkMetric(node.ID, node.Name, node.Name, node.Instance, "Node", "memory", node.Memory.Usage, thresholds.Memory, nil)
|
||||
m.checkMetric(node.ID, node.Name, node.Name, node.Instance, "Node", "disk", node.Disk.Usage, thresholds.Disk, nil)
|
||||
|
||||
// Check temperature if available
|
||||
if node.Temperature != nil && node.Temperature.Available && thresholds.Temperature != nil {
|
||||
// Use CPU package temp if available, otherwise use max core temp
|
||||
temp := node.Temperature.CPUPackage
|
||||
if temp == 0 {
|
||||
temp = node.Temperature.CPUMax
|
||||
// Check temperature if available
|
||||
if node.Temperature != nil && node.Temperature.Available && thresholds.Temperature != nil {
|
||||
// Use CPU package temp if available, otherwise use max core temp
|
||||
temp := node.Temperature.CPUPackage
|
||||
if temp == 0 {
|
||||
temp = node.Temperature.CPUMax
|
||||
}
|
||||
m.checkMetric(node.ID, node.Name, node.Name, node.Instance, "Node", "temperature", temp, thresholds.Temperature, nil)
|
||||
}
|
||||
m.checkMetric(node.ID, node.Name, node.Name, node.Instance, "Node", "temperature", temp, thresholds.Temperature, nil)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// RegisterHostAgentHostname registers a host agent hostname for deduplication.
|
||||
// When a host agent is actively monitoring a machine, we prefer its alerts
|
||||
// over Proxmox node alerts to avoid duplicate monitoring of the same machine.
|
||||
func (m *Manager) RegisterHostAgentHostname(hostname string) {
|
||||
normalized := strings.ToLower(strings.TrimSpace(hostname))
|
||||
if normalized == "" {
|
||||
return
|
||||
}
|
||||
m.mu.Lock()
|
||||
m.hostAgentHostnames[normalized] = struct{}{}
|
||||
m.mu.Unlock()
|
||||
|
||||
log.Debug().
|
||||
Str("hostname", hostname).
|
||||
Msg("Registered host agent hostname for deduplication")
|
||||
}
|
||||
|
||||
// UnregisterHostAgentHostname removes a host agent hostname from deduplication tracking.
|
||||
func (m *Manager) UnregisterHostAgentHostname(hostname string) {
|
||||
normalized := strings.ToLower(strings.TrimSpace(hostname))
|
||||
if normalized == "" {
|
||||
return
|
||||
}
|
||||
m.mu.Lock()
|
||||
delete(m.hostAgentHostnames, normalized)
|
||||
m.mu.Unlock()
|
||||
|
||||
log.Debug().
|
||||
Str("hostname", hostname).
|
||||
Msg("Unregistered host agent hostname from deduplication")
|
||||
}
|
||||
|
||||
// hasHostAgentForNode checks if a host agent is monitoring a machine with the same
|
||||
// hostname as the given Proxmox node. If so, we should suppress node alerts to
|
||||
// avoid duplicate alerting.
|
||||
func (m *Manager) hasHostAgentForNode(nodeName string) bool {
|
||||
normalized := strings.ToLower(strings.TrimSpace(nodeName))
|
||||
if normalized == "" {
|
||||
return false
|
||||
}
|
||||
m.mu.RLock()
|
||||
_, exists := m.hostAgentHostnames[normalized]
|
||||
m.mu.RUnlock()
|
||||
return exists
|
||||
}
|
||||
|
||||
|
||||
func hostResourceID(hostID string) string {
|
||||
trimmed := strings.TrimSpace(hostID)
|
||||
if trimmed == "" {
|
||||
|
|
@ -2449,6 +2509,12 @@ func (m *Manager) CheckHost(host models.Host) {
|
|||
return
|
||||
}
|
||||
|
||||
// Register this host agent hostname for deduplication with Proxmox nodes.
|
||||
// This prevents duplicate alerts when both a Node and Host agent monitor the same machine.
|
||||
if host.Hostname != "" {
|
||||
m.RegisterHostAgentHostname(host.Hostname)
|
||||
}
|
||||
|
||||
// Fresh telemetry marks the host as online and clears offline tracking.
|
||||
m.HandleHostOnline(host)
|
||||
|
||||
|
|
@ -2697,6 +2763,11 @@ func (m *Manager) HandleHostRemoved(host models.Host) {
|
|||
return
|
||||
}
|
||||
|
||||
// Unregister the host agent hostname since it's being removed.
|
||||
if host.Hostname != "" {
|
||||
m.UnregisterHostAgentHostname(host.Hostname)
|
||||
}
|
||||
|
||||
m.HandleHostOnline(host)
|
||||
m.clearHostMetricAlerts(host.ID)
|
||||
m.clearHostDiskAlerts(host.ID)
|
||||
|
|
@ -2708,6 +2779,12 @@ func (m *Manager) HandleHostOffline(host models.Host) {
|
|||
return
|
||||
}
|
||||
|
||||
// Unregister the host agent hostname since it's no longer actively monitoring.
|
||||
// This allows node alerts to resume if a Proxmox node with the same hostname exists.
|
||||
if host.Hostname != "" {
|
||||
m.UnregisterHostAgentHostname(host.Hostname)
|
||||
}
|
||||
|
||||
m.mu.RLock()
|
||||
if !m.config.Enabled {
|
||||
m.mu.RUnlock()
|
||||
|
|
|
|||
172
internal/alerts/host_dedup_test.go
Normal file
172
internal/alerts/host_dedup_test.go
Normal file
|
|
@ -0,0 +1,172 @@
|
|||
package alerts
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/models"
|
||||
)
|
||||
|
||||
func TestHostAgentDeduplicatesNodeAlerts(t *testing.T) {
|
||||
// Test 1: Without a host agent registered, node metrics ARE checked
|
||||
t.Run("node_metrics_checked_without_host_agent", func(t *testing.T) {
|
||||
m := NewManager()
|
||||
m.config.Enabled = true
|
||||
m.config.NodeDefaults.CPU = &HysteresisThreshold{Trigger: 80, Clear: 75}
|
||||
|
||||
// Verify no host agent is registered
|
||||
if m.hasHostAgentForNode("pi") {
|
||||
t.Error("Expected no host agent for 'pi' initially")
|
||||
}
|
||||
|
||||
node := models.Node{
|
||||
ID: "node/pi",
|
||||
Name: "pi",
|
||||
Status: "online",
|
||||
CPU: 0.95, // 95% CPU
|
||||
}
|
||||
|
||||
// This should attempt to check metrics (even if alert doesn't fire immediately due to time thresholds)
|
||||
m.CheckNode(node)
|
||||
|
||||
// The key test: pendingAlerts should have an entry because metrics WERE checked
|
||||
m.mu.RLock()
|
||||
_, hasPending := m.pendingAlerts["node/pi-cpu"]
|
||||
m.mu.RUnlock()
|
||||
|
||||
if !hasPending {
|
||||
t.Error("Expected pending alert for node CPU when no host agent registered")
|
||||
}
|
||||
})
|
||||
|
||||
// Test 2: With host agent registered, node metrics are NOT checked
|
||||
t.Run("node_metrics_skipped_with_host_agent", func(t *testing.T) {
|
||||
m := NewManager()
|
||||
m.config.Enabled = true
|
||||
m.config.NodeDefaults.CPU = &HysteresisThreshold{Trigger: 80, Clear: 75}
|
||||
|
||||
// Register a host agent with the same hostname BEFORE checking the node
|
||||
m.RegisterHostAgentHostname("pi")
|
||||
|
||||
// Verify host agent IS registered
|
||||
if !m.hasHostAgentForNode("pi") {
|
||||
t.Error("Expected host agent for 'pi' to be registered")
|
||||
}
|
||||
|
||||
node := models.Node{
|
||||
ID: "node/pi",
|
||||
Name: "pi",
|
||||
Status: "online",
|
||||
CPU: 0.95, // 95% CPU
|
||||
}
|
||||
|
||||
m.CheckNode(node)
|
||||
|
||||
// The key test: pendingAlerts should NOT have an entry because metrics were SKIPPED
|
||||
m.mu.RLock()
|
||||
_, hasPending := m.pendingAlerts["node/pi-cpu"]
|
||||
m.mu.RUnlock()
|
||||
|
||||
if hasPending {
|
||||
t.Error("Expected NO pending alert for node CPU when host agent is registered")
|
||||
}
|
||||
})
|
||||
|
||||
// Test 3: After unregistering host agent, node metrics ARE checked again
|
||||
t.Run("node_metrics_resume_after_host_agent_unregistered", func(t *testing.T) {
|
||||
m := NewManager()
|
||||
m.config.Enabled = true
|
||||
m.config.NodeDefaults.CPU = &HysteresisThreshold{Trigger: 80, Clear: 75}
|
||||
|
||||
// Register and then unregister
|
||||
m.RegisterHostAgentHostname("pi")
|
||||
m.UnregisterHostAgentHostname("pi")
|
||||
|
||||
// Verify host agent is NOT registered
|
||||
if m.hasHostAgentForNode("pi") {
|
||||
t.Error("Expected host agent for 'pi' to be unregistered")
|
||||
}
|
||||
|
||||
node := models.Node{
|
||||
ID: "node/pi",
|
||||
Name: "pi",
|
||||
Status: "online",
|
||||
CPU: 0.95, // 95% CPU
|
||||
}
|
||||
|
||||
m.CheckNode(node)
|
||||
|
||||
// The key test: pendingAlerts should have an entry because metrics WERE checked
|
||||
m.mu.RLock()
|
||||
_, hasPending := m.pendingAlerts["node/pi-cpu"]
|
||||
m.mu.RUnlock()
|
||||
|
||||
if !hasPending {
|
||||
t.Error("Expected pending alert for node CPU after host agent unregistration")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestHostAgentDeduplicationCaseInsensitive(t *testing.T) {
|
||||
m := NewManager()
|
||||
|
||||
// Register with lowercase
|
||||
m.RegisterHostAgentHostname("myhost")
|
||||
|
||||
// Check should match regardless of case
|
||||
if !m.hasHostAgentForNode("MYHOST") {
|
||||
t.Error("Expected hasHostAgentForNode to match uppercase hostname")
|
||||
}
|
||||
if !m.hasHostAgentForNode("MyHost") {
|
||||
t.Error("Expected hasHostAgentForNode to match mixed-case hostname")
|
||||
}
|
||||
if !m.hasHostAgentForNode("myhost") {
|
||||
t.Error("Expected hasHostAgentForNode to match lowercase hostname")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckHostRegistersHostname(t *testing.T) {
|
||||
m := NewManager()
|
||||
m.config.Enabled = true
|
||||
|
||||
host := models.Host{
|
||||
ID: "host-test-123",
|
||||
Hostname: "testhost",
|
||||
CPUUsage: 50,
|
||||
}
|
||||
|
||||
// Initially no hostname registered
|
||||
if m.hasHostAgentForNode("testhost") {
|
||||
t.Error("Expected no host agent registered initially")
|
||||
}
|
||||
|
||||
// CheckHost should register the hostname
|
||||
m.CheckHost(host)
|
||||
|
||||
if !m.hasHostAgentForNode("testhost") {
|
||||
t.Error("Expected host agent hostname to be registered after CheckHost")
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleHostOfflineUnregistersHostname(t *testing.T) {
|
||||
m := NewManager()
|
||||
m.config.Enabled = true
|
||||
|
||||
host := models.Host{
|
||||
ID: "host-test-456",
|
||||
Hostname: "offlinehost",
|
||||
}
|
||||
|
||||
// Register the hostname
|
||||
m.RegisterHostAgentHostname(host.Hostname)
|
||||
|
||||
if !m.hasHostAgentForNode("offlinehost") {
|
||||
t.Error("Expected host agent registered")
|
||||
}
|
||||
|
||||
// HandleHostOffline should unregister the hostname
|
||||
m.HandleHostOffline(host)
|
||||
|
||||
if m.hasHostAgentForNode("offlinehost") {
|
||||
t.Error("Expected host agent to be unregistered after HandleHostOffline")
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue