diff --git a/cmd/mesh-controller/batches.go b/cmd/mesh-controller/batches.go index 44f6426a..28e63435 100644 --- a/cmd/mesh-controller/batches.go +++ b/cmd/mesh-controller/batches.go @@ -1210,13 +1210,14 @@ func planBatch(ctx context.Context, open *stores, batch *inventory.Plan, carry [ } } sort.Strings(also) - ordered, err := changeOrderEdges(ctx, merges, members, entries, moved, edges) + ordered, together, err := changeOrderEdges(ctx, merges, members, entries, moved, edges) if err != nil { return err } var withheld []string plan := planOfMoves(moved, append(append([]inventory.Edge{}, edges...), ordered...)) placedBy(&plan, ordered) + placedTogether(&plan, together) // **Withheld, a stopped merge's modules** (novox/hq ADR 0299): a module is built at its branch, which holds the // merge a stop left out of the walk these merges were walked again from — built here only as a dependent, it // would deliver that merge. One a merge of this batch moves is built: that merge carries the stopped change, @@ -1341,6 +1342,19 @@ func placedBy(plan *inventory.Plan, ordered []inventory.Edge) { } } +// placedTogether writes the switch's line on the controller's and the node-engine's tier (ADR 0539 §11), so the plan +// says why they share one: the record and each edge overruled. +func placedTogether(plan *inventory.Plan, together string) { + if together == "" { + return + } + for _, n := range []string{theController, theEngine} { + if m, ok := plan.Modules[n]; ok { + m.Placed = strings.Trim(m.Placed+"; "+together, "; ") + } + } +} + // unchangedSaid names the modules a walk says matched what runs, for its note. func unchangedSaid(unchanged map[string]string) string { names := make([]string, 0, len(unchanged)) @@ -1536,7 +1550,7 @@ type batchMember struct { // A cycle is said, never lost: the plan puts the modules left in its last tier, and the composed check fails the // group before it merges. func changeOrderEdges(ctx context.Context, merges []inventory.BatchedMerge, members []batchMember, - entries []inventory.Entry, moved []string, edges []inventory.Edge) ([]inventory.Edge, error) { + entries []inventory.Entry, moved []string, edges []inventory.Edge) ([]inventory.Edge, string, error) { // What each repository's branch held before the batch: the first parent of its earliest merge in it. earliest := map[string]inventory.BatchedMerge{} for _, k := range merges { @@ -1559,17 +1573,27 @@ func changeOrderEdges(ctx context.Context, merges []inventory.BatchedMerge, memb } changes, err := changesOf(ctx, sources) if err != nil { - return nil, fmt.Errorf("what the merges say of their manifests cannot be read, so the order of their modules "+ + return nil, "", fmt.Errorf("what the merges say of their manifests cannot be read, so the order of their modules "+ "cannot be drawn: %w", err) } - order := orderEdges(moved, changes, edges) + named := false + for _, k := range merges { + var e link.SourceMoved + if json.Unmarshal(k.Event, &e) == nil && switchNamed(e.Body) { + named = true + } + } + order := orderEdges(moved, changes, edges, named) for _, e := range order.Edges { fmt.Printf(" order: %s\n", edgeSaid(e)) } + if order.Together != "" { + fmt.Printf(" order: %s\n", order.Together) + } if said := order.CycleSaid(); said != "" { fmt.Printf(" %s\n", said) } - return order.Edges, nil + return order.Edges, order.Together, nil } // searchAfterFailures starts the search a failed walk leaves (ADR 0276 decision 3): every merge it answered diff --git a/cmd/mesh-controller/delivery.go b/cmd/mesh-controller/delivery.go index cad30ba5..d626ad10 100644 --- a/cmd/mesh-controller/delivery.go +++ b/cmd/mesh-controller/delivery.go @@ -711,7 +711,7 @@ func orderOf(members []orderMember, reach map[string]orderReach, edges []invento } } } - order := orderEdges(all, changes, edges) + order := orderEdges(all, changes, edges, false) out.Edges = order.Edges out.CycleSaid = order.CycleSaid() for _, e := range order.Edges { diff --git a/cmd/mesh-controller/delivery_edges.go b/cmd/mesh-controller/delivery_edges.go index 8126201f..57f0bfdb 100644 --- a/cmd/mesh-controller/delivery_edges.go +++ b/cmd/mesh-controller/delivery_edges.go @@ -20,6 +20,11 @@ const ( ruleEngineFirst = "engine before controller" ) +// switchField is the line of a pull request's description that names release 6's switch (novox/hq ADR 0539 §11): +// "Release-switch: 6". A plan whose merges include a pull request with it puts the node-engine and the controller in +// one tier. Read by switchNamed, from the description the forge announced with the merge. +const switchField = "Release-switch:" + // The two modules the rollout rules name. const ( theController = "mesh-controller" @@ -39,6 +44,20 @@ type planOrder struct { Edges []inventory.Edge // Cycle is every edge of a cycle among the order's edges and the graph's, the module graph's included. Cycle []inventory.Edge + // Together is, when the switch is named and the plan moves the controller, the line its tier carries (ADR 0539 + // §11): the record and each edge it overruled (ADR 0419 clause 5). Empty otherwise. + Together string +} + +// switchNamed says whether a pull request's description names release 6's switch: a line that is exactly the field +// with a value. +func switchNamed(body string) bool { + for _, line := range strings.Split(body, "\n") { + if rest, ok := strings.CutPrefix(strings.TrimSpace(line), switchField); ok && strings.TrimSpace(rest) != "" { + return true + } + } + return false } // CycleSaid names every module and edge of the cycle, for a check to fail with (ADR 0419 clause 6). @@ -72,7 +91,9 @@ func orders(e inventory.Edge) bool { // it, goes before every module whose manifest the plan changes, the node-engine excepted; and the node-engine goes // before the controller by default, when the plan moves both and no edge orders them either way, transitively. An // edge is "From after To". graph is the module graph, whose edges order too and are never drawn again. -func orderEdges(moved []string, changes map[string]moduleChange, graph []inventory.Edge) planOrder { +// switchNamed is release 6's switch (ADR 0539 §11): the version-skew edge and the engine-first default are not drawn +// and the plan says it overruled them (Together); the verb edges and the graph still order. +func orderEdges(moved []string, changes map[string]moduleChange, graph []inventory.Edge, switchNamed bool) planOrder { var out planOrder sorted := append([]string(nil), moved...) sort.Strings(sorted) @@ -89,15 +110,25 @@ func orderEdges(moved []string, changes map[string]moduleChange, graph []invento } } } + var overruled []string if slices.Contains(sorted, theController) { for _, m := range sorted { if m != theController && m != theEngine && changes[m].Manifest { + if switchNamed { + overruled = append(overruled, ruleVersionSkew+" ("+m+" after "+theController+")") + continue + } out.Edges = append(out.Edges, inventory.Edge{From: m, To: theController, Kind: edgeChangeOrder, Why: ruleVersionSkew}) } } } - if slices.Contains(sorted, theController) && slices.Contains(sorted, theEngine) { + if switchNamed && slices.Contains(sorted, theController) && slices.Contains(sorted, theEngine) { + overruled = append(overruled, ruleEngineFirst) + } + if switchNamed && slices.Contains(sorted, theController) { + out.Together = switchSaid(overruled) + } else if slices.Contains(sorted, theController) && slices.Contains(sorted, theEngine) { all := orderingEdges(graph, out.Edges) switch { case pathBetween(all, theController, theEngine) != nil: @@ -127,6 +158,15 @@ func orderEdges(moved []string, changes map[string]moduleChange, graph []invento return out } +// switchSaid is the line of a tier the switch shapes: the record that decided it and each edge it overruled. +func switchSaid(overruled []string) string { + said := "release 6's switch (ADR 0539 §11)" + if len(overruled) > 0 { + said += " overrules " + strings.Join(overruled, ", ") + " (ADR 0419 clause 5)" + } + return said +} + // orderingEdges is the graph's edges that order and the edges drawn, together. func orderingEdges(graph, drawn []inventory.Edge) []inventory.Edge { var out []inventory.Edge diff --git a/cmd/mesh-controller/delivery_edges_test.go b/cmd/mesh-controller/delivery_edges_test.go index 96da775d..1912e73d 100644 --- a/cmd/mesh-controller/delivery_edges_test.go +++ b/cmd/mesh-controller/delivery_edges_test.go @@ -2,6 +2,7 @@ package main import ( "context" + "slices" "strings" "testing" @@ -28,7 +29,7 @@ func TestAVerbIsServedBeforeItsCallerIsSent(t *testing.T) { "server": {Serves: []string{"mesh-delivery.groups"}, Manifest: true}, "caller": {Invokes: []string{"mesh-delivery.groups"}, Manifest: true}, } - got := orderEdges([]string{"caller", "server"}, changes, nil) + got := orderEdges([]string{"caller", "server"}, changes, nil, false) e, ok := hasEdge(got.Edges, "caller", "server") if !ok || !strings.Contains(e.Why, "verb before caller") || !strings.Contains(e.Why, "mesh-delivery.groups") { t.Fatalf("the caller does not follow the server by the verb: %+v", got.Edges) @@ -47,7 +48,7 @@ func TestEdgesBothWaysAreACycleNamingEveryModuleAndEdge(t *testing.T) { "x": {Serves: []string{"s.a"}, Invokes: []string{"s.b"}}, "y": {Serves: []string{"s.b"}, Invokes: []string{"s.a"}}, } - got := orderEdges([]string{"x", "y"}, changes, nil) + got := orderEdges([]string{"x", "y"}, changes, nil, false) if len(got.Cycle) < 2 { t.Fatalf("no cycle found: %+v", got) } @@ -62,7 +63,7 @@ func TestEdgesBothWaysAreACycleNamingEveryModuleAndEdge(t *testing.T) { // The node-engine goes before the controller by default, and says so; an edge of the manifests the other way // overrules it, and the edge that did is said. func TestTheEngineGoesBeforeTheControllerUnlessAnEdgeSaysOtherwise(t *testing.T) { - got := orderEdges([]string{"mesh-controller", "mesh-host"}, nil, nil) + got := orderEdges([]string{"mesh-controller", "mesh-host"}, nil, nil, false) e, ok := hasEdge(got.Edges, "mesh-controller", "mesh-host") if !ok || e.Why != "engine before controller" { t.Fatalf("the engine does not go first: %+v", got.Edges) @@ -71,7 +72,7 @@ func TestTheEngineGoesBeforeTheControllerUnlessAnEdgeSaysOtherwise(t *testing.T) "mesh-host": {Serves: []string{"s.v"}}, "mesh-controller": {Invokes: []string{"s.v"}}, } - got = orderEdges([]string{"mesh-controller", "mesh-host"}, changes, nil) + got = orderEdges([]string{"mesh-controller", "mesh-host"}, changes, nil, false) if _, ok := hasEdge(got.Edges, "mesh-controller", "mesh-host"); !ok || len(got.Edges) != 1 || !strings.Contains(got.Edges[0].Why, "verb before caller") { t.Fatalf("the verb edge does not place the controller after the engine: %+v", got.Edges) @@ -81,7 +82,7 @@ func TestTheEngineGoesBeforeTheControllerUnlessAnEdgeSaysOtherwise(t *testing.T) "mesh-controller": {Serves: []string{"s.v"}}, "mesh-host": {Invokes: []string{"s.v"}}, } - got = orderEdges([]string{"mesh-controller", "mesh-host"}, changes, nil) + got = orderEdges([]string{"mesh-controller", "mesh-host"}, changes, nil, false) e, ok = hasEdge(got.Edges, "mesh-host", "mesh-controller") if !ok || len(got.Edges) != 1 || !strings.Contains(e.Why, "overrules engine before controller") { t.Fatalf("the verb edge does not say it overruled the default: %+v", got.Edges) @@ -91,14 +92,14 @@ func TestTheEngineGoesBeforeTheControllerUnlessAnEdgeSaysOtherwise(t *testing.T) // The controller goes before every module whose manifest the plan changes (version skew), the node-engine excepted. func TestTheControllerGoesBeforeEveryChangedManifest(t *testing.T) { changes := map[string]moduleChange{"app": {Manifest: true}, "other": {}} - got := orderEdges([]string{"mesh-controller", "app", "other"}, changes, nil) + got := orderEdges([]string{"mesh-controller", "app", "other"}, changes, nil, false) if e, ok := hasEdge(got.Edges, "app", "mesh-controller"); !ok || e.Why != "version skew" { t.Fatalf("the changed manifest does not follow the controller: %+v", got.Edges) } if _, ok := hasEdge(got.Edges, "other", "mesh-controller"); ok { t.Fatalf("a module with an unchanged manifest was ordered: %+v", got.Edges) } - if got := orderEdges([]string{"app", "other"}, changes, nil); len(got.Edges) != 0 { + if got := orderEdges([]string{"app", "other"}, changes, nil, false); len(got.Edges) != 0 { t.Fatalf("version skew ordered a plan that does not move the controller: %+v", got.Edges) } } @@ -109,7 +110,7 @@ func TestTheOrderIsPerModuleNotPerRepository(t *testing.T) { "a": {Serves: []string{"s.v"}}, "c": {Invokes: []string{"s.v"}}, } - got := orderEdges([]string{"a", "b", "c"}, changes, nil) + got := orderEdges([]string{"a", "b", "c"}, changes, nil, false) tiers := tiersOf([]string{"a", "b", "c"}, got.Edges) if len(tiers) != 2 || strings.Join(tiers[0], ",") != "a,b" || strings.Join(tiers[1], ",") != "c" { t.Fatalf("tiers %v, want [a b] then [c]", tiers) @@ -181,3 +182,53 @@ func TestATruncatedChangeIsReadAsChangingTheManifestsOfWhatItMoves(t *testing.T) t.Fatalf("the member's change is read as %+v", src) } } + +// 1942/AC1: release 6's switch puts the node-engine and the controller in one tier; without it the controller's +// tier comes first as today (ADR 0539 §11). +func TestTheSwitchPutsTheEngineAndTheControllerInOneTier(t *testing.T) { + moved := []string{"mesh-controller", "mesh-host", "app"} + changes := map[string]moduleChange{"mesh-host": {Manifest: true}, "app": {Manifest: true}} + with := orderEdges(moved, changes, nil, true) + if len(with.Edges) != 0 || len(with.Cycle) != 0 { + t.Fatalf("the switch still draws edges: %+v", with) + } + if tiers := tiersOf(moved, with.Edges); len(tiers) != 1 || len(tiers[0]) != 3 { + t.Fatalf("tiers %v, want one tier", tiers) + } + without := orderEdges(moved, changes, nil, false) + tiers := tiersOf(moved, without.Edges) + if len(tiers) < 2 || slices.Contains(tiers[0], "mesh-controller") && slices.Contains(tiers[0], "mesh-host") { + t.Fatalf("without the switch the tiers are %v, want the engine's tier before the controller's", tiers) + } + if without.Together != "" { + t.Fatalf("a plan without the switch carries its line: %q", without.Together) + } + // A verb edge between them still orders them. + verb := map[string]moduleChange{"mesh-host": {Serves: []string{"s.v"}}, "mesh-controller": {Invokes: []string{"s.v"}}} + if got := orderEdges([]string{"mesh-controller", "mesh-host"}, verb, nil, true); len(got.Edges) != 1 { + t.Fatalf("the switch dropped a verb edge: %+v", got.Edges) + } +} + +// 1942/AC2: the shared tier's line names the record and each edge overruled. +func TestTheSwitchSaysTheEdgesItOverruled(t *testing.T) { + changes := map[string]moduleChange{"app": {Manifest: true}} + got := orderEdges([]string{"mesh-controller", "mesh-host", "app"}, changes, nil, true) + for _, want := range []string{"ADR 0539 §11", "engine before controller", "version skew (app after mesh-controller)", "ADR 0419"} { + if !strings.Contains(got.Together, want) { + t.Fatalf("the line %q lacks %q", got.Together, want) + } + } + plan := inventory.Plan{Modules: map[string]*inventory.PlanModule{"mesh-controller": {}, "mesh-host": {}}} + placedTogether(&plan, got.Together) + if plan.Modules["mesh-controller"].Placed != got.Together || plan.Modules["mesh-host"].Placed != got.Together { + t.Fatalf("the tier does not carry the line: %+v %+v", plan.Modules["mesh-controller"], plan.Modules["mesh-host"]) + } +} + +// The switch is named by a line of a pull request's description. +func TestTheSwitchIsNamedByAFieldOfTheDescription(t *testing.T) { + if !switchNamed("Fixes 1\n\nRelease-switch: 6\n") || switchNamed("Release-switch:\n") || switchNamed("about the Release-switch: 6") { + t.Fatal("the field is not read as a line with a value") + } +}