Merge pull request 'A module moves only when its program changes (hq issue 721)' (#336) from fix/a-module-moves-only-when-its-program-changes into main

cmd/mesh-builder/main.go

@@ -329,7 +329,7 @@ func answer(ctx context.Context, publisher builder.Publisher, on, workspace stri
 		// at build time has no reuse key anyway, so nothing is lost by refusing it here.
 		if isRegistry && len(spec.Builds) > 0 {
 			spec.BuildWith = &builder.CheckBuilding{Publish: publisher, Scratch: registryHere, Held: request.Held,
-				Seats: request.Seats, Named: namedOf(request.Check.Named)}
+				Running: request.Check.Running, Seats: request.Seats, Named: namedOf(request.Check.Named)}
 		}
 		var v builder.CheckVerdict
 		v, err = builder.Check(building, builder.Command, spec, workspace, registry, forgeFrom(), say)
@@ -525,7 +525,7 @@ func checkBuiltOf(built []builder.CheckBuilt) []link.CheckBuilt {
 	var out []link.CheckBuilt
 	for _, b := range built {
 		c := link.CheckBuilt{Module: b.Module, Key: b.Key, NotBuilt: b.NotBuilt, Took: b.Took.Round(time.Second).String(),
-			Kept: b.Kept}
+			Kept: b.Kept, Same: b.Same, SameAs: b.SameAs}
 		for _, m := range b.Made {
 			c.Made = append(c.Made, link.MadeArtifact{Name: m.Name, Kind: m.Kind, Reference: m.Reference})
 		}

cmd/mesh-controller/batches.go

@@ -1000,10 +1000,15 @@ func planBatch(ctx context.Context, open *stores, batch *inventory.Plan, carry [
 	plan.Delivery.Merges = named
 	plan.Delivery.Alone = batch.Delivery != nil && batch.Delivery.Alone
 	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, moved, append(append([]inventory.Edge{}, edges...), ordered...)) {
+		unchanged[n] = why
+	}
 	plan.Delivery.Unchanged = unchangedLeft(unchanged, plan.Modules)
 	// **Its moments and its class** (novox/hq ADR 0282 decision 6): measured, never acted on.
 	plan.Times = walkTimesAtCut(*batch, plan, entries, now)
-	if len(moved) == 0 {
+	if len(moved) == 0 || len(plan.Modules) == 0 {
 		plan.State = inventory.PlanDone
 		plan.Tiers = [][]string{}
 		plan.Note = "moved nothing the mesh holds: nothing to walk"

cmd/mesh-controller/build.go

@@ -899,6 +899,34 @@ type answers struct {
 //
 // Routed through the artifact store as the network reaches it now (novox/hq 04-ISSUES/102): a
 // base is recorded by digest and path, and a build machine needs something it can pull.
+// runningBy is, keyed "<module>/<artifact>", each artifact of the build the mesh has registered for its module, as a
+// builder reaches the artifact store (novox/hq issue 721): what a check compares its builds with. Empty when it cannot
+// be read, said, and then nothing is said unchanged.
+func runningBy(ctx context.Context) map[string]string {
+	open, err := openStores(ctx)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "could not read what this mesh runs, so the check compares nothing: %v\n", err)
+		// empty-on-error: said above; a check given nothing to compare with says no module unchanged, and all move
+		return nil
+	}
+	defer open.Close()
+	running, err := open.inventory.RegisteredHeld(ctx)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "could not read what this mesh runs, so the check compares nothing: %v\n", err)
+		// empty-on-error: said above; a check given nothing to compare with says no module unchanged, and all move
+		return nil
+	}
+	address, err := whereABuilderReachesTheStore(ctx, open.inventory)
+	if err != nil || address == "" {
+		return running
+	}
+	routed := make(map[string]string, len(running))
+	for key, reference := range running {
+		routed[key] = catalogue.Rerouted(reference, address)
+	}
+	return routed
+}
+
 func heldBy(ctx context.Context) map[string]string {
 	open, err := openStores(ctx)
 	if err != nil {

cmd/mesh-controller/checks.go

@@ -269,7 +269,7 @@ func checkRequestFor(ctx context.Context, open *stores, p link.PullUpdated, scop
 		Check: &link.CheckRequest{Owner: p.Owner, Repo: p.Repo, Number: p.Number, Base: p.Base,
 			Paths: p.Paths, Beside: beside, Modules: scope.Modules, Dependents: scope.Dependents, New: scope.New,
 			Manifests: scope.Manifests, Judge: scope.Judge, Builds: builds, Named: namedForTheCheck(ctx, p, builds),
-			Release: release},
+			Release: release, Running: runningBy(ctx)},
 	}, nil
 }
 

cmd/mesh-controller/same_program.go

@@ -0,0 +1,176 @@
+package main
+
+import (
+	"context"
+	"fmt"
+	"slices"
+	"sort"
+	"strings"
+
+	"github.com/novox/mesh-controller/internal/inventory"
+	"github.com/novox/mesh-controller/internal/link"
+)
+
+// **A module moves only when its program changes** (novox/hq issue 721). A merge check builds every module the
+// delivery plan moves and compares each with the build the mesh holds of it; a walk is cut without a module whose
+// check found its whole image the same as that build, while the mesh still holds it, and says so among the modules
+// it left unchanged. A merge of one sentence in a package a program imports but never reaches moves no build of it.
+
+// sameAsRunning are the modules of a walk its checks found the same as the build the mesh holds, each with what its
+// check said. **Only when every pull request the walk answers was checked at the head it merged, and every one of
+// those checks built the module and found it the same as the build the mesh still holds**: a merge the walk answers
+// without a pull request, a check of another head, a check that built it differently, or a held build that moved
+// since, leaves the module in the walk.
+func sameAsRunning(plan *inventory.Plan, builds []link.CheckedBuild, held map[string]string) map[string]string {
+	if plan.Delivery == nil || len(plan.Delivery.Merges) == 0 {
+		return nil
+	}
+	for _, m := range plan.Delivery.Merges {
+		if _, ok := pullOf(m); !ok {
+			return nil
+		}
+	}
+	pulls := reuseAskOf(plan).Pulls
+	heads := mergedHeadsOf(plan)
+	out := map[string]string{}
+	for name := range plan.Modules {
+		said := ""
+		for _, pull := range pulls {
+			b, ok := checkedBuildOf(builds, pull, heads[pull], name)
+			if !ok || !stillHeld(b.Built, name, held) {
+				said = ""
+				break
+			}
+			if said == "" {
+				said = b.Built.Same
+			}
+		}
+		if said != "" {
+			out[name] = said
+		}
+	}
+	if len(out) == 0 {
+		return nil
+	}
+	return out
+}
+
+// checkedBuildOf is the newest build a check of the pull request at head made of the module; false when none did.
+func checkedBuildOf(builds []link.CheckedBuild, pull link.PullRef, head, module string) (link.CheckedBuild, bool) {
+	if head == "" {
+		return link.CheckedBuild{}, false
+	}
+	for _, b := range builds {
+		if b.Pull == pull && b.Built.Module == module && link.SameHead(b.Head, head) {
+			return b, true
+		}
+	}
+	return link.CheckedBuild{}, false
+}
+
+// stillHeld is whether a check found every artifact of the module the same as a build the mesh still holds.
+func stillHeld(b link.CheckBuilt, module string, held map[string]string) bool {
+	if b.Same == "" || b.NotBuilt != "" || len(b.Made) == 0 || len(b.SameAs) != len(b.Made) {
+		return false
+	}
+	for _, m := range b.Made {
+		was := pinnedDigest(b.SameAs[m.Name])
+		if was == "" || was != pinnedDigest(held[module+"/"+m.Name]) {
+			return false
+		}
+	}
+	return true
+}
+
+// pinnedDigest is a reference's pinned digest, empty when it is not pinned.
+func pinnedDigest(ref string) string {
+	if i := strings.LastIndex(ref, "@"); i >= 0 {
+		return ref[i+1:]
+	}
+	return ""
+}
+
+// leaveSameAsRunning takes from a walk being cut every module its checks found the same as the build the mesh holds
+// (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
+	}
+	pulls := reuseAskOf(plan).Pulls
+	if len(pulls) == 0 {
+		return nil
+	}
+	builds, err := checkedBuildsOf(ctx, pulls)
+	if err != nil {
+		fmt.Printf("  the checks' builds could not be read, so every module moves: %v\n", err)
+		// empty-on-error: said above; nothing left out of the walk is the safe answer, every module moves as before
+		return nil
+	}
+	held, err := inv.RegisteredHeld(ctx)
+	if err != nil {
+		fmt.Printf("  what the mesh holds could not be read, so every module moves: %v\n", err)
+		// empty-on-error: said above; nothing left out of the walk is the safe answer, every module moves as before
+		return nil
+	}
+	same := withDependentsOf(sameAsRunning(plan, builds, held), plan, moved, edges)
+	if len(same) == 0 {
+		return nil
+	}
+	names := make([]string, 0, len(same))
+	for n := range same {
+		names = append(names, n)
+	}
+	sort.Strings(names)
+	withhold(plan, names, nil)
+	for _, n := range names {
+		fmt.Printf("  %s left out: %s\n", n, same[n])
+	}
+	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
+}

cmd/mesh-controller/same_program_test.go

@@ -0,0 +1,141 @@
+package main
+
+import (
+	"context"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/novox/mesh-controller/internal/catalogue"
+	"github.com/novox/mesh-controller/internal/inventory"
+	"github.com/novox/mesh-controller/internal/link"
+)
+
+// TestAWalkLeavesOutAModuleWhoseProgramDidNotChange is novox/hq issue 721: mesh-controller #335 changed one sentence
+// that mesh-builder never reaches; its check found build-agent the same as the build the mesh holds, so the walk moves
+// mesh-controller alone and says build-agent is unchanged.
+func TestAWalkLeavesOutAModuleWhoseProgramDidNotChange(t *testing.T) {
+	pull := link.PullRef{Owner: "novox", Repo: "mesh-controller", Number: 335}
+	head := "34050dc6aaaabbbbccccdddd"
+	aPlan := func() *inventory.Plan {
+		return &inventory.Plan{Tiers: [][]string{{"mesh-controller", "build-agent"}},
+			Modules: map[string]*inventory.PlanModule{"mesh-controller": {}, "build-agent": {}},
+			Delivery: &inventory.PlanDelivery{Merges: []inventory.PlanMerge{{Repository: "novox/mesh-controller",
+				Commit: "c2ec0a60", Number: 335, Head: head}}}}
+	}
+	held := map[string]string{"build-agent/agent": "store/build-agent@sha256:aaaa",
+		"mesh-controller/controller": "store/mesh-controller@sha256:cccc"}
+	builds := []link.CheckedBuild{
+		{Pull: pull, Head: head[:12], Built: link.CheckBuilt{Module: "build-agent", Key: "k1",
+			Made:   []link.MadeArtifact{{Name: "agent", Kind: "image", Reference: "scratch/build-agent@sha256:bbbb"}},
+			Same:   "unchanged: its program is the same as running build agent aaaa",
+			SameAs: map[string]string{"agent": "10.0.0.1:5000/build-agent@sha256:aaaa"}}},
+		{Pull: pull, Head: head[:12], Built: link.CheckBuilt{Module: "mesh-controller", Key: "k2",
+			Made: []link.MadeArtifact{{Name: "controller", Kind: "image", Reference: "scratch/mesh-controller@sha256:dddd"}}}},
+	}
+
+	p := aPlan()
+	same := sameAsRunning(p, builds, held)
+	if len(same) != 1 || same["build-agent"] == "" {
+		t.Fatalf("the walk leaves out %v, want build-agent alone", same)
+	}
+	withhold(p, []string{"build-agent"}, nil)
+	if len(p.Tiers) != 1 || len(p.Tiers[0]) != 1 || p.Tiers[0][0] != "mesh-controller" {
+		t.Fatalf("the walk moves %v, want mesh-controller alone", p.Tiers)
+	}
+
+	// **The held build moved since the check**: what it was compared with no longer runs.
+	moved := map[string]string{"build-agent/agent": "store/build-agent@sha256:eeee"}
+	if same := sameAsRunning(aPlan(), builds, moved); len(same) != 0 {
+		t.Fatalf("a module compared with a build the mesh no longer holds is left out: %v", same)
+	}
+	// **A check of another head** says nothing of this merge.
+	other := aPlan()
+	other.Delivery.Merges[0].Head = "ffffffffffffffff"
+	if same := sameAsRunning(other, builds, held); len(same) != 0 {
+		t.Fatalf("a check of another head left out %v", same)
+	}
+	// **Another pull request the walk answers** that did not find it the same leaves it in.
+	two := aPlan()
+	two.Delivery.Merges = append(two.Delivery.Merges, inventory.PlanMerge{Repository: "novox/mesh-catalog",
+		Commit: "1234", Number: 9, Head: "abcdefabcdefabcdef"})
+	if same := sameAsRunning(two, builds, held); len(same) != 0 {
+		t.Fatalf("a module another merge moves is left out: %v", same)
+	}
+	// **A merge with no pull request** leaves the walk as it is.
+	direct := aPlan()
+	direct.Delivery.Merges[0].Number = 0
+	if same := sameAsRunning(direct, builds, held); len(same) != 0 {
+		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)
+	}
+	// What runs: the module registered with that build's image (RegisteredHeld reads the module table).
+	if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "one", Version: "1", Resources: []map[string]any{
+		{"id": "app", "type": "container", "name": "one", "image": "store/one@sha256:aaaa"}}},
+		inventory.Source{Repository: "novox/one", Ref: "main", BuiltFrom: "c0", Asked: time.Now()}); 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)
+	}
+}