ipnlocal, magicsock: store last suggested exit node id in local backend

Updates tailscale/corp#19681

Signed-off-by: Claire Wang <claire@tailscale.com>

ipn/ipnlocal/local.go

@@ -151,6 +151,12 @@ type watchSession struct {
 	sessionID string
 }
 
+// lastSuggestedExitNode stores the last suggested exit node ID and name in local backend.
+type lastSuggestedExitNode struct {
+	id   tailcfg.StableNodeID
+	name string
+}
+
 // LocalBackend is the glue between the major pieces of the Tailscale
 // network software: the cloud control plane (via controlclient), the
 // network data plane (via wgengine), and the user-facing UIs and CLIs
@@ -324,6 +330,10 @@ type LocalBackend struct {
 
 	// outgoingFiles keeps track of Taildrop outgoing files keyed to their OutgoingFile.ID
 	outgoingFiles map[string]*ipn.OutgoingFile
+
+	// lastSuggestedExitNode stores the last suggested exit node ID and name.
+	// lastSuggestedExitNode updates whenever the suggestion changes.
+	lastSuggestedExitNode lastSuggestedExitNode
 }
 
 // HealthTracker returns the health tracker for the backend.
@@ -5958,7 +5968,8 @@ func (b *LocalBackend) resetForProfileChangeLockedOnEntry(unlock unlockOnce) err
 	}
 	b.lastServeConfJSON = mem.B(nil)
 	b.serveConfig = ipn.ServeConfigView{}
-	b.enterStateLockedOnEntry(ipn.NoState, unlock) // Reset state; releases b.mu
+	b.lastSuggestedExitNode = lastSuggestedExitNode{} // Reset last suggested exit node.
+	b.enterStateLockedOnEntry(ipn.NoState, unlock)    // Reset state; releases b.mu
 	b.health.SetLocalLogConfigHealth(nil)
 	return b.Start(ipn.Options{})
 }
@@ -6335,6 +6346,7 @@ func mayDeref[T any](p *T) (v T) {
 
 var ErrNoPreferredDERP = errors.New("no preferred DERP, try again later")
 var ErrCannotSuggestExitNode = errors.New("unable to suggest an exit node, try again later")
+var ErrUnableToSuggestLastExitNode = errors.New("unable to suggest last exit node")
 
 // SuggestExitNode computes a suggestion based on the current netmap and last netcheck report. If
 // there are multiple equally good options, one is selected at random, so the result is not stable. To be
@@ -6347,13 +6359,41 @@ func (b *LocalBackend) SuggestExitNode() (response apitype.ExitNodeSuggestionRes
 	b.mu.Lock()
 	lastReport := b.MagicConn().GetLastNetcheckReport(b.ctx)
 	netMap := b.netMap
+	lastSuggestedExitNode := b.lastSuggestedExitNode
 	b.mu.Unlock()
 	if lastReport == nil || netMap == nil {
-		return response, ErrCannotSuggestExitNode
+		last, err := suggestLastExitNode(lastSuggestedExitNode)
+		if err != nil {
+			return response, ErrCannotSuggestExitNode
+		}
+		return last, err
 	}
 	seed := time.Now().UnixNano()
 	r := rand.New(rand.NewSource(seed))
-	return suggestExitNode(lastReport, netMap, r)
+	res, err := suggestExitNode(lastReport, netMap, r)
+	if err != nil {
+		last, err := suggestLastExitNode(lastSuggestedExitNode)
+		if err != nil {
+			return response, ErrCannotSuggestExitNode
+		}
+		return last, err
+	}
+	b.mu.Lock()
+	b.lastSuggestedExitNode.id = res.ID
+	b.lastSuggestedExitNode.name = res.Name
+	b.mu.Unlock()
+	return res, err
+}
+
+// suggestLastExitNode formats a response with the last suggested exit node's ID and name.
+// Used as a fallback before returning a nil response and error.
+func suggestLastExitNode(lastSuggestedExitNode lastSuggestedExitNode) (res apitype.ExitNodeSuggestionResponse, err error) {
+	if lastSuggestedExitNode.id != "" && lastSuggestedExitNode.name != "" {
+		res.ID = lastSuggestedExitNode.id
+		res.Name = lastSuggestedExitNode.name
+		return res, nil
+	}
+	return res, ErrUnableToSuggestLastExitNode
 }
 
 func suggestExitNode(report *netcheck.Report, netMap *netmap.NetworkMap, r *rand.Rand) (res apitype.ExitNodeSuggestionResponse, err error) {

ipn/ipnlocal/local_test.go

@@ -25,6 +25,7 @@ import (
 	"golang.org/x/net/dns/dnsmessage"
 	"tailscale.com/appc"
 	"tailscale.com/appc/appctest"
+	"tailscale.com/client/tailscale/apitype"
 	"tailscale.com/clientupdate"
 	"tailscale.com/control/controlclient"
 	"tailscale.com/drive"
@@ -3436,6 +3437,357 @@ func TestMinLatencyDERPregion(t *testing.T) {
 	}
 }
 
+func TestSuggestLastExitNode(t *testing.T) {
+	tests := []struct {
+		name                      string
+		lastSuggestedExitNode     lastSuggestedExitNode
+		wantRes                   apitype.ExitNodeSuggestionResponse
+		wantLastSuggestedExitNode lastSuggestedExitNode
+		wantErr                   error
+	}{
+		{
+			name:                      "last suggested exit node is populated",
+			lastSuggestedExitNode:     lastSuggestedExitNode{id: "test", name: "test"},
+			wantRes:                   apitype.ExitNodeSuggestionResponse{ID: "test", Name: "test"},
+			wantLastSuggestedExitNode: lastSuggestedExitNode{id: "test", name: "test"},
+		},
+		{
+			name:    "last suggested exit node is not populated",
+			wantErr: ErrUnableToSuggestLastExitNode,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := suggestLastExitNode(tt.lastSuggestedExitNode)
+			if got != tt.wantRes || err != tt.wantErr {
+				t.Errorf("got %v error %v, want %v error %v", got, err, tt.wantRes, tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestLocalBackendSuggestExitNode(t *testing.T) {
+	tests := []struct {
+		name                      string
+		lastSuggestedExitNode     lastSuggestedExitNode
+		report                    netcheck.Report
+		netMap                    netmap.NetworkMap
+		wantID                    tailcfg.StableNodeID
+		wantName                  string
+		wantErr                   error
+		wantLastSuggestedExitNode lastSuggestedExitNode
+	}{
+		{
+			name:                  "nil netmap, returns last suggested exit node",
+			lastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"},
+			report: netcheck.Report{
+				RegionLatency: map[int]time.Duration{
+					1: 0,
+					2: -1,
+					3: 0,
+				},
+			},
+			wantID:                    "test",
+			wantName:                  "test",
+			wantLastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"},
+		},
+		{
+			name:                  "nil report, returns last suggested exit node",
+			lastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"},
+			netMap: netmap.NetworkMap{
+				SelfNode: (&tailcfg.Node{
+					Addresses: []netip.Prefix{
+						netip.MustParsePrefix("100.64.1.1/32"),
+						netip.MustParsePrefix("fe70::1/128"),
+					},
+				}).View(),
+				DERPMap: &tailcfg.DERPMap{
+					Regions: map[int]*tailcfg.DERPRegion{
+						1: {},
+						2: {},
+						3: {},
+					},
+				},
+			},
+			wantID:                    "test",
+			wantName:                  "test",
+			wantLastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"},
+		},
+		{
+			name:                  "found better derp node, last suggested exit node updates",
+			lastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"},
+			report: netcheck.Report{
+				RegionLatency: map[int]time.Duration{
+					1: 10,
+					2: 10,
+					3: 5,
+				},
+				PreferredDERP: 1,
+			},
+			netMap: netmap.NetworkMap{
+				SelfNode: (&tailcfg.Node{
+					Addresses: []netip.Prefix{
+						netip.MustParsePrefix("100.64.1.1/32"),
+						netip.MustParsePrefix("fe70::1/128"),
+					},
+				}).View(),
+				DERPMap: &tailcfg.DERPMap{
+					Regions: map[int]*tailcfg.DERPRegion{
+						1: {},
+						2: {},
+						3: {},
+					},
+				},
+				Peers: []tailcfg.NodeView{
+					(&tailcfg.Node{
+						ID:       2,
+						StableID: "test",
+						Name:     "test",
+						DERP:     "127.3.3.40:1",
+						AllowedIPs: []netip.Prefix{
+							netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"),
+						},
+						CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{
+							tailcfg.NodeAttrSuggestExitNode: {},
+						}),
+					}).View(),
+					(&tailcfg.Node{
+						ID:       3,
+						StableID: "foo",
+						Name:     "foo",
+						DERP:     "127.3.3.40:3",
+						AllowedIPs: []netip.Prefix{
+							netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"),
+						},
+						CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{
+							tailcfg.NodeAttrSuggestExitNode: {},
+						}),
+					}).View(),
+				},
+			},
+			wantID:                    "foo",
+			wantName:                  "foo",
+			wantLastSuggestedExitNode: lastSuggestedExitNode{name: "foo", id: "foo"},
+		},
+		{
+			name:                  "found better mullvad node, last suggested exit node updates",
+			lastSuggestedExitNode: lastSuggestedExitNode{name: "San Jose", id: "3"},
+			report: netcheck.Report{
+				RegionLatency: map[int]time.Duration{
+					1: 0,
+					2: 0,
+					3: 0,
+				},
+				PreferredDERP: 1,
+			},
+			netMap: netmap.NetworkMap{
+				SelfNode: (&tailcfg.Node{
+					Addresses: []netip.Prefix{
+						netip.MustParsePrefix("100.64.1.1/32"),
+						netip.MustParsePrefix("fe70::1/128"),
+					},
+				}).View(),
+				DERPMap: &tailcfg.DERPMap{
+					Regions: map[int]*tailcfg.DERPRegion{
+						1: {
+							Latitude:  40.73061,
+							Longitude: -73.935242,
+						},
+						2: {},
+						3: {},
+					},
+				},
+				Peers: []tailcfg.NodeView{
+					(&tailcfg.Node{
+						ID:       2,
+						StableID: "2",
+						AllowedIPs: []netip.Prefix{
+							netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"),
+						},
+						Name: "Dallas",
+						Hostinfo: (&tailcfg.Hostinfo{
+							Location: &tailcfg.Location{
+								Latitude:  32.89748,
+								Longitude: -97.040443,
+								Priority:  100,
+							},
+						}).View(),
+						CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{
+							tailcfg.NodeAttrSuggestExitNode: {},
+						}),
+					}).View(),
+					(&tailcfg.Node{
+						ID:       3,
+						StableID: "3",
+						AllowedIPs: []netip.Prefix{
+							netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"),
+						},
+						Name: "San Jose",
+						Hostinfo: (&tailcfg.Hostinfo{
+							Location: &tailcfg.Location{
+								Latitude:  37.3382082,
+								Longitude: -121.8863286,
+								Priority:  20,
+							},
+						}).View(),
+						CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{
+							tailcfg.NodeAttrSuggestExitNode: {},
+						}),
+					}).View(),
+				},
+			},
+			wantID:                    "2",
+			wantName:                  "Dallas",
+			wantLastSuggestedExitNode: lastSuggestedExitNode{name: "Dallas", id: "2"},
+		},
+		{
+			name:                  "ErrNoPreferredDERP, use last suggested exit node",
+			lastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"},
+			report: netcheck.Report{
+				RegionLatency: map[int]time.Duration{
+					1: 10,
+					2: 10,
+					3: 5,
+				},
+				PreferredDERP: 0,
+			},
+			netMap: netmap.NetworkMap{
+				SelfNode: (&tailcfg.Node{
+					Addresses: []netip.Prefix{
+						netip.MustParsePrefix("100.64.1.1/32"),
+						netip.MustParsePrefix("fe70::1/128"),
+					},
+				}).View(),
+				DERPMap: &tailcfg.DERPMap{
+					Regions: map[int]*tailcfg.DERPRegion{
+						1: {},
+						2: {},
+						3: {},
+					},
+				},
+				Peers: []tailcfg.NodeView{
+					(&tailcfg.Node{
+						ID:       2,
+						StableID: "test",
+						Name:     "test",
+						DERP:     "127.3.3.40:1",
+						AllowedIPs: []netip.Prefix{
+							netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"),
+						},
+						CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{
+							tailcfg.NodeAttrSuggestExitNode: {},
+						}),
+					}).View(),
+					(&tailcfg.Node{
+						ID:       3,
+						StableID: "foo",
+						Name:     "foo",
+						DERP:     "127.3.3.40:3",
+						AllowedIPs: []netip.Prefix{
+							netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"),
+						},
+						CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{
+							tailcfg.NodeAttrSuggestExitNode: {},
+						}),
+					}).View(),
+				},
+			},
+			wantID:                    "test",
+			wantName:                  "test",
+			wantLastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"},
+		},
+		{
+			name:                  "ErrNoPreferredDERP, use last suggested exit node",
+			lastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"},
+			report: netcheck.Report{
+				RegionLatency: map[int]time.Duration{
+					1: 10,
+					2: 10,
+					3: 5,
+				},
+				PreferredDERP: 0,
+			},
+			netMap: netmap.NetworkMap{
+				SelfNode: (&tailcfg.Node{
+					Addresses: []netip.Prefix{
+						netip.MustParsePrefix("100.64.1.1/32"),
+						netip.MustParsePrefix("fe70::1/128"),
+					},
+				}).View(),
+				DERPMap: &tailcfg.DERPMap{
+					Regions: map[int]*tailcfg.DERPRegion{
+						1: {},
+						2: {},
+						3: {},
+					},
+				},
+				Peers: []tailcfg.NodeView{
+					(&tailcfg.Node{
+						ID:       2,
+						StableID: "test",
+						Name:     "test",
+						DERP:     "127.3.3.40:1",
+						AllowedIPs: []netip.Prefix{
+							netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"),
+						},
+						CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{
+							tailcfg.NodeAttrSuggestExitNode: {},
+						}),
+					}).View(),
+					(&tailcfg.Node{
+						ID:       3,
+						StableID: "foo",
+						Name:     "foo",
+						DERP:     "127.3.3.40:3",
+						AllowedIPs: []netip.Prefix{
+							netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0"),
+						},
+						CapMap: (tailcfg.NodeCapMap)(map[tailcfg.NodeCapability][]tailcfg.RawMessage{
+							tailcfg.NodeAttrSuggestExitNode: {},
+						}),
+					}).View(),
+				},
+			},
+			wantID:                    "test",
+			wantName:                  "test",
+			wantLastSuggestedExitNode: lastSuggestedExitNode{name: "test", id: "test"},
+		},
+		{
+			name: "unable to use last suggested exit node",
+			report: netcheck.Report{
+				RegionLatency: map[int]time.Duration{
+					1: 10,
+					2: 10,
+					3: 5,
+				},
+				PreferredDERP: 0,
+			},
+			wantErr: ErrCannotSuggestExitNode,
+		},
+	}
+
+	for _, tt := range tests {
+		lb := newTestLocalBackend(t)
+		lb.lastSuggestedExitNode = tt.lastSuggestedExitNode
+		lb.netMap = &tt.netMap
+		lb.sys.MagicSock.Get().SetLastNetcheckReport(context.Background(), tt.report)
+		got, err := lb.SuggestExitNode()
+		if got.ID != tt.wantID {
+			t.Errorf("ID=%v, want=%v", got.ID, tt.wantID)
+		}
+		if got.Name != tt.wantName {
+			t.Errorf("Name=%v, want=%v", got.Name, tt.wantName)
+		}
+		if lb.lastSuggestedExitNode != tt.wantLastSuggestedExitNode {
+			t.Errorf("lastSuggestedExitNode=%v, want=%v", lb.lastSuggestedExitNode, tt.wantLastSuggestedExitNode)
+		}
+		if err != tt.wantErr {
+			t.Errorf("Error=%v, want=%v", err, tt.wantErr)
+		}
+	}
+}
+
 func TestEnableAutoUpdates(t *testing.T) {
 	lb := newTestLocalBackend(t)
 

wgengine/magicsock/magicsock.go

@@ -3088,3 +3088,9 @@ func (c *Conn) GetLastNetcheckReport(ctx context.Context) *netcheck.Report {
 	}
 	return lastReport
 }
+
+// SetLastNetcheckReport sets local backend's last netcheck report.
+// Used for testing purposes.
+func (c *Conn) SetLastNetcheckReport(ctx context.Context, report netcheck.Report) {
+	c.lastNetCheckReport.Store(&report)
+}