diff --git a/cmd/mesh-controller/delivery.go b/cmd/mesh-controller/delivery.go index 0a8a4a88..1a4d7400 100644 --- a/cmd/mesh-controller/delivery.go +++ b/cmd/mesh-controller/delivery.go @@ -117,28 +117,180 @@ func letGo(ctx context.Context, inv *inventory.Inventory, id, by, why string) (i // stopWalk ends a walk on its delivery's word: failed, said as stopped by whom, why. What it asked still // builds and registers; nothing further is asked or sent. func stopWalk(ctx context.Context, inv *inventory.Inventory, id, by, why string) (inventory.Plan, error) { + p, _, err := stopWalkWithout(ctx, inv, id, by, why, nil, time.Now().UTC()) + return p, err +} + +// stopAnswer is what a stop that leaves merges out says, as data (novox/hq ADR 0299): the walk stopped, the +// batch that walks its other merges again, those merges as `@`, and every merge left out. +type stopAnswer struct { + Stopped string `json:"stopped"` + WalkedAgainBy string `json:"walked_again_by,omitempty"` + WalkedAgain []string `json:"walked_again,omitempty"` + LeftOut []inventory.PlanLeftOut `json:"left_out,omitempty"` +} + +// stopWalkWithout ends a walk on its delivery's word and, given merges to leave out, walks every other merge it +// answered again in a fresh batch without them (novox/hq issue 459, ADR 0299): one walk carries every merge of its +// batch, so ending it for one group member ended every other delivery on it. Left out are the merges named and +// each later merge of their repository's branch, which contains one of them and cannot be walked without +// delivering it. The fresh batch's window is closed — its merges were heard before — and it is cut by the usual +// rules, as a failed walk's merges are walked again (ADR 0276 decision 3). With nothing left to walk, or no merge +// named, the walk is only ended. A merge named that the walk does not answer refuses the stop whole. +func stopWalkWithout(ctx context.Context, inv *inventory.Inventory, id, by, why string, without []string, + now time.Time) (inventory.Plan, stopAnswer, error) { + said := stopAnswer{Stopped: id} release, err := inv.HoldPlans(ctx, true) if err != nil { - return inventory.Plan{}, err + return inventory.Plan{}, said, err } defer release() p, err := inv.PlanByID(ctx, id) if err != nil { - return inventory.Plan{}, err + return inventory.Plan{}, said, err } if !p.Open() { - return p, fmt.Errorf("%s is already %s", p.ID, p.State) + return p, said, fmt.Errorf("%s is already %s", p.ID, p.State) + } + var again []inventory.BatchedMerge + if len(without) > 0 { + merges, err := inv.MergesOf(ctx, p.ID) + if err != nil { + return p, said, err + } + if again, said.LeftOut, err = leaveOut(merges, without); err != nil { + return p, said, fmt.Errorf("%s is not stopped: %w", p.ID, err) + } } if p.Delivery == nil { p.Delivery = &inventory.PlanDelivery{} } p.Delivery.Stopped, p.Delivery.StoppedWhy = by, why + p.Delivery.LeftOut = said.LeftOut p.State = inventory.PlanFailed p.Note = fmt.Sprintf("stopped by %s at tier %d: %s", by, p.Tier, why) - if err := inv.SavePlan(ctx, &p); err != nil { - return p, err + if len(again) > 0 { + batch, err := walkAgain(ctx, inv, p, again, now) + if err != nil { + return p, said, fmt.Errorf("%s is not stopped: its other merges could not be batched again: %w", p.ID, err) + } + said.WalkedAgainBy = batch.ID + for _, m := range again { + said.WalkedAgain = append(said.WalkedAgain, m.Repository+"@"+m.Commit) + } + p.Delivery.WalkedAgainBy = batch.ID + p.Note += fmt.Sprintf("; its other merges are walked again without %s, by %s: %s", mergesLeftOutWords(said.LeftOut), + batch.ID, strings.Join(said.WalkedAgain, ", ")) + if err := inv.SavePlan(ctx, &p); err != nil { + // The merges go back to the walk that still answers them, and the batch holds none: it is never walked. + for _, m := range again { + _ = inv.AnswerMerge(ctx, m.Repository, m.Commit, p.ID, m.Alone) + } + return p, said, err + } + fmt.Printf("%s stopped by %s; %s walks its other merges again: %s\n", p.ID, by, batch.ID, + strings.Join(said.WalkedAgain, ", ")) + return p, said, nil } - return p, nil + if len(said.LeftOut) > 0 { + p.Note += fmt.Sprintf("; left out %s, nothing else to walk", mergesLeftOutWords(said.LeftOut)) + } + if err := inv.SavePlan(ctx, &p); err != nil { + return p, said, err + } + return p, said, nil +} + +// walkAgain puts merges of a stopped walk into a fresh batch of their kind, its window closed: kept before its +// first merge names it, then each merge moved to it. +func walkAgain(ctx context.Context, inv *inventory.Inventory, stopped inventory.Plan, again []inventory.BatchedMerge, + now time.Time) (inventory.Plan, error) { + own := false + for _, m := range again { + for _, n := range announcedOf(m).Moves { + if _, on := onTheControllersPath[n]; on { + own = true + } + } + } + first := again[0] + b := inventory.Plan{ID: fmt.Sprintf("plan-%d", now.UnixNano()), Repository: first.Repository, Branch: first.Branch, + Commit: first.Commit, Merged: first.Merged, Created: now, State: inventory.PlanAssembling, + Tiers: [][]string{}, Modules: map[string]*inventory.PlanModule{}, + Delivery: &inventory.PlanDelivery{Batch: &inventory.PlanBatch{Own: own}}} + if b.ID == stopped.ID { + b.ID += "-again" + } + if err := inv.SavePlan(ctx, &b); err != nil { + return b, err + } + for _, m := range again { + if err := inv.AnswerMerge(ctx, m.Repository, m.Commit, b.ID, m.Alone); err != nil { + return b, err + } + } + // Its merges were heard before: the window, measured from the last of them, is closed, and the cutter cuts it + // once no walk of its kind is open. + return b, keepBatch(ctx, inv, &b, now, "") +} + +// leaveOut splits a walk's merges by the merges named to leave out, each `@` (a commit's +// first characters will do): the merges walked again, and those left out — each named one, and each later merge +// of its repository's branch, which contains it. A name the walk does not answer is refused. +func leaveOut(merges []inventory.BatchedMerge, without []string) ([]inventory.BatchedMerge, []inventory.PlanLeftOut, error) { + var stopped []inventory.BatchedMerge + for _, w := range without { + repository, commit, ok := strings.Cut(strings.TrimSpace(w), "@") + if !ok || repository == "" || len(commit) < 7 { + return nil, nil, fmt.Errorf("%q names no merge: @", w) + } + i := slices.IndexFunc(merges, func(m inventory.BatchedMerge) bool { + return strings.EqualFold(m.Repository, repository) && strings.HasPrefix(m.Commit, commit) + }) + if i < 0 { + return nil, nil, fmt.Errorf("it answers no merge %s@%s", repository, commit) + } + stopped = append(stopped, merges[i]) + } + // The earliest stopped merge of each repository's branch: every merge of that branch after it contains it. + earliest := map[string]inventory.BatchedMerge{} + for _, s := range stopped { + k := strings.ToLower(s.Repository) + "\x00" + s.Branch + if e, ok := earliest[k]; !ok || laterMerge(e, s) { + earliest[k] = s + } + } + var again []inventory.BatchedMerge + var out []inventory.PlanLeftOut + for _, m := range merges { + named := slices.ContainsFunc(stopped, func(s inventory.BatchedMerge) bool { + return strings.EqualFold(s.Repository, m.Repository) && s.Commit == m.Commit + }) + e, has := earliest[strings.ToLower(m.Repository)+"\x00"+m.Branch] + switch { + case named: + out = append(out, inventory.PlanLeftOut{Repository: m.Repository, Commit: m.Commit}) + case has && laterMerge(m, e): + out = append(out, inventory.PlanLeftOut{Repository: m.Repository, Commit: m.Commit, Contains: e.Commit}) + default: + again = append(again, m) + } + } + sort.SliceStable(out, func(i, j int) bool { return out[i].Contains == "" && out[j].Contains != "" }) + return again, out, nil +} + +// mergesLeftOutWords is the merges a stop left out, as a person reads them: each that contains a stopped one says so. +func mergesLeftOutWords(out []inventory.PlanLeftOut) string { + var words []string + for _, o := range out { + s := o.Repository + "@" + short(o.Commit) + if o.Contains != "" { + s += " (which contains " + short(o.Contains) + ")" + } + words = append(words, s) + } + return strings.Join(words, ", ") } // orderMember is one member of a group, as mesh-delivery says it. @@ -466,6 +618,8 @@ func deliveryCommand(ctx context.Context, args []string) error { group := set.String("group", "", "a delivery group's id") by := set.String("by", catalogue.DeliverySeat, "who says it") why := set.String("why", "", "why") + without := set.String("without", "", "merges a stop leaves out, @, comma-separated: the walk's "+ + "other merges are walked again without them (novox/hq ADR 0299)") limit := set.Int("n", 50, "how many ended walks to answer beside the open ones") planID := set.String("plan", "", "one walk") positionals, err := parseAround(set, rest) @@ -552,15 +706,21 @@ func deliveryCommand(ctx context.Context, args []string) error { return nil case "stop": if len(positionals) != 1 || strings.TrimSpace(*why) == "" { - return errors.New("delivery stop --why [--by ]") + return errors.New("delivery stop --why [--by ] [--without @,…]") } var p inventory.Plan + var said stopAnswer if err := sayingOnTheBus(ctx, func() (err error) { - p, err = stopWalk(ctx, inv, positionals[0], *by, strings.TrimSpace(*why)) + p, said, err = stopWalkWithout(ctx, inv, positionals[0], *by, strings.TrimSpace(*why), splitList(*without), + time.Now().UTC()) return err }); err != nil { return err } + if *without != "" { + // What it left out and what it walks again, as data: the delivery's owner reads it (ADR 0299). + return answer(said) + } fmt.Printf("%s stopped by %s at tier %d of %d; what was asked still builds and registers, nothing further "+ "is asked or sent\n", p.ID, *by, p.Tier, len(p.Tiers)) return nil diff --git a/cmd/mesh-controller/issue459_test.go b/cmd/mesh-controller/issue459_test.go new file mode 100644 index 00000000..c5591b3d --- /dev/null +++ b/cmd/mesh-controller/issue459_test.go @@ -0,0 +1,177 @@ +package main + +import ( + "reflect" + "strings" + "testing" + "time" + + "github.com/novox/mesh-controller/internal/inventory" +) + +// novox/hq issue 459, ADR 0299: since the merge window one walk carries every merge of its batch, so a stop of one +// group member through `delivery-stop` ended the walk of every other delivery in it. The stop now names the merges +// to leave out: the walk is ended, and every other merge it answered is walked again in a fresh batch without them +// — the stopped merge, and each later merge of its repository's branch, which contains it and ends with it. + +// leaved is a kept merge of a repository, made at a moment. +func leaved(repository, commit string, made time.Time) inventory.BatchedMerge { + return inventory.BatchedMerge{Repository: repository, Branch: "main", Commit: commit, Merged: made, Heard: made} +} + +// The stopped merge is left out, and so is each later merge of its repository's branch, naming the merge it +// contains; an earlier merge of that repository and every other repository's merge are walked again. +func TestLeftOutAreTheStoppedMergeAndTheMergesThatContainIt(t *testing.T) { + c1 := leaved("novox/mesh-catalog", "c1aaaaaaaaaa", t0) + c2 := leaved("novox/mesh-catalog", "c2bbbbbbbbbb", t0.Add(10*time.Second)) + c3 := leaved("novox/mesh-catalog", "c3cccccccccc", t0.Add(20*time.Second)) + o1 := leaved("novox/one", "o1dddddddddd", t0.Add(5*time.Second)) + again, out, err := leaveOut([]inventory.BatchedMerge{c1, o1, c2, c3}, []string{"novox/Mesh-Catalog@c2bbbbbbbbbb"}) + if err != nil { + t.Fatal(err) + } + var walked []string + for _, m := range again { + walked = append(walked, m.Commit) + } + if !reflect.DeepEqual(walked, []string{c1.Commit, o1.Commit}) { + t.Fatalf("walked again: %v; want the earlier catalogue merge and the other repository's", walked) + } + want := []inventory.PlanLeftOut{{Repository: "novox/mesh-catalog", Commit: c2.Commit}, + {Repository: "novox/mesh-catalog", Commit: c3.Commit, Contains: c2.Commit}} + if !reflect.DeepEqual(out, want) { + t.Fatalf("left out: %+v; want %+v", out, want) + } + // A merge the walk does not answer is refused, naming it: nothing ends. + if _, _, err := leaveOut([]inventory.BatchedMerge{c1, o1}, []string{"novox/mesh-catalog@ffffffffffff"}); err == nil || + !strings.Contains(err.Error(), "ffffffffffff") { + t.Fatalf("a merge the walk does not answer was left out: %v", err) + } + if _, _, err := leaveOut([]inventory.BatchedMerge{c1}, []string{"no-commit"}); err == nil { + t.Fatal("a merge without its commit was taken") + } +} + +// The seat verb passes the merges to leave out to the command. +func TestDeliveryStopTakesTheMergesToLeaveOut(t *testing.T) { + got, err := argvFor("delivery-stop", map[string]any{"plan": "plan-1", "why": "w", "by": "jochen", + "without": "novox/mesh-lab@b1,novox/mesh-lab@b2"}) + want := []string{"delivery", "stop", "plan-1", "--why", "w", "--by", "mesh-delivery for jochen", + "--without", "novox/mesh-lab@b1,novox/mesh-lab@b2"} + if err != nil || !reflect.DeepEqual(got, want) { + t.Fatalf("delivery-stop with without → %v %v, wanted %v", got, err, want) + } +} + +// A walk of the catalogue's merge and another repository's is stopped without the catalogue's: it ends stopped, +// names the batch that walks the other merge again and the merge it left out, and that batch is cut into a walk +// that builds the other repository's module alone. +func TestAStopWithoutAMergeWalksTheOthersAgain(t *testing.T) { + open := windowed(t) + asksRecorded(t) + ctx := t.Context() + cat := catalogueMerge("cat1aaaaaaaa", "app", t0) + one := repoMerge("one", "one1bbbbbbbb", t0.Add(5*time.Second)) + hear(t, open, cat, t0.Add(time.Second)) + hear(t, open, one, t0.Add(6*time.Second)) + cutAt(t, open, t0.Add(2*time.Minute)) + ws, _ := walks(t, open) + if len(ws) != 1 || !ws[0].Open() { + t.Fatalf("no open walk to stop: %+v", ws) + } + stopped := ws[0] + p, said, err := stopWalkWithout(ctx, open.inventory, stopped.ID, "mesh-delivery for jochen", "its group stopped", + []string{"novox/mesh-catalog@" + cat.Commit}, t0.Add(3*time.Minute)) + if err != nil { + t.Fatal(err) + } + if p.State != inventory.PlanFailed || p.Delivery.Stopped == "" || p.Delivery.WalkedAgainBy == "" || + !reflect.DeepEqual(p.Delivery.LeftOut, []inventory.PlanLeftOut{{Repository: "novox/mesh-catalog", Commit: cat.Commit}}) { + t.Fatalf("the stopped walk reads %s %+v", p.State, p.Delivery) + } + if said.WalkedAgainBy != p.Delivery.WalkedAgainBy || !reflect.DeepEqual(said.WalkedAgain, []string{"novox/one@" + one.Commit}) { + t.Fatalf("the stop answered %+v", said) + } + again, err := open.inventory.MergesOf(ctx, p.Delivery.WalkedAgainBy) + if err != nil || len(again) != 1 || again[0].Commit != one.Commit { + t.Fatalf("the fresh batch answers %+v (%v); want the other repository's merge alone", again, err) + } + if left, _ := open.inventory.MergesOf(ctx, stopped.ID); len(left) != 1 || left[0].Commit != cat.Commit { + t.Fatalf("the stopped walk still answers %+v; want the left-out merge alone", left) + } + cutAt(t, open, t0.Add(4*time.Minute)) + fresh, err := open.inventory.PlanByID(ctx, p.Delivery.WalkedAgainBy) + if err != nil || fresh.Batch() || !fresh.Open() { + t.Fatalf("the fresh batch was not cut into a walk: %s %v", fresh.State, err) + } + if _, in := fresh.Modules["one"]; !in { + t.Fatalf("the fresh walk does not build one: %v", fresh.Modules) + } + if _, in := fresh.Modules["app"]; in { + t.Fatalf("the fresh walk builds app, whose merge was left out: %v", fresh.Modules) + } + if len(fresh.Delivery.Merges) != 1 || fresh.Delivery.Merges[0].Commit != one.Commit { + t.Fatalf("the fresh walk answers %+v", fresh.Delivery.Merges) + } + // A stopped walk starts no search of its own (ADR 0276 decision 3 is for a failed one). + cutAt(t, open, t0.Add(5*time.Minute)) + if alone, _ := open.inventory.AloneMerges(ctx); len(alone) != 0 { + t.Fatalf("a merge of the stopped walk waits to be walked alone: %+v", alone) + } +} + +// A later merge of the stopped merge's repository contains it: it is left out too, named with the merge it +// contains, and only the other repository's merge is walked again. With nothing left to walk, the walk is only +// ended. A merge the walk does not answer is refused, and nothing ends. +func TestAStopWithoutAMergeEndsTheMergesThatContainIt(t *testing.T) { + open := windowed(t) + asksRecorded(t) + ctx := t.Context() + first := catalogueMerge("cat1aaaaaaaa", "app", t0) + later := catalogueMerge("cat2cccccccc", "notes", t0.Add(10*time.Second)) + one := repoMerge("one", "one1bbbbbbbb", t0.Add(5*time.Second)) + hear(t, open, first, t0.Add(time.Second)) + hear(t, open, one, t0.Add(6*time.Second)) + hear(t, open, later, t0.Add(11*time.Second)) + cutAt(t, open, t0.Add(2*time.Minute)) + ws, _ := walks(t, open) + walk := ws[0] + + if _, _, err := stopWalkWithout(ctx, open.inventory, walk.ID, "mesh-delivery for jochen", "x", + []string{"novox/mesh-catalog@ffffffffffff"}, t0.Add(3*time.Minute)); err == nil { + t.Fatal("a stop leaving out a merge the walk does not answer was taken") + } + if p, _ := open.inventory.PlanByID(ctx, walk.ID); !p.Open() { + t.Fatalf("a refused stop ended the walk: %s", p.State) + } + + p, said, err := stopWalkWithout(ctx, open.inventory, walk.ID, "mesh-delivery for jochen", "its group stopped", + []string{"novox/mesh-catalog@" + first.Commit}, t0.Add(3*time.Minute)) + if err != nil { + t.Fatal(err) + } + want := []inventory.PlanLeftOut{{Repository: "novox/mesh-catalog", Commit: first.Commit}, + {Repository: "novox/mesh-catalog", Commit: later.Commit, Contains: first.Commit}} + if !reflect.DeepEqual(p.Delivery.LeftOut, want) || !reflect.DeepEqual(said.LeftOut, want) { + t.Fatalf("left out %+v, said %+v; want %+v", p.Delivery.LeftOut, said.LeftOut, want) + } + if !strings.Contains(p.Note, later.Commit[:8]) || !strings.Contains(p.Note, p.Delivery.WalkedAgainBy) { + t.Fatalf("the stopped walk's note does not name the merge ended with it and the fresh batch: %q", p.Note) + } + again, _ := open.inventory.MergesOf(ctx, p.Delivery.WalkedAgainBy) + if len(again) != 1 || again[0].Commit != one.Commit { + t.Fatalf("walked again: %+v; want the other repository's merge alone", again) + } + + // Nothing left once the other repository's merge is left out too: the fresh walk is stopped, and nothing more. + cutAt(t, open, t0.Add(4*time.Minute)) + fresh, _ := open.inventory.PlanByID(ctx, p.Delivery.WalkedAgainBy) + q, said, err := stopWalkWithout(ctx, open.inventory, fresh.ID, "mesh-delivery for jochen", "that one too", + []string{"novox/one@" + one.Commit}, t0.Add(5*time.Minute)) + if err != nil || q.State != inventory.PlanFailed || q.Delivery.WalkedAgainBy != "" || said.WalkedAgainBy != "" { + t.Fatalf("a stop leaving nothing to walk: %s %+v %+v %v", q.State, q.Delivery, said, err) + } + if _, bs := walks(t, open); len(bs) != 0 { + t.Fatalf("a batch of nothing was made: %+v", bs) + } +} diff --git a/cmd/mesh-controller/seatverbs.go b/cmd/mesh-controller/seatverbs.go index 81838973..ac0030d0 100644 --- a/cmd/mesh-controller/seatverbs.go +++ b/cmd/mesh-controller/seatverbs.go @@ -410,6 +410,10 @@ func (a *verbArguments) commandLine() ([]string, error) { if b := str("by"); b != "" { argv = append(argv, "--by", catalogue.DeliverySeat+" for "+b) } + // The merges to leave out: the walk's other merges are walked again without them (novox/hq ADR 0299). + if w := str("without"); w != "" { + argv = append(argv, "--without", w) + } return argv, nil case "delivery-walks": argv := []string{"delivery", "walks"} diff --git a/internal/catalogue/verbs.go b/internal/catalogue/verbs.go index f3aaedaf..965f06bf 100644 --- a/internal/catalogue/verbs.go +++ b/internal/catalogue/verbs.go @@ -160,8 +160,12 @@ var ControllerVerbs = []Verb{ Input: schema(map[string]string{"plan": "the walk's id", "why": "what lets it go: its turn, a person's release"}, []string{"plan"})}, {Name: "delivery-stop", Description: "End a walk on its delivery's word (novox/hq ADR 0239): failed, said as stopped " + - "by whom and why; what it asked still builds and registers, nothing further is asked or sent.", - Input: schema(map[string]string{"plan": "the walk's id", "why": "why", "by": "who stopped the delivery"}, + "by whom and why; what it asked still builds and registers, nothing further is asked or sent. Given merges to " + + "leave out, every other merge the walk answered is walked again in a fresh batch without them and without each " + + "later merge of their repository, which contains one and is left out too, named (novox/hq ADR 0299); the " + + "answer names the batch, the merges walked again and the merges left out.", + Input: schema(map[string]string{"plan": "the walk's id", "why": "why", "by": "who stopped the delivery", + "without": "merges to leave out, @, comma-separated"}, []string{"plan", "why"})}, {Name: "delivery-walks", Description: "The walks the controller keeps (novox/hq ADR 0239): every open one and the " + "last ended ones, each whole — its tiers, each module's state, first machines and first-node gate with its readings, and its " + diff --git a/internal/inventory/plans.go b/internal/inventory/plans.go index bd03edc5..cbc20f3f 100644 --- a/internal/inventory/plans.go +++ b/internal/inventory/plans.go @@ -187,6 +187,11 @@ type PlanDelivery struct { // TakenOverBy names the walk a walk folded before it started was taken over by: its merges are that // walk's now. TakenOverBy string `json:"taken_over_by,omitempty"` + // WalkedAgainBy names, on a walk stopped without some of its merges, the batch that walks its other merges + // again, and LeftOut the merges it left out (novox/hq issue 459, ADR 0299): a delivery of a merge left out + // ends stopped with the walk, one of a merge walked again follows that batch's walk. + WalkedAgainBy string `json:"walked_again_by,omitempty"` + LeftOut []PlanLeftOut `json:"left_out,omitempty"` // Batch is the window of a batch; nil once it is cut into a walk. Batch *PlanBatch `json:"batch,omitempty"` // Alone says the walk walks one merge on its own commit, after a failed walk carried it in a later one: @@ -194,6 +199,16 @@ type PlanDelivery struct { Alone bool `json:"alone,omitempty"` } +// PlanLeftOut is a merge a stop left out of the batch that walks a stopped walk's merges again (novox/hq ADR +// 0299): a merge stopped, or a later merge of its repository's branch, which contains the stopped one and so +// cannot be walked without delivering it. +type PlanLeftOut struct { + Repository string `json:"repository"` + Commit string `json:"commit"` + // Contains is the stopped merge a later merge contains; empty for a merge stopped itself. + Contains string `json:"contains,omitempty"` +} + // Waiting is whether the walk waits for its delivery's word. func (p Plan) Waiting() bool { return p.Delivery != nil && p.Delivery.Awaits != "" && p.Delivery.Go == nil