From 672aa401587e71f5ff797b159ce95f1e7c6f6a2b Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sat, 31 Oct 2026 14:14:32 +0000 Subject: [PATCH] state: move an offline router's routes in one frame As SaaS. A peer held an offline primary between two frames: the patch, then the recompute, which carries the offline state anyway. --- hscontrol/servertest/ha_dynamic_test.go | 45 +++++++++++++++++++++++++ hscontrol/state/connect_test.go | 8 +++-- hscontrol/state/state.go | 12 +++---- 3 files changed, 56 insertions(+), 9 deletions(-) diff --git a/hscontrol/servertest/ha_dynamic_test.go b/hscontrol/servertest/ha_dynamic_test.go index 475b34dca..2086e2ce5 100644 --- a/hscontrol/servertest/ha_dynamic_test.go +++ b/hscontrol/servertest/ha_dynamic_test.go @@ -89,6 +89,51 @@ func TestHAFailover_ViewerSeesPrimaryFlip(t *testing.T) { }) } +// TestHAFailover_OfflineMovesRouteInOneFrame: SaaS moved a route off a killed +// router in one frame, so a viewer never holds an offline primary. +func TestHAFailover_OfflineMovesRouteInOneFrame(t *testing.T) { + t.Parallel() + + srv := servertest.NewServer(t) + user := srv.CreateUser(t, "ha-dyn-offline") + + route := netip.MustParsePrefix("10.100.0.0/24") + + r1 := servertest.NewClient(t, srv, "dyn-offline-r1", servertest.WithUser(user)) + r2 := servertest.NewClient(t, srv, "dyn-offline-r2", servertest.WithUser(user)) + viewer := servertest.NewClient(t, srv, "dyn-offline-view", servertest.WithUser(user)) + + r1.WaitForPeers(t, 2, 10*time.Second) + r2.WaitForPeers(t, 2, 10*time.Second) + viewer.WaitForPeers(t, 2, 10*time.Second) + + advertiseAndApproveRoute(t, srv, r1, route) + advertiseAndApproveRoute(t, srv, r2, route) + + viewer.WaitForCondition(t, "viewer sees route via r1", 10*time.Second, + func(nm *netmap.NetworkMap) bool { + return hasPeerPrimaryRoute(nm, "dyn-offline-r1", route) + }) + + r1.Disconnect(t) + + viewer.WaitForCondition(t, "viewer sees route via r2", 30*time.Second, + func(nm *netmap.NetworkMap) bool { + return hasPeerPrimaryRoute(nm, "dyn-offline-r2", route) + }) + + for _, nm := range viewer.History() { + for _, p := range nm.Peers { + online, known := p.Online().GetOk() + offline := known && !online + + if p.Hostinfo().Hostname() == "dyn-offline-r1" && offline { + assert.False(t, hasPeerPrimaryRoute(nm, "dyn-offline-r1", route), "a netmap holds an offline primary") + } + } + } +} + // TestHAFailover_ViewerSeesRouteRevoke verifies that when the primary // revokes its approved route, the viewer's [netmap.NetworkMap] re-elects the // standby and the old primary no longer advertises the route. diff --git a/hscontrol/state/connect_test.go b/hscontrol/state/connect_test.go index 4af3d36e8..901ad214c 100644 --- a/hscontrol/state/connect_test.go +++ b/hscontrol/state/connect_test.go @@ -186,8 +186,6 @@ func TestConnectDisconnectSubnetRouterEmitsPolicyChangeNotFull(t *testing.T) { assert.NotEmpty(t, runtimePeerComputationReasons(cs), "subnet router must still drive a runtime peer recompute") - assert.True(t, hasPeerPatch(cs), - "subnet router should still emit the lightweight online/offline patch") for _, c := range cs { assert.Falsef(t, c.IsFull(), @@ -200,6 +198,8 @@ func TestConnectDisconnectSubnetRouterEmitsPolicyChangeNotFull(t *testing.T) { require.NotZero(t, epoch) assertRecomputeNotFull(t, cs) + assert.True(t, hasPeerPatch(cs), + "subnet router should still emit the lightweight online patch") }) t.Run("disconnect", func(t *testing.T) { @@ -207,6 +207,8 @@ func TestConnectDisconnectSubnetRouterEmitsPolicyChangeNotFull(t *testing.T) { cs := drainSessions(t, s, nodeID, epoch) assertRecomputeNotFull(t, cs) + assert.False(t, hasPeerPatch(cs), + "the recompute carries the offline state, so one frame moves the routes") }) } @@ -237,7 +239,7 @@ func TestReconnectWhileOnlineAnnouncesNothing(t *testing.T) { cs, err = s.Disconnect(nodeID, second) require.NoError(t, err) - require.True(t, hasPeerPatch(cs)) + require.True(t, forcesPeerRecompute(cs)) cs, _ = s.Connect(nodeID) assert.True(t, hasPeerPatch(cs), "a node that went offline must be announced online") diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index 34c53d639..174ba8b04 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -1045,15 +1045,15 @@ func (s *State) Disconnect(id types.NodeID, epoch uint64) ([]change.Change, erro log.Error().Err(err).EmbedObject(node).Msg("failed to update last seen in database") } - // An ordinary node going offline only needs the lightweight offline - // patch. Subnet routers, relay targets, and via targets change what - // peers compute, so they additionally force a peer recompute. - cs := []change.Change{change.NodeOffline(node.ID(), node.LastSeen().Get())} + // Subnet routers, relay targets, and via targets change what peers compute. + // The recompute also carries the node's offline state, so one frame moves + // its routes, as on SaaS. if s.polMan.NodeNeedsPeerRecompute(node) { - cs = append(cs, change.PolicyChange()) + return []change.Change{change.PolicyChange()}, nil } - return cs, nil + // An ordinary node going offline only needs the lightweight offline patch. + return []change.Change{change.NodeOffline(node.ID(), node.LastSeen().Get())}, nil } // GetNodeByID retrieves a node by ID.