diff --git a/modules/mesh-provisioner/cmd/mesh-provisioner/composed_test.go b/modules/mesh-provisioner/cmd/mesh-provisioner/composed_test.go index fba00dafc..b044b58d7 100644 --- a/modules/mesh-provisioner/cmd/mesh-provisioner/composed_test.go +++ b/modules/mesh-provisioner/cmd/mesh-provisioner/composed_test.go @@ -215,10 +215,32 @@ func TestARefusedOrMissingCompositionHoldsItsModule(t *testing.T) { if !strings.Contains(comp.Newest.Held["restic"], row.held) || slices.Contains(comp.UnpublishedNewest, "restic") != (row.composed == "") { t.Errorf("%s: newest held %v, unpublished at the newest %v", row.name, comp.Newest.Held, comp.UnpublishedNewest) } + // A pass writes the newest hold into `refused` beside the wanted build's, and deletes it once it is read. + h.Compose = func(context.Context, string, map[string]string) Composition { return comp } + if _, err := h.ComposeNode(context.Background(), c.Node); err != nil { + t.Fatal(err) + } + if e, _ := s.Get(RefusedState, "n1.restic@newest"); e == nil || !strings.Contains(string(e.Value), row.held) { + t.Errorf("%s: refused holds %s", row.name, valueOf(e)) + } + h.Compose = FromCompositions(s) if _, held := comp.AtWants.Held["restic"]; held { t.Errorf("%s: its wanted build is composed, but the module is held there: %v", row.name, comp.AtWants.Held) } } + s.write(t, ModulesRead, "restic", strings.ReplaceAll(valueOf(mustEntry(t, s)), "src2:"+strings.Repeat("7", 64), fpsOf["restic"])) + if _, err := h.ComposeNode(context.Background(), c.Node); err != nil { + t.Fatal(err) + } + if e, _ := s.Get(RefusedState, "n1.restic@newest"); e != nil { + t.Errorf("the newest hold stays once the composition is read: %s", valueOf(e)) + } +} + +func mustEntry(t *testing.T, s *memStore) *Entry { + t.Helper() + e, _ := s.Get(ModulesRead, "restic") + return e } func readJSON(t *testing.T, path string, v any) { diff --git a/modules/mesh-provisioner/cmd/mesh-provisioner/holder.go b/modules/mesh-provisioner/cmd/mesh-provisioner/holder.go index 599b81e9c..c936414bb 100644 --- a/modules/mesh-provisioner/cmd/mesh-provisioner/holder.go +++ b/modules/mesh-provisioner/cmd/mesh-provisioner/holder.go @@ -64,6 +64,9 @@ type Refusal struct { Module string `json:"module"` Why string `json:"why"` Since time.Time `json:"since"` + // Scope is "newest" for a module held at its newest build (its composition there unread: key `.@newest`), + // "" for one held at its wanted build. + Scope string `json:"scope,omitempty"` } // The reads it makes the plans from. ComposedRead is every node's published compositions, `composed-` (ADR 0593 @@ -150,9 +153,6 @@ 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 - // newestHeld is, by node and module, each module held at its newest build with why (unpublished or refused), - // rederived by every pass from the node-engines' compositions and said by provisioner_plans. - newestHeld map[string]map[string]string // 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 @@ -254,13 +254,12 @@ func (h *Holder) ComposeNodeWith(ctx context.Context, node string, retireMany bo if len(heldAtWants) > 0 { out.Held = heldAtWants } - h.setNewestHeld(node, heldNewest) for _, m := range sortedKeys(heldNewest) { if _, ok := heldAtWants[m]; !ok { out.Problems = append(out.Problems, fmt.Sprintf("%s at its newest build is not composed: %s", m, heldNewest[m])) } } - if err := h.writeRefused(node, heldAtWants); err != nil { + if err := h.writeRefused(node, heldAtWants, heldNewest); err != nil { out.Problems = append(out.Problems, err.Error()) } // A module whose composition is not published at its build is held (heldAtWants): its own plan and keys are kept, @@ -363,16 +362,35 @@ func (h *Holder) ComposeNodeWith(ctx context.Context, node string, retireMany bo // writeRefused keeps, for each module held on node, why, and deletes the refusal of one that composes again: a change // of the state is the event a watcher hears. -func (h *Holder) writeRefused(node string, held map[string]string) error { +func (h *Holder) writeRefused(node string, held, heldNewest map[string]string) error { var problems []error - for _, module := range sortedKeys(held) { - key, err := meshprovisionerclient.PlanKey(node, module) - if err != nil { - problems = append(problems, err) - continue + // Each hold has a key: `.` at the wanted build, `.@newest` at the newest. + type hold struct { + key string + r Refusal + } + var holds []hold + for _, scope := range []struct { + name string + by map[string]string + }{{"", held}, {"newest", heldNewest}} { + for _, module := range sortedKeys(scope.by) { + key, err := meshprovisionerclient.PlanKey(node, module) + if err != nil { + problems = append(problems, err) + continue + } + if scope.name != "" { + key += "@" + scope.name + } + holds = append(holds, hold{key, Refusal{Node: node, Module: module, Why: scope.by[module], Since: h.Now().UTC(), Scope: scope.name}}) } - r := Refusal{Node: node, Module: module, Why: held[module], Since: h.Now().UTC()} - if e, err := h.Store.Get(RefusedState, key); err == nil && e != nil { + } + still := map[string]bool{} + for _, d := range holds { + still[d.key] = true + r := d.r + if e, err := h.Store.Get(RefusedState, d.key); err == nil && e != nil { var was Refusal if json.Unmarshal(e.Value, &was) == nil && was.Why == r.Why { continue @@ -383,11 +401,11 @@ func (h *Holder) writeRefused(node string, held map[string]string) error { problems = append(problems, err) continue } - if err := h.Store.Put(RefusedState, key, json.RawMessage(raw)); err != nil { - problems = append(problems, fmt.Errorf("the refusal %s: %w", key, err)) + if err := h.Store.Put(RefusedState, d.key, json.RawMessage(raw)); err != nil { + problems = append(problems, fmt.Errorf("the refusal %s: %w", d.key, err)) continue } - h.Logf("[%s] %s on %s has no plan composed: %s", Module, module, node, r.Why) + h.Logf("[%s] %s on %s has no plan composed%s: %s", Module, r.Module, node, map[bool]string{true: " at its newest build"}[r.Scope != ""], r.Why) } keys, err := h.Store.Keys(RefusedState) if err != nil { @@ -395,10 +413,7 @@ func (h *Holder) writeRefused(node string, held map[string]string) error { } for _, k := range keys { n, module, ok := strings.Cut(k, ".") - if !ok || n != node { - continue - } - if _, still := held[module]; still { + if !ok || n != node || still[k] { continue } if err := h.Store.Delete(RefusedState, k); err != nil { @@ -476,12 +491,17 @@ func (h *Holder) judgeElsewhere(keys map[string]meshprovisionerclient.Provision, next := keys[key] var have meshprovisionerclient.Provision e, err := h.Store.Get(ProvisionsState, key) + decoded := false if err == nil && e != nil { if have, err = meshprovisionerclient.Decode(e.Value); err == nil { next.Reference = have.Reference + decoded = true } } h.outcome(key, &next) + if decoded { + h.withdrawAsk(key, have, &next, out) + } if e != nil && same(have, next) { continue } @@ -657,11 +677,8 @@ func (h *Holder) judgeNode(node string, needed map[string]meshprovisionerclient. h.outcome(key, &next) // A key needed again is not asked about any more: its open ask is withdrawn on the operator's channel, and // kept on the key until the router took the cancel, so the next pass tries again (issue 1966). - if isHeld && have.Ask != "" && have.Warrant == nil { - if err := h.cancelAsk(have.Ask); err != nil { - out.Problems = append(out.Problems, fmt.Sprintf("the ask %s of %s could not be withdrawn: %v", have.Ask, key, err)) - next.Ask = have.Ask - } + if isHeld { + h.withdrawAsk(key, have, &next, out) } if module, isAhead := ahead[key]; isAhead && next.State == meshprovisionerclient.Asked { // The reason the provider's helper says too: no plan names it yet, and it binds it from this key's entry. @@ -1128,6 +1145,18 @@ func (h *Holder) ask(p *meshprovisionerclient.Provision, key string) error { return nil } +// withdrawAsk withdraws the open retire ask of a key that is needed again (next), keeping the ask on it when the router +// does not take the cancel, so the next pass tries again. +func (h *Holder) withdrawAsk(key string, have meshprovisionerclient.Provision, next *meshprovisionerclient.Provision, out *Composed) { + if have.Ask == "" || have.Warrant != nil { + return + } + if err := h.cancelAsk(have.Ask); err != nil { + out.Problems = append(out.Problems, fmt.Sprintf("the ask %s of %s could not be withdrawn: %v", have.Ask, key, err)) + next.Ask = have.Ask + } +} + // cancelAsk withdraws a retiring ask on the operator's channel, under mesh-provisioner's own name (accept.cancel). func (h *Holder) cancelAsk(id string) error { if h.Asks == nil { @@ -1332,31 +1361,6 @@ const ( newestKept = "its newest build's composition is unread: " ) -// setNewestHeld keeps the modules held at their newest build on node, with why, for provisioner_plans. -func (h *Holder) setNewestHeld(node string, held map[string]string) { - if h.newestHeld == nil { - h.newestHeld = map[string]map[string]string{} - } - if len(held) == 0 { - delete(h.newestHeld, node) - return - } - h.newestHeld[node] = held -} - -// NewestHeld is, by ".", each module held at its newest build, with why. -func (h *Holder) NewestHeld() map[string]string { - h.mu.Lock() - defer h.mu.Unlock() - out := map[string]string{} - for n, mods := range h.newestHeld { - for m, why := range mods { - out[n+"."+m] = why - } - } - return out -} - // 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() diff --git a/modules/mesh-provisioner/cmd/mesh-provisioner/holder_test.go b/modules/mesh-provisioner/cmd/mesh-provisioner/holder_test.go index 132a9968c..d8d973002 100644 --- a/modules/mesh-provisioner/cmd/mesh-provisioner/holder_test.go +++ b/modules/mesh-provisioner/cmd/mesh-provisioner/holder_test.go @@ -364,6 +364,19 @@ func TestRetiringIsAskedAndActsOnlyOnTheWarrant(t *testing.T) { } m.atWant["g14"] = map[string]catalogue.Plan{"gitea": planOf(oidc)} m.compose(t, "g14") + // A key of another node (a provider's shared credential) needed again withdraws its ask too, by the same helper. + retiring := m.state(t, pg) + if retiring.Ask == "" { + t.Fatal("the key holds no ask") + } + needed := retiring + needed.State, needed.Ask, needed.Warrant = "", "", nil + before := len(m.r.cancelled) + m.h.judgeElsewhere(map[string]meshprovisionerclient.Provision{pg: needed}, &Composed{}) + if len(m.r.cancelled) != before+1 || m.r.cancelled[before] != retiring.Ask || m.state(t, pg).Ask != "" { + t.Errorf("a credential key needed again: withdrawn %v, ask %q", m.r.cancelled, m.state(t, pg).Ask) + } + m.compose(t, "g14") // gone again: asked again p := m.state(t, pg) a := m.r.asked[p.Ask] if !strings.Contains(a.Explanation, "postgres") || !strings.Contains(a.Explanation, "ace") || @@ -717,8 +730,10 @@ func TestARetirementIsJudgedOnlyFromAWholePicture(t *testing.T) { if !strings.Contains(strings.Join(c.Changed, " "), "novox.de-spiegel.route.1 kept: its newest build's composition is unread") { t.Errorf("the pass does not say the key is kept: %v", c.Changed) } - if got := m.h.NewestHeld(); !strings.Contains(got["novox.de-spiegel"], "not published") { - t.Errorf("newest-held says %v", got) + // The hold is on the bus, in `refused` beside the wanted build's, so a restart or another instance sees it. + if e, _ := m.s.Get(RefusedState, "novox.de-spiegel@newest"); e == nil || !strings.Contains(string(e.Value), `"scope":"newest"`) || + !strings.Contains(string(e.Value), "not published") { + t.Errorf("refused holds %s", valueOf(e)) } now := m.h.Now() m.h.Now = func() time.Time { return now.Add(unpublishedFor + time.Minute) } diff --git a/modules/mesh-provisioner/cmd/mesh-provisioner/holdertools.go b/modules/mesh-provisioner/cmd/mesh-provisioner/holdertools.go index 02853c7f2..778e488f2 100644 --- a/modules/mesh-provisioner/cmd/mesh-provisioner/holdertools.go +++ b/modules/mesh-provisioner/cmd/mesh-provisioner/holdertools.go @@ -56,10 +56,11 @@ func holderTools(h *Holder) []stdio.Tool { return nil, err } out.(map[string]any)[RefusedState] = refused.(map[string]any)[RefusedState] - newest := map[string]string{} - for k, why := range h.NewestHeld() { - if node := arg(a, "node"); node == "" || strings.HasPrefix(k, node+".") { - newest[k] = why + // The modules held at their newest build are the refused rows of that scope, kept on the bus beside the others. + newest := []map[string]any{} + for _, row := range refused.(map[string]any)[RefusedState].([]map[string]any) { + if row["scope"] == "newest" { + newest = append(newest, row) } } out.(map[string]any)["newest-held"] = newest @@ -156,7 +157,7 @@ func (h *Holder) read(state, key, node, want string) (any, error) { if err := json.Unmarshal(e.Value, &r); err != nil { row["unreadable"] = err.Error() } else { - row["module"], row["why"], row["since"] = r.Module, r.Why, r.Since + row["module"], row["why"], row["since"], row["scope"] = r.Module, r.Why, r.Since, r.Scope } } rows = append(rows, row)