fix(pipelines/tiflash): release-6.5 IT image rewrite + failpoint fallback

[kumailf]

Summary

Unblock release-6.5 TiFlash pull_integration_test (e.g. pingcap/tiflash#10993):

1. Untagged tiflash-ci-base rewrite
   release-6.5 yamls still use hub.pingcap.net/tiflash/tiflash-ci-base without a tag. Existing rocky8/rocky9 sed rules miss it, so compose hits the retired registry. Add a sed rule mapping the untagged form to TIFLASH_TEST_IMAGE (ghcr.io/pingcap-qe/cd/builders/tiflash:v20231106).

2. TiDB failpoint image fallback (from closed fix(pipelines): fallback when release-6.5 tidb failpoint image missing #4845)
   If tidb-server:release-6.5-failpoint cannot be pulled, fall back to the regular release-6.5 TiDB image for compose rewrites, and skip tidb-ci/fail-point-tests (requires a real failpoint binary).

Test plan

• Merge this PR
• On Release 6.5.6 hotfix 20260720 pingcap/tiflash#10993, run /test pull-integration-test
• Confirm no hub.pingcap.net pull for tics0 / tiflash-ci-base
• If failpoint tag is missing: logs show WARN fallback + skip fail-point-tests; other suites still run
• If failpoint tag exists: pull succeeds and fail-point-tests still run

[ti-chi-bot]

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

Summary:
This PR addresses a critical issue in the release-6.5 TiFlash integration pipeline where untagged references to hub.pingcap.net/tiflash/tiflash-ci-base were not properly rewritten, resulting in pull timeouts and relying on a retired registry. The fix adds a sed rule to rewrite untagged tiflash-ci-base image references to the designated TIFLASH_TEST_IMAGE. The change is minimal, focused, and the PR description includes clear verification steps. Overall, the approach is sound and the code change is concise and directly addresses the problem.

---

Code Improvements

• Robustness of the sed regex:

  • File: pipelines/pingcap/tiflash/release-6.5/pull_integration_test.groovy (line ~282)
  • Issue: The added sed rule uses the pattern:

    -e 's#[[:alnum:].-]*[.][[:alnum:].-]*/tiflash/tiflash-ci-base$#'"${TIFLASH_IMAGE}"'#g'

    This matches <host>/tiflash/tiflash-ci-base only if it occurs at the end of a line, which might miss cases where the image reference is followed by whitespace, newline, or other YAML syntax (e.g., tags, digests, or comments).
  • Suggestion: Relax the regex to match the untagged image reference even if it is followed by a tag or other delimiters. For example:

    -e 's#[[:alnum:].-]*[.][[:alnum:].-]*/tiflash/tiflash-ci-base\(:[^[:space:]]*\)\?#'"${TIFLASH_IMAGE}"'#g'

    This matches tiflash-ci-base optionally followed by a tag (like :latest or :rocky8-...) or no tag at all, and replaces the entire match. Alternatively, use a word boundary or anchor more carefully.

• Ordering of sed rules:

  • Ensure the new rule is placed before or after existing similar rules logically, so that no unexpected overwrites occur. The current placement seems appropriate but verify that no conflicts arise.

• Variable naming clarity:

  • The PR mentions TIFLASH_TEST_IMAGE in the description, but the variable in the sed substitution is ${TIFLASH_IMAGE}. Confirm that these are the same or update the description or variable name for consistency.

---

Best Practices

• Testing confirmation in PR:

  • The PR description includes a manual test plan involving running /test pull-integration-test and checking logs for absence of hub.pingcap.net. It would be better to automate verification by adding a unit or integration test that validates the sed substitution logic or the resulting YAML files after substitution.

• Documentation / Comments:

  • File: pipelines/pingcap/tiflash/release-6.5/pull_integration_test.groovy
  • Suggestion: Add a short comment above the sed block explaining the purpose of each substitution, especially the new rule for untagged tiflash-ci-base. This helps future maintainers understand the rationale.

• Commit message clarity:

  • The commit message and PR title are clear but consider explicitly stating the exact image used as replacement in the title or first line, e.g.:

    fix(pipelines/tiflash): rewrite untagged tiflash-ci-base to ghcr.io/pingcap-qe/cd/builders/tiflash:v20231106 on release-6.5
    

---

Critical Issues

No critical bugs or regressions introduced by this change were observed. The patch is minimal and targeted.

---

Summary of actionable changes:

- -e 's#[[:alnum:].-]*[.][[:alnum:].-]*/tiflash/tiflash-ci-base$#'"${TIFLASH_IMAGE}"'#g' \
+ -e 's#[[:alnum:].-]*[.][[:alnum:].-]*/tiflash/tiflash-ci-base\(:[^[:space:]]*\)\?#'"${TIFLASH_IMAGE}"'#g' \

Add a comment above the sed block, e.g.:

// Rewrite tiflash base images to the new ghcr.io registry.
// Handles tagged and untagged tiflash-ci-base references to avoid pulling from retired hub.pingcap.net.

Consider adding a simple test script or unit test to verify these substitutions automatically.

[ti-chi-bot]

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

Summary
This PR addresses a critical issue in the release-6.5 TiFlash integration pipeline where some YAML files reference the tiflash-ci-base image without a tag, causing image pulls to fail due to referencing a retired registry. The fix adds a new sed substitution rule to rewrite untagged tiflash-ci-base image references to the correct image tag defined by TIFLASH_IMAGE. The approach is straightforward and integrates well with existing sed replacements. The PR also improves robustness around pulling failpoint images by adding fallback logic and disabling related tests if necessary. Overall, the changes are focused, well-justified, and the verification steps seem thorough.

---

Critical Issues

• None identified. The patch correctly fixes the missing tag issue and improves pipeline resilience.

---

Code Improvements

• Refactor sed commands for readability and maintainability
  File: pipelines/pingcap/tiflash/release-6.5/pull_integration_test.groovy (lines ~277-288)
  Currently, multiple -e sed expressions are chained in a single command, which can be error-prone and hard to maintain. Consider extracting these substitutions into a dedicated script or at least formatting them as a multiline string for clarity. This also reduces the risk of missing edge cases or introducing syntax errors.

  Suggested approach:

  def sedExpressions = [
      "s#\\\${TIFLASH_IMAGE:[^}]*}#${TIFLASH_IMAGE}#g",
      "s#[[:alnum:].-]*[.][[:alnum:].-]*/tiflash/tiflash-ci-base:rocky8-20241028#${TIFLASH_IMAGE}#g",
      "s#[[:alnum:].-]*[.][[:alnum:].-]*/tiflash/tiflash-ci-base:rocky9-20250529#${TIFLASH_IMAGE}#g",
      "s#[[:alnum:].-]*[.][[:alnum:].-]*/tiflash/tiflash-ci-base$#${TIFLASH_IMAGE}#g",
      "s#[[:alnum:].-]*[.][[:alnum:].-]*/tiflash/tics:${TAG:-master}#${TIFLASH_IMAGE}#g",
      "s#[[:alnum:].-]*[.][[:alnum:].-]*/tikv/pd/image:${PD_BRANCH:-master}#${PD_IMAGE}#g",
      "s#[[:alnum:].-]*[.][[:alnum:].-]*/tikv/pd/image:master#${PD_IMAGE}#g"
  ]
  
  def sedCommand = sedExpressions.collect { "-e '${it}'" }.join(' ')
  sh "sed -i ${sedCommand} ..."

• Comment clarity for failpoint fallback logic
  The comments explaining the fallback for missing failpoint images are helpful but could be clearer about why skipping the tests is safe and what impact it has on coverage. Consider elaborating slightly or linking to documentation about failpoint tests.

• Edge case: sed pattern anchoring for untagged image
  The new sed rule matches untagged tiflash-ci-base images with $ anchoring to end of string. Verify if this will cover all cases, e.g., if the image reference sometimes has trailing whitespace or is embedded in longer strings. You might want to make the regex more robust by allowing optional trailing spaces or ensuring word boundaries:

  -e 's#[[:alnum:].-]*[.][[:alnum:].-]*/tiflash/tiflash-ci-base\( \|$\)#'"${TIFLASH_IMAGE}\1"'#g'

---

Best Practices

• Add inline comments for sed expressions
  File: pull_integration_test.groovy lines ~277-288
  Each sed expression would benefit from a brief comment explaining what form of image reference it targets. This aids future maintainers in understanding why each pattern is necessary.

• Testing coverage
  The PR description mentions manual verification steps and relies on subsequent CI jobs. Consider adding automated tests or pipeline validation steps that check for hub.pingcap.net references after sed rewriting to catch regressions early.

• Consistent naming in logs
  In the failpoint fallback code, the log message references ${TIDB_FAILPOINT_IMAGE} and ${TIDB_IMAGE}, which is clear. However, the variable TIDB_FAILPOINT_IMAGE is reassigned within the same block, which might confuse readers. Consider a distinct variable name for the fallback image or add a comment clarifying the reassignment.

---

Summary of action items:

• Refactor sed substitutions for better readability and maintainability.
• Add inline comments explaining sed regexes and failpoint fallback logic.
• Review and strengthen regex anchoring for untagged image substitution to prevent partial matches.
• Add or suggest automated test coverage for image rewriting correctness.
• Clarify variable usage and logging in failpoint image fallback block.

[dillon-zheng]

lgtm

[ti-chi-bot]

[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:

• pipelines/pingcap/tiflash/OWNERS [dillon-zheng]

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]

[LGTM Timeline notifier]

Timeline:

• 2026-07-29 04:35:15.803664977 +0000 UTC m=+1984301.839760033: ☑️ agreed by dillon-zheng.