From 35252af66508d9f71b764846cf507f9b8382b0de Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 28 Sep 2026 03:51:14 +0200 Subject: [PATCH] A build records the bases it was handed, and the mesh reads its edges from builds Bases reach a recipe as build arguments, so the digest was never in the file the builder read edges from: no build on the mesh recorded what it stood on, and 'build --on', the bases-first order and the merge follow-up all walked a graph with no edges (novox/hq 04-ISSUES/131). The builder now reports every base it resolved; the controller records them by artifact path and reads the newest build's edges from the store, since a recorded manifest carries no build.on. --- cmd/mesh-controller/build.go | 32 +++++++------ cmd/mesh-controller/order_test.go | 45 +++++++++++++++---- cmd/mesh-controller/upgrades.go | 67 ++++++++++++++++------------ internal/builder/builder.go | 49 +++++++++++++------- internal/builder/standing_on_test.go | 40 ++++++++++++++--- internal/inventory/builds.go | 35 +++++++++++++++ 6 files changed, 198 insertions(+), 70 deletions(-) diff --git a/cmd/mesh-controller/build.go b/cmd/mesh-controller/build.go index df62f6d..159f11b 100644 --- a/cmd/mesh-controller/build.go +++ b/cmd/mesh-controller/build.go @@ -42,23 +42,21 @@ func buildOn(ctx context.Context, base string, wait time.Duration) error { if err != nil { return err } + against, err := open.inventory.BuiltAgainst(ctx) + if err != nil { + return err + } var on []inventory.Entry for _, e := range held { - if e.Manifest.Build == nil { - continue - } - for _, b := range e.Manifest.Build.On { - if standsOnModule(b, base) { - on = append(on, e) - break - } + if standsOnModule(e, base, against) { + on = append(on, e) } } if len(on) == 0 { fmt.Printf("nothing the mesh holds stands on %s\n", base) return nil } - on = orderByBases(on) + on = orderByBases(on, against) fmt.Printf("%d module(s) stand on %s:\n", len(on), base) var failed []string for _, e := range on { @@ -134,8 +132,9 @@ func buildCommand(ctx context.Context, args []string) error { // // **By digest and path, never by where it was pushed** (novox/hq 04-ISSUES/102). The builder // says `://@sha256:…`; the mesh records the artifact-store -// reference and composes the store's address back in where a reference is used. `against` is kept -// as announced: it is what the build stood on as the builder saw it, and the catalogue's edge. +// reference and composes the store's address back in where a reference is used. `against` — what +// the build stood on, the catalogue's edge — is recorded the same way, so an edge names a module's +// artifact and not the machine it was pulled from. func buildFrom(result link.BuildResult) inventory.Build { kept := inventory.Build{ ID: result.ID, Repository: result.Repository, Ref: result.Ref, @@ -145,7 +144,10 @@ func buildFrom(result link.BuildResult) inventory.Build { // edges, and it is not always listening when a build happens — on a fresh mesh it cannot // be, for exactly the modules it needs most. Keeping them is what makes a replay able to // rebuild the graph rather than a list of names. - Path: result.Path, Against: result.Against, + Path: result.Path, + } + for _, ref := range result.Against { + kept.Against = append(kept.Against, catalogue.Recorded(ref)) } var announced []inventory.Artifact for _, made := range result.Made { @@ -344,7 +346,11 @@ func buildBehind(ctx context.Context, wait time.Duration) error { // Bases first: a module built before the module it stands on is built against the old one // and reports success (novox/hq 04-ISSUES/131). - stale = orderByBases(stale) + against, err := inv.BuiltAgainst(ctx) + if err != nil { + return err + } + stale = orderByBases(stale, against) var failed []string for _, e := range stale { diff --git a/cmd/mesh-controller/order_test.go b/cmd/mesh-controller/order_test.go index 1b61675..9a78a07 100644 --- a/cmd/mesh-controller/order_test.go +++ b/cmd/mesh-controller/order_test.go @@ -1,6 +1,7 @@ package main import ( + "strings" "testing" "github.com/novox/mesh-controller/internal/catalogue" @@ -8,19 +9,28 @@ import ( "github.com/novox/mesh-controller/internal/link" ) -func entry(module string, on ...string) inventory.Entry { - b := &catalogue.Build{} - for _, o := range on { - b.On = append(b.On, catalogue.BuildsOn{Arg: "X", Module: o, Artifact: "runtime"}) +// entry is a module as the catalogue holds it: built, so its manifest carries no `build` any more. +func entry(module string, _ ...string) inventory.Entry { + return inventory.Entry{Manifest: catalogue.Manifest{Module: module}} +} + +// stoodOn is what each module's newest build recorded it was handed. +func stoodOn(edges map[string][]string) map[string][]string { + out := map[string][]string{} + for module, bases := range edges { + for _, b := range bases { + out[module] = append(out[module], catalogue.ArtifactStoreScheme+b+"/runtime@sha256:"+strings.Repeat("0", 64)) + } } - return inventory.Entry{Manifest: catalogue.Manifest{Module: module, Build: b}} + return out } // A module built before the module it stands on is built against the old one and reports success // (novox/hq 04-ISSUES/131). So bases come first, however the set arrived. func TestBasesAreBuiltBeforeWhatStandsOnThem(t *testing.T) { - in := []inventory.Entry{entry("app", "runtime"), entry("runtime", "base"), entry("other"), entry("base")} - got := orderByBases(in) + in := []inventory.Entry{entry("app"), entry("runtime"), entry("other"), entry("base")} + edges := stoodOn(map[string][]string{"app": {"runtime"}, "runtime": {"base"}}) + got := orderByBases(in, edges) pos := map[string]int{} for i, e := range got { pos[e.Manifest.Module] = i @@ -32,12 +42,31 @@ func TestBasesAreBuiltBeforeWhatStandsOnThem(t *testing.T) { t.Fatalf("an entry was lost or doubled: %d", len(got)) } // A base outside the set is not waited for: it is not being rebuilt. - got = orderByBases([]inventory.Entry{entry("app", "elsewhere")}) + got = orderByBases([]inventory.Entry{entry("app")}, stoodOn(map[string][]string{"app": {"elsewhere"}})) if len(got) != 1 { t.Fatalf("a dependency outside the set changed the set: %v", got) } } +// A module registered from its manifest and never built still names its bases there; once built, +// the recorded edge is what says so. Both are read, and a module never stands on itself. +func TestWhatStandsOnAModuleIsReadFromItsBuildOrItsManifest(t *testing.T) { + built := entry("gitea") + edges := stoodOn(map[string][]string{"gitea": {"mesh-tools"}}) + if !standsOnModule(built, "mesh-tools", edges) { + t.Fatal("a recorded edge was not read") + } + if standsOnModule(built, "gitea", edges) || standsOnModule(built, "postgres", edges) { + t.Fatal("an edge was invented") + } + fresh := inventory.Entry{Manifest: catalogue.Manifest{Module: "plex", Build: &catalogue.Build{ + On: []catalogue.BuildsOn{{Arg: "RUNTIME_BASE", Module: "mesh-tools", Artifact: "runtime"}}, + }}} + if !standsOnModule(fresh, "mesh-tools", nil) { + t.Fatal("a manifest's own base was not read") + } +} + // A merge names a repository the way the forge does; a source is recorded the way a build was // asked for. The two meet on owner/repo and branch, whichever form the record took. func TestAMergeMatchesTheSourcesBuiltFromIt(t *testing.T) { diff --git a/cmd/mesh-controller/upgrades.go b/cmd/mesh-controller/upgrades.go index 51f7f05..5b78207 100644 --- a/cmd/mesh-controller/upgrades.go +++ b/cmd/mesh-controller/upgrades.go @@ -250,7 +250,11 @@ func (f following) SourceMoved(ctx context.Context, m link.SourceMoved) error { m.Owner, m.Repo, m.Base, m.Commit) return nil } - ordered := orderByBases(moved) + against, err := inv.BuiltAgainst(ctx) + if err != nil { + return notNow(err) + } + ordered := orderByBases(moved, against) names := make([]string, 0, len(ordered)) for _, e := range ordered { names = append(names, e.Manifest.Module) @@ -265,7 +269,7 @@ func (f following) SourceMoved(ctx context.Context, m link.SourceMoved) error { failed = append(failed, e.Manifest.Module) // A base that failed is a reason to stop: what stands on it would be built against // the old one, and report success (novox/hq 04-ISSUES/131). - if standsOn(ordered, e.Manifest.Module) { + if standsOn(ordered, e.Manifest.Module, against) { fmt.Printf(" stopping: %s is a base of what was still to build\n", e.Manifest.Module) break } @@ -292,10 +296,13 @@ func sourceIs(s inventory.Source, m link.SourceMoved) bool { return s.Ref == "" || s.Ref == m.Base } -// orderByBases is the entries with every base before what stands on it: a module whose build names -// another's artifact under build.on comes after that module. Entries outside the set are not -// waited for — they are not being rebuilt. Stable for what has no order between it. -func orderByBases(entries []inventory.Entry) []inventory.Entry { +// orderByBases is the entries with every base before what stands on it: a module whose build stood +// on another's artifact comes after that module. Entries outside the set are not waited for — they +// are not being rebuilt. Stable for what has no order between it. +// +// `against` is what each module's newest build stood on (inventory.BuiltAgainst): the edges are +// derived from builds, not declared, because a recorded manifest no longer carries `build.on`. +func orderByBases(entries []inventory.Entry, against map[string][]string) []inventory.Entry { inSet := map[string]bool{} for _, e := range entries { inSet[e.Manifest.Module] = true @@ -309,13 +316,9 @@ func orderByBases(entries []inventory.Entry) []inventory.Entry { return } seen[name] = true - if e.Manifest.Build != nil { - for _, on := range e.Manifest.Build.On { - for _, base := range entries { - if base.Manifest.Module != name && inSet[base.Manifest.Module] && standsOnModule(on, base.Manifest.Module) { - place(base, seen) - } - } + for _, base := range entries { + if base.Manifest.Module != name && inSet[base.Manifest.Module] && standsOnModule(e, base.Manifest.Module, against) { + place(base, seen) } } placed[name] = true @@ -328,26 +331,34 @@ func orderByBases(entries []inventory.Entry) []inventory.Entry { } // standsOn is whether anything in the set is built on the named module's artifacts. -func standsOn(entries []inventory.Entry, module string) bool { +func standsOn(entries []inventory.Entry, module string, against map[string][]string) bool { for _, e := range entries { - if e.Manifest.Build == nil { - continue - } - for _, on := range e.Manifest.Build.On { - if standsOnModule(on, module) { - return true - } + if standsOnModule(e, module, against) { + return true } } return false } -// standsOnModule is whether a base names the module: as written in a manifest (`module`), or as -// recorded after a build, when the mesh has replaced it with the artifact it resolved to -// (`artifact-store:///@…`). A recorded manifest is what the catalogue holds. -func standsOnModule(on catalogue.BuildsOn, module string) bool { - if on.Module == module { - return true +// standsOnModule is whether an entry's build stood on the named module: by what its newest build +// recorded it was handed (`artifact-store:///@…`, the module's own artifact), or +// — for a module registered from a manifest and not yet built — by the base its manifest names. +func standsOnModule(e inventory.Entry, module string, against map[string][]string) bool { + if e.Manifest.Module == module { + return false } - return strings.HasPrefix(on.Image, "artifact-store://"+module+"/") + if e.Manifest.Build != nil { + for _, on := range e.Manifest.Build.On { + if on.Module == module { + return true + } + } + } + prefix := catalogue.ArtifactStoreScheme + module + "/" + for _, ref := range against[e.Manifest.Module] { + if strings.HasPrefix(ref, prefix) { + return true + } + } + return false } diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 3aa56f3..e6b3097 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -169,6 +169,8 @@ func Build(ctx context.Context, run Runner, publish Publisher, } var built []catalogue.Built + // stoodOn is every base the build was handed, as resolved — the edges the catalogue derives. + var stoodOn []string if manifest.Build != nil { // What this module said it stands on, answered with what this mesh actually holds. Done // before anything is built, so a missing base is refused in front of the person who can @@ -186,11 +188,12 @@ func Build(ctx context.Context, run Runner, publish Publisher, } return from, nil } - args, err := standingOn(ctx, manifest, held, mirror) + args, bases, err := standingOn(ctx, manifest, held, mirror) if err != nil { say("bases", "UNMET: %v", err) return Result{}, err } + stoodOn = bases if len(args) > 0 { say("bases", "%d resolved from what the mesh holds", len(args)/2) } @@ -217,7 +220,7 @@ func Build(ctx context.Context, run Runner, publish Publisher, } say("done", "%s at %s — %d artifact(s) pinned", manifest.Module, short(commit), len(built)) return Result{Manifest: resolved, Commit: commit, Built: built, - Against: against(within, manifest)}, nil + Against: against(within, manifest, stoodOn)}, nil } // Log is where a build says what it is doing, step by step. Nil is silent — the tests pass none, @@ -339,14 +342,27 @@ func describe(path string) string { // allowed to name — a tag is something somebody else can move under you. var pinnedImage = regexp.MustCompile(`[A-Za-z0-9][A-Za-z0-9._/:-]*@sha256:[0-9a-f]{64}`) -// against reads what this module's image artifacts are built on top of, out of the files that -// build them. Nothing is guessed: a reference that is not written down is not reported. -func against(within string, manifest catalogue.Manifest) []string { +// against is what this module's image artifacts are built on top of: every base the mesh resolved +// and handed the recipe as a build argument (`build.on`), and any image a recipe pins by digest +// itself. Nothing is guessed: a reference that was neither resolved nor written down is not +// reported. +// +// **The resolved bases are the edges.** A recipe reads its base from an argument (`FROM +// ${RUNTIME_BASE}`), so the digest is never in the file, and a derivation that read files alone +// recorded no edge for any module on the mesh — which is why nothing knew what a changed base +// meant to rebuild (novox/hq 04-ISSUES/131). +func against(within string, manifest catalogue.Manifest, resolved []string) []string { if manifest.Build == nil { return nil } seen := map[string]bool{} var out []string + for _, r := range resolved { + if r != "" && !seen[r] { + seen[r] = true + out = append(out, r) + } + } for _, a := range manifest.Build.Artifacts { if a.Kind != catalogue.ArtifactImage || a.From == "" { continue @@ -704,40 +720,42 @@ var _ io.Writer = (*stringWriter)(nil) // built cannot be built here yet, and the useful sentence names which module is missing — not the // one a container runtime produces when a recipe's first line refers to an image nobody has. // -// The order is fixed so two builds of one commit invoke the same command. +// The order is fixed so two builds of one commit invoke the same command. Returned alongside the +// arguments is every reference they resolved to, which is what the build stood on. func standingOn(ctx context.Context, manifest catalogue.Manifest, held map[string]string, - mirror func(ctx context.Context, from, repository string) (string, error)) ([]string, error) { + mirror func(ctx context.Context, from, repository string) (string, error)) ([]string, []string, error) { if manifest.Build == nil || len(manifest.Build.On) == 0 { - return nil, nil + return nil, nil, nil } on := append([]catalogue.BuildsOn{}, manifest.Build.On...) sort.Slice(on, func(i, j int) bool { return on[i].Arg < on[j].Arg }) - var args []string + var args, resolved []string for _, base := range on { if base.Image != "" { // A vendor's image, declared (novox/hq 04-ISSUES/064, ADR 0097). Pinned, because a tag // is what somebody else can move; copied into the mesh's registry, because a build // that reaches a public registry on its own is a build that works sometimes. if base.Arg == "" || base.Module != "" || base.Artifact != "" { - return nil, fmt.Errorf( + return nil, nil, fmt.Errorf( "%s stands on the image %s, and a base is either a module's artifact or an "+ "image — never both — read from one build argument", manifest.Module, base.Image) } if !strings.Contains(base.Image, "@sha256:") { - return nil, fmt.Errorf( + return nil, nil, fmt.Errorf( "%s stands on the image %q, which is not pinned by digest. A tag is what "+ "somebody else can move; name it as @sha256:…", manifest.Module, base.Image) } reference, err := mirror(ctx, base.Image, manifest.Module+"/on-"+strings.ToLower(base.Arg)) if err != nil { - return nil, fmt.Errorf("%s stands on %s: %w", manifest.Module, base.Image, err) + return nil, nil, fmt.Errorf("%s stands on %s: %w", manifest.Module, base.Image, err) } args = append(args, "--build-arg", base.Arg+"="+reference) + resolved = append(resolved, reference) continue } if base.Arg == "" || base.Module == "" || base.Artifact == "" { - return nil, fmt.Errorf( + return nil, nil, fmt.Errorf( "%s says its build stands on something, and does not say all of what: a base "+ "needs the module, the artifact, and the build argument the recipe reads it "+ "from", manifest.Module) @@ -745,14 +763,15 @@ func standingOn(ctx context.Context, manifest catalogue.Manifest, held map[strin key := base.Module + "/" + base.Artifact reference, has := held[key] if !has { - return nil, fmt.Errorf( + return nil, nil, fmt.Errorf( "%s builds on %s, and this mesh has not built it. Build %s first — every module "+ "in this toolchain stands on it, so it is the thing to have before anything "+ "else", manifest.Module, key, base.Module) } args = append(args, "--build-arg", base.Arg+"="+reference) + resolved = append(resolved, reference) } - return args, nil + return args, resolved, nil } // compile runs a module's own code through its toolchain, and says where the result is. diff --git a/internal/builder/standing_on_test.go b/internal/builder/standing_on_test.go index 9f87782..d3f2d88 100644 --- a/internal/builder/standing_on_test.go +++ b/internal/builder/standing_on_test.go @@ -22,7 +22,7 @@ func TestABaseTheMeshHasNotBuiltIsRefused(t *testing.T) { On: []catalogue.BuildsOn{{Arg: "RUNTIME_BASE", Module: "mesh-tools", Artifact: "runtime"}}, }, } - _, err := standingOn(context.Background(), manifest, map[string]string{}, noMirror) + _, _, err := standingOn(context.Background(), manifest, map[string]string{}, noMirror) if err == nil { t.Fatal("a base nothing has built was accepted; the build would have failed on its first line") } @@ -42,7 +42,7 @@ func TestABaseTheMeshHoldsBecomesABuildArgument(t *testing.T) { }, } held := map[string]string{"mesh-tools/runtime": "127.0.0.1:5000/mesh-tools/runtime@sha256:" + strings.Repeat("a", 64)} - args, err := standingOn(context.Background(), manifest, held, noMirror) + args, _, err := standingOn(context.Background(), manifest, held, noMirror) if err != nil { t.Fatalf("a base this mesh holds was refused: %v", err) } @@ -54,7 +54,7 @@ func TestABaseTheMeshHoldsBecomesABuildArgument(t *testing.T) { // A module naming no base asks for nothing, which is most modules. func TestAModuleNamingNoBaseAddsNoArguments(t *testing.T) { - args, err := standingOn(context.Background(), catalogue.Manifest{Module: "hello-web", Build: &catalogue.Build{}}, nil, noMirror) + args, _, err := standingOn(context.Background(), catalogue.Manifest{Module: "hello-web", Build: &catalogue.Build{}}, nil, noMirror) if err != nil || args != nil { t.Fatalf("a module naming no base produced %v, %v", args, err) } @@ -66,7 +66,7 @@ func TestAnIncompleteBaseIsRefused(t *testing.T) { Module: "postgres", Build: &catalogue.Build{On: []catalogue.BuildsOn{{Module: "mesh-tools", Artifact: "runtime"}}}, } - if _, err := standingOn(context.Background(), manifest, map[string]string{"mesh-tools/runtime": "x"}, noMirror); err == nil { + if _, _, err := standingOn(context.Background(), manifest, map[string]string{"mesh-tools/runtime": "x"}, noMirror); err == nil { t.Fatal("a base with no build argument was accepted; nothing would have read it") } } @@ -86,7 +86,7 @@ func TestADeclaredVendorImageIsCopiedInAndHandedToTheRecipe(t *testing.T) { }, } var asked []string - args, err := standingOn(context.Background(), manifest, nil, func(_ context.Context, from, repository string) (string, error) { + args, _, err := standingOn(context.Background(), manifest, nil, func(_ context.Context, from, repository string) (string, error) { asked = append(asked, from+" -> "+repository) return "127.0.0.1:5000/" + repository + "@sha256:" + strings.Repeat("d", 64), nil }) @@ -101,7 +101,7 @@ func TestADeclaredVendorImageIsCopiedInAndHandedToTheRecipe(t *testing.T) { } // Unpinned, it is refused: a tag is what somebody else can move. manifest.Build.On[0].Image = "quay.io/minio/mc:latest" - if _, err := standingOn(context.Background(), manifest, nil, noMirror); err == nil || !strings.Contains(err.Error(), "not pinned") { + if _, _, err := standingOn(context.Background(), manifest, nil, noMirror); err == nil || !strings.Contains(err.Error(), "not pinned") { t.Fatalf("an unpinned vendor image was accepted: %v", err) } } @@ -151,3 +151,31 @@ func TestARecipeIsReadAsInstructions(t *testing.T) { t.Fatalf("a heredoc line or a continued stage was read as a base: %v", bases) } } + +// What a build was handed as its bases is what it stood on — recorded, so a changed base knows what +// to rebuild (novox/hq 04-ISSUES/131). A recipe reads the base from an argument, so nothing else +// could know. +func TestTheBasesABuildWasHandedAreWhatItStoodOn(t *testing.T) { + manifest := catalogue.Manifest{ + Module: "gitea", + Build: &catalogue.Build{ + On: []catalogue.BuildsOn{ + {Arg: "RUNTIME_BASE", Module: "mesh-tools", Artifact: "runtime"}, + {Arg: "BUILD_BASE", Module: "mesh-tools", Artifact: "build"}, + }, + Artifacts: []catalogue.Artifact{{Name: "runtime", Kind: catalogue.ArtifactImage, From: "Dockerfile"}}, + }, + } + held := map[string]string{ + "mesh-tools/runtime": "127.0.0.1:5000/mesh-tools/runtime@sha256:" + strings.Repeat("a", 64), + "mesh-tools/build": "127.0.0.1:5000/mesh-tools/build@sha256:" + strings.Repeat("b", 64), + } + _, resolved, err := standingOn(context.Background(), manifest, held, noMirror) + if err != nil { + t.Fatal(err) + } + got := against(t.TempDir(), manifest, resolved) + if len(got) != 2 || got[0] != held["mesh-tools/build"] || got[1] != held["mesh-tools/runtime"] { + t.Fatalf("the bases the build was handed were not what it stood on: %v", got) + } +} diff --git a/internal/inventory/builds.go b/internal/inventory/builds.go index 037a531..fab94d5 100644 --- a/internal/inventory/builds.go +++ b/internal/inventory/builds.go @@ -164,6 +164,41 @@ func (i *Inventory) Held(ctx context.Context) (map[string]string, error) { return held, rows.Err() } +// BuiltAgainst is what each module's newest successful build stood on, as recorded — the build +// edges (ADR 0009). A module whose last build recorded no bases is absent, which is also what a +// module standing on nothing looks like: an edge the mesh has not derived is not an edge. +func (i *Inventory) BuiltAgainst(ctx context.Context) (map[string][]string, error) { + rows, err := i.store.Pool().Query(ctx, + `select distinct on (module) module, built_against + from build + where module is not null and module <> '' and failed = '' + order by module, at desc`) + if err != nil { + return nil, err + } + defer rows.Close() + + against := map[string][]string{} + for rows.Next() { + var module string + var raw []byte + if err := rows.Scan(&module, &raw); err != nil { + return nil, err + } + if len(raw) == 0 { + continue + } + var refs []string + if err := json.Unmarshal(raw, &refs); err != nil { + continue + } + if len(refs) > 0 { + against[module] = refs + } + } + return against, rows.Err() +} + // manifestOrNil keeps the difference between "declared nothing" and "predates this being kept". // // A build recorded before the mesh kept manifests has no manifest, and that is not the same as one -- 2.54.0