diff --git a/cmd/mesh-controller/planner_records_test.go b/cmd/mesh-controller/planner_records_test.go new file mode 100644 index 00000000..bb636f13 --- /dev/null +++ b/cmd/mesh-controller/planner_records_test.go @@ -0,0 +1,131 @@ +package main + +import ( + "reflect" + "strings" + "testing" + "time" + + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/inventory" +) + +// **The planner over edges the store derives**, not edges written by hand: modules registered, builds +// recorded with what they stood on and which repositories they read, the relation answered by +// inventory.Dependencies (dependenciesOf over the records), and the merge planned by reachOfMerge — the +// path a real merge takes, short of the bus. +// +// **The repository rows are CURRENT BEHAVIOUR, documented — not the rule the operator states** +// (novox/hq issue 338, and the decision pending on it): a build that read a repository gives its module a +// packages edge to every module built from that repository, and mergeCandidates moves it on any merge to +// that repository, whatever the files. So a change to C alone, or to a README, moves the module that +// packages C's repository. ADR 0238 §3 records exactly that today ("a repository a recipe names"); the +// expectations marked 338 change with that decision. +func TestASharedRepositoryIsPlannedFromTheRecordsAsItIsToday(t *testing.T) { + inv := inventory.ForTest(t) + ctx := t.Context() + asked := time.Now().Add(-time.Hour) + register := func(m catalogue.Manifest, repository, path string, against []string, read []inventory.ReadRepository) { + t.Helper() + if err := inv.RegisterModule(ctx, m, inventory.Source{Repository: repository, Seat: "git", Path: path, + Ref: "main", BuiltFrom: "old", Asked: asked}); err != nil { + t.Fatal(err) + } + if err := inv.RecordBuild(ctx, inventory.Build{ID: "build-" + m.Module, Repository: repository, Ref: "main", + Module: m.Module, Commit: "old", On: "builder", Path: path, Against: against, Read: read, Asked: asked}); err != nil { + t.Fatal(err) + } + } + controllerRead := []inventory.ReadRepository{{Repository: "novox/mesh-controller", Ref: "main"}} + agent := catalogue.Manifest{Module: "build-agent", Version: "1", + Claims: []catalogue.Claim{{Name: "node-build-agent", Scope: catalogue.ScopeNode}}} + + // The shape of issue 338. + register(catalogue.Manifest{Module: "mesh-controller", Version: "1"}, "novox/mesh-controller", "", nil, nil) + register(agent, "novox/mesh-catalog", "modules/build-agent", nil, controllerRead) + register(catalogue.Manifest{Module: "route-proxy", Version: "1"}, "novox/mesh-catalog", "modules/route-proxy", nil, controllerRead) + register(catalogue.Manifest{Module: "gitea", Version: "1"}, "novox/mesh-catalog", "modules/gitea", nil, nil) + // A, B and C in one repository; D built against A's artifact, E declaring B, P packaging the repository. + for _, n := range []string{"a", "b", "c"} { + register(catalogue.Manifest{Module: n, Version: "1"}, "novox/one", "modules/"+n, nil, nil) + } + register(catalogue.Manifest{Module: "d", Version: "1"}, "novox/two", "d", + []string{catalogue.ArtifactStoreScheme + "a/runtime@sha256:" + strings.Repeat("0", 64)}, nil) + register(catalogue.Manifest{Module: "e", Version: "1", Build: &catalogue.Build{ + On: []catalogue.BuildsOn{{Arg: "BASE", Module: "b", Artifact: "runtime"}}}}, "novox/two", "e", nil, nil) + register(catalogue.Manifest{Module: "p", Version: "1"}, "novox/two", "p", nil, + []inventory.ReadRepository{{Repository: "novox/one", Ref: "main"}}) + + entries, err := inv.Catalogued(ctx) + if err != nil { + t.Fatal(err) + } + read, err := inv.ReadRepositories(ctx) + if err != nil { + t.Fatal(err) + } + edges, err := inv.Dependencies(ctx) + if err != nil { + t.Fatal(err) + } + + // The edges the hand-written rows of TestASharedRepositoryMovesWhatPackagesItAsItDoesToday use are + // the ones derived here. + var shared []inventory.Edge + in338 := map[string]bool{"mesh-controller": true, "build-agent": true, "route-proxy": true, "gitea": true} + for _, e := range edges { + if in338[e.From] && in338[e.To] { + shared = append(shared, e) + } + } + if !reflect.DeepEqual(shared, sharedRepositoryEdges) { + t.Errorf("derived %v\nthe hand-written rows use %v", shared, sharedRepositoryEdges) + } + // Each kind derived from its record: built against (stands-on), build.on (declared), read (packages). + for _, want := range []inventory.Edge{ + dep("d", inventory.EdgeStandsOn, "a"), + dep("e", inventory.EdgeDeclared, "b"), + dep("p", inventory.EdgePackages, "a"), + dep("p", inventory.EdgePackages, "b"), + dep("p", inventory.EdgePackages, "c"), + dep("d", inventory.EdgeBuiltBy, "build-agent"), + } { + found := false + for _, e := range edges { + found = found || e == want + } + if !found { + t.Errorf("no %s %s %s derived: %v", want.From, want.Kind, want.To, edges) + } + } + + for _, c := range []struct { + what, repo string + paths []string + want string + issue338 bool + }{ + {"A and B changed, C untouched: D after A, E after B; P packages their repository", "one", + []string{"modules/a/x.go", "modules/b/x.go"}, "a,b,p | d,e", false}, + {"C alone: C, and P, which packages C's repository", "one", + []string{"modules/c/x.go"}, "c,p", true}, + {"a README of the repository P packages: P moves, nothing built from it does", "one", + []string{"README.md"}, "p", true}, + {"the dependent's repository: D alone", "two", []string{"d/main.go"}, "d", false}, + {"a README of the controller's repository: all three, three tiers", "mesh-controller", + []string{"README.md"}, "mesh-controller | build-agent | route-proxy", true}, + {"the route proxy's directory in the catalogue: it alone", "mesh-catalog", + []string{"modules/route-proxy/module.json"}, "route-proxy", false}, + {"the build agent's directory: it alone, nothing it builds", "mesh-catalog", + []string{"modules/build-agent/module.json"}, "build-agent", false}, + } { + _, got := planMerge(t, c.repo, c.paths, entries, read, edges) + if got != c.want { + tag := "" + if c.issue338 { + tag = " (current behaviour, issue 338)" + } + t.Errorf("%s: planned %q, wanted %q%s", c.what, got, c.want, tag) + } + } +} diff --git a/cmd/mesh-controller/planner_rules_test.go b/cmd/mesh-controller/planner_rules_test.go new file mode 100644 index 00000000..6b8dd23d --- /dev/null +++ b/cmd/mesh-controller/planner_rules_test.go @@ -0,0 +1,447 @@ +package main + +import ( + "fmt" + "math/rand/v2" + "sort" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// The delivery planner's rules, as recorded (novox/hq ADR 0162 §1, ADR 0238 §3), held by one table and +// one property over reachOfMerge — the planner's one answer to "what does this merge move, and in which +// order". A row that fails here on main is a planner that breaks a recorded rule: the row stays, the +// expectation is not bent to the code. +// +// The kinds of edge, as the code reads them (release_plan.go): +// +// kind widens the plan orders the tiers +// stands-on yes yes, after its base is built +// declared yes yes, after its base is built +// packages yes no, the same tier (a code dependency) +// built-by no yes, after the build machine — except for what the build machine stands +// on, and for the controller whose worker it binds +// worker-of no yes, the build seat's holder after the controller (hq issue 206) + +const ( + repoOne = "http://forge.internal:20000/novox/one.git" + repoTwo = "http://forge.internal:20000/novox/two.git" +) + +// dep is one edge of the catalogue's relation: from depends on to, in the way kind says. +func dep(from, kind, to string) inventory.Edge { + return inventory.Edge{From: from, To: to, Kind: kind} +} + +// tiered is a plan's tiers as one line: a tier's modules by comma, tiers by " | ". Empty for no plan. +func tiered(tiers [][]string) string { + var out []string + for _, t := range tiers { + out = append(out, strings.Join(t, ",")) + } + return strings.Join(out, " | ") +} + +// planMerge is what reachOfMerge plans for a merge of these files into a repository's main: its tiers as +// one line, and the files no build reads. It also holds the plan to its own shape: every module it builds +// is in exactly one tier. +func planMerge(t *testing.T, repo string, paths []string, entries []inventory.Entry, + read map[string][]inventory.ReadRepository, edges []inventory.Edge) (mergeReach, string) { + t.Helper() + m := link.SourceMoved{Owner: "novox", Repo: repo, Base: "main", Commit: "head", Paths: paths} + r := reachOfMerge(m, entries, read, edges) + seen := map[string]int{} + for _, tier := range r.Plan.Tiers { + for _, name := range tier { + seen[name]++ + } + } + for name := range r.Plan.Modules { + if seen[name] != 1 { + t.Errorf("%s/%v: %s is in %d tiers of %v", repo, paths, name, seen[name], r.Plan.Tiers) + } + } + if len(seen) != len(r.Plan.Modules) { + t.Errorf("%s/%v: the tiers %v hold modules the plan does not (%d)", repo, paths, r.Plan.Tiers, len(r.Plan.Modules)) + } + return r, tiered(r.Plan.Tiers) +} + +// **A merge moves the modules whose directory it changed, and everything built on them; nothing else.** +// Each row is a catalogue (modules in directories of one or two repositories), its dependencies with +// their kinds, the files one commit changed, and the exact plan: the moved set and its tier order. +func TestAPlanIsWhatTheChangeTouchedAndWhatIsBuiltOnIt(t *testing.T) { + in := func(repo, dir string, names ...string) []inventory.Entry { + var out []inventory.Entry + for _, n := range names { + d := dir + "/" + n + if dir == "" { + d = n + } + out = append(out, fromRepo(n, repo, d)) + } + return out + } + // Modules a to f, x and z in novox/one under modules/; g in novox/two at g/. + entries := append(in(repoOne, "modules", "a", "b", "c", "d", "e", "f", "x", "z"), in(repoTwo, "", "g")...) + const ( + standsOn = inventory.EdgeStandsOn + declared = inventory.EdgeDeclared + packages = inventory.EdgePackages + builtBy = inventory.EdgeBuiltBy + workerOf = inventory.EdgeWorkerOf + ) + // The two real cycles the kinds resolve themselves (ADR 0162 §1, hq issue 206). + runtime := append(in(repoOne, "modules", "runtime", "builder"), fromRepo("controller", repoTwo, "")) + for _, c := range []struct { + what string + entries []inventory.Entry + edges []inventory.Edge + repo string + paths []string + want string // the tiers, " | " between them + unread string + cycle bool + }{ + // The operator's case: two modules changed, a third beside them in the same repository untouched, + // each changed one with a dependent. + {what: "A and B changed, C untouched beside them, D on A and E on B", + edges: []inventory.Edge{dep("d", standsOn, "a"), dep("e", declared, "b")}, + repo: "one", paths: []string{"modules/a/main.go", "modules/b/module.json"}, want: "a,b | d,e"}, + {what: "only A's directory: A and what stands on it", + edges: []inventory.Edge{dep("d", standsOn, "a"), dep("e", declared, "b")}, + repo: "one", paths: []string{"modules/a/main.go"}, want: "a | d"}, + {what: "only C's directory: C alone, nothing is built on it", + edges: []inventory.Edge{dep("d", standsOn, "a"), dep("e", declared, "b")}, + repo: "one", paths: []string{"modules/c/Dockerfile"}, want: "c"}, + {what: "only the dependent changed: its base does not move", + edges: []inventory.Edge{dep("d", standsOn, "a")}, + repo: "one", paths: []string{"modules/d/main.go"}, want: "d"}, + {what: "transitive: F on D on A, A changed", + edges: []inventory.Edge{dep("f", standsOn, "d"), dep("d", standsOn, "a")}, + repo: "one", paths: []string{"modules/a/x"}, want: "a | d | f"}, + {what: "transitive across kinds: F declared on D, D packages A", + edges: []inventory.Edge{dep("f", declared, "d"), dep("d", packages, "a")}, + repo: "one", paths: []string{"modules/a/x"}, want: "a,d | f"}, + + // Each kind alone: X depends on A, A changed (widening), then both changed (ordering). + {what: "stands-on (built against A's artifact) widens", edges: []inventory.Edge{dep("x", standsOn, "a")}, + repo: "one", paths: []string{"modules/a/x"}, want: "a | x"}, + {what: "stands-on orders", edges: []inventory.Edge{dep("x", standsOn, "a")}, + repo: "one", paths: []string{"modules/a/x", "modules/x/y"}, want: "a | x"}, + {what: "declared (build.on) widens", edges: []inventory.Edge{dep("x", declared, "a")}, + repo: "one", paths: []string{"modules/a/x"}, want: "a | x"}, + {what: "declared orders", edges: []inventory.Edge{dep("x", declared, "a")}, + repo: "one", paths: []string{"modules/a/x", "modules/x/y"}, want: "a | x"}, + {what: "packages widens, into the same tier", edges: []inventory.Edge{dep("x", packages, "a")}, + repo: "one", paths: []string{"modules/a/x"}, want: "a,x"}, + {what: "packages does not order", edges: []inventory.Edge{dep("x", packages, "a")}, + repo: "one", paths: []string{"modules/a/x", "modules/x/y"}, want: "a,x"}, + {what: "built-by never widens", edges: []inventory.Edge{dep("x", builtBy, "a")}, + repo: "one", paths: []string{"modules/a/x"}, want: "a"}, + {what: "built-by orders", edges: []inventory.Edge{dep("x", builtBy, "a")}, + repo: "one", paths: []string{"modules/a/x", "modules/x/y"}, want: "a | x"}, + {what: "worker-of never widens", edges: []inventory.Edge{dep("x", workerOf, "a")}, + repo: "one", paths: []string{"modules/a/x"}, want: "a"}, + {what: "worker-of orders", edges: []inventory.Edge{dep("x", workerOf, "a")}, + repo: "one", paths: []string{"modules/a/x", "modules/x/y"}, want: "a | x"}, + {what: "a change to the base alone does not move what it builds", edges: []inventory.Edge{dep("x", builtBy, "a"), + dep("d", builtBy, "a"), dep("e", standsOn, "a")}, + repo: "one", paths: []string{"modules/a/module.json"}, want: "a | e"}, + + // The cycles the kinds resolve: the build machine stands on the runtime image the runtime image is + // built by; the build seat's holder follows the controller that is built by it. + {what: "the build machine's base comes first, built by the build machine that runs", entries: runtime, + edges: []inventory.Edge{dep("runtime", builtBy, "builder"), dep("builder", standsOn, "runtime")}, + repo: "one", paths: []string{"modules/runtime/Dockerfile", "modules/builder/main.go"}, want: "runtime | builder"}, + {what: "the runtime image alone takes the build machine on it along", entries: runtime, + edges: []inventory.Edge{dep("runtime", builtBy, "builder"), dep("builder", standsOn, "runtime")}, + repo: "one", paths: []string{"modules/runtime/Dockerfile"}, want: "runtime | builder"}, + {what: "the build machine alone moves alone", entries: runtime, + edges: []inventory.Edge{dep("runtime", builtBy, "builder"), dep("builder", standsOn, "runtime")}, + repo: "one", paths: []string{"modules/builder/main.go"}, want: "builder"}, + {what: "the build seat's holder follows the controller it binds the worker of", entries: runtime, + edges: []inventory.Edge{dep("controller", builtBy, "builder"), dep("builder", workerOf, "controller")}, + repo: "two", paths: []string{"cmd/main.go"}, want: "controller"}, + + // Two repositories. + {what: "a dependent in another repository follows its base", + edges: []inventory.Edge{dep("g", standsOn, "a")}, + repo: "one", paths: []string{"modules/a/x"}, want: "a | g"}, + {what: "a directory of the same name in another repository is not this one's", + edges: []inventory.Edge{dep("g", standsOn, "a")}, + repo: "two", paths: []string{"modules/a/x"}, want: "", unread: "modules/a/x"}, + {what: "the dependent's own repository moves the dependent alone", + edges: []inventory.Edge{dep("g", standsOn, "a")}, + repo: "two", paths: []string{"g/main.go"}, want: "g"}, + + // A diamond. + {what: "a diamond, one side changed", edges: []inventory.Edge{dep("d", standsOn, "a"), dep("d", standsOn, "b")}, + repo: "one", paths: []string{"modules/a/x"}, want: "a | d"}, + {what: "a diamond, both sides changed", edges: []inventory.Edge{dep("d", standsOn, "a"), dep("d", standsOn, "b")}, + repo: "one", paths: []string{"modules/a/x", "modules/b/x"}, want: "a,b | d"}, + {what: "a diamond on one base", edges: []inventory.Edge{dep("d", standsOn, "a"), dep("d", declared, "b"), + dep("a", standsOn, "z"), dep("b", standsOn, "z")}, + repo: "one", paths: []string{"modules/z/x"}, want: "z | a,b | d"}, + {what: "a diamond of mixed kinds orders on the ordering side only", + edges: []inventory.Edge{dep("d", standsOn, "a"), dep("d", packages, "b")}, + repo: "one", paths: []string{"modules/b/x"}, want: "b,d"}, + + // A cycle the catalogue should never produce: what remains is one last tier, and said. + {what: "a cycle is one last tier, not lost", edges: []inventory.Edge{dep("a", standsOn, "b"), dep("b", standsOn, "a"), + dep("f", standsOn, "c")}, + repo: "one", paths: []string{"modules/a/x", "modules/c/x"}, want: "c | f | a,b", cycle: true}, + + // Files no build reads. + {what: "a README at the root of a repository whose modules all live below it", + edges: []inventory.Edge{dep("d", standsOn, "a")}, + repo: "one", paths: []string{"README.md"}, want: "", unread: "README.md"}, + {what: "a directory no module lives in", edges: []inventory.Edge{dep("d", standsOn, "a")}, + repo: "one", paths: []string{"modules/lib/x.go", "modules/README.md"}, want: "", + unread: "modules/lib/x.go,modules/README.md"}, + {what: "a module's directory beside a root file", edges: []inventory.Edge{dep("d", standsOn, "a")}, + repo: "one", paths: []string{"merge-check.sh", "modules/a/x"}, want: "a | d", unread: "merge-check.sh"}, + {what: "a directory whose name begins with a module's", edges: []inventory.Edge{dep("d", standsOn, "a")}, + repo: "one", paths: []string{"modules/ab/x"}, want: "", unread: "modules/ab/x"}, + } { + e := entries + if c.entries != nil { + e = c.entries + } + r, got := planMerge(t, c.repo, c.paths, e, nil, c.edges) + if got != c.want { + t.Errorf("%s: planned %q, wanted %q", c.what, got, c.want) + } + if u := strings.Join(r.Unread, ","); u != c.unread { + t.Errorf("%s: unread %q, wanted %q", c.what, u, c.unread) + } + // A packages edge in the last tier is no cycle; hasCycle said one on main at 8170fc5. + if hasCycle(r.Plan.Tiers, c.edges) != c.cycle { + t.Errorf("%s: a cycle said %v, wanted %v (%v)", c.what, !c.cycle, c.cycle, r.Plan.Tiers) + } + } +} + +// **CURRENT BEHAVIOUR, documented — not the rule the operator states.** novox/hq issue 338 (a module +// built from a shared repository moves on every merge to it) and the decision pending on it would change +// every row here. Today: +// +// - mesh-controller is built from its repository's root, so every file of that repository touches it; +// - route-proxy and build-agent package the whole of that repository (a build context), so the build +// record's `read` makes them move on any merge to it, whatever the files, and dependenciesOf gives +// each a packages edge to every module built from it; +// - built-by (route-proxy on build-agent) and worker-of (build-agent on the controller) make it three +// tiers. +// +// These follow ADR 0238 §3 as written ("the whole repository for a module built from its root, and a +// repository a recipe names"), so they are not failures; when the decision on issue 338 lands, these +// expectations change with it. The edges are the ones dependenciesOf derives from this catalogue — held +// to that by TestASharedRepositoryIsPlannedFromTheRecordsAsItIsToday, which derives them from the store. +func TestASharedRepositoryMovesWhatPackagesItAsItDoesToday(t *testing.T) { + const catalogueRepo = "http://forge.internal:20000/novox/mesh-catalog.git" + const controllerRepo = "http://forge.internal:20000/novox/mesh-controller.git" + entries := []inventory.Entry{ + fromRepo("mesh-controller", controllerRepo, ""), + fromRepo("build-agent", catalogueRepo, "modules/build-agent"), + fromRepo("route-proxy", catalogueRepo, "modules/route-proxy"), + fromRepo("gitea", catalogueRepo, "modules/gitea"), + } + read := map[string][]inventory.ReadRepository{ + "build-agent": {{Repository: "novox/mesh-controller", Ref: "main"}}, + "route-proxy": {{Repository: "novox/mesh-controller", Ref: "main"}}, + } + edges := sharedRepositoryEdges + for _, c := range []struct { + what, repo string + paths []string + want string + }{ + // The live three-tier plan of 2026-10-08 (issue 338), in the worker-of order (issue 206) that + // TestAMergeIsPlannedInTiersAlongTheThreeKindsOfDependency's controller case holds too. + {"a README of the controller's repository moves all three, in three tiers", "mesh-controller", + []string{"README.md"}, "mesh-controller | build-agent | route-proxy"}, + {"the controller's own code: the same", "mesh-controller", + []string{"cmd/mesh-controller/main.go"}, "mesh-controller | build-agent | route-proxy"}, + {"the route proxy's program alone: the same, the controller with it", "mesh-controller", + []string{"examples/route-proxy/main.go"}, "mesh-controller | build-agent | route-proxy"}, + // In the catalogue, where they live, the rule is path-precise. + {"the route proxy's directory in the catalogue: it alone", "mesh-catalog", + []string{"modules/route-proxy/module.json"}, "route-proxy"}, + {"the build agent's directory: it alone, nothing it builds", "mesh-catalog", + []string{"modules/build-agent/module.json"}, "build-agent"}, + {"another module of the catalogue: neither", "mesh-catalog", + []string{"modules/gitea/index.ts"}, "gitea"}, + } { + r, got := planMerge(t, c.repo, c.paths, entries, read, edges) + if got != c.want { + t.Errorf("%s: planned %q, wanted %q (as today; issue 338)", c.what, got, c.want) + } + if c.repo == "mesh-controller" && strings.Join(r.Unread, ",") != "" { + t.Errorf("%s: a root-built module reads every file, and %v were said unread", c.what, r.Unread) + } + } +} + +// sharedRepositoryEdges is what dependenciesOf derives for the catalogue of the test above, sorted as it +// sorts them. +var sharedRepositoryEdges = []inventory.Edge{ + dep("build-agent", inventory.EdgePackages, "mesh-controller"), + dep("build-agent", inventory.EdgeWorkerOf, "mesh-controller"), + dep("gitea", inventory.EdgeBuiltBy, "build-agent"), + dep("mesh-controller", inventory.EdgeBuiltBy, "build-agent"), + dep("route-proxy", inventory.EdgeBuiltBy, "build-agent"), + dep("route-proxy", inventory.EdgePackages, "mesh-controller"), +} + +// **The planner's invariant, over random catalogues.** For any catalogue whose dependencies form no cycle +// and any set of changed files in one repository: +// +// - the plan is exactly the modules of that repository whose directory holds a changed file (every file, +// for a module built from the root), and everything reachable from them along stands-on, declared and +// packages — never along built-by or worker-of; +// - every stands-on, declared, built-by and worker-of edge with both ends in the plan has the module +// depended on in an earlier tier; +// - no cycle is said. +// +// Seeded, so a failure is replayed by its seed and case. +func TestAPlanIsTheTouchedModulesAndWhatIsReachableAlongTheWideningEdges(t *testing.T) { + kinds := []string{inventory.EdgeStandsOn, inventory.EdgeDeclared, inventory.EdgePackages, + inventory.EdgeBuiltBy, inventory.EdgeWorkerOf} + widens := map[string]bool{inventory.EdgeStandsOn: true, inventory.EdgeDeclared: true, inventory.EdgePackages: true} + orders := map[string]bool{inventory.EdgeStandsOn: true, inventory.EdgeDeclared: true, + inventory.EdgeBuiltBy: true, inventory.EdgeWorkerOf: true} + // Directory names drawn from one pool, so two repositories hold directories of the same name, and one + // is a prefix of another. + dirs := []string{"a", "ab", "b", "c", "lib/x", "lib/y", "modules/a", "modules/a/sub"} + files := []string{"README.md", "merge-check.sh", "lib/z.go", "docs/x.md", "modules/README.md"} + + falseCycles, firstFalseCycle := 0, "" + for _, seed := range []uint64{1, 2, 3, 0x338, 0x162} { + rng := rand.New(rand.NewPCG(seed, seed^0x9e3779b97f4a7c15)) + for n := 0; n < 100; n++ { + repos := 1 + rng.IntN(3) + repoName := func(i int) string { return fmt.Sprintf("r%d", i) } + count := 1 + rng.IntN(12) + var entries []inventory.Entry + repoOf, dirOf := map[string]int{}, map[string]string{} + for i := 0; i < count; i++ { + name := fmt.Sprintf("m%02d", i) + repo := rng.IntN(repos) + dir := dirs[rng.IntN(len(dirs))] + if rng.IntN(12) == 0 { + dir = "" // built from the repository's root + } + repoOf[name], dirOf[name] = repo, dir + entries = append(entries, fromRepo(name, "http://forge.internal:20000/novox/"+repoName(repo)+".git", dir)) + } + // A graph with no cycle: a module depends only on modules made before it. + var edges []inventory.Edge + for i := 1; i < count; i++ { + for j := 0; j < i; j++ { + if rng.IntN(4) == 0 { + edges = append(edges, dep(fmt.Sprintf("m%02d", i), kinds[rng.IntN(len(kinds))], fmt.Sprintf("m%02d", j))) + } + } + } + merged := rng.IntN(repos) + var paths []string + for k := 1 + rng.IntN(4); k > 0; k-- { + if rng.IntN(3) == 0 { + paths = append(paths, files[rng.IntN(len(files))]) + } else { + paths = append(paths, dirs[rng.IntN(len(dirs))]+"/f.go") + } + } + + // What the rules say. + want := map[string]bool{} + for _, e := range entries { + name := e.Manifest.Module + if repoOf[name] != merged { + continue + } + for _, p := range paths { + if d := dirOf[name]; d == "" || p == d || strings.HasPrefix(p, d+"/") { + want[name] = true + } + } + } + for grew := true; grew; { + grew = false + for _, e := range edges { + if widens[e.Kind] && want[e.To] && !want[e.From] { + want[e.From], grew = true, true + } + } + } + + r, _ := planMerge(t, repoName(merged), paths, entries, nil, edges) + got := map[string]bool{} + tierOf := map[string]int{} + for i, tier := range r.Plan.Tiers { + for _, name := range tier { + got[name], tierOf[name] = true, i + } + } + replay := func() string { + return fmt.Sprintf("seed %#x case %d: repository %s, files %v\n modules %v\n edges %v\n tiers %v", + seed, n, repoName(merged), paths, describe(entries), edges, r.Plan.Tiers) + } + if !sameSet(got, want) { + t.Fatalf("planned %v, wanted %v\n%s", keys(got), keys(want), replay()) + } + for _, e := range edges { + if orders[e.Kind] && got[e.From] && got[e.To] && tierOf[e.To] >= tierOf[e.From] { + t.Fatalf("%s %s %s, and %s is in tier %d, not before %s's %d\n%s", e.From, e.Kind, e.To, + e.To, tierOf[e.To], e.From, tierOf[e.From], replay()) + } + } + if hasCycle(r.Plan.Tiers, edges) { + falseCycles++ + if firstFalseCycle == "" { + firstFalseCycle = replay() + } + } + } + } + // Said once, after the other invariants have run over every case, so it hides none of them (hasCycle + // counted a packages edge on main at 8170fc5: 19 of these 500 cases). + if falseCycles > 0 { + t.Errorf("a cycle said of a graph with none in %d of 500 cases; "+ + "the first:\n%s", falseCycles, firstFalseCycle) + } +} + +func sameSet(a, b map[string]bool) bool { + if len(a) != len(b) { + return false + } + for k := range a { + if !b[k] { + return false + } + } + return true +} + +func keys(m map[string]bool) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + sort.Strings(out) + return out +} + +func describe(entries []inventory.Entry) []string { + var out []string + for _, e := range entries { + out = append(out, fmt.Sprintf("%s@%s:%q", e.Manifest.Module, + strings.TrimSuffix(strings.TrimPrefix(e.Source.Repository, "http://forge.internal:20000/novox/"), ".git"), + e.Source.Path)) + } + return out +} diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index c240d478..8688705b 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -157,7 +157,9 @@ func reachableFrom(moved []string, edges []inventory.Edge) []string { return out } -// hasCycle says whether the tiers' last tier holds modules that still depend on each other. +// hasCycle says whether the tiers' last tier holds modules that still depend on each other. A packages +// edge orders nothing (tiersOf), so a module and what packages its source share a tier by rule: that is +// no cycle, and saying one was is a false report in every such plan's log. func hasCycle(tiers [][]string, edges []inventory.Edge) bool { if len(tiers) == 0 { return false @@ -167,6 +169,9 @@ func hasCycle(tiers [][]string, edges []inventory.Edge) bool { last[m] = true } for _, e := range edges { + if e.Kind == inventory.EdgePackages { + continue + } if last[e.From] && last[e.To] { return true } diff --git a/cmd/mesh-controller/release_plan_test.go b/cmd/mesh-controller/release_plan_test.go index 066c6023..118a495b 100644 --- a/cmd/mesh-controller/release_plan_test.go +++ b/cmd/mesh-controller/release_plan_test.go @@ -27,6 +27,8 @@ func TestAMergeIsPlannedInTiersAlongTheThreeKindsOfDependency(t *testing.T) { {From: "mesh-controller", To: "builder", Kind: inventory.EdgeBuiltBy}, {From: "mesh-tools", To: "builder", Kind: inventory.EdgeBuiltBy}, {From: "builder", To: "mesh-tools", Kind: inventory.EdgeStandsOn}, + // the build seat's holder follows the controller that defines its worker (hq issue 206) + {From: "builder", To: "mesh-controller", Kind: inventory.EdgeWorkerOf}, {From: "unrelated", To: "alpine", Kind: inventory.EdgeStandsOn}, } // The runtime image moved: everything on it, and what is built by what is on it. @@ -62,12 +64,15 @@ func TestAMergeIsPlannedInTiersAlongTheThreeKindsOfDependency(t *testing.T) { if len(small) != 3 { t.Fatalf("a controller merge rebuilds the controller and what packages it: %v", small) } - // The builder packages the controller's source (same tier by that edge) and the controller is - // built by the builder (next tier by that one): the builder first, then the controller and the - // proxy together — a code dependency in one tier, a runtime dependency across tiers. + // The builder and the proxy package the controller's source, which orders nothing. The builder holds + // the build seat, whose worker the controller defines, so it follows the controller (worker-of, + // novox/hq issue 206), and the controller's built-by edge to it yields: the controller is built by the + // build machine that is running. The proxy is built by the new builder: the controller, the builder, + // the proxy — the live plan of every controller merge. (This read "the builder, then the controller + // and the proxy together" before issue 206, and the fixture had no worker-of edge.) smallTiers := tiersOf(small, edges) - if len(smallTiers) != 2 || smallTiers[0][0] != "builder" || len(smallTiers[1]) != 2 { - t.Fatalf("the builder, then the controller and the proxy together: %v", smallTiers) + if got := tiered(smallTiers); got != "mesh-controller | builder | route-proxy" { + t.Fatalf("the controller, then the builder, then the proxy: %v", smallTiers) } // The builder alone moved: the builder, and nothing it builds. if only := reachableFrom([]string{"builder"}, edges); len(only) != 1 {