Say a module's health is not yet judged while its change is under way and until a look after it

A have said applying and failed for the controller on novox at 15:27:36, from a look at the container being replaced, two seconds before its launch said healthy; the assigner held the canary on it and set novox back. A look now judges only a module that is not in motion, and a module whose composition moved carries unknown until a look after the move.
2026-10-25 15:32:59 +01:00
parent 1cdb4d1a29
commit c16e625a08
2 changed files with 112 additions and 18 deletions
@@ -47,6 +47,52 @@ type haveState struct {
 	// said, until the key is written once with the phase removed (issue 1369, novox/hq ADR 0408 §4: never deleted).
 	written map[string]string
 	removed map[string]string
+	// unjudged is each module whose composition moved (an apply began, or the fact names another build) and that no look
+	// has judged since it stopped moving: its health is not yet judged, never the last look's of what ran before.
+	unjudged map[string]bool
+}
+
+// inMotion says a module's phase is one of a change still under way: what a look finds of it then judges nothing of the
+// composition being applied (a container stopped to be replaced reads unhealthy), so its health is not yet judged.
+func inMotion(phase string) bool {
+	switch delivery.Phase(phase) {
+	case delivery.PhaseBackingUp, delivery.PhaseApplying, delivery.PhaseChecking:
+		return true
+	}
+	return false
+}
+
+// healthOf is the health a module's have carries: unknown (not yet judged) while its change is under way or until a look
+// after it, the last look's otherwise. The caller holds the lock.
+func (s *haveState) healthOf(module string, f ModuleFact) delivery.Health {
+	if inMotion(f.Phase) {
+		return delivery.HealthUnknown
+	}
+	if got, known := s.health[module]; known {
+		return got
+	}
+	if s.unjudged[module] {
+		return delivery.HealthUnknown
+	}
+	return nothingJudged(f, s.looked)
+}
+
+// takeLook keeps a look's health: of a module still in motion nothing, since the look judged what is being replaced;
+// of every other module what the look found, which is then judged. The caller holds the lock.
+func (s *haveState) takeLook(h *Health) {
+	next := moduleHealths(h)
+	for module, f := range s.modules {
+		if inMotion(f.Phase) {
+			delete(next, module)
+			if s.unjudged == nil {
+				s.unjudged = map[string]bool{}
+			}
+			s.unjudged[module] = true
+			continue
+		}
+		delete(s.unjudged, module)
+	}
+	s.health, s.looked = next, true
 }

 // moduleHealths is the health of each module a look names, from the states of its resources: failed when any is
@@ -113,7 +159,7 @@ func (q *Session) noteApply(r Report) bool {
 		s.mu.Lock()
 		defer s.mu.Unlock()
 		if r.Health != nil {
-			s.health, s.looked = moduleHealths(r.Health), true
+			s.takeLook(r.Health)
 		}
 		n := nodeFacts(r)
 		s.node = &n
@@ -155,7 +201,7 @@ func (q *Session) noteApply(r Report) bool {
 	}
 	s.modules = next
 	if r.Health != nil {
-		s.health, s.looked = moduleHealths(r.Health), true
+		s.takeLook(r.Health)
 	}
 	n := nodeFacts(r)
 	s.node = &n
@@ -167,22 +213,17 @@ func (q *Session) noteHealth(h Health) bool {
 	s := &q.haves
 	s.mu.Lock()
 	defer s.mu.Unlock()
-	next := moduleHealths(&h)
+	before := map[string]delivery.Health{}
+	for module, f := range s.modules {
+		before[module] = s.healthOf(module, f)
+	}
+	s.takeLook(&h)
 	changed := false
 	for module, f := range s.modules {
-		want, ok := next[module]
-		if !ok {
-			want = nothingJudged(f, true)
-		}
-		have, known := s.health[module]
-		if !known {
-			have = nothingJudged(f, s.looked)
-		}
-		if have != want {
+		if s.healthOf(module, f) != before[module] {
 			changed = true
 		}
 	}
-	s.health, s.looked = next, true
 	return changed
 }

@@ -233,11 +274,7 @@ func (q *Session) writeHaves(ctx context.Context, bus Bus) {
 		case f.Failed:
 			h.Phase = delivery.PhaseFailed
 		}
-		if got, known := s.health[module]; known {
-			h.Health = got
-		} else {
-			h.Health = nothingJudged(f, s.looked)
-		}
+		h.Health = s.healthOf(module, f)
 		key, err := delivery.HaveKey(node, module)
 		if err != nil {
 			q.sayHave(fmt.Sprintf("module %q has no key in the have bucket: %v", module, err))
@@ -378,6 +415,16 @@ func (q *Session) NoteModule(ctx context.Context, f ModuleFact) {
 	if s.modules == nil {
 		s.modules = map[string]ModuleFact{}
 	}
+	// A change under way, or a fact naming another composition than the one looked at, leaves the module's health not
+	// yet judged until a look after it: the last look judged what ran before (2026-10-25, the canary of the controller on
+	// novox held on a have saying applying and failed two seconds before its launch said healthy).
+	if prev, known := s.modules[f.Module]; inMotion(f.Phase) || known && (prev.FP != f.FP || prev.Composed != f.Composed) {
+		delete(s.health, f.Module)
+		if s.unjudged == nil {
+			s.unjudged = map[string]bool{}
+		}
+		s.unjudged[f.Module] = true
+	}
 	s.modules[f.Module] = f
 	delete(s.removed, f.Module)
 	s.mu.Unlock()
@@ -543,3 +543,50 @@ func TestAModuleSaidAgainOverAnOlderEnginesUnknownIsWrittenWithThisEnginesHealth
 		t.Fatalf("said again after a look: %+v", h)
 	}
 }
+
+// A module's health is not yet judged (unknown) while its change is under way and until a look after it, never failed
+// from a look at what was being replaced: on 2026-10-25 15:27:36 the controller's canary on novox was held on a have
+// saying applying and failed, two seconds before its launch said healthy.
+func TestAHealthIsNotJudgedWhileAModuleIsApplied(t *testing.T) {
+	l := newHaveLink()
+	q, _ := aHaveQueue(l)
+	ctx := context.Background()
+	old := ModuleFact{Module: "ctl", FP: "src2:" + strings.Repeat("a", 64), Composed: "sha256:" + strings.Repeat("1", 64)}
+	q.NoteModule(ctx, old)
+	q.SayHealth(ctx, *healthOf(map[string]string{"ctl": StateUnhealthy}))
+	if h := l.have(t, "n1.ctl"); h.Health != delivery.HealthFailed {
+		t.Fatalf("the running module's failed look was not said: %+v", h)
+	}
+	next := ModuleFact{Module: "ctl", FP: "src2:" + strings.Repeat("b", 64), Composed: "sha256:" + strings.Repeat("2", 64)}
+	applying := next
+	applying.Phase = string(delivery.PhaseApplying)
+	q.NoteModule(ctx, applying)
+	if h := l.have(t, "n1.ctl"); h.Phase != delivery.PhaseApplying || h.Health != delivery.HealthUnknown {
+		t.Fatalf("applying carried the last look's health: %+v", h)
+	}
+	q.SayHealth(ctx, *healthOf(map[string]string{"ctl": StateUnhealthy}))
+	if h := l.have(t, "n1.ctl"); h.Health != delivery.HealthUnknown {
+		t.Fatalf("a look during the apply judged it: %+v", h)
+	}
+	q.NoteModule(ctx, next)
+	if h := l.have(t, "n1.ctl"); h.Phase != delivery.PhaseRunning || h.Health != delivery.HealthUnknown {
+		t.Fatalf("running before a look after the apply: %+v", h)
+	}
+	q.SayHealth(ctx, *healthOf(map[string]string{"ctl": StateHealthy}))
+	if h := l.have(t, "n1.ctl"); h.Health != delivery.HealthOK {
+		t.Fatalf("the look after the apply was not taken: %+v", h)
+	}
+}
+
+// A fact naming another composition than the one looked at, with no apply phase between, is not judged by the old look.
+func TestAHealthIsNotCarriedOverToAnotherComposition(t *testing.T) {
+	l := newHaveLink()
+	q, _ := aHaveQueue(l)
+	ctx := context.Background()
+	q.NoteModule(ctx, ModuleFact{Module: "ctl", FP: "src2:" + strings.Repeat("a", 64)})
+	q.SayHealth(ctx, *healthOf(map[string]string{"ctl": StateUnhealthy}))
+	q.NoteModule(ctx, ModuleFact{Module: "ctl", FP: "src2:" + strings.Repeat("b", 64)})
+	if h := l.have(t, "n1.ctl"); h.Health != delivery.HealthUnknown {
+		t.Fatalf("the old composition's look judged the new one: %+v", h)
+	}
+}