Repository navigation
revert(prow-jobs): resolve error-log-review tool via Go module proxy - #5361
Conversation
This reverts commit 99b33ce (#4909). `GOPROXY=direct` forces the go command to resolve every dependency from its origin host instead of a module proxy. On 2026-10-13 a transient gopkg.in outage broke all error-log-review presubmits (tiflow/ticdc/tiflash/pd/tikv) with: config.go:9:2: unrecognized import path "gopkg.in/yaml.v3": reading https://gopkg.in/yaml.v3?go-get=1: 502 Proxy Error Revert to the default Go module proxy resolution. The cost is that `go run ...@main` may briefly resolve to a stale pseudo-version right after a push to main, which is acceptable for this presubmit.
![ti-chi-bot[bot]](https://avatars.githubusercontent.com/in/214286?s=60&v=4){kind=small}
There was a problem hiding this comment.
I have already done a preliminary review for you, and I hope to help you do a better job.
Summary
This PR reverts the previous change that added GOPROXY=direct to the go run command for the error-log-review tool in presubmit jobs. The revert restores the default Go module proxy usage to improve reliability, addressing transient 502 errors from gopkg.in that caused presubmit failures. The approach is a straightforward revert across multiple YAML files configuring presubmit jobs. The changes are minimal, clean, and well-justified based on recent flakiness observed in the build pipeline.
Critical Issues
- None found. The revert addresses a real reliability problem and has no regression risk since it simply removes an environment override.
Code Improvements
- None critical. However, consider centralizing the
go runcommand invocation forerror-log-reviewtool in a single shared snippet or template, if possible, to reduce duplication across these many presubmit job YAML files. This would make future changes easier and reduce the risk of drift.
Best Practices
-
Documentation:
-
The PR description is very thorough. It would be helpful to add a short comment next to the
go runcommand in each YAML file explaining why we avoidGOPROXY=direct, referencing this revert and the transient outage. This helps future maintainers understand the rationale without diving into PR history. -
Example:
# Use default GOPROXY to avoid transient gopkg.in outages that break presubmits. go run github.com/PingCAP-QE/ci/tools/error-log-review@main \
-
-
Testing:
- Since these are presubmit job configs, manual validation is typical. However, if you have infrastructure or scripts to validate these YAML changes (e.g.,
kubectl apply --dry-runor prow config lint), please mention that validation was done. - Consider adding automated tests or monitoring alerts for presubmit flakiness to catch such issues early.
- Since these are presubmit job configs, manual validation is typical. However, if you have infrastructure or scripts to validate these YAML changes (e.g.,
-
Style / Consistency:
- The diff correctly removes the
GOPROXY=directprefix only, preserving indentation and formatting. - No naming or style issues detected.
- The diff correctly removes the
Summary of Files Impacted
prow-jobs/pingcap/ticdc/latest-presubmits.yaml(line ~249)prow-jobs/pingcap/tiflash/latest-presubmits.yaml(line ~233)prow-jobs/pingcap/tiflow/latest-presubmits.yaml(line ~216)prow-jobs/tikv/pd/latest-presubmits.yaml(line ~124)prow-jobs/tikv/tikv/latest-presubmits.yaml(line ~97)
All changes are identical removals of the GOPROXY=direct environment override before go run.
Overall: This is a well-justified, low-risk revert improving presubmit stability. Adding brief comments in the YAML files would improve maintainability. Consider DRYing the presubmit job config snippet in the future to reduce duplication.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wuhuizuo The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1 participant
What is changed
Revert #4909: remove the
GOPROXY=directprefix from theerror-log-reviewpresubmit jobs, so the go command resolves the tool and its dependencies through the default Go module proxy again.Files (5):
prow-jobs/pingcap/ticdc/latest-presubmits.yamlprow-jobs/pingcap/tiflash/latest-presubmits.yamlprow-jobs/pingcap/tiflow/latest-presubmits.yamlprow-jobs/tikv/pd/latest-presubmits.yamlprow-jobs/tikv/tikv/latest-presubmits.yamlWhy
GOPROXY=directmakes the go command resolve every dependency from its origin host instead of a module proxy. On 2026-10-13 (~02:00–02:35 UTC)gopkg.inreturned a transient502 Proxy Error, which broke all of these presubmits with:Example failed run: https://prow.tidb.net/view/gs/prow-tidb-logs/pr-logs/pull/pingcap_tiflow/12883/pull-error-log-review/2109830078522200064
Failing runs in that window: pingcap/tiflow#12883, pingcap/ticdc#6466, tikv/pd#11337, tikv/pd#11338, tikv/pd#11349; a later run at 02:36 UTC passed. The error-log-review presubmits are non-optional, so this blocked Tide merge.
#4909 added
GOPROXY=directso thatgo run ...@mainalways resolves the latest commit immediately, bypassing module-proxy branch-query caching. Reverting trades that freshness for reliability: right after a push to main,@mainmay briefly resolve to a stale pseudo-version, which is acceptable for this presubmit.Validation
git revert 99b33ce9298fe3e3f82ece3b74196c427668d740applies cleanly (auto-merge, no conflicts).prow-jobs/kustomization.yamlusesdisableNameSuffixHash: true, so no hash regeneration is needed.