MPIIT: Migrate Quay to new lpGA variant and Use ExitTrap for mapping - #82955
MPIIT: Migrate Quay to new lpGA variant and Use ExitTrap for mapping#82955oharan2 wants to merge 3 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: oharan2 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 |
|
/pj-rehearse periodic-ci-quay-quay-tests-master-ocp-4.22-quay-lpGA-lp-ocp-compat-cr--quay--e2e-tests-aws |
|
@oharan2: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse abort |
|
@oharan2: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
WalkthroughQuay test jobs now define ReportPortal component names and optional test-suite mapping. Test scripts conditionally load a shared exit-trap helper to generate remapped JUnit artifacts. The Quay E2E script also changes artifact collection and installation order. ChangesQuay test reporting
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant QuayTestScript
participant ExitTrapPostProcessPrep
participant JUnitArtifacts
QuayTestScript->>ExitTrapPostProcessPrep: Load helper when MAP_TESTS=true
QuayTestScript->>JUnitArtifacts: Copy available test artifacts
QuayTestScript->>ExitTrapPostProcessPrep: Run EXIT trap with component name
ExitTrapPostProcessPrep->>JUnitArtifacts: Generate remapped JUnit artifact
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/pj-rehearse periodic-ci-quay-quay-tests-master-ocp-4.22-quay-lpGA-lp-ocp-compat-cr--quay--e2e-tests-aws |
|
@oharan2: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
A total of 59 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@ci-operator/step-registry/quay-tests/test-quay-e2e/quay-tests-test-quay-e2e-commands.sh`:
- Around line 7-19: Update copyArtifacts so the artifact-renaming loop skips
unmatched glob paths before calling basename or mv, while preserving the
existing behavior for actual artifacts and runs with no Cypress results.
🪄 Autofix
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: 78bba44b-f8d5-4b75-acd3-298bdd1c37a8
⛔ Files ignored due to path filters (1)
ci-operator/jobs/quay/quay-tests/quay-quay-tests-master-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (9)
ci-operator/config/quay/quay-tests/quay-quay-tests-master__ocp-4.22-quay-lpGA-lp-ocp-compat.yamlci-operator/step-registry/quay-tests/cso-qe-test/quay-tests-cso-qe-test-commands.shci-operator/step-registry/quay-tests/cso-qe-test/quay-tests-cso-qe-test-ref.yamlci-operator/step-registry/quay-tests/deploy-quay-aws-s3/quay-tests-deploy-quay-aws-s3-commands.shci-operator/step-registry/quay-tests/deploy-quay-aws-s3/quay-tests-deploy-quay-aws-s3-ref.yamlci-operator/step-registry/quay-tests/qbo-qe-test/quay-tests-qbo-qe-test-commands.shci-operator/step-registry/quay-tests/qbo-qe-test/quay-tests-qbo-qe-test-ref.yamlci-operator/step-registry/quay-tests/test-quay-e2e/quay-tests-test-quay-e2e-commands.shci-operator/step-registry/quay-tests/test-quay-e2e/quay-tests-test-quay-e2e-ref.yaml
| mkdir -p $ARTIFACT_DIR | ||
|
|
||
| function copyArtifacts { | ||
| typeset junitPrefix="junit_" | ||
| cp -r ./cypress/results/* "$ARTIFACT_DIR" || true | ||
|
|
||
| for file in "$ARTIFACT_DIR"/*; do | ||
| if [[ ! "$(basename "$file")" =~ ^"$junitPrefix" ]]; then | ||
| result_file="$ARTIFACT_DIR"/"$junitPrefix""$(basename "$file")" | ||
| mv "$file" "$result_file" | ||
| fi | ||
| done | ||
| cp -r ./cypress/videos/* "$ARTIFACT_DIR" || true |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Skip unmatched artifact paths.
cp tolerates missing Cypress results, but an unmatched "$ARTIFACT_DIR"/* remains a literal path in Bash. The loop then calls mv on that nonexistent path. A run without results can fail from the EXIT trap.
Proposed fix
- mkdir -p $ARTIFACT_DIR
+ mkdir -p "$ARTIFACT_DIR"
for file in "$ARTIFACT_DIR"/*; do
+ [[ -e "$file" ]] || continue
if [[ ! "$(basename "$file")" =~ ^"$junitPrefix" ]]; then📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mkdir -p $ARTIFACT_DIR | |
| function copyArtifacts { | |
| typeset junitPrefix="junit_" | |
| cp -r ./cypress/results/* "$ARTIFACT_DIR" || true | |
| for file in "$ARTIFACT_DIR"/*; do | |
| if [[ ! "$(basename "$file")" =~ ^"$junitPrefix" ]]; then | |
| result_file="$ARTIFACT_DIR"/"$junitPrefix""$(basename "$file")" | |
| mv "$file" "$result_file" | |
| fi | |
| done | |
| cp -r ./cypress/videos/* "$ARTIFACT_DIR" || true | |
| mkdir -p "$ARTIFACT_DIR" | |
| function copyArtifacts { | |
| typeset junitPrefix="junit_" | |
| cp -r ./cypress/results/* "$ARTIFACT_DIR" || true | |
| for file in "$ARTIFACT_DIR"/*; do | |
| [[ -e "$file" ]] || continue | |
| if [[ ! "$(basename "$file")" =~ ^"$junitPrefix" ]]; then | |
| result_file="$ARTIFACT_DIR"/"$junitPrefix""$(basename "$file")" | |
| mv "$file" "$result_file" | |
| fi | |
| done | |
| cp -r ./cypress/videos/* "$ARTIFACT_DIR" || true |
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 7-7: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 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
`@ci-operator/step-registry/quay-tests/test-quay-e2e/quay-tests-test-quay-e2e-commands.sh`
around lines 7 - 19, Update copyArtifacts so the artifact-renaming loop skips
unmatched glob paths before calling basename or mv, while preserving the
existing behavior for actual artifacts and runs with no Cypress results.
Source: Linters/SAST tools
|
Hey @oharan2, reviewed the PR — looks great overall! Clean migration from the legacy yq/mapTestsForComponentReadiness approach to ExitTrap. A few things I noticed: 1. Missing Both refs now use Only 2. No download timeouts on wget/curl The type -t wget 1>/dev/null && _fURL=(wget --timeout=30 -qO-) || _fURL=(curl --connect-timeout 10 --max-time 30 -fsSL)3. No function existence guard after If the download fails, 4. Indentation — URL not indented in 2 files In "${_fURL[@]}" \
https://raw.githubusercontent.com/... # <-- should be indentedSee commit |
|
@oharan2: all tests passed! 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. |
|
@amiskin94 In the files I didn't add grace period - it's already there |
|
/pj-rehearse ack |
|
@oharan2: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-merge-bot
Bot
added
the
rehearsals-ack
2 participants
Summary
Migrates Quay OCP 4.22 LP interop CI to the
lpGA-lp-ocp-compatvariant and wires MPIIT / Component Readiness junit remapping through the sharedExitTrap--PostProcessPrephelper.This replaces the legacy
ocp-4.22-quay-lp-interopvariant and per-stepREPORTPORTAL_CMP/mapTestsForComponentReadinesshandling.Changes
CI operator config (
quay-quay-tests-master)__ocp-4.22-quay-lp-interop.yaml→__ocp-4.22-quay-lpGA-lp-ocp-compat.yamlzz_generated_metadata.varianttoocp-4.22-quay-lpGA-lp-ocp-compatcr-quay-e2e-tests-aws→cr--quay--e2e-tests-awsaws-fips→quay--aws-fipsDR__RP__CR_COMP_NAME: lp-ocp-compat--Quayon CR, FIPS, and yearly AWS S3 jobsREPORTPORTAL_CMP: Quay-lp-interop(superseded byDR__RP__CR_COMP_NAME+ ExitTrap)Step registry (CR job
test:steps)For
quay-tests-deploy-quay-aws-s3,quay-tests-test-quay-e2e,quay-tests-qbo-qe-test, andquay-tests-cso-qe-test:DR__RP__CR_COMP_NAME(defaultlp-ocp-compat--Quay) andMAP_TESTS(defaultfalse) to each-ref.yamlMAP_TESTS=true, source ExitTrap--PostProcessPrep and register anEXITtrap to remap junit suite names tolp-ocp-compat--Quay--<test>mapTestsForComponentReadinessin e2e; hard-coded ReportPortal component names elsewhere)quay-tests-test-quay-e2ekeeps artifact copy onEXITand chains ExitTrap +CopyArtifactswhen mapping is enabled.Summary by CodeRabbit
lpGA-lp-ocp-compatvariant.DR__RP__CR_COMP_NAME.REPORTPORTAL_CMPsetting.MAP_TESTSsupport to Quay QE and deployment steps.ExitTrap--PostProcessPrep.shhelper.