fix(stackrox): strip system-out from JUnit XMLs before SHARED_DIR copy#80796
fix(stackrox): strip system-out from JUnit XMLs before SHARED_DIR copy#80796davdhacs wants to merge 3 commits into
Conversation
|
Skipping CI for Draft Pull Request. |
WalkthroughInside ChangesJUnit XML Cleanup
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@davdhacs, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
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/stackrox/qa-e2e/stackrox-qa-e2e-commands.sh`:
- Around line 59-60: The issue is that the `-path "${original_results}" -prune`
argument in the find command at line 59 fails to exclude the backup directory
due to a trailing slash mismatch, causing the sed command on line 60 to
incorrectly modify and overwrite the original backup XML files. To fix this,
ensure the path pattern used in the `-path` argument properly matches the actual
directory structure by removing any trailing slash from the `original_results`
variable before using it in the find command, so that the prune correctly
excludes the backup directory and preserves the original results.
🪄 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: ebb64165-212a-4695-9c4d-69526c36ff9c
📒 Files selected for processing (1)
ci-operator/step-registry/stackrox/qa-e2e/stackrox-qa-e2e-commands.sh
| find "${ARTIFACT_DIR}" -path "${original_results}" -prune -o -type f -iname "*.xml" -print0 | while IFS= read -r -d '' xml_file; do | ||
| sed -i '/<system-out>.*<\/system-out>/d; /<system-out>/,/<\/system-out>/d; /<system-err>.*<\/system-err>/d; /<system-err>/,/<\/system-err>/d' "${xml_file}" |
There was a problem hiding this comment.
Prune path can miss original_results due to trailing slash mismatch.
At Line 59, -path "${original_results}" may not match because original_results includes a trailing /. If prune misses, Line 60 will also rewrite backup XMLs, so originals are no longer preserved.
Suggested fix
- find "${ARTIFACT_DIR}" -path "${original_results}" -prune -o -type f -iname "*.xml" -print0 | while IFS= read -r -d '' xml_file; do
+ original_results_prune="${original_results%/}"
+ find "${ARTIFACT_DIR}" -path "${original_results_prune}" -prune -o -type f -iname "*.xml" -print0 | while IFS= read -r -d '' xml_file; do
sed -i '/<system-out>.*<\/system-out>/d; /<system-out>/,/<\/system-out>/d; /<system-err>.*<\/system-err>/d; /<system-err>/,/<\/system-err>/d' "${xml_file}"
done🤖 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/stackrox/qa-e2e/stackrox-qa-e2e-commands.sh` around
lines 59 - 60, The issue is that the `-path "${original_results}" -prune`
argument in the find command at line 59 fails to exclude the backup directory
due to a trailing slash mismatch, causing the sed command on line 60 to
incorrectly modify and overwrite the original backup XML files. To fix this,
ensure the path pattern used in the `-path` argument properly matches the actual
directory structure by removing any trailing slash from the `original_results`
variable before using it in the find command, so that the prune correctly
excludes the backup directory and preserves the original results.
b2e0cef to
6b7e13b
Compare
|
/hold |
|
/pj-rehearse periodic-ci-stackrox-stackrox-master-ocp-4.22-lpMainline-lp-ocp-compat-cr--acs--tests-aws |
|
@davdhacs: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
The QA E2E suite produces ~2.4 MiB of JUnit XML, of which 91% is <system-out> content (Gradle test output). The EXIT trap merges these and copies to SHARED_DIR (a Kubernetes Secret with a 1 MiB limit), failing with "Secret is invalid: data: Too long" on every run. Strip <system-out> and <system-err> inside the EXIT trap, before ExitTrap--PostProcessPrep merges and copies. By this point, dispatch.sh has already exited and its ci_exit_trap has run junit2jira and flakechecker with the full data. The stripping cannot go after dispatch.sh because handle_dangling_processes kills the parent step script, so code after dispatch.sh never executes. The single-line sed pattern must come first because GNU sed's range /start/,/end/d looks for end on the NEXT line — when both tags are on the same line (Gradle's empty CDATA), the range extends to EOF. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
6b7e13b to
98ef3eb
Compare
|
/pj-rehearse periodic-ci-stackrox-stackrox-master-ocp-4.22-lpMainline-lp-ocp-compat-cr--acs--tests-aws |
|
@davdhacs: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Temporary config to rehearse the LP interop job against the stackrox PR branch that fixes handle_dangling_processes killing the step script. Combined with the step registry fix (system-out stripping), this exercises both fixes end-to-end. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/pj-rehearse periodic-ci-stackrox-stackrox-fix-dangling-process-kill-ocp-4.22-lpMainline-lp-ocp-compat-cr--acs--tests-aws |
|
@davdhacs: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@davdhacs: job(s): periodic-ci-stackrox-stackrox-fix-dangling-process-kill-ocp-4.22-lpMainline-lp-ocp-compat-cr--acs--tests-aws either don't exist or were not found to be affected, and cannot be rehearsed |
Generated by `make jobs`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: davdhacs 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 |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse periodic-ci-stackrox-stackrox-fix-dangling-process-kill-ocp-4.22-lpMainline-lp-ocp-compat-cr--acs--tests-aws |
|
@davdhacs: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-stackrox-stackrox-fix-dangling-process-kill-ocp-4.22-lpMainline-lp-ocp-compat-cr--acs--tests-aws |
|
@davdhacs: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-stackrox-stackrox-fix-dangling-process-kill-ocp-4.22-lpMainline-lp-ocp-compat-cr--acs--tests-aws |
|
@davdhacs: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-stackrox-stackrox-fix-dangling-process-kill-ocp-4.22-lpMainline-lp-ocp-compat-cr--acs--tests-aws |
|
@davdhacs: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-stackrox-stackrox-fix-dangling-process-kill-ocp-4.22-lpMainline-lp-ocp-compat-cr--acs--tests-aws |
|
@davdhacs: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@davdhacs: 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. |
What
Strip
<system-out>and<system-err>blocks from JUnit XML files in thestackrox-qa-e2estepafter
dispatch.shreturns but before the EXIT trap fires.Why
The StackRox QA E2E suite produces ~2.4 MiB of merged JUnit XML, of which 91% is
<system-out>content (Gradle's captured test output). The
ExitTrap--PostProcessPrepEXIT trap merges allJUnit XMLs and copies the result to
SHARED_DIR, which is backed by a Kubernetes Secret with ahard 1 MiB size limit. Every LP interop run fails with:
All tests pass — the job fails purely in post-processing.
How
The stripping runs after
dispatch.shreturns — at that point,junit2jira(which includesstdout/stderr in Jira issue descriptions) and
flakecheckerhave already read the full JUnitdata during dispatch.sh's own exit trap. Then when this script exits, the
ExitTrap--PostProcessPrepfinds the stripped files and the merged result fits in the Secret.
The
sedpattern handles both single-line (<system-out><![CDATA[]]></system-out>) and multi-lineblocks. The single-line pattern comes first because GNU sed's range
/start/,/end/dlooks for theend on the next line — when both tags are on the same line, the range extends to EOF.
After stripping, the total drops from ~2.4 MiB to ~230 KiB.