Repository navigation
chore: upgrade Go to 1.27.3 and update dependencies - #7102
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 27.08% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 48 functions across 51 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title "chore: upgrade Go to 1.27.3 and update dependencies" matches the main change. The diff raises the Go toolchain in image/Dockerfile to golang:1.27.3, raises the go directive to 1.27.0 across… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "chore: upgrade Go to 1.27.3 and update dependencies" matches the main change. The diff raises the Go toolchain in image/Dockerfile to golang:1.27.3, raises the go directive to 1.27.0 across the module files, and refreshes dependencies. The title is short, specific, and states the primary change. The changeset also includes about 172 task-message formatting changes (%w to %v and similar) for Go 1.27 vet findings, which the title does not mention. That is expected because the title need not cover every detail.
-
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks the task results with care
And finds %v where wrapped errors were
New Go tools hop along the path
Old kind loads the image at last
Dependencies settle in their burrow
The rabbit nods, then bounds tomorrow
Comment @coderabbitai help to get the list of available commands.
There was a problem hiding this comment.
🔵 Needs a closer look
It is a broad Go toolchain upgrade and dependency refresh across 14 modules whose build/test/lint compatibility cannot be verified in this environment and which the author notes was not fully validated locally, warranting human review.
No open issues 🎉
What changed in this PR
This PR upgrades the Go toolchain and Docker builder image to 1.27.3, raises the go directive to 1.27.0 across all 14 Go modules, and refreshes dependencies via hack/update-modules.sh (following #7017). Go 1.27's vet now detects printf-style wrappers through interfaces, which flags the task.*().With(...) message helpers; the PR adapts ~172 call sites accordingly. Since With(format string, args ...any) delegates to fmt.Sprintf, %w was never actually supported there (it produced malformed %!w(...) output), so the conversions are a correct, net-positive fix rather than a loss of error wrapping.
Changes:
- Bump Go toolchain to 1.27.3 (Dockerfile digest-pinned) and
godirective to 1.27.0 in every module; refresh go.mod/go.sum dependencies. - Replace unsupported
%wwith%vintask.Fail()/Complete()/Retry().With(...)messages. - Wrap dynamic messages passed to
With(...)with a constant"%s"format (e.g..With("%s", fmt.Sprintf(...)),.With("%s", err.Error())) to satisfy vet's printf check.
| File | Description |
|---|---|
| image/Dockerfile | Bumps builder image to golang:1.27.3 with pinned digest |
| go.mod / go.sum (root, api, tests/validation, tools/*) | Raises go directive to 1.27.0 and refreshes dependency versions across all modules |
| pkg/controllers/*/tasks/*.go (updater, svc, status, pod, cm, ctx, etc.) | Converts %w→%v and wraps dynamic With(...) messages with "%s" to satisfy Go 1.27 vet |
| pkg/controllers/cluster/tasks/feature.go, ctx.go | Bulk %w→%v conversions in per-group feature/list error messages |
| pkg/controllers/common/task.go, task_*.go, revision.go | Wraps err.Error()/dynamic strings with "%s" and %w→%v |
| pkg/controllers/tidb/tasks/pod.go | Additional gofmt reindentation of defaultSlowLogVolumeAndMount (semantics unchanged) |
Review effort: Balanced
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7102 +/- ##
==========================================
- Coverage 42.49% 42.00% -0.49%
==========================================
Files 447 447
Lines 25923 22917 -3006
==========================================
- Hits 11016 9627 -1389
+ Misses 14907 13290 -1617
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
fgksgf
left a comment
There was a problem hiding this comment.
Thanks! Reviewed the toolchain bump and the printf sweep — the PR description's claims all check out:
- Exactly 172
.With(call sites changed (136%w→%vin constant formats, 36 dynamic messages passed through a constant%s); no remaining%wor non-constant-format.With(calls left in the repo — the sweep is complete. - All 14 modules at
go 1.27.0, root pinned totoolchain go1.27.3; CI workflows usego-version-file: ./go.mod, so nothing else contradicts the bump. Builder image digest-pinned to 1.27.3,--platform=$BUILDPLATFORMunchanged. - The dependency refresh looks like a plain
hack/update-modules.shresult: k8s.io/controller-runtime and k8s.io/* unchanged, only patch/minor moves (grpc, ginkgo/gomega, testify, otelgrpc, cloud SDKs), no replace additions/removals.
Since With is fmt.Sprintf-based, %w never rendered an error chain there, so %w→%v is purely a message-rendering fix with no loss of error identity — good.
One non-blocking follow-up: docs/CONTRIBUTING.md still says "install Go 1.23.x" — worth a tiny docs pass whenever convenient.
LGTM.
| pdCfg, err := getPDConfig(ctx, pdc.Underlay()) | ||
| if err != nil { | ||
| return task.Fail().With(err.Error()) | ||
| return task.Fail().With("%s", err.Error()) |
There was a problem hiding this comment.
nit (non-blocking): With("%v", err) would be a bit shorter and matches the direct-format form used a few lines below in the same function (line 240). The %s + err.Error() wrap is fine for a mechanical pass — identical output either way.
| Name: v1alpha1.VolumeNameTiDBSlowLogDefault, | ||
| MountPath: v1alpha1.DirPathTiDBSlowLogDefault, | ||
| } | ||
| Name: v1alpha1.VolumeNameTiDBSlowLogDefault, |
There was a problem hiding this comment.
Flagging for the record: this hunk is indentation-only (gofmt re-canonicalization of the composite literal), unrelated to the printf fix. Fine to keep here since the file is touched anyway — just noting it so nobody reads it as a logic change.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fgksgf 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 |
[LGTM Timeline notifier]Timeline:
|
|
/test pull-e2e |
4 similar comments
|
/test pull-e2e |
|
/test pull-e2e |
|
/test pull-e2e |
|
/test pull-e2e |
|
/retest |
e432f6a to
74f4efe
Compare
|
/hold |
|
/unhold |
Upgrade the Go toolchain and Docker builder image to 1.27.3, with the verified multi-platform image digest. Raise the Go directive to 1.27.0 across all 14 modules and refresh dependencies using
hack/update-modules.sh, following #7017.Go 1.27 vet detects printf wrappers through interfaces. Fix 172 task-message formatting calls by replacing unsupported
%wwith%vand passing dynamic messages through a constant%sformat.Upgrade E2E now uses the old checkout's
kindwrapper to load both its prestop-checker and operator images. This avoids invoking the current branch's Go 1.27 tool module from an older checkout that selects Go 1.25.Validation:
make buildmake unit(including race detection; run outside the sandbox for local HTTP test servers)cd api && go test -race ./...make tidy(no additional go.mod/go.sum changes)git diff --checkupstream/main:bash -n hack/lib/e2e.shandgit diff --check upstream/main...HEADFull lint, generated-file verification, and E2E tests were not run locally.
Summary by CodeRabbit