MPICT: Apply mapping mechanism on Chaos steps#80748
Conversation
WalkthroughAdds ChangesChaos Step Reporting and lp-chaos Config Wiring
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[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 |
|
@oharan2, 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/redhat-chaos/container-scenarios/etcd-hangup/redhat-chaos-container-scenarios-etcd-hangup-commands.sh`:
- Around line 8-13: The issue is that the wget/curl download is executed inside
the eval command, so if the network request fails silently, eval still succeeds
with an empty string and the EXIT trap remains undefined, causing the script to
fail later. Separate the download operation from the eval by first capturing the
helper script content to a variable, then check if the capture was successful
(verify the variable is not empty), and only then eval the captured content.
Apply this same pattern to all similar download blocks throughout the file,
including all MAP_TESTS blocks for redhat-chaos time-scenarios,
pod-scenarios/etcd-disruption, stackrox qa-e2e, compliance-e2e,
openshift-pipelines tests and install, and interop-tests cnv-tests-e2e-deploy
and ocs-tests.
🪄 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: 8a9dbd44-4ff2-4a03-9034-cc23f4a1d64a
⛔ Files ignored due to path filters (2)
ci-operator/jobs/redhat-chaos/lp-chaos/redhat-chaos-lp-chaos-main-periodics.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/redhat-chaos/lp-chaos/redhat-chaos-lp-chaos-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (12)
ci-operator/config/redhat-chaos/lp-chaos/redhat-chaos-lp-chaos-main__ocp-4.22-lpGA-lp-chaos.yamlci-operator/step-registry/redhat-chaos/container-scenarios/etcd-hangup/redhat-chaos-container-scenarios-etcd-hangup-commands.shci-operator/step-registry/redhat-chaos/container-scenarios/etcd-hangup/redhat-chaos-container-scenarios-etcd-hangup-ref.yamlci-operator/step-registry/redhat-chaos/pod-scenarios/etcd-disruption/redhat-chaos-pod-scenarios-etcd-disruption-commands.shci-operator/step-registry/redhat-chaos/pod-scenarios/etcd-disruption/redhat-chaos-pod-scenarios-etcd-disruption-ref.yamlci-operator/step-registry/redhat-chaos/pod-scenarios/kube-apiserver-disruption/redhat-chaos-pod-scenarios-kube-apiserver-disruption-ref.yamlci-operator/step-registry/redhat-chaos/pod-scenarios/ovn-cp-disruption/redhat-chaos-pod-scenarios-ovn-cp-disruption-ref.yamlci-operator/step-registry/redhat-chaos/pod-scenarios/ovn-disruption/redhat-chaos-pod-scenarios-ovn-disruption-ref.yamlci-operator/step-registry/redhat-chaos/pod-scenarios/prometheus-disruption/redhat-chaos-pod-scenarios-prometheus-disruption-ref.yamlci-operator/step-registry/redhat-chaos/pod-scenarios/random-system-pods/redhat-chaos-pod-scenarios-random-system-pods-ref.yamlci-operator/step-registry/redhat-chaos/time-scenarios/redhat-chaos-time-scenarios-commands.shci-operator/step-registry/redhat-chaos/time-scenarios/redhat-chaos-time-scenarios-ref.yaml
| eval "$( | ||
| typeset -a _fURL=() | ||
| type -t wget 1>/dev/null && _fURL=(wget -qO-) || _fURL=(curl -fsSL) | ||
| "${_fURL[@]}" \ | ||
| https://raw.githubusercontent.com/RedHatQE/OpenShift-LP-QE--Tools/refs/heads/main/libs/bash/ci-operator/interop/common/ExitTrap--PostProcessPrep.sh | ||
| )"; trap ' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Demonstrates why a failed command substitution hidden inside eval is not a reliable fail-fast boundary.
bash -c 'set -e; eval "$(false)"; echo "eval continued after failed substitution"'Repository: openshift/release
Length of output: 102
🏁 Script executed:
# Locate and read the file mentioned in the review
fd -t f 'redhat-chaos-container-scenarios-etcd-hangup-commands.sh'Repository: openshift/release
Length of output: 189
🏁 Script executed:
# Once file is found, read it to verify the code matches the review snippet
cat -n ci-operator/step-registry/redhat-chaos/container-scenarios/etcd-hangup/redhat-chaos-container-scenarios-etcd-hangup-commands.shRepository: openshift/release
Length of output: 2560
🏁 Script executed:
# Search for all MAP_TESTS blocks in step registry to scope the impact
rg -t sh 'MAP_TESTS' ci-operator/step-registry/ -A 10 -B 2Repository: openshift/release
Length of output: 50373
Capture the helper download before eval to fail fast on network errors.
When the wget/curl command fails silently, eval "$(...)" still succeeds with an empty string, leaving the EXIT trap undefined and causing the script to fail later. Separate the download from the eval and check for errors before proceeding. Apply the same fix to all MAP_TESTS blocks in this PR (redhat-chaos time-scenarios, pod-scenarios/etcd-disruption; stackrox qa-e2e, compliance-e2e; openshift-pipelines tests, install; interop-tests cnv-tests-e2e-deploy, ocs-tests).
Proposed fix
if [ "${MAP_TESTS:-}" = "true" ]; then
- eval "$(
- typeset -a _fURL=()
- type -t wget 1>/dev/null && _fURL=(wget -qO-) || _fURL=(curl -fsSL)
- "${_fURL[@]}" \
+ typeset -a _fURL=()
+ type -t wget 1>/dev/null && _fURL=(wget -qO-) || _fURL=(curl -fsSL)
+ helper_script="$("${_fURL[@]}" \
https://raw.githubusercontent.com/RedHatQE/OpenShift-LP-QE--Tools/refs/heads/main/libs/bash/ci-operator/interop/common/ExitTrap--PostProcessPrep.sh
- )"; trap '
+ )" || {
+ echo "Failed to download ExitTrap--PostProcessPrep.sh" >&2
+ exit 1
+ }
+ eval "${helper_script}"
+ trap '
LP_IO__ET_PPP__NEW_TS_NAME="${DR__RP__CR_COMP_NAME}--%s" \
ExitTrap--PostProcessPrep junit--redhat-chaos-container-scenarios-etcd-hangup.xml
' EXIT📝 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.
| eval "$( | |
| typeset -a _fURL=() | |
| type -t wget 1>/dev/null && _fURL=(wget -qO-) || _fURL=(curl -fsSL) | |
| "${_fURL[@]}" \ | |
| https://raw.githubusercontent.com/RedHatQE/OpenShift-LP-QE--Tools/refs/heads/main/libs/bash/ci-operator/interop/common/ExitTrap--PostProcessPrep.sh | |
| )"; trap ' | |
| if [ "${MAP_TESTS:-}" = "true" ]; then | |
| typeset -a _fURL=() | |
| type -t wget 1>/dev/null && _fURL=(wget -qO-) || _fURL=(curl -fsSL) | |
| helper_script="$("${_fURL[@]}" \ | |
| https://raw.githubusercontent.com/RedHatQE/OpenShift-LP-QE--Tools/refs/heads/main/libs/bash/ci-operator/interop/common/ExitTrap--PostProcessPrep.sh | |
| )" || { | |
| echo "Failed to download ExitTrap--PostProcessPrep.sh" >&2 | |
| exit 1 | |
| } | |
| eval "${helper_script}" | |
| trap ' | |
| LP_IO__ET_PPP__NEW_TS_NAME="${DR__RP__CR_COMP_NAME}--%s" \ | |
| ExitTrap--PostProcessPrep junit--redhat-chaos-container-scenarios-etcd-hangup.xml | |
| ' EXIT |
🤖 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/container-scenarios/etcd-hangup/redhat-chaos-container-scenarios-etcd-hangup-commands.sh`
around lines 8 - 13, The issue is that the wget/curl download is executed inside
the eval command, so if the network request fails silently, eval still succeeds
with an empty string and the EXIT trap remains undefined, causing the script to
fail later. Separate the download operation from the eval by first capturing the
helper script content to a variable, then check if the capture was successful
(verify the variable is not empty), and only then eval the captured content.
Apply this same pattern to all similar download blocks throughout the file,
including all MAP_TESTS blocks for redhat-chaos time-scenarios,
pod-scenarios/etcd-disruption, stackrox qa-e2e, compliance-e2e,
openshift-pipelines tests and install, and interop-tests cnv-tests-e2e-deploy
and ocs-tests.
|
/pj-rehearse periodic-ci-redhat-chaos-lp-chaos-main-ocp-4.22-lpGA-lp-chaos-cr--ocp--krkn-hub-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 85 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: |
|
@oharan2: 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. |
1 participant
Summary
This PR has two parts:
1. New OCP 4.22 chaos CI config
Adds
redhat-chaos-lp-chaos-main__ocp-4.22-lpGA-lp-chaos.yaml, a Component Readiness (CR) compliant periodic CI configuration for OCP 4.22. The tests useMAP_TESTS: "true"andDR__RP__CR_COMP_NAMEto enable JUnit testsuite name mapping for CR reporting viampiit-data-router-reporter.2. MPICT mapping mechanism on Chaos step-registry steps
Adds the three environment variables required for the MPICT (MPIIT Platform Interop Chaos Testing) mapping mechanism to the following step refs:
redhat-chaos-pod-scenarios-etcd-disruptionredhat-chaos-pod-scenarios-kube-apiserver-disruptionredhat-chaos-pod-scenarios-ovn-disruptionredhat-chaos-pod-scenarios-ovn-cp-disruptionredhat-chaos-pod-scenarios-prometheus-disruptionredhat-chaos-pod-scenarios-random-system-podsredhat-chaos-container-scenarios-etcd-hangupredhat-chaos-time-scenariosThe added variables are:
MAP_TESTS"true"to enable JUnit XML generation and testsuite name mapping for CR reporting viaExitTrap--PostProcessPrep. Defaults to"false"to preserve existing behavior.DR__RP__CR_COMP_NAMElp-chaos--OCP). Used as the JUnit testsuite name prefix and as the Report Portal launch name.DR__RP__JUNIT_FILE_NAMEjunit--redhat-chaos-pod-scenarios-etcd-disruption.xml).Note on
DR__RP__JUNIT_FILE_NAMEand symlinked commands scripts:Several pod-scenario steps (e.g.
ovn-cp-disruption,prometheus-disruption,random-system-pods) share a common-commands.shimplementation via symbolic links rather than maintaining separate copies. WhenMAP_TESTS=true, the shared script invokesExitTrap--PostProcessPrep ${DR__RP__JUNIT_FILE_NAME}on exit. Without a step-specific filename, all steps sharing the same script would attempt to write to an identical JUnit filename, causing collisions. By definingDR__RP__JUNIT_FILE_NAMEas an env var in each step's ref.yaml with a unique, step-derived default, each step produces a distinct JUnit XML file regardless of the shared underlying script.Summary by CodeRabbit
This PR applies a standardized mapping mechanism to OpenShift chaos and disruption test steps in the LP-chaos CI infrastructure, enabling Component Readiness (CR) reporting and JUnit XML test-to-component name mapping.
What Changed
The PR modifies chaos test step definitions and their execution scripts to support a new
MAP_TESTSfeature that conditionally enables test result mapping. When enabled, this mechanism:ExitTrap--PostProcessPrep.shhelper script during step executionDR__RP__CR_COMP_NAME) for integration with Component Readiness reporting and Report PortalAffected Chaos Test Steps
The following step definitions are updated with the new mapping configuration:
Each affected step now includes new environment variables:
MAP_TESTS(default:"false") – feature flag for enabling JUnit XML mappingDR__RP__CR_COMP_NAME(default:"") – component readiness name prefixDR__RP__JUNIT_FILE_NAME– JUnit XML output filenamegrace_period: 10m– step execution timeoutConfiguration Updates
The main
redhat-chaos-lp-chaos-main__ocp-4.22-lpGA-lp-chaos.yamljob manifest is restructured to:postexecution section withmpiit-data-router-reporterreference for automated result reportingNODE_OUTAGE_TIMEOUT,POWER_OUTAGE_TIMEOUT)ocp-4.22-lpGA-lp-chaosImpact
These changes establish infrastructure for consistently mapping chaos test results to Component Readiness metrics and Report Portal integrations, improving test result attribution and tracking across OpenShift's chaos engineering test suite.