fix(monitoring): show offline nodes for PVE instances unreachable since startup

A configured PVE instance that was already down when Pulse started had
no node state, so the unreachable-poll path returned without writing
anything and the instance was invisible on the Proxmox overview until
its first successful poll. Synthesize offline node entries from the
instance config in that case: cluster configs get one row per
discovered member endpoint with the same node ID a live poll would
assign, standalone instances get a single row named after the
configured instance. The next successful poll replaces them wholesale.

Also stop the offline grace policy from resurrecting such placeholders.
Previous node state round-trips through the unified registry, which
stamps zero LastSeen values with the ingest time, so a never-seen
placeholder came back "recently seen" every cycle and flipped to a
permanent online/degraded state. Grace now requires a real online
sighting: a nodeLastOnline entry from this process, or a previous
snapshot that itself reports the node online with a recent poll
timestamp.

Verified live on the dev stack: a synthetic instance pointing at an
unreachable TEST-NET address appears as an offline node row on the
Proxmox overview immediately after a fresh-state backend start and
stays offline past the grace and stale windows.

Fixes #1433
This commit is contained in:
rcourtman 2026-06-11 09:47:42 +01:00
parent f9fd30a3cd
commit 8372a22c51
2 changed files with 206 additions and 21 deletions

View file

@ -624,11 +624,80 @@ func (m *Monitor) preserveNodesWhenEmpty(instanceName string, modelNodes []model
func (m *Monitor) markPVEInstanceNodesUnreachable(instanceName string) {
_, prevInstanceNodes := m.snapshotPrevNodes(instanceName)
if len(prevInstanceNodes) == 0 {
// Never polled successfully this process lifetime (e.g. Pulse
// started while the host was already down). Synthesize offline
// entries from the instance config so the configured instance is
// still represented on the platform pages instead of vanishing
// until its first successful poll (#1433).
if placeholders := m.placeholderNodesForInstance(instanceName); len(placeholders) > 0 {
m.state.UpdateNodesForInstance(instanceName, placeholders)
}
return
}
m.state.UpdateNodesForInstance(instanceName, m.preserveOrExpireNodes(prevInstanceNodes))
}
// placeholderNodesForInstance builds offline node entries from the instance
// configuration for an instance with no node state yet. Cluster configs carry
// their discovered member endpoints, so each member gets its own row with the
// same node ID a live poll would assign; a standalone instance gets a single
// row named after the configured instance. The next successful poll replaces
// these wholesale via UpdateNodesForInstance.
func (m *Monitor) placeholderNodesForInstance(instanceName string) []models.Node {
instanceCfg := m.getInstanceConfig(instanceName)
if instanceCfg == nil {
return nil
}
makeNode := func(nodeName string) models.Node {
nodeID := instanceName + "-" + nodeName
if instanceCfg.IsCluster && instanceCfg.ClusterName != "" {
nodeID = instanceCfg.ClusterName + "-" + nodeName
}
connectionHost, guestURL := resolveNodeConnectionInfo(instanceCfg, monitorDiscoveryConfig(m), nodeName)
return models.Node{
ID: nodeID,
Name: nodeName,
DisplayName: getNodeDisplayName(instanceCfg, nodeName),
Instance: instanceName,
Host: connectionHost,
GuestURL: guestURL,
Status: "offline",
Type: "node",
ConnectionHealth: "error",
LoadAverage: []float64{},
IsClusterMember: instanceCfg.IsCluster,
ClusterName: instanceCfg.ClusterName,
TemperatureMonitoringEnabled: instanceCfg.TemperatureMonitoringEnabled,
}
}
if instanceCfg.IsCluster && len(instanceCfg.ClusterEndpoints) > 0 {
nodes := make([]models.Node, 0, len(instanceCfg.ClusterEndpoints))
seen := make(map[string]struct{}, len(instanceCfg.ClusterEndpoints))
for _, endpoint := range instanceCfg.ClusterEndpoints {
nodeName := strings.TrimSpace(endpoint.NodeName)
if nodeName == "" {
continue
}
if _, dup := seen[strings.ToLower(nodeName)]; dup {
continue
}
seen[strings.ToLower(nodeName)] = struct{}{}
nodes = append(nodes, makeNode(nodeName))
}
if len(nodes) > 0 {
return nodes
}
}
nodeName := strings.TrimSpace(instanceCfg.Name)
if nodeName == "" {
nodeName = instanceName
}
return []models.Node{makeNode(nodeName)}
}
// preserveOrExpireNodes keeps recently seen nodes online through transient
// gaps and marks nodes offline once the grace period has lapsed.
func (m *Monitor) preserveOrExpireNodes(prevInstanceNodes []models.Node) []models.Node {
@ -639,16 +708,22 @@ func (m *Monitor) preserveOrExpireNodes(prevInstanceNodes []models.Node) []model
// Keep recently seen nodes online during transient GetNodes gaps.
// This mirrors the node grace behavior used in regular node polling.
lastSeen := prevNode.LastSeen
if lastSeen.IsZero() {
m.mu.Lock()
if lastOnline, ok := m.nodeLastOnline[prevNode.ID]; ok {
lastSeen = lastOnline
}
m.mu.Unlock()
}
// Grace requires evidence of a real online sighting: either this
// process saw the node online (nodeLastOnline), or the previous
// snapshot itself reports it online with a recent poll timestamp.
// prevNode.LastSeen alone is NOT enough for an offline node — prev
// state round-trips through the unified registry, which stamps
// zero LastSeen values with the ingest time, so a synthesized
// offline placeholder would otherwise come back "recently seen"
// every cycle and be resurrected to online forever.
m.mu.Lock()
lastOnline, sawOnline := m.nodeLastOnline[prevNode.ID]
m.mu.Unlock()
withinGrace := !lastSeen.IsZero() && now.Sub(lastSeen) < nodeOfflineGracePeriod
withinGrace := sawOnline && now.Sub(lastOnline) < nodeOfflineGracePeriod
if !withinGrace && strings.EqualFold(strings.TrimSpace(prevNode.Status), "online") {
withinGrace = !prevNode.LastSeen.IsZero() && now.Sub(prevNode.LastSeen) < nodeOfflineGracePeriod
}
if withinGrace {
if strings.TrimSpace(nodeCopy.Status) == "" || strings.EqualFold(nodeCopy.Status, "offline") {
nodeCopy.Status = "online"

View file

@ -39,6 +39,31 @@ func TestPreserveOrExpireNodes(t *testing.T) {
}
})
t.Run("offline node with stamped LastSeen is not resurrected to online", func(t *testing.T) {
// Simulates the unified-registry round-trip: ingest stamps zero
// LastSeen with the ingest time, so a synthesized offline
// placeholder comes back from prev state looking "recently seen"
// even though no poll ever reached the node. Grace must not flip
// it to online — that loop made never-seen hosts permanently
// report online/degraded.
preserved := m.preserveOrExpireNodes([]models.Node{{
ID: "inst-placeholder",
Name: "placeholder",
Status: "offline",
ConnectionHealth: "error",
LastSeen: time.Now().Add(-5 * time.Second),
}})
if len(preserved) != 1 {
t.Fatalf("preserved = %d nodes, want 1", len(preserved))
}
if preserved[0].Status != "offline" {
t.Fatalf("Status = %q, want offline (no online sighting to justify grace)", preserved[0].Status)
}
if preserved[0].ConnectionHealth != "error" {
t.Fatalf("ConnectionHealth = %q, want error", preserved[0].ConnectionHealth)
}
})
t.Run("past grace marks node offline", func(t *testing.T) {
preserved := m.preserveOrExpireNodes([]models.Node{{
ID: "inst-node1",
@ -71,17 +96,10 @@ func (c *unreachablePVEClient) GetNodes(ctx context.Context) ([]proxmox.Node, er
return nil, fmt.Errorf("connection refused")
}
// Regression test for #1441: when the whole instance stops answering (host
// shut down), the poll error path must still run the offline grace policy
// instead of freezing the last online snapshot in state forever.
func TestPollPVEInstanceMarksNodesOfflineWhenUnreachable(t *testing.T) {
func newUnreachableTestMonitor(t *testing.T, cfg *config.Config) *Monitor {
t.Helper()
m := &Monitor{
config: &config.Config{
PVEInstances: []config.PVEInstance{
// Non-default port so the portless fallback path stays out of the way.
{Name: "pve-test", Host: "https://localhost:9999"},
},
},
config: cfg,
state: models.NewState(),
pveClients: make(map[string]PVEClientInterface),
nodeLastOnline: make(map[string]time.Time),
@ -101,8 +119,21 @@ func TestPollPVEInstanceMarksNodesOfflineWhenUnreachable(t *testing.T) {
alertManager: alerts.NewManager(),
notificationMgr: notifications.NewNotificationManager(""),
}
defer m.alertManager.Stop()
defer m.notificationMgr.Stop()
t.Cleanup(m.alertManager.Stop)
t.Cleanup(m.notificationMgr.Stop)
return m
}
// Regression test for #1441: when the whole instance stops answering (host
// shut down), the poll error path must still run the offline grace policy
// instead of freezing the last online snapshot in state forever.
func TestPollPVEInstanceMarksNodesOfflineWhenUnreachable(t *testing.T) {
m := newUnreachableTestMonitor(t, &config.Config{
PVEInstances: []config.PVEInstance{
// Non-default port so the portless fallback path stays out of the way.
{Name: "pve-test", Host: "https://localhost:9999"},
},
})
// Seed state as if a previous poll saw the node online, longer ago than
// the offline grace period.
@ -131,3 +162,82 @@ func TestPollPVEInstanceMarksNodesOfflineWhenUnreachable(t *testing.T) {
t.Fatalf("node ConnectionHealth = %q, want error", nodes[0].ConnectionHealth)
}
}
// Regression test for #1433: an instance that is already down when Pulse
// starts has no previous node state, so the unreachable path must synthesize
// an offline placeholder from config instead of leaving the configured
// instance invisible until its first successful poll.
func TestPollPVEInstanceSynthesizesPlaceholderWhenNeverSeen(t *testing.T) {
m := newUnreachableTestMonitor(t, &config.Config{
PVEInstances: []config.PVEInstance{
// Non-default port so the portless fallback path stays out of the way.
{Name: "pve-test", Host: "https://localhost:9999"},
},
})
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
defer cancel()
m.pollPVEInstance(ctx, "pve-test", &unreachablePVEClient{})
nodes := m.state.GetSnapshot().Nodes
if len(nodes) != 1 {
t.Fatalf("state nodes = %d, want 1 synthesized placeholder", len(nodes))
}
if nodes[0].ID != "pve-test-pve-test" {
t.Fatalf("node ID = %q, want pve-test-pve-test", nodes[0].ID)
}
if nodes[0].Name != "pve-test" {
t.Fatalf("node Name = %q, want pve-test", nodes[0].Name)
}
if nodes[0].Status != "offline" {
t.Fatalf("node Status = %q, want offline", nodes[0].Status)
}
if nodes[0].ConnectionHealth != "error" {
t.Fatalf("node ConnectionHealth = %q, want error", nodes[0].ConnectionHealth)
}
if nodes[0].Host != "https://localhost:9999" {
t.Fatalf("node Host = %q, want configured endpoint", nodes[0].Host)
}
}
func TestPlaceholderNodesForInstanceCluster(t *testing.T) {
m := &Monitor{
state: models.NewState(),
config: &config.Config{
PVEInstances: []config.PVEInstance{{
Name: "cluster-inst",
Host: "https://10.0.0.1:8006",
IsCluster: true,
ClusterName: "homelab",
ClusterEndpoints: []config.ClusterEndpoint{
{NodeName: "pve1", Host: "https://10.0.0.1:8006"},
{NodeName: "pve2", Host: "https://10.0.0.2:8006"},
{NodeName: ""}, // skipped: no node name
{NodeName: "pve1"}, // skipped: duplicate
},
}},
},
}
nodes := m.placeholderNodesForInstance("cluster-inst")
if len(nodes) != 2 {
t.Fatalf("placeholders = %d nodes, want 2 (deduped, blank skipped)", len(nodes))
}
for i, want := range []string{"pve1", "pve2"} {
if nodes[i].Name != want {
t.Fatalf("nodes[%d].Name = %q, want %q", i, nodes[i].Name, want)
}
if wantID := "homelab-" + want; nodes[i].ID != wantID {
t.Fatalf("nodes[%d].ID = %q, want %q (cluster ID convention)", i, nodes[i].ID, wantID)
}
if nodes[i].Status != "offline" {
t.Fatalf("nodes[%d].Status = %q, want offline", i, nodes[i].Status)
}
if !nodes[i].IsClusterMember {
t.Fatalf("nodes[%d].IsClusterMember = false, want true", i)
}
if nodes[i].ClusterName != "homelab" {
t.Fatalf("nodes[%d].ClusterName = %q, want homelab", i, nodes[i].ClusterName)
}
}
}