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>
pull/12055/head
Brad Fitzpatrick 6 months ago
parent 727c0d6cfd
commit 85a63c8511

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

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

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

@ -672,7 +672,10 @@ func newTestBackend(t *testing.T) *LocalBackend {
} }
sys := &tsd.System{} 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 { if err != nil {
t.Fatal(err) t.Fatal(err)
} }

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

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

@ -48,11 +48,12 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netip.
} }
s1 := new(tsd.System) s1 := new(tsd.System)
e1, err := wgengine.NewUserspaceEngine(l1, wgengine.Config{ e1, err := wgengine.NewUserspaceEngine(l1, wgengine.Config{
Router: router.NewFake(l1), Router: router.NewFake(l1),
NetMon: nil, NetMon: nil,
ListenPort: 0, ListenPort: 0,
Tun: t1, Tun: t1,
SetSubsystem: s1.Set, SetSubsystem: s1.Set,
HealthTracker: s1.HealthTracker(),
}) })
if err != nil { if err != nil {
log.Fatalf("e1 init: %v", err) log.Fatalf("e1 init: %v", err)
@ -74,11 +75,12 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netip.
} }
s2 := new(tsd.System) s2 := new(tsd.System)
e2, err := wgengine.NewUserspaceEngine(l2, wgengine.Config{ e2, err := wgengine.NewUserspaceEngine(l2, wgengine.Config{
Router: router.NewFake(l2), Router: router.NewFake(l2),
NetMon: nil, NetMon: nil,
ListenPort: 0, ListenPort: 0,
Tun: t2, Tun: t2,
SetSubsystem: s2.Set, SetSubsystem: s2.Set,
HealthTracker: s2.HealthTracker(),
}) })
if err != nil { if err != nil {
log.Fatalf("e2 init: %v", err) log.Fatalf("e2 init: %v", err)

@ -44,9 +44,10 @@ func TestInjectInboundLeak(t *testing.T) {
} }
sys := new(tsd.System) sys := new(tsd.System)
eng, err := wgengine.NewUserspaceEngine(logf, wgengine.Config{ eng, err := wgengine.NewUserspaceEngine(logf, wgengine.Config{
Tun: tunDev, Tun: tunDev,
Dialer: dialer, Dialer: dialer,
SetSubsystem: sys.Set, SetSubsystem: sys.Set,
HealthTracker: sys.HealthTracker(),
}) })
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
@ -100,9 +101,10 @@ func makeNetstack(t *testing.T, config func(*Impl)) *Impl {
dialer := new(tsdial.Dialer) dialer := new(tsdial.Dialer)
logf := tstest.WhileTestRunningLogger(t) logf := tstest.WhileTestRunningLogger(t)
eng, err := wgengine.NewUserspaceEngine(logf, wgengine.Config{ eng, err := wgengine.NewUserspaceEngine(logf, wgengine.Config{
Tun: tunDev, Tun: tunDev,
Dialer: dialer, Dialer: dialer,
SetSubsystem: sys.Set, SetSubsystem: sys.Set,
HealthTracker: sys.HealthTracker(),
}) })
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)

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

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

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

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

Loading…
Cancel
Save