From 149f89e8f9bfd2dbac28d652295374abad16db83 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 12:13:33 +0200 Subject: [PATCH] Keep every refused registration by its build id, so a Retry never settles it built A build heard and recorded and not registered kept its refusal only when a plan was open or the plans were held. A tier's first failure fails its plan, so later refusals were lost; a Retry then settled those builds built from their records, sent the modules' old versions and reported the plan done. A build without a readable manifest never reached its plan at all. Keep the refusal in the store and the process whatever the plans' state, and fail the asking plan by the build's id when the module is not known. Issue 1713. --- cmd/mesh-controller/main.go | 9 +- cmd/mesh-controller/take_refused.go | 48 +++++++++ cmd/mesh-controller/take_refused_test.go | 119 +++++++++++++++++++++++ 3 files changed, 173 insertions(+), 3 deletions(-) create mode 100644 cmd/mesh-controller/take_refused.go create mode 100644 cmd/mesh-controller/take_refused_test.go diff --git a/cmd/mesh-controller/main.go b/cmd/mesh-controller/main.go index 4382d52a..17189ffe 100644 --- a/cmd/mesh-controller/main.go +++ b/cmd/mesh-controller/main.go @@ -448,9 +448,12 @@ func (b builds) Built(ctx context.Context, result link.BuildResult) error { 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(), asked, result.ID) - } + takeRefused(ctx, b.inv, result, manifest.Module, err.Error(), refusedOutcome{ + built: func(module, why string) { + planBuilt(ctx, b.open, module, result.Commit, why, asked, result.ID) + }, + failedBuild: func(failed link.BuildResult) { planFailedBuild(ctx, b.open, failed) }, + }) return nil } fmt.Printf("%s: %s %s registered, built on %s from %s\n", diff --git a/cmd/mesh-controller/take_refused.go b/cmd/mesh-controller/take_refused.go new file mode 100644 index 00000000..2d31edab --- /dev/null +++ b/cmd/mesh-controller/take_refused.go @@ -0,0 +1,48 @@ +package main + +import ( + "context" + "fmt" + "time" + + "github.com/novox/mesh-controller/internal/link" +) + +// refusalKeeper keeps a refused registration in the store (*inventory.Inventory, or a test's). +type refusalKeeper interface { + RecordRefused(ctx context.Context, id, module, why string) error +} + +// refusedOutcome is how a refused build reaches the plans: built takes it in by its module (planBuilt), failedBuild +// by the id the plan asked it under (planFailedBuild). +type refusedOutcome struct { + built func(module, why string) + failedBuild func(result link.BuildResult) +} + +// takeRefused takes in a build heard and recorded, and not registered (novox/hq issue 1713). +// +// **The refusal is kept by the build's id, whatever the plans' state**: in the store (RecordRefused) and in this +// process (keepRefusalFor). Taking it into the plans keeps it only when a plan is open or the plans are held, and a +// tier's first failure fails its plan, which no outcome reaches after that: the build record says the builder's words, +// which worked, and a Retry settled from it sent the module's old version and reported the plan done (on 2026-10-23 +// 12:01, plan-1792748906937927069 sent mesh-build-go, mesh-build-node, mosquitto and ssh-client so). +// +// **A build whose module is not known fails the plan that asked it by its id** (planFailedBuild, moduleAskedAs), as +// a failed build that names no module does: a manifest that could not be read is no reason for the plan not to hear. +func takeRefused(ctx context.Context, keeper refusalKeeper, result link.BuildResult, module, why string, + plans refusedOutcome) { + if result.ID != "" { + if err := keeper.RecordRefused(ctx, result.ID, module, why); err != nil { + fmt.Printf("%s: the refusal could not be kept in the store, only in this controller: %v\n", result.ID, err) + } + keepRefusalFor(result.ID, why, time.Now()) + } + if module != "" { + plans.built(module, why) + return + } + failed := result + failed.Failed = why + plans.failedBuild(failed) +} diff --git a/cmd/mesh-controller/take_refused_test.go b/cmd/mesh-controller/take_refused_test.go new file mode 100644 index 00000000..8e7cc6b0 --- /dev/null +++ b/cmd/mesh-controller/take_refused_test.go @@ -0,0 +1,119 @@ +package main + +import ( + "context" + "reflect" + "strings" + "testing" + "time" + + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// keptRefusals is a store that keeps refusals, by id. +type keptRefusals map[string]string + +func (k keptRefusals) RecordRefused(_ context.Context, id, _, why string) error { + if _, ok := k[id]; !ok { + k[id] = why + } + return nil +} + +// failedPlanOf is a plan already failed on gitea, its tier also asking module under id: open() does not count it, so +// no outcome taken in reaches it (as plan-1792748906937927069 on 2026-10-23 11:51). +func failedPlanOf(module, id string, asked time.Time) inventory.Plan { + return inventory.Plan{ID: "plan-1713", State: inventory.PlanFailed, Tier: 0, + Tiers: [][]string{{"gitea", module}}, Note: "gitea failed to build in tier 0", + Modules: map[string]*inventory.PlanModule{ + "gitea": {State: "failed", AskedAt: &asked, Build: "build-1713-gitea", Why: "not registered"}, + module: {State: "asked", AskedAt: &asked, Build: id}, + }} +} + +// retried is what a Retry of the plan asks again, its records read as recordsOfAsked reads them: the build record of +// id (which says the builder's words, worked) with every refusal kept for it in the store and in this process. +func retried(p *inventory.Plan, store keptRefusals, id, module string, at time.Time) []string { + byID := map[string]inventory.Build{id: {ID: id, Module: module, Commit: "6c0024e4", At: at}} + refused := map[string]string{} + for k, why := range store { + refused[k] = why + } + for k, why := range refusalsKeptFor([]string{id}) { + refused[k] = why + } + overlayRefusals(nil, byID, refused) + return toRetry(p, nil, byID) +} + +// 1713/AC1: given a build of a module heard and recorded and not registered while its plan is failed and the plans are +// not held, when the controller takes it in, then the refusal is kept by id, and a Retry asks the module again instead +// of settling it built and sending its old version (plan-1792748906937927069 sent mosquitto's on 2026-10-23 12:01). +func TestARefusalWhileThePlanIsFailedIsKeptAndRetried(t *testing.T) { + asked := time.Date(2026, 10, 23, 11, 51, 0, 0, time.UTC) + id := "build-1713-ac1-mosquitto" + p := failedPlanOf("mosquitto", id, asked) + store := keptRefusals{} + var took []string + takeRefused(t.Context(), store, link.BuildResult{ID: id, Commit: "6c0024e4"}, "mosquitto", + "the trunk requires no status", refusedOutcome{ + built: func(module, why string) { took = append(took, module+": "+why) }, + failedBuild: func(link.BuildResult) { t.Fatal("a refusal with its module was taken by id") }, + }) + + if !reflect.DeepEqual(took, []string{"mosquitto: the trunk requires no status"}) { + t.Fatalf("the plans were told %v", took) + } + if store[id] == "" { + t.Fatal("the refusal was not kept in the store") + } + if got := retried(&p, keptRefusals{}, id, "mosquitto", asked.Add(time.Minute)); !reflect.DeepEqual(got, + []string{"gitea", "mosquitto"}) { + t.Fatalf("a Retry, the refusal kept only in this controller, asks %v; mosquitto was settled %q", + got, p.Modules["mosquitto"].State) + } +} + +// 1713/AC2: given a build that came back without a readable manifest and was not registered, when the controller takes +// it in, then the asking plan is failed by the build's id and the refusal is kept by id, so a Retry asks it again. +func TestARefusalWithoutAModuleFailsItsPlanByIDAndIsRetried(t *testing.T) { + asked := time.Date(2026, 10, 23, 11, 51, 0, 0, time.UTC) + id := "build-1713-ac2-ssh-client" + p := failedPlanOf("ssh-client", id, asked) + store := keptRefusals{} + var failed []link.BuildResult + takeRefused(t.Context(), store, link.BuildResult{ID: id, Commit: "6c0024e4"}, "", + "what came back is not a manifest", refusedOutcome{ + built: func(module, _ string) { t.Fatalf("a refusal without a module was taken as %q", module) }, + failedBuild: func(r link.BuildResult) { failed = append(failed, r) }, + }) + + if len(failed) != 1 || failed[0].ID != id || !strings.Contains(failed[0].Failed, "not a manifest") { + t.Fatalf("the asking plan was not failed by the build's id: %+v", failed) + } + if store[id] == "" { + t.Fatal("the refusal was not kept in the store") + } + if got := retried(&p, store, id, "", asked.Add(time.Minute)); !reflect.DeepEqual(got, + []string{"gitea", "ssh-client"}) { + t.Fatalf("a Retry asks %v; ssh-client was settled %q", got, p.Modules["ssh-client"].State) + } +} + +// 1713/AC3: given a refused build kept by id, when the records settle a building plan's tier, then the module and the +// plan are failed, never built: the plan never reports done for it. +func TestAPlanNeverSettlesARefusedBuildBuilt(t *testing.T) { + asked := time.Date(2026, 10, 23, 11, 51, 0, 0, time.UTC) + id := "build-1713-ac3-mesh-build-go" + takeRefused(t.Context(), keptRefusals{}, link.BuildResult{ID: id}, "mesh-build-go", "not registered", + refusedOutcome{built: func(string, string) {}, failedBuild: func(link.BuildResult) {}}) + p := inventory.Plan{ID: "plan-1713-ac3", State: inventory.PlanBuilding, Tiers: [][]string{{"mesh-build-go"}}, + Modules: map[string]*inventory.PlanModule{"mesh-build-go": {State: "asked", AskedAt: &asked, Build: id}}} + byID := map[string]inventory.Build{id: {ID: id, Module: "mesh-build-go", At: asked.Add(time.Minute)}} + overlayRefusals(nil, byID, refusalsKeptFor([]string{id})) + settleFromRecords(&p, p.Tiers[0], nil, byID) + if s := p.Modules["mesh-build-go"]; s.State != "failed" || p.State != inventory.PlanFailed { + t.Fatalf("a refused build settled as %q, the plan %q", s.State, p.State) + } +}