Skip to content

chore(dockerfiles): add Go 1.27.2 CI runtime profile - #5365

Merged
dillon-zheng merged 2 commits into
PingCAP-QE:mainfrom
dillon-zheng:chore/go1.27.2-runtime-images
Oct 16, 2026
Merged

dillon-zheng merged 2 commits into
PingCAP-QE:mainfrom
dillon-zheng:chore/go1.27.2-runtime-images

Conversation

@dillon-zheng

@dillon-zheng dillon-zheng commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Add a go-1.27 Skaffold profile for Go 1.27.2, preserving the existing go-1.25 profile and Dockerfile defaults.
  • Include the new profile in the CI runtime PR-build and publish matrices. The ci module builds base and dependent jenkins images for linux/amd64 and linux/arm64, using the existing -go1.27 tag convention and go-version label propagation.
  • Repair the existing ci-tici MinIO download step by replacing the two retired dl.min.io URLs with official GitHub release assets, retaining exactly MinIO RELEASE.2025-02-07T23-21-09Z and mc RELEASE.2025-02-08T19-14-21Z, both architectures, installed names, and destinations.
  • No CI job image references, package routing, or unrelated defaults change.

Validation

  • YAML/profile-preservation and workflow trigger/filter/matrix assertions: passed.
  • Skaffold v2.23.0 effective-config checks and build --dry-run for both Go profiles on both architectures: passed. Workflows retain their existing Skaffold v2.16.0.
  • actionlint v1.7.11 on both workflows: passed (-shellcheck=; shellcheck is not installed locally).
  • Go 1.27.2 download endpoints for Linux amd64 and arm64: HTTP 200.
  • git diff --check: passed.
  • Original Go-profile change: read-only DeepSeek review-loop (pi-codex-company, code, max 3 rounds, 2 retries): CLEAN in round 1, 53 seconds; separate final verification PASS, 15 seconds.
  • MinIO follow-up: all four official binary downloads completed, all upstream SHA-256 checksums verified, ELF architectures checked, and four Docker --version smoke tests passed (MinIO/mc × Linux amd64/arm64), confirming both retained releases.
  • MinIO follow-up: Skaffold ci-tici effective-config check and workflow actionlint passed; read-only pi-deepseek review-loop CLEAN in round 1, 55 seconds; separate final verification PASS after all binary/runtime checks completed.
  • Full CI image builds were not run locally; the PR workflow performs real build-only validation. The MinIO runtime smoke tests are isolated binary checks, not full image-build claims.
  • Test coverage: not measured (declarative configuration and two Dockerfile URL substitutions); focused configuration and binary/runtime assertions as listed above. Review/test evidence is local and not committed.

Initial CI failure and follow-up

Initial run: https://github.com/PingCAP-QE/ci/actions/runs/38847005508

  • ci-tici arm64 failed at MinIO download (wget exit 8). Both old server/client URLs return HTTP 410 Gone for both architectures; the retired site no longer serves community releases, so retrying alone cannot recover this job. The follow-up changes only the two download URLs, not the MinIO versions or installation policy.
  • Go 1.25 arm64 failed connecting to archive.apache.org for Maven (wget exit 4, connection timeout). The same Maven download succeeded in the same run's Go 1.27 arm64 job, and the endpoint now returns HTTP 200: no Maven source change is made for this transient failure.
  • Go 1.27 arm64 base and jenkins builds succeeded in the initial run. The two Go amd64 jobs were cancelled by matrix fail-fast, not shown to have Go compilation failures.
  • The follow-up ordinary push triggers a fresh PR workflow, covering the failed and cancelled jobs; final online build results remain required before merge.

Publication boundary

PR checks use --push=false: this PR does not publish an image or establish a usable downstream tag. After a separately authorized merge and a successful publish run, retrieve the exact ghcr.io/pingcap-qe/ci/{base,jenkins}:<tag>-go1.27 tags and verify the final manifests include both Linux architectures and the image's go-version=1.27.2 before updating downstream CI job references.

This PR is left unmerged for user review; auto-merge is not enabled.

@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 introduces a new Skaffold profile for Go 1.27.2 alongside the existing Go 1.25 profile, updating GitHub Actions workflows to build and release images for both Go versions on amd64 and arm64 platforms. The approach uses YAML patches in Skaffold config and updates the build matrix accordingly. The changes are well scoped, preserving existing defaults and CI logic. Overall, the PR is clean and follows a safe incremental approach. However, some improvements and verifications can enhance maintainability and robustness.


Critical Issues

  • No critical issues identified. The changes are limited to CI and build configuration updates without breaking existing functionality.

Code Improvements

  • Inconsistent patch operation in skaffold.yaml (lines 37-51):
    The replace operation for the /build/tagPolicy/customTemplate/template path assumes that this path always exists. If it does not, replace will fail. Using add or replace conditionally or confirming the path exists in the base config is safer.

    Suggestion:
    Confirm the base Skaffold config has this path for the go-1.25 profile or consider using add with - op: add if uncertain, or combine with - op: replace only after validation.

  • Hardcoded Go version in Skaffold profile buildArgs (lines 45-50):
    The Go version string 1.27.2 is hardcoded in both the patch and the Dockerfile build arguments. To reduce duplication and ease future upgrades, consider using a variable or centralized version management.

    Suggestion:
    If possible, define a variable or use a renovate-compatible datasource comment on the version line to automate updates.

  • Lack of explicit Skaffold buildArgs merge (lines 45-50):
    The patch uses - op: add at /build/artifacts/0/docker/buildArgs. If the base profile already has buildArgs, this will cause an error or overwrite. Confirm that the base profile has no existing buildArgs or use a merge strategy.

    Suggestion:
    Use - op: add with merge strategy or verify base config. Alternatively, patch the GOLANG_VERSION key inside existing buildArgs rather than replacing the whole object.


Best Practices

  • Missing comments for new Skaffold profile (lines 37-52):
    The new go-1.27 profile YAML patch lacks explanatory comments like the go-1.25 profile’s # renovate: datasource=docker depName=golang comment on the Go version. Adding comments improves maintainability and helps automation tools.

    Suggestion:
    Add comments before the profile or buildArgs section to explain purpose and renovate integration.

  • Testing coverage note in PR description:
    The PR notes that test coverage is "not measured." While understandable for declarative config, consider adding integration or smoke tests that confirm the new Go version profile builds the expected images successfully.

    Suggestion:
    Add a minimal CI job or test to build the new profile image and verify the go-version label or Go runtime version inside the container.

  • Workflow matrix duplication (lines 60-65 in both workflow files):
    The matrix update is repeated in two similar workflow files. Consider extracting matrix definitions into reusable workflow templates or YAML anchors to reduce duplication and risk of inconsistent updates.

    Suggestion:
    Refactor workflows to use reusable matrices or templates if the project growth justifies it.


Minor style / naming notes

  • The go-profile matrix values use go-1.25 and go-1.27. Consider using a consistent version format like go1.25 / go1.27 without dashes if that matches other parts of the repo or Docker tags to avoid confusion.

Summary of suggested actions:

- Confirm Skaffold patch `replace` operation path exists or switch to safer patch operation.
- Ensure `buildArgs` patch merges with existing args rather than overwriting.
- Add renovate datasource comments and clarifying comments to new Skaffold profile.
- Add minimal integration tests verifying build and Go version label.
- Consider workflow matrix DRY refactor if applicable.
- Review naming consistency for `go-profile` values.

@ti-chi-bot ti-chi-bot Bot added the size/S label Oct 16, 2026

@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 introduces a new Skaffold profile go-1.27 to support Go 1.27.2 in CI runtime Docker builds, alongside the existing Go 1.25 profile. It updates CI workflow matrices to build and test both Go versions on amd64 and arm64 platforms. The changes are well-contained to declarative configuration and workflow YAML, preserving backward compatibility and existing defaults. The PR is well-documented with thorough validation steps, though Docker builds were not tested locally. Overall, the changes appear sound and maintainable.


Critical Issues

  • Dockerfile MinIO and mc download URLs (dockerfiles/ci/tici/Dockerfile, lines ~9-15)
    The PR changes MinIO and mc download URLs from dl.min.io to GitHub release assets with a hardcoded future release date (RELEASE.2025-02-07T23-21-09Z).
    • Issue: This introduces a potential fragility or breakage if those GitHub release URLs are not stable or the release artifacts are not present as expected. It also introduces a hardcoded future timestamp, which may cause build failures if the files are not available yet.
    • Recommendation: Confirm if this URL change is intentional and tested. If this is a fix or update unrelated to Go version, consider splitting it out into a separate PR. Otherwise, at minimum, document the rationale and confirm the URLs' availability. Consider adding a retry or checksum verification for downloads to improve reliability.

Code Improvements

  • Skaffold Profile Patch Path (dockerfiles/ci/skaffold.yaml, lines ~38-48)
    The profile for go-1.27 patches the tagPolicy.customTemplate.template and adds build args for GOLANG_VERSION.

    • Improvement: Consider explicitly replacing the whole buildArgs map instead of using add which may cause merge conflicts if other build args exist. Using replace for the whole buildArgs path ensures the profile is self-contained and avoids unintended inheritance.
    • Suggested patch snippet:
      - op: replace
        path: /build/artifacts/0/docker/buildArgs
        value:
          GOLANG_VERSION: 1.27.2
  • Workflow Matrix Consistency (.github/workflows/pull-ci-runtime-images.yaml and release-ci-runtime-images.yaml)
    The matrix adds go-profile: [go-1.25, go-1.27].

    • Improvement: Verify that all jobs and steps consuming go-profile handle multiple profiles correctly, including any conditional logic. Consider adding explicit job names or output annotations reflecting the Go version for easier debugging in CI logs.

Best Practices

  • Documentation
    The PR description is detailed and helpful. However, the Skaffold profile addition in skaffold.yaml lacks inline comments explaining the patch rationale. Adding comments improves maintainability.

    • Suggested addition:
      - name: go-1.27
        # Profile for Go 1.27.2 builds - tags images with go1.27 and sets Go version build arg
        patches:
        ...
  • Test Coverage
    The PR notes that test coverage is not measured due to declarative config. Consider adding CI steps that run basic smoke tests or build verification on the new Go 1.27 images once published, to catch runtime incompatibilities early.

  • Naming Conventions
    The go-profile names go-1.25 and go-1.27 follow a consistent pattern. Confirm this naming is aligned with other tooling consuming these labels to avoid confusion.


Overall, this is a well-prepared PR extending CI support for Go 1.27 with minimal risk. Address the noted Dockerfile download URL concerns and consider the suggested improvements for Skaffold patching and documentation to enhance robustness and maintainability.

@wuhuizuo

Copy link
Copy Markdown
Contributor

PTAL done — reviewed the full diff. Looks good:

  • The go-1.27 profile mirrors go-1.25 exactly (same patch ops, {{ .GIT }}-go1.27 tag template, Renovate annotation). GOLANG_VERSION flows into the base artifact and reaches jenkins via BASE_IMG; the go-version label will reflect the selected toolchain.
  • Coverage is complete: cross-checked go-1.25 references on main — only dockerfiles/ci/skaffold.yaml plus the two workflows, all updated here. Downstream job image references are correctly left untouched.
  • The bundled ci-tici MinIO/mc repair is verified: all four replacement URLs (minio + mc × linux/amd64 + linux/arm64) return HTTP 200, and the old dl.min.io URLs indeed return HTTP 410 Gone. CI now passes on both arches for ci-tici, where the arm64 job previously failed — bundling this fix was justified since it was blocking this PR's own CI, and the pinned release versions are unchanged.
  • Publication boundary is properly stated.

Thanks @dillon-zheng — LGTM 👍

@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 lgtm 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:51:34.411848265 +0000 UTC m=+973104.159079134: ☑️ agreed by wuhuizuo.

@ti-chi-bot ti-chi-bot Bot added the approved label Oct 16, 2026
@dillon-zheng
dillon-zheng merged commit a06200f into PingCAP-QE:main Oct 16, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants