A version healthy on another node is not held for one node's failed health check (ADR 0557, issue 1496)

On ace, 2026-10-26 21:19, a newly assigned mesh-build-node failed its first health check at 8a98386f (later 31be2f24) while shanks, g14 and novox had run it healthy for an hour. Being first by name, ace was the canary, and the assigner held the version and moved the three healthy nodes back. A failed health check of the newest fingerprint now holds it only when no other node's have names it healthy (ADR 0557 §2 keeps ADR 0429 §6's hold and set-back for that case). Otherwise the failure is that node's (ADR 0557 §1): nothing is held or set back, the node keeps its want, and the assigner raises the node's condition once (failed-alone.<node>.<module>), naming the node, the module, the fingerprint and the have's words, and clears it once when it ends; the first pass after a start clears what a previous process may have raised.
2026-10-26 22:06:32 +01:00
parent d581f9659a
commit 2ee7589474
5 changed files with 141 additions and 4 deletions
@@ -39,6 +39,9 @@ type passer struct {
 	// noWant is, by node and module, the reason last said why the node gets no want for the module (issue 1945): a reason is
 	// said in the journal when it appears or changes, and its clearing when the node has a want again; one line per change.
 	noWant map[string]string
+	// nodeFailed is, by node and module, the words of the condition last raised for a failure that is the node's own (ADR 0557
+	// §1, issue 1496): raised once per words, cleared once when it ends.
+	nodeFailed map[string]string
 }

 func (p *passer) clock() time.Time {
@@ -156,6 +159,7 @@ func (p *passer) wants(ctx context.Context) {
 		gatherNeither(neither, name, out)
 		leftNeither(left, m, wanted[name], out)
 		p.sayNoWant(name, m.Nodes, wanted[name], out)
+		p.sayNodeFailed(name, fp, m.Nodes, out, first)
 		if first {
 			startClears(stale, m, out)
 			if fp != "" && !m.Held && out.Held == nil && !out.Lifted {
@@ -230,6 +234,45 @@ func (p *passer) sayNoWant(module string, nodes []string, before map[string]deli
 	}
 }

+// sayNodeFailed raises, once per node and words, the condition of a failed health check of a module's newest fingerprint fp
+// that is the node's own, since fp runs healthy elsewhere and is not held (ADR 0557 §1, issue 1496), and says it in the journal;
+// it clears the condition once when the node no longer fails alone. The first pass after a start clears it for every node that
+// does not fail alone, since a previous process may have raised it (a clear of a condition not open is quiet at the router).
+// A condition the router did not take is said again at the next pass.
+func (p *passer) sayNodeFailed(module, fp string, nodes []string, out assign.Outcome, first bool) {
+	if p.nodeFailed == nil {
+		p.nodeFailed = map[string]string{}
+	}
+	for _, node := range nodes {
+		key := node + "." + module
+		f, failing := out.NodeFailed[node]
+		last, said := p.nodeFailed[key]
+		switch {
+		case failing && f.Words+"|"+strings.Join(f.Others, ",") != last:
+			p.logf("%s on %s: the health check at %s failed here (%s), while it runs healthy on %s: the failure is %s's, so nothing "+
+				"is held or set back", module, node, fp, f.Words, strings.Join(f.Others, ", "), node)
+			if p.say != nil {
+				if err := p.say(assign.NodeFailedCondition(node, module, fp, f, true)); err != nil {
+					p.logf("the condition of %s failing on %s alone could not be said, and is said at the next pass: %v", module, node, err)
+					continue
+				}
+			}
+			p.nodeFailed[key] = f.Words + "|" + strings.Join(f.Others, ",")
+		case !failing && (said || first):
+			if said {
+				p.logf("%s on %s no longer fails its health check alone", module, node)
+			}
+			if p.say != nil {
+				if err := p.say(assign.NodeFailedCondition(node, module, fp, assign.NodeFailure{}, false)); err != nil && said {
+					p.logf("the condition of %s failing on %s alone could not be cleared, and is cleared at the next pass: %v", module, node, err)
+					continue
+				}
+			}
+			delete(p.nodeFailed, key)
+		}
+	}
+}
+
 // neitherOne is a module assigned to a node that holds no want for it after a pass, and the rule holding its first want back.
 type neitherOne struct {
 	node, module string
@@ -443,6 +443,30 @@ func TestThePassGathersTheModulesOnNeitherList(t *testing.T) {
 			t.Errorf("pass %d said %q, want %q", i, got, c.line)
 		}
 	}
+	// 1496, ADR 0557 §1: a failure that is the node's own (its fingerprint healthy elsewhere, not held) raises the node's condition
+	// once, naming the node, the module, F and the have's words, and clears it once when it ends.
+	var conds []asks.Condition
+	p.say = func(c asks.Condition) error { conds = append(conds, c); return nil }
+	failing := map[string]assign.NodeFailure{"shanks": {Others: []string{"ace"}, Words: "health failed, phase running: w"}}
+	for i, c := range []struct {
+		failed map[string]assign.NodeFailure
+		state  string // the condition said in this pass, "" for none
+	}{
+		{failing, asks.StateRaised},
+		{failing, ""},
+		{nil, asks.StateCleared},
+		{nil, ""},
+	} {
+		conds = nil
+		p.sayNodeFailed("rofi", fpTest, []string{"shanks"}, assign.Outcome{NodeFailed: c.failed}, false)
+		switch {
+		case c.state == "" && len(conds) != 0, c.state != "" && (len(conds) != 1 || conds[0].State != c.state):
+			t.Errorf("node failed, pass %d said %+v, want %q", i, conds, c.state)
+		case c.state == asks.StateRaised && (conds[0].Key != "mesh-delivery-assigner.failed-alone.shanks.rofi" ||
+			!strings.Contains(conds[0].Explanation, "failed on shanks (health failed, phase running: w), while it runs healthy on ace")):
+			t.Errorf("node failed, raised %+v", conds[0])
+		}
+	}
 }

 // A neither or held condition a previous process raised is cleared by the first pass after a start: a fresh pass remembers
@@ -81,6 +81,29 @@ func HeldCondition(module, fp, node, words string, raised, needsYou bool) asks.C
 	return c
 }

+// NodeFailedCondition is the condition the assigner raises for a module whose newest fingerprint fp failed its health check on
+// node while other nodes run it healthy (ADR 0557 §1, issue 1496): the failure is the node's, so nothing is held or set back and
+// the node keeps its want. It names the node, the module, fp and the have's words, keyed by node and module, and is cleared
+// (raised false) once the node no longer fails alone.
+func NodeFailedCondition(node, module, fp string, f NodeFailure, raised bool) asks.Condition {
+	key := delivery.AssignerModule + ".failed-alone." + node + "." + module
+	subject := asks.Scope{Scope: "module", ID: module, Machine: node}
+	if !raised {
+		return asks.Condition{Key: key, Subject: subject, State: asks.StateCleared,
+			Resolved: fmt.Sprintf("%s on %s no longer fails its health check alone.", module, node)}
+	}
+	headline := fmt.Sprintf("%s fails on %s alone", module, node)
+	if n := len([]rune(headline)); n > asks.HeadlineLength {
+		headline = string([]rune(headline)[:asks.HeadlineLength])
+	}
+	return asks.Condition{Key: key, Severity: asks.SeverityWarning, Subject: subject, State: asks.StateRaised,
+		Summary: headline, Headline: headline,
+		Explanation: fmt.Sprintf("%s The health check of %s at %s failed on %s (%s), while it runs healthy on %s. The failure is %s's, "+
+			"so the assigner holds nothing and sets no node back, and %s keeps its want. This clears once %s runs it healthy or "+
+			"another version is the newest.", asks.NothingToDo, module, short(fp), node, f.Words, strings.Join(f.Others, ", "), node,
+			node, node)}
+}
+
 // NeitherCondition is the condition of a module assigned to node that holds no want for it (issue 1945): the node's own composition
 // gives up every module with a fingerprint of the new form, so nothing applies it there and its files are gone. It is raised (urgent)
 // naming the module, the node and the rule holding its first want back with the rule's words, under the assigner's own name keyed by
@@ -169,6 +169,9 @@ type Outcome struct {
 	// Lifted is the hold of m.FP lifted by itself: every node that runs the module runs m.FP healthy (issue 1906). The held
 	// record and its ask are taken away, and the wants move by the rules again in this pass.
 	Lifted bool
+	// NodeFailed is, by node, a failed health check of m.FP on a node while another node runs m.FP healthy: the failure is
+	// that node's, so m.FP is not held and nothing is set back (ADR 0557 §1, issue 1496). The pass raises its condition once.
+	NodeFailed map[string]NodeFailure
 	// Record is the moves a person must allow before the want names them: each is asked once (ADR 0539 §14).
 	Record []RecordNeed
 	// Restore is the nodes of a module holding data whose held fingerprint failed its health check there and that are not set
@@ -228,11 +231,24 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st
 	}
 	if !held && !out.Lifted && !m.Released && m.FP != "" {
 		for _, node := range slices.Sorted(slices.Values(m.Nodes)) {
-			if h, ok := n.Haves[node][m.Name]; ok && h.FP == m.FP && failed(h) {
-				out.Held = &delivery.Held{Node: node, Words: HeldWords(h, m.Now)}
-				held = true
-				break
+			h, ok := n.Haves[node][m.Name]
+			if !ok || h.FP != m.FP || !failed(h) {
+				continue
 			}
+			if others := healthyElsewhere(m, n, node); len(others) > 0 {
+				// F runs healthy on another node, so its failure here is this node's, not F's: F is not held, no node is set back,
+				// and the failing node keeps its want (ADR 0557 §1; issue 1496: on ace a newly assigned mesh-build-node failed its
+				// first check and the hold moved shanks, g14 and novox, healthy on F for an hour, back). The pass raises the node's
+				// condition. With no healthy witness the hold and the set-back stand (ADR 0557 §2, ADR 0429 §6).
+				if out.NodeFailed == nil {
+					out.NodeFailed = map[string]NodeFailure{}
+				}
+				out.NodeFailed[node] = NodeFailure{Others: others, Words: HeldWords(h, time.Time{})} // said once: no clock in it
+				continue
+			}
+			out.Held = &delivery.Held{Node: node, Words: HeldWords(h, m.Now)}
+			held = true
+			break
 		}
 	}
 	mm := m
@@ -419,6 +435,24 @@ func failed(h delivery.Have) bool {
 	return h.Health == delivery.HealthFailed && (h.Phase == delivery.PhaseRunning || h.Phase == delivery.PhaseFailed)
 }

+// healthyElsewhere is every node of m but node that runs m.FP healthy, by name.
+func healthyElsewhere(m Module, n Nodes, node string) []string {
+	var out []string
+	for _, other := range slices.Sorted(slices.Values(m.Nodes)) {
+		if other != node && healthy(n.Haves[other][m.Name], m.FP) {
+			out = append(out, other)
+		}
+	}
+	return out
+}
+
+// NodeFailure is a failed health check of a module's newest fingerprint on one node while it runs healthy on Others: the have's
+// health, phase and words (HeldWords without a time), and the healthy witnesses by name.
+type NodeFailure struct {
+	Others []string
+	Words  string
+}
+
 // runs says whether a have is of a module the node runs: running, whatever its health.
 func runs(h delivery.Have) bool {
 	return h.Phase == delivery.PhaseRunning
@@ -150,6 +150,19 @@ func TestAFailedHealthHoldsAndSetsBack(t *testing.T) {
 	if w, ok := out.Wants["g14"]; ok && w.FP == fNew {
 		t.Errorf("g14 moved to a held fingerprint: %+v", w)
 	}
+
+	// 1496, ADR 0557 §1, §2: the same failure while F runs healthy on another node is that node's: F is not held, nothing is set
+	// back, the failing node keeps its want, and the outcome names it (on ace a newly assigned module's first check held a version healthy for an hour on three nodes).
+	n.Haves["g14"]["mail"] = delivery.Have{FP: fNew, Phase: delivery.PhaseRunning, Health: delivery.HealthOK}
+	before := map[string]delivery.Want{"ace": {FP: fNew, Composed: dig(fNew)}, "g14": {FP: fNew, Composed: dig(fNew)}}
+	out = Wants(m, n, before, map[string]string{"ace": fOld, "g14": fOld})
+	if out.Held != nil || len(out.Wants) != 0 {
+		t.Fatalf("held %+v, wants %+v: want no hold and no want changed", out.Held, out.Wants)
+	}
+	if f := out.NodeFailed["ace"]; !slices.Equal(f.Others, []string{"g14"}) || f.Words != "health failed, phase running: connection refused" ||
+		len(out.NodeFailed) != 1 {
+		t.Errorf("node failed = %+v", out.NodeFailed)
+	}
 }

 // 1297/B4: a module holding data is not set back; its want stays and says why.