From 9745c1ab31fdbdef7a801533c8680dc47ce85830 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 4 Oct 2026 00:22:11 +0200 Subject: [PATCH] An older build request never replaces a newer one's artifact Builds of one module in flight together finish in any order, and the mesh took whatever it heard last as what the module is: RegisterModule overwrote the module's manifest unconditionally, and Held/BuiltAgainst/ReadRepositories ordered builds by when they were recorded. A postgres build asked before the mesh-tools runtime fix finished after the one asked after it, and the next push deployed the stale image (novox/hq issue 219). A build is now ordered by when it was asked, read from the build- id the controller writes: build.asked and module.built_asked (migration 0055). A registration from an earlier request than the module's current one is recorded and refused as superseded. A plan takes as its outcome only a build asked at or after its own ask, so an earlier plan's leftover build cannot settle a later plan. Ids of any other shape keep the old order. --- cmd/mesh-controller/build.go | 16 ++- cmd/mesh-controller/build_test.go | 50 +++++++ cmd/mesh-controller/main.go | 16 ++- cmd/mesh-controller/release_plan.go | 25 +++- cmd/mesh-controller/release_plan_test.go | 18 +++ internal/inventory/builds.go | 57 ++++++-- internal/inventory/catalogue.go | 44 ++++++- ...-an-older-build-never-replaces-a-newer.sql | 23 ++++ internal/inventory/older_build_test.go | 124 ++++++++++++++++++ internal/link/build.go | 29 ++++ 10 files changed, 375 insertions(+), 27 deletions(-) create mode 100644 internal/inventory/migrations/0055-an-older-build-never-replaces-a-newer.sql create mode 100644 internal/inventory/older_build_test.go diff --git a/cmd/mesh-controller/build.go b/cmd/mesh-controller/build.go index 452a74e..26684d6 100644 --- a/cmd/mesh-controller/build.go +++ b/cmd/mesh-controller/build.go @@ -148,6 +148,11 @@ func buildFrom(result link.BuildResult) inventory.Build { // rebuild the graph rather than a list of names. Path: result.Path, } + // When it was asked, which is what orders it against another build of the same module + // (novox/hq 04-ISSUES/219) — not when it was heard. + if asked, ok := link.BuildAskedAt(result.ID); ok { + kept.Asked = asked + } for _, ref := range result.Against { kept.Against = append(kept.Against, catalogue.Recorded(ref)) } @@ -409,7 +414,7 @@ func buildOne(ctx context.Context, source buildSource, path, ref string, wait ti // Correlated by something the control plane makes, not by the module's name: two builds of one // module can be in flight, and the second answer is not the first one's. request := link.BuildRequest{ - ID: fmt.Sprintf("%s-%d", "build", time.Now().UnixNano()), + ID: link.NewBuildID(time.Now()), Repository: repository, Path: path, Ref: ref, @@ -526,6 +531,9 @@ func takeIn(ctx context.Context, inv *inventory.Inventory, result link.BuildResu BuiltFrom: result.Commit, Head: result.Commit, // What it stood on, so registration can judge a built manifest's base (to-be 38 WP2.4). Against: kept.Against, + // When it was asked, so an older request heard later does not replace a newer one + // (novox/hq 04-ISSUES/219). + Asked: kept.Asked, } if result.Source != nil && result.Source.Seat != "" { recorded.Repository, recorded.Seat = result.Source.Repository, result.Source.Seat @@ -544,6 +552,10 @@ func takeIn(ctx context.Context, inv *inventory.Inventory, result link.BuildResu result.On, result.Repository, short(result.Commit), err) } if err := inv.RegisterModule(ctx, manifest, recorded); err != nil { + if errors.Is(err, inventory.ErrSuperseded) { + return manifest, kept, fmt.Errorf("%s built %s (%s), recorded and not registered: %w", + result.On, manifest.Module, short(result.Commit), err) + } return manifest, kept, err } return manifest, kept, nil @@ -573,7 +585,7 @@ func buildAndShow(ctx context.Context, source buildSource, path, ref string, wai defer ask.Close() result, err := ask.Submit(ctx, link.BuildRequest{ - ID: fmt.Sprintf("%s-%d", "build", time.Now().UnixNano()), + ID: link.NewBuildID(time.Now()), Repository: repository, Path: path, Ref: ref, Held: heldBy(ctx), Seats: seatBases(ctx), }, wait) diff --git a/cmd/mesh-controller/build_test.go b/cmd/mesh-controller/build_test.go index 29d0d73..a7b0c69 100644 --- a/cmd/mesh-controller/build_test.go +++ b/cmd/mesh-controller/build_test.go @@ -2,9 +2,12 @@ package main import ( "encoding/json" + "errors" "strings" "testing" + "time" + "github.com/novox/mesh-controller/internal/inventory" "github.com/novox/mesh-controller/internal/link" ) @@ -100,3 +103,50 @@ func TestABuildAtACommitKeepsTheBranchTheModuleFollows(t *testing.T) { t.Errorf("a module first built at a commit follows %q, want the default branch", src.Ref) } } + +// novox/hq 04-ISSUES/219: an older request heard after a newer one is recorded and not registered, +// so a push sends what the newer request built. +func TestAnOlderBuildHeardLaterDoesNotReplaceTheNewer(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + older := time.Date(2026, 10, 3, 21, 33, 45, 0, time.UTC) + newer := time.Date(2026, 10, 3, 21, 51, 57, 0, time.UTC) + result := func(asked time.Time, image string) link.BuildResult { + manifest, _ := json.Marshal(map[string]any{"module": "postgres", "version": image}) + return link.BuildResult{ID: link.NewBuildID(asked), Repository: "http://forge.internal:20000/novox/mesh-catalog.git", + Path: "modules/postgres", Ref: "main", On: "anchor", Commit: "efff5415", Manifest: manifest, + Source: &link.SourceOnSeat{Seat: "git", Repository: "novox/mesh-catalog"}} + } + if _, _, err := takeIn(ctx, open.inventory, result(newer, "4bcd5f73")); err != nil { + t.Fatal(err) + } + _, _, err := takeIn(ctx, open.inventory, result(older, "0ab07fa9")) + if !errors.Is(err, inventory.ErrSuperseded) { + t.Fatalf("the older request's outcome was taken in as current: %v", err) + } + shelf, err := open.inventory.Catalogue(ctx) + if err != nil { + t.Fatal(err) + } + if got := shelf["postgres"].Version; got != "4bcd5f73" { + t.Errorf("postgres is %q; want the newer request's 4bcd5f73", got) + } + if builds, _ := open.inventory.Builds(ctx, "postgres", 5); len(builds) != 2 { + t.Errorf("the late build was not recorded: %v", builds) + } +} + +func TestABuildIDSaysWhenItWasAsked(t *testing.T) { + at := time.Date(2026, 10, 3, 21, 51, 57, 392539762, time.UTC) + if got, ok := link.BuildAskedAt(link.NewBuildID(at)); !ok || !got.Equal(at) { + t.Errorf("read back %v %v; want %v", got, ok, at) + } + if got, ok := link.BuildAskedAt("build-1791064317392539762"); !ok || got.Format(time.TimeOnly) != "21:51:57" { + t.Errorf("the incident's id reads as %v %v", got, ok) + } + for _, id := range []string{"b-1", "build-2", "build-", "build-x", ""} { + if _, ok := link.BuildAskedAt(id); ok { + t.Errorf("%q read as a request time", id) + } + } +} diff --git a/cmd/mesh-controller/main.go b/cmd/mesh-controller/main.go index ee30421..7441b5b 100644 --- a/cmd/mesh-controller/main.go +++ b/cmd/mesh-controller/main.go @@ -8,6 +8,7 @@ package main import ( "context" + "errors" "flag" "fmt" "os" @@ -253,25 +254,34 @@ func parseAround(set *flag.FlagSet, args []string) ([]string, error) { // became of a build nobody was watching. func (b builds) Built(ctx context.Context, result link.BuildResult) error { manifest, _, err := takeIn(ctx, b.inv, result) + // When it was asked, so a plan takes as its outcome only a build asked for it or after it + // (novox/hq 04-ISSUES/219). Zero when the id does not say. + asked, _ := link.BuildAskedAt(result.ID) switch { case err != nil && result.Failed != "": fmt.Printf("%s: %v\n", result.ID, err) if result.Module != "" { - planBuilt(ctx, b.open, result.Module, result.Commit, result.Failed) + planBuilt(ctx, b.open, result.Module, result.Commit, result.Failed, asked) } else { planFailedBuild(ctx, b.open, result) } return nil + case errors.Is(err, inventory.ErrSuperseded): + // Not a failure: the module is already at what a later request built. A plan that asked + // before that later request is answered by it; one that asked after it ignores this. + fmt.Printf("%s: %v\n", result.ID, err) + planBuilt(ctx, b.open, manifest.Module, result.Commit, "", asked) + return nil case err != nil: fmt.Printf("%s: heard and recorded, and not registered: %v\n", result.ID, err) if manifest.Module != "" { - planBuilt(ctx, b.open, manifest.Module, result.Commit, err.Error()) + planBuilt(ctx, b.open, manifest.Module, result.Commit, err.Error(), asked) } return nil } fmt.Printf("%s: %s %s registered, built on %s from %s\n", result.ID, manifest.Module, manifest.Version, result.On, short(result.Commit)) saysWhenThePolicyActs(ctx, b.inv, manifest.Module) - planBuilt(ctx, b.open, manifest.Module, result.Commit, "") + planBuilt(ctx, b.open, manifest.Module, result.Commit, "", asked) return nil } diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 65c2b01..ef8cf64 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -288,7 +288,13 @@ func askTier(ctx context.Context, inv *inventory.Inventory, p *inventory.Plan) e // planBuilt marks a module built (or failed) in every open plan whose current tier holds it, and // advances what that completes. Called from the daemon's take-in of every outcome. -func planBuilt(ctx context.Context, open *stores, module, commit, failed string) { +// +// **Only a build asked at or after the plan's ask is its outcome** (novox/hq 04-ISSUES/219). Two +// plans a few minutes apart both ask for a module; the earlier plan's build, finishing late, is not +// the later plan's answer — it stood on the bases from before the later plan's merge, and taking it +// would send machines, and the next tier, what the later merge replaced. asked is zero when the +// build's request time is not known, and such an outcome is taken as before. +func planBuilt(ctx context.Context, open *stores, module, commit, failed string, asked time.Time) { inv := open.inventory plans, err := inv.OpenPlans(ctx) if err != nil { @@ -315,6 +321,9 @@ func planBuilt(ctx context.Context, open *stores, module, commit, failed string) state = &inventory.PlanModule{} p.Modules[module] = state } + if askedBefore(asked, state.AskedAt) { + continue + } if failed != "" { state.State = "failed" state.Why = failed @@ -549,7 +558,8 @@ func planFailedBuild(ctx context.Context, open *stores, result link.BuildResult) } for _, e := range entries { if repositoryMatches(e.Source.Repository, result.Repository) && e.Source.Path == result.Path { - planBuilt(ctx, open, e.Manifest.Module, result.Commit, result.Failed) + asked, _ := link.BuildAskedAt(result.ID) + planBuilt(ctx, open, e.Manifest.Module, result.Commit, result.Failed, asked) return } } @@ -791,6 +801,11 @@ func settleFromRecords(p *inventory.Plan, tier []string, recorded map[string][]i if b.At.Before(*s.AskedAt) { break } + // Recorded after the ask and asked before it: an earlier ask's late outcome, not this + // one's (novox/hq 04-ISSUES/219). + if askedBefore(b.Asked, s.AskedAt) { + continue + } outcome = &b } if outcome == nil { @@ -812,3 +827,9 @@ func settleFromRecords(p *inventory.Plan, tier []string, recorded map[string][]i } return changed } + +// askedBefore is whether a build asked at asked was asked before a plan asked for its module — and +// so is not that plan's outcome (novox/hq 04-ISSUES/219). False when either time is not known. +func askedBefore(asked time.Time, planAsked *time.Time) bool { + return !asked.IsZero() && planAsked != nil && asked.Before(*planAsked) +} diff --git a/cmd/mesh-controller/release_plan_test.go b/cmd/mesh-controller/release_plan_test.go index c414023..93a2288 100644 --- a/cmd/mesh-controller/release_plan_test.go +++ b/cmd/mesh-controller/release_plan_test.go @@ -155,6 +155,24 @@ func TestAPlanSettlesAnAskedBuildFromTheRecords(t *testing.T) { t.Errorf("an ask with no record after it was settled: %+v", s) } + // novox/hq 04-ISSUES/219: a build recorded after the ask but asked before it — an earlier + // plan's late outcome — is not this ask's, built or failed. + r := inventory.Plan{ID: "plan-3", Tiers: [][]string{{"postgres"}}, + Modules: map[string]*inventory.PlanModule{"postgres": {State: "asked", AskedAt: &asked}}} + late := map[string][]inventory.Build{"postgres": { + {ID: "build-old", Commit: "efff5415", Asked: asked.Add(-18 * time.Minute), At: asked.Add(12 * time.Minute)}, + }} + if settleFromRecords(&r, r.Tiers[0], late) || r.Modules["postgres"].State != "asked" { + t.Errorf("an earlier ask's late outcome settled this ask: %+v", r.Modules["postgres"]) + } + // Newest heard first: the earlier ask's late outcome, then this ask's own, heard before it. + late["postgres"] = append(late["postgres"], inventory.Build{ID: "build-mine", Commit: "4bcd5f73", + Asked: asked.Add(time.Second), At: asked.Add(5 * time.Minute)}) + if !settleFromRecords(&r, r.Tiers[0], late) || r.Modules["postgres"].State != "built" || + r.Modules["postgres"].Commit != "4bcd5f73" { + t.Errorf("this ask's own outcome, heard before the earlier ask's, did not settle it: %+v", r.Modules["postgres"]) + } + // A failure recorded after the ask fails the plan, as hearing it would have. q := inventory.Plan{ID: "plan-2", Tiers: [][]string{{"x"}}, Modules: map[string]*inventory.PlanModule{"x": {State: "asked", AskedAt: &asked}}} diff --git a/internal/inventory/builds.go b/internal/inventory/builds.go index a4e6a7c..5400acf 100644 --- a/internal/inventory/builds.go +++ b/internal/inventory/builds.go @@ -42,9 +42,29 @@ type Build struct { // Failed is the builder's own words, empty when it worked. Failed string Made []Artifact - At time.Time + // Asked is when the build was requested, zero when that is not known (an id of another shape, + // or a build recorded before the mesh kept it). **What orders one build of a module against + // another** (novox/hq 04-ISSUES/219): builds in flight together finish in any order, and the + // one asked last stood on the newest bases. + Asked time.Time + // At is when the outcome was recorded — when it finished, not when it was asked. + At time.Time } +// AskedOrAt is when the build was asked, or when it was recorded when that is not known — the +// order the mesh had before it kept the request time. +func (b Build) AskedOrAt() time.Time { + if !b.Asked.IsZero() { + return b.Asked + } + return b.At +} + +// newestRequestFirst is the ordering every "what a module currently is" question uses: the newest +// request wins, whenever it finished (novox/hq 04-ISSUES/219). A build whose request time is not +// known is placed at the moment it was recorded, which is the rule that held before. +const newestRequestFirst = `coalesce(asked, at) desc, at desc` + // ReadRepository is a repository a build read source from besides the module's own. type ReadRepository struct { Repository string `json:"repository"` @@ -83,13 +103,17 @@ func (i *Inventory) RecordBuild(ctx context.Context, b Build) error { if b.Module != "" { module = &b.Module } + var asked *time.Time + if !b.Asked.IsZero() { + asked = &b.Asked + } _, err = i.store.Pool().Exec(ctx, `insert into build (id, repository, ref, module, commit_hash, built_on, failed, made, - source_path, manifest, built_against, built_contexts) - values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) + source_path, manifest, built_against, built_contexts, asked) + values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13) on conflict (id) do nothing`, b.ID, b.Repository, b.Ref, module, b.Commit, b.On, b.Failed, made, - b.Path, manifestOrNil(b.Manifest), against, read) + b.Path, manifestOrNil(b.Manifest), against, read, asked) return err } @@ -102,11 +126,11 @@ func (i *Inventory) Builds(ctx context.Context, module string, limit int) ([]Bui if limit <= 0 { limit = 20 } - query := `select id, repository, ref, coalesce(module,''), commit_hash, built_on, failed, made, at + query := `select id, repository, ref, coalesce(module,''), commit_hash, built_on, failed, made, asked, at from build order by at desc limit $1` args := []any{limit} if module != "" { - query = `select id, repository, ref, coalesce(module,''), commit_hash, built_on, failed, made, at + query = `select id, repository, ref, coalesce(module,''), commit_hash, built_on, failed, made, asked, at from build where module = $2 order by at desc limit $1` args = append(args, module) } @@ -121,10 +145,14 @@ func (i *Inventory) Builds(ctx context.Context, module string, limit int) ([]Bui for rows.Next() { var b Build var made []byte + var asked *time.Time if err := rows.Scan(&b.ID, &b.Repository, &b.Ref, &b.Module, &b.Commit, - &b.On, &b.Failed, &made, &b.At); err != nil { + &b.On, &b.Failed, &made, &asked, &b.At); err != nil { return nil, err } + if asked != nil { + b.Asked = *asked + } if err := json.Unmarshal(made, &b.Made); err != nil { return nil, err } @@ -135,8 +163,9 @@ func (i *Inventory) Builds(ctx context.Context, module string, limit int) ([]Bui // Held is every artifact this mesh has built, keyed "/". // -// **The newest successful build of each module wins**, which is the same rule the rest of the mesh -// uses for what a module currently is. A module rebuilt to something broken and then rebuilt again +// **The successful build of each module asked last wins**, which is the same rule the rest of the +// mesh uses for what a module currently is — asked last, not finished last (novox/hq +// 04-ISSUES/219): an older request that finishes later stood on older bases. A module rebuilt to something broken and then rebuilt again // is at the second one; a module whose last build failed is at the last one that worked, because a // failure published nothing and the thing it published before is still what exists. // @@ -147,7 +176,7 @@ func (i *Inventory) Held(ctx context.Context) (map[string]string, error) { `select distinct on (module) module, made from build where module is not null and module <> '' and failed = '' - order by module, at desc`) + order by module, `+newestRequestFirst) if err != nil { return nil, err } @@ -185,7 +214,7 @@ func (i *Inventory) BuiltAgainst(ctx context.Context) (map[string][]string, erro `select distinct on (module) module, built_against from build where module is not null and module <> '' and failed = '' - order by module, at desc`) + order by module, `+newestRequestFirst) if err != nil { return nil, err } @@ -223,7 +252,7 @@ func (i *Inventory) ReadRepositories(ctx context.Context) (map[string][]ReadRepo `select distinct on (module) module, built_contexts from build where module is not null and module <> '' and failed = '' - order by module, at desc`) + order by module, `+newestRequestFirst) if err != nil { return nil, err } @@ -274,7 +303,7 @@ func manifestOrNil(raw []byte) any { // follows when it decides whether to announce at all. // // One row per module and commit: a module built twice at the same commit is one fact, and the -// latest row is the one whose artifacts are current. +// row asked last is the one whose artifacts are current (novox/hq 04-ISSUES/219). func (i *Inventory) Announceable(ctx context.Context) ([]Build, error) { rows, err := i.store.Pool().Query(ctx, `select distinct on (module, commit_hash) @@ -282,7 +311,7 @@ func (i *Inventory) Announceable(ctx context.Context) ([]Build, error) { source_path, manifest, built_against, at from build where failed = '' and module is not null and module <> '' and commit_hash <> '' - order by module, commit_hash, at desc`) + order by module, commit_hash, `+newestRequestFirst) if err != nil { return nil, err } diff --git a/internal/inventory/catalogue.go b/internal/inventory/catalogue.go index b6d23c1..04218f6 100644 --- a/internal/inventory/catalogue.go +++ b/internal/inventory/catalogue.go @@ -17,6 +17,10 @@ import ( // ErrNoSuchModule is what the mesh says about a module it has never been told about. var ErrNoSuchModule = errors.New("no module of that name") +// ErrSuperseded is a registration from a build asked before the one the module is already at +// (novox/hq 04-ISSUES/219). The build is recorded; what the module is does not change. +var ErrSuperseded = errors.New("a build asked later is already what the module is") + // ErrStillAssigned is why a module cannot be forgotten. // // Its own error because it is not a fault: it means a machine is running that module now, and @@ -47,6 +51,10 @@ type Source struct { // itself no longer carries its build (novox/hq to-be 38 WP2.4). Empty for a manifest handed over // by hand, which carries its `build.on` itself. Against []string + // Asked is when the build this manifest came from was requested (novox/hq 04-ISSUES/219). Zero + // is a manifest handed over by hand, or a build whose request time is not known: either is + // taken as asked at the moment it is registered. + Asked time.Time } // Current reports whether what the mesh holds is what the source last had. @@ -97,13 +105,23 @@ func (i *Inventory) RegisterModule(ctx context.Context, m catalogue.Manifest, fr return err } + asked := from.Asked + if asked.IsZero() { + asked = time.Now() + } + // A module registered without provenance keeps whatever it had. Handing over a manifest by // hand is a legitimate way to fix something in a hurry, and it should not silently erase the // record of where the module normally comes from — which is the only thing that would say, // afterwards, that the machine is running something nobody can rebuild. - _, err = i.store.Pool().Exec(ctx, - `insert into module (name, manifest, version, source, source_path, source_seat, ref, built_from, source_head) - values ($1, $2, nullif($3,''), nullif($4,''), $7, $8, nullif($5,''), nullif($6,''), nullif($6,'')) + // + // **An older request never replaces a newer one** (novox/hq 04-ISSUES/219). Builds of one + // module in flight together finish in any order, and each stood on the bases the mesh held when + // it was asked; the one asked later is what the module is, whichever is heard last. An outcome + // of an earlier request is kept in the build records and changes nothing here. + tag, err := i.store.Pool().Exec(ctx, + `insert into module (name, manifest, version, source, source_path, source_seat, ref, built_from, source_head, built_asked) + values ($1, $2, nullif($3,''), nullif($4,''), $7, $8, nullif($5,''), nullif($6,''), nullif($6,''), $9) on conflict (name) do update set manifest = excluded.manifest, version = excluded.version, @@ -115,9 +133,23 @@ func (i *Inventory) RegisterModule(ctx context.Context, m catalogue.Manifest, fr else excluded.source_seat end, ref = coalesce(excluded.ref, module.ref), built_from = coalesce(excluded.built_from, module.built_from), - source_head = coalesce(excluded.built_from, module.source_head)`, - m.Module, raw, m.Version, from.Repository, from.Ref, from.BuiltFrom, from.Path, from.Seat) - return err + source_head = coalesce(excluded.built_from, module.source_head), + built_asked = excluded.built_asked + where module.built_asked is null or module.built_asked <= excluded.built_asked`, + m.Module, raw, m.Version, from.Repository, from.Ref, from.BuiltFrom, from.Path, from.Seat, asked) + if err != nil { + return err + } + if tag.RowsAffected() == 0 { + var current time.Time + if err := i.store.Pool().QueryRow(ctx, + `select built_asked from module where name = $1`, m.Module).Scan(¤t); err != nil { + return err + } + return fmt.Errorf("%w: %s is at a build asked %s, and this one was asked %s", + ErrSuperseded, m.Module, current.UTC().Format(time.RFC3339), asked.UTC().Format(time.RFC3339)) + } + return nil } // registeredInThatShape is whether the catalogue already holds this module as a tools container on diff --git a/internal/inventory/migrations/0055-an-older-build-never-replaces-a-newer.sql b/internal/inventory/migrations/0055-an-older-build-never-replaces-a-newer.sql new file mode 100644 index 0000000..4ceb25d --- /dev/null +++ b/internal/inventory/migrations/0055-an-older-build-never-replaces-a-newer.sql @@ -0,0 +1,23 @@ +-- A build is ordered by when it was asked, not when it finished (novox/hq 04-ISSUES/219). +-- +-- Two builds of one module can be in flight together — two merge plans a few minutes apart, each +-- asking for everything standing on what it changed — and they finish in any order. Each build +-- stands on the bases the mesh held when it was *asked*, so the one asked later is the newer one. +-- The mesh ordered builds by `at`, which is when the outcome was recorded, and registered whatever +-- it heard last: an older request that took longer replaced a newer one as what the module is, and +-- the next push sent machines an image built on a base the mesh had already replaced. +-- +-- `build.asked` is when the build was requested, read from the correlation id the controller wrote +-- (`build-`). Nullable: an id of any other shape says no request time, and such a +-- build is placed where it was recorded, which is the order the mesh had before this. +alter table build add column asked timestamptz; + +update build + set asked = to_timestamp((substring(id from '^build-([0-9]{19})$'))::numeric / 1000000000) + where id ~ '^build-[0-9]{19}$'; + +-- `module.built_asked` is when the build the module's registered manifest came from was asked, so +-- a later-heard outcome of an earlier request is recorded and not registered. A manifest handed over +-- by hand is a request made when it is handed over. Null for a module registered before this was +-- kept: its next registration, whichever it is, sets it. +alter table module add column built_asked timestamptz; diff --git a/internal/inventory/older_build_test.go b/internal/inventory/older_build_test.go new file mode 100644 index 0000000..a0d3c7a --- /dev/null +++ b/internal/inventory/older_build_test.go @@ -0,0 +1,124 @@ +package inventory + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/novox/mesh-controller/internal/catalogue" +) + +// novox/hq 04-ISSUES/219: two builds of one module in flight together, the one asked first heard +// last. The newer request stood on the newer base; the older one's late outcome is recorded and is +// not what the module is. + +func postgresBuild(id string, asked time.Time, image string) Build { + b := aBuild(id, "postgres", "") + b.Asked = asked + b.Against = []string{"mesh-tools/runtime@sha256:" + id} + b.Made = []Artifact{{Name: "server", Kind: "image", Reference: "postgres@sha256:" + image}} + return b +} + +func TestAnOlderRequestFinishingLaterIsNotWhatTheModuleHolds(t *testing.T) { + inv := fresh(t) + ctx := context.Background() + older := time.Date(2026, 10, 3, 21, 33, 45, 0, time.UTC) + newer := time.Date(2026, 10, 3, 21, 51, 57, 0, time.UTC) + + // The newer request finishes first, the older one last — recorded in that order. + if err := inv.RecordBuild(ctx, postgresBuild("newer", newer, "4bcd5f73")); err != nil { + t.Fatal(err) + } + if err := inv.RecordBuild(ctx, postgresBuild("older", older, "0ab07fa9")); err != nil { + t.Fatal(err) + } + + held, err := inv.Held(ctx) + if err != nil { + t.Fatal(err) + } + if got := held["postgres/server"]; got != "postgres@sha256:4bcd5f73" { + t.Errorf("postgres holds %q; want the newer request's image 4bcd5f73", got) + } + against, err := inv.BuiltAgainst(ctx) + if err != nil { + t.Fatal(err) + } + if got := against["postgres"]; len(got) != 1 || got[0] != "mesh-tools/runtime@sha256:newer" { + t.Errorf("postgres stands on %v; want what the newer request stood on", got) + } + + // Both are still recorded, the late one first as what happened lately. + builds, err := inv.Builds(ctx, "postgres", 5) + if err != nil { + t.Fatal(err) + } + if len(builds) != 2 || builds[0].ID != "older" || !builds[0].Asked.Equal(older) { + t.Fatalf("both builds, newest heard first, with when they were asked: %+v", builds) + } +} + +func TestABuildWithNoKnownRequestTimeIsOrderedByWhenItWasRecorded(t *testing.T) { + // What the mesh did before it kept the request time, so a row from before still answers. + inv := fresh(t) + ctx := context.Background() + for _, id := range []string{"first", "second"} { + b := aBuild(id, "shell", "") + b.Made = []Artifact{{Name: "config", Kind: "archive", Reference: "…/" + id}} + if err := inv.RecordBuild(ctx, b); err != nil { + t.Fatal(err) + } + } + held, err := inv.Held(ctx) + if err != nil { + t.Fatal(err) + } + if got := held["shell/config"]; got != "…/second" { + t.Errorf("shell holds %q; want the one recorded last", got) + } +} + +func TestARegistrationFromAnOlderRequestDoesNotReplaceANewerOne(t *testing.T) { + inv := fresh(t) + ctx := context.Background() + older := time.Date(2026, 10, 3, 21, 33, 45, 0, time.UTC) + newer := time.Date(2026, 10, 3, 21, 51, 57, 0, time.UTC) + from := func(asked time.Time) Source { + return Source{Repository: "novox/mesh-catalog", Seat: "git", Path: "modules/postgres", + BuiltFrom: "efff5415", Asked: asked} + } + + if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "postgres", Version: "fixed"}, from(newer)); err != nil { + t.Fatal(err) + } + err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "postgres", Version: "stale"}, from(older)) + if !errors.Is(err, ErrSuperseded) { + t.Fatalf("an older request's registration was not refused as superseded: %v", err) + } + shelf, err := inv.Catalogue(ctx) + if err != nil { + t.Fatal(err) + } + if got := shelf["postgres"].Version; got != "fixed" { + t.Fatalf("postgres is %q; want the newer request's manifest", got) + } + + // A later request, and a manifest handed over by hand — asked when it is handed over — both + // replace it as before. + if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "postgres", Version: "later"}, + from(newer.Add(time.Minute))); err != nil { + t.Fatal(err) + } + if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "postgres", Version: "by-hand"}, Source{}); err != nil { + t.Fatal(err) + } + if shelf, _ := inv.Catalogue(ctx); shelf["postgres"].Version != "by-hand" { + t.Fatalf("postgres is %q; want the manifest handed over by hand", shelf["postgres"].Version) + } + src, err := inv.SourceOf(ctx, "postgres") + if err != nil || src.Repository != "novox/mesh-catalog" { + t.Fatalf("a hand registration erased the provenance: %+v %v", src, err) + } +} diff --git a/internal/link/build.go b/internal/link/build.go index 14c38ee..c7b0500 100644 --- a/internal/link/build.go +++ b/internal/link/build.go @@ -2,6 +2,9 @@ package link import ( "encoding/json" + "strconv" + "strings" + "time" ) // Asking a machine to build a module, and hearing what came out. @@ -17,6 +20,32 @@ import ( // holds no opinion about what they contain, and a host that also built things would be a host // with a container runtime requirement and a git dependency (novox/hq ADR 0005). +// NewBuildID is the correlation for a build asked at that moment: `build-`. +// +// **The id carries when the build was asked, and that is read back** (novox/hq 04-ISSUES/219). Builds +// of one module can be in flight together and finish in any order; what a module currently is must +// be the newest *request's* outcome, not the last one heard, and the id is the one thing every +// outcome echoes whichever builder answered it. One place writes the shape and one reads it. +func NewBuildID(asked time.Time) string { + return "build-" + strconv.FormatInt(asked.UnixNano(), 10) +} + +// BuildAskedAt is when the build with this id was asked, as NewBuildID wrote it. False for an id +// of any other shape — one written before this was read, or by hand — whose request time the mesh +// does not know. +func BuildAskedAt(id string) (time.Time, bool) { + digits, ok := strings.CutPrefix(id, "build-") + if !ok || digits == "" { + return time.Time{}, false + } + nanos, err := strconv.ParseInt(digits, 10, 64) + // A number too small to be a moment this mesh could have asked at is a name, not a time. + if err != nil || nanos < time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC).UnixNano() { + return time.Time{}, false + } + return time.Unix(0, nanos).UTC(), true +} + // BuildRequest is one module to build. type BuildRequest struct { // ID correlates the answer with the asking. Not the module name: two builds of one module can