diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index b7a4691ba..9b9ea58fb 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -1037,13 +1037,37 @@ func (b *LocalBackend) pauseOrResumeControlClientLocked() { if b.cc == nil { return } - networkUp := b.interfaceState.AnyInterfaceUp() - pauseForNetwork := (b.state == ipn.Stopped && b.NetMapNoPeers() != nil) || (!networkUp && !testenv.InTest() && !envknob.AssumeNetworkUp()) + b.cc.SetPaused(b.shouldPauseControlClientLocked(b.pm.CurrentPrefs())) +} + +// shouldPauseControlClientLocked reports whether the control client should be paused: +// if the LocalBackend is in Stopped state with a valid NetMap, +// if there is no network available, +// or if syncing preferences from the control plane has been disabled. +// +// b.mu must be held. +func (b *LocalBackend) shouldPauseControlClientLocked(prefs ipn.PrefsView) bool { + syncs.RequiresMutex(&b.mu) - prefs := b.pm.CurrentPrefs() pauseForSyncPref := prefs.Valid() && prefs.Sync().EqualBool(false) + if pauseForSyncPref { + return true + } - b.cc.SetPaused(pauseForNetwork || pauseForSyncPref) + // If tailscaled is being restarted, but it is supposed to be stopped, + // it mustn’t pause until the initial netmap has been loaded. + isStopped := b.state == ipn.Stopped && b.NetMapNoPeers() != nil + if isStopped { + return true + } + + networkUp := b.interfaceState.AnyInterfaceUp() + pauseForNetwork := !networkUp && !testenv.InTest() && !envknob.AssumeNetworkUp() + if pauseForNetwork { + return true + } + + return false } // DisconnectControl shuts down control client. This can be run before node shutdown to force control to consider this ndoe @@ -2961,7 +2985,7 @@ func (b *LocalBackend) startLocked(opts ipn.Options) error { ControlKnobs: b.sys.ControlKnobs(), Shutdown: ccShutdown, Bus: b.sys.Bus.Get(), - StartPaused: prefs.Sync().EqualBool(false), + StartPaused: b.shouldPauseControlClientLocked(prefs), }) if err != nil { return err diff --git a/tstest/integration/integration.go b/tstest/integration/integration.go index 190360950..b1903cb29 100644 --- a/tstest/integration/integration.go +++ b/tstest/integration/integration.go @@ -919,6 +919,20 @@ func (n *TestNode) MustDown() { if err := n.Tailscale("down", "--accept-risk=all").Run(); err != nil { t.Fatalf("down: %v", err) } + + // The tailscale down command is asynchronous, so it returns early. + // Wait for tailscaled to drop its connection before continuing. + if err := tstest.WaitFor(time.Second, func() error { + if err := t.Context().Err(); err != nil { + return err + } + if c := n.env.Control.InServeMap(); c != 0 { + return fmt.Errorf("%d connections remaining in serve map", c) + } + return nil + }); err != nil { + t.Fatalf("tailscale down: %v", err) + } } func (n *TestNode) MustLogOut() { diff --git a/tstest/integration/integration_test.go b/tstest/integration/integration_test.go index 1501da6a9..48f57e121 100644 --- a/tstest/integration/integration_test.go +++ b/tstest/integration/integration_test.go @@ -1058,8 +1058,6 @@ func TestC2NPingRequest(t *testing.T) { // Issue 2434: when "down" (WantRunning false), tailscaled shouldn't // be connected to control. func TestNoControlConnWhenDown(t *testing.T) { - flakytest.Mark(t, "https://github.com/tailscale/tailscale/issues/19831") - tstest.Shard(t) tstest.Parallel(t) env := NewTestEnv(t) @@ -1083,14 +1081,24 @@ func TestNoControlConnWhenDown(t *testing.T) { n1.AwaitBackendState("Stopped") + // The real test: verify our daemon doesn't have an HTTP request open. + // Stopping the client may take some time to disconnect from testcontrol. + if err := tstest.WaitFor(time.Second, func() error { + if n := env.Control.InServeMap(); n != 0 { + return fmt.Errorf("in serve map = %d; want 0", n) + } + return nil + }); err != nil { + t.Fatalf("unexpected connections while stopped: %v", err) + } + ip2 := n1.AwaitIP4() if ip1 != ip2 { t.Errorf("IPs different: %q vs %q", ip1, ip2) } - // The real test: verify our daemon doesn't have an HTTP request open. if n := env.Control.InServeMap(); n != 0 { - t.Errorf("in serve map = %d; want 0", n) + t.Fatalf("unexpected connection triggered by tailscale ip: in serve map = %d; want 0", n) } d2.MustCleanShutdown(t)