diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 3efb023..74455b1 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -184,6 +184,7 @@ func planOfMerge(m link.SourceMoved, moved []string, edges []inventory.Edge) inv return inventory.Plan{ ID: fmt.Sprintf("plan-%d", time.Now().UnixNano()), Repository: m.Owner + "/" + m.Repo, + Branch: m.Base, Commit: m.Commit, Created: time.Now().UTC(), State: inventory.PlanBuilding, @@ -192,6 +193,57 @@ func planOfMerge(m link.SourceMoved, moved []string, edges []inventory.Edge) inv } } +// supersededBy is what a newer plan takes over from the open plans it supersedes (novox/hq issue +// 254, ADR 0218): the modules they had not finished, and those plans closed as superseded. +// +// **A merge looked at no plan but its own.** Two merges of one repository a few minutes apart were +// two open plans asking for the same modules, each sending machines what it built; and a plan that +// would never move again — waiting on a report that could not come, at 97b1b2b — stayed open for +// ever beside the newer ones, read as work in progress by everyone who looked. The newer merge is the +// newer intent for that repository and branch, so its plan takes over: every open plan of the same +// repository and branch **created before it** — by the time the plans were made, never by comparing +// commits, which have no order of their own — gives up the modules it had not built, and those are +// planned again in the newer plan beside what the newer merge moved. +// +// "Not built" is a module not yet asked, or asked and not answered; **and a module built and not +// yet sent to its machines**, where its policy rolls it out: closed, the older plan would never send +// it, and the catalogue announces no move for a rebuild (issue 189), so the newer plan builds and +// sends it. A build the older plan asked still finishes and registers as any build does — ordered by +// when it was asked (issue 219), so the newer plan's ask, made later, is the one that stands. +// +// A plan with no branch recorded is from before branches were kept, and is superseded by the next +// plan of its repository: what it had not built is folded in, so nothing is lost by it. +func supersededBy(newer inventory.Plan, open []inventory.Plan, rollsOut func(string) bool) ([]string, []inventory.Plan) { + folded := map[string]bool{} + var closed []inventory.Plan + for _, old := range open { + if old.ID == newer.ID || !old.Open() || !strings.EqualFold(old.Repository, newer.Repository) || + (old.Branch != "" && old.Branch != newer.Branch) || !old.Created.Before(newer.Created) { + continue + } + var took []string + for name, s := range old.Modules { + if s == nil || s.State != "built" || (s.SentAt == nil && rollsOut(name)) { + folded[name] = true + took = append(took, name) + } + } + sort.Strings(took) + old.State = inventory.PlanSuperseded + old.Note = fmt.Sprintf("superseded at tier %d by %s (%s at %s)", old.Tier, newer.ID, newer.Repository, short(newer.Commit)) + if len(took) > 0 { + old.Note += "; " + strings.Join(took, ", ") + " planned there again" + } + closed = append(closed, old) + } + out := make([]string, 0, len(folded)) + for name := range folded { + out = append(out, name) + } + sort.Strings(out) + return out, closed +} + // gates is what the next tier needs running from this one: a module of the tier that a later // tier is built by — the runtime dependency — and whose policy rolls it out, must be applied by // the machines running it before the next tier is asked. A base an image stands on need only be @@ -693,6 +745,8 @@ func planLine(p inventory.Plan, now time.Time) string { return fmt.Sprintf("%s %s done, %d tier(s)", p.Repository, short(p.Commit), len(p.Tiers)) case inventory.PlanFailed: return fmt.Sprintf("%s %s FAILED at %s: %s", p.Repository, short(p.Commit), where, p.Note) + case inventory.PlanSuperseded: + return fmt.Sprintf("%s %s %s", p.Repository, short(p.Commit), p.Note) } since := now.Sub(p.Updated).Round(time.Minute) late := "" @@ -823,7 +877,19 @@ func plansCommand(ctx context.Context, args []string) error { if *whatIf != "" { return planWhatIf(ctx, inv, *whatIf, splitList(*paths), splitList(*modules)) } - if len(positionals) == 2 && positionals[0] == "stop" { + // `stop`, or `close` (novox/hq issue 254): a person ending a plan that will not move again — one + // waiting on a report that cannot come — so it stops reading as work in progress. Marked failed + // with who ended it; what it asked still builds and registers. + if len(positionals) == 2 && (positionals[0] == "stop" || positionals[0] == "close") { + how := "stopped" + if positionals[0] == "close" { + how = "closed" + } + release, err := inv.HoldPlans(ctx, true) + if err != nil { + return err + } + defer release() p, err := inv.PlanByID(ctx, positionals[1]) if err != nil { return err @@ -832,25 +898,12 @@ func plansCommand(ctx context.Context, args []string) error { return fmt.Errorf("%s is already %s", p.ID, p.State) } p.State = inventory.PlanFailed - p.Note = "stopped by hand at tier " + fmt.Sprint(p.Tier) - release, err := inv.HoldPlans(ctx, true) - if err != nil { - return err - } - defer release() - if p, err = inv.PlanByID(ctx, positionals[1]); err != nil { - return err - } - if !p.Open() { - return fmt.Errorf("%s is already %s", p.ID, p.State) - } - p.State = inventory.PlanFailed - p.Note = "stopped by hand at tier " + fmt.Sprint(p.Tier) + p.Note = how + " by hand at tier " + fmt.Sprint(p.Tier) if err := inv.SavePlan(ctx, p); err != nil { return err } - fmt.Printf("%s stopped at tier %d of %d; what was asked still builds and registers, nothing further is asked\n", - p.ID, p.Tier, len(p.Tiers)) + fmt.Printf("%s %s at tier %d of %d; what was asked still builds and registers, nothing further is asked\n", + p.ID, how, p.Tier, len(p.Tiers)) return nil } plans, err := inv.RecentPlans(ctx, *limit) diff --git a/cmd/mesh-controller/seatverbs.go b/cmd/mesh-controller/seatverbs.go index 3591da5..d7dec42 100644 --- a/cmd/mesh-controller/seatverbs.go +++ b/cmd/mesh-controller/seatverbs.go @@ -100,6 +100,9 @@ func argvFor(verb string, args map[string]any) ([]string, error) { if id := str("stop"); id != "" { return []string{"plans", "stop", id}, nil } + if id := str("close"); id != "" { + return []string{"plans", "close", id}, nil + } if id := str("id"); id != "" { return []string{"plans", id}, nil } diff --git a/cmd/mesh-controller/supersede_test.go b/cmd/mesh-controller/supersede_test.go new file mode 100644 index 0000000..2c269fb --- /dev/null +++ b/cmd/mesh-controller/supersede_test.go @@ -0,0 +1,95 @@ +package main + +import ( + "reflect" + "strings" + "testing" + "time" + + "github.com/novox/mesh-controller/internal/inventory" +) + +// novox/hq issue 254, ADR 0218: a newer plan takes over what the older open plans of its repository +// and branch had not built, and closes them as superseded; another repository's plan, another +// branch's, and a plan made after it are left alone. +func TestANewerPlanSupersedesTheOlderOpenPlansOfItsRepository(t *testing.T) { + at := time.Date(2026, 10, 5, 12, 0, 0, 0, time.UTC) + sent := at.Add(time.Minute) + plan := func(id, repository, branch string, created time.Time, modules map[string]*inventory.PlanModule) inventory.Plan { + return inventory.Plan{ID: id, Repository: repository, Branch: branch, Commit: id + "-commit", + Created: created, State: inventory.PlanRolling, Modules: modules} + } + older := plan("plan-1", "novox/mesh-catalog", "main", at, map[string]*inventory.PlanModule{ + "gitea": {State: "built", SentAt: &sent}, // done with: stays done + "keycloak": {State: "asked"}, // asked, not answered: folded + "plex": {}, // not yet asked: folded + "agent": {State: "built"}, // built, rolls out, not sent: folded + "notes": {State: "built"}, // built, records: nothing to send + }) + stuck := plan("plan-0", "Novox/Mesh-Catalog", "", at.Add(-time.Hour), map[string]*inventory.PlanModule{ + "runtime": {State: "asked"}, + }) + other := plan("plan-2", "novox/mesh-controller", "main", at, map[string]*inventory.PlanModule{"mesh-controller": {}}) + release := plan("plan-3", "novox/mesh-catalog", "release", at, map[string]*inventory.PlanModule{"lemurs": {}}) + later := plan("plan-5", "novox/mesh-catalog", "main", at.Add(2*time.Hour), map[string]*inventory.PlanModule{"later": {}}) + done := plan("plan-6", "novox/mesh-catalog", "main", at, map[string]*inventory.PlanModule{"finished": {}}) + done.State = inventory.PlanDone + + newer := plan("plan-4", "novox/mesh-catalog", "main", at.Add(time.Hour), nil) + newer.Commit = "97b1b2b0c0ffee" + rollsOut := func(m string) bool { return m != "notes" } + folded, closed := supersededBy(newer, []inventory.Plan{stuck, older, other, release, later, done, newer}, rollsOut) + + if want := []string{"agent", "keycloak", "plex", "runtime"}; !reflect.DeepEqual(folded, want) { + t.Fatalf("folded %v, wanted %v", folded, want) + } + var ids []string + for _, p := range closed { + ids = append(ids, p.ID) + if p.State != inventory.PlanSuperseded || p.Open() { + t.Errorf("%s was left %s", p.ID, p.State) + } + if !strings.Contains(p.Note, "plan-4") || !strings.Contains(p.Note, "97b1b2b0") { + t.Errorf("%s does not name the plan that superseded it: %q", p.ID, p.Note) + } + } + if want := []string{"plan-0", "plan-1"}; !reflect.DeepEqual(ids, want) { + t.Fatalf("superseded %v, wanted %v — another repository, another branch, a later plan and a "+ + "finished one are left alone", ids, want) + } + if other.State != inventory.PlanRolling { + t.Fatal("the plan handed in was changed in place") + } + if line := planLine(closed[1], time.Now()); !strings.Contains(line, "superseded") { + t.Fatalf("a superseded plan reads %q", line) + } +} + +// novox/hq issue 254: a person closes a plan that will not move again, by its id. +func TestAPersonClosesAStuckPlan(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + stuck := inventory.Plan{ID: "plan-97b1b2b", Repository: "novox/mesh-catalog", Commit: "97b1b2b", + Created: time.Now().UTC(), State: inventory.PlanRolling, Tier: 1, Tiers: [][]string{{"a"}, {"b"}}, + Modules: map[string]*inventory.PlanModule{"a": {State: "built"}, "b": {}}} + if err := open.inventory.SavePlan(ctx, stuck); err != nil { + t.Fatal(err) + } + if err := plansCommand(ctx, []string{"close", stuck.ID}); err != nil { + t.Fatal(err) + } + closed, err := open.inventory.PlanByID(ctx, stuck.ID) + if err != nil { + t.Fatal(err) + } + if closed.State != inventory.PlanFailed || !strings.Contains(closed.Note, "closed by hand") { + t.Fatalf("the plan was left %s: %q", closed.State, closed.Note) + } + if err := plansCommand(ctx, []string{"close", stuck.ID}); err == nil { + t.Fatal("a plan already closed was closed again") + } + if argv, err := argvFor("plans", map[string]any{"close": stuck.ID}); err != nil || + !reflect.DeepEqual(argv, []string{"plans", "close", stuck.ID}) { + t.Fatalf("the seat's verb does not close a plan: %v %v", argv, err) + } +} diff --git a/cmd/mesh-controller/upgrades.go b/cmd/mesh-controller/upgrades.go index f82ccb2..50389b6 100644 --- a/cmd/mesh-controller/upgrades.go +++ b/cmd/mesh-controller/upgrades.go @@ -324,6 +324,45 @@ func (f following) SourceMoved(ctx context.Context, m link.SourceMoved) error { } defer release() plan := planOfMerge(m, movedNames, edges) + // **A newer plan supersedes the older open plans of this repository and branch** (novox/hq issue + // 254, ADR 0218): what they had not built is planned here again, and they are closed, so one plan + // works a repository's modules at a time and a stuck one ends at the next merge. + working, err := inv.OpenPlans(ctx) + if err != nil { + return notNow(err) + } + rollsOut := func(module string) bool { + u, err := inv.UpgradeOf(ctx, module) + return err == nil && u.RollOut + } + folded, superseded := supersededBy(plan, working, rollsOut) + if len(folded) > 0 { + held := map[string]bool{} + for _, e := range entries { + held[e.Manifest.Module] = true + } + names := map[string]bool{} + for _, name := range movedNames { + names[name] = true + } + var also []string + for _, name := range folded { + // One the catalogue no longer holds would fail the newer plan's ask; it is not this + // merge's to build. + if held[name] && !names[name] { + names[name] = true + movedNames = append(movedNames, name) + also = append(also, name) + } + } + if len(also) > 0 { + again := planOfMerge(m, movedNames, edges) + again.ID, again.Created = plan.ID, plan.Created + plan = again + fmt.Printf(" %s, left unbuilt by an older plan of %s, are planned here again\n", + strings.Join(also, ", "), plan.Repository) + } + } if hasCycle(plan.Tiers, edges) { fmt.Printf(" the last tier depends on itself: %s — built together, in no order\n", strings.Join(plan.Tiers[len(plan.Tiers)-1], ", ")) @@ -331,6 +370,14 @@ func (f following) SourceMoved(ctx context.Context, m link.SourceMoved) error { if err := inv.SavePlan(ctx, plan); err != nil { return notNow(err) } + // Closed after the newer plan is kept, never before: a controller replaced between the two leaves + // both open, which the next merge settles, rather than neither. + for _, old := range superseded { + if err := inv.SavePlan(ctx, old); err != nil { + return notNow(err) + } + fmt.Printf(" %s (%s at %s) is %s\n", old.ID, old.Repository, short(old.Commit), old.Note) + } var tiers []string for i, t := range plan.Tiers { tiers = append(tiers, fmt.Sprintf("%d: %s", i, strings.Join(t, ", "))) diff --git a/internal/catalogue/verbs.go b/internal/catalogue/verbs.go index f4a1352..8898585 100644 --- a/internal/catalogue/verbs.go +++ b/internal/catalogue/verbs.go @@ -96,6 +96,7 @@ var ControllerVerbs = []Verb{ Input: schema(map[string]string{ "id": "a plan's id (as `plans` lists them): that plan, tier by tier", "stop": "a plan's id: stop it — what was asked still builds, nothing further is asked", + "close": "a plan's id: close a plan that will not move again, as failed by hand (novox/hq issue 254)", "repository": "owner/repository: the plan a merge there would produce, saving nothing (what-if); with paths or modules", "paths": "with repository: the files the merge would change, comma-separated, from the repository's root", "modules": "with repository: or the modules it would change, comma-separated", diff --git a/internal/inventory/migrations/0057-a-newer-plan-supersedes-an-older.sql b/internal/inventory/migrations/0057-a-newer-plan-supersedes-an-older.sql new file mode 100644 index 0000000..8a83eb6 --- /dev/null +++ b/internal/inventory/migrations/0057-a-newer-plan-supersedes-an-older.sql @@ -0,0 +1,14 @@ +-- A newer plan supersedes the older open plans of the same repository and branch (novox/hq issue 254, +-- ADR 0218). +-- +-- A merge produced a plan without looking at the plans still open, so two merges a few minutes apart +-- were two plans working the same modules, and a plan stuck waiting on something that would never +-- come stayed open for ever beside the newer ones. The newer plan now takes over what the older had +-- not yet built and the older is closed as `superseded` — a state of its own, so `plans` can say +-- which plan replaced it rather than reading as a failure. +-- +-- `branch` is the branch the merge went into, so only a plan of the same branch is superseded. Empty +-- for every plan from before this was kept: which branch it answered is not known, and such a plan +-- is superseded by the next plan of its repository, whichever branch — nothing is lost by it, since +-- what it had not built is folded into the plan that supersedes it. +alter table release_plan add column branch text not null default ''; diff --git a/internal/inventory/plans.go b/internal/inventory/plans.go index 170ef31..cf90380 100644 --- a/internal/inventory/plans.go +++ b/internal/inventory/plans.go @@ -15,16 +15,19 @@ import ( // the store so a controller replaced mid-plan resumes it, and so `status` can say what a merge // still waits for. type Plan struct { - ID string `json:"id"` - Repository string `json:"repository"` - Commit string `json:"commit"` - Created time.Time `json:"created"` - Updated time.Time `json:"updated"` - State string `json:"state"` - Tier int `json:"tier"` - Tiers [][]string `json:"tiers"` - Modules map[string]*PlanModule `json:"modules"` - Note string `json:"note,omitempty"` + ID string `json:"id"` + Repository string `json:"repository"` + // Branch is the branch the merge went into (novox/hq issue 254): a newer plan supersedes the open + // ones of the same repository and branch. Empty for a plan from before it was kept. + Branch string `json:"branch,omitempty"` + Commit string `json:"commit"` + Created time.Time `json:"created"` + Updated time.Time `json:"updated"` + State string `json:"state"` + Tier int `json:"tier"` + Tiers [][]string `json:"tiers"` + Modules map[string]*PlanModule `json:"modules"` + Note string `json:"note,omitempty"` } // PlanModule is one module's state within a plan. @@ -54,6 +57,9 @@ const ( PlanRolling = "rolling" PlanDone = "done" PlanFailed = "failed" + // PlanSuperseded is a plan a newer merge of the same repository and branch took over (novox/hq + // issue 254, ADR 0218): what it had not built is in the newer plan, and its note names it. + PlanSuperseded = "superseded" ) // Open says whether the plan is still being worked. @@ -70,11 +76,11 @@ func (i *Inventory) SavePlan(ctx context.Context, p Plan) error { return err } _, err = i.store.Pool().Exec(ctx, - `insert into release_plan (id, repository, commit_hash, created, updated, state, tier, tiers, modules, note) - values ($1, $2, $3, $4, now(), $5, $6, $7, $8, $9) + `insert into release_plan (id, repository, commit_hash, created, updated, state, tier, tiers, modules, note, branch) + values ($1, $2, $3, $4, now(), $5, $6, $7, $8, $9, $10) on conflict (id) do update set updated = now(), state = excluded.state, tier = excluded.tier, - tiers = excluded.tiers, modules = excluded.modules, note = excluded.note`, - p.ID, p.Repository, p.Commit, p.Created, p.State, p.Tier, tiers, modules, p.Note) + tiers = excluded.tiers, modules = excluded.modules, note = excluded.note, branch = excluded.branch`, + p.ID, p.Repository, p.Commit, p.Created, p.State, p.Tier, tiers, modules, p.Note, p.Branch) return err } @@ -102,7 +108,7 @@ func (i *Inventory) PlanByID(ctx context.Context, id string) (Plan, error) { func (i *Inventory) plans(ctx context.Context, tail string) ([]Plan, error) { rows, err := i.store.Pool().Query(ctx, - `select id, repository, commit_hash, created, updated, state, tier, tiers, modules, note + `select id, repository, commit_hash, created, updated, state, tier, tiers, modules, note, branch from release_plan `+tail) if err != nil { return nil, err @@ -113,7 +119,7 @@ func (i *Inventory) plans(ctx context.Context, tail string) ([]Plan, error) { var p Plan var tiers, modules []byte if err := rows.Scan(&p.ID, &p.Repository, &p.Commit, &p.Created, &p.Updated, &p.State, - &p.Tier, &tiers, &modules, &p.Note); err != nil { + &p.Tier, &tiers, &modules, &p.Note, &p.Branch); err != nil { return nil, err } if err := json.Unmarshal(tiers, &p.Tiers); err != nil { diff --git a/internal/inventory/plans_test.go b/internal/inventory/plans_test.go index a9153a6..e4804a7 100644 --- a/internal/inventory/plans_test.go +++ b/internal/inventory/plans_test.go @@ -46,3 +46,30 @@ func TestAPlanIsKeptAdvancedAndResumedFromTheStore(t *testing.T) { t.Fatalf("a done plan is still among the recent ones: %+v", recent) } } + +// novox/hq issue 254: a plan keeps the branch its merge went into, and a superseded plan is not open. +func TestASupersededPlanIsNotOpen(t *testing.T) { + inv := ForTest(t) + ctx := t.Context() + p := Plan{ID: "plan-1", Repository: "novox/mesh-catalog", Branch: "main", Commit: "abc", + Created: time.Now().UTC(), State: PlanBuilding, Tiers: [][]string{{"gitea"}}, + Modules: map[string]*PlanModule{"gitea": {}}} + if err := inv.SavePlan(ctx, p); err != nil { + t.Fatal(err) + } + kept, err := inv.PlanByID(ctx, "plan-1") + if err != nil || kept.Branch != "main" { + t.Fatalf("the branch was not kept: %v %+v", err, kept) + } + kept.State = PlanSuperseded + kept.Note = "superseded at tier 0 by plan-2" + if err := inv.SavePlan(ctx, kept); err != nil { + t.Fatal(err) + } + if open, err := inv.OpenPlans(ctx); err != nil || len(open) != 0 { + t.Fatalf("a superseded plan is still open: %v %+v", err, open) + } + if recent, _ := inv.RecentPlans(ctx, 5); len(recent) != 1 || recent[0].State != PlanSuperseded { + t.Fatalf("a superseded plan is not among the recent ones as superseded: %+v", recent) + } +}