diff --git a/health/health.go b/health/health.go index 079b3195c..fa608ea73 100644 --- a/health/health.go +++ b/health/health.go @@ -214,9 +214,11 @@ type Warnable struct { // TODO(angott): turn this into a SeverityFunc, which allows the Warnable to change its severity based on // the Args of the unhappy state, just like we do in the Text function. Severity Severity - // DependsOn is a set of Warnables that this Warnable depends, on and need to be healthy - // before this Warnable can also be healthy again. The GUI can use this information to ignore + // DependsOn is a set of Warnables that this Warnable depends on and need to be healthy + // before this Warnable is relevant. The GUI can use this information to ignore // this Warnable if one of its dependencies is unhealthy. + // That is, if any of these Warnables are unhealthy, then this Warnable is not relevant + // and should be considered healthy to bother the user about. DependsOn []*Warnable // MapDebugFlag is a MapRequest.DebugFlag that is sent to control when this Warnable is unhealthy @@ -940,6 +942,9 @@ func (t *Tracker) stringsLocked() []string { // Do not append invisible warnings. continue } + if t.isEffectivelyHealthyLocked(w) { + continue + } if ws.Args == nil { result = append(result, w.Text(Args{})) } else { diff --git a/health/health_test.go b/health/health_test.go index ebdddc988..cc7b9d5aa 100644 --- a/health/health_test.go +++ b/health/health_test.go @@ -257,9 +257,15 @@ func TestCheckDependsOnAppearsInUnhealthyState(t *testing.T) { } ht.SetUnhealthy(w2, Args{ArgError: "w2 is also unhealthy now"}) us2, ok := ht.CurrentState().Warnings[w2.Code] - if !ok { - t.Fatalf("Expected an UnhealthyState for w2, got nothing") + if ok { + t.Fatalf("Saw w2 being unhealthy but it shouldn't be, as it depends on unhealthy w1") } + ht.SetHealthy(w1) + us2, ok = ht.CurrentState().Warnings[w2.Code] + if !ok { + t.Fatalf("w2 wasn't unhealthy; want it to be unhealthy now that w1 is back healthy") + } + wantDependsOn = slices.Concat([]WarnableCode{w1.Code}, wantDependsOn) if !reflect.DeepEqual(us2.DependsOn, wantDependsOn) { t.Fatalf("Expected DependsOn = %v in the unhealthy state, got: %v", wantDependsOn, us2.DependsOn) diff --git a/health/state.go b/health/state.go index 17a646794..3bfa6f99b 100644 --- a/health/state.go +++ b/health/state.go @@ -90,6 +90,11 @@ func (t *Tracker) CurrentState() *State { // Skip invisible Warnables. continue } + if t.isEffectivelyHealthyLocked(w) { + // Skip Warnables that are unhealthy if they have dependencies + // that are unhealthy. + continue + } wm[w.Code] = *w.unhealthyState(ws) } @@ -97,3 +102,23 @@ func (t *Tracker) CurrentState() *State { Warnings: wm, } } + +// isEffectivelyHealthyLocked reports whether w is effectively healthy. +// That means it's either actually healthy or it has a dependency that +// that's unhealthy, so we should treat w as healthy to not spam users +// with multiple warnings when only the root cause is relevant. +func (t *Tracker) isEffectivelyHealthyLocked(w *Warnable) bool { + if _, ok := t.warnableVal[w]; !ok { + // Warnable not found in the tracker. So healthy. + return true + } + for _, d := range w.DependsOn { + if !t.isEffectivelyHealthyLocked(d) { + // If one of our deps is unhealthy, we're healthy. + return true + } + } + // If we have no unhealthy deps and had warnableVal set, + // we're unhealthy. + return false +}