From bbadbb2af0215284b9b87733858b87170d334de7 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 01:59:18 +0200 Subject: [PATCH] Judge the ask and the settle by one say of the builder (issue 1886, merge check hang) The settle read the say from the builder's bucket while the ask read it through builderSaysReferences; where they differed (a test's say against a bus with no bucket) a module was handed back and taken from its key again at every turn, and the look's loop never ended. --- cmd/mesh-controller/builder_says_test.go | 24 ++++++++++++++++++++++-- cmd/mesh-controller/plan_keys.go | 6 +++--- cmd/mesh-controller/plan_keys_test.go | 4 ---- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/cmd/mesh-controller/builder_says_test.go b/cmd/mesh-controller/builder_says_test.go index cafe16cd..c85bdc17 100644 --- a/cmd/mesh-controller/builder_says_test.go +++ b/cmd/mesh-controller/builder_says_test.go @@ -127,7 +127,6 @@ func TestAnOpenWalkWaitingOnAnUnreferencedKeyIsAskedOfBuildAgent(t *testing.T) { // Not said: handed back, and the next look asks build-agent. p, byName, keys := walk() - keys.notSaid = true theBuilderSays(t, false, nil) asked := askingBuildAgent(t) changed, err := settleKeys(t.Context(), records, keys, p, p.Tiers[0], take) @@ -154,7 +153,8 @@ func TestAFailedKeyOfABuilderThatDoesNotSayFailsNoWalk(t *testing.T) { askModule(t.Context(), p, "sudo", byName) _ = byName fp := "src2:" + strings.Repeat("f", 64) - keys := planKeysOf{notSaid: true, values: map[string]sdkdelivery.Module{"sudo@c0ffee0123456789": {FP2: fp}}, + theBuilderSays(t, false, nil) + keys := planKeysOf{values: map[string]sdkdelivery.Module{"sudo@c0ffee0123456789": {FP2: fp}}, keys: map[string]builderkeys.Key{"sudo": {Module: "sudo", Fingerprint: fp, Found: true, Build: sdkdelivery.BuilderBuild{State: sdkdelivery.BuildFailed, Error: "not stored: tools: no reference"}}}} if _, err := settleKeys(t.Context(), recordsOf{}, keys, p, p.Tiers[0], func(string, sdkdelivery.Module, string) {}); err != nil { @@ -221,3 +221,23 @@ func TestTheAskGuardUsesTheSayTheWalkRead(t *testing.T) { } } } + +// 1886 merge check hang: the look must judge an ask and a settle by one say. A settle that read the say from another +// source than the ask (the bus's bucket, against the walk's read) handed a module back, the next ask took it from its key +// again, and the look's loop never ended: walk_from_keys_integration_test hung. +func TestTheSettleJudgesByTheSameSayAsTheAsk(t *testing.T) { + theBuilderSays(t, true, nil) + p, _ := aWalkOfOne("sudo") + p.Modules["sudo"] = &inventory.PlanModule{} + askModule(t.Context(), p, "sudo", map[string]inventory.Entry{"sudo": {Source: inventory.Source{Repository: "novox/mesh-catalog", Path: "modules/sudo", Ref: "main"}}}) + fp := "src2:" + strings.Repeat("1", 64) + keys := planKeysOf{values: map[string]sdkdelivery.Module{"sudo@c0ffee0123456789": {FP2: fp}}, + keys: map[string]builderkeys.Key{"sudo": {Module: "sudo", Fingerprint: fp, Found: true, + Build: sdkdelivery.BuilderBuild{State: sdkdelivery.BuildBuilding, By: "shanks"}}}} + if _, err := settleKeys(t.Context(), recordsOf{}, keys, p, p.Tiers[0], func(string, sdkdelivery.Module, string) {}); err != nil { + t.Fatal(err) + } + if s := p.Modules["sudo"]; !s.Key || s.State != "asked" { + t.Fatalf("the settle judged by another say than the ask's: %+v", s) + } +} diff --git a/cmd/mesh-controller/plan_keys.go b/cmd/mesh-controller/plan_keys.go index abe732bd..1bdb1bcc 100644 --- a/cmd/mesh-controller/plan_keys.go +++ b/cmd/mesh-controller/plan_keys.go @@ -47,8 +47,6 @@ var buildersOwn = func(module string) bool { return buildAgentsOwn[module] } type planKeyReads interface { ModuleAt(ctx context.Context, module, commit string) (sdkdelivery.Module, error) Build(ctx context.Context, module, fp string) (builderkeys.Key, error) - // SaysReferences is whether a running builder says it writes references (builderkeys.Reader.SaysReferences). - SaysReferences(ctx context.Context) (bool, error) } // builderSayKey carries, on an ask's context, the builder's say the walk read before asking (withBuilderSay). @@ -135,7 +133,9 @@ func settleKeys(ctx context.Context, inv keyRecords, keys planKeyReads, p *inven changed = true } } - says, err := keys.SaysReferences(ctx) + // One say for the ask and the settle (builderSaysReferences): two sources that differ hand a module back and take it + // from its key again at every turn of the look, which then never ends. + says, err := builderSaysReferences(ctx) if err != nil { return changed, fmt.Errorf("whether the builder writes references cannot be read: %w", err) } diff --git a/cmd/mesh-controller/plan_keys_test.go b/cmd/mesh-controller/plan_keys_test.go index 1a04a48b..29190f32 100644 --- a/cmd/mesh-controller/plan_keys_test.go +++ b/cmd/mesh-controller/plan_keys_test.go @@ -16,12 +16,8 @@ import ( type planKeysOf struct { values map[string]sdkdelivery.Module // by module + "@" + commit keys map[string]builderkeys.Key - // notSaid is a builder that does not say it writes references (issue 1886); the zero value says it. - notSaid bool } -func (k planKeysOf) SaysReferences(context.Context) (bool, error) { return !k.notSaid, nil } - func (k planKeysOf) ModuleAt(_ context.Context, module, commit string) (sdkdelivery.Module, error) { if v, ok := k.values[module+"@"+commit]; ok { return v, nil