diff --git a/modules/mesh-provisioner/cmd/mesh-provisioner/holder.go b/modules/mesh-provisioner/cmd/mesh-provisioner/holder.go index 13d2eb5e..fd6f63eb 100644 --- a/modules/mesh-provisioner/cmd/mesh-provisioner/holder.go +++ b/modules/mesh-provisioner/cmd/mesh-provisioner/holder.go @@ -130,6 +130,9 @@ 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 map[string]map[string]unpublishedHold } // Composed is what one node's composition did. @@ -236,22 +239,10 @@ func (h *Holder) ComposeNodeWith(ctx context.Context, node string, retireMany bo if err := h.writeRefused(node, heldAtWants); err != nil { out.Problems = append(out.Problems, err.Error()) } - // partial is why the node's picture is not whole in this pass: then nothing is removed or retired on it. - partial := "" - if len(c.Unpublished) > 0 { - partial = fmt.Sprintf("the composition of %s on %s is not published yet", strings.Join(c.Unpublished, ", "), node) - } - keep := heldAtWants - if partial != "" { - keep = map[string]string{} - for m, why := range heldAtWants { - keep[m] = why - } - for _, m := range c.Unpublished { - keep[m] = partial - } - } - if err := h.writePlans(node, atWants, keep, partial != "", &out); err != nil { + // A module whose composition is not published at its build is held (heldAtWants): its own plan and keys are kept, + // and nothing else on the node waits for it (review 1 of issue 940). A hold that lasts is said on the health. + h.countUnpublished(node, c.Unpublished, heldAtWants) + if err := h.writePlans(node, atWants, heldAtWants, &out); err != nil { out.Problems = append(out.Problems, err.Error()) } lap("plans written") @@ -324,7 +315,7 @@ func (h *Holder) ComposeNodeWith(ctx context.Context, node string, retireMany bo } } h.judgeElsewhere(elsewhere, &out) - h.judgeNode(node, needed, ahead, untouched, partial, retireMany, &out) + h.judgeNode(node, needed, ahead, untouched, retireMany, &out) lap("provisions judged") return out, nil } @@ -506,9 +497,8 @@ func (h *Holder) runningOn(node string) (map[string]string, error) { } // writePlans writes each plan that differs and removes the node's plans of modules no longer composed there; a held -// module's plan is kept as it was (issue 774), and in a pass whose picture is partial no plan is removed (issue 940). -func (h *Holder) writePlans(node string, plans map[string]catalogue.Plan, held map[string]string, partial bool, - out *Composed) error { +// module's plan is kept as it was (issue 774). +func (h *Holder) writePlans(node string, plans map[string]catalogue.Plan, held map[string]string, out *Composed) error { var problems []error for _, module := range sortedKeys(plans) { key, err := provisions.PlanKey(node, module) @@ -542,7 +532,7 @@ func (h *Holder) writePlans(node string, plans map[string]catalogue.Plan, held m if _, still := plans[module]; still { continue } - if _, kept := held[module]; kept || partial { + if _, kept := held[module]; kept { continue } if err := h.Store.Delete(PlansState, k); err != nil { @@ -556,11 +546,10 @@ func (h *Holder) writePlans(node string, plans map[string]catalogue.Plan, held m // judgeNode brings the node's provision keys to what is needed and what each provider said. A key of a module left // untouched (held on the node, or with a want or a have and no plan read in this pass) that its composition no longer -// names is left as it is: not composed is not unneeded. While partial says why the node's picture is not whole, no key -// turns retiring; and a pass that would retire most of the node's consumers at once retires none, said as a fault of +// names is left as it is: not composed is not unneeded. A pass that would retire most of the node's consumers at once retires none, said as a fault of // the mesh (issue 940), unless retireMany. func (h *Holder) judgeNode(node string, needed map[string]provisions.Provision, ahead map[string]string, - untouched map[string]string, partial string, retireMany bool, out *Composed) { + untouched map[string]string, retireMany bool, out *Composed) { held := h.held(node) consumers := map[string]bool{} for key, have := range held { @@ -596,12 +585,10 @@ func (h *Holder) judgeNode(node string, needed map[string]provisions.Provision, } out.Problems = append(out.Problems, why) } + switch { case len(newly) == 0: h.clearFault(node) - case partial != "": - hold(fmt.Sprintf("%d key(s) of %s are not judged retiring while %s: %s", len(newly), node, partial, - strings.Join(newly, ", "))) case manyAtOnce(len(leaving), len(consumers)) && !retireMany: fault := fmt.Sprintf("On %s, one pass would retire the provisions of %d of the %d consumers that hold any (%s): "+ "most likely a read stopped listing them (a node's compositions published in part, a want, a have or the "+ @@ -1187,6 +1174,28 @@ 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 + +type unpublishedHold struct { + passes int + why string +} + +// countUnpublished counts, for each module held unpublished on node, the passes in a row it was; a module published +// again is forgotten. +func (h *Holder) countUnpublished(node string, modules []string, held map[string]string) { + if h.unpublished == nil { + h.unpublished = map[string]map[string]unpublishedHold{} + } + was := h.unpublished[node] + now := map[string]unpublishedHold{} + for _, m := range modules { + now[m] = unpublishedHold{passes: was[m].passes + 1, why: held[m]} + } + h.unpublished[node] = now +} + // Faults is, by node, each retirement a pass held back as a fault of the mesh. func (h *Holder) Faults() map[string]string { h.mu.Lock() @@ -1198,6 +1207,22 @@ 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. +func (h *Holder) LastingHolds() map[string]string { + h.mu.Lock() + defer h.mu.Unlock() + out := map[string]string{} + 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) + } + } + } + return out +} + func (h *Holder) put(key string, p provisions.Provision) { p.At = h.Now().UTC() if raw, err := provisions.Encode(p); err == nil { diff --git a/modules/mesh-provisioner/cmd/mesh-provisioner/holder_test.go b/modules/mesh-provisioner/cmd/mesh-provisioner/holder_test.go index a1148ba1..8a276e69 100644 --- a/modules/mesh-provisioner/cmd/mesh-provisioner/holder_test.go +++ b/modules/mesh-provisioner/cmd/mesh-provisioner/holder_test.go @@ -598,8 +598,23 @@ func TestARetirementIsJudgedOnlyFromAWholePicture(t *testing.T) { }{ {name: "a read not loaded once: nothing judged", consumer: 6, gone: 6, change: func(m *mesh) { m.h.Loaded = func() string { return WantsRead } }}, - {name: "a module's composition not published: nothing retires", consumer: 2, gone: 1, - change: func(m *mesh) { m.unpublished["novox"] = []string{"gitea"} }, problem: "not published yet"}, + {name: "another module's composition not published: a removed consumer still retires", consumer: 2, gone: 1, + change: func(m *mesh) { + 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. + 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) + } + m.unpublished["novox"], m.heldAt["novox"] = nil, nil + m.compose(t, "novox") + if a := m.h.health(); !a.Healthy { + t.Errorf("the hold is said after it ended: %+v", a) + } + }}, {name: "a consumer that still has a want: not retired", consumer: 2, gone: 1, change: func(m *mesh) { m.s.write(t, WantsRead, "want.novox.de-spiegel", `{"v":1,"fp":"`+fp+`","self":true}`) diff --git a/modules/mesh-provisioner/cmd/mesh-provisioner/holdertools.go b/modules/mesh-provisioner/cmd/mesh-provisioner/holdertools.go index a617e3d0..2280e7cf 100644 --- a/modules/mesh-provisioner/cmd/mesh-provisioner/holdertools.go +++ b/modules/mesh-provisioner/cmd/mesh-provisioner/holdertools.go @@ -80,21 +80,26 @@ func holderTools(h *Holder) []stdio.Tool { {Name: HealthTool, 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; healthy once a pass over that node judges none.", + "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.", Run: func(map[string]any) (any, error) { return h.health(), nil }}, } } // health is the holder's runtime health: unhealthy while a retirement is held back as a fault of the mesh. func (h *Holder) health() health.Answer { - faults := h.Faults() - if len(faults) == 0 { - return health.Healthy("No pass holds back a retirement of most of a node's consumers.") + 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.") } var said []string for _, n := range sortedKeys(faults) { said = append(said, faults[n]) } + for _, k := range sortedKeys(holds) { + said = append(said, k+": "+holds[k]+"; its plan and its provisions are kept as they were.") + } return health.Unhealthy(strings.Join(said, " ")) }