health: skip no-derp-home warnings when not connectedjonathan/derp-health-spam

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 <jonathan@tailscale.com>

2 files changed, 76 insertions, 1 deletions

diff --git a/health/health.go b/health/health.go
index 0cfe570c4..9e28712b3 100644
--- a/health/health.go
+++ b/health/health.go
@@ -1140,7 +1140,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 953c4dca2..5b76b7982 100644
--- a/health/health_test.go
+++ b/health/health_test.go
@@ -934,6 +934,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.