Let a newer plan supersede the older open plans of its repository (hq issue 254, ADR 0218)
A merge planned without looking at open plans, so two plans worked the same modules and a stuck plan stayed open for ever. The newer plan folds in what older plans of the same repository and branch had not built or sent, and closes them as superseded. A person can close a stuck plan by id with `plans close <id>`.
This commit is contained in:
@@ -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 '';
|
||||
+22
-16
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user