ipn/ipnlocal: update suggestExitNode to skip offline candidates and fix TestSetControlClientStatusAutoExitNode

TestSetControlClientStatusAutoExitNode is broken similarly to TestUpdateNetmapDeltaAutoExitNode
as suggestExitNode didn't previously check the online status of exit nodes, and similarly to the other test
it succeeded because the test itself is also broken.

However, it is easier to fix as it sends out a full netmap update rather than a delta peer update,
so it doesn't depend on the same refactoring as TestSetControlClientStatusAutoExitNode.

Updates #16455
Updates tailscale/corp#29969

Signed-off-by: Nick Khyl <nickk@tailscale.com>
This commit is contained in:
Nick Khyl
2025-07-03 11:51:27 -05:00
committed by Nick Khyl
parent 6ecc25b26a
commit 0098822981
2 changed files with 15 additions and 11 deletions

View File

@@ -7433,7 +7433,7 @@ func suggestExitNode(report *netcheck.Report, netMap *netmap.NetworkMap, prevSug
}
candidates := make([]tailcfg.NodeView, 0, len(netMap.Peers))
for _, peer := range netMap.Peers {
if !peer.Valid() {
if !peer.Valid() || !peer.Online().Get() {
continue
}
if allowList != nil && !allowList.Contains(peer.StableID()) {

View File

@@ -2166,8 +2166,8 @@ func TestAutoExitNodeSetNetInfoCallback(t *testing.T) {
}
func TestSetControlClientStatusAutoExitNode(t *testing.T) {
peer1 := makePeer(1, withCap(26), withSuggest(), withExitRoutes(), withNodeKey())
peer2 := makePeer(2, withCap(26), withSuggest(), withExitRoutes(), withNodeKey())
peer1 := makePeer(1, withCap(26), withSuggest(), withExitRoutes(), withOnline(true), withNodeKey())
peer2 := makePeer(2, withCap(26), withSuggest(), withExitRoutes(), withOnline(true), withNodeKey())
derpMap := &tailcfg.DERPMap{
Regions: map[int]*tailcfg.DERPRegion{
1: {
@@ -2210,22 +2210,25 @@ func TestSetControlClientStatusAutoExitNode(t *testing.T) {
))
syspolicy.MustRegisterStoreForTest(t, "TestStore", setting.DeviceScope, policyStore)
b.currentNode().SetNetMap(nm)
b.lastSuggestedExitNode = peer1.StableID()
// Peer 2 should be the initial exit node, as it's better than peer 1
// in terms of latency and DERP region.
b.lastSuggestedExitNode = peer2.StableID()
b.sys.MagicSock.Get().SetLastNetcheckReportForTest(b.ctx, report)
b.SetPrefsForTest(b.pm.CurrentPrefs().AsStruct())
firstExitNode := b.Prefs().ExitNodeID()
newPeer1 := makePeer(1, withCap(26), withSuggest(), withExitRoutes(), withOnline(false), withNodeKey())
offlinePeer2 := makePeer(2, withCap(26), withSuggest(), withExitRoutes(), withOnline(false), withNodeKey())
updatedNetmap := &netmap.NetworkMap{
Peers: []tailcfg.NodeView{
newPeer1,
peer2,
peer1,
offlinePeer2,
},
DERPMap: derpMap,
}
b.SetControlClientStatus(b.cc, controlclient.Status{NetMap: updatedNetmap})
lastExitNode := b.Prefs().ExitNodeID()
if firstExitNode == lastExitNode {
t.Errorf("did not switch exit nodes despite auto exit node going offline")
// But now that peer 2 is offline, we should switch to peer 1.
wantExitNode := peer1.StableID()
gotExitNode := b.Prefs().ExitNodeID()
if gotExitNode != wantExitNode {
t.Errorf("did not switch exit nodes despite auto exit node going offline: got %q; want %q", gotExitNode, wantExitNode)
}
}
@@ -3289,6 +3292,7 @@ func makePeer(id tailcfg.NodeID, opts ...peerOptFunc) tailcfg.NodeView {
Key: makeNodeKeyFromID(id),
StableID: tailcfg.StableNodeID(fmt.Sprintf("stable%d", id)),
Name: fmt.Sprintf("peer%d", id),
Online: ptr.To(true),
HomeDERP: int(id),
}
for _, opt := range opts {