ipn/ipnlocal: deflake some tests

* don't try to re-Start (and thus create a new client) during Shutdown
* in tests, wait for controlclient to fully shut down when replacing it
* log a bit more

Updates tailscale/corp#14139
Updates tailscale/corp#13175 etc
Updates #9178 and its flakes.

Change-Id: I3ed2440644dc157aa6e616fe36fbd29a6056846c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This commit is contained in:
Brad Fitzpatrick 2023-08-31 09:13:04 -07:00 committed by Brad Fitzpatrick
parent 04e1ce0034
commit 21247f766f

View File

@ -565,7 +565,14 @@ func (b *LocalBackend) Shutdown() {
b.mu.Unlock()
ctx, cancel := context.WithTimeout(b.ctx, 5*time.Second)
defer cancel()
b.LogoutSync(ctx) // best effort
t0 := time.Now()
err := b.LogoutSync(ctx) // best effort
td := time.Since(t0).Round(time.Millisecond)
if err != nil {
b.logf("failed to log out ephemeral node on shutdown after %v: %v", td, err)
} else {
b.logf("logged out ephemeral node on shutdown")
}
b.mu.Lock()
}
cc := b.cc
@ -977,6 +984,7 @@ func (b *LocalBackend) SetControlClientStatus(st controlclient.Status) {
// Lock b once and do only the things that require locking.
b.mu.Lock()
inShutdown := b.shutdownCalled
if st.LogoutFinished() {
if p := b.pm.CurrentPrefs(); !p.Persist().Valid() || p.Persist().UserProfile().LoginName() == "" {
@ -986,6 +994,10 @@ func (b *LocalBackend) SetControlClientStatus(st controlclient.Status) {
if err := b.pm.DeleteProfile(b.pm.CurrentProfile().ID); err != nil {
b.logf("error deleting profile: %v", err)
}
if inShutdown {
b.mu.Unlock()
return
}
if err := b.resetForProfileChangeLockedOnEntry(); err != nil {
b.logf("resetForProfileChangeLockedOnEntry err: %v", err)
}
@ -3872,7 +3884,18 @@ func (b *LocalBackend) resetControlClientLockedAsync() {
b.numClientStatusCalls.Add(1)
}
go b.cc.Shutdown()
if testenv.InTest() {
// From 2021-04-20 to 2023-08-31 we always did this shutdown
// asynchronously. But tests flaked on it sometimes, as our
// tests often do resource leak checks at the end to make sure
// everything's shut down. So do this synchronously in tests
// to deflake tests.
//
// TODO(bradfitz,maisem): do this always?
b.cc.Shutdown()
} else {
go b.cc.Shutdown()
}
b.cc = nil
b.ccAuto = nil
}