ipn/ipnlocal, all: plumb health trackers in tests

I saw some panics in CI, like:

    2024-05-08T04:30:25.9553518Z ## WARNING: (non-fatal) nil health.Tracker (being strict in CI):
    2024-05-08T04:30:25.9554043Z goroutine 801 [running]:
    2024-05-08T04:30:25.9554489Z tailscale.com/health.(*Tracker).nil(0x0)
    2024-05-08T04:30:25.9555086Z 	tailscale.com/health/health.go:185 +0x70
    2024-05-08T04:30:25.9555688Z tailscale.com/health.(*Tracker).SetUDP4Unbound(0x0, 0x0)
    2024-05-08T04:30:25.9556373Z 	tailscale.com/health/health.go:532 +0x2f
    2024-05-08T04:30:25.9557296Z tailscale.com/wgengine/magicsock.(*Conn).bindSocket(0xc0003b4808, 0xc0003b4878, {0x1fbca53, 0x4}, 0x0)
    2024-05-08T04:30:25.9558301Z 	tailscale.com/wgengine/magicsock/magicsock.go:2481 +0x12c5
    2024-05-08T04:30:25.9559026Z tailscale.com/wgengine/magicsock.(*Conn).rebind(0xc0003b4808, 0x0)
    2024-05-08T04:30:25.9559874Z 	tailscale.com/wgengine/magicsock/magicsock.go:2510 +0x16f
    2024-05-08T04:30:25.9561038Z tailscale.com/wgengine/magicsock.NewConn({0xc000063c80, 0x0, 0xc000197930, 0xc000197950, 0xc000197960, {0x0, 0x0}, 0xc000197970, 0xc000198ee0, 0x0, ...})
    2024-05-08T04:30:25.9562402Z 	tailscale.com/wgengine/magicsock/magicsock.go:476 +0xd5f
    2024-05-08T04:30:25.9563779Z tailscale.com/wgengine.NewUserspaceEngine(0xc000063c80, {{0x22c8750, 0xc0001976b0}, 0x0, {0x22c3210, 0xc000063c80}, {0x22c31d8, 0x2d3c900}, 0x0, 0x0, ...})
    2024-05-08T04:30:25.9564982Z 	tailscale.com/wgengine/userspace.go:389 +0x159d
    2024-05-08T04:30:25.9565529Z tailscale.com/ipn/ipnlocal.newTestBackend(0xc000358b60)
    2024-05-08T04:30:25.9566086Z 	tailscale.com/ipn/ipnlocal/serve_test.go:675 +0x2a5
    2024-05-08T04:30:25.9566612Z ta

Updates #11874

Change-Id: I3432ed52d670743e532be4642f38dbd6e3763b1b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This commit is contained in:
Brad Fitzpatrick 2024-05-07 21:37:33 -07:00 committed by Brad Fitzpatrick
parent 727c0d6cfd
commit 21509db121
12 changed files with 62 additions and 35 deletions

View File

@ -431,7 +431,7 @@ func newTestLocalBackend(t testing.TB) *LocalBackend {
sys := new(tsd.System)
store := new(mem.Store)
sys.Set(store)
eng, err := wgengine.NewFakeUserspaceEngine(logf, sys.Set)
eng, err := wgengine.NewFakeUserspaceEngine(logf, sys.Set, sys.HealthTracker())
if err != nil {
t.Fatalf("NewFakeUserspaceEngine: %v", err)
}

View File

@ -50,7 +50,7 @@ func TestLocalLogLines(t *testing.T) {
sys := new(tsd.System)
store := new(mem.Store)
sys.Set(store)
e, err := wgengine.NewFakeUserspaceEngine(logf, sys.Set)
e, err := wgengine.NewFakeUserspaceEngine(logf, sys.Set, sys.HealthTracker())
if err != nil {
t.Fatal(err)
}

View File

@ -642,8 +642,9 @@ func TestPeerAPIReplyToDNSQueries(t *testing.T) {
h.isSelf = false
h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345")
eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0)
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
ht := new(health.Tracker)
eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0, ht)
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, ht))
h.ps = &peerAPIServer{
b: &LocalBackend{
e: eng,
@ -692,8 +693,9 @@ func TestPeerAPIPrettyReplyCNAME(t *testing.T) {
var h peerAPIHandler
h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345")
eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0)
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
ht := new(health.Tracker)
eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0, ht)
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, ht))
var a *appc.AppConnector
if shouldStore {
a = appc.NewAppConnector(t.Logf, &appctest.RouteCollector{}, &appc.RouteInfo{}, fakeStoreRoutes)
@ -764,8 +766,9 @@ func TestPeerAPIReplyToDNSQueriesAreObserved(t *testing.T) {
h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345")
rc := &appctest.RouteCollector{}
eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0)
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
ht := new(health.Tracker)
eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0, ht)
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, ht))
var a *appc.AppConnector
if shouldStore {
a = appc.NewAppConnector(t.Logf, rc, &appc.RouteInfo{}, fakeStoreRoutes)
@ -826,9 +829,10 @@ func TestPeerAPIReplyToDNSQueriesAreObservedWithCNAMEFlattening(t *testing.T) {
var h peerAPIHandler
h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345")
ht := new(health.Tracker)
rc := &appctest.RouteCollector{}
eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0)
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, new(health.Tracker)))
eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0, ht)
pm := must.Get(newProfileManager(new(mem.Store), t.Logf, ht))
var a *appc.AppConnector
if shouldStore {
a = appc.NewAppConnector(t.Logf, rc, &appc.RouteInfo{}, fakeStoreRoutes)

View File

@ -672,7 +672,10 @@ func newTestBackend(t *testing.T) *LocalBackend {
}
sys := &tsd.System{}
e, err := wgengine.NewUserspaceEngine(logf, wgengine.Config{SetSubsystem: sys.Set})
e, err := wgengine.NewUserspaceEngine(logf, wgengine.Config{
SetSubsystem: sys.Set,
HealthTracker: sys.HealthTracker(),
})
if err != nil {
t.Fatal(err)
}

View File

@ -298,7 +298,7 @@ func TestStateMachine(t *testing.T) {
sys := new(tsd.System)
store := new(testStateStorage)
sys.Set(store)
e, err := wgengine.NewFakeUserspaceEngine(logf, sys.Set)
e, err := wgengine.NewFakeUserspaceEngine(logf, sys.Set, sys.HealthTracker())
if err != nil {
t.Fatalf("NewFakeUserspaceEngine: %v", err)
}
@ -903,7 +903,7 @@ func TestEditPrefsHasNoKeys(t *testing.T) {
logf := tstest.WhileTestRunningLogger(t)
sys := new(tsd.System)
sys.Set(new(mem.Store))
e, err := wgengine.NewFakeUserspaceEngine(logf, sys.Set)
e, err := wgengine.NewFakeUserspaceEngine(logf, sys.Set, sys.HealthTracker())
if err != nil {
t.Fatalf("NewFakeUserspaceEngine: %v", err)
}

View File

@ -314,7 +314,7 @@ func newTestLocalBackend(t testing.TB) *ipnlocal.LocalBackend {
sys := new(tsd.System)
store := new(mem.Store)
sys.Set(store)
eng, err := wgengine.NewFakeUserspaceEngine(logf, sys.Set)
eng, err := wgengine.NewFakeUserspaceEngine(logf, sys.Set, sys.HealthTracker())
if err != nil {
t.Fatalf("NewFakeUserspaceEngine: %v", err)
}

View File

@ -53,6 +53,7 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netip.
ListenPort: 0,
Tun: t1,
SetSubsystem: s1.Set,
HealthTracker: s1.HealthTracker(),
})
if err != nil {
log.Fatalf("e1 init: %v", err)
@ -79,6 +80,7 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netip.
ListenPort: 0,
Tun: t2,
SetSubsystem: s2.Set,
HealthTracker: s2.HealthTracker(),
})
if err != nil {
log.Fatalf("e2 init: %v", err)

View File

@ -47,6 +47,7 @@ func TestInjectInboundLeak(t *testing.T) {
Tun: tunDev,
Dialer: dialer,
SetSubsystem: sys.Set,
HealthTracker: sys.HealthTracker(),
})
if err != nil {
t.Fatal(err)
@ -103,6 +104,7 @@ func makeNetstack(t *testing.T, config func(*Impl)) *Impl {
Tun: tunDev,
Dialer: dialer,
SetSubsystem: sys.Set,
HealthTracker: sys.HealthTracker(),
})
if err != nil {
t.Fatal(err)

View File

@ -47,6 +47,7 @@
"tailscale.com/util/deephash"
"tailscale.com/util/mak"
"tailscale.com/util/set"
"tailscale.com/util/testenv"
"tailscale.com/version"
"tailscale.com/wgengine/capture"
"tailscale.com/wgengine/filter"
@ -260,6 +261,10 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error)
var closePool closeOnErrorPool
defer closePool.closeAllIfError(&reterr)
if testenv.InTest() && conf.HealthTracker == nil {
panic("NewUserspaceEngine called without HealthTracker (being strict in tests)")
}
if conf.Tun == nil {
logf("[v1] using fake (no-op) tun device")
conf.Tun = tstun.NewFake()

View File

@ -19,7 +19,10 @@ func TestIsNetstack(t *testing.T) {
sys := new(tsd.System)
e, err := wgengine.NewUserspaceEngine(
tstest.WhileTestRunningLogger(t),
wgengine.Config{SetSubsystem: sys.Set},
wgengine.Config{
SetSubsystem: sys.Set,
HealthTracker: sys.HealthTracker(),
},
)
if err != nil {
t.Fatal(err)
@ -68,6 +71,7 @@ func TestIsNetstackRouter(t *testing.T) {
}
conf := tt.conf
conf.SetSubsystem = sys.Set
conf.HealthTracker = sys.HealthTracker()
e, err := wgengine.NewUserspaceEngine(logger.Discard, conf)
if err != nil {
t.Fatal(err)

View File

@ -15,6 +15,7 @@
"tailscale.com/cmd/testwrapper/flakytest"
"tailscale.com/control/controlknobs"
"tailscale.com/envknob"
"tailscale.com/health"
"tailscale.com/net/dns"
"tailscale.com/net/netaddr"
"tailscale.com/net/tstun"
@ -98,7 +99,8 @@ func nodeViews(v []*tailcfg.Node) []tailcfg.NodeView {
}
func TestUserspaceEngineReconfig(t *testing.T) {
e, err := NewFakeUserspaceEngine(t.Logf, 0)
ht := new(health.Tracker)
e, err := NewFakeUserspaceEngine(t.Logf, 0, ht)
if err != nil {
t.Fatal(err)
}
@ -164,9 +166,10 @@ func TestUserspaceEnginePortReconfig(t *testing.T) {
// Keep making a wgengine until we find an unused port
var ue *userspaceEngine
ht := new(health.Tracker)
for i := range 100 {
attempt := uint16(defaultPort + i)
e, err := NewFakeUserspaceEngine(t.Logf, attempt, &knobs)
e, err := NewFakeUserspaceEngine(t.Logf, attempt, &knobs, ht)
if err != nil {
t.Fatal(err)
}
@ -245,7 +248,8 @@ func TestUserspaceEnginePeerMTUReconfig(t *testing.T) {
var knobs controlknobs.Knobs
e, err := NewFakeUserspaceEngine(t.Logf, 0, &knobs)
ht := new(health.Tracker)
e, err := NewFakeUserspaceEngine(t.Logf, 0, &knobs, ht)
if err != nil {
t.Fatal(err)
}

View File

@ -7,6 +7,8 @@
"runtime"
"testing"
"time"
"tailscale.com/health"
)
func TestWatchdog(t *testing.T) {
@ -21,7 +23,8 @@ func TestWatchdog(t *testing.T) {
t.Run("default watchdog does not fire", func(t *testing.T) {
t.Parallel()
e, err := NewFakeUserspaceEngine(t.Logf, 0)
ht := new(health.Tracker)
e, err := NewFakeUserspaceEngine(t.Logf, 0, ht)
if err != nil {
t.Fatal(err)
}