From 61ac021c5dd53bd3f49add12c683ba555da3c20d Mon Sep 17 00:00:00 2001 From: Harry Harpham Date: Fri, 27 Mar 2026 21:13:39 -0600 Subject: [PATCH] wgengine/magicsock: assume network up for tests Without this, any test relying on underlying use of magicsock will fail without network connectivity, even when the test logic has no need for a network connection. Tests currently in this bucket include many in tstest/integration and in tsnet. Further explanation: ipn only becomes Running when it sees at least one live peer or DERP connection: https://github.com/tailscale/tailscale/blob/0cc1b2ff76560ee4675909272fa37ba6b397744c/ipn/ipnlocal/local.go#L5861-L5866 When tests only use a single node, they will never see a peer, so the node has to wait to see a DERP server. magicsock sets the preferred DERP server in updateNetInfo(), but this function returns early if the network is down. https://github.com/tailscale/tailscale/blob/0cc1b2ff76560ee4675909272fa37ba6b397744c/wgengine/magicsock/magicsock.go#L1053-L1106 Because we're checking the real network, this prevents ipn from entering "Running" and causes the test to fail or hang. In tests, we can assume the network is up unless we're explicitly testing the behaviour of tailscaled when the network is down. We do something similar in magicsock/derp.go, where we assume we're connected to control unless explicitly testing otherwise: https://github.com/tailscale/tailscale/blob/7d2101f3520f16b86f2ed5e15f23c44d720534e6/wgengine/magicsock/derp.go#L166-L177 This is the template for the changes to `networkDown()`. Fixes #17122 Co-authored-by: Alex Chan Signed-off-by: Harry Harpham --- envknob/envknob.go | 3 +++ ipn/ipnlocal/local.go | 4 +--- wgengine/magicsock/magicsock.go | 13 ++++++++++++- wgengine/magicsock/magicsock_test.go | 2 ++ 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/envknob/envknob.go b/envknob/envknob.go index 2b1461f11..73a0da700 100644 --- a/envknob/envknob.go +++ b/envknob/envknob.go @@ -405,6 +405,9 @@ func SSHIgnoreTailnetPolicy() bool { return Bool("TS_DEBUG_SSH_IGNORE_TAILNET_PO // TKASkipSignatureCheck reports whether to skip node-key signature checking for development. func TKASkipSignatureCheck() bool { return Bool("TS_UNSAFE_SKIP_NKS_VERIFICATION") } +// AssumeNetworkUp reports whether to assume network connectivity for development. +func AssumeNetworkUp() bool { return Bool("TS_ASSUME_NETWORK_UP_FOR_TEST") } + // App returns the tailscale app type of this instance, if set via // TS_INTERNAL_APP env var. TS_INTERNAL_APP can be used to set app type for // components that wrap tailscaled, such as containerboot. App type is intended diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 5c25583e5..edeb2967b 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -963,8 +963,6 @@ func (b *LocalBackend) setConfigLocked(conf *conffile.Config) error { return nil } -var assumeNetworkUpdateForTest = envknob.RegisterBool("TS_ASSUME_NETWORK_UP_FOR_TEST") - // pauseOrResumeControlClientLocked pauses b.cc if there is no network available // or if the LocalBackend is in Stopped state with a valid NetMap. In all other // cases, it unpauses it. It is a no-op if b.cc is nil. @@ -976,7 +974,7 @@ func (b *LocalBackend) pauseOrResumeControlClientLocked() { return } networkUp := b.interfaceState.AnyInterfaceUp() - pauseForNetwork := (b.state == ipn.Stopped && b.NetMap() != nil) || (!networkUp && !testenv.InTest() && !assumeNetworkUpdateForTest()) + pauseForNetwork := (b.state == ipn.Stopped && b.NetMap() != nil) || (!networkUp && !testenv.InTest() && !envknob.AssumeNetworkUp()) prefs := b.pm.CurrentPrefs() pauseForSyncPref := prefs.Valid() && prefs.Sync().EqualBool(false) diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 4ad8022d8..5c4a385f7 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -1438,7 +1438,18 @@ func (c *Conn) LocalPort() uint16 { var errNetworkDown = errors.New("magicsock: network down") -func (c *Conn) networkDown() bool { return !c.networkUp.Load() } +// This allows tests to pass when the user's machine is offline, but allows us +// to still test network-down behaviour when desired. +var checkNetworkDownDuringTests = false + +func (c *Conn) networkDown() bool { + // For tests, always assume the network is up unless we're explicitly + // testing this behaviour. + if envknob.AssumeNetworkUp() || (testenv.InTest() && !checkNetworkDownDuringTests) { + return false + } + return !c.networkUp.Load() +} // Send implements conn.Bind. // diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index 4ecea8b18..f05ee2c6c 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/wgengine/magicsock/magicsock_test.go @@ -3241,6 +3241,8 @@ func TestNetworkSendErrors(t *testing.T) { t.Skipf("skipping on %s", runtime.GOOS) } + tstest.Replace(t, &checkNetworkDownDuringTests, true) + conn, reg, close := newTestConnAndRegistry(t) defer close()