diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go index 40ed0d7..1d3a461 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go @@ -308,7 +308,7 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro go func() { errs <- follow(ctx, sw, func(e jetstream.KeyValueEntry) { if e.Key() == delivery.SourceMainKey && trunks.Put(e.Value(), !opOf(e)) { - ch.Reconsider(ctx) // the trunk moved: a verdict for the old trunk is judged again + ch.TrunkMoved(ctx) // the trunk moved: its statuses go pending, and a verdict for the old trunk is judged again } }) }() diff --git a/modules/mesh-delivery-checker/internal/check/case.go b/modules/mesh-delivery-checker/internal/check/case.go index 24cb19e..a6ea453 100644 --- a/modules/mesh-delivery-checker/internal/check/case.go +++ b/modules/mesh-delivery-checker/internal/check/case.go @@ -4,6 +4,7 @@ import ( "fmt" "maps" "sort" + "strings" "sync" "time" @@ -35,7 +36,9 @@ type Case struct { Key string Group string HeadsHash string - Pulls []Pull + // Heads is the heads hash the gate matches an Approve's members by (GateHeads): a verdict carries it. + Heads string + Pulls []Pull // Trunk is the trunk head of each repository of Pulls. Trunk map[string]string // Left says why pull requests of the candidate are not judged (hq's, a repository with no known trunk); empty when all are. @@ -75,7 +78,7 @@ func CaseOf(c delivery.Candidate, trunk map[string]string) *Case { if err != nil { return nil } - cs := &Case{Key: key, Group: c.Group, HeadsHash: c.HeadsHash, Trunk: map[string]string{}, Progress: &Progress{}} + cs := &Case{Key: key, Group: c.Group, HeadsHash: c.HeadsHash, Heads: GateHeads(c), Trunk: map[string]string{}, Progress: &Progress{}} for _, p := range c.Pulls { switch head, known := trunk[p.Repository]; { case p.Repository == HQRepository: @@ -99,6 +102,22 @@ func CaseOf(c delivery.Candidate, trunk map[string]string) *Case { return cs } +// GateHeads is the hash the gate matches the members an Approve covers by (mesh-catalog mesh-delivery-gate onestatus.go +// headsOf): delivery.HeadsHash over every pull request of the candidate, hq's included, as +// `#=`. A lone pull request is a group of one. Empty for a candidate +// with no pull request. +func GateHeads(c delivery.Candidate) string { + heads := make([]delivery.Head, 0, len(c.Pulls)) + for _, p := range c.Pulls { + heads = append(heads, delivery.Head{Repository: strings.ToLower(p.Repository), Number: p.Number, Head: strings.ToLower(p.Head.Head)}) + } + hash, err := delivery.HeadsHash(heads) + if err != nil { + return "" + } + return hash +} + // sameTrunk says whether two verdicts' trunk heads are the same. func sameTrunk(a, b map[string]string) bool { return maps.Equal(a, b) } diff --git a/modules/mesh-delivery-checker/internal/check/checker.go b/modules/mesh-delivery-checker/internal/check/checker.go index 84753ed..7e36d5b 100644 --- a/modules/mesh-delivery-checker/internal/check/checker.go +++ b/modules/mesh-delivery-checker/internal/check/checker.go @@ -234,7 +234,7 @@ type claimed struct{ rev uint64 } // - a pass or a failure for the trunk heads the case has stands; one for other trunk heads (the trunk moved) is taken // again by compare-and-set on the same key, and so is a failure of the nodes after the nodes changed. func (ch *Checker) claim(ctx context.Context, cs *Case, first bool, epoch uint64) (*claimed, error) { - running := delivery.Verdict{Group: cs.Group, Heads: cs.HeadsHash, Trunk: cs.Trunk, State: delivery.VerdictRunning} + running := delivery.Verdict{Group: cs.Group, Heads: cs.Heads, Trunk: cs.Trunk, State: delivery.VerdictRunning} data, err := delivery.EncodeVerdict(running) if err != nil { return nil, err @@ -313,14 +313,14 @@ func (ch *Checker) try(ctx context.Context, key string, cs *Case) { return } v := out.Verdict - v.Group, v.Heads, v.Trunk = cs.Group, cs.HeadsHash, cs.Trunk + v.Group, v.Heads, v.Trunk = cs.Group, cs.Heads, cs.Trunk if v.Took.Queue == 0 && !cs.Queued.IsZero() && began.After(cs.Queued) { v.Took.Queue = began.Sub(cs.Queued).Milliseconds() } data, err := delivery.EncodeVerdict(v) if err != nil { ch.logf("%s: the verdict cannot be written: %v", cs.Key, err) - v = delivery.Verdict{Group: cs.Group, Heads: cs.HeadsHash, Trunk: cs.Trunk, State: delivery.VerdictFail, + v = delivery.Verdict{Group: cs.Group, Heads: cs.Heads, Trunk: cs.Trunk, State: delivery.VerdictFail, Failed: delivery.FailedFirst{Machine: "checker", Words: "the verdict could not be written: " + err.Error()}} if data, err = delivery.EncodeVerdict(v); err != nil { return @@ -335,6 +335,13 @@ func (ch *Checker) try(ctx context.Context, key string, cs *Case) { ch.nodesFail[cs.Key] = epoch ch.mu.Unlock() } + if ch.movedSince(cs) { + // Judged on a trunk that moved while it ran: the verdict is kept as judged, and the status stays pending until the + // check of the new trunk, which Reconsider starts, ends. + ch.writeOwed(ctx, cs, delivery.Verdict{State: delivery.VerdictRunning}, nil, nil) + ch.logf("%s: %s on a trunk that moved while it ran; the status stays pending", cs.Key, v.State) + return + } ch.writeOwed(ctx, cs, v, out.Builds, out.Slowest) ch.logf("%s: %s", cs.Key, v.State) } @@ -467,3 +474,44 @@ func (ch *Checker) Sweep(ctx context.Context) error { } return nil } + +// TrunkMoved is Reconsider after a move of a trunk, which first owes mesh/check pending on every open pull request of a +// candidate whose verdict was judged on other trunk heads, every member's for a group (the lead's decision L4, the checker's +// part of issue 1302): the forge's success of the old trunk never stands while the check waits for a place or for a delivery +// that holds it, so the gate never reads it as the head's. +func (ch *Checker) TrunkMoved(ctx context.Context) { + ch.init() + ch.mu.Lock() + known := make(map[string]delivery.Candidate, len(ch.known)) + for k, c := range ch.known { + known[k] = c + } + ch.mu.Unlock() + heads := ch.heads() + for _, c := range known { + cs := CaseOf(c, heads) + if cs == nil { + continue + } + value, _, found, err := ch.Verdicts.Get(ctx, cs.Key) + if err != nil || !found { + continue // never judged: no status to put out of date + } + if v, err := delivery.DecodeVerdict(value); err == nil && sameTrunk(v.Trunk, cs.Trunk) { + continue + } + ch.writeOwed(ctx, cs, delivery.Verdict{State: delivery.VerdictRunning}, nil, nil) + } + ch.Reconsider(ctx) +} + +// movedSince says whether a trunk the case was judged on is no longer the trunk's head. +func (ch *Checker) movedSince(cs *Case) bool { + now := ch.heads() + for repo, head := range cs.Trunk { + if now[repo] != head { + return true + } + } + return false +} diff --git a/modules/mesh-delivery-checker/internal/check/gate_test.go b/modules/mesh-delivery-checker/internal/check/gate_test.go new file mode 100644 index 0000000..6c709e0 --- /dev/null +++ b/modules/mesh-delivery-checker/internal/check/gate_test.go @@ -0,0 +1,106 @@ +package check + +import ( + "context" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// What the gate reads of the checker (R4e, lead's interface with dev-r4e-gate): the verdict's heads are the gate's hash over +// `#=`, a lone pull request is a group of one, and a move of a trunk marks +// mesh/check pending on every open pull request into it, every member's for a group, before any check starts. + +func owedState(t testing.TB, r *rig, repo string, number int, head string) string { + t.Helper() + key, err := delivery.CheckerOwedKey(repo, number, head) + if err != nil { + t.Fatal(err) + } + raw, _, found, _ := r.owed.Get(context.Background(), key) + if !found { + return "" + } + o, err := delivery.DecodeCheckerOwed(raw) + if err != nil { + t.Fatal(err) + } + return o.State +} + +func TestTheVerdictsHeadsAreTheGatesHashOverLowercaseHeads(t *testing.T) { // gate (b), (c) + ctx := context.Background() + r := newRig(t, passJudge()) + upper := strings.ToUpper(head1[:20]) + head1[20:] + r.setTrunks(t, map[string]string{"Novox/Mesh-Delivery": trunkA, repoA: trunkA, repoSDK: trunkS}) + cand := candidateOf(t, "feat/x", pullOf("Novox/Mesh-Delivery", 7, upper)) + r.ch.Candidate(ctx, "feat%2Fx", cand) + r.ch.Wait() + want, _ := delivery.HeadsHash([]delivery.Head{{Repository: repoA, Number: 7, Head: head1}}) + if v := r.verdict(t, keyOf(t, "feat/x", cand)); v.Heads != want { + t.Errorf("heads = %q, want the gate's hash of the lone pull request, a group of one, %q", v.Heads, want) + } +} + +func TestAGroupsVerdictHeadsCoverEveryMember(t *testing.T) { // gate (b): hq's member too, as the gate's ask covers it + ctx := context.Background() + r := newRig(t, passJudge()) + cand := candidateOf(t, "feat/x", pullOf(repoA, 7, head1), pullOf(repoSDK, 12, head2), pullOf(HQRepository, 3, head2)) + r.ch.Candidate(ctx, "feat%2Fx", cand) + r.ch.Wait() + want, _ := delivery.HeadsHash([]delivery.Head{{Repository: repoA, Number: 7, Head: head1}, {Repository: repoSDK, Number: 12, Head: head2}, + {Repository: HQRepository, Number: 3, Head: head2}}) + if v := r.verdict(t, keyOf(t, "feat/x", cand)); v.Heads != want { + t.Errorf("heads = %q, want %q", v.Heads, want) + } +} + +func TestATrunkMoveMarksEveryOpenPullRequestIntoItPendingAtOnce(t *testing.T) { // gate (a), lead decision L4 (1302's checker part) + ctx := context.Background() + r := newRig(t, passJudge()) + r.ch.Candidate(ctx, "feat%2Fg", candidateOf(t, "feat/g", pullOf(repoA, 7, head1), pullOf(repoSDK, 12, head2))) + r.ch.Candidate(ctx, "feat%2Fs", candidateOf(t, "feat/s", pullOf(repoSDK, 13, head1))) + r.ch.Wait() + for _, p := range []struct { + repo string + n int + head string + }{{repoA, 7, head1}, {repoSDK, 12, head2}, {repoSDK, 13, head1}} { + if s := owedState(t, r, p.repo, p.n, p.head); s != "success" { + t.Fatalf("%s#%d is %q before the move", p.repo, p.n, s) + } + } + // A delivery builds, so no check starts; mesh-delivery's trunk moves. + r.ch.Hold = func() bool { return true } + r.setTrunks(t, map[string]string{repoA: trunkA2, repoSDK: trunkS}) + r.ch.TrunkMoved(ctx) + r.ch.Wait() + if s := owedState(t, r, repoA, 7, head1); s != "pending" { + t.Errorf("mesh-delivery#7, into the moved trunk, is %q, want pending", s) + } + if s := owedState(t, r, repoSDK, 12, head2); s != "pending" { + t.Errorf("mesh-sdk#12, a member of the same group, is %q, want pending", s) + } + if s := owedState(t, r, repoSDK, 13, head1); s != "success" { + t.Errorf("mesh-sdk#13, alone into a trunk that did not move, is %q, want its success to stand", s) + } +} + +func TestACheckThatEndsAfterItsTrunkMovedOwesPendingNotItsVerdict(t *testing.T) { // gate (a): a pass on an old trunk is never shown + ctx := context.Background() + var r *rig + j := &judgeFunc{f: func(ctx context.Context, c *Case, call int) (Outcome, error) { + if call == 1 { + r.setTrunks(t, map[string]string{repoA: trunkA2, repoSDK: trunkS}) // the trunk moves while it runs + r.ch.Hold = func() bool { return true } // and a delivery holds the next check + } + return Outcome{Verdict: passing()}, nil + }} + r = newRig(t, j) + r.ch.Candidate(ctx, "feat%2Fx", candidateOf(t, "feat/x", pullOf(repoA, 7, head1))) + r.ch.Wait() + if s := owedState(t, r, repoA, 7, head1); s != "pending" { + t.Errorf("the status is %q, want pending: the pass was judged on a trunk that moved", s) + } +} diff --git a/modules/mesh-delivery-checker/internal/check/read.go b/modules/mesh-delivery-checker/internal/check/read.go index d02b04c..e4eebd3 100644 --- a/modules/mesh-delivery-checker/internal/check/read.go +++ b/modules/mesh-delivery-checker/internal/check/read.go @@ -68,7 +68,7 @@ func (r Reader) read(key string, raw json.RawMessage) (VerdictRead, error) { return out, fmt.Errorf("%s holds what is not a verdict: %w", key, err) } c, ok := r.candidate(v.Group) - if !ok || c.HeadsHash != v.Heads { + if !ok || (c.HeadsHash != v.Heads && GateHeads(c) != v.Heads) { out.Note = "the candidate no longer holds these heads" return out, nil }