diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index f866527d1..4b47f20c4 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -2380,12 +2380,10 @@ func (b *LocalBackend) Start(opts ipn.Options) error { } b.applyPrefsToHostinfoLocked(hostinfo, prefs) - b.setNetMapLocked(nil) persistv := prefs.Persist().AsStruct() if persistv == nil { persistv = new(persist.Persist) } - b.updateFilterLocked(nil, ipn.PrefsView{}) if b.portpoll != nil { b.portpollOnce.Do(func() { @@ -7531,6 +7529,7 @@ func (b *LocalBackend) resetForProfileChangeLockedOnEntry(unlock unlockOnce) err return nil } b.setNetMapLocked(nil) // Reset netmap. + b.updateFilterLocked(nil, ipn.PrefsView{}) // Reset the NetworkMap in the engine b.e.SetNetworkMap(new(netmap.NetworkMap)) if prevCC := b.resetControlClientLocked(); prevCC != nil { diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index bdccdb53d..5b74b8180 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -1510,6 +1510,15 @@ func TestReconfigureAppConnector(t *testing.T) { func TestBackfillAppConnectorRoutes(t *testing.T) { // Create backend with an empty app connector. b := newTestBackend(t) + // newTestBackend creates a backend with a non-nil netmap, + // but this test requires a nil netmap. + // Otherwise, instead of backfilling, [LocalBackend.reconfigAppConnectorLocked] + // uses the domains and routes from netmap's [appctype.AppConnectorAttr]. + // Additionally, a non-nil netmap makes reconfigAppConnectorLocked + // asynchronous, resulting in a flaky test. + // Therefore, we set the netmap to nil to simulate a fresh backend start + // or a profile switch where the netmap is not yet available. + b.setNetMapLocked(nil) if err := b.Start(ipn.Options{}); err != nil { t.Fatal(err) } diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go index 1b3b43af6..a4180de86 100644 --- a/ipn/ipnlocal/state_test.go +++ b/ipn/ipnlocal/state_test.go @@ -735,12 +735,10 @@ func TestStateMachine(t *testing.T) { // b.Shutdown() explicitly ourselves. previousCC.assertShutdown(false) - // Note: unpause happens because ipn needs to get at least one netmap - // on startup, otherwise UIs can't show the node list, login - // name, etc when in state ipn.Stopped. - // Arguably they shouldn't try. But they currently do. nn := notifies.drain(2) - cc.assertCalls("New", "Login") + // We already have a netmap for this node, + // and WantRunning is false, so cc should be paused. + cc.assertCalls("New", "Login", "pause") c.Assert(nn[0].Prefs, qt.IsNotNil) c.Assert(nn[1].State, qt.IsNotNil) c.Assert(nn[0].Prefs.WantRunning(), qt.IsFalse) @@ -751,7 +749,11 @@ func TestStateMachine(t *testing.T) { // When logged in but !WantRunning, ipn leaves us unpaused to retrieve // the first netmap. Simulate that netmap being received, after which // it should pause us, to avoid wasting CPU retrieving unnecessarily - // additional netmap updates. + // additional netmap updates. Since our LocalBackend instance already + // has a netmap, we will reset it to nil to simulate the first netmap + // retrieval. + b.setNetMapLocked(nil) + cc.assertCalls("unpause") // // TODO: really the various GUIs and prefs should be refactored to // not require the netmap structure at all when starting while @@ -853,7 +855,7 @@ func TestStateMachine(t *testing.T) { // The last test case is the most common one: restarting when both // logged in and WantRunning. t.Logf("\n\nStart5") - notifies.expect(1) + notifies.expect(2) c.Assert(b.Start(ipn.Options{}), qt.IsNil) { // NOTE: cc.Shutdown() is correct here, since we didn't call @@ -861,30 +863,32 @@ func TestStateMachine(t *testing.T) { previousCC.assertShutdown(false) cc.assertCalls("New", "Login") - nn := notifies.drain(1) + nn := notifies.drain(2) cc.assertCalls() c.Assert(nn[0].Prefs, qt.IsNotNil) c.Assert(nn[0].Prefs.LoggedOut(), qt.IsFalse) c.Assert(nn[0].Prefs.WantRunning(), qt.IsTrue) - c.Assert(b.State(), qt.Equals, ipn.NoState) + // We're logged in and have a valid netmap, so we should + // be in the Starting state. + c.Assert(nn[1].State, qt.IsNotNil) + c.Assert(*nn[1].State, qt.Equals, ipn.Starting) + c.Assert(b.State(), qt.Equals, ipn.Starting) } // Control server accepts our valid key from before. t.Logf("\n\nLoginFinished5") - notifies.expect(1) + notifies.expect(0) cc.send(nil, "", true, &netmap.NetworkMap{ SelfNode: (&tailcfg.Node{MachineAuthorized: true}).View(), }) { - nn := notifies.drain(1) + notifies.drain(0) cc.assertCalls() // NOTE: No LoginFinished message since no interactive // login was needed. - c.Assert(nn[0].State, qt.IsNotNil) - c.Assert(ipn.Starting, qt.Equals, *nn[0].State) // NOTE: No prefs change this time. WantRunning stays true. // We were in Starting in the first place, so that doesn't - // change either. + // change either, so we don't expect any notifications. c.Assert(ipn.Starting, qt.Equals, b.State()) } t.Logf("\n\nExpireKey")