Decline a build ask for another toolchain instead of failing it (ADR 0584 clause 2, issue 1578)
With the toolchain agreed by majority, a node on the minority version can pull an ask for the agreed one. It now declines it (the issue 890 path) for the asker to put again, so it never fails a build. Claude-Session: https://claude.ai/code/session_01NcbgfJ9Gum47zkFKWRaovL
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user