Merge pull request 'A directory used as found is not a module's health, and a failed have names its cause (issue 1496, ADR 0557 §4, §5)' (#150) from fix/1496-start-health into main

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.
+// unhealthy, waits when any waits (waiting, starting, held), ok when all are healthy, unknown otherwise; and, for a
-func moduleHealths(h *Health) map[string]delivery.Health {
+// module failed or waiting, the words of the first resource that made it so. A directory used as found is not the
-	out := map[string]delivery.Health{}
+// 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.
@@ -268,14 +306,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
 		}
 	}
@@ -335,6 +377,9 @@ func (q *Session) writeHaves(ctx context.Context, bus Bus) {
 		}
 		h.Sealed = f.Sealed
 		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
 		}

internal/link/have_test.go

@@ -199,25 +199,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)
@@ -226,8 +236,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 {
+	if got := l.have(t, "n1.a"); got.Health != delivery.HealthOK || got.Detail != "" {
-		t.Errorf("a after a healthy look is %s", got)
+		t.Errorf("a after a healthy look is %s, saying %q", got.Health, got.Detail)
 	}
 }
 
@@ -752,3 +762,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
+}