From 7953592f40b41ec120b9c0a9940dcdf6bac274f6 Mon Sep 17 00:00:00 2001 From: Alex Chan Date: Mon, 15 Sep 2025 09:30:03 +0100 Subject: [PATCH] tstest/integration: get TestOneNodeUpAuth to pass when offline How TestOneNodeUpAuth works: * it starts a test version of tailscaled * it runs `tailscale up` * it checks that tailscaled becomes "Running" However, if the network is down, the test either fails or times out, because tailscaled never becomes Running. ## Why does the test fail? 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 The test only uses a single node, so it's never going to see a peer -- it has to wait to see a DERP server. The test environment does have a DERP map of in-memory DERP nodes, but we aren't connecting to them. 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. ## What's the fix? 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()`. Additionally, the `testenv.InTest()` function currently checks for the `-test.v` flag, but this isn't visible to the version of tailscaled running in the test environment. Instead, we add a new `TEST` env var so this process knows it's inside a test. Fixes #17122 Signed-off-by: Alex Chan --- tstest/integration/integration.go | 1 + util/testenv/testenv.go | 3 ++- wgengine/magicsock/magicsock.go | 14 +++++++++++++- wgengine/magicsock/magicsock_test.go | 2 ++ 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/tstest/integration/integration.go b/tstest/integration/integration.go index 987bb569a..a32920899 100644 --- a/tstest/integration/integration.go +++ b/tstest/integration/integration.go @@ -838,6 +838,7 @@ func (n *TestNode) StartDaemonAsIPNGOOS(ipnGOOS string) *Daemon { "TS_PANIC_IF_HIT_MAIN_CONTROL=1", "TS_DISABLE_PORTMAPPER=1", // shouldn't be needed; test is all localhost "TS_DEBUG_LOG_RATE=all", + "TEST=1", ) if n.env.loopbackPort != nil { cmd.Env = append(cmd.Env, "TS_DEBUG_NETSTACK_LOOPBACK_PORT="+strconv.Itoa(*n.env.loopbackPort)) diff --git a/util/testenv/testenv.go b/util/testenv/testenv.go index aa6660411..e990c1bf1 100644 --- a/util/testenv/testenv.go +++ b/util/testenv/testenv.go @@ -8,6 +8,7 @@ package testenv import ( "context" "flag" + "os" "tailscale.com/types/lazy" ) @@ -17,7 +18,7 @@ var lazyInTest lazy.SyncValue[bool] // InTest reports whether the current binary is a test binary. func InTest() bool { return lazyInTest.Get(func() bool { - return flag.Lookup("test.v") != nil + return flag.Lookup("test.v") != nil || os.Getenv("TEST") == "1" }) } diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 8ab7957ca..bf99624c6 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -1450,7 +1450,19 @@ func (c *Conn) LocalPort() uint16 { var errNetworkDown = errors.New("magicsock: network down") -func (c *Conn) networkDown() bool { return !c.networkUp.Load() } +// This allows existing tests to pass, but allows us to still test the +// behaviour during tests. +var checkNetworkDownDuringTests = false + +func (c *Conn) networkDown() bool { + // For tests, always assume the network is up unless we're explicitly + // testing this behaviour. + if testenv.InTest() && !checkNetworkDownDuringTests { + return false + } else { + return !c.networkUp.Load() + } +} // Send implements conn.Bind. // diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index 5b27e7a1b..caf12ccb1 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/wgengine/magicsock/magicsock_test.go @@ -3164,6 +3164,8 @@ func TestNetworkSendErrors(t *testing.T) { t.Skipf("skipping on %s", runtime.GOOS) } + tstest.Replace(t, &checkNetworkDownDuringTests, true) + reg := new(usermetric.Registry) conn.metrics = registerMetrics(reg)