ci(prow): migrate pingcap/tidb release-7.1 jobs to target jenkins - #5135
ci(prow): migrate pingcap/tidb release-7.1 jobs to target jenkins#5135wuhuizuo wants to merge 2 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![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 the presubmit jobs configuration for the pingcap/tidb repository, specifically for the release-7.1 branch, by changing the labels.master value from "1" to "0" for all Jenkins agent jobs. This effectively flips the scheduling target to a new Jenkins instance. The change is straightforward, consistent, and limited to a single YAML file, reflecting a migration step in CI infrastructure. The patch is clean and focused, with no apparent syntax errors.
Code Improvements
-
Avoid repeated manual edits for similar jobs (
prow-jobs/pingcap/tidb/release-7.1-presubmits.yamllines ~10–220)
The PR changes the same field (labels.master) from"1"to"0"in many jobs. This repetitive manual change is error-prone and harder to maintain. Instead, consider:- Using YAML anchors and aliases to define the common labels block once and reuse it.
- Or, if supported by your tooling, introducing a variable or parameter for
labels.masterto make future flips easier. - Or scripting the change to avoid manual copy-paste errors.
Example snippet using YAML anchor:
default_labels: &default_labels master: "0" presubmits: - name: pingcap/tidb/release-7.1/ghpr_build agent: jenkins labels: *default_labels ...
-
Add explanatory comments about the label flip
The single-line comment# need add this.ondecorate: falseis unclear and should be clarified or removed. Also, add a top-level comment in this YAML file or near thelabels.masterkeys explaining what"0"vs"1"means (e.g., which Jenkins instance "0" refers to) for maintainability.
Best Practices
-
Testing coverage
Since this PR changes job scheduling, validate that the jobs are correctly triggered on the new Jenkins setup. Consider adding or updating integration tests or manual verification steps documented in the PR or repo docs. -
Documentation update
This migration affects CI infrastructure. Ensure that any relevant documentation (internal or external) referencing the Jenkins agent labels or job scheduling reflects this change. If this PR is part of a larger migration (#4959), link to or update those docs accordingly. -
Style and formatting consistency
The comment# need add this.appears multiple times. Standardize comment style and grammar, e.g.,# Required to enable decoration.or remove if not needed.
No critical issues were found as the change is a simple label value update affecting scheduling. The suggestions above help improve maintainability and clarity for future changes.
Jenkins Migration VerificationMigrating jenkins-agent Prow jobs to the to Jenkins (
Summary: success=0 submitted=0 failed=1 infra-fail=2 skipped=8 dry-run=0 total=11 |
Flip labels.master "1" -> "0" for the jenkins-agent jobs in prow-jobs/pingcap/tidb/release-7.1-presubmits.yaml so they are scheduled on the to Jenkins. Part of #4959
f74ae77 to
55b690f
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 PR updates the scheduling labels for all Jenkins agent jobs in the release-7.1-presubmits.yaml file for pingcap/tidb from master: "1" to master: "0". This change reroutes the jobs to a new Jenkins target as part of a CI migration effort. The approach is a straightforward label value flip repeated consistently across many job definitions. The change is simple and low risk given the uniform pattern, but it involves a large number of repetitive edits.
Code Improvements
-
Avoid repetitive manual edits:
The PR changes the same label key-value pair (master: "1"→master: "0") in many places. This is error-prone and hard to maintain. Consider introducing a variable or a template mechanism if your CI configuration system supports it, or use a script to bulk update these labels. For example, if you regenerate this YAML from a template or a config management tool, you can update the label once and regenerate all jobs. This reduces the chance of missing jobs or inconsistent labels. -
Verify label semantics and correctness:
Double-check that the labelmasterwith values"0"or"1"correctly controls the scheduling target as intended. It might be clearer to rename the label to something more descriptive, e.g.,jenkins-target: "old"or"new", if feasible. This reduces ambiguity about whatmaster: "0"means.
Best Practices
-
Add a comment explaining the label change:
In the YAML file near the labels section, add a comment like:labels: # Changed from "1" to "0" to migrate jobs to the new Jenkins target as per #4959 master: "0"
This helps future maintainers understand why the label changed without needing to reference the PR or issue.
-
Test coverage and validation:
Since this is a CI configuration change, ensure that these jobs are tested or validated on the target Jenkins environment before merging. If there is an automated validation step for prow job YAMLs, run it to catch syntax or schema errors. -
Consider documenting migration in CI docs:
Since this is part of a broader migration (#4959), consider updating any relevant internal documentation or README files about job scheduling or Jenkins targets to reflect this change.
No critical issues detected given the nature of this change. It is a low-risk, mechanical update. The main recommendation is to improve maintainability and clarity around the label usage.
## Summary Migrate the verified `pingcap/tidb/release-7.1/ghpr_build` job to the target Jenkins by setting `labels.master` to `"0"`. This is split from #5135 after the migration verification run reported this job successful. ## Validation - `.ci/update-prow-job-kustomization.sh` - YAML parse with `yq` - `git diff --check`
Migrate only Jenkins jobs that passed the release migration verification round. Included jobs: - pingcap/tidb/release-6.5/ghpr_build (verified by #5134) - pingcap/tidb/release-8.5/pull_build (verified by #5138) - pingcap/tidb/release-8.5/pull_integration_nodejs_test (verified by #5138) - pingcap/tidb/release-8.5/pull_integration_python_orm_test (verified by #5138) The successful pingcap/tidb/release-7.1/ghpr_build from #5135 is already present on main via #5174, so it is not duplicated here. Verification: `.ci/verify-prow-jobs-run.sh` passed with dynamic creation disabled.
|
/retest |
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 prow job configurations for the pingcap/tidb release-7.1 branch by changing the labels.master value from "1" to "0" for jenkins-agent jobs. This change appears to reroute job scheduling from a current Jenkins infrastructure to a new target Jenkins, as part of the ongoing migration effort (#4959, #4947, #4942). The changes are straightforward and consistent across multiple job definitions, and the patch is clean without syntax errors.
Code Improvements
-
DRY / Bulk Update Opportunity
The same label change (master: "1"→master: "0") is repeated verbatim for many jobs. Since this is a large-scale migration across many jobs in the same file, consider if the prow job configuration supports templating or includes to avoid repetitive manual edits. This would reduce human error and improve maintainability for future migrations.File:
prow-jobs/pingcap/tidb/release-7.1-presubmits.yaml
Suggestion: Investigate using YAML anchors or external templating tools (e.g.,kustomize,ytt, or Helm) to factor out common properties like labels, so changes can be made in one place. -
Validation of Label Semantics
Ensure that changingmasterlabel from"1"to"0"aligns with the expectations of the Jenkins scheduling mechanism. If these labels are boolean flags, consider usingtrue/falseor descriptive labels rather than numeric strings to improve readability and reduce confusion.Suggestion: Confirm with team or tooling docs whether
"0"is the correct and intended value, or if a booleanfalseor a different label would be more idiomatic.
Best Practices
-
Add Comments Clarifying the Label Change Purpose
The PR description mentions the migration but the YAML file itself lacks comments explaining why themasterlabel is flipped from"1"to"0". Adding inline comments would help future maintainers understand the purpose without needing to reference external PRs/issues.Example:
labels: # Changed from "1" to "0" to route jobs to the new Jenkins target as part of migration #4959 master: "0"
-
Testing & Verification
This kind of CI configuration change can break job scheduling if labels are incorrect. The PR should ideally include notes or evidence of testing — for example, confirmation that jobs are now correctly routed to the target Jenkins, and older Jenkins no longer picks them up.Suggestion: Include details in the PR description about how this migration was tested or validated, or add a checklist for manual verification.
No critical issues were found since this is a config-only change with straightforward label edits. The main recommendations are around maintainability, clarity, and validation to ensure a smooth Jenkins migration.
|
/retest |
|
@wuhuizuo: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
2 participants
Part of #4959.
Summary
Flip
labels.master"1"->"0"for the jenkins-agent jobs inprow-jobs/pingcap/tidb/release-7.1-presubmits.yamlso they are scheduled on the to Jenkins.Part of #4947 / #4942