ipn/ipnlocal: skip TestUpdateNetmapDeltaAutoExitNode

suggestExitNode never checks whether an exit node candidate is online.
It also accepts a full netmap, which doesn't include changes from delta updates.
The test can't work correctly until both issues are fixed.

Previously, it passed only because the test itself is flawed.
It doesn't succeed because the currently selected node goes offline and a new one is chosen.
Instead, it succeeds because lastSuggestedExitNode is incorrect, and suggestExitNode picks
the correct node the first time it runs, based on the DERP map and the netcheck report.
The node in exitNodeIDWant just happens to be the optimal choice.

Fixing SuggestExitNode requires refactoring its callers first, which in turn reveals the flawed test,
as suggestExitNode ends up being called slightly earlier.

In this PR, we update the test to correctly fail due to existing bugs in SuggestExitNode,
and temporarily skip it until those issues are addressed in a future commit.

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:50:27 -05:00
committed by Nick Khyl
parent 56d772bd63
commit 6ecc25b26a

View File

@@ -1918,8 +1918,10 @@ func TestSetExitNodeIDPolicy(t *testing.T) {
}
func TestUpdateNetmapDeltaAutoExitNode(t *testing.T) {
peer1 := makePeer(1, withCap(26), withSuggest(), withExitRoutes())
peer2 := makePeer(2, withCap(26), withSuggest(), withExitRoutes())
t.Skip("TODO(tailscale/tailscale#16455): suggestExitNode does not check for online status of exit nodes")
peer1 := makePeer(1, withCap(26), withSuggest(), withOnline(true), withExitRoutes())
peer2 := makePeer(2, withCap(26), withSuggest(), withOnline(true), withExitRoutes())
derpMap := &tailcfg.DERPMap{
Regions: map[int]*tailcfg.DERPRegion{
1: {
@@ -1958,8 +1960,10 @@ func TestUpdateNetmapDeltaAutoExitNode(t *testing.T) {
}{
{
// selected auto exit node goes offline
name: "exit-node-goes-offline",
lastSuggestedExitNode: peer1.StableID(),
name: "exit-node-goes-offline",
// PreferredDERP is 2, and it's also the region with the lowest latency.
// So, peer2 should be selected as the exit node.
lastSuggestedExitNode: peer2.StableID(),
netmap: &netmap.NetworkMap{
Peers: []tailcfg.NodeView{
peer1,
@@ -1970,14 +1974,14 @@ func TestUpdateNetmapDeltaAutoExitNode(t *testing.T) {
muts: []*tailcfg.PeerChange{
{
NodeID: 1,
Online: ptr.To(false),
Online: ptr.To(true),
},
{
NodeID: 2,
Online: ptr.To(true),
Online: ptr.To(false), // the selected exit node goes offline
},
},
exitNodeIDWant: peer2.StableID(),
exitNodeIDWant: peer1.StableID(),
report: report,
},
{
@@ -1994,7 +1998,7 @@ func TestUpdateNetmapDeltaAutoExitNode(t *testing.T) {
muts: []*tailcfg.PeerChange{
{
NodeID: 1,
Online: ptr.To(false),
Online: ptr.To(false), // a different exit node goes offline
},
{
NodeID: 2,