diff --git a/hscontrol/state/node_health_test.go b/hscontrol/state/node_health_test.go index cd61a25b..c7b27623 100644 --- a/hscontrol/state/node_health_test.go +++ b/hscontrol/state/node_health_test.go @@ -65,3 +65,27 @@ func TestScanNodeHealthReportsInvalidNameWithoutMutating(t *testing.T) { require.True(t, ok) require.Empty(t, nv.GivenName(), "boot scan must not mutate the stored name") } + +// TestBatchSetNodeHealthUnchangedSkipsWrite ensures asking for the health +// value already stored publishes no snapshot — no peersFunc invocation, no +// peer-map rebuild. +func TestBatchSetNodeHealthUnchangedSkipsWrite(t *testing.T) { + _, s, nodeID := persistTestSetup(t) + t.Cleanup(func() { _ = s.Close() }) + + // Use the State's nodeStore directly to confirm the node exists. + _, exists := s.nodeStore.GetNode(nodeID) + require.True(t, exists) + + initialSnapshot := s.nodeStore.data.Load() + + // The node starts with Unhealthy=false. Asking for "healthy=true" + // means Unhealthy stays false — no change. + for range 20 { + changed := s.BatchSetNodeHealth(map[types.NodeID]bool{nodeID: true}) + require.False(t, changed, + "unchanged health write must not report a route-primary change") + require.Same(t, initialSnapshot, s.nodeStore.data.Load(), + "unchanged health write must not publish a new snapshot") + } +} diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index a6d69534..3b33e1df 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -33,6 +33,8 @@ import ( "github.com/juanfont/headscale/hscontrol/util" "github.com/juanfont/headscale/hscontrol/util/zlog" "github.com/juanfont/headscale/hscontrol/util/zlog/zf" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" "github.com/puzpuzpuz/xsync/v4" "github.com/rs/zerolog" "github.com/rs/zerolog/log" @@ -1368,11 +1370,25 @@ func (s *State) BatchSetNodeHealth(updates map[types.NodeID]bool) bool { prevRoutes := s.nodeStore.PrimaryRoutes() + // Skip writes that would not change anything so an all-unchanged + // probe cycle publishes no snapshot. healthSetter stays authoritative + // for the candidacy and race checks under the writer. fns := make(map[types.NodeID]UpdateNodeFunc, len(updates)) + for id, healthy := range updates { + if nv, ok := s.nodeStore.GetNode(id); ok && nv.Unhealthy() == !healthy { + haHealthUpdates.WithLabelValues("unchanged").Inc() + + continue + } + fns[id] = healthSetter(healthy) } + if len(fns) == 0 { + return false + } + s.nodeStore.UpdateNodes(fns) return !maps.Equal(prevRoutes, s.nodeStore.PrimaryRoutes()) @@ -1383,15 +1399,27 @@ func (s *State) BatchSetNodeHealth(updates map[types.NodeID]bool) bool { // an unhealthy mark only sticks when the node is still online and // still advertises approved routes, so a node that left HA candidacy // between probe dispatch and result does not carry a stale bit. +// Bounded labels only: every requested health update lands in exactly one +// outcome, so the series can be summed to the probe request count. +var haHealthUpdates = promauto.NewCounterVec(prometheus.CounterOpts{ + Namespace: prometheusNamespace, + Name: "ha_health_updates_total", + Help: "HA health updates by outcome: unchanged and skipped, applied, or rejected because the node left candidacy.", +}, []string{"result"}) + func healthSetter(healthy bool) UpdateNodeFunc { return func(n *types.Node) { if !healthy { online := n.IsOnline != nil && *n.IsOnline if !online || len(n.AllApprovedRoutes()) == 0 { + haHealthUpdates.WithLabelValues("rejected").Inc() + return } } + haHealthUpdates.WithLabelValues("changed").Inc() + n.Unhealthy = !healthy } }