Keep a fill warning for an unreadable bucket or log only when its condition was read open

Second review of #231: when neither the stream nor its condition can be
read, nothing is known of its fill, so none is said; the unread warning
covers it. Only a condition read back as open is kept, with its own words.

cmd/mesh-controller/bucket_log_fill.go

@@ -103,19 +103,12 @@ func unreadObservation(s bucketOrLog, why error) conditions.Observation {
 }
 
 // keptFilling is an open fill condition said again for a bucket or log that could not be read this
-// time: not knowing how full it is now is no reason to say it has room.
+// time: not knowing how full it is now is no reason to say it has room. Only ever made from a condition
+// read back as open — its own summary and severity, never words made up for it.
 func keptFilling(s bucketOrLog, open conditions.Condition, why error) conditions.Observation {
-	severity := open.Severity
-	if severity == "" {
-		severity = conditions.Warning
-	}
-	summary := open.Summary
-	if summary == "" {
-		summary = fmt.Sprintf("%s's %s %s was filling when it was last read", s.Module, s.Kind, s.Name)
-	}
 	return conditions.Observation{
 		Scope: conditions.ScopeBus, ID: s.Stream, Token: "filling", Kind: kindBucketOrLogFilling,
-		Severity: severity, Summary: summary,
+		Severity: open.Severity, Summary: open.Summary,
 		Said: fmt.Sprintf("module %s, %s %s (stream %s): not read this time (%v); kept open as it was",
 			s.Module, s.Kind, s.Name, s.Stream, why),
 	}
@@ -129,18 +122,21 @@ func mibWords(b uint64) string {
 // readFill is how full one bucket or log is on the bus; found false when it is not on the bus.
 type readFill func(ctx context.Context, s bucketOrLog) (f filled, found bool, err error)
 
-// openFill is the fill condition open under a key, if one is.
-type openFill func(ctx context.Context, key string) (conditions.Condition, bool)
+// openFill is the fill condition open under a key, if one is, or why it could not be read.
+type openFill func(ctx context.Context, key string) (conditions.Condition, bool, error)
 
 // judgeFills judges every bucket and log on its own: one that cannot be read is said as unread, with
-// its open fill condition kept, and every other is judged by its fill.
+// its fill condition kept only when that condition was read back and is open — when it cannot be read
+// either, nothing is said of its fill, which the unread finding already covers. Every other is judged
+// by its fill; for one of those, a condition that cannot be read is taken as open, so an unknown never
+// clears a fill measured between fillClearBelow and fillRaiseAt.
 func judgeFills(ctx context.Context, all []bucketOrLog, read readFill, open openFill) []conditions.Observation {
 	var out []conditions.Observation
 	for _, s := range all {
 		f, found, err := read(ctx, s)
 		if err != nil {
 			out = append(out, unreadObservation(s, err))
-			if c, isOpen := open(ctx, fillKey(s)); isOpen {
+			if c, isOpen, cerr := open(ctx, fillKey(s)); cerr == nil && isOpen {
 				out = append(out, keptFilling(s, c, err))
 			}
 			continue
@@ -148,7 +144,8 @@ func judgeFills(ctx context.Context, all []bucketOrLog, read readFill, open open
 		if !found {
 			continue // not on the bus: created on the controller's next raise, and nothing there can fill
 		}
-		_, isOpen := open(ctx, fillKey(s))
+		_, isOpen, cerr := open(ctx, fillKey(s))
+		isOpen = isOpen || cerr != nil
 		if o, said := fillObservation(s, f, isOpen); said {
 			out = append(out, o)
 		}
@@ -200,13 +197,11 @@ func probeFill(ctx context.Context, d *doctor) ([]conditions.Observation, error)
 		}
 		return filled{Bytes: info.State.Bytes, Max: uint64(info.Config.MaxBytes)}, true, nil
 	}
-	open := func(ctx context.Context, key string) (conditions.Condition, bool) {
+	open := func(ctx context.Context, key string) (conditions.Condition, bool, error) {
 		if d.keeper == nil {
-			return conditions.Condition{}, false
+			return conditions.Condition{}, false, nil
 		}
-		// One that cannot be read is taken as open, as confirm.go takes it: unknown is no reason to clear.
-		c, isOpen, err := d.keeper.Get(ctx, key)
-		return c, isOpen || err != nil
+		return d.keeper.Get(ctx, key)
 	}
 	return judgeFills(ctx, all, read, open), nil
 }

cmd/mesh-controller/bucket_log_fill_test.go

@@ -84,12 +84,12 @@ func TestAnUnreadableBucketOrLogIsSaidAndTheRestAreStillJudged(t *testing.T) {
 		}
 		return filled{}, false, nil
 	}
-	open := func(_ context.Context, key string) (conditions.Condition, bool) {
+	open := func(_ context.Context, key string) (conditions.Condition, bool, error) {
 		if key == fillKey(brokenOpen) {
 			return conditions.Condition{Key: key, Severity: conditions.Warning,
-				Summary: "a's log was-filling holds 800.0 MiB of 1024.0 MiB, 78%"}, true
+				Summary: "a's log was-filling holds 800.0 MiB of 1024.0 MiB, 78%"}, true, nil
 		}
-		return conditions.Condition{}, false
+		return conditions.Condition{}, false, nil
 	}
 	got := map[string]conditions.Observation{}
 	for _, o := range judgeFills(context.Background(), []bucketOrLog{broken, brokenOpen, full, empty, absent}, read, open) {
@@ -128,3 +128,38 @@ func TestAFillingOrUnreadBucketOrLogIsSaidInPlainWords(t *testing.T) {
 		}
 	}
 }
+
+// **When neither the bucket or log nor its condition can be read, nothing is said of its fill** (second
+// review of #231): the unread finding covers it, and no fill is invented for it. For one that is read and
+// measured between seven tenths and three quarters, a condition that cannot be read is taken as open, so
+// an unknown never clears it.
+func TestNothingIsSaidOfAFillWhenNeitherItNorItsConditionCanBeRead(t *testing.T) {
+	const mib = 1024 * 1024
+	broken := bucketOrLog{Module: "a", Kind: "log", Name: "changes", Stream: "LOG_a_changes"}
+	between := bucketOrLog{Module: "b", Kind: "bucket", Name: "issues", Stream: "KV_b_issues"}
+	read := func(_ context.Context, s bucketOrLog) (filled, bool, error) {
+		if s == broken {
+			return filled{}, false, errors.New("the bus did not answer")
+		}
+		return filled{Bytes: 72 * mib, Max: 100 * mib}, true, nil
+	}
+	open := func(context.Context, string) (conditions.Condition, bool, error) {
+		return conditions.Condition{}, false, errors.New("the conditions bucket did not answer")
+	}
+	got := map[string]conditions.Observation{}
+	for _, o := range judgeFills(context.Background(), []bucketOrLog{broken, between}, read, open) {
+		got[o.Key()] = o
+	}
+	if o, said := got[fillKey(broken)]; said {
+		t.Fatalf("a fill was said for a log whose fill and condition could not be read: %+v", o)
+	}
+	if _, said := got[conditions.Key(conditions.ScopeBus, broken.Stream, "unread")]; !said {
+		t.Error("the log that could not be read was not said as unread")
+	}
+	if _, kept := got[fillKey(between)]; !kept {
+		t.Error("a bucket at 72% whose condition could not be read was cleared")
+	}
+	if len(got) != 2 {
+		t.Errorf("said %d findings, want 2: %v", len(got), got)
+	}
+}