Add hack/trigger-qe-tests.sh to trigger WMCO QE z-stream tests - #4324
Add hack/trigger-qe-tests.sh to trigger WMCO QE z-stream tests#4324rrasouli wants to merge 1 commit into
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (19 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rrasouli 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
hack/trigger-qe-tests.sh (2)
117-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated rename logic; consider a helper (also addresses shellcheck SC2001).
The
winc-→winc-zstream-substitution viased 's/winc-/winc-zstream-/'is duplicated at Line 120 (display) and Line 154 (actual rename). Static analysis also flags both as SC2001, suggesting bash parameter expansion instead ofsedfor a simple substitution. Extracting a small helper avoids the two call sites drifting apart and satisfies the lint hint:♻️ Proposed refactor
+to_zstream_name() { + echo "${1/winc-/winc-zstream-}" +} + echo "Will rename:" echo -e "$JOBS_TO_RENAME" | while read -r job; do [[ -z "$job" ]] && continue - echo " $job -> $(echo "$job" | sed 's/winc-/winc-zstream-/')" + echo " $job -> $(to_zstream_name "$job")" doneecho -e "$JOBS_TO_RENAME" | while read -r job; do [[ -z "$job" ]] && continue - new_name=$(echo "$job" | sed 's/winc-/winc-zstream-/') + new_name=$(to_zstream_name "$job") sed -i.bak "s/^- as: ${job}$/- as: ${new_name}/" "$WORKTREE_CONFIG" doneAlso applies to: 150-157
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hack/trigger-qe-tests.sh` around lines 117 - 121, Extract the winc- to winc-zstream- transformation into a small helper in hack/trigger-qe-tests.sh, using Bash parameter expansion instead of sed to address SC2001. Update both the rename preview loop and the actual rename logic near the existing JOBS_TO_RENAME handling to call this helper, preserving the current substitution behavior.Source: Linters/SAST tools
98-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFragile newline-in-string pattern for job lists.
JOBS_TO_RENAME/SKIPPEDare built by concatenating literal\ninside double quotes and later re-expanded withecho -e(Lines 118, 152, 239). This works but is a brittle idiom (breaks silently if any call site forgets-e, and doesn't handle job names containing\well). A bash array (JOBS_TO_RENAME=(); JOBS_TO_RENAME+=("$job")) would be more robust and idiomatic for accumulating a list of job names.Also applies to: 152-156, 239-244
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hack/trigger-qe-tests.sh` around lines 98 - 115, Replace the literal-newline string accumulation for JOBS_TO_RENAME and SKIPPED with Bash arrays, appending each job as a distinct quoted element in the loop. Update the related output and emptiness checks at the shown call sites to use array-safe length and iteration, removing reliance on echo -e while preserving the existing skip and rename messages.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hack/trigger-qe-tests.sh`:
- Around line 46-51: Extend cleanup_temp_repo to handle failure cleanup for the
branch and worktree created later in the script: when WORKTREE_DIR exists,
remove it with the appropriate git worktree cleanup, then remove the local
BRANCH only after confirming it exists. Install an ERR/exit failure trap that
invokes this cleanup after those resources are created, while preserving normal
cleanup of TEMP_RELEASE_REPO and avoiding removal of resources that were never
created.
- Around line 34-44: Ensure the temporary clone flow creates an upstream remote
before calling validate_release_repo, or allow that flow to validate using its
clone remote. In validate_release_repo, tighten the remote URL check to match
the openshift/release repository exactly, preventing similarly named
repositories such as openshift/release-tools from passing.
---
Nitpick comments:
In `@hack/trigger-qe-tests.sh`:
- Around line 117-121: Extract the winc- to winc-zstream- transformation into a
small helper in hack/trigger-qe-tests.sh, using Bash parameter expansion instead
of sed to address SC2001. Update both the rename preview loop and the actual
rename logic near the existing JOBS_TO_RENAME handling to call this helper,
preserving the current substitution behavior.
- Around line 98-115: Replace the literal-newline string accumulation for
JOBS_TO_RENAME and SKIPPED with Bash arrays, appending each job as a distinct
quoted element in the loop. Update the related output and emptiness checks at
the shown call sites to use array-safe length and iteration, removing reliance
on echo -e while preserving the existing skip and rename messages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 7576baf5-9bb5-4896-ac35-35464ba679e9
📒 Files selected for processing (1)
hack/trigger-qe-tests.sh
4956e8c to
baeaa4d
Compare
baeaa4d to
884f4fe
Compare
884f4fe to
aa57bf3
Compare
|
@rrasouli: The following tests 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. |
Manual process to trigger WMCO QE z-stream/y-stream tests
Periodic CI jobs cannot be triggered on demand. The workaround is to rename the job
as:field in the openshift/release config, which makes Prow see them as "new" jobs eligible for/pj-rehearse.openshift/releaseci-operator/config/openshift/openshift-tests-private/openshift-openshift-tests-private-release-<OCP_VER>__amd64-nightly.yamlgrep 'as:.*winc.*-f[0-9]')as:field by insertingzstream(e.g.aws-ipi-ovn-winc-f14->aws-ipi-ovn-winc-zstream-f14)make updateto regenerate Prow job configsDEBUG Do not merge: <WMCO_VER> Z stream triggered jobs/pj-rehearse(or/pj-rehearse maxif more than 5 jobs) as a commentWhat this script automates
hack/trigger-qe-tests.shperforms all of the above steps:-s y(y-stream, all jobs) and-s z(z-stream, subset) with-j/-xflags for overridesmake update/pj-rehearseto trigger the testsReference: PR openshift/release#81646 (original manual 4.21 z-stream trigger)