Clear an old pass on a wait, and say a change of waits (review of #78)

A resource that passed, then waited, then failed once after its grace was
still stated healthy from the old pass; and a change of what it waits for
alone raised no change event, so the controller could miss it.

internal/liveness/liveness.go

@@ -368,7 +368,8 @@ func (j *Judge) Look(ctx context.Context) (Statement, bool) {
 		seen[r.ID] = true
 		st.Resources = append(st.Resources, State{Resource: k.Resource, State: k.State, Reason: k.Reason, Since: k.Since,
 			Streak: k.Streak, Restarts: k.Counted, Waits: waitsOf(k)})
-		word := k.State + "/" + k.Reason
+		// What it waits for is part of what is said: a change of waits alone is a change.
+		word := k.State + "/" + k.Reason + "/" + waitsWord(waitsOf(k))
 		if j.said[r.ID] != word {
 			changed = true
 			j.said[r.ID] = word
@@ -579,3 +580,12 @@ func waitsOf(k *kept) []health.Wait {
 	}
 	return append([]health.Wait(nil), k.Waits...)
 }
+
+// waitsWord is a resource's waits in one comparable word.
+func waitsWord(ws []health.Wait) string {
+	var b strings.Builder
+	for _, w := range ws {
+		b.WriteString(w.Part + "|" + w.Secret + "|" + w.Setting + "|" + w.What + ";")
+	}
+	return b.String()
+}

internal/liveness/readiness.go

@@ -214,7 +214,8 @@ func (j *Judge) Record(id string, started time.Time, ok bool, why string, waits
 	case ok:
 		k.Passed, k.Failing, k.Why = true, 0, ""
 	case len(waits) > 0:
-		k.Failing, k.Why, k.Waits = 0, why, append([]health.Wait(nil), waits...)
+		// A pass from before the wait says nothing now: a failing look after it counts from no pass.
+		k.Passed, k.Failing, k.Why, k.Waits = false, 0, why, append([]health.Wait(nil), waits...)
 	case j.Now().Before(k.Started.Add(j.graceOf(k.Resource))):
 		k.Why = why
 	default:

internal/liveness/waiting_test.go

@@ -18,20 +18,27 @@ import (
 
 var password = health.Wait{Part: "the source games", Secret: "smb-password-games", What: "the password of the source games"}
 
+var toolCheck = &declaration.Health{Kind: declaration.HealthTool, Tool: "mounts_health", Interval: "30s", Timeout: "5s",
+	Looks: 2, Grace: "1m30s"}
+
 // toolJudge is a judge of one running process whose tool check answers what *answer holds.
 func toolJudge(t *testing.T, answer *[]byte) (*Judge, *clock) {
+	j, c, _, _ := toolJudgeAt(t, answer, filepath.Join(t.TempDir(), FileName))
+	return j, c
+}
+
+// toolJudgeAt is toolJudge keeping its file at path, with its runtime.
+func toolJudgeAt(t *testing.T, answer *[]byte, path string) (*Judge, *clock, *fakeRuntime, string) {
 	t.Helper()
 	c := &clock{now: t0}
 	rt := &fakeRuntime{containers: map[string]Observed{"watch": running("c1", 0, t0.Format(time.RFC3339Nano))}}
-	j := aJudge(t, rt, c, filepath.Join(t.TempDir(), FileName))
+	j := aJudge(t, rt, c, path)
 	j.Probes = &Probes{AskTool: func(context.Context, string, string) (health.Answer, error) {
 		return health.Read(*answer)
 	}}
-	check := &declaration.Health{Kind: declaration.HealthTool, Tool: "mounts_health", Interval: "30s", Timeout: "5s",
-		Looks: 2, Grace: "1m30s"}
-	j.Set([]Resource{{Module: "mounts", ID: "mounts.watch", Kind: KindContainer, Target: "watch", Check: check}})
+	j.Set([]Resource{{Module: "mounts", ID: "mounts.watch", Kind: KindContainer, Target: "watch", Check: toolCheck}})
 	j.Look(t.Context())
-	return j, c
+	return j, c, rt, path
 }
 
 func answerOf(t *testing.T, a health.Answer) []byte {
@@ -90,7 +97,7 @@ func TestAnUnhealthyAnswerAfterAWaitFollowsTheFailingLooks(t *testing.T) {
 	next(t, j, c, 2*time.Minute)
 	answer = answerOf(t, health.Unhealthy("the share is unreachable"))
 	s := next(t, j, c, 30*time.Second)
-	if s.State == Unhealthy || s.State == Waiting || len(s.Waits) != 0 {
+	if s.State == Unhealthy || s.State == Waiting || s.State == Healthy || len(s.Waits) != 0 {
 		t.Fatalf("one failing look after a wait made it %+v", s)
 	}
 	if s = next(t, j, c, 30*time.Second); s.State != Unhealthy || !strings.Contains(s.Reason, "the share is unreachable") {
@@ -120,3 +127,89 @@ func TestAMalformedWaitIsUnhealthyAndExcusesNothing(t *testing.T) {
 		t.Fatalf("a wait naming nothing: %+v", s)
 	}
 }
+
+// A pass from before a wait is not kept through it: passed, waited, then one failing look after the grace is
+// not healthy.
+func TestAPassBeforeAWaitIsNotKeptThroughIt(t *testing.T) {
+	answer := answerOf(t, health.Healthy("ok"))
+	j, c := toolJudge(t, &answer)
+	if s := next(t, j, c, 0); s.State != Healthy {
+		t.Fatalf("passing: %+v", s)
+	}
+	answer = answerOf(t, health.Waiting("waits", password))
+	next(t, j, c, 2*time.Minute)
+	answer = answerOf(t, health.Unhealthy("the share is unreachable"))
+	if s := next(t, j, c, 30*time.Second); s.State == Healthy || s.State == Waiting {
+		t.Fatalf("one failing look after a wait that followed a pass: %+v", s)
+	}
+}
+
+// A resource that is down is down, whatever its last look said it waits for.
+func TestDownBeatsAWait(t *testing.T) {
+	answer := answerOf(t, health.Waiting("waits", password))
+	j, c, rt, _ := toolJudgeAt(t, &answer, filepath.Join(t.TempDir(), FileName))
+	next(t, j, c, 2*time.Minute)
+	rt.containers["watch"] = Observed{Found: true, Identity: "c1", Started: t0.Format(time.RFC3339Nano)}
+	c.now = c.now.Add(15 * time.Second)
+	st, _ := j.Look(t.Context())
+	if s := stateOf(t, st, "mounts.watch"); s.State != Unhealthy || s.Reason != ReasonDown || len(s.Waits) != 0 {
+		t.Fatalf("a stopped process that waited: %+v", s)
+	}
+}
+
+// A changed check says nothing of what the old one found waiting.
+func TestAChangedCheckClearsTheWaits(t *testing.T) {
+	answer := answerOf(t, health.Waiting("waits", password))
+	j, c := toolJudge(t, &answer)
+	next(t, j, c, 2*time.Minute)
+	other := *toolCheck
+	other.Interval = "60s"
+	j.Set([]Resource{{Module: "mounts", ID: "mounts.watch", Kind: KindContainer, Target: "watch", Check: &other}})
+	st, _ := j.Look(t.Context())
+	if s := stateOf(t, st, "mounts.watch"); s.State == Waiting || len(s.Waits) != 0 {
+		t.Fatalf("after its check changed: %+v", s)
+	}
+}
+
+// The engine restarted keeps what a resource waits for, as it keeps everything else it judged.
+func TestWaitsAreKeptAcrossTheEnginesRestart(t *testing.T) {
+	answer := answerOf(t, health.Waiting("waits", password))
+	path := filepath.Join(t.TempDir(), FileName)
+	j, c, rt, _ := toolJudgeAt(t, &answer, path)
+	next(t, j, c, 2*time.Minute)
+	again := aJudge(t, rt, c, path)
+	again.Set([]Resource{{Module: "mounts", ID: "mounts.watch", Kind: KindContainer, Target: "watch", Check: toolCheck}})
+	st, _ := again.Look(t.Context())
+	if s := stateOf(t, st, "mounts.watch"); s.State != Waiting || len(s.Waits) != 1 || s.Waits[0].Secret != password.Secret {
+		t.Fatalf("after the engine restarted: %+v", s)
+	}
+}
+
+// Waiting and healthy each way is a change, and so is a change of what it waits for alone.
+func TestAChangeOfWaitingOrOfItsWaitsIsAChange(t *testing.T) {
+	answer := answerOf(t, health.Healthy("ok"))
+	j, c := toolJudge(t, &answer)
+	look := func(by time.Duration) bool {
+		c.now = c.now.Add(by)
+		lookOnce(t, j)
+		_, changed := j.Look(t.Context())
+		return changed
+	}
+	look(0)
+	answer = answerOf(t, health.Waiting("waits", password))
+	if !look(30 * time.Second) {
+		t.Fatal("healthy to waiting was no change")
+	}
+	if look(30 * time.Second) {
+		t.Fatal("the same wait again was a change")
+	}
+	user := health.Wait{Part: "the source games", Setting: "smb-users", What: "the username of the source games"}
+	answer = answerOf(t, health.Waiting("waits", password, user))
+	if !look(30 * time.Second) {
+		t.Fatal("a wait added was no change")
+	}
+	answer = answerOf(t, health.Healthy("ok"))
+	if !look(30 * time.Second) {
+		t.Fatal("waiting to healthy was no change")
+	}
+}