Remove ROSACLI Report Portal integration - #82252
Conversation
WalkthroughROSA test commands now copy JUnit XML directly to the artifact directory, optionally strip output sections, preserve test exit status, and remove the standalone report step from chains, job references, and its step-registry definition. ChangesROSA JUnit artifact flow
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant ROSA tests
participant JUnit XML
participant Artifact directory
ROSA tests->>JUnit XML: generate test results
ROSA tests->>JUnit XML: optionally strip system-out/system-err
ROSA tests->>Artifact directory: copy JUnit XML
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)ci-operator/config/openshift/rosa/openshift-rosa-master__e2e.yamlTraceback (most recent call last): ci-operator/step-registry/rosa/tests/rosa-tests-ref.yamlTraceback (most recent call last): 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 |
|
@jerichokeyne: GitHub didn't allow me to request PR reviews from the following users: openshift/ocm-qe. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn response to this:
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. |
0c2f95c to
09033ab
Compare
|
/pj-rehearse periodic-ci-openshift-rosa-master-e2e-rosa-ocm-resources-f3 |
|
@jerichokeyne: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
09033ab to
3fa7987
Compare
|
/pj-rehearse periodic-ci-openshift-rosa-master-e2e-rosa-ocm-resources-f3 |
|
@jerichokeyne: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
3fa7987 to
91819cf
Compare
|
/pj-rehearse periodic-ci-openshift-rosa-master-e2e-rosa-ocm-resources-f3 |
|
@jerichokeyne: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Remove the rosa-tests-report step and all references to it from ROSACLI CI jobs. The upload_junit_result function was tarring junit XML into SHARED_DIR (backed by a K8s Secret with a 1 MiB limit), causing jobs to fail with "Secret is invalid: data: Too long" when verbose Ginkgo output inflated the XML. Replace upload_junit_result calls with a direct cp to ARTIFACT_DIR, which has no size limit and is the standard approach used by other test steps (e.g. terraform-provider-rhcs). Add STRIP_JUNIT_OUTPUT option to rosa-tests step to strip system-out/system-err from junit XML before the sidecar censors it. Enable for rosa-ocm-resources-f3 where the cluster profile secrets cause the sidecar to censor the entire junit file. Co-authored-by: Cursor <cursoragent@cursor.com>
91819cf to
09ebb8e
Compare
|
/pj-rehearse periodic-ci-openshift-rosa-master-e2e-rosa-ocm-resources-f3 |
|
/pj-rehearse periodic-ci-openshift-rosa-master-e2e-rosa-hcp-advanced-f3 |
|
@jerichokeyne: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
A total of 27 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: |
|
@jerichokeyne: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
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/rosa/tests/rosa-tests-commands.sh`:
- Around line 70-83: Update the JUnit sanitization and artifact-copy flow
following the test execution to track failures in an artifact_exit status,
explicitly capture python3 sanitization and cp failures without exiting
prematurely, and preserve test_exit when tests fail. Return artifact_exit only
when the tests succeeded but artifact processing failed, ensuring artifact
failures cannot be silently treated as success.
🪄 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: 42c3882b-9a56-40f4-bb9b-bbaf0bffb043
📒 Files selected for processing (15)
ci-operator/config/openshift/rosa/openshift-rosa-master__e2e.yamlci-operator/step-registry/rosa/tests/e2e/fullcycle/rosa-tests-e2e-fullcycle-chain.yamlci-operator/step-registry/rosa/tests/pull-request/rosa-tests-pull-request-chain.yamlci-operator/step-registry/rosa/tests/pull-request/rosa-tests-pull-request-commands.shci-operator/step-registry/rosa/tests/report/OWNERSci-operator/step-registry/rosa/tests/report/rosa-tests-report-commands.shci-operator/step-registry/rosa/tests/report/rosa-tests-report-ref.metadata.jsonci-operator/step-registry/rosa/tests/report/rosa-tests-report-ref.yamlci-operator/step-registry/rosa/tests/rosa-tests-commands.shci-operator/step-registry/rosa/tests/rosa-tests-ref.yamlci-operator/step-registry/rosa/tests/runtime/day1-post/rosa-tests-runtime-day1-post-commands.shci-operator/step-registry/rosa/tests/runtime/day2/rosa-tests-runtime-day2-commands.shci-operator/step-registry/rosa/tests/runtime/destroy-post/rosa-tests-runtime-destroy-post-commands.shci-operator/step-registry/rosa/tests/runtime/destroy/rosa-tests-runtime-destroy-commands.shci-operator/step-registry/rosa/tests/runtime/destructive/rosa-tests-runtime-destructive-commands.sh
💤 Files with no reviewable changes (6)
- ci-operator/step-registry/rosa/tests/pull-request/rosa-tests-pull-request-chain.yaml
- ci-operator/step-registry/rosa/tests/report/rosa-tests-report-ref.metadata.json
- ci-operator/step-registry/rosa/tests/report/OWNERS
- ci-operator/step-registry/rosa/tests/report/rosa-tests-report-commands.sh
- ci-operator/step-registry/rosa/tests/e2e/fullcycle/rosa-tests-e2e-fullcycle-chain.yaml
- ci-operator/step-registry/rosa/tests/report/rosa-tests-report-ref.yaml
🚧 Files skipped from review as they are similar to previous changes (8)
- ci-operator/step-registry/rosa/tests/runtime/day2/rosa-tests-runtime-day2-commands.sh
- ci-operator/step-registry/rosa/tests/runtime/destroy-post/rosa-tests-runtime-destroy-post-commands.sh
- ci-operator/step-registry/rosa/tests/runtime/day1-post/rosa-tests-runtime-day1-post-commands.sh
- ci-operator/step-registry/rosa/tests/runtime/destroy/rosa-tests-runtime-destroy-commands.sh
- ci-operator/step-registry/rosa/tests/runtime/destructive/rosa-tests-runtime-destructive-commands.sh
- ci-operator/step-registry/rosa/tests/rosa-tests-ref.yaml
- ci-operator/step-registry/rosa/tests/pull-request/rosa-tests-pull-request-commands.sh
- ci-operator/config/openshift/rosa/openshift-rosa-master__e2e.yaml
| python3 -c " | ||
| import xml.etree.ElementTree as ET, sys | ||
| tree = ET.parse(sys.argv[1]) | ||
| for elem in tree.iter(): | ||
| for child in list(elem): | ||
| if child.tag in ('system-out', 'system-err'): | ||
| elem.remove(child) | ||
| tree.write(sys.argv[1], xml_declaration=True, encoding='unicode') | ||
| " "${JUNIT_XML}" | ||
| fi | ||
| cp "${JUNIT_XML}" "${ARTIFACT_DIR}/" | ||
|
|
||
| log "Testing is finished and uploaded." | ||
| exit ${test_exit} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle sanitization and copy failures explicitly.
python3 and cp are not folded into the final status policy. With set -e, either failure exits before exit ${test_exit}; without it, the failure can be ignored and the job may report success without the JUnit artifact.
Track an artifact_exit, return the test failure when present, and otherwise return the artifact-processing failure.
Proposed status handling
+artifact_exit=0
+
- python3 -c "..." "${JUNIT_XML}"
+ python3 -c "..." "${JUNIT_XML}" || artifact_exit=$?
...
-cp "${JUNIT_XML}" "${ARTIFACT_DIR}/"
+cp "${JUNIT_XML}" "${ARTIFACT_DIR}/" || artifact_exit=$?
...
-exit ${test_exit}
+if (( test_exit != 0 )); then
+ exit "${test_exit}"
+fi
+exit "${artifact_exit}"Based on the PR objective to preserve the test exit status while reliably producing JUnit artifacts.
📝 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.
| python3 -c " | |
| import xml.etree.ElementTree as ET, sys | |
| tree = ET.parse(sys.argv[1]) | |
| for elem in tree.iter(): | |
| for child in list(elem): | |
| if child.tag in ('system-out', 'system-err'): | |
| elem.remove(child) | |
| tree.write(sys.argv[1], xml_declaration=True, encoding='unicode') | |
| " "${JUNIT_XML}" | |
| fi | |
| cp "${JUNIT_XML}" "${ARTIFACT_DIR}/" | |
| log "Testing is finished and uploaded." | |
| exit ${test_exit} | |
| artifact_exit=0 | |
| python3 -c " | |
| import xml.etree.ElementTree as ET, sys | |
| tree = ET.parse(sys.argv[1]) | |
| for elem in tree.iter(): | |
| for child in list(elem): | |
| if child.tag in ('system-out', 'system-err'): | |
| elem.remove(child) | |
| tree.write(sys.argv[1], xml_declaration=True, encoding='unicode') | |
| " "${JUNIT_XML}" || artifact_exit=$? | |
| fi | |
| cp "${JUNIT_XML}" "${ARTIFACT_DIR}/" || artifact_exit=$? | |
| log "Testing is finished and uploaded." | |
| if (( test_exit != 0 )); then | |
| exit "${test_exit}" | |
| fi | |
| exit "${artifact_exit}" |
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 83-83: 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/rosa/tests/rosa-tests-commands.sh` around lines 70
- 83, Update the JUnit sanitization and artifact-copy flow following the test
execution to track failures in an artifact_exit status, explicitly capture
python3 sanitization and cp failures without exiting prematurely, and preserve
test_exit when tests fail. Return artifact_exit only when the tests succeeded
but artifact processing failed, ensuring artifact failures cannot be silently
treated as success.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: amandahla, jerichokeyne The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/pj-rehearse ack |
|
@jerichokeyne: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@jerichokeyne: 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. |
8fbeb48
into
openshift:main
Remove the rosa-tests-report step and all references to it from ROSACLI CI jobs. The upload_junit_result function was tarring junit XML into SHARED_DIR (backed by a K8s Secret with a 1 MiB limit), causing jobs to fail with "Secret is invalid: data: Too long" when verbose Ginkgo output inflated the XML. Replace upload_junit_result calls with a direct cp to ARTIFACT_DIR, which has no size limit and is the standard approach used by other test steps (e.g. terraform-provider-rhcs). Add STRIP_JUNIT_OUTPUT option to rosa-tests step to strip system-out/system-err from junit XML before the sidecar censors it. Enable for rosa-ocm-resources-f3 where the cluster profile secrets cause the sidecar to censor the entire junit file. Co-authored-by: Cursor <cursoragent@cursor.com>
Remove the rosa-tests-report step and all references to it from ROSACLI CI jobs. The upload_junit_result function was tarring junit XML into SHARED_DIR (backed by a K8s Secret with a 1 MiB limit), causing jobs to fail with "Secret is invalid: data: Too long" when verbose Ginkgo output inflated the XML. Replace upload_junit_result calls with a direct cp to ARTIFACT_DIR, which has no size limit and is the standard approach used by other test steps (e.g. terraform-provider-rhcs). Add STRIP_JUNIT_OUTPUT option to rosa-tests step to strip system-out/system-err from junit XML before the sidecar censors it. Enable for rosa-ocm-resources-f3 where the cluster profile secrets cause the sidecar to censor the entire junit file. Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
rosa-tests-reportstep and all references to it from ROSACLI CI jobsupload_junit_resultcalls (which tar junit intoSHARED_DIR) with a directcptoARTIFACT_DIRSecret "..." is invalid: data: Too long: may not be more than 1048576 byteswhen verbose Ginkgo output inflates junit XML beyond the K8s Secret 1 MiB limitThis file contained potentially sensitive information and has been removed.. I enabled it for theocm-resourcesjob since that job has been consistently getting censored and the results were not being shown because of that. The other jobs haven't been getting censored, so I've left it disabled to try to keep more info to debug other jobsChanges
Deleted:
ci-operator/step-registry/rosa/tests/report/(4 files — step ref, commands script, metadata, OWNERS)Removed
rosa-tests-reportreferences from:rosa-tests-pull-request-chain.yamlrosa-tests-e2e-fullcycle-chain.yamlopenshift-rosa-master__e2e.yaml(10 inline test definitions)Replaced
upload_junit_resultwithcpin 7 step scripts:rosa-tests-commands.shrosa-tests-pull-request-commands.shrosa-tests-runtime-day1-post-commands.shrosa-tests-runtime-day2-commands.shrosa-tests-runtime-destructive-commands.shrosa-tests-runtime-destroy-commands.shrosa-tests-runtime-destroy-post-commands.shTest plan
make updatepasses cleanly (done locally)Made with Cursor
Summary by CodeRabbit
rosa-tests-reportstep and removing it from the ROSA e2e fullcycle and ROSA pull-request chain definitions (openshift-rosa-master__e2e.yamlplus the corresponding chain YAMLs).upload_junit_resultwith directcpof${JUNIT_XML}into${ARTIFACT_DIR}in the mainrosa-testsstep and runtime step scripts (day1-post, day2, destroy, destroy-post, destructive), plus the PR variant (rosa-tests-pull-request-commands.sh).STRIP_JUNIT_OUTPUT(default"false"). When set to"true",${JUNIT_XML}is rewritten to remove<system-out>/<system-err>before copying to${ARTIFACT_DIR}; therosa-ocm-resources-f3job enables this flag.rosa-tests-reportcommand script and its step/ref/metadata definitions, and clears the associatedOWNERSlinkage.Work to verify:
make updatepassed locally;pj-rehearsevalidation and post-merge GCS artifact confirmation remain pending.pj-rehearsewas requested three times forperiodic-ci-openshift-rosa-master-e2e-rosa-ocm-resources-f3.