mesh-provisioner: say a lasting unpublished hold by time, not by passes (issue 940, review 2)

A pass runs on every change of its node, and a newly placed module stays
unpublished for the minutes its build takes, so counting passes made every
new placement unhealthy while nothing was wrong. The health now says a hold
only once a module has stayed unpublished at one build for longer than 45
minutes (the build step's bound is 30), with since when; a published module
or another build starts the clock again.

Claude-Session: https://claude.ai/code/session_01NcbgfJ9Gum47zkFKWRaovL

modules/mesh-provisioner/cmd/mesh-provisioner/holder.go

@@ -130,8 +130,8 @@ type Holder struct {
 	mu sync.Mutex
 	// faults is, by node, the retirement a pass held back as a fault of the mesh (issue 940), said by provisioner_health.
 	faults map[string]string
-	// unpublished is, by node and module, how many passes in a row held the module because its composition was not
+	// unpublished is, by node and module, since when the module is held because its composition at this build is not
-	// published, with the hold's words: one held more than unpublishedPasses is said by provisioner_health.
+	// published, with the hold's words: one held longer than unpublishedFor is said by provisioner_health.
 	unpublished map[string]map[string]unpublishedHold
 }
 
@@ -1174,16 +1174,18 @@ func (h *Holder) clearFault(node string) {
 	}
 }
 
-// unpublishedPasses is how many passes in a row a module may be held unpublished before the health says it.
+// unpublishedFor is how long a module may be held unpublished at one build before the health says it: longer than a
-const unpublishedPasses = 2
+// build takes (the build step's bound is 30 minutes), so a newly placed module waiting for its build is no fault. A
+// pass runs on every change of its node, so passes are no measure of it (review 2 of issue 940).
+const unpublishedFor = 45 * time.Minute
 
 type unpublishedHold struct {
-	passes int
+	since time.Time
-	why    string
+	why   string // the hold's words, which name the build
 }
 
-// countUnpublished counts, for each module held unpublished on node, the passes in a row it was; a module published
+// countUnpublished keeps, for each module held unpublished on node, since when it is held at that build; a module
-// again is forgotten.
+// published again, or held at another build, starts again.
 func (h *Holder) countUnpublished(node string, modules []string, held map[string]string) {
 	if h.unpublished == nil {
 		h.unpublished = map[string]map[string]unpublishedHold{}
@@ -1191,7 +1193,11 @@ func (h *Holder) countUnpublished(node string, modules []string, held map[string
 	was := h.unpublished[node]
 	now := map[string]unpublishedHold{}
 	for _, m := range modules {
-		now[m] = unpublishedHold{passes: was[m].passes + 1, why: held[m]}
+		u, ok := was[m]
+		if !ok || u.why != held[m] {
+			u = unpublishedHold{since: h.Now().UTC(), why: held[m]}
+		}
+		now[m] = u
 	}
 	h.unpublished[node] = now
 }
@@ -1207,16 +1213,17 @@ func (h *Holder) Faults() map[string]string {
 	return out
 }
 
-// LastingHolds is each module held unpublished for more than unpublishedPasses passes in a row, as "<node>.<module>"
+// LastingHolds is each module held unpublished at one build for longer than unpublishedFor, as "<node>.<module>" with
-// with the hold's words, which name its build.
+// the hold's words, which name its build, and since when.
 func (h *Holder) LastingHolds() map[string]string {
 	h.mu.Lock()
 	defer h.mu.Unlock()
 	out := map[string]string{}
+	now := h.Now()
 	for n, mods := range h.unpublished {
 		for m, u := range mods {
-			if u.passes > unpublishedPasses {
+			if now.Sub(u.since) > unpublishedFor {
-				out[n+"."+m] = fmt.Sprintf("%s (held %d passes in a row)", u.why, u.passes)
+				out[n+"."+m] = fmt.Sprintf("%s (held since %s UTC)", u.why, u.since.Format("2006-01-02 15:04"))
 			}
 		}
 	}

modules/mesh-provisioner/cmd/mesh-provisioner/holder_test.go

@@ -603,11 +603,34 @@ func TestARetirementIsJudgedOnlyFromAWholePicture(t *testing.T) {
 				m.unpublished["novox"] = []string{"gitea"}
 				m.heldAt["novox"] = map[string]string{"gitea": "the composition of gitea at " + fp + " is not published yet"}
 			}, retiring: 1, asked: 1, after: func(t *testing.T, m *mesh) {
-				// Held for more than two passes in a row: the health names the module and its build.
+				// Held at one build: healthy over many passes within the bound (a build takes minutes), unhealthy past it,
+				// naming the module, its build and since when; another build starts the clock again.
+				now := m.h.Now()
+				m.h.Now = func() time.Time { return now }
+				for _, row := range []struct {
+					name    string
+					after   time.Duration
+					healthy bool
+				}{
+					{"many passes within the bound", 0, true},
+					{"still within the bound", unpublishedFor - time.Minute, true},
+					{"past the bound", unpublishedFor + time.Minute, false},
+				} {
+					now = now.Add(row.after)
+					for range 5 {
+						m.compose(t, "novox")
+					}
+					a := m.h.health()
+					if a.Healthy != row.healthy || (!row.healthy && (!strings.Contains(a.Why, "novox.gitea") ||
+						!strings.Contains(a.Why, fp) || !strings.Contains(a.Why, "held since"))) {
+						t.Errorf("%s: %+v", row.name, a)
+					}
+				}
+				m.heldAt["novox"] = map[string]string{"gitea": "the composition of gitea at src2:" + strings.Repeat("cd", 32) +
+					" is not published yet"}
 				m.compose(t, "novox")
-				m.compose(t, "novox")
+				if a := m.h.health(); !a.Healthy {
-				if a := m.h.health(); a.Healthy || !strings.Contains(a.Why, "novox.gitea") || !strings.Contains(a.Why, fp) {
+					t.Errorf("another build did not start the clock again: %+v", a)
-					t.Errorf("a lasting hold is not said: %+v", a)
 				}
 				m.unpublished["novox"], m.heldAt["novox"] = nil, nil
 				m.compose(t, "novox")

modules/mesh-provisioner/cmd/mesh-provisioner/holdertools.go

@@ -81,7 +81,8 @@ func holderTools(h *Holder) []stdio.Tool {
 			Description: "The provisioner's runtime health, as the node-engine asks it (novox/hq ADR 0366): unhealthy while " +
 				"a pass held back a retirement of most of a node's consumers at once as a fault of the mesh (issue 940), " +
 				"naming the node, the consumers and the likely cause, or while a module's composition has stayed " +
-				"unpublished at its build for more than two passes in a row, naming the module and the build.",
+				"unpublished at one build for longer than 45 minutes (longer than a build takes), naming the module, " +
+				"the build and since when.",
 			Run: func(map[string]any) (any, error) { return h.health(), nil }},
 	}
 }
@@ -91,7 +92,7 @@ func (h *Holder) health() health.Answer {
 	faults, holds := h.Faults(), h.LastingHolds()
 	if len(faults)+len(holds) == 0 {
 		return health.Healthy("No pass holds back a retirement of most of a node's consumers, and no module's " +
-			"composition has stayed unpublished for more than " + fmt.Sprint(unpublishedPasses) + " passes.")
+			"composition has stayed unpublished at one build for longer than " + unpublishedFor.String() + ".")
 	}
 	var said []string
 	for _, n := range sortedKeys(faults) {