diff --git a/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go b/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go index 8ec081e4..ef43241b 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go @@ -474,3 +474,197 @@ func TestAMemberHeldForItsComposedCheckIsLiftedWhenItPasses(t *testing.T) { }) } } + +// **A module a release carried to every machine the held delivery sends it to covers it** (novox/hq issue +// 351). A build the controller held back for a person and released on their word reaches each machine by a +// backlog walk, which credits the delivery that published the build one step per machine — never the walk's +// own "every machine running it" or "the rest running it". The held delivery's plan says which machines +// receive the module: a passed step on each of them is the module sent everywhere. A machine still judging, +// one that failed, or a plan that names no machines leaves it held, as before. +func TestAModuleAReleaseCarriedToEveryMachineCovers(t *testing.T) { + const cat = "novox/mesh-catalog" + merged1, merged2 := t0, t0.Add(2*time.Hour) + laptop, desktop := "laptop", "desktop" + plan := func(machines ...string) *DeliveryPlan { + p := planMoving(cat, "openrazer") + for _, m := range machines { + p.Machines = append(p.Machines, MachinePlan{Machine: m, Receives: []string{"openrazer"}}) + } + return p + } + held := func(p *DeliveryPlan) *Delivery { + why := "its group feat/j's composed check did not pass for the heads that merged; a person decides that it goes on" + return &Delivery{ID: IDOf(cat, head), Repository: cat, Commit: head, Number: 7, Base: "main", State: Held, + Since: merged1.Add(10 * time.Minute), Created: merged1.Add(-time.Hour), MergedAs: merge, MergedAt: merged1, + HeldWhy: why, Plan: p, Walk: &WalkSeen{ID: "plan-7", Commit: merge, State: walkSuperseded, Waits: true, WaitedFor: "mesh-delivery"}, + Transitions: []Transition{{At: merged1, From: Published, To: Held, Event: EvHold, Why: why}}} + } + step := func(machine string, state StepState) Step { + return Step{Machine: machine, Module: "openrazer", Build: "build-1", State: state, Why: "carried by release-1"} + } + coverer := func(steps ...Step) *Delivery { + return &Delivery{ID: IDOf(cat, "c0c0c0c0c0c0c0c0"), Repository: cat, Commit: "c0c0c0c0c0c0c0c0", Number: 90, + Base: "main", State: Delivered, Since: merged2.Add(time.Hour), Created: merged2.Add(-time.Hour), + MergedAs: "f0f0f0f0f0f0f0f0", MergedAt: merged2, Plan: planMoving(cat, "power"), Check: &Verdict{Gate: "pass"}, + Walk: &WalkSeen{ID: "plan-90", Commit: "f0f0f0f0f0f0f0f0", State: walkDone}, + Steps: append([]Step{{Machine: "anchor", Module: "power", State: StepPassed}, + {Machine: "the rest running it", Module: "power", State: StepSent}}, steps...)} + } + for _, c := range []struct { + name string + held *Delivery + by *Delivery + want State + }{ + {"passed on every machine its plan sends it to", held(plan(laptop, desktop)), + coverer(step(laptop, StepPassed), step(desktop, StepPassed)), Covered}, + {"still judging on one of them", held(plan(laptop, desktop)), + coverer(step(laptop, StepPassed), step(desktop, StepJudging)), Held}, + {"failed on one of them", held(plan(laptop, desktop)), + coverer(step(laptop, StepPassed), step(desktop, StepFailed)), Held}, + {"never sent to one of them", held(plan(laptop, desktop)), coverer(step(laptop, StepPassed)), Held}, + {"its plan names no machines", held(plan()), coverer(step(laptop, StepPassed), step(desktop, StepPassed)), Held}, + } { + t.Run(c.name, func(t *testing.T) { + store := newMemStore() + for _, d := range []*Delivery{c.held, c.by} { + if err := store.PutDelivery(d); err != nil { + t.Fatal(err) + } + } + forge := newFakeForge() + forge.contains[merge+" in f0f0f0f0f0f0f0f0"] = true + w := newWorldOver(t, store, newFakeController(), forge, merged2.Add(30*time.Hour)) + if err := w.h.Reconcile(); err != nil { + t.Fatal(err) + } + w.settleAll() + d := w.delivery(c.held.ID) + if d.State != c.want { + t.Fatalf("it is %s (%s), want %s", d.State, d.HeldWhy, c.want) + } + if c.want == Covered { + if s := statusOn(w, cat, head); !strings.HasPrefix(s, "success covered: a later merge that contains it was delivered") { + t.Fatalf("its pull request's mesh/delivery says %q", s) + } + if last := d.Transitions[len(d.Transitions)-1]; !strings.Contains(last.Why, "openrazer") { + t.Fatalf("its transition does not name the module: %q", last.Why) + } + } + }) + } +} + +// **A finished release moves the steps it carried to passed** (novox/hq issue 351): a backlog walk's gate +// credits a step per machine while it judges, and its last record keeps no gate — only which machines it is +// done on. A step credited to it, on a machine it is done on, is passed; a step it failed on, one on a machine +// it is not done on, and a step another walk carried are left as they are. +func TestAFinishedReleaseMovesTheStepsItCarriedToPassed(t *testing.T) { + const cat = "novox/mesh-catalog" + carried := func(machine string, state StepState, by string) Step { + return Step{Machine: machine, Module: "openrazer", Build: "build-1", State: state, Why: "carried by " + by} + } + for _, c := range []struct { + name string + steps []Step + done []string + want map[string]StepState + }{ + {"done on both", []Step{carried("laptop", StepJudging, "release-1"), carried("desktop", StepJudging, "release-1")}, + []string{"laptop", "desktop"}, map[string]StepState{"laptop": StepPassed, "desktop": StepPassed}}, + {"done on one, the other still to come", []Step{carried("laptop", StepJudging, "release-1"), carried("desktop", StepJudging, "release-1")}, + []string{"laptop"}, map[string]StepState{"laptop": StepPassed, "desktop": StepJudging}}, + {"a step it failed on stays failed", []Step{carried("laptop", StepFailed, "release-1")}, + []string{"laptop"}, map[string]StepState{"laptop": StepFailed}}, + {"a step another walk carried is not its", []Step{carried("laptop", StepJudging, "release-2")}, + []string{"laptop"}, map[string]StepState{"laptop": StepJudging}}, + } { + t.Run(c.name, func(t *testing.T) { + d := &Delivery{ID: IDOf(cat, "c0c0c0c0c0c0c0c0"), Repository: cat, Commit: "c0c0c0c0c0c0c0c0", Number: 90, + Base: "main", State: Delivered, Since: t0.Add(time.Hour), Created: t0, MergedAs: "f0f0f0f0f0f0f0f0", MergedAt: t0, + Plan: planMoving(cat, "power"), Check: &Verdict{Gate: "pass"}, + Walk: &WalkSeen{ID: "plan-90", Commit: "f0f0f0f0f0f0f0f0", State: walkDone}, Steps: c.steps} + store := newMemStore() + if err := store.PutDelivery(d); err != nil { + t.Fatal(err) + } + w := newWorldOver(t, store, newFakeController(), newFakeForge(), t0.Add(2*time.Hour)) + w.h.WalkMoved(Walk{ID: "release-1", Repository: "the builds waiting for a gate", State: walkDone, + Tiers: [][]string{{"openrazer"}}, Modules: map[string]*WalkModule{}, + Release: &WalkRelease{Order: []string{"laptop", "desktop"}, Next: len(c.done), Done: c.done}}) + got := map[string]StepState{} + for _, s := range w.delivery(d.ID).Steps { + got[s.Machine] = s.State + } + for machine, want := range c.want { + if got[machine] != want { + t.Fatalf("openrazer on %s is %s, want %s (steps %v)", machine, got[machine], want, got) + } + } + }) + } +} + +// The day's case whole (novox/hq issue 351): a member held for its group's check, its own walk taken over by +// later merges; the one later walk that carried its module failed its gate; a later merge's walk was delivered +// without the module; the controller then held the module's build back for a person, who released it, and the +// release reached both machines. The held delivery is covered by that later merge once the release is done — +// read by its id, since a walk that ended before the holder's restart may be older than the ended walks the +// controller lists back. +func TestAHeldDeliveryWhoseModuleAReleaseCarriedIsCoveredOnceTheReleaseIsDone(t *testing.T) { + const cat = "novox/mesh-catalog" + merged1, merged2 := t0, t0.Add(2*time.Hour) + plan := planMoving(cat, "openrazer") + plan.Machines = []MachinePlan{{Machine: "laptop", Receives: []string{"openrazer"}}, {Machine: "desktop", Receives: []string{"openrazer"}}} + why := "its group feat/j's composed check did not pass for the heads that merged; a person decides that it goes on" + held := &Delivery{ID: IDOf(cat, head), Repository: cat, Commit: head, Number: 7, Base: "main", State: Held, + Since: merged1.Add(10 * time.Minute), Created: merged1.Add(-time.Hour), MergedAs: merge, MergedAt: merged1, + HeldWhy: why, Plan: plan, Walk: &WalkSeen{ID: "plan-7", Commit: merge, State: walkSuperseded, Waits: true, WaitedFor: "mesh-delivery"}, + Transitions: []Transition{{At: merged1, From: Published, To: Held, Event: EvHold, Why: why}}} + later := &Delivery{ID: IDOf(cat, "c0c0c0c0c0c0c0c0"), Repository: cat, Commit: "c0c0c0c0c0c0c0c0", Number: 90, + Base: "main", State: Delivered, Since: merged2.Add(time.Hour), Created: merged2.Add(-time.Hour), + MergedAs: "f0f0f0f0f0f0f0f0", MergedAt: merged2, Plan: planMoving(cat, "power"), Check: &Verdict{Gate: "pass"}, + Walk: &WalkSeen{ID: "plan-90", Commit: "f0f0f0f0f0f0f0f0", State: walkDone}, + Steps: []Step{{Machine: "anchor", Module: "power", State: StepPassed}, {Machine: "the rest running it", Module: "power", State: StepSent}}} + store := newMemStore() + for _, d := range []*Delivery{held, later} { + if err := store.PutDelivery(d); err != nil { + t.Fatal(err) + } + } + forge := newFakeForge() + forge.contains[merge+" in f0f0f0f0f0f0f0f0"] = true + w := newWorldOver(t, store, newFakeController(), forge, merged2.Add(3*time.Hour)) + if err := w.h.Reconcile(); err != nil { + t.Fatal(err) + } + w.settleAll() + if s := w.state(held.ID); s != Held { + t.Fatalf("before any release it is %s", s) + } + // The release judges the build on each machine in turn: its gate names what it carried. + release := Walk{ID: "release-1", Repository: "the builds waiting for a gate", State: walkRolling, + Tiers: [][]string{{"openrazer"}}, Modules: map[string]*WalkModule{}, + Release: &WalkRelease{Order: []string{"laptop", "desktop"}, Gate: &WalkGate{Machines: []string{"laptop"}, + Carried: []WalkCarry{{Module: "openrazer", Node: "laptop", To: "f0f0f0f0f0f0f0f0", Build: "build-1"}, + {Module: "openrazer", Node: "desktop", To: "f0f0f0f0f0f0f0f0", Build: "build-1"}}}}} + w.h.WalkMoved(release) + w.settleAll() + if s := w.state(held.ID); s != Held { + t.Fatalf("while the release judges it is %s", s) + } + // Done on both: its last record keeps no gate. It ended while this holder was down, and is older than the + // ended walks the controller lists back: the holder asks for it by its id, since a step it carried still judges. + release.State, release.Release.Gate, release.Release.Done, release.Release.Next = walkDone, nil, []string{"laptop", "desktop"}, 2 + w.ctl.put(release) + w.later(time.Hour) // a walk a step still judges is asked for once an hour; at a start, at once + w.settleAll() + d := w.delivery(held.ID) + if d.State != Covered { + t.Fatalf("once the release is done it is %s (%s), want covered", d.State, d.HeldWhy) + } + if last := d.Transitions[len(d.Transitions)-1]; !strings.Contains(last.Why, IDOf(cat, "c0c0c0c0c0c0c0c0")) || + !strings.Contains(last.Why, "openrazer") { + t.Fatalf("its transition does not name what covered it: %q", last.Why) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index 0ed12ca6..5cbe008c 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -40,7 +40,9 @@ type Holder struct { unreached string // askTried is when a check this owner asks was last tried and could not be asked: tried again a minute on. askTried map[string]time.Time - refusals []string + // walkAsked is when a walk that carried a step still judging was last asked for by its id: once an hour. + walkAsked map[string]time.Time + refusals []string // unchecked are the open pull requests' heads seen without a merge check (unchecked.go). unchecked map[string]unchecked } @@ -82,6 +84,7 @@ func (h *Holder) init() { h.dirtyGroup = map[string]bool{} h.unmatched = map[string]unmatched{} h.askTried = map[string]time.Time{} + h.walkAsked = map[string]time.Time{} h.unchecked = map[string]unchecked{} h.started = h.Now() } @@ -646,6 +649,58 @@ func (h *Holder) creditBacklog(w Walk) { if w.Release.Gate != nil { h.creditCarried(w.Release.Gate, w.ID, "") } + if w.State == walkDone { + h.creditReleased(w) + } +} + +// creditReleased passes the steps a finished backlog walk carried: its last record keeps no gate, only the +// machines it is done on, and a release ends done only when its gate passed on each of them (novox/hq issue +// 351). A step it failed on, and a step another walk carried, are left as they are. +func (h *Holder) creditReleased(w Walk) { + why := carriedBy + w.ID + for _, d := range h.deliveries { + moved := false + for _, s := range d.Steps { + if s.Why != why || !containsString(w.Release.Done, s.Machine) || s.State != StepSent && s.State != StepJudging { + continue + } + s.State, s.Since = StepPassed, h.Now() + if h.step(d, s) { + moved = true + } + } + if moved { + h.keep(d) + } + } +} + +// carriedBy begins the why of a step another walk's send carried, followed by that walk's id. +const carriedBy = "carried by " + +// askWalk is the ask of one walk's record by its id, once an hour while a step it carried still judges; +// what it answers is taken as said. +func (h *Holder) askWalk(id string, now time.Time) func() { + if asked, was := h.walkAsked[id]; was && now.Sub(asked) < time.Hour { + return nil + } + h.walkAsked[id] = now + return func() { + _, walks, err := h.Controller.Walks(id) + h.mu.Lock() + defer h.mu.Unlock() + if err != nil { + h.Logf("[mesh-delivery] the walk %s, which carried a step still judging, cannot be read yet: %v", id, err) + h.walkAsked[id] = h.Now().Add(time.Minute - time.Hour) // asked again a minute on + return + } + for _, w := range walks { + if w.ID == id { + h.walkMoved(w) + } + } + } } // creditCarried gives the delivery that published each carried build its machine step. @@ -665,7 +720,7 @@ func (h *Holder) creditCarried(g *WalkGate, walk, except string) { continue } if h.step(d, Step{Machine: c.Node, Module: c.Module, Build: c.Build, State: state, Since: h.Now(), - Why: "carried by " + walk}) { + Why: carriedBy + walk}) { h.keep(d) } } @@ -781,6 +836,20 @@ func (h *Holder) plan() []func() { } } + // A step a walk carried and left judging: that walk's record is asked for by its id. A walk that ended + // while this holder was down may be older than the ended walks the controller lists back at its start, + // and its end is what passes the step (novox/hq issue 351). + for _, d := range h.deliveries { + for _, s := range d.Steps { + if s.State != StepSent && s.State != StepJudging || !strings.HasPrefix(s.Why, carriedBy) { + continue + } + if a := h.askWalk(strings.TrimPrefix(s.Why, carriedBy), now); a != nil { + asks = append(asks, a) + } + } + } + // Merged, and no walk opened for it after the walks were read again: never silent — held, saying so. for _, d := range h.deliveries { if h.unwalkedPast(d, now) { @@ -884,7 +953,11 @@ func (h *Holder) coverOf(d *Delivery) (string, []string) { // carries is whether c delivered a module after d: of the same repository, merged later into the same trunk, // delivered on its own passing check — a merge held without one is covered only by a later merge that passed // its own — by its own walk, of its merge commit on that trunk, which sent the module to every machine running -// it. Whether c's merge commit contains d's is the forge's to say (coverOf). +// it. Its machine steps say so in one of two ways: the walk's own record, which sends to every machine at once +// or to the rest after its first machines passed; or, for a build the controller held back and a person +// released, one step per machine credited by the backlog walk that carried it — passed on every machine d's +// plan sends the module to (novox/hq issue 351). Whether c's merge commit contains d's is the forge's to say +// (coverOf). func carries(c, d *Delivery, module string) bool { if c.ID == d.ID || c.State != Delivered || !c.Check.Passes() || !strings.EqualFold(c.Repository, d.Repository) || !strings.EqualFold(c.Base, d.Base) || c.MergedAs == "" || sameCommit(c.MergedAs, d.MergedAs) || @@ -893,6 +966,7 @@ func carries(c, d *Delivery, module string) bool { return false } everywhere := false + passed := map[string]bool{} for _, s := range c.Steps { if s.Module != module { continue @@ -902,9 +976,23 @@ func carries(c, d *Delivery, module string) bool { return false case s.State == StepSent && (s.Machine == stepEveryMachine || s.Machine == stepTheRest): everywhere = true + case s.State == StepPassed: + passed[s.Machine] = true } } - return everywhere + if everywhere { + return true + } + machines := d.Plan.receiving(module) + if len(machines) == 0 { + return false + } + for _, m := range machines { + if !passed[m] { + return false + } + } + return true } // askContains is the ask of the forge whether a later merge commit contains a delivery's merge: at most once a diff --git a/modules/mesh-delivery/cmd/mesh-delivery/model.go b/modules/mesh-delivery/cmd/mesh-delivery/model.go index ef31ee77..17004128 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/model.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/model.go @@ -144,6 +144,21 @@ type DeliveryPlan struct { Summary string `json:"summary"` } +// receiving are the machines the plan sends a module to, in the plan's order; none when the plan names no +// machines, as a plan kept before they were said may not. +func (p *DeliveryPlan) receiving(module string) []string { + if p == nil { + return nil + } + var out []string + for _, m := range p.Machines { + if containsString(m.Receives, module) { + out = append(out, m.Machine) + } + } + return out +} + // MachinePlan is one machine's part of a plan. type MachinePlan struct { Machine string `json:"machine"`