From 9545121ec93b57774eeeb8f4475a6335b16f434d Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 10 Oct 2026 21:14:14 +0200 Subject: [PATCH] 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 | 12 +++- internal/liveness/readiness.go | 3 +- internal/liveness/waiting_test.go | 105 ++++++++++++++++++++++++++++-- 3 files changed, 112 insertions(+), 8 deletions(-) diff --git a/internal/liveness/liveness.go b/internal/liveness/liveness.go index 140d64a..0a5bc94 100644 --- a/internal/liveness/liveness.go +++ b/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() +} diff --git a/internal/liveness/readiness.go b/internal/liveness/readiness.go index 2b37b5d..d8f62b6 100644 --- a/internal/liveness/readiness.go +++ b/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: diff --git a/internal/liveness/waiting_test.go b/internal/liveness/waiting_test.go index cfaf403..df367ba 100644 --- a/internal/liveness/waiting_test.go +++ b/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") + } +}