diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 55730489e..48eceb36c 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -1709,9 +1709,6 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control // Now complete the lock-free parts of what we started while locked. if st.NetMap != nil { - // Check and update the exit node if needed, now that we have a new netmap. - b.RefreshExitNode() - if envknob.NoLogsNoSupport() && st.NetMap.HasCap(tailcfg.CapabilityDataPlaneAuditLogs) { msg := "tailnet requires logging to be enabled. Remove --no-logs-no-support from tailscaled command line." b.health.SetLocalLogConfigHealth(errors.New(msg)) @@ -1751,6 +1748,16 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control b.health.SetDERPMap(st.NetMap.DERPMap) b.send(ipn.Notify{NetMap: st.NetMap}) + + // Check and update the exit node if needed, now that we have a new netmap. + // + // This must happen after the netmap change is sent via [ipn.Notify], + // so the GUI can correctly display the exit node if it has changed + // since the last netmap was sent. + // + // Otherwise, it might briefly show the exit node as offline and display a warning, + // if the node wasn't online or wasn't advertising default routes in the previous netmap. + b.RefreshExitNode() } if st.URL != "" { b.logf("Received auth URL: %.20v...", st.URL) diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index e70e5ad2a..bb7f433c0 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -1407,6 +1407,60 @@ func TestPrefsChangeDisablesExitNode(t *testing.T) { } } +func TestExitNodeNotifyOrder(t *testing.T) { + const controlURL = "https://localhost:1/" + + report := &netcheck.Report{ + RegionLatency: map[int]time.Duration{ + 1: 5 * time.Millisecond, + 2: 10 * time.Millisecond, + }, + PreferredDERP: 1, + } + + exitNode1 := makeExitNode(1, withName("node-1"), withDERP(1), withAddresses(netip.MustParsePrefix("100.64.1.1/32"))) + exitNode2 := makeExitNode(2, withName("node-2"), withDERP(2), withAddresses(netip.MustParsePrefix("100.64.1.2/32"))) + selfNode := makeExitNode(3, withName("node-3"), withDERP(1), withAddresses(netip.MustParsePrefix("100.64.1.3/32"))) + clientNetmap := buildNetmapWithPeers(selfNode, exitNode1, exitNode2) + + lb := newTestLocalBackend(t) + lb.sys.MagicSock.Get().SetLastNetcheckReportForTest(lb.ctx, report) + lb.SetPrefsForTest(&ipn.Prefs{ + ControlURL: controlURL, + AutoExitNode: ipn.AnyExitNode, + }) + + nw := newNotificationWatcher(t, lb, ipnauth.Self) + + // Updating the netmap should trigger both a netmap notification + // and an exit node ID notification (since an exit node is selected). + // The netmap notification should be sent first. + nw.watch(0, []wantedNotification{ + wantNetmapNotify(clientNetmap), + wantExitNodeIDNotify(exitNode1.StableID()), + }) + lb.SetControlClientStatus(lb.cc, controlclient.Status{NetMap: clientNetmap}) + nw.check() +} + +func wantNetmapNotify(want *netmap.NetworkMap) wantedNotification { + return wantedNotification{ + name: "Netmap", + cond: func(t testing.TB, _ ipnauth.Actor, n *ipn.Notify) bool { + return n.NetMap == want + }, + } +} + +func wantExitNodeIDNotify(want tailcfg.StableNodeID) wantedNotification { + return wantedNotification{ + name: fmt.Sprintf("ExitNodeID-%s", want), + cond: func(_ testing.TB, _ ipnauth.Actor, n *ipn.Notify) bool { + return n.Prefs != nil && n.Prefs.Valid() && n.Prefs.ExitNodeID() == want + }, + } +} + func TestInternalAndExternalInterfaces(t *testing.T) { type interfacePrefix struct { i netmon.Interface