diff --git a/control/controlclient/map_test.go b/control/controlclient/map_test.go index 09441d066..ccc57ae2b 100644 --- a/control/controlclient/map_test.go +++ b/control/controlclient/map_test.go @@ -17,6 +17,7 @@ import ( "github.com/google/go-cmp/cmp" "go4.org/mem" "tailscale.com/control/controlknobs" + "tailscale.com/health" "tailscale.com/tailcfg" "tailscale.com/tstest" "tailscale.com/tstime" @@ -1136,3 +1137,34 @@ func BenchmarkMapSessionDelta(b *testing.B) { }) } } + +// TestNetmapHealthIntegration checks that we get the expected health warnings +// from processing a map response and passing the NetworkMap to a health tracker +func TestNetmapHealthIntegration(t *testing.T) { + ms := newTestMapSession(t, nil) + ht := health.Tracker{} + + ht.SetIPNState("NeedsLogin", true) + ht.GotStreamedMapResponse() + + nm := ms.netmapForResponse(&tailcfg.MapResponse{ + Health: []string{"Test message"}, + }) + ht.SetControlHealth(nm.ControlHealth) + + state := ht.CurrentState() + warning, ok := state.Warnings["control-health"] + + if !ok { + t.Fatal("no warning found in current state with code 'control-health'") + } + if got, want := warning.Title, "Coordination server reports an issue"; got != want { + t.Errorf("warning.Title = %q, want %q", got, want) + } + if got, want := warning.Severity, health.SeverityMedium; got != want { + t.Errorf("warning.Severity = %s, want %s", got, want) + } + if got, want := warning.Text, "The coordination server is reporting an health issue: Test message"; got != want { + t.Errorf("warning.Text = %q, want %q", got, want) + } +} diff --git a/health/health.go b/health/health.go index b0733f353..65d4402ae 100644 --- a/health/health.go +++ b/health/health.go @@ -402,7 +402,7 @@ func (t *Tracker) setUnhealthyLocked(w *Warnable, args Args) { // executed immediately. Otherwise, the callback should be enqueued to run once the Warnable // becomes visible. if w.IsVisible(ws, t.now) { - go cb(w, w.unhealthyState(ws)) + cb(w, w.unhealthyState(ws)) continue } @@ -415,7 +415,7 @@ func (t *Tracker) setUnhealthyLocked(w *Warnable, args Args) { // Check if the Warnable is still unhealthy, as it could have become healthy between the time // the timer was set for and the time it was executed. if t.warnableVal[w] != nil { - go cb(w, w.unhealthyState(ws)) + cb(w, w.unhealthyState(ws)) delete(t.pendingVisibleTimers, w) } }) @@ -449,7 +449,7 @@ func (t *Tracker) setHealthyLocked(w *Warnable) { } for _, cb := range t.watchers { - go cb(w, nil) + cb(w, nil) } } @@ -483,6 +483,16 @@ func (t *Tracker) AppendWarnableDebugFlags(base []string) []string { // The provided callback function will be executed in its own goroutine. The returned function can be used // to unregister the callback. func (t *Tracker) RegisterWatcher(cb func(w *Warnable, r *UnhealthyState)) (unregister func()) { + return t.registerSyncWatcher(func(w *Warnable, r *UnhealthyState) { + go cb(w, r) + }) +} + +// registerSyncWatcher adds a function that will be called whenever the health +// state of any Warnable changes. The provided callback function will be +// executed synchronously. Call RegisterWatcher to register any callbacks that +// won't return from execution immediately. +func (t *Tracker) registerSyncWatcher(cb func(w *Warnable, r *UnhealthyState)) (unregister func()) { if t.nil() { return func() {} } diff --git a/health/health_test.go b/health/health_test.go index abc0ec07e..aa3904581 100644 --- a/health/health_test.go +++ b/health/health_test.go @@ -451,3 +451,88 @@ func TestNoDERPHomeWarnableManual(t *testing.T) { t.Fatalf("got unexpected noDERPHomeWarnable warnable: %v", ws) } } + +func TestControlHealth(t *testing.T) { + ht := Tracker{} + ht.SetIPNState("NeedsLogin", true) + ht.GotStreamedMapResponse() + + ht.SetControlHealth([]string{"Test message"}) + state := ht.CurrentState() + warning, ok := state.Warnings["control-health"] + + if !ok { + t.Fatal("no warning found in current state with code 'control-health'") + } + if got, want := warning.Title, "Coordination server reports an issue"; got != want { + t.Errorf("warning.Title = %q, want %q", got, want) + } + if got, want := warning.Severity, SeverityMedium; got != want { + t.Errorf("warning.Severity = %s, want %s", got, want) + } + if got, want := warning.Text, "The coordination server is reporting an health issue: Test message"; got != want { + t.Errorf("warning.Text = %q, want %q", got, want) + } +} + +func TestControlHealthNotifiesOnChange(t *testing.T) { + ht := Tracker{} + ht.SetIPNState("NeedsLogin", true) + ht.GotStreamedMapResponse() + + gotNotified := false + ht.registerSyncWatcher(func(_ *Warnable, _ *UnhealthyState) { + gotNotified = true + }) + + ht.SetControlHealth([]string{"Test message"}) + + if !gotNotified { + t.Errorf("watcher did not get called, want it to be called") + } +} + +func TestControlHealthNoNotifyOnUnchanged(t *testing.T) { + ht := Tracker{} + ht.SetIPNState("NeedsLogin", true) + ht.GotStreamedMapResponse() + + // Set up an existing control health issue + ht.SetControlHealth([]string{"Test message"}) + + // Now register our watcher + gotNotified := false + ht.registerSyncWatcher(func(_ *Warnable, _ *UnhealthyState) { + gotNotified = true + }) + + // Send the same control health message again - should not notify + ht.SetControlHealth([]string{"Test message"}) + + if gotNotified { + t.Errorf("watcher got called, want it to not be called") + } +} + +func TestControlHealthIgnoredOutsideMapPoll(t *testing.T) { + ht := Tracker{} + ht.SetIPNState("NeedsLogin", true) + + gotNotified := false + ht.registerSyncWatcher(func(_ *Warnable, _ *UnhealthyState) { + gotNotified = true + }) + + ht.SetControlHealth([]string{"Test message"}) + + state := ht.CurrentState() + _, ok := state.Warnings["control-health"] + + if ok { + t.Error("got a warning with code 'control-health', want none") + } + + if gotNotified { + t.Error("watcher got called, want it to not be called") + } +}