diff --git a/modules/mesh-provisioner/cmd/mesh-provisioner/holder.go b/modules/mesh-provisioner/cmd/mesh-provisioner/holder.go index fd6f63eb..4a63e956 100644 --- a/modules/mesh-provisioner/cmd/mesh-provisioner/holder.go +++ b/modules/mesh-provisioner/cmd/mesh-provisioner/holder.go @@ -130,8 +130,8 @@ type Holder struct { mu sync.Mutex // faults is, by node, the retirement a pass held back as a fault of the mesh (issue 940), said by provisioner_health. faults map[string]string - // unpublished is, by node and module, how many passes in a row held the module because its composition was not - // published, with the hold's words: one held more than unpublishedPasses is said by provisioner_health. + // unpublished is, by node and module, since when the module is held because its composition at this build is not + // published, with the hold's words: one held longer than unpublishedFor is said by provisioner_health. unpublished map[string]map[string]unpublishedHold } @@ -1174,16 +1174,18 @@ func (h *Holder) clearFault(node string) { } } -// unpublishedPasses is how many passes in a row a module may be held unpublished before the health says it. -const unpublishedPasses = 2 +// unpublishedFor is how long a module may be held unpublished at one build before the health says it: longer than a +// build takes (the build step's bound is 30 minutes), so a newly placed module waiting for its build is no fault. A +// pass runs on every change of its node, so passes are no measure of it (review 2 of issue 940). +const unpublishedFor = 45 * time.Minute type unpublishedHold struct { - passes int - why string + since time.Time + why string // the hold's words, which name the build } -// countUnpublished counts, for each module held unpublished on node, the passes in a row it was; a module published -// again is forgotten. +// countUnpublished keeps, for each module held unpublished on node, since when it is held at that build; a module +// published again, or held at another build, starts again. func (h *Holder) countUnpublished(node string, modules []string, held map[string]string) { if h.unpublished == nil { h.unpublished = map[string]map[string]unpublishedHold{} @@ -1191,7 +1193,11 @@ func (h *Holder) countUnpublished(node string, modules []string, held map[string was := h.unpublished[node] now := map[string]unpublishedHold{} for _, m := range modules { - now[m] = unpublishedHold{passes: was[m].passes + 1, why: held[m]} + u, ok := was[m] + if !ok || u.why != held[m] { + u = unpublishedHold{since: h.Now().UTC(), why: held[m]} + } + now[m] = u } h.unpublished[node] = now } @@ -1207,16 +1213,17 @@ func (h *Holder) Faults() map[string]string { return out } -// LastingHolds is each module held unpublished for more than unpublishedPasses passes in a row, as "." -// with the hold's words, which name its build. +// LastingHolds is each module held unpublished at one build for longer than unpublishedFor, as "." with +// the hold's words, which name its build, and since when. func (h *Holder) LastingHolds() map[string]string { h.mu.Lock() defer h.mu.Unlock() out := map[string]string{} + now := h.Now() for n, mods := range h.unpublished { for m, u := range mods { - if u.passes > unpublishedPasses { - out[n+"."+m] = fmt.Sprintf("%s (held %d passes in a row)", u.why, u.passes) + if now.Sub(u.since) > unpublishedFor { + out[n+"."+m] = fmt.Sprintf("%s (held since %s UTC)", u.why, u.since.Format("2006-01-02 15:04")) } } } diff --git a/modules/mesh-provisioner/cmd/mesh-provisioner/holder_test.go b/modules/mesh-provisioner/cmd/mesh-provisioner/holder_test.go index 8a276e69..63504236 100644 --- a/modules/mesh-provisioner/cmd/mesh-provisioner/holder_test.go +++ b/modules/mesh-provisioner/cmd/mesh-provisioner/holder_test.go @@ -603,11 +603,34 @@ func TestARetirementIsJudgedOnlyFromAWholePicture(t *testing.T) { m.unpublished["novox"] = []string{"gitea"} m.heldAt["novox"] = map[string]string{"gitea": "the composition of gitea at " + fp + " is not published yet"} }, retiring: 1, asked: 1, after: func(t *testing.T, m *mesh) { - // Held for more than two passes in a row: the health names the module and its build. + // Held at one build: healthy over many passes within the bound (a build takes minutes), unhealthy past it, + // naming the module, its build and since when; another build starts the clock again. + now := m.h.Now() + m.h.Now = func() time.Time { return now } + for _, row := range []struct { + name string + after time.Duration + healthy bool + }{ + {"many passes within the bound", 0, true}, + {"still within the bound", unpublishedFor - time.Minute, true}, + {"past the bound", unpublishedFor + time.Minute, false}, + } { + now = now.Add(row.after) + for range 5 { + m.compose(t, "novox") + } + a := m.h.health() + if a.Healthy != row.healthy || (!row.healthy && (!strings.Contains(a.Why, "novox.gitea") || + !strings.Contains(a.Why, fp) || !strings.Contains(a.Why, "held since"))) { + t.Errorf("%s: %+v", row.name, a) + } + } + m.heldAt["novox"] = map[string]string{"gitea": "the composition of gitea at src2:" + strings.Repeat("cd", 32) + + " is not published yet"} m.compose(t, "novox") - m.compose(t, "novox") - if a := m.h.health(); a.Healthy || !strings.Contains(a.Why, "novox.gitea") || !strings.Contains(a.Why, fp) { - t.Errorf("a lasting hold is not said: %+v", a) + if a := m.h.health(); !a.Healthy { + t.Errorf("another build did not start the clock again: %+v", a) } m.unpublished["novox"], m.heldAt["novox"] = nil, nil m.compose(t, "novox") diff --git a/modules/mesh-provisioner/cmd/mesh-provisioner/holdertools.go b/modules/mesh-provisioner/cmd/mesh-provisioner/holdertools.go index 2280e7cf..7256ed64 100644 --- a/modules/mesh-provisioner/cmd/mesh-provisioner/holdertools.go +++ b/modules/mesh-provisioner/cmd/mesh-provisioner/holdertools.go @@ -81,7 +81,8 @@ func holderTools(h *Holder) []stdio.Tool { Description: "The provisioner's runtime health, as the node-engine asks it (novox/hq ADR 0366): unhealthy while " + "a pass held back a retirement of most of a node's consumers at once as a fault of the mesh (issue 940), " + "naming the node, the consumers and the likely cause, or while a module's composition has stayed " + - "unpublished at its build for more than two passes in a row, naming the module and the build.", + "unpublished at one build for longer than 45 minutes (longer than a build takes), naming the module, " + + "the build and since when.", Run: func(map[string]any) (any, error) { return h.health(), nil }}, } } @@ -91,7 +92,7 @@ func (h *Holder) health() health.Answer { faults, holds := h.Faults(), h.LastingHolds() if len(faults)+len(holds) == 0 { return health.Healthy("No pass holds back a retirement of most of a node's consumers, and no module's " + - "composition has stayed unpublished for more than " + fmt.Sprint(unpublishedPasses) + " passes.") + "composition has stayed unpublished at one build for longer than " + unpublishedFor.String() + ".") } var said []string for _, n := range sortedKeys(faults) {