From affb75791fbab6b3528ab7d63935c65c832151c2 Mon Sep 17 00:00:00 2001 From: Jonathan Nobels Date: Tue, 23 Sep 2025 14:14:18 -0400 Subject: [PATCH] health: skip no-derp-home warnings when not connected updates tailscale/corp#32661 We were pushing noDerpConnection health warnings after a tailscale down which is spammy in the logs. The lack of a derp-home when tailscale isn't running isn't a warnable condition. Signed-off-by: Jonathan Nobels --- health/health.go | 2 +- health/health_test.go | 75 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/health/health.go b/health/health.go index 3d1c46a3d..431997e5b 100644 --- a/health/health.go +++ b/health/health.go @@ -1131,7 +1131,7 @@ func (t *Tracker) updateBuiltinWarnablesLocked() { ArgDuration: d.Round(time.Second).String(), }) } - } else if homeDERP != 0 { + } else if homeDERP != 0 && t.ipnWantRunning && !recentlyOn { t.setUnhealthyLocked(noDERPConnectionWarnable, Args{ ArgDERPRegionID: fmt.Sprint(homeDERP), ArgDERPRegionName: t.derpRegionNameLocked(homeDERP), diff --git a/health/health_test.go b/health/health_test.go index 3ada37755..873f11166 100644 --- a/health/health_test.go +++ b/health/health_test.go @@ -887,6 +887,81 @@ func TestCurrentStateETagControlHealth(t *testing.T) { } } +// TestIgnoreNoDERPConnectionUnlessRunning check that noDERPConnectionWarnable is only +// posted when wantRunning is true and has been set relatively recently. +func TestIgnoreNoDERPConnectionUnlessRunning(t *testing.T) { + type test struct { + name string + derpHomeRegion int + ipnState string + ipnWantRunning bool + ipnWantRunningLastTrue time.Time + wantWarning bool + } + tests := []test{ + { + // wantRunning was far enough in the past that we should warn + name: "Running Derp Home Set", + derpHomeRegion: 1, + ipnState: "Running", + ipnWantRunning: true, + ipnWantRunningLastTrue: time.Now().Add(-time.Minute), + wantWarning: true, + }, + { + // wantRunning wasn't set far enough in the past + name: "Running Derp Home Set Recently", + derpHomeRegion: 1, + ipnState: "Running", + ipnWantRunning: true, + ipnWantRunningLastTrue: time.Now().Add(-1 * time.Second), + wantWarning: false, + }, + { + // no warning if derpHomeRegion is unset + name: "Running No Derp Home Set", + derpHomeRegion: 0, + ipnState: "Running", + ipnWantRunning: true, + ipnWantRunningLastTrue: time.Now().Add(-time.Minute), + wantWarning: false, + }, + { + // no warning in the stopped state + name: "Stopped Derp Home Set", + derpHomeRegion: 1, + ipnState: "Stopped", + ipnWantRunning: false, + ipnWantRunningLastTrue: time.Now().Add(-time.Minute), + wantWarning: false, + }, + { + // no warning in the stopped state + name: "Stopped Derp Home Set", + derpHomeRegion: 0, + ipnState: "Stopped", + ipnWantRunning: false, + ipnWantRunningLastTrue: time.Now().Add(-time.Minute), + wantWarning: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ht := NewTracker(eventbustest.NewBus(t)) + ht.derpHomeRegion = tc.derpHomeRegion + ht.SetIPNState(tc.ipnState, tc.ipnWantRunning) + ht.ipnWantRunningLastTrue = tc.ipnWantRunningLastTrue + ht.updateBuiltinWarnablesLocked() + + _, ok := ht.warnableVal[noDERPConnectionWarnable] + if ok != tc.wantWarning { + t.Fatalf("got warning = %v, want %v", ok, tc.wantWarning) + } + }) + } +} + // TestCurrentStateETagWarnable tests that the ETag on an [UnhealthyState] // created from a Warnable & returned by [Tracker.CurrentState] is different // when the details of the Warnable are different.