ci(ticdc): add legacy safepoint integration tests

[wk989898]

No description provided.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign dillon-zheng for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• jobs/OWNERS
• pipelines/pingcap/ticdc/OWNERS
• prow-jobs/OWNERS

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]

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

Summary

This PR introduces integration tests for legacy safepoint functionality across different data pipelines (Kafka, MySQL, Pulsar, Storage) in the ticdc repository. The approach involves defining Jenkins pipeline configurations and Kubernetes pod templates for each integration test type, with corresponding presubmit job configurations added to the Prow job definitions. The code is well-structured and adheres to Jenkins DSL conventions, but there are areas for improvement in error handling, resource allocation, and testing coverage.

---

Critical Issues

None identified.

---

Code Improvements

• Unnecessary duplication in pipeline definitions:

  • The pipeline.groovy files for each integration type (Kafka, MySQL, Pulsar, Storage) share significant portions of logic, such as binary preparation, workspace stashing, and testing steps. This duplication makes maintenance harder.
  • File(s): All pipeline.groovy files.
  • Suggestion: Refactor common logic into shared helper methods or scripts. For example:

    void prepareWorkspace(def refs) {
        prow.checkoutRefsWithCacheLock(refs, timeout = 5, credentialsId = 'github-sre-bot-ssh')
        cdc.prepareIntegrationTestCommonBinariesWithCacheLock(refs, 'ng-binary')
    }

    Call this method in each pipeline to avoid redundancy.

• Hard-coded retry values:

  • The retry count (e.g., retry(2)) is hardcoded in multiple places for artifact downloads and workspace preparation.
  • File(s): Multiple pipeline files.
  • Why it's an issue: Hardcoding these values makes them harder to adjust globally if needed.
  • Suggestion: Define retry counts as constants at the top of each file or in a shared library.

• Resource allocation inconsistencies:

  • Kubernetes pod templates specify varying CPU/memory limits across different jobs (golang container in Pulsar pipeline uses 24Gi, while others use 16Gi).
  • File(s): pod.yaml files for Kafka, MySQL, Pulsar, and Storage pipelines.
  • Why it's an issue: It may lead to resource contention or inefficient resource usage.
  • Suggestion: Standardize resource allocation across all pipelines unless explicitly required for specific integrations.

---

Best Practices

• Missing comments for environment variables:

  • Environment variables like NEXT_GEN, LEGACY_SAFEPOINT, and OCI_TAG_* are defined but lack documentation explaining their purpose.
  • File(s): All pipeline.groovy files.
  • Why it's an issue: Future developers may struggle to understand these variables' roles without context.
  • Suggestion: Add comments explaining the purpose of each variable:

    environment {
        NEXT_GEN = 1 // Enables next-gen features in integration tests.
        LEGACY_SAFEPOINT = 1 // Enables legacy safepoint functionality for testing.
    }

• Testing coverage gaps:

  • Integration tests rely on matrix configurations (TEST_GROUP values), but there is no verification of whether all critical test cases are covered.
  • File(s): All pipeline.groovy files.
  • Why it's an issue: Missing test cases could lead to untested edge cases.
  • Suggestion: Add validation scripts or checks to ensure comprehensive test coverage. For example:

    assert TEST_GROUP.values.containsAll(['G00', 'G01', 'G02', 'G03', 'G04']) : "Some test groups are missing!"

• Trigger patterns in Prow jobs:

  • The trigger regex patterns for presubmit jobs are verbose and repetitive.
  • File(s): latest-presubmits-next-gen.yaml.
  • Why it's an issue: Regex patterns are prone to errors and hard to maintain.
  • Suggestion: Use a standardized naming convention to simplify regex:

    trigger: "(?m)^/test (?:.*? )?(pull-cdc-${JOB_NAME})(?: .*?)?$"

---

Suggested Follow-Up Actions

1. Refactor duplicated code across pipeline files into shared libraries or functions.
2. Standardize resource allocations in Kubernetes pod templates.
3. Add detailed comments for environment variables and improve documentation for test groups.
4. Validate test group coverage programmatically.
5. Simplify Prow job regex patterns using a consistent naming convention.

[ti-chi-bot]

@wk989898: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-verify-pr-content-policy	4dbd9ae	link	true	/test pull-verify-pr-content-policy

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.