From d70cf862ec9c7cd9d8d133193bf723f33f406511 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Thu, 11 Jun 2026 10:02:52 +0100 Subject: [PATCH] fix(monitoring): keep unreachable PVE instances visible with offline nodes The pollPVEInstance error path returned without touching node state, so a host that stopped answering kept showing its last online snapshot forever (#1441, reported against v5 in #1135), and a host that was already down when Pulse started never appeared on the dashboard at all (#1433). Route the poll error through the same grace policy as the existing empty-node fallback: nodes seen within the grace window keep their prior state with degraded connection health, then flip to offline with cleared uptime and CPU once the window lapses. An instance with no node state yet gets the synthesized offline entry from removeFailedPVENode so it still shows on the dashboard. Backport of the v6 fixes 02955619f and 8372a22c5. --- internal/monitoring/monitor.go | 46 +++++ .../monitor_pve_unreachable_test.go | 160 ++++++++++++++++++ 2 files changed, 206 insertions(+) create mode 100644 internal/monitoring/monitor_pve_unreachable_test.go diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index cf40bef83..c25988a92 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -6801,6 +6801,7 @@ func (m *Monitor) pollPVEInstance(ctx context.Context, instanceName string, clie pollErr = monErr log.Error().Err(monErr).Str("instance", instanceName).Msg("Failed to get nodes") m.state.SetConnectionHealth(instanceName, false) + m.markPVEInstanceNodesUnreachable(instanceName) // Track auth failure if it's an authentication error if errors.IsAuthError(err) { @@ -11600,6 +11601,51 @@ func (m *Monitor) resetAuthFailures(instanceName string, nodeType string) { } } +// markPVEInstanceNodesUnreachable keeps a PVE instance visible when the whole +// instance stops answering (host shut down, network loss, connection refused). +// Previously the poll error path returned without touching node state, so a +// host that went dark kept showing its last online snapshot forever, and a +// host that was already down when Pulse started never appeared at all. Nodes +// seen within the offline grace window keep their prior state with degraded +// connection health (mirroring the empty-node fallback in pollPVEInstance); +// past the window they flip to offline. An instance with no node state yet +// gets the synthesized offline entry from removeFailedPVENode so it still +// shows on the dashboard. +func (m *Monitor) markPVEInstanceNodesUnreachable(instanceName string) { + prevState := m.GetState() + prevInstanceNodes := make([]models.Node, 0) + for _, existingNode := range prevState.Nodes { + if existingNode.Instance == instanceName { + prevInstanceNodes = append(prevInstanceNodes, existingNode) + } + } + + if len(prevInstanceNodes) == 0 { + m.removeFailedPVENode(instanceName) + return + } + + preserved := make([]models.Node, 0, len(prevInstanceNodes)) + now := time.Now() + for _, prevNode := range prevInstanceNodes { + nodeCopy := prevNode + + if !nodeCopy.LastSeen.IsZero() && now.Sub(nodeCopy.LastSeen) < nodeOfflineGracePeriod { + if nodeCopy.ConnectionHealth == "" || nodeCopy.ConnectionHealth == "healthy" { + nodeCopy.ConnectionHealth = "degraded" + } + } else { + nodeCopy.Status = "offline" + nodeCopy.ConnectionHealth = "error" + nodeCopy.Uptime = 0 + nodeCopy.CPU = 0 + } + + preserved = append(preserved, nodeCopy) + } + m.state.UpdateNodesForInstance(instanceName, preserved) +} + // removeFailedPVENode updates a PVE node to show failed authentication status func (m *Monitor) removeFailedPVENode(instanceName string) { // Get instance config to get host URL diff --git a/internal/monitoring/monitor_pve_unreachable_test.go b/internal/monitoring/monitor_pve_unreachable_test.go new file mode 100644 index 000000000..eda38d17c --- /dev/null +++ b/internal/monitoring/monitor_pve_unreachable_test.go @@ -0,0 +1,160 @@ +package monitoring + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/models" + "github.com/rcourtman/pulse-go-rewrite/pkg/proxmox" +) + +type unreachablePVEClient struct { + stubPVEClient +} + +func (c *unreachablePVEClient) GetNodes(ctx context.Context) ([]proxmox.Node, error) { + return nil, fmt.Errorf("dial tcp: i/o timeout") +} + +// A host that is already down when Pulse starts has no node state, so the +// poll error path must synthesize an offline entry from the instance config +// instead of leaving the configured instance invisible until its first +// successful poll. +func TestPollPVEInstanceSynthesizesOfflineNodeWhenNeverSeen(t *testing.T) { + t.Setenv("PULSE_DATA_DIR", t.TempDir()) + + mon := newTestPVEMonitor("test") + defer mon.alertManager.Stop() + defer mon.notificationMgr.Stop() + + mon.pollPVEInstance(context.Background(), "test", &unreachablePVEClient{}) + + snapshot := mon.state.GetSnapshot() + if len(snapshot.Nodes) != 1 { + t.Fatalf("expected one synthesized node in state, got %d", len(snapshot.Nodes)) + } + + node := snapshot.Nodes[0] + if node.Status != "offline" { + t.Fatalf("expected synthesized node offline, got %q", node.Status) + } + if node.ConnectionHealth != "error" { + t.Fatalf("expected synthesized node connection health error, got %q", node.ConnectionHealth) + } + if node.Name != "test" { + t.Fatalf("expected synthesized node named after instance, got %q", node.Name) + } + if node.Host != "https://pve" { + t.Fatalf("expected synthesized node to carry configured host, got %q", node.Host) + } +} + +// A node seen online recently rides out an unreachable poll inside the grace +// window with its prior status and degraded connection health. +func TestPollPVEInstanceKeepsRecentNodesThroughUnreachablePoll(t *testing.T) { + t.Setenv("PULSE_DATA_DIR", t.TempDir()) + + client := &stubPVEClient{ + nodes: []proxmox.Node{ + { + Node: "node1", + Status: "online", + CPU: 0.10, + MaxCPU: 8, + Mem: 4 * 1024 * 1024 * 1024, + MaxMem: 8 * 1024 * 1024 * 1024, + Uptime: 7200, + }, + }, + nodeStatus: &proxmox.NodeStatus{ + Memory: &proxmox.MemoryStatus{ + Total: 8 * 1024 * 1024 * 1024, + Used: 4 * 1024 * 1024 * 1024, + Free: 4 * 1024 * 1024 * 1024, + }, + }, + } + + mon := newTestPVEMonitor("test") + defer mon.alertManager.Stop() + defer mon.notificationMgr.Stop() + + mon.pollPVEInstance(context.Background(), "test", client) + + mon.pollPVEInstance(context.Background(), "test", &unreachablePVEClient{}) + + snapshot := mon.state.GetSnapshot() + if len(snapshot.Nodes) != 1 { + t.Fatalf("expected one node in state, got %d", len(snapshot.Nodes)) + } + + node := snapshot.Nodes[0] + if node.Status != "online" { + t.Fatalf("expected recent node to remain online during grace window, got %q", node.Status) + } + if node.ConnectionHealth != "degraded" { + t.Fatalf("expected recent node connection health degraded, got %q", node.ConnectionHealth) + } +} + +// Once the grace window lapses, an unreachable instance's nodes flip to +// offline instead of freezing at their last online snapshot forever. +func TestPollPVEInstanceMarksStaleNodesOfflineWhenUnreachable(t *testing.T) { + t.Setenv("PULSE_DATA_DIR", t.TempDir()) + + client := &stubPVEClient{ + nodes: []proxmox.Node{ + { + Node: "node1", + Status: "online", + CPU: 0.10, + MaxCPU: 8, + Mem: 4 * 1024 * 1024 * 1024, + MaxMem: 8 * 1024 * 1024 * 1024, + Uptime: 7200, + }, + }, + nodeStatus: &proxmox.NodeStatus{ + Memory: &proxmox.MemoryStatus{ + Total: 8 * 1024 * 1024 * 1024, + Used: 4 * 1024 * 1024 * 1024, + Free: 4 * 1024 * 1024 * 1024, + }, + }, + } + + mon := newTestPVEMonitor("test") + defer mon.alertManager.Stop() + defer mon.notificationMgr.Stop() + + mon.pollPVEInstance(context.Background(), "test", client) + + first := mon.state.GetSnapshot() + if len(first.Nodes) != 1 { + t.Fatalf("expected one node after first poll, got %d", len(first.Nodes)) + } + + staleNode := first.Nodes[0] + staleNode.LastSeen = time.Now().Add(-nodeOfflineGracePeriod - 2*time.Second) + mon.state.UpdateNodesForInstance("test", []models.Node{staleNode}) + + mon.pollPVEInstance(context.Background(), "test", &unreachablePVEClient{}) + + second := mon.state.GetSnapshot() + if len(second.Nodes) != 1 { + t.Fatalf("expected one node after unreachable poll, got %d", len(second.Nodes)) + } + + node := second.Nodes[0] + if node.Status != "offline" { + t.Fatalf("expected stale node to be marked offline, got %q", node.Status) + } + if node.ConnectionHealth != "error" { + t.Fatalf("expected stale node connection health error, got %q", node.ConnectionHealth) + } + if node.Uptime != 0 { + t.Fatalf("expected stale node uptime cleared, got %d", node.Uptime) + } +}