Revert "break up health watchers"

This reverts commit 2e5cf9e15a9348145208a3bf8ff5a55001b662b9.

control/controlclient/direct.go

@@ -1614,6 +1614,10 @@ func postPingResult(start time.Time, logf logger.Logf, c *http.Client, pr *tailc
 // ReportHealthChange reports to the control plane a change to this node's
 // health. w must be non-nil. us can be nil to indicate a healthy state for w.
 func (c *Direct) ReportHealthChange(w *health.Warnable, us *health.UnhealthyState) {
+	// TODO: Don't tell control about health messages that came from control?
+	if w == nil {
+		return
+	}
 	if w == health.NetworkStatusWarnable || w == health.IPNStateWarnable || w == health.LoginStateWarnable {
 		// We don't report these. These include things like the network is down
 		// (in which case we can't report anyway) or the user wanted things

health/health.go

@@ -87,11 +87,9 @@ type Tracker struct {
 
 	// sysErr maps subsystems to their current error (or nil if the subsystem is healthy)
 	// Deprecated: using Warnables should be preferred
-	sysErr      map[Subsystem]error
+	sysErr   map[Subsystem]error
-	watchers    set.HandleSet[func(*Warnable, *UnhealthyState)] // opt func to run if error state changes
+	watchers set.HandleSet[func(*Warnable, *UnhealthyState)] // opt func to run if error state changes
-	anyWatchers set.HandleSet[func()]                           // opt func to run if any error state changes
+	timer    tstime.TimerController
-
-	timer tstime.TimerController
 
 	latestVersion   *tailcfg.ClientVersion // or nil
 	checkForUpdates bool
@@ -425,25 +423,6 @@ func (t *Tracker) setUnhealthyLocked(w *Warnable, args Args) {
 			})
 			mak.Set(&t.pendingVisibleTimers, w, tc)
 		}
-
-		for _, cb := range t.anyWatchers {
-			if w.IsVisible(ws, t.now) {
-				go cb()
-				continue
-			}
-			visibleIn := w.TimeToVisible - t.now().Sub(brokenSince)
-			var tc tstime.TimerController = t.clock().AfterFunc(visibleIn, func() {
-				t.mu.Lock()
-				defer t.mu.Unlock()
-				// 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()
-					delete(t.pendingVisibleTimers, w)
-				}
-			})
-			mak.Set(&t.pendingVisibleTimers, w, tc)
-		}
 	}
 }
 
@@ -474,9 +453,6 @@ func (t *Tracker) setHealthyLocked(w *Warnable) {
 	for _, cb := range t.watchers {
 		go cb(w, nil)
 	}
-	for _, cb := range t.anyWatchers {
-		go cb()
-	}
 }
 
 // AppendWarnableDebugFlags appends to base any health items that are currently in failed
@@ -533,28 +509,6 @@ func (t *Tracker) RegisterWatcher(cb func(w *Warnable, r *UnhealthyState)) (unre
 	}
 }
 
-// this function will register an observer for notifications when the set of
-// current UnhealthyStates (Warnables + control Health messages) changes. Unlike
-// RegisterWatcher it will not fire for each Warnable but only once any time any
-// amount of Warnables (or control health messages) change.
-func (t *Tracker) RegisterAnyWatcher(cb func()) (unregister func()) {
-	if t.nil() {
-		return func() {}
-	}
-	t.initOnce.Do(t.doOnceInit)
-	t.mu.Lock()
-	defer t.mu.Unlock()
-	if t.anyWatchers == nil {
-		t.anyWatchers = set.HandleSet[func()]{}
-	}
-	handle := t.anyWatchers.Add(cb)
-	return func() {
-		t.mu.Lock()
-		defer t.mu.Unlock()
-		delete(t.anyWatchers, handle)
-	}
-}
-
 // SetRouterHealth sets the state of the wgengine/router.Router.
 //
 // Deprecated: Warnables should be preferred over Subsystem errors.
@@ -1198,8 +1152,13 @@ func (t *Tracker) updateBuiltinWarnablesLocked() {
 	}
 
 	if t.controlHealth != nil {
-		for _, cb := range t.anyWatchers {
+		// for _, cb := range t.watchers {
-			go cb()
+		// 	go cb(nil, nil)
+		// }
+		if len(t.controlHealth) > 0 {
+			t.setUnhealthyLocked(controlHealthWarnable, nil)
+		} else {
+			t.setHealthyLocked(controlHealthWarnable)
 		}
 	}
 

ipn/ipnlocal/local.go

@@ -240,7 +240,6 @@ type LocalBackend struct {
 	backendLogID             logid.PublicID
 	unregisterNetMon         func()
 	unregisterHealthWatch    func()
-	unregisterAnyHealthWatch func()
 	unregisterSysPolicyWatch func()
 	portpoll                 *portlist.Poller // may be nil
 	portpollOnce             sync.Once        // guards starting readPoller
@@ -610,7 +609,6 @@ func NewLocalBackend(logf logger.Logf, logID logid.PublicID, sys *tsd.System, lo
 	b.unregisterNetMon = netMon.RegisterChangeCallback(b.linkChange)
 
 	b.unregisterHealthWatch = b.health.RegisterWatcher(b.onHealthChange)
-	b.unregisterAnyHealthWatch = b.health.RegisterAnyWatcher(b.onAnyHealthChange)
 
 	if tunWrap, ok := b.sys.Tun.GetOK(); ok {
 		tunWrap.PeerAPIPort = b.GetPeerAPIPort
@@ -970,16 +968,15 @@ func (b *LocalBackend) linkChange(delta *netmon.ChangeDelta) {
 }
 
 func (b *LocalBackend) onHealthChange(w *health.Warnable, us *health.UnhealthyState) {
+	if w == nil {
+		return
+	}
 	if us == nil {
 		b.logf("health(warnable=%s): ok", w.Code)
 	} else {
 		b.logf("health(warnable=%s): error: %s", w.Code, us.Text)
 	}
 
-	b.onAnyHealthChange()
-}
-
-func (b *LocalBackend) onAnyHealthChange() {
 	// Whenever health changes, send the current health state to the frontend.
 	state := b.health.CurrentState()
 	b.send(ipn.Notify{
@@ -1130,7 +1127,6 @@ func (b *LocalBackend) Shutdown() {
 
 	b.unregisterNetMon()
 	b.unregisterHealthWatch()
-	b.unregisterAnyHealthWatch()
 	b.unregisterSysPolicyWatch()
 	if cc != nil {
 		cc.Shutdown()