Skip to content

chore(cd): rebuild all CentOS 7 Go builder images - #1115

Merged
ti-chi-bot[bot] merged 1 commit into
PingCAP-QE:mainfrom
dillon-zheng:chore/trigger-all-centos7-go-builders
Oct 16, 2026
Merged

ti-chi-bot[bot] merged 1 commit into
PingCAP-QE:mainfrom
dillon-zheng:chore/trigger-all-centos7-go-builders

Conversation

@dillon-zheng

Copy link
Copy Markdown
Contributor

Purpose

Rebuild and publish the full seven-component CentOS 7 Go-builder matrix using the repaired code, rather than stopping after TiDBx alone was recovered.

Change

Seven whitespace-only insertions, one blank line per existing skaffold.centos7.yaml:

  • ng-monitoring
  • pd
  • ticdc
  • tidb
  • tidb-dashboard
  • tidbx
  • tiflow

The parsed configurations are unchanged. The merge push matches the existing release paths and changed-builder parser, selecting all seven builders for Go 1.27 / 1.26 / 1.25, including root + non-root images and linux/amd64 + linux/arm64.

This produces the existing 21 component/profile publish jobs, each building both image variants for both architectures. The branch starts from upstream/main after the TiDBx repository repair.

No Dockerfile, version, profile, workflow logic, Rust/C++ builder, routing, or generated release-file change is included.

Validation

  • Exactly seven one-blank-line insertions; YAML parsing is identical before/after for every changed file.
  • Replayed both actual existing PR/release changed-scope shell parsers on the uncommitted diff: exactly all seven Go builders selected, Rust/C++ arrays empty.
  • Both workflows' path filters, three Go profiles, and root/non-root build steps cover the changes.
  • Skaffold 2.23.0: 42/42 root/non-root × profile × component effective configs resolve, preserving exact Go 1.27.2 / 1.26.9 / 1.25.14 and both architectures.
  • Branch contains merged repair e392e375; fetched/rebased against upstream before submission.
  • git diff --check.
  • Read-only pi-deepseek review-loop: CLEAN, first round (23 seconds), no findings/fixes.

Runtime line coverage: not measured (whitespace-only trigger). Full image builds are performed by PR CI and, after merge, the release workflow; no publication success or future tag is claimed yet.

Completion boundary

PR checks use --push=false; merging this trigger PR is what starts the full publication run. Record exact tags from that successful publish run, verify final root/non-root manifest lists contain both architectures and the expected per-architecture Go labels, and only then consider the builder publication complete. No downstream routing changes are made here.

Leave 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 aims to trigger a full rebuild of all seven CentOS 7 Go builder images by inserting a single blank line in each skaffold.centos7.yaml file for those components. This is a minimal, whitespace-only change designed to force CI and release workflows to pick up and publish all builders, not just the previously fixed TiDBx one. The approach is sound and low-risk since it does not alter any actual build configuration or logic. The PR is well-documented and validated for YAML parsing and workflow selection. Overall, this is a clean and effective way to restart the full build matrix.


Code Improvements

  • Unnecessary use of whitespace change to trigger rebuilds

    • Files: All changed skaffold.centos7.yaml files (e.g., dockerfiles/cd/builders/ng-monitoring/skaffold.centos7.yaml and others)
    • Why: While the whitespace-only blank line insertions trigger rebuilds, this approach can be confusing to reviewers and future maintainers since it does not explicitly document the intent in the YAML or workflow files. It relies on side-effect rather than explicit configuration.
    • Suggestion: Consider adding a dedicated flag or comment in the YAML or the CI workflow to indicate a forced rebuild rather than whitespace hacks. For example:
      # Force rebuild trigger: true
      Or update workflow logic to support a manual "rebuild all" trigger event or input parameter, improving clarity and maintainability.
  • No validation that CI actually triggers rebuilds of all images

    • Why: The PR description states that image builds are performed by PR CI and release workflow, but no evidence (logs, artifacts links) is provided that all 21 component/profile jobs were successfully triggered and ran.
    • Suggestion: Include links or outputs from the triggered workflow run showing all jobs started and completed, or add a smoke test step to validate all images are rebuilt and pushed as expected.

Best Practices

  • Documentation could be clearer in YAML files

    • Files: All changed skaffold.centos7.yaml files
    • Why: The blank lines added are not self-explanatory. New maintainers might not understand why these whitespace changes exist or what triggers the rebuilds.
    • Suggestion: Add an inline comment above or below the blank line, e.g.:
      tryImportMissing: true
      # Blank line added to force CI rebuild of this component
      This improves transparency and future troubleshooting.
  • No tests or CI validation for rebuild trigger logic

    • Why: Since the PR is whitespace-only, there is no automated test coverage or validation that the forced rebuild actually happens.
    • Suggestion: Implement a CI job or workflow step that explicitly checks which builders are selected and built, comparing against the expected list. This would help catch regressions in rebuild triggering logic.

Summary of Actionable Feedback

  • Avoid relying on whitespace-only changes to trigger rebuilds; instead, introduce explicit flags or parameters in YAML or workflows to control rebuilds.
  • Add inline comments in each YAML file where blank lines are inserted to document the purpose of the change.
  • Consider adding or linking to CI outputs demonstrating that all expected build jobs are triggered and completed.
  • Implement CI validation steps to verify rebuild triggers and selection logic, preventing silent failures or missed rebuilds in the future.

This PR is low-risk and well-documented but would benefit from explicitness and improved observability in the rebuild triggering mechanism.

@ti-chi-bot ti-chi-bot Bot added the size/XS label Oct 16, 2026
@dillon-zheng

Copy link
Copy Markdown
Contributor Author

/approve

@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: dillon-zheng

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 merged commit 4832c5c into PingCAP-QE:main Oct 16, 2026
52 checks passed
@dillon-zheng
dillon-zheng deleted the chore/trigger-all-centos7-go-builders branch October 16, 2026 14:02
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.

1 participant