Skip to content

feat(dockerfiles): add Go 1.27.2 builder images - #1113

Merged
ti-chi-bot[bot] merged 1 commit into
PingCAP-QE:mainfrom
dillon-zheng:chore/go1.27.2-builder-images
Oct 16, 2026
Merged

ti-chi-bot[bot] merged 1 commit into
PingCAP-QE:mainfrom
dillon-zheng:chore/go1.27.2-builder-images

Conversation

@dillon-zheng

Copy link
Copy Markdown
Contributor

Summary

Prepare Go 1.27.2 CD builder images without switching any package routes or image consumers.

  • Add go-1.27 profiles to all 30 existing Go-builder Skaffold variants, preserving go-1.26 / go-1.25, Renovate annotations, tag conventions, and amd64 + arm64 platforms.
  • Update the 15 corresponding Dockerfile defaults to 1.27.2.
  • Add go-1.27 to the Go matrix in all four pull/publish workflows; both root and non-root builds use these matrices.
  • Redeclare ARG GOLANG_VERSION inside the tidb-operator builder stage so its existing go-version label receives the selected version.
Builders Existing variants updated
tidb, pd, ticdc, tiflow, ng-monitoring, tidb-dashboard root/non-root, standard + CentOS 7
tidbx CentOS 7 root/non-root only
tem, tidb-operator standard root/non-root only

Rust/TiKV and C++ builders, existing Go profiles, yum source/verification policy, package routing, generated release YAML, and CI job image references are unchanged. The unrelated pre-existing C++ matrix in the CentOS 7 pull workflow is intentionally unchanged. Original PD Dockerfile CRLF endings are preserved.

Validation

  • All 30 configs parsed with Skaffold 2.23.0: 90/90 effective profile checks across Go 1.27.2, 1.26.9, and 1.25.14; build arguments, targets, image names, tag templates, and both architectures verified.
  • 30/30 skaffold build --dry-run --push=false tag checks.
  • All four workflows: matrix/profile/root/non-root command compatibility, path-trigger coverage, and changed-builder selection checked (7 CentOS 7 + 8 standard builders).
  • actionlint 1.7.11 on the four changed workflows (-shellcheck=; no shell script changes).
  • Docker BuildKit --check for the operator builder with GOLANG_VERSION=1.27.2: no warnings.
  • CRLF-aware whitespace check: git -c core.whitespace=blank-at-eol,blank-at-eof,space-before-tab,cr-at-eol diff --check.
  • Confirmed upstream Go 1.27.2 Linux amd64/arm64 tarball URLs return HTTP 200.
  • Full local image builds: attempted TiDB arm64 non-root build hit a 360-second bound during Rocky Linux dnf package installation, before Go installation; no successful full local build is claimed. Full builds are delegated to PR CI.

Runtime line coverage: not measured (Docker/Skaffold/workflow configuration change). Configuration checks cover every changed profile and workflow.

Change review

Reviewed using pi-deepseek through review-loop (read-only reviewer, Codex author/fixer): round 1 VERDICT: CLEAN, no Critical/Warning/Suggestion findings, 1 minute 26 seconds. No fixer/adjudication step was needed. Branch created from and rechecked/rebased against latest upstream/main before submission.

Publication boundary

This PR does not publish images or change routing. PR image checks use --push=false; publication runs after a separately authorized merge to main. Exact tags must be read from successful publish runs, then verified as manifest lists containing linux/amd64 + linux/arm64, with go-version=1.27.2 on each architecture. No published Go 1.27 tag is claimed here.

Known existing CentOS 7 vault/arm64 failures may still block build/publication; this change does not weaken repository verification or replace yum sources.

Leave unmerged for user review.

@ti-chi-bot ti-chi-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have already done a preliminary review for you, and I hope to help you do a better job.

Summary

This PR updates the continuous delivery (CD) builder pipeline to support Go version 1.27.2 while retaining compatibility with prior Go versions (1.26 and 1.25). The changes involve adding new profiles to Skaffold configurations, modifying Dockerfiles to default to 1.27.2, and updating relevant GitHub workflows to include the new Go version. The approach is thorough, following established conventions for profiles, tags, and environments. The validation checklist and metadata suggest high attention to detail, though the lack of full local builds is noted. Overall, the changes appear well-structured and maintain backward compatibility without introducing major risks.


Critical Issues

No critical issues detected.


Code Improvements

  • Repeated ARG declaration in dockerfiles/cd/builders/tidb-operator/Dockerfile:

    • File: dockerfiles/cd/builders/tidb-operator/Dockerfile, Lines 8–9
    • Issue: ARG GOLANG_VERSION is declared twice in this Dockerfile (before and after FROM) which is redundant and unnecessary.
    • Suggested Solution: Remove the duplicate declaration before FROM golang:${GOLANG_VERSION}.
      # Remove this line:
      ARG GOLANG_VERSION=1.27.2
  • Performance bottleneck in full image builds:

    • File: Not specific to a file, but mentioned in the PR description under "Validation."
    • Issue: The Rocky Linux dnf package installation during arm64 builds hits a 360-second timeout. This could lead to CI failures or deployment delays.
    • Suggested Solution: Investigate and optimize the dnf package installation commands or use a local cache to speed up installation. Alternatively, consider increasing the timeout in CI/CD for this specific build step.

Best Practices

  • Testing coverage gaps:

    • File: Multiple Skaffold configurations (e.g., dockerfiles/cd/builders/ng-monitoring/skaffold.centos7.yaml, etc.)
    • Issue: No runtime validation or integration testing is included in the PR, especially for the newly added Go 1.27 profiles. While dry-run checks are helpful, full builds and compatibility tests would ensure the configurations work as expected.
    • Suggested Solution: Include a CI step to perform actual builds and runtime tests for the new profiles. You could add a matrix job targeting Go 1.27 builds to validate functionality beyond dry-run checks.
  • Documentation for builder changes:

    • File: Multiple Dockerfiles and Skaffold YAML files.
    • Issue: While the PR description is detailed, the individual files lack inline comments explaining the purpose of the new Go 1.27 profiles or environment updates.
    • Suggested Solution: Add comments in each modified Dockerfile and Skaffold configuration to clarify the context and rationale for adding go-1.27 profiles. Example:
      # Added go-1.27 profile for compatibility with Go 1.27.2 toolchain. Retains backward compatibility with Go 1.26 and 1.25.
  • Style guideline deviations in Dockerfiles:

    • File: Multiple Dockerfiles (e.g., dockerfiles/cd/builders/ng-monitoring/Dockerfile)
    • Issue: The curl commands lack retries or verification flags, which can lead to unstable builds if the tarball download fails or is corrupted.
    • Suggested Solution: Use --retry and --retry-connrefused flags, and validate the checksum of the downloaded file. Example:
      RUN OS=linux; ARCH=$([ "$(arch)" = "x86_64" ] && echo amd64 || echo arm64); \
          curl --retry 5 --retry-connrefused -fsSL https://dl.google.com/go/go${GOLANG_VERSION}.linux-${ARCH}.tar.gz | \
          tar -C /usr/local -xz

Conclusion

The PR is generally clean and well-structured, with no major issues. Addressing the redundant ARG declaration, optimizing the dnf installation, enhancing test coverage, and improving comments/documentation will make this update more robust and maintainable.

@ti-chi-bot ti-chi-bot Bot added the size/XL label Oct 16, 2026
@ti-chi-bot ti-chi-bot changed the title feat(cd): add Go 1.27.2 builder images feat(dockerfiles): add Go 1.27.2 builder images Oct 16, 2026
@wuhuizuo

Copy link
Copy Markdown
Contributor

PTAL done — reviewed the full diff against the existing go-1.26 conventions. Clean +1 from me:

  • All 30 Skaffold variants get a go-1.27 profile structurally identical to go-1.26 (same patch ops, tag templates, Renovate annotations, amd64 + arm64). Cross-checked main: go1.26 / go-1.26 references exist only in these 30 files + the 4 workflows — every one is covered here, nothing left behind.
  • The ARG GOLANG_VERSION redeclaration in the tidb-operator builder stage is the canonical Docker pattern (the pre-FROM ARG feeds FROM golang:${GOLANG_VERSION}; the in-stage redeclare feeds LABEL go-version). It is required, not redundant — and it fixes this builder's previously-empty go-version label. (Note for the earlier automated review: please don't act on the suggestion to drop either declaration.)
  • Defaults bumped to 1.27.2 in the 15 Dockerfiles with the Renovate annotations retained. Pull-test CI is green for go-1.27 across all changed builders on both platforms (8 standard + 7 CentOS 7 variants, amd64 + arm64), with go-1.26 / go-1.25 unaffected.
  • Publication boundary is well stated (--push=false, no routing / job-ref changes).

One non-blocking follow-up (pre-existing, not introduced by this PR): PD's Dockerfiles have never been included in Renovate's Go patch PRs — they were stuck at 1.26.1 while every other builder tracked 1.26.5 → 1.26.9. Root cause: pd/Dockerfile uses CRLF line endings, which breaks the repo's customManager regex (depending on the engine, the match fails entirely or captures a trailing \r into the captured dep/version fields), and pd/centos7/Dockerfile is missing the # renovate: datasource=docker depName=golang annotation altogether. This PR correctly aligns both to 1.27.2, but they will keep missing future patch bumps unless the line endings are normalized and the annotation added (or the regex is made CRLF-tolerant). Worth a small follow-up.

Thanks @dillon-zheng — LGTM 👍

@ti-chi-bot ti-chi-bot Bot added the lgtm label Oct 16, 2026
@ti-chi-bot

ti-chi-bot Bot commented Oct 16, 2026

Copy link
Copy Markdown

[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

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot Bot added the approved label Oct 16, 2026
@ti-chi-bot

ti-chi-bot Bot commented Oct 16, 2026

Copy link
Copy Markdown

[LGTM Timeline notifier]

Timeline:

  • 2026-10-16 06:20:12.051307397 +0000 UTC m=+971221.798538256: ☑️ agreed by wuhuizuo.

@ti-chi-bot
ti-chi-bot Bot merged commit 29710e1 into PingCAP-QE:main Oct 16, 2026
107 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants