From c16e625a08561ea5f85ed98f5a4e85e6dfa41bec Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 15:32:59 +0100 Subject: [PATCH] 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. --- internal/link/have.go | 83 +++++++++++++++++++++++++++++--------- internal/link/have_test.go | 47 +++++++++++++++++++++ 2 files changed, 112 insertions(+), 18 deletions(-) diff --git a/internal/link/have.go b/internal/link/have.go index 35ec1c0..6e9b8f8 100644 --- a/internal/link/have.go +++ b/internal/link/have.go @@ -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() diff --git a/internal/link/have_test.go b/internal/link/have_test.go index 8101fd9..6706f86 100644 --- a/internal/link/have_test.go +++ b/internal/link/have_test.go @@ -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) + } +}