feat(pipelines/ticdc): update Jenkins image tag to v2026.6.28-8-g92327eaa-go1.25#4767
Conversation
![ti-chi-bot[bot]](https://avatars.githubusercontent.com/in/214286?s=60&v=4){kind=small}
There was a problem hiding this comment.
I have already done a preliminary review for you, and I hope to help you do a better job.
Summary
This PR updates 66 pipeline pod template files to use a new Jenkins image tag (v2026.6.28-8-g92327eaa) that fixes Python 3.12 symlink issues. The change is mechanical, replacing image tags uniformly across multiple Kubernetes YAML files. The changes are straightforward, but as they touch many files, ensuring consistency and proper testing is critical. Overall, the PR is low-risk but requires verification of downstream effects on CI pipelines and integration tests.
Critical Issues
None identified. The PR is a mechanical change limited to updating image tags, and the new image has been verified to include the required fixes.
Code Improvements
- Testing Verification:
- Files: All YAML files affected.
- Issue: The PR description mentions testing, but there is no evidence of automated testing or validation scripts included in the PR to verify pipeline functionality with the new image tag.
- Suggestion: Add or link to a script that validates pipeline functionality (e.g., checking that
python3 --versionoutputs Python 3.12 across all pipelines).
Best Practices
-
Consistency Checks:
- Files: All updated YAML files.
- Issue: The PR touches many files, increasing the risk of missing a file or introducing inconsistent changes (e.g., mismatched tags). While the diff appears consistent, cross-referencing these changes with a list of all affected pipelines would avoid potential omissions.
- Suggestion: Include a checklist or automated validation showing all pipeline files have been updated.
-
Documentation:
- Files: PR description and/or repository docs.
- Issue: The PR does not specify why Python 3.12 is necessary or what specific issues the symlink fix resolves.
- Suggestion: Add a brief note to the repository or PR description explaining the importance of Python 3.12 and the symlink fix for future maintainability.
-
Testing Coverage:
- Files: All YAML files affected.
- Issue: No mention of backward compatibility testing (i.e., ensuring older pipelines won't break with the new image).
- Suggestion: Define backward compatibility tests, if applicable, or explicitly state why they are unnecessary.
Suggested Actions
- Add automated validation for pipeline functionality with the updated image.
- Confirm all affected pipelines are accounted for and uniformly updated.
- Enhance documentation to explain the significance of the Python 3.12 update and symlink fix.
- Define backward compatibility testing or confirm it is not relevant.
Closing Notes
The changes are low-risk and mechanically applied, but their widespread nature necessitates careful validation to avoid disruptions in CI pipelines. Adding automated validation and updating documentation would improve confidence in this PR.
…7eaa Update all ticdc pod templates from v2026.6.28-3-g80620cc5-go1.25 to v2026.6.28-8-g92327eaa to pick up the Python 3.12 symlink fix from PR #4766. Ref: FLA-231 Signed-off-by: wuhuizuo <wuhuizuo@126.com>
2a65d83 to
e31b9e0
Compare
There was a problem hiding this comment.
I have already done a preliminary review for you, and I hope to help you do a better job.
Summary
This pull request updates the Jenkins image tag across 66 configuration files in the ticdc pipelines to a newer version (v2026.6.28-8-g92327eaa). The new image includes fixes for Python 3.12 symlink issues, ensuring compatibility and improved functionality. The changes are consistent and straightforward, replacing the old image tag with the new one in YAML files for CI pipeline configurations. Overall, the PR is low-risk and well-documented, focusing solely on a version update without introducing new functionality or logic.
Critical Issues
No critical issues identified. The PR is a simple image tag update and does not introduce new logic or potential vulnerabilities.
Code Improvements
- Image Version Standardization:
-
Issue: In some files, the image tag includes the suffix
-go1.25, while others omit it (e.g.,pull_cdc_kafka_integration_heavy/pod-test.yaml). -
Impact: This inconsistency might lead to confusion or unexpected behaviors during deployments, as it is unclear whether all configurations should use the same variant of the image.
-
Suggested Fix:
Ensure all image tags consistently include the appropriate suffix or remove the suffix entirely, depending on the intended behavior. Verify the correct image version to use for each pipeline.# Example fix (ensure consistent suffix across files) image: "ghcr.io/pingcap-qe/ci/jenkins:v2026.6.28-8-g92327eaa-go1.25"
-
Best Practices
-
Testing Coverage:
- Issue: The PR description mentions verifying CI pipelines and integration tests, but there is no indication of automated tests explicitly checking the functionality of the updated image tag.
- Impact: Without automated validation, manual verification may miss edge cases or regressions introduced by the new image.
- Suggested Fix:
Add automated tests to confirm that pipelines using the updated image tag successfully build and run the expected Python version (3.12.x). For example, append a test step to existing CI pipelines that runspython3 --versionand validates the output.
-
Documentation:
-
Issue: There is no explicit note in the YAML files indicating why the image tag was updated.
-
Impact: Future maintainers may not understand the purpose of the change without additional context.
-
Suggested Fix:
Add a comment in each updated YAML file to document the reason for the change.# Updated image tag to v2026.6.28-8-g92327eaa to fix Python 3.12 symlink issues. image: "ghcr.io/pingcap-qe/ci/jenkins:v2026.6.28-8-g92327eaa-go1.25"
-
-
Style Consistency:
-
Issue: Some YAML files use double quotes for the image tag, while others use single quotes or no quotes.
-
Impact: Inconsistent quoting can reduce readability and increase the likelihood of formatting errors.
-
Suggested Fix:
Standardize quoting throughout all YAML files (e.g., use double quotes for all image tag strings).image: "ghcr.io/pingcap-qe/ci/jenkins:v2026.6.28-8-g92327eaa-go1.25"
-
Conclusion
The PR is well-structured and achieves its intended purpose effectively, but there are minor consistency issues and documentation gaps that should be addressed. Implementing the suggested improvements would enhance maintainability and reduce potential confusion for future updates.
wuhuizuo
changed the title
There was a problem hiding this comment.
I have already done a preliminary review for you, and I hope to help you do a better job.
Summary
This pull request updates 66 Kubernetes pod templates across multiple directories to use a new Jenkins image (v2026.6.28-8-g92327eaa-go1.25) that includes fixes for Python 3.12 symlinks. The changes are primarily YAML modifications to update the image field, with minor adjustments to validation scripts to support image verification using crane. Overall, the PR is low-risk but introduces potential dependencies on the crane binary, which could impact CI validation workflows.
Critical Issues
-
Dependency on
cranebinary:- File:
.ci/verify-k8s-pod-yaml.sh(lines 16–112),prow-jobs/pingcap-qe/ci/presubmits.yaml(lines 188–192) - Issue: The addition of
cranefor image validation introduces a dependency that may not be available in all environments. Ifcranefails to install during CI setup (e.g., due to network issues or changes in the source URL), it could break validation workflows. - Suggested Solution: Add fallback logic to skip
cranevalidation if installation fails, or ensure the binary is pre-installed in the CI environment. Example:if ! wget -qO /tmp/crane.tar.gz "https://github.com/google/go-containerregistry/releases/download/v0.21.5/go-containerregistry_Linux_x86_64.tar.gz"; then echo "Failed to download crane. Skipping image validation." crane_validation_enabled=0 else tar xzf /tmp/crane.tar.gz -C /usr/local/bin/ crane && rm /tmp/crane.tar.gz fi
- File:
-
Hardcoded
cranebinary URL:- File:
prow-jobs/pingcap-qe/ci/presubmits.yaml(lines 188–192) - Issue: The hardcoded URL for the
cranebinary creates a risk of future breakages if the URL changes or becomes unavailable. - Suggested Solution: Pin the
craneversion as part of a centralized dependency management strategy or use a mirror/repository controlled by the organization.
- File:
Code Improvements
- Validation fallback for missing
crane:- File:
.ci/verify-k8s-pod-yaml.sh(lines 16–112) - Issue: The script assumes
craneis installed and executable without providing adequate fallback mechanisms. Ifcraneis unavailable, the script should gracefully skip the validation step rather than fail outright. - Suggested Solution: Add a fallback mechanism as described under "Critical Issues."
- File:
Best Practices
-
Testing gaps for
cranevalidation:- File:
.ci/verify-k8s-pod-yaml.sh - Issue: There is no indication that the
cranevalidation logic has been tested under different scenarios, such as missing/inaccessible images or failures in the registry lookup. - Suggested Solution: Include unit tests or integration tests to verify that
cranehandles edge cases (e.g., invalid image tags, inaccessible registries).
- File:
-
Documentation for
cranedependency:- File:
.ci/verify-k8s-pod-yaml.sh,prow-jobs/pingcap-qe/ci/presubmits.yaml - Issue: The addition of
craneis not documented, which could hinder future developers or maintainers from understanding its purpose and installation process. - Suggested Solution: Add comments explaining the purpose of
craneand its installation steps. For example:# Install crane for container image validation # Crane is used to verify image digests in Kubernetes pod templates.
- File:
-
General YAML formatting consistency:
- Files: Multiple
.yamlfiles (e.g.,pipelines/pingcap/ticdc/latest/...) - Issue: Some YAML files have inconsistent indentation (e.g., line 14 in
.ci/verify-jenkins-pipelines.sh). - Suggested Solution: Ensure consistent YAML formatting using tools like
yamllintor IDE auto-formatting.
- Files: Multiple
Additional Notes
- The PR is primarily a tag update, and the risk level is low. However, the added
cranedependency warrants careful attention to ensure CI workflows remain robust. - No performance bottlenecks or logical errors are evident in the changes.
There was a problem hiding this comment.
I have already done a preliminary review for you, and I hope to help you do a better job.
Summary
This PR updates the image tag for Jenkins containers in 66 Kubernetes pod templates across the ticdc pipelines. The new image tag addresses Python 3.12 symlink issues and includes a minor script improvement for validating container images using crane. The changes are straightforward, focusing on maintaining compatibility and ensuring pipelines run with the correct environment. Overall, the PR is well-contained but introduces a minor script enhancement that warrants closer review.
Critical Issues
- Validation Logic in
.ci/verify-k8s-pod-yaml.sh- File:
.ci/verify-k8s-pod-yaml.sh - Lines: 59–112
- Issue: The new
validate_images_with_cranefunction fails gracefully whencraneis unavailable; however, it does not distinguish between missing images in a manifest and errors during validation. Additionally, sorting and deduplication of images could miss invalid entries ifyqparsing fails silently. - Suggested Fix: Explicitly handle errors from
yqandcraneto ensure validation failures are reliably flagged. For example:validate_images_with_crane() { file=$1 images=$(yq e '.spec.containers[].image, .spec.initContainers[].image' "$file" 2>/dev/null || { echo "Error parsing YAML: $file"; failed=1; return; }) [ -z "$images" ] && return images=$(echo "$images" | grep -v '^$' | grep -v '^null$' | sort -u) for image in $images; do if ! "$CRANE_BIN" digest "$image" >/dev/null 2>&1; then echo "$file: image not found or invalid: $image" failed=1 fi done }
- File:
Code Improvements
-
Script Error Handling Enhancements
- File:
.ci/verify-k8s-pod-yaml.sh - Lines: 52–112
- Issue: The script does not enforce error handling for temporary file operations (
manifest,manifest_with_ns,stderr_file). Errors during cleanup or file creation could lead to silent failures. - Suggested Fix: Use
trapto ensure proper cleanup of temporary files and add error handling for file operations:trap 'rm -f "$manifest" "$manifest_with_ns" "$stderr_file"' EXIT validate_with_kubectl() { file=$1 manifest=$(mktemp) || { echo "Failed to create temp file"; exit 1; } manifest_with_ns=$(mktemp) || { echo "Failed to create temp file"; exit 1; } stderr_file=$(mktemp) || { echo "Failed to create temp file"; exit 1; } ... }
- File:
-
craneInstallation in CI Pipeline- File:
prow-jobs/pingcap-qe/ci/presubmits.yaml - Lines: 185–190
- Issue: The pipeline installs
craneusing a hard-coded URL. This approach lacks version management and introduces risk if the URL changes or the binary is corrupted. - Suggested Fix: Use version-controlled installation mechanisms and validate the checksum of the binary:
args: - | apk add --no-cache kubectl yq-go wget tar CRANE_VERSION="v0.21.5" wget -qO /tmp/crane.tar.gz "https://github.com/google/go-containerregistry/releases/download/${CRANE_VERSION}/go-containerregistry_Linux_x86_64.tar.gz" echo "expected_checksum_value /tmp/crane.tar.gz" | sha256sum -c - || { echo "Checksum verification failed"; exit 1; } tar xzf /tmp/crane.tar.gz -C /usr/local/bin/ crane rm /tmp/crane.tar.gz
- File:
Best Practices
-
Testing Coverage for New Validation Logic
- File:
.ci/verify-k8s-pod-yaml.sh - Issue: The newly added
cranevalidation lacks explicit testing coverage in the PR description. While integration tests are listed, there is no mention of testing for edge cases like missing images, invalid manifests, or unavailablecrane. - Suggested Fix: Add unit tests for the script, specifically targeting scenarios where
craneis absent, manifests are malformed, and images are missing:# Simulate crane absence CRANE_BIN="/invalid/path/to/crane" .ci/verify-k8s-pod-yaml.sh || echo "Test passed: Validation failed as expected due to missing crane" # Test with malformed YAML echo "invalid_yaml" > test.yaml .ci/verify-k8s-pod-yaml.sh test.yaml || echo "Test passed: Validation failed as expected due to invalid YAML"
- File:
-
Documentation for
craneIntegration- File:
.ci/verify-k8s-pod-yaml.sh,prow-jobs/pingcap-qe/ci/presubmits.yaml - Issue: No documentation was added explaining the
cranevalidation logic or its integration into the CI pipeline. - Suggested Fix: Add comments in the script and update the CI pipeline documentation to describe
craneusage, installation, and expected behavior.
- File:
Conclusion
The core functionality of the PR (image tag updates) is low-risk and straightforward. However, the new validation logic introduces potential issues with error handling and reliability that should be addressed. The suggested improvements will enhance robustness, maintainability, and documentation clarity.
|
/approve |
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1 participant
Summary
Update all ticdc pod templates to use the new Jenkins image tag that includes the Python 3.12 symlink fix.
Changes
Update image tag from
v2026.6.28-3-g80620cc5-go1.25→v2026.6.28-8-g92327eaa-go1.25in:pipelines/pingcap/ticdc/(52 files)pipelines/pingcap-inc/ticdc/(14 files)Total: 66 files
Testing
python3 --versionreturns Python 3.12.xRisk
Low. Pure tag update. The new image includes both Python 3.12 (PR #4760) and the symlink fix ensuring
python3/pip3resolve to 3.12 (PR #4766).