diff --git a/cmd/mesh-controller/batches.go b/cmd/mesh-controller/batches.go index 363ca36d..2399cad3 100644 --- a/cmd/mesh-controller/batches.go +++ b/cmd/mesh-controller/batches.go @@ -1002,7 +1002,7 @@ func planBatch(ctx context.Context, open *stores, batch *inventory.Plan, carry [ plan.Delivery.Withheld = withheld // **Left where it is, a module whose program did not change** (novox/hq issue 721): its check found it the same as // the build the mesh holds. - for n, why := range leaveSameAsRunning(ctx, inv, &plan) { + for n, why := range leaveSameAsRunning(ctx, inv, &plan, moved, append(append([]inventory.Edge{}, edges...), ordered...)) { unchanged[n] = why } plan.Delivery.Unchanged = unchangedLeft(unchanged, plan.Modules) diff --git a/cmd/mesh-controller/same_program.go b/cmd/mesh-controller/same_program.go index c4161a76..20645e74 100644 --- a/cmd/mesh-controller/same_program.go +++ b/cmd/mesh-controller/same_program.go @@ -3,6 +3,7 @@ package main import ( "context" "fmt" + "slices" "sort" "strings" @@ -90,8 +91,9 @@ func pinnedDigest(ref string) string { } // leaveSameAsRunning takes from a walk being cut every module its checks found the same as the build the mesh holds -// (sameAsRunning): what was taken, with why. **Anything not read leaves the walk as it was**, said. -func leaveSameAsRunning(ctx context.Context, inv *inventory.Inventory, plan *inventory.Plan) map[string]string { +// (sameAsRunning), and every dependent in the walk only for one of those (withDependentsOf): what was taken, with why. **Anything not read leaves the walk as it was**, said. +func leaveSameAsRunning(ctx context.Context, inv *inventory.Inventory, plan *inventory.Plan, moved []string, + edges []inventory.Edge) map[string]string { if plan.Delivery == nil || len(plan.Modules) == 0 { return nil } @@ -111,7 +113,7 @@ func leaveSameAsRunning(ctx context.Context, inv *inventory.Inventory, plan *inv // empty-on-error: said above; nothing left out of the walk is the safe answer, every module moves as before return nil } - same := sameAsRunning(plan, builds, held) + same := withDependentsOf(sameAsRunning(plan, builds, held), plan, moved, edges) if len(same) == 0 { return nil } @@ -126,3 +128,49 @@ func leaveSameAsRunning(ctx context.Context, inv *inventory.Inventory, plan *inv } return same } + +// withDependentsOf adds to the modules found the same every module of the walk that is in it only as a dependent of +// one of them (novox/hq ADR 0267 rule 3): **a dependent is left out when its base comes out the same**, since the +// base it stands on stays the build the mesh holds. A module a merge moves itself, or a dependent of a base that +// still moves, stays. moved are the modules the walk's merges move; edges the graph the walk was planned along. +func withDependentsOf(same map[string]string, plan *inventory.Plan, moved []string, edges []inventory.Edge) map[string]string { + if len(same) == 0 { + return same + } + var still []string + for _, n := range moved { + if same[n] == "" { + still = append(still, n) + } + } + keep := map[string]bool{} + for _, n := range reachableFrom(still, edges) { + keep[n] = true + } + // Until nothing more is found: a dependent of a dependent left out is left out too. + for grew := true; grew; { + grew = false + for n := range plan.Modules { + if keep[n] || same[n] != "" { + continue + } + var bases []string + for _, e := range edges { + ordering := e.Kind == inventory.EdgeBuiltBy || e.Kind == inventory.EdgeWorkerOf || + e.Kind == inventory.EdgePackages || e.Kind == edgeGroupOrder + if !ordering && e.From == n && same[e.To] != "" && !slices.Contains(bases, e.To) { + bases = append(bases, e.To) + } + } + sort.Strings(bases) + if len(bases) == 0 { + // In the walk for no base found the same: whatever brought it in is not said here, so it moves. + continue + } + same[n] = "unchanged: it is in the walk only for " + strings.Join(bases, ", ") + + ", which came out the same as the build the mesh holds (ADR 0267 rule 3)" + grew = true + } + } + return same +} diff --git a/cmd/mesh-controller/same_program_test.go b/cmd/mesh-controller/same_program_test.go index 9aeedb7b..a7c91932 100644 --- a/cmd/mesh-controller/same_program_test.go +++ b/cmd/mesh-controller/same_program_test.go @@ -1,7 +1,10 @@ package main import ( + "context" + "strings" "testing" + "time" "github.com/novox/mesh-controller/internal/inventory" "github.com/novox/mesh-controller/internal/link" @@ -65,3 +68,67 @@ func TestAWalkLeavesOutAModuleWhoseProgramDidNotChange(t *testing.T) { t.Fatalf("a walk answering a merge without a pull request left out %v", same) } } + +// TestADependentOfABaseThatCameOutTheSameIsLeftOut is ADR 0267 rule 3 (novox/hq issue 607): a dependent is in a walk +// for its base; when the base comes out the same as the build the mesh holds, the dependent stands on that build +// still, and leaves the walk with it — and so does a dependent of that dependent. One a merge moves itself stays. +func TestADependentOfABaseThatCameOutTheSameIsLeftOut(t *testing.T) { + edges := []inventory.Edge{{From: "app", To: "base", Kind: inventory.EdgeDeclared}, + {From: "app2", To: "app", Kind: inventory.EdgeDeclared}, {From: "own", To: "base", Kind: inventory.EdgeDeclared}} + plan := &inventory.Plan{Modules: map[string]*inventory.PlanModule{"base": {}, "app": {}, "app2": {}, "own": {}}} + same := withDependentsOf(map[string]string{"base": "unchanged: its program is the same"}, plan, + []string{"base", "own"}, edges) + if !strings.Contains(same["app"], "only for base") || !strings.Contains(same["app2"], "only for app") { + t.Fatalf("the dependents of a base that came out the same are not left out: %v", same) + } + if _, left := same["own"]; left { + t.Fatalf("a module its merge moves itself is left out for its base: %v", same) + } + // A dependent of a base that still moves stays. + edges = append(edges, inventory.Edge{From: "app", To: "own", Kind: inventory.EdgeDeclared}) + same = withDependentsOf(map[string]string{"base": "unchanged"}, plan, []string{"base", "own"}, edges) + if _, left := same["app"]; left { + t.Fatalf("a dependent of a base that still moves is left out: %v", same) + } +} + +// Against the store, the path a real merge takes (novox/hq issues 721 and 611): a merge whose check built its one +// module the same as the build the mesh holds is cut into a walk done with nothing to walk, whose record names the +// module unchanged and why — the walk mesh-delivery reads as delivered, never stranded. +func TestAWalkWhoseEveryBuildCameOutTheSameIsDone(t *testing.T) { + open := windowed(t) + endsItsLine(t) + asksRecorded(t) + ctx := t.Context() + inv := open.inventory + if err := inv.RecordBuild(ctx, inventory.Build{ID: "build-one", Repository: "novox/one", Ref: "main", + Module: "one", Commit: "c0", On: "builder", Asked: t0.Add(-time.Hour), + Made: []inventory.Artifact{{Name: "one", Kind: "image", Reference: "store/one@sha256:aaaa"}}, + Sources: []inventory.BuildSource{{Paths: []string{"main.go", "go.mod"}}}}); err != nil { + t.Fatal(err) + } + pull := link.PullRef{Owner: "novox", Repo: "one", Number: 7} + was := checkedBuildsOf + checkedBuildsOf = func(context.Context, []link.PullRef) ([]link.CheckedBuild, error) { + return []link.CheckedBuild{{Pull: pull, Head: "headaaaaaaaa", Built: link.CheckBuilt{Module: "one", Key: "k", + Made: []link.MadeArtifact{{Name: "one", Kind: "image", Reference: "scratch/one@sha256:bbbb"}}, + Same: "unchanged: its program is the same as running build one aaaa", + SameAs: map[string]string{"one": "10.0.0.1:5000/one@sha256:aaaa"}}}}, nil + } + t.Cleanup(func() { checkedBuildsOf = was }) + code := repoMerge("one", "one1aaaaaaaa", t0) + code.Number, code.HeadSHA = 7, "headaaaaaaaa1111" + hear(t, open, code, t0.Add(time.Second)) + cutAt(t, open, t0.Add(2*time.Minute)) + ws, _ := walks(t, open) + if len(ws) != 1 { + t.Fatalf("walks: %+v", ws) + } + w := ws[0] + if w.State != inventory.PlanDone || len(w.Tiers) != 0 || len(w.Modules) != 0 { + t.Fatalf("the walk reads %s, tiers %v, modules %v; want done with nothing to walk", w.State, w.Tiers, w.Modules) + } + if w.Delivery == nil || !strings.Contains(w.Delivery.Unchanged["one"], "its program is the same as running build") { + t.Fatalf("the walk does not say one matched what runs: %+v", w.Delivery) + } +}