mirror of
https://github.com/tailscale/tailscale.git
synced 2025-08-13 22:47:30 +00:00
ipn/ipnlocal: change order of exit node refresh and netmap update so that clients receive the new netmap first
If the GUI receives a new exit node ID before the new netmap, it may treat the node as offline or invalid if the previous netmap didn't include the peer at all, or if the peer was offline or not advertised as an exit node. This may result in briefly issuing and dismissing a warning, or a similar issue, which isn't ideal. In this PR, we change the operation order to send the new netmap to clients first before selecting the new exit node and notifying them of the Exit Node change. Updates tailscale/corp#30252 (an old issue discovered during testing this) Signed-off-by: Nick Khyl <nickk@tailscale.com>
This commit is contained in:
@@ -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)
|
||||
|
@@ -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
|
||||
|
Reference in New Issue
Block a user