feat(tekton): enable kernel image sync step with configurable stages

[wuhuizuo]

What

• Add a stages param to the request-kernel-image-sync step action so target stages (dev/prod) are configurable instead of hardcoded.
• Enable the previously-disabled request-kernel-image-sync step in the pingcap-notify-to-deliver-images-to-cloud-tidbx delivery task and wire it to both dev and prod stages.
• Remove the now-unused notify-config workspace from the delivery task.

Related

Closes follow-up of #4936/#4938.

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

• tekton/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 enhances the Tekton CI pipeline by making the kernel image sync step configurable with respect to target stages (dev/prod), replacing hardcoded values with a flexible stages parameter. It also re-enables the kernel image sync step in the delivery task and cleans up an unused workspace. The implementation is straightforward and aligns well with the stated goals, improving maintainability and configurability. Overall, the changes are well-structured but there are a few critical and improvement points to consider.

---

Critical Issues

• Parameter name mismatch in request-kernel-image-sync.yaml

  • File/Line: tekton/v1/step-actions/request-kernel-image-sync.yaml, lines ~24-29 and 41
  • Issue: The env variable TARGET_STAGES is set from $(params.stage), but the parameter is declared as stages (plural). This discrepancy will cause the env var to be empty, breaking the logic that loops over $TARGET_STAGES.
  • Fix: Change the env var assignment to use $(params.stages):

    - name: TARGET_STAGES
      value: $(params.stages)

• Shell word-splitting for stages may cause issues

  • File/Line: request-kernel-image-sync.yaml, line ~43
  • Issue: The for stage in $TARGET_STAGES; do relies on shell word splitting to iterate over stages. If stages contains multiple stages separated by spaces (like "dev prod"), this works, but if the input has commas or extra spaces, it could break. Also, no trimming or validation is done.
  • Fix: Ideally, document the expected format strictly as space-separated, or use a more robust parsing approach (e.g., IFS manipulation). For example:

    IFS=' ' read -r -a stage_array <<< "$TARGET_STAGES"
    for stage in "${stage_array[@]}"; do
      ...
    done

    Or at least note in the param description the accepted format clearly.

---

Code Improvements

• Delivery task cleanup: remove large commented-out block instead of leaving it in the code

  • File/Line: pingcap-notify-to-deliver-images-to-cloud-tidbx.yaml, lines ~37-100
  • Issue: The previously disabled notify-to-sync-images step is removed, but the entire script block is left commented out, making the file unnecessarily long and harder to read.
  • Fix: Since the step is removed, delete this entire commented-out script block to improve maintainability and readability.

• Explicitly document the accepted format for the stages parameter

  • File/Line: request-kernel-image-sync.yaml, param stages description
  • Issue: The description says "dev prod" or "dev" etc., but does not specify how strings are split or if other separators are accepted. Ambiguity can lead to misuse.
  • Fix: Update the description:

    description: target stage(s), space-separated (e.g., "dev", "prod", or "dev prod")

• Add error handling in the script for invalid or empty stages

  • File/Line: request-kernel-image-sync.yaml, script section
  • Issue: If TARGET_STAGES is empty or contains invalid stages, the script silently skips or fails without clear error messages.
  • Fix: Add a check early in the script:

    if [ -z "$TARGET_STAGES" ]; then
      echo "Error: TARGET_STAGES is empty. Please provide at least one stage."
      exit 1
    fi

---

Best Practices

• Testing coverage

  • No evidence of added or updated tests covering the new stages parameter or the re-enabled step. Consider adding unit or integration tests validating the multi-stage sync behavior in Tekton pipelines.

• Documentation for the delivery task changes

  • The PR description mentions removing the notify-config workspace, but the updated task YAML lacks any comments describing the removal or the rationale. Adding a short comment block about the removal helps future maintainers.

• Consistent naming

  • The param is named stages but the env var is TARGET_STAGES. While not a bug, aligning naming conventions might improve clarity (e.g., use STAGES or TARGET_STAGES consistently in both declarations and usage).

---

Summary of actionable fixes:

# In tekton/v1/step-actions/request-kernel-image-sync.yaml
env:
  - name: TARGET_STAGES
    value: $(params.stages)       # fix param name mismatch

parameters:
  - name: stages
    type: string
    description: target stage(s), space-separated (e.g., "dev", "prod", or "dev prod")
    default: "dev"

# In the script section, add:
if [ -z "$TARGET_STAGES" ]; then
  echo "Error: TARGET_STAGES is empty. Please provide at least one stage."
  exit 1
fi

# Optional: safer iteration over stages
IFS=' ' read -r -a stage_array <<< "$TARGET_STAGES"
for stage in "${stage_array[@]}"; do
  ...
done

# In tekton/v1/tasks/delivery/pingcap-notify-to-deliver-images-to-cloud-tidbx.yaml
# Remove the large commented-out notify-to-sync-images script block entirely

Addressing these will ensure the PR achieves its goal without runtime errors and improves maintainability.