From 95b44713c193779ddd2ce61cbfaa39739752d410 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sat, 17 Oct 2026 12:33:37 +0000 Subject: [PATCH] state: keep probing a lone advertiser while it is unhealthy --- hscontrol/servertest/ha_health_test.go | 43 ++++++++++++++++++++++++++ hscontrol/state/node_store.go | 10 ++++-- hscontrol/state/primaries_test.go | 23 ++++++++++++++ 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/hscontrol/servertest/ha_health_test.go b/hscontrol/servertest/ha_health_test.go index c1904724b..223845694 100644 --- a/hscontrol/servertest/ha_health_test.go +++ b/hscontrol/servertest/ha_health_test.go @@ -392,3 +392,46 @@ func TestHAHealthProbe_NoHARoutes(t *testing.T) { }) assert.False(t, dispatched, "no HA routes should produce no changes") } + +func TestHAHealthProbe_LastAdvertiserRecovers(t *testing.T) { + t.Parallel() + + srv := servertest.NewServer(t) + user := srv.CreateUser(t, "ha-last") + route := netip.MustParsePrefix("10.107.0.0/24") + c1 := servertest.NewClient(t, srv, "ha-la-r1", servertest.WithUser(user)) + c2 := servertest.NewClient(t, srv, "ha-la-r2", servertest.WithUser(user)) + + c1.WaitForPeers(t, 1, 10*time.Second) + c2.WaitForPeers(t, 1, 10*time.Second) + + nodeID1 := advertiseAndApproveRoute(t, srv, c1, route) + nodeID2 := advertiseAndApproveRoute(t, srv, c2, route) + + srv.State().SetNodeHealth(nodeID1, false) + require.Contains(t, srv.State().GetNodePrimaryRoutes(nodeID2), route) + + // The remaining advertiser needs a probe before its route can return. + _, rc, err := srv.State().SetApprovedRoutes(nodeID2, nil) + require.NoError(t, err) + srv.App.Change(rc) + require.Empty(t, srv.State().GetNodePrimaryRoutes(nodeID1)) + + prober := state.NewHAHealthProber( + srv.State(), + types.HARouteConfig{ + ProbeInterval: 30 * time.Second, + ProbeTimeout: 5 * time.Second, + }, + srv.URL, + srv.App.MapBatcher().IsConnected, + ) + + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + defer cancel() + + prober.ProbeOnce(ctx, srv.App.Change) + + assert.True(t, srv.State().IsNodeHealthy(nodeID1)) + assert.Contains(t, srv.State().GetNodePrimaryRoutes(nodeID1), route) +} diff --git a/hscontrol/state/node_store.go b/hscontrol/state/node_store.go index 81b08ff7a..ca4656750 100644 --- a/hscontrol/state/node_store.go +++ b/hscontrol/state/node_store.go @@ -1046,8 +1046,8 @@ func (s *NodeStore) PrimaryRoutesForNode(id types.NodeID) []netip.Prefix { return out } -// HANodes returns the prefixes with two or more online advertisers, the -// candidate set the HA prober needs to monitor. +// HANodes returns prefixes with multiple online advertisers and lone unhealthy +// advertisers that need another probe before their route can return. func (s *NodeStore) HANodes() map[netip.Prefix][]types.NodeID { snap := s.data.Load() @@ -1056,10 +1056,14 @@ func (s *NodeStore) HANodes() map[netip.Prefix][]types.NodeID { slices.Sorted(maps.Keys(snap.nodesByID)), ) + unhealthy := func(id types.NodeID) bool { + return snap.nodesByID[id].Unhealthy + } + out := make(map[netip.Prefix][]types.NodeID) for p, ids := range advertisers { - if len(ids) < 2 { + if len(ids) < 2 && !slices.ContainsFunc(ids, unhealthy) { continue } diff --git a/hscontrol/state/primaries_test.go b/hscontrol/state/primaries_test.go index 1ad13d156..8b17e8bd3 100644 --- a/hscontrol/state/primaries_test.go +++ b/hscontrol/state/primaries_test.go @@ -281,6 +281,29 @@ func TestPrimaries_HANodes(t *testing.T) { }, want: map[netip.Prefix][]types.NodeID{}, }, + { + name: "unhealthy-node-left-alone-is-still-monitored", + setup: func(f *primariesFixture) { + f.advertise(1, mp("192.168.1.0/24")) + f.advertise(2, mp("192.168.1.0/24")) + f.unhealthy(1) + f.approveRoutes(2) + }, + want: map[netip.Prefix][]types.NodeID{ + mp("192.168.1.0/24"): {1}, + }, + }, + { + name: "node-left-alone-not-ha-once-healthy", + setup: func(f *primariesFixture) { + f.advertise(1, mp("192.168.1.0/24")) + f.advertise(2, mp("192.168.1.0/24")) + f.unhealthy(1) + f.approveRoutes(2) + f.healthy(1) + }, + want: map[netip.Prefix][]types.NodeID{}, + }, { name: "two-nodes-same-prefix-is-ha", setup: func(f *primariesFixture) {