diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/forge.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/forge.go index 31702bf..b3aabfa 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/forge.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/forge.go @@ -3,10 +3,13 @@ package main import ( "context" "encoding/json" + "errors" "fmt" "regexp" "strings" + "github.com/nats-io/nats.go" + "mesh-delivery-checker/internal/check" ) @@ -27,6 +30,16 @@ type forgeReads struct { call func(key string, body any) (json.RawMessage, error) } +// ask calls a forge tool; a call that reached nobody (no responders: the forge's tool runner re-serving, or the forge +// down) is marked Unheard, so the check fails at the forge and is judged again when the nodes change (issue 1624). +func (f forgeReads) ask(key string, body any) (json.RawMessage, error) { + raw, err := f.call(key, body) + if errors.Is(err, nats.ErrNoResponders) || errors.Is(err, nats.ErrTimeout) || errors.Is(err, context.DeadlineExceeded) { + return nil, &check.Unheard{Err: err} + } + return raw, err +} + func ownerRepo(repository string) (string, string) { owner, repo, _ := strings.Cut(repository, "/") return owner, repo @@ -35,7 +48,7 @@ func ownerRepo(repository string) (string, string) { // File is the text of a file of a repository at a ref. func (f forgeReads) File(_ context.Context, repository, path, ref string) (string, error) { owner, repo := ownerRepo(repository) - raw, err := f.call(getFileTool, map[string]string{"owner": owner, "repo": repo, "path": path, "ref": ref}) + raw, err := f.ask(getFileTool, map[string]string{"owner": owner, "repo": repo, "path": path, "ref": ref}) if err != nil { if notFoundStatus.MatchString(err.Error()) { // The forge answers 404 for a file the ref does not hold (issue 1845). The status is matched in its own place, @@ -59,7 +72,7 @@ func (f forgeReads) File(_ context.Context, repository, path, ref string) (strin // not. func (f forgeReads) OnTrunk(_ context.Context, repository, commit string) (bool, error) { owner, repo := ownerRepo(repository) - raw, err := f.call(compareTool, map[string]string{"owner": owner, "repo": repo, "base": "main", "head": commit}) + raw, err := f.ask(compareTool, map[string]string{"owner": owner, "repo": repo, "base": "main", "head": commit}) if err != nil { return false, err } @@ -75,7 +88,7 @@ func (f forgeReads) OnTrunk(_ context.Context, repository, commit string) (bool, // FullSHA is the whole sha of a commit named by its first digits: the newest commit of the history at that ref. func (f forgeReads) FullSHA(_ context.Context, repository, commit string) (string, error) { owner, repo := ownerRepo(repository) - raw, err := f.call(commitsTool, map[string]any{"owner": owner, "repo": repo, "ref": commit, "limit": 1}) + raw, err := f.ask(commitsTool, map[string]any{"owner": owner, "repo": repo, "ref": commit, "limit": 1}) if err != nil { return "", err } diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/forge_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/forge_test.go index e7ecc85..d6487d6 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/forge_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/forge_test.go @@ -9,6 +9,8 @@ import ( "strings" "testing" + "github.com/nats-io/nats.go" + "mesh-delivery-checker/internal/check" ) @@ -86,3 +88,18 @@ func TestAFileTheRefDoesNotHoldIsNotFoundAndOtherFailuresAreNot(t *testing.T) { t.Errorf("a 500 is %v, want a failure that is not ErrNotFound", err) } } + +func TestAForgeReadThatReachesNobodyIsUnheard(t *testing.T) { // 1624 C7: "nats: no responders" on mesh-catalog #293 + f := forgeReads{call: func(string, any) (json.RawMessage, error) { return nil, nats.ErrNoResponders }} + _, err := f.File(context.Background(), "novox/mesh-catalog", "checks/agenthome/go.mod", "950e37a46e8d") + var u *check.Unheard + if !errors.As(err, &u) { + t.Errorf("err = %v, want it marked as reaching nobody", err) + } + if _, err := f.OnTrunk(context.Background(), "novox/mesh-sdk", "abc"); !errors.As(err, &u) { + t.Errorf("compare: %v", err) + } + if _, err := f.FullSHA(context.Background(), "novox/mesh-sdk", "abc"); !errors.As(err, &u) { + t.Errorf("commits: %v", err) + } +} diff --git a/modules/mesh-delivery-checker/internal/check/checker.go b/modules/mesh-delivery-checker/internal/check/checker.go index a53aa5f..1236af4 100644 --- a/modules/mesh-delivery-checker/internal/check/checker.go +++ b/modules/mesh-delivery-checker/internal/check/checker.go @@ -18,6 +18,10 @@ const DefaultBound = 30 * time.Minute // is judged again when the nodes change, by a watch and never a timer (ADR 0437 §8). const NodesWords = "build node qualifies" +// UnheardWords end the Words of a failure because a part of the mesh the check reads was not heard (the forge, issue 1624): +// like a failure of the nodes, it is not the change's, and is judged again when the nodes change. +const UnheardWords = "not heard; judged again when the nodes change" + // The events the checker says on its stream checks (module.json): a preview or a status was written, so a part that reads // the bucket, which is made when the checker is assigned and may not exist when that part starts, finds it (ADR 0437 §8: no // timer). The body is {"key": ...}. @@ -287,7 +291,7 @@ func (ch *Checker) claim(ctx context.Context, cs *Case, first bool, epoch uint64 // IsNodesFailure says whether a verdict failed because no build node could take the check. func IsNodesFailure(v delivery.Verdict) bool { - return v.State == delivery.VerdictFail && strings.Contains(v.Failed.Words, NodesWords) + return v.State == delivery.VerdictFail && (strings.Contains(v.Failed.Words, NodesWords) || strings.Contains(v.Failed.Words, UnheardWords)) } // try claims the check, judges it within its bound and writes the verdict and the owed statuses. A check that ends diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index d0072b8..5593b58 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -304,7 +304,7 @@ func (st *state) fetchAll(ctx context.Context) *delivery.Verdict { if bm == buildrun.GoBuildModule { beside, err := st.j.besideFor(ctx, st.c, p) if err != nil { - return fail(node, err.Error()) + return fail(forgeMachine(err, node), err.Error()+unheardWords(err)) } if len(beside) > 0 { args["beside"] = beside diff --git a/modules/mesh-delivery-checker/internal/check/pins.go b/modules/mesh-delivery-checker/internal/check/pins.go index 326d606..f1e2937 100644 --- a/modules/mesh-delivery-checker/internal/check/pins.go +++ b/modules/mesh-delivery-checker/internal/check/pins.go @@ -77,7 +77,7 @@ func (st *state) checkPins(ctx context.Context) verdictPtr { text, err = st.j.Forge.File(ctx, t.pull.Repository, file, t.pull.Trunk) } if err != nil { - return failed(forgeMachine(err, t.node), fmt.Sprintf("%s#%d: %s cannot be read to check its mesh-sdk pin: %v", t.pull.Repository, t.pull.Number, file, err)) + return failed(forgeMachine(err, t.node), fmt.Sprintf("%s#%d: %s cannot be read to check its mesh-sdk pin: %v%s", t.pull.Repository, t.pull.Number, file, err, unheardWords(err))) } pin, ok := SDKPin(text) if !ok { @@ -88,7 +88,7 @@ func (st *state) checkPins(ctx context.Context) verdictPtr { } onTrunk, err := st.j.Forge.OnTrunk(ctx, SDKRepository, pin) if err != nil { - return failed(forgeMachine(err, t.node), fmt.Sprintf("%s#%d: whether mesh-sdk %s is on its trunk cannot be told: %v", t.pull.Repository, t.pull.Number, pin, err)) + return failed(forgeMachine(err, t.node), fmt.Sprintf("%s#%d: whether mesh-sdk %s is on its trunk cannot be told: %v%s", t.pull.Repository, t.pull.Number, pin, err, unheardWords(err))) } if onTrunk { continue @@ -115,6 +115,14 @@ func pinHint(c *Case, fetchError string) string { return " (a pin to the head of the group's mesh-sdk pull request names a commit only that head carries, which go mod download cannot resolve; " + Issue1148 + ")" } +// unheardWords ends the words of a failed read of the forge that reached nobody: the forge was not heard (UnheardWords). +func unheardWords(err error) string { + if isUnheard(err) { + return " (the forge was " + UnheardWords + ")" + } + return "" +} + // forgeMachine is the machine a failed read of the forge is put at: the forge when nobody answered it (no build node is to // blame, issue 1820), the build node whose workspace was read otherwise. func forgeMachine(err error, node string) string { diff --git a/modules/mesh-delivery-checker/internal/check/pins_test.go b/modules/mesh-delivery-checker/internal/check/pins_test.go index 9198b17..703a11b 100644 --- a/modules/mesh-delivery-checker/internal/check/pins_test.go +++ b/modules/mesh-delivery-checker/internal/check/pins_test.go @@ -253,3 +253,15 @@ func TestThePinOfTheControllerThatCannotBeReadFailsTheCheck(t *testing.T) { // r t.Errorf("verdict = %+v", v) } } + +func TestAForgeThatIsNotHeardFailsAtTheForgeAndIsJudgedAgainWhenTheNodesChange(t *testing.T) { // 1624 C7 + b := newBench(t) + b.ex.Forge = &fakeForge{err: &Unheard{Err: fmt.Errorf("nats: no responders available for request")}} + v := b.judge(t, caseOf(t, pullOf(repoA, 7, head1))).Verdict + if v.State != delivery.VerdictFail || v.Failed.Machine != "forge" { + t.Fatalf("verdict = %+v, want failed at the forge", v) + } + if !IsNodesFailure(v) { + t.Errorf("a forge that was not heard is not the change's failure: %+v is not judged again when the nodes change", v.Failed) + } +}