Archive pdf - #82757
Open
paigerube14 wants to merge 3 commits into
Open
Conversation
Contributor
WalkthroughFifteen chaos step scripts now check for ChangesChaos report artifact collection
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 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 |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: paigerube14 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 |
openshift-ci
Bot
added
the
approved
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: paigerube14 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
ci-operator/step-registry/redhat-chaos/application-outages/console/redhat-chaos-application-outages-console-commands.sh (1)
54-54: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winQuote the artifact destination in every wrapper.
The new
cpcommands pass an unquoted destination. If${ARTIFACT_DIR}contains whitespace or glob characters, the copy can receive incorrect arguments.
ci-operator/step-registry/redhat-chaos/application-outages/console/redhat-chaos-application-outages-console-commands.sh#L54-L54: use"${ARTIFACT_DIR}/kraken.report.pdf".ci-operator/step-registry/redhat-chaos/container-scenarios/etcd-hangup/redhat-chaos-container-scenarios-etcd-hangup-commands.sh#L50-L50: use"${ARTIFACT_DIR}/kraken.report.pdf".ci-operator/step-registry/redhat-chaos/hog-scenarios/cpu/redhat-chaos-hog-scenarios-cpu-commands.sh#L50-L50: use"${ARTIFACT_DIR}/kraken.report.pdf".ci-operator/step-registry/redhat-chaos/hog-scenarios/io/redhat-chaos-hog-scenarios-io-commands.sh#L51-L51: use"${ARTIFACT_DIR}/kraken.report.pdf".ci-operator/step-registry/redhat-chaos/service-disruption/redhat-chaos-service-disruption-commands.sh#L49-L49: use"${ARTIFACT_DIR}/kraken.report.pdf".ci-operator/step-registry/redhat-chaos/syn-flood/redhat-chaos-syn-flood-commands.sh#L49-L49: use"${ARTIFACT_DIR}/kraken.report.pdf".ci-operator/step-registry/redhat-chaos/time-scenarios/redhat-chaos-time-scenarios-commands.sh#L52-L52: use"${ARTIFACT_DIR}/kraken.report.pdf".ci-operator/step-registry/redhat-chaos/zone-outage/redhat-chaos-zone-outage-commands.sh#L83-L83: use"${ARTIFACT_DIR}/kraken.report.pdf".🤖 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/redhat-chaos/application-outages/console/redhat-chaos-application-outages-console-commands.sh` at line 54, Quote the artifact destination in each cp command so ARTIFACT_DIR values containing whitespace or glob characters are passed as one path: update the commands at ci-operator/step-registry/redhat-chaos/application-outages/console/redhat-chaos-application-outages-console-commands.sh:54, ci-operator/step-registry/redhat-chaos/container-scenarios/etcd-hangup/redhat-chaos-container-scenarios-etcd-hangup-commands.sh:50, ci-operator/step-registry/redhat-chaos/hog-scenarios/cpu/redhat-chaos-hog-scenarios-cpu-commands.sh:50, ci-operator/step-registry/redhat-chaos/hog-scenarios/io/redhat-chaos-hog-scenarios-io-commands.sh:51, ci-operator/step-registry/redhat-chaos/service-disruption/redhat-chaos-service-disruption-commands.sh:49, ci-operator/step-registry/redhat-chaos/syn-flood/redhat-chaos-syn-flood-commands.sh:49, ci-operator/step-registry/redhat-chaos/time-scenarios/redhat-chaos-time-scenarios-commands.sh:52, and ci-operator/step-registry/redhat-chaos/zone-outage/redhat-chaos-zone-outage-commands.sh:83 to use the quoted "${ARTIFACT_DIR}/kraken.report.pdf" destination.Source: Linters/SAST tools
🤖 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/redhat-chaos/application-outages/console/redhat-chaos-application-outages-console-commands.sh`:
- Around line 53-55: Update the prow_run.sh invocation and report-copy flow in
all eight listed files—application-outages console (53-55), container-scenarios
etcd-hangup (49-51), hog-scenarios CPU (49-51), hog-scenarios IO (50-52),
service-disruption (48-50), syn-flood (48-50), time-scenarios (51-53), and
zone-outage (82-84)—to capture the scenario exit status without triggering
errexit, copy kraken.report.pdf regardless of that status, then return the
original status.
In
`@ci-operator/step-registry/redhat-chaos/hog-scenarios/memory/redhat-chaos-hog-scenarios-memory-commands.sh`:
- Around line 51-53: Make the report-collection flow failure-safe by capturing
the direct prow_run.sh status with rc=$? via an || rc=$? guard before copying
the PDF, then preserve the existing conditional copy and exit "$rc" afterward.
Apply this sequence in
ci-operator/step-registry/redhat-chaos/hog-scenarios/memory/redhat-chaos-hog-scenarios-memory-commands.sh:51-53,
ci-operator/step-registry/redhat-chaos/network-chaos/redhat-chaos-network-chaos-commands.sh:49-51,
ci-operator/step-registry/redhat-chaos/node-disruptions/master-outage/redhat-chaos-node-disruptions-master-outage-commands.sh:113-115,
ci-operator/step-registry/redhat-chaos/pod-network-chaos/redhat-chaos-pod-network-chaos-commands.sh:50-52,
ci-operator/step-registry/redhat-chaos/pod-scenarios/etcd-disruption/redhat-chaos-pod-scenarios-etcd-disruption-commands.sh:74-76,
and
ci-operator/step-registry/redhat-chaos/power-outage/redhat-chaos-power-outage-commands.sh:94-96.
- Line 52: Quote the ARTIFACT_DIR destination in each report-copy command to
prevent shell word splitting and glob expansion. Update the cp commands at
ci-operator/step-registry/redhat-chaos/hog-scenarios/memory/redhat-chaos-hog-scenarios-memory-commands.sh:52-52,
ci-operator/step-registry/redhat-chaos/kubevirt-outage/redhat-chaos-kubevirt-outage-commands.sh:79-79,
ci-operator/step-registry/redhat-chaos/network-chaos/redhat-chaos-network-chaos-commands.sh:50-50,
ci-operator/step-registry/redhat-chaos/node-disruptions/master-outage/redhat-chaos-node-disruptions-master-outage-commands.sh:114-114,
ci-operator/step-registry/redhat-chaos/pod-network-chaos/redhat-chaos-pod-network-chaos-commands.sh:51-51,
ci-operator/step-registry/redhat-chaos/pod-scenarios/etcd-disruption/redhat-chaos-pod-scenarios-etcd-disruption-commands.sh:75-75,
and
ci-operator/step-registry/redhat-chaos/power-outage/redhat-chaos-power-outage-commands.sh:95-95
so each destination uses the quoted ARTIFACT_DIR form.
In
`@ci-operator/step-registry/redhat-chaos/kubevirt-outage/redhat-chaos-kubevirt-outage-commands.sh`:
- Around line 78-80: Preserve the chaos runner’s exit status in the flow around
./kubevirt-outage/prow_run.sh: initialize rc before invoking it, keep the
conditional rc capture, remove the later rc=$? assignment after PDF collection,
and exit with rc only after artifact handling completes.
---
Nitpick comments:
In
`@ci-operator/step-registry/redhat-chaos/application-outages/console/redhat-chaos-application-outages-console-commands.sh`:
- Line 54: Quote the artifact destination in each cp command so ARTIFACT_DIR
values containing whitespace or glob characters are passed as one path: update
the commands at
ci-operator/step-registry/redhat-chaos/application-outages/console/redhat-chaos-application-outages-console-commands.sh:54,
ci-operator/step-registry/redhat-chaos/container-scenarios/etcd-hangup/redhat-chaos-container-scenarios-etcd-hangup-commands.sh:50,
ci-operator/step-registry/redhat-chaos/hog-scenarios/cpu/redhat-chaos-hog-scenarios-cpu-commands.sh:50,
ci-operator/step-registry/redhat-chaos/hog-scenarios/io/redhat-chaos-hog-scenarios-io-commands.sh:51,
ci-operator/step-registry/redhat-chaos/service-disruption/redhat-chaos-service-disruption-commands.sh:49,
ci-operator/step-registry/redhat-chaos/syn-flood/redhat-chaos-syn-flood-commands.sh:49,
ci-operator/step-registry/redhat-chaos/time-scenarios/redhat-chaos-time-scenarios-commands.sh:52,
and
ci-operator/step-registry/redhat-chaos/zone-outage/redhat-chaos-zone-outage-commands.sh:83
to use the quoted "${ARTIFACT_DIR}/kraken.report.pdf" destination.
🪄 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: Pro Plus
Run ID: 520be517-9f2a-4bc8-9607-bf48f0ad4b0d
📒 Files selected for processing (15)
ci-operator/step-registry/redhat-chaos/application-outages/console/redhat-chaos-application-outages-console-commands.shci-operator/step-registry/redhat-chaos/container-scenarios/etcd-hangup/redhat-chaos-container-scenarios-etcd-hangup-commands.shci-operator/step-registry/redhat-chaos/hog-scenarios/cpu/redhat-chaos-hog-scenarios-cpu-commands.shci-operator/step-registry/redhat-chaos/hog-scenarios/io/redhat-chaos-hog-scenarios-io-commands.shci-operator/step-registry/redhat-chaos/hog-scenarios/memory/redhat-chaos-hog-scenarios-memory-commands.shci-operator/step-registry/redhat-chaos/kubevirt-outage/redhat-chaos-kubevirt-outage-commands.shci-operator/step-registry/redhat-chaos/network-chaos/redhat-chaos-network-chaos-commands.shci-operator/step-registry/redhat-chaos/node-disruptions/master-outage/redhat-chaos-node-disruptions-master-outage-commands.shci-operator/step-registry/redhat-chaos/pod-network-chaos/redhat-chaos-pod-network-chaos-commands.shci-operator/step-registry/redhat-chaos/pod-scenarios/etcd-disruption/redhat-chaos-pod-scenarios-etcd-disruption-commands.shci-operator/step-registry/redhat-chaos/power-outage/redhat-chaos-power-outage-commands.shci-operator/step-registry/redhat-chaos/service-disruption/redhat-chaos-service-disruption-commands.shci-operator/step-registry/redhat-chaos/syn-flood/redhat-chaos-syn-flood-commands.shci-operator/step-registry/redhat-chaos/time-scenarios/redhat-chaos-time-scenarios-commands.shci-operator/step-registry/redhat-chaos/zone-outage/redhat-chaos-zone-outage-commands.sh
Comment on lines
+53
to
+55
| if [[ -f /home/krkn/kraken/kraken.report.pdf ]]; then | ||
| cp /home/krkn/kraken/kraken.report.pdf ${ARTIFACT_DIR}/kraken.report.pdf | ||
| fi |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Preserve report collection on scenario failure.
All eight wrappers run prow_run.sh with errexit enabled. A nonzero scenario result exits the wrapper before the PDF copy runs.
ci-operator/step-registry/redhat-chaos/application-outages/console/redhat-chaos-application-outages-console-commands.sh#L53-L55: capture the status without triggeringerrexit, then copy the report.ci-operator/step-registry/redhat-chaos/container-scenarios/etcd-hangup/redhat-chaos-container-scenarios-etcd-hangup-commands.sh#L49-L51: capture the status without triggeringerrexit, then copy the report.ci-operator/step-registry/redhat-chaos/hog-scenarios/cpu/redhat-chaos-hog-scenarios-cpu-commands.sh#L49-L51: capture the status without triggeringerrexit, then copy the report.ci-operator/step-registry/redhat-chaos/hog-scenarios/io/redhat-chaos-hog-scenarios-io-commands.sh#L50-L52: capture the status without triggeringerrexit, then copy the report.ci-operator/step-registry/redhat-chaos/service-disruption/redhat-chaos-service-disruption-commands.sh#L48-L50: capture the status without triggeringerrexit, then copy the report.ci-operator/step-registry/redhat-chaos/syn-flood/redhat-chaos-syn-flood-commands.sh#L48-L50: capture the status without triggeringerrexit, then copy the report.ci-operator/step-registry/redhat-chaos/time-scenarios/redhat-chaos-time-scenarios-commands.sh#L51-L53: capture the status without triggeringerrexit, then copy the report.ci-operator/step-registry/redhat-chaos/zone-outage/redhat-chaos-zone-outage-commands.sh#L82-L84: capture the status without triggeringerrexit, then copy the report.
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 54-54: Double quote to prevent globbing and word splitting.
(SC2086)
📍 Affects 8 files
ci-operator/step-registry/redhat-chaos/application-outages/console/redhat-chaos-application-outages-console-commands.sh#L53-L55(this comment)ci-operator/step-registry/redhat-chaos/container-scenarios/etcd-hangup/redhat-chaos-container-scenarios-etcd-hangup-commands.sh#L49-L51ci-operator/step-registry/redhat-chaos/hog-scenarios/cpu/redhat-chaos-hog-scenarios-cpu-commands.sh#L49-L51ci-operator/step-registry/redhat-chaos/hog-scenarios/io/redhat-chaos-hog-scenarios-io-commands.sh#L50-L52ci-operator/step-registry/redhat-chaos/service-disruption/redhat-chaos-service-disruption-commands.sh#L48-L50ci-operator/step-registry/redhat-chaos/syn-flood/redhat-chaos-syn-flood-commands.sh#L48-L50ci-operator/step-registry/redhat-chaos/time-scenarios/redhat-chaos-time-scenarios-commands.sh#L51-L53ci-operator/step-registry/redhat-chaos/zone-outage/redhat-chaos-zone-outage-commands.sh#L82-L84
🤖 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/redhat-chaos/application-outages/console/redhat-chaos-application-outages-console-commands.sh`
around lines 53 - 55, Update the prow_run.sh invocation and report-copy flow in
all eight listed files—application-outages console (53-55), container-scenarios
etcd-hangup (49-51), hog-scenarios CPU (49-51), hog-scenarios IO (50-52),
service-disruption (48-50), syn-flood (48-50), time-scenarios (51-53), and
zone-outage (82-84)—to capture the scenario exit status without triggering
errexit, copy kraken.report.pdf regardless of that status, then return the
original status.
Comment on lines
+51
to
+53
| if [[ -f /home/krkn/kraken/kraken.report.pdf ]]; then | ||
| cp /home/krkn/kraken/kraken.report.pdf ${ARTIFACT_DIR}/kraken.report.pdf | ||
| fi |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make PDF collection failure-safe.
Each listed script enables set -o errexit before a direct prow_run.sh call. A non-zero runner status exits before the new copy block, so a report that exists after a failed run is not archived. Capture the status with || rc=$?, perform the copy, then exit "$rc".
ci-operator/step-registry/redhat-chaos/hog-scenarios/memory/redhat-chaos-hog-scenarios-memory-commands.sh#L51-L53: Replace the direct runner/status sequence with failure capture before copying the report.ci-operator/step-registry/redhat-chaos/network-chaos/redhat-chaos-network-chaos-commands.sh#L49-L51: Replace the direct runner/status sequence with failure capture before copying the report.ci-operator/step-registry/redhat-chaos/node-disruptions/master-outage/redhat-chaos-node-disruptions-master-outage-commands.sh#L113-L115: Replace the direct runner/status sequence with failure capture before copying the report.ci-operator/step-registry/redhat-chaos/pod-network-chaos/redhat-chaos-pod-network-chaos-commands.sh#L50-L52: Replace the direct runner/status sequence with failure capture before copying the report.ci-operator/step-registry/redhat-chaos/pod-scenarios/etcd-disruption/redhat-chaos-pod-scenarios-etcd-disruption-commands.sh#L74-L76: Replace the direct runner/status sequence with failure capture before copying the report.ci-operator/step-registry/redhat-chaos/power-outage/redhat-chaos-power-outage-commands.sh#L94-L96: Replace the direct runner/status sequence with failure capture before copying the report.
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 52-52: Double quote to prevent globbing and word splitting.
(SC2086)
📍 Affects 6 files
ci-operator/step-registry/redhat-chaos/hog-scenarios/memory/redhat-chaos-hog-scenarios-memory-commands.sh#L51-L53(this comment)ci-operator/step-registry/redhat-chaos/network-chaos/redhat-chaos-network-chaos-commands.sh#L49-L51ci-operator/step-registry/redhat-chaos/node-disruptions/master-outage/redhat-chaos-node-disruptions-master-outage-commands.sh#L113-L115ci-operator/step-registry/redhat-chaos/pod-network-chaos/redhat-chaos-pod-network-chaos-commands.sh#L50-L52ci-operator/step-registry/redhat-chaos/pod-scenarios/etcd-disruption/redhat-chaos-pod-scenarios-etcd-disruption-commands.sh#L74-L76ci-operator/step-registry/redhat-chaos/power-outage/redhat-chaos-power-outage-commands.sh#L94-L96
🤖 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/redhat-chaos/hog-scenarios/memory/redhat-chaos-hog-scenarios-memory-commands.sh`
around lines 51 - 53, Make the report-collection flow failure-safe by capturing
the direct prow_run.sh status with rc=$? via an || rc=$? guard before copying
the PDF, then preserve the existing conditional copy and exit "$rc" afterward.
Apply this sequence in
ci-operator/step-registry/redhat-chaos/hog-scenarios/memory/redhat-chaos-hog-scenarios-memory-commands.sh:51-53,
ci-operator/step-registry/redhat-chaos/network-chaos/redhat-chaos-network-chaos-commands.sh:49-51,
ci-operator/step-registry/redhat-chaos/node-disruptions/master-outage/redhat-chaos-node-disruptions-master-outage-commands.sh:113-115,
ci-operator/step-registry/redhat-chaos/pod-network-chaos/redhat-chaos-pod-network-chaos-commands.sh:50-52,
ci-operator/step-registry/redhat-chaos/pod-scenarios/etcd-disruption/redhat-chaos-pod-scenarios-etcd-disruption-commands.sh:74-76,
and
ci-operator/step-registry/redhat-chaos/power-outage/redhat-chaos-power-outage-commands.sh:94-96.
|
|
||
|
|
||
| if [[ -f /home/krkn/kraken/kraken.report.pdf ]]; then | ||
| cp /home/krkn/kraken/kraken.report.pdf ${ARTIFACT_DIR}/kraken.report.pdf |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Quote ${ARTIFACT_DIR} in every new cp destination.
Each destination is unquoted. Shell word splitting or glob expansion can change the destination if ARTIFACT_DIR contains whitespace or wildcard characters.
ci-operator/step-registry/redhat-chaos/hog-scenarios/memory/redhat-chaos-hog-scenarios-memory-commands.sh#L52-L52: Use"${ARTIFACT_DIR}/kraken.report.pdf".ci-operator/step-registry/redhat-chaos/kubevirt-outage/redhat-chaos-kubevirt-outage-commands.sh#L79-L79: Use"${ARTIFACT_DIR}/kraken.report.pdf".ci-operator/step-registry/redhat-chaos/network-chaos/redhat-chaos-network-chaos-commands.sh#L50-L50: Use"${ARTIFACT_DIR}/kraken.report.pdf".ci-operator/step-registry/redhat-chaos/node-disruptions/master-outage/redhat-chaos-node-disruptions-master-outage-commands.sh#L114-L114: Use"${ARTIFACT_DIR}/kraken.report.pdf".ci-operator/step-registry/redhat-chaos/pod-network-chaos/redhat-chaos-pod-network-chaos-commands.sh#L51-L51: Use"${ARTIFACT_DIR}/kraken.report.pdf".ci-operator/step-registry/redhat-chaos/pod-scenarios/etcd-disruption/redhat-chaos-pod-scenarios-etcd-disruption-commands.sh#L75-L75: Use"${ARTIFACT_DIR}/kraken.report.pdf".ci-operator/step-registry/redhat-chaos/power-outage/redhat-chaos-power-outage-commands.sh#L95-L95: Use"${ARTIFACT_DIR}/kraken.report.pdf".
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 52-52: Double quote to prevent globbing and word splitting.
(SC2086)
📍 Affects 7 files
ci-operator/step-registry/redhat-chaos/hog-scenarios/memory/redhat-chaos-hog-scenarios-memory-commands.sh#L52-L52(this comment)ci-operator/step-registry/redhat-chaos/kubevirt-outage/redhat-chaos-kubevirt-outage-commands.sh#L79-L79ci-operator/step-registry/redhat-chaos/network-chaos/redhat-chaos-network-chaos-commands.sh#L50-L50ci-operator/step-registry/redhat-chaos/node-disruptions/master-outage/redhat-chaos-node-disruptions-master-outage-commands.sh#L114-L114ci-operator/step-registry/redhat-chaos/pod-network-chaos/redhat-chaos-pod-network-chaos-commands.sh#L51-L51ci-operator/step-registry/redhat-chaos/pod-scenarios/etcd-disruption/redhat-chaos-pod-scenarios-etcd-disruption-commands.sh#L75-L75ci-operator/step-registry/redhat-chaos/power-outage/redhat-chaos-power-outage-commands.sh#L95-L95
🤖 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/redhat-chaos/hog-scenarios/memory/redhat-chaos-hog-scenarios-memory-commands.sh`
at line 52, Quote the ARTIFACT_DIR destination in each report-copy command to
prevent shell word splitting and glob expansion. Update the cp commands at
ci-operator/step-registry/redhat-chaos/hog-scenarios/memory/redhat-chaos-hog-scenarios-memory-commands.sh:52-52,
ci-operator/step-registry/redhat-chaos/kubevirt-outage/redhat-chaos-kubevirt-outage-commands.sh:79-79,
ci-operator/step-registry/redhat-chaos/network-chaos/redhat-chaos-network-chaos-commands.sh:50-50,
ci-operator/step-registry/redhat-chaos/node-disruptions/master-outage/redhat-chaos-node-disruptions-master-outage-commands.sh:114-114,
ci-operator/step-registry/redhat-chaos/pod-network-chaos/redhat-chaos-pod-network-chaos-commands.sh:51-51,
ci-operator/step-registry/redhat-chaos/pod-scenarios/etcd-disruption/redhat-chaos-pod-scenarios-etcd-disruption-commands.sh:75-75,
and
ci-operator/step-registry/redhat-chaos/power-outage/redhat-chaos-power-outage-commands.sh:95-95
so each destination uses the quoted ARTIFACT_DIR form.
Source: Linters/SAST tools
Comment on lines
+78
to
+80
| if [[ -f /home/krkn/kraken/kraken.report.pdf ]]; then | ||
| cp /home/krkn/kraken/kraken.report.pdf ${ARTIFACT_DIR}/kraken.report.pdf | ||
| fi |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve the chaos runner status while collecting the PDF.
./kubevirt-outage/prow_run.sh || rc=$? allows this block to run, but the following rc=$? overwrites a non-zero status with 0. The step can pass after a failed disruption. Remove the second assignment, initialize rc before the runner, and exit with rc after artifact collection.
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 79-79: 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/redhat-chaos/kubevirt-outage/redhat-chaos-kubevirt-outage-commands.sh`
around lines 78 - 80, Preserve the chaos runner’s exit status in the flow around
./kubevirt-outage/prow_run.sh: initialize rc before invoking it, keep the
conditional rc capture, remove the later rc=$? assignment after PDF collection,
and exit with rc only after artifact handling completes.
Contributor
Author
|
/pj-rehearse periodic-ci-redhat-chaos-prow-scripts-main-cr-4.22-nightly-krkn-hub-aws |
Contributor
|
@paigerube14: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Contributor
|
@paigerube14: 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. |
Contributor
Author
|
/pj-rehearse periodic-ci-redhat-chaos-prow-scripts-main-cr-4.22-nightly-krkn-hub-aws |
Contributor
|
@paigerube14: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Contributor
|
[REHEARSALNOTIFIER]
A total of 155 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: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/redhat-chaos/application-outages/console/redhat-chaos-application-outages-console-commands.sh`:
- Around line 53-55: Preserve the chaos runner exit status while collecting PDF
artifacts in all wrappers: in
ci-operator/step-registry/redhat-chaos/application-outages/console/redhat-chaos-application-outages-console-commands.sh
(53-55),
container-scenarios/etcd-hangup/redhat-chaos-container-scenarios-etcd-hangup-commands.sh
(49-51), hog-scenarios/io/redhat-chaos-hog-scenarios-io-commands.sh (50-52),
network-chaos/redhat-chaos-network-chaos-commands.sh (49-51),
node-disruptions/master-outage/redhat-chaos-node-disruptions-master-outage-commands.sh
(113-115),
pod-scenarios/etcd-disruption/redhat-chaos-pod-scenarios-etcd-disruption-commands.sh
(74-76), power-outage/redhat-chaos-power-outage-commands.sh (94-96),
service-disruption/redhat-chaos-service-disruption-commands.sh (48-50),
syn-flood/redhat-chaos-syn-flood-commands.sh (48-50),
time-scenarios/redhat-chaos-time-scenarios-commands.sh (51-53), and
zone-outage/redhat-chaos-zone-outage-commands.sh (82-84), capture the runner
failure with `|| rc=$?`, collect artifacts, then exit with rc; in
kubevirt-outage/redhat-chaos-kubevirt-outage-commands.sh (78-80), remove the
later rc=$? assignment so the captured failure is not overwritten.
In
`@ci-operator/step-registry/redhat-chaos/hog-scenarios/cpu/redhat-chaos-hog-scenarios-cpu-commands.sh`:
- Around line 49-51: Preserve report collection when prow_run.sh fails by
applying the failure-safe runner sequence in
ci-operator/step-registry/redhat-chaos/hog-scenarios/cpu/redhat-chaos-hog-scenarios-cpu-commands.sh:49-51,
ci-operator/step-registry/redhat-chaos/hog-scenarios/memory/redhat-chaos-hog-scenarios-memory-commands.sh:51-53,
and
ci-operator/step-registry/redhat-chaos/pod-network-chaos/redhat-chaos-pod-network-chaos-commands.sh:50-52:
capture the runner return status without triggering errexit, execute all
report-copy blocks, then exit with the captured status via exit "$rc".
🪄 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: Pro Plus
Run ID: 0276aeb2-8dda-421c-85da-ce92d619d616
📒 Files selected for processing (15)
ci-operator/step-registry/redhat-chaos/application-outages/console/redhat-chaos-application-outages-console-commands.shci-operator/step-registry/redhat-chaos/container-scenarios/etcd-hangup/redhat-chaos-container-scenarios-etcd-hangup-commands.shci-operator/step-registry/redhat-chaos/hog-scenarios/cpu/redhat-chaos-hog-scenarios-cpu-commands.shci-operator/step-registry/redhat-chaos/hog-scenarios/io/redhat-chaos-hog-scenarios-io-commands.shci-operator/step-registry/redhat-chaos/hog-scenarios/memory/redhat-chaos-hog-scenarios-memory-commands.shci-operator/step-registry/redhat-chaos/kubevirt-outage/redhat-chaos-kubevirt-outage-commands.shci-operator/step-registry/redhat-chaos/network-chaos/redhat-chaos-network-chaos-commands.shci-operator/step-registry/redhat-chaos/node-disruptions/master-outage/redhat-chaos-node-disruptions-master-outage-commands.shci-operator/step-registry/redhat-chaos/pod-network-chaos/redhat-chaos-pod-network-chaos-commands.shci-operator/step-registry/redhat-chaos/pod-scenarios/etcd-disruption/redhat-chaos-pod-scenarios-etcd-disruption-commands.shci-operator/step-registry/redhat-chaos/power-outage/redhat-chaos-power-outage-commands.shci-operator/step-registry/redhat-chaos/service-disruption/redhat-chaos-service-disruption-commands.shci-operator/step-registry/redhat-chaos/syn-flood/redhat-chaos-syn-flood-commands.shci-operator/step-registry/redhat-chaos/time-scenarios/redhat-chaos-time-scenarios-commands.shci-operator/step-registry/redhat-chaos/zone-outage/redhat-chaos-zone-outage-commands.sh
Comment on lines
+53
to
+55
| if [[ -f /tmp/report.out.pdf ]]; then | ||
| cp /tmp/report.out.pdf ${ARTIFACT_DIR}/kraken.report.pdf | ||
| fi |
Contributor
There was a problem hiding this comment.
Preserve the runner status while collecting the PDF.
All twelve wrappers can skip PDF collection when the chaos runner fails because errexit terminates the script first. kubevirt-outage also overwrites its captured failure status.
ci-operator/step-registry/redhat-chaos/application-outages/console/redhat-chaos-application-outages-console-commands.sh#L53-L55: capture the runner status with|| rc=$?, collect artifacts, then exit withrc.ci-operator/step-registry/redhat-chaos/container-scenarios/etcd-hangup/redhat-chaos-container-scenarios-etcd-hangup-commands.sh#L49-L51: capture the runner status before artifact collection.ci-operator/step-registry/redhat-chaos/hog-scenarios/io/redhat-chaos-hog-scenarios-io-commands.sh#L50-L52: capture the runner status before artifact collection.ci-operator/step-registry/redhat-chaos/kubevirt-outage/redhat-chaos-kubevirt-outage-commands.sh#L78-L80: remove the laterrc=$?that resets the failure status.ci-operator/step-registry/redhat-chaos/network-chaos/redhat-chaos-network-chaos-commands.sh#L49-L51: capture the runner status before artifact collection.ci-operator/step-registry/redhat-chaos/node-disruptions/master-outage/redhat-chaos-node-disruptions-master-outage-commands.sh#L113-L115: capture the runner status before artifact collection.ci-operator/step-registry/redhat-chaos/pod-scenarios/etcd-disruption/redhat-chaos-pod-scenarios-etcd-disruption-commands.sh#L74-L76: capture the runner status before artifact collection.ci-operator/step-registry/redhat-chaos/power-outage/redhat-chaos-power-outage-commands.sh#L94-L96: capture the runner status before artifact collection.ci-operator/step-registry/redhat-chaos/service-disruption/redhat-chaos-service-disruption-commands.sh#L48-L50: capture the runner status before artifact collection.ci-operator/step-registry/redhat-chaos/syn-flood/redhat-chaos-syn-flood-commands.sh#L48-L50: capture the runner status before artifact collection.ci-operator/step-registry/redhat-chaos/time-scenarios/redhat-chaos-time-scenarios-commands.sh#L51-L53: capture the runner status before artifact collection.ci-operator/step-registry/redhat-chaos/zone-outage/redhat-chaos-zone-outage-commands.sh#L82-L84: capture the runner status before artifact collection.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 53-53: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/report.out.pdf
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
🪛 Shellcheck (0.11.0)
[info] 54-54: Double quote to prevent globbing and word splitting.
(SC2086)
📍 Affects 12 files
ci-operator/step-registry/redhat-chaos/application-outages/console/redhat-chaos-application-outages-console-commands.sh#L53-L55(this comment)ci-operator/step-registry/redhat-chaos/container-scenarios/etcd-hangup/redhat-chaos-container-scenarios-etcd-hangup-commands.sh#L49-L51ci-operator/step-registry/redhat-chaos/hog-scenarios/io/redhat-chaos-hog-scenarios-io-commands.sh#L50-L52ci-operator/step-registry/redhat-chaos/kubevirt-outage/redhat-chaos-kubevirt-outage-commands.sh#L78-L80ci-operator/step-registry/redhat-chaos/network-chaos/redhat-chaos-network-chaos-commands.sh#L49-L51ci-operator/step-registry/redhat-chaos/node-disruptions/master-outage/redhat-chaos-node-disruptions-master-outage-commands.sh#L113-L115ci-operator/step-registry/redhat-chaos/pod-scenarios/etcd-disruption/redhat-chaos-pod-scenarios-etcd-disruption-commands.sh#L74-L76ci-operator/step-registry/redhat-chaos/power-outage/redhat-chaos-power-outage-commands.sh#L94-L96ci-operator/step-registry/redhat-chaos/service-disruption/redhat-chaos-service-disruption-commands.sh#L48-L50ci-operator/step-registry/redhat-chaos/syn-flood/redhat-chaos-syn-flood-commands.sh#L48-L50ci-operator/step-registry/redhat-chaos/time-scenarios/redhat-chaos-time-scenarios-commands.sh#L51-L53ci-operator/step-registry/redhat-chaos/zone-outage/redhat-chaos-zone-outage-commands.sh#L82-L84
🤖 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/redhat-chaos/application-outages/console/redhat-chaos-application-outages-console-commands.sh`
around lines 53 - 55, Preserve the chaos runner exit status while collecting PDF
artifacts in all wrappers: in
ci-operator/step-registry/redhat-chaos/application-outages/console/redhat-chaos-application-outages-console-commands.sh
(53-55),
container-scenarios/etcd-hangup/redhat-chaos-container-scenarios-etcd-hangup-commands.sh
(49-51), hog-scenarios/io/redhat-chaos-hog-scenarios-io-commands.sh (50-52),
network-chaos/redhat-chaos-network-chaos-commands.sh (49-51),
node-disruptions/master-outage/redhat-chaos-node-disruptions-master-outage-commands.sh
(113-115),
pod-scenarios/etcd-disruption/redhat-chaos-pod-scenarios-etcd-disruption-commands.sh
(74-76), power-outage/redhat-chaos-power-outage-commands.sh (94-96),
service-disruption/redhat-chaos-service-disruption-commands.sh (48-50),
syn-flood/redhat-chaos-syn-flood-commands.sh (48-50),
time-scenarios/redhat-chaos-time-scenarios-commands.sh (51-53), and
zone-outage/redhat-chaos-zone-outage-commands.sh (82-84), capture the runner
failure with `|| rc=$?`, collect artifacts, then exit with rc; in
kubevirt-outage/redhat-chaos-kubevirt-outage-commands.sh (78-80), remove the
later rc=$? assignment so the captured failure is not overwritten.
Comment on lines
+49
to
+51
| if [[ -f /tmp/report.out.pdf ]]; then | ||
| cp /tmp/report.out.pdf ${ARTIFACT_DIR}/kraken.report.pdf | ||
| fi |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
Preserve report collection on runner failure.
errexit prevents all three copy blocks from running when prow_run.sh fails. Capture the runner status, copy the report, and restore the runner status with exit "$rc".
ci-operator/step-registry/redhat-chaos/hog-scenarios/cpu/redhat-chaos-hog-scenarios-cpu-commands.sh#L49-L51: Apply the failure-safe runner sequence.ci-operator/step-registry/redhat-chaos/hog-scenarios/memory/redhat-chaos-hog-scenarios-memory-commands.sh#L51-L53: Apply the failure-safe runner sequence.ci-operator/step-registry/redhat-chaos/pod-network-chaos/redhat-chaos-pod-network-chaos-commands.sh#L50-L52: Apply the failure-safe runner sequence.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 49-49: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/report.out.pdf
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
🪛 Shellcheck (0.11.0)
[info] 50-50: Double quote to prevent globbing and word splitting.
(SC2086)
📍 Affects 3 files
ci-operator/step-registry/redhat-chaos/hog-scenarios/cpu/redhat-chaos-hog-scenarios-cpu-commands.sh#L49-L51(this comment)ci-operator/step-registry/redhat-chaos/hog-scenarios/memory/redhat-chaos-hog-scenarios-memory-commands.sh#L51-L53ci-operator/step-registry/redhat-chaos/pod-network-chaos/redhat-chaos-pod-network-chaos-commands.sh#L50-L52
🤖 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/redhat-chaos/hog-scenarios/cpu/redhat-chaos-hog-scenarios-cpu-commands.sh`
around lines 49 - 51, Preserve report collection when prow_run.sh fails by
applying the failure-safe runner sequence in
ci-operator/step-registry/redhat-chaos/hog-scenarios/cpu/redhat-chaos-hog-scenarios-cpu-commands.sh:49-51,
ci-operator/step-registry/redhat-chaos/hog-scenarios/memory/redhat-chaos-hog-scenarios-memory-commands.sh:51-53,
and
ci-operator/step-registry/redhat-chaos/pod-network-chaos/redhat-chaos-pod-network-chaos-commands.sh:50-52:
capture the runner return status without triggering errexit, execute all
report-copy blocks, then exit with the captured status via exit "$rc".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Red Hat Chaos CI steps now archive
/tmp/report.out.pdfas${ARTIFACT_DIR}/kraken.report.pdfwhen the report exists. This makes Kraken PDF reports available as CI artifacts across outage, disruption, resource-hog, network, and scenario workflows.