From b225c7bb4cf5020f7d9e08511277601aa33e121d Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 22:04:24 +0100 Subject: [PATCH] =?UTF-8?q?A=20directory=20used=20as=20found=20is=20not=20?= =?UTF-8?q?a=20module's=20health,=20and=20a=20failed=20have=20says=20why?= =?UTF-8?q?=20(ADR=200557=20=C2=A73,=20issue=201496)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On ace, 2026-10-26 21:19, mesh-build-node's cache directory was there before the mesh and used as found. The health statement says such a directory unhealthy, and the have folded it into the module's health as failed, with no words, while the module's own check answered healthy; the assigner read it as its canary failing and held the version on all four nodes. The directory waits for the operator's hand-over and keeps its own condition, raised from the same statement; it no longer counts in the module's have. A have whose health is failed or waits and whose apply said nothing carries the words of the resource that made it so, so the next one is readable. --- internal/link/have.go | 67 +++++++++++++++++++++++++++++++------- internal/link/have_test.go | 32 ++++++++++++++---- 2 files changed, 82 insertions(+), 17 deletions(-) diff --git a/internal/link/have.go b/internal/link/have.go index fa923fb..3f98f9d 100644 --- a/internal/link/have.go +++ b/internal/link/have.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "sort" + "strings" "sync" "time" @@ -37,6 +38,9 @@ type haveState struct { mu sync.Mutex modules map[string]ModuleFact health map[string]delivery.Health + // words is, by module, what the last look found of the resource that made its health failed or waits: the have says + // it, so a failed health is never without words (issue 1496: "the node gave no words" hid a directory used as found). + words map[string]string // looked says a look's health was taken: a running module it judges no resource of then has nothing that could be // down, and is ok (issue 1945). looked bool @@ -108,10 +112,11 @@ func (s *haveState) owed() bool { // 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) + next, words := moduleHealths(h) for module, f := range s.modules { if inMotion(f.Phase) { delete(next, module) + delete(words, module) if s.unjudged == nil { s.unjudged = map[string]bool{} } @@ -120,19 +125,32 @@ func (s *haveState) takeLook(h *Health) { } delete(s.unjudged, module) } - s.health, s.looked = next, true + s.health, s.words, s.looked = next, words, true +} + +// wordsOf is what a have says of its health when the apply said nothing: the words of the resource that made it failed +// or waits, while it is so. The caller holds the lock. +func (s *haveState) wordsOf(module string, health delivery.Health) string { + if health != delivery.HealthFailed && health != delivery.HealthWaits { + return "" + } + return s.words[module] } // moduleHealths is the health of each module a look names, from the states of its resources: failed when any is -// unhealthy, waits when any waits (waiting, starting, held), ok when all are healthy, unknown otherwise. -func moduleHealths(h *Health) map[string]delivery.Health { - out := map[string]delivery.Health{} +// unhealthy, waits when any waits (waiting, starting, held), ok when all are healthy, unknown otherwise; and, for a +// module failed or waiting, the words of the first resource that made it so. A directory used as found is not the +// module's health: it says nothing of how the module runs, waits for the operator's hand-over and keeps its own +// condition, raised from the same statement (issue 1496: on ace mesh-build-node's cache directory, there before the +// mesh, made its have failed while its check answered healthy, and the assigner held its version on every node). +func moduleHealths(h *Health) (map[string]delivery.Health, map[string]string) { + out, words := map[string]delivery.Health{}, map[string]string{} if h == nil { - return out + return out, words } rank := map[string]int{} for _, r := range h.Resources { - if r.Module == "" { + if r.Module == "" || usedAsFound(r) { continue } var this int @@ -148,12 +166,32 @@ func moduleHealths(h *Health) map[string]delivery.Health { } if was, seen := rank[r.Module]; !seen || this > was { rank[r.Module] = this + if this >= 2 { + words[r.Module] = resourceWords(r) + } } } for module, r := range rank { out[module] = [...]delivery.Health{delivery.HealthOK, delivery.HealthUnknown, delivery.HealthWaits, delivery.HealthFailed}[r] + if r < 2 { + delete(words, module) + } } - return out + return out, words +} + +// usedAsFound says a resource of a statement is a directory used as found (issue 339). +func usedAsFound(r ResourceHealth) bool { + return r.Kind == KindDirectory && strings.HasPrefix(r.Reason, ReasonUsedAsFound) +} + +// resourceWords is one resource's state in the words a have carries: its id, its state and why. +func resourceWords(r ResourceHealth) string { + w := r.Resource + " " + r.State + if why := strings.TrimSpace(r.Reason); why != "" { + w += ": " + why + } + return w } // nodeFacts is the node's own facts of a report, each as the JSON the report carries it in. @@ -252,14 +290,18 @@ func (q *Session) noteHealth(h Health) bool { s := &q.haves s.mu.Lock() defer s.mu.Unlock() - before := map[string]delivery.Health{} + said := func(module string, f ModuleFact) string { + health := s.healthOf(module, f) + return string(health) + "\x00" + s.wordsOf(module, health) + } + before := map[string]string{} for module, f := range s.modules { - before[module] = s.healthOf(module, f) + before[module] = said(module, f) } s.takeLook(&h) changed := false for module, f := range s.modules { - if s.healthOf(module, f) != before[module] { + if said(module, f) != before[module] { changed = true } } @@ -315,6 +357,9 @@ func (q *Session) writeHaves(ctx context.Context, bus Bus) { h.Phase = delivery.PhaseFailed } h.Health = s.healthOf(module, f) + if h.Detail == "" { + h.Detail = s.wordsOf(module, h.Health) + } if w, ok := s.userList[module]; ok { h.UserList = &w } diff --git a/internal/link/have_test.go b/internal/link/have_test.go index 6b36223..74add72 100644 --- a/internal/link/have_test.go +++ b/internal/link/have_test.go @@ -198,25 +198,35 @@ func TestAModulesHealthFollowsTheLook(t *testing.T) { l := newHaveLink() q, _ := aHaveQueue(l) rep := Report{Modules: []ModuleFact{{Module: "a", Build: "b"}, {Module: "b", Build: "b"}, - {Module: "c", Build: "b"}, {Module: "d", Build: "b"}}} + {Module: "c", Build: "b"}, {Module: "d", Build: "b"}, {Module: "e", Build: "b"}}} q.SayFacts(context.Background(), rep) - for _, m := range []string{"a", "b", "c", "d"} { + for _, m := range []string{"a", "b", "c", "d", "e"} { if h := l.have(t, "n1."+m).Health; h != delivery.HealthUnknown { t.Errorf("%s with no look is %s, want unknown", m, h) } } h := healthOf(map[string]string{"a": StateHealthy, "b": StateUnhealthy, "c": StateStarting}) + h.Resources[slicesIndex(h.Resources, "b.server")].Reason = ReasonRestarting + // 1496: a directory used as found is not the module's health (e runs healthy beside one); a failed or waiting module's + // have says the words of the resource that made it so. h.Resources = append(h.Resources, ResourceHealth{Module: "a", Resource: "a.two", State: StateWaiting}, - ResourceHealth{Module: "b", Resource: "b.two", State: StateHealthy}, ResourceHealth{Module: "d", Resource: "d.x", State: StateUnknown}) + ResourceHealth{Module: "b", Resource: "b.two", State: StateHealthy}, ResourceHealth{Module: "d", Resource: "d.x", State: StateUnknown}, + ResourceHealth{Module: "e", Resource: "e.server", State: StateHealthy}, + ResourceHealth{Module: "e", Resource: "e.cache", Kind: KindDirectory, State: StateUnhealthy, Reason: ReasonUsedAsFound + " owned by 1:1"}) if !q.SayHealth(context.Background(), *h) { t.Fatal("not said") } for m, want := range map[string]delivery.Health{"a": delivery.HealthWaits, "b": delivery.HealthFailed, - "c": delivery.HealthWaits, "d": delivery.HealthUnknown} { + "c": delivery.HealthWaits, "d": delivery.HealthUnknown, "e": delivery.HealthOK} { if got := l.have(t, "n1."+m).Health; got != want { t.Errorf("%s is %s, want %s", m, got, want) } } + for m, want := range map[string]string{"a": "a.two waiting", "b": "b.server unhealthy: restarting", "d": "", "e": ""} { + if got := l.have(t, "n1."+m).Detail; got != want { + t.Errorf("%s says %q, want %q", m, got, want) + } + } before := l.setCount() h2 := *h q.SayHealth(context.Background(), h2) @@ -225,8 +235,8 @@ func TestAModulesHealthFollowsTheLook(t *testing.T) { } h2.Resources = []ResourceHealth{{Module: "a", Resource: "a.server", State: StateHealthy}} q.SayHealth(context.Background(), h2) - if got := l.have(t, "n1.a").Health; got != delivery.HealthOK { - t.Errorf("a after a healthy look is %s", got) + if got := l.have(t, "n1.a"); got.Health != delivery.HealthOK || got.Detail != "" { + t.Errorf("a after a healthy look is %s, saying %q", got.Health, got.Detail) } } @@ -714,3 +724,13 @@ func TestAUserListReachesTheHaveOfAModuleNoApplyHereSays(t *testing.T) { t.Fatalf("the first have of the module lost the list: %+v", got) } } + +// slicesIndex is the index of the resource with id in rs, or -1. +func slicesIndex(rs []ResourceHealth, id string) int { + for i, r := range rs { + if r.Resource == id { + return i + } + } + return -1 +} -- 2.54.0