diff --git a/go/buildstep/askrun.go b/go/buildstep/askrun.go index 496019b..1415331 100644 --- a/go/buildstep/askrun.go +++ b/go/buildstep/askrun.go @@ -25,7 +25,7 @@ type StepFunc func(ctx context.Context, args map[string]any) (any, error) // AskSteps are the step tools a build module runs an ask with. Copy is the image module's alone; Digest the Go module's. type AskSteps struct { Fetch, Build, Copy, Digest, Test StepFunc - // Toolchain is the toolchain the module builds with now; a build ask naming another is refused. Nil checks nothing + // Toolchain is the toolchain the module builds with now; a build ask naming another is declined (Declining). Nil checks nothing // (an image or an archive names none). Toolchain func(ctx context.Context) (delivery.ToolchainUsed, error) // Kept looks up a kept verdict by level and digest (the checker's kept bucket, delivery.KeptKey): whether one is @@ -115,8 +115,10 @@ func (s AskSteps) build(ctx context.Context, job delivery.BuildJob) (any, error) return nil, err } if tc.Version != job.Toolchain.Version || tc.Recipe != job.Toolchain.Recipe { - return nil, Refuse("the ask is for %s (recipe %s) and this node builds with %s (recipe %s)", - job.Toolchain.Version, job.Toolchain.Recipe, tc.Version, tc.Recipe) + // This node is not on the agreed toolchain: it declines the ask for a node that is (novox/hq ADR 0584 clause 2). + return nil, &Declining{Lacks: []string{fmt.Sprintf("the agreed toolchain %s (recipe %s)", job.Toolchain.Version, job.Toolchain.Recipe)}, + Text: fmt.Sprintf("the ask is for %s (recipe %s) and this node builds with %s (recipe %s): it sits out, and the ask is left to a node on the agreed toolchain", + job.Toolchain.Version, job.Toolchain.Recipe, tc.Version, tc.Recipe)} } answer.Toolchain = &tc } diff --git a/go/buildstep/askrun_test.go b/go/buildstep/askrun_test.go index a21dd88..343bdc4 100644 --- a/go/buildstep/askrun_test.go +++ b/go/buildstep/askrun_test.go @@ -103,7 +103,7 @@ build {"artifact":"lib","module":"modules/m","publish":true,"workspace":"/w/1"}` } } -// A build ask for another toolchain is refused, so its step ends refused and the asker reads why. +// A build ask for another toolchain is declined (ADR 0584 clause 2), so the node sits out and the asker puts it again. func TestRunAskRefusesAnotherToolchain(t *testing.T) { r := &recorder{fetch: map[string]any{"workspace": "/w"}} s := r.steps() @@ -112,8 +112,8 @@ func TestRunAskRefusesAnotherToolchain(t *testing.T) { } _, err := RunAsk(context.Background(), s, ask(delivery.BuildJob{Module: "m", Path: ".", Fetch: map[string]any{"repository": "novox/x", "commit": runCommit}, Artifacts: []delivery.AskArtifact{{Artifact: "bundle"}}, Toolchain: &delivery.ToolchainUsed{Version: "go1.26", Recipe: "3"}})) - var ref *Refusal - if !errors.As(err, &ref) || len(r.calls) != 0 { + var dec *Declining + if !errors.As(err, &dec) || len(dec.Lacks) != 1 || len(r.calls) != 0 { t.Fatalf("err %v, calls %v", err, r.calls) } } diff --git a/go/buildstep/pull.go b/go/buildstep/pull.go index da1e540..b1cf7f3 100644 --- a/go/buildstep/pull.go +++ b/go/buildstep/pull.go @@ -538,6 +538,28 @@ func (p *Puller) decline(msg *Pulled, ask delivery.StepAsk, lacks []string) { p.mu.Unlock() } +// declineRun ends the step of an ask that began and then found this node should not run it (Declining), as decline does +// before it begins: refused as declined, the message dropped for its asker to put again, and the worker it came from left +// alone until the next pull's wait. +func (p *Puller) declineRun(msg *Pulled, ask delivery.StepAsk, key string, step delivery.Step, d *Declining) { + result, _ := json.Marshal(delivery.StepDeclined{Declined: true, Needs: ask.Needs, Lacks: d.Lacks}) + step.Ended = now().UTC() + step.State, step.Reason, step.Result = delivery.StepRefused, d.Text, result + if err := writeEnd(p.Put, key, step); err != nil { + p.logf("the ask %s cannot be declined on %s, so it is given back: %v", ask.ID, p.Node, err) + _ = msg.Nak() + return + } + p.logf("the ask %s is declined on %s: %s", ask.ID, p.Node, d.Text) + _ = msg.Term() + p.mu.Lock() + if p.declined == nil { + p.declined = map[string]bool{} + } + p.declined[msg.Kind] = true + p.mu.Unlock() +} + func withKind(msg *Pulled, kind string) *Pulled { if msg != nil { msg.Kind = kind @@ -625,6 +647,11 @@ func (p *Puller) run(ctx context.Context, msg *Pulled, ask delivery.StepAsk, key }) step = ph.end() // its end keeps the phase it ended in close(done) + var dec *Declining + if errors.As(err, &dec) && ctx.Err() == nil { + p.declineRun(msg, ask, key, step, dec) + return + } if ctx.Err() != nil { // The process stops: the ask is neither ended nor acknowledged here, so the stream gives it to a node that runs. return diff --git a/go/buildstep/pull_test.go b/go/buildstep/pull_test.go index 24656f6..d50f28c 100644 --- a/go/buildstep/pull_test.go +++ b/go/buildstep/pull_test.go @@ -230,6 +230,21 @@ func TestPullDropsWhatIsNoAsk(t *testing.T) { Run: func(ctx context.Context, a delivery.StepAsk) (any, error) { return nil, nil }} go q.Pull(ctx) waitFor(t, "the placed ask acked where its need is met", func() bool { a, _, _ := has.Fates(); return len(a) == 1 }) + // A run that finds its node is not on the agreed toolchain declines the ask after it began (ADR 0584 clause 2); a node on it builds. + sit, sitSteps := NewFakeAsks(), newSteps() + toolchain := testAsk(delivery.AskDelivery) + _ = sit.PublishAsk(toolchain) + s2 := &Puller{Source: sit, Language: delivery.AskGo, Node: "ace", Put: sitSteps.put, Capacity: func() int { return 1 }, PullWait: 10 * time.Millisecond, + Run: func(ctx context.Context, a delivery.StepAsk) (any, error) { + return nil, &Declining{Text: "the ask is for 26.13.0 and this node builds with 26.12.1", Lacks: []string{"the agreed toolchain 26.13.0 (recipe 3)"}} + }} + go s2.Pull(ctx) + waitFor(t, "the ask termed on the node that sits out", func() bool { _, tm, _ := sit.Fates(); return len(tm) == 1 }) + sk, _ := delivery.AskStepKey("ace", toolchain.ID) + ss, _ := sitSteps.get(sk) + if d, ok := delivery.DeclinedOf(ss); !ok || len(d.Lacks) != 1 || !strings.Contains(ss.Reason, "26.12.1") { + t.Fatalf("the sitting-out step = %+v", ss) + } } // load reads the recorded steps state's raw values: the pause key is not a step, so it is kept apart. diff --git a/go/buildstep/refusal.go b/go/buildstep/refusal.go index dcf0c48..9e4fa73 100644 --- a/go/buildstep/refusal.go +++ b/go/buildstep/refusal.go @@ -16,3 +16,14 @@ func (r *Refusal) Error() string { return r.Text } // Refuse is a Refusal with its text formatted. func Refuse(format string, args ...any) *Refusal { return &Refusal{Text: fmt.Sprintf(format, args...)} } + +// Declining is a step a node does not run because another node should: it ends refused as declined (delivery.StepDeclined), +// the ask's message is dropped, and the asker puts the ask again under a new id (issue 890). A build ask naming a +// toolchain other than this node's is one (novox/hq ADR 0584 clause 2): a node that is not in the toolchain majority +// sits out and never fails a build. +type Declining struct { + Text string + Lacks []string // what this node lacks, in words the operator reads +} + +func (d *Declining) Error() string { return d.Text } diff --git a/go/delivery/buildrun/standing_test.go b/go/delivery/buildrun/standing_test.go index e60b5a6..6a55ef4 100644 --- a/go/delivery/buildrun/standing_test.go +++ b/go/delivery/buildrun/standing_test.go @@ -91,8 +91,8 @@ func TestToolchainGate(t *testing.T) { gate, set, said := start(t, true) key, value := gateReport(t, "ace", "v26.12.0", old) gate.Toolchain(NodeBuildModule, key, value, false) - if _, none := agreedOf(set); !strings.HasPrefix(none, "builders disagree") { - t.Fatalf("before the placement is settled every report counts; got %q", none) + if a := delivery.ToolchainAgreement(set.Entries("typescript")); len(a.SittingOut) != 1 || a.SittingOut[0].Node != "ace" { // ADR 0584: the majority is agreed, ace's report counts as the one that sits out + t.Fatalf("before the placement is settled every report counts; got %+v", a) } k, v := placing(t, "ace", GoBuildModule) // another build module on ace: no matter gate.Placement(k, v, false, old) @@ -111,8 +111,8 @@ func TestToolchainGate(t *testing.T) { // Placed on ace with a report written since: it counts, and the builders really disagree. k, v = placing(t, "ace", GoBuildModule, NodeBuildModule) gate.Placement(k, v, false, old.Add(-time.Minute)) - if _, none := agreedOf(set); !strings.HasPrefix(none, "builders disagree: v26.12.0 recipe f59571cf4be75adf on ace") { - t.Fatalf("builders that run different versions still disagree; got %q", none) + if a := delivery.ToolchainAgreement(set.Entries("typescript")); a.Toolchain.Version != "v26.12.1" || len(a.SittingOut) != 1 || a.SittingOut[0].Version != "v26.12.0" { + t.Fatalf("builders that run different versions: the majority is agreed and ace sits out; got %+v", a) } // Taken off ace again (its key deleted): its report goes again. if !gate.Placement(k, nil, true, placed) {