diff --git a/modules/mesh-delivery-checker/internal/check/examiner_test.go b/modules/mesh-delivery-checker/internal/check/examiner_test.go index 1e619d5..8578f77 100644 --- a/modules/mesh-delivery-checker/internal/check/examiner_test.go +++ b/modules/mesh-delivery-checker/internal/check/examiner_test.go @@ -314,6 +314,12 @@ func TestOnlyThePackageWithoutAKeptVerdictRuns(t *testing.T) { // 1288/AC1 if i := out.Verdict.Levels[delivery.LevelIntegration]; i.Run != 2*slow.Tests || i.Kept != 0 { t.Errorf("integration = %+v, want %d run and nothing counted kept", i, 2*slow.Tests) } + // And where no node has the need, the check fails once the declines outlast the bound, naming the need. + b.kept, b.declines = newMemKV(), 1000 + b.ex.Kept, b.ex.DeclineBound = Kept{Store: b.kept}, 200*time.Millisecond + if v := b.judge(t, caseOf(t, pullOf(repoA, 7, head1))).Verdict; v.State != delivery.VerdictFail || !strings.Contains(fmt.Sprint(v), "needs a node with node-build-node") { + t.Errorf("with no node having the need: %+v", v) + } } func TestAReAskRunsOnlyWhatFailedAndWhatChanged(t *testing.T) { // 1288/AC2, AC3 diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index 499de26..938c1af 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -80,6 +80,9 @@ var askLanguage = map[string]string{buildrun.GoBuildModule: delivery.AskGo, buil // stopping at the first failure; it judges the step's answer as it judged the tools' answers. type Examiner struct { Asks Asks + // DeclineBound is how long a placed ask may go on being declined, from its first decline, before the check fails + // saying no node with its need took it (issue 890, ADR 0552); DefaultDeclineBound when zero. + DeclineBound time.Duration // Toolchains are the nodes' toolchains: a kept verdict names the toolchain the nodes agree on. Toolchains ToolchainEntries Kept Kept @@ -381,6 +384,10 @@ func (st *state) judgeAll(ctx context.Context) *delivery.Verdict { return nil } +// DefaultDeclineBound is an Examiner's DeclineBound when it names none: longer than a check runs, so a node with the +// need that is busy is waited for, and short enough that a mesh where no node has it says so. +const DefaultDeclineBound = 30 * time.Minute + // ask puts the target's check ask, waits for the step of the node that pulled it and judges its answer: the fetch, the // pins of its go.mod files, then each level's digest, kept verdicts and tests. When the answer leaves packages unmet, // their tests needing what that node lacks (issue 890), the passes are kept and the levels from the first such one are @@ -408,6 +415,7 @@ func (st *state) askNeeding(ctx context.Context, t *target, merged *[]Fetched, l a := delivery.StepAsk{ID: buildstep.NewID(), Kind: kind, Language: askLanguage[t.module], Asker: delivery.CheckerModule, At: st.j.now().UTC(), Check: job, Needs: needs} var step delivery.Step + var firstDecline time.Time for { what := fmt.Sprintf("the %s ask %s of %s#%d", kind, a.ID, p.Repository, p.Number) if placed { @@ -431,8 +439,20 @@ func (st *state) askNeeding(ctx context.Context, t *target, merged *[]Fetched, l } // A node that lacks the need dropped the ask (issue 890): it is put again, for a node that has it to take; the // node that declined looks at the stream again only after its next pull's wait. + now := st.j.now() + if firstDecline.IsZero() { + firstDecline = now + } + bound := st.j.DeclineBound + if bound <= 0 { + bound = DefaultDeclineBound + } + if now.Sub(firstDecline) > bound { + return fail(step.Node, fmt.Sprintf("the check of %s#%d needs a node with %s, and none took it in %s: every node that pulled it lacks it (last %s)", + p.Repository, p.Number, strings.Join(needs, ", "), bound, step.Node)) + } st.j.logf("%s#%d: the ask %s was declined on %s, which lacks %s; asked again", p.Repository, p.Number, a.ID, step.Node, strings.Join(d.Lacks, ", ")) - a.ID, a.At = buildstep.NewID(), st.j.now().UTC() + a.ID, a.At = buildstep.NewID(), now.UTC() } // The fetch and the digests are what of the ask's time its levels' running did not take (Took.Clone). took := time.Duration(0)