Skip to content

Add JUnit report and migrate to external CI image#79900

Open
krisnababu wants to merge 2 commits into
openshift:mainfrom
krisnababu:krisnababu-cnv-junit-report
Open

Add JUnit report and migrate to external CI image#79900
krisnababu wants to merge 2 commits into
openshift:mainfrom
krisnababu:krisnababu-cnv-junit-report

Conversation

@krisnababu
Copy link
Copy Markdown
Contributor

@krisnababu krisnababu commented May 31, 2026

What this PR does

  • Builds cli-plus-tools image from an external Dockerfile in redhat-chaos/lp-chaos
    and promotes it to the ci/lp-chaos-images namespace
  • Adds JUnit XML report generation to the redhat-lp-chaos-lp-cnv-vm-snapshot step
    using Python stdlib (xml.etree.ElementTree) instead of shell HEREDOC, ensuring
    proper XML construction and attribute escaping
  • Derives step_id from BASH_SOURCE[0] so the report filename and test case name
    correctly reflect the step name
  • Makes testsuite and testcase names configurable via LPC_LP_CNV__JUNIT__SUITE_NAME
    and LPC_LP_CNV__JUNIT__CASE_NAME env vars, following the team naming convention
  • Adds grace_period: 60s to allow the JUnit report to be written before pod termination

Notes for reviewers

  • Based on initial work by @Userweiwei, PR [WIP]Add JUnit reporting and migrate to CI image build #76783
  • Python3 is required for the JUnit generation script. The quay.io/openshift/ci:ocp_4.20_cli
    base image access is restricted locally so this could not be verified pre-PR.
    The rehearsal run will confirm availability. If Python3 is absent, a follow-up PR
    to redhat-chaos/lp-chaos adding python3 to Dockerfile.cli-plus-tools will be raised.

Summary by CodeRabbit

This PR updates the OpenShift CI configuration for the redhat-chaos/lp-chaos repository to improve test reporting and externalize container image definitions:

Container Image Changes:

  • Migrates the cli-plus-tools image build from an inline Dockerfile definition to an external Dockerfile located at image/container/ocp-test/Dockerfile.cli-plus-tools. The image continues to be promoted to the ci/lp-chaos-images namespace.

JUnit Report Generation:

  • Adds automatic JUnit XML report generation to the redhat-lp-chaos-lp-cnv-vm-snapshot step using Python's standard xml.etree.ElementTree library for proper XML construction and attribute escaping.
  • The step exit handler generates reports to ${ARTIFACT_DIR}/junit_${step_id}.xml with step name derived from the script filename.
  • Captures test duration and conditionally includes failure details when the step exits with a non-zero code.

Step Configuration:

  • Introduces configurable test suite and case names via LPC_LP_CNV__JUNIT__SUITE_NAME and LPC_LP_CNV__JUNIT__CASE_NAME environment variables to align with team naming conventions.
  • Adds grace_period: 60s to allow the JUnit report to be written before pod termination.

These changes enhance the chaos testing workflow by producing CI-parseable test reports while modernizing how the build image is managed.

…e to external CI image

- Build cli-plus-tools image from external Dockerfile in redhat-chaos/lp-chaos
  and promote it to ci/lp-chaos-images namespace
- Add JUnit XML report generation using Python stdlib (xml.etree.ElementTree)
  replacing HEREDOC approach for proper XML construction and escaping
- Derive step_id from BASH_SOURCE[0] to correctly identify the step in
  the report filename and test case name
- Make testsuite and testcase names configurable via LPC_LP_CNV__JUNIT__SUITE_NAME
  and LPC_LP_CNV__JUNIT__CASE_NAME env vars following team naming convention
- Add grace_period: 60s to allow report generation before pod termination

Co-authored-by: Wei Liu <weiliu@redhat.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 31, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: dc6e1e30-2d7c-4d62-9d3a-a26a9dad0964

📥 Commits

Reviewing files that changed from the base of the PR and between a5191fd and 1e76a20.

⛔ Files ignored due to path filters (3)
  • ci-operator/jobs/redhat-chaos/lp-chaos/redhat-chaos-lp-chaos-main-periodics.yaml is excluded by !ci-operator/jobs/**
  • ci-operator/jobs/redhat-chaos/lp-chaos/redhat-chaos-lp-chaos-main-postsubmits.yaml is excluded by !ci-operator/jobs/**
  • ci-operator/jobs/redhat-chaos/lp-chaos/redhat-chaos-lp-chaos-main-presubmits.yaml is excluded by !ci-operator/jobs/**
📒 Files selected for processing (1)
  • ci-operator/step-registry/redhat-lp-chaos/lp/cnv/vm/snapshot/redhat-lp-chaos-lp-cnv-vm-snapshot-commands.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • ci-operator/step-registry/redhat-lp-chaos/lp/cnv/vm/snapshot/redhat-lp-chaos-lp-cnv-vm-snapshot-commands.sh

Walkthrough

Refactors the cli-plus-tools CI image to use an external Dockerfile, adds grace_period and JUnit env vars to the LP-CNV VM snapshot step reference, and installs an exit-trap that writes a JUnit XML report on script exit (writes to ${ARTIFACT_DIR}).

Changes

LP-CNV VM Snapshot Step Updates

Layer / File(s) Summary
CLI tools image build refactoring
ci-operator/config/redhat-chaos/lp-chaos/redhat-chaos-lp-chaos-main__ocp4.21-nightly--cnv-4.21-stable-cnvcases--aws.yaml
The cli-plus-tools base_images build switches from an inline dockerfile_literal to an external Dockerfile reference (image/container/ocp-test/Dockerfile.cli-plus-tools) with the same from/to mapping and unchanged promotion target.
Step configuration for JUnit reporting
ci-operator/step-registry/redhat-lp-chaos/lp/cnv/vm/snapshot/redhat-lp-chaos-lp-cnv-vm-snapshot-ref.yaml
Adds grace_period: 60s and two environment variables: LPC_LP_CNV__JUNIT__SUITE_NAME (default lp-cnv-chaos-suite) and LPC_LP_CNV__JUNIT__CASE_NAME (default empty; falls back to step name when unset).
JUnit XML report generation on exit
ci-operator/step-registry/redhat-lp-chaos/lp/cnv/vm/snapshot/redhat-lp-chaos-lp-cnv-vm-snapshot-commands.sh
Adds an EXIT trap (finalize_junit) that computes duration, captures exit code, generates a JUnit XML (with optional <failure> on non-zero exit), and writes ${ARTIFACT_DIR}/junit_${step_id}.xml. Appends a trailing true after the VM snapshot loop.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

lgtm, approved, rehearsals-ack

Suggested reviewers

  • etirta
  • ebenahar
🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (14 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the two main changes: adding JUnit report generation and migrating the CLI image build to an external Dockerfile.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed PR contains no Ginkgo test files. Changes are limited to CI/operator config YAML files and a bash script for VM snapshots; custom check for stable Ginkgo test names is not applicable.
Test Structure And Quality ✅ Passed This PR contains no Ginkgo test code. It modifies only CI configuration YAML files and shell scripts, not Go test files that would need Ginkgo quality review.
Microshift Test Compatibility ✅ Passed PR contains no new Ginkgo e2e tests. Changes are CI infrastructure: image build config, step automation script, and step registry YAML only.
Single Node Openshift (Sno) Test Compatibility ✅ Passed PR does not add new Ginkgo e2e tests; all changes are CI/operator infrastructure files (YAML configs and shell scripts), not test code.
Topology-Aware Scheduling Compatibility ✅ Passed PR contains only CI/operator infrastructure config and automation scripts; no deployment manifests or operator code with scheduling constraints that would affect OpenShift topology compatibility.
Ote Binary Stdout Contract ✅ Passed PR contains only CI configuration YAML and shell scripts, not OTE test binaries. The check is not applicable.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No Ginkgo e2e tests are added. Changes are CI infrastructure only: YAML configs and a shell script for JUnit report generation. Check applies only to new tests.
No-Weak-Crypto ✅ Passed No weak cryptography patterns (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB), custom crypto implementations, or non-constant-time secret comparisons found in the modified files.
Container-Privileges ✅ Passed PR contains only CI operator config and bash scripts, not K8s manifests. No privileged container configurations found.
No-Sensitive-Data-In-Logs ✅ Passed No sensitive data (passwords, API keys, tokens, PII, session IDs, or customer data) is logged; variables logged via xtrace contain only test infrastructure identifiers.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci openshift-ci Bot requested review from chaitanyaenr and etirta May 31, 2026 18:54
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 31, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: krisnababu
Once this PR has been reviewed and has the lgtm label, please assign etirta for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-lp-chaos/lp/cnv/vm/snapshot/redhat-lp-chaos-lp-cnv-vm-snapshot-commands.sh`:
- Around line 12-41: The heredoc currently interpolates shell variables into the
embedded Python (e.g. step_id, duration, exit_code, out_file, suite_name,
case_name) which can break/enable injection; stop shell expansion by quoting the
delimiter (use 'PYEOF') and have the Python code read values from os.environ
(os.environ.get("LPC_LP_CNV__JUNIT__SUITE_NAME", ...)) or sys.argv instead of
relying on ${...} inside the script; update the Python variables suite_name and
case_name to use os.environ.get(..., default) and parse
step_id/duration/exit_code/out_file from env/argv so the shell no longer injects
raw strings into the source.
🪄 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: c0f76890-0d26-4329-a34f-743f43e5c160

📥 Commits

Reviewing files that changed from the base of the PR and between 9626cc0 and a5191fd.

⛔ Files ignored due to path filters (1)
  • ci-operator/jobs/redhat-chaos/lp-chaos/redhat-chaos-lp-chaos-main-postsubmits.yaml is excluded by !ci-operator/jobs/**
📒 Files selected for processing (3)
  • ci-operator/config/redhat-chaos/lp-chaos/redhat-chaos-lp-chaos-main__ocp4.21-nightly--cnv-4.21-stable-cnvcases--aws.yaml
  • ci-operator/step-registry/redhat-lp-chaos/lp/cnv/vm/snapshot/redhat-lp-chaos-lp-cnv-vm-snapshot-commands.sh
  • ci-operator/step-registry/redhat-lp-chaos/lp/cnv/vm/snapshot/redhat-lp-chaos-lp-cnv-vm-snapshot-ref.yaml

@openshift-merge-bot
Copy link
Copy Markdown
Contributor

[REHEARSALNOTIFIER]
@krisnababu: the pj-rehearse plugin accommodates running rehearsal tests for the changes in this PR. Expand 'Interacting with pj-rehearse' for usage details. The following rehearsable tests have been affected by this change:

Test name Repo Type Reason
pull-ci-redhat-chaos-lp-chaos-main-ocp4.21-nightly--cnv-4.21-stable-cnvcases--aws-images redhat-chaos/lp-chaos presubmit Ci-operator config changed
periodic-ci-redhat-chaos-lp-chaos-main-ocp4.21-nightly--cnv-4.21-stable-cnvcases--aws-lp-chaos--cnv--node-outage N/A periodic Ci-operator config changed
Interacting with pj-rehearse

Comment: /pj-rehearse to run up to 5 rehearsals
Comment: /pj-rehearse skip to opt-out of rehearsals
Comment: /pj-rehearse {test-name}, with each test separated by a space, to run one or more specific rehearsals
Comment: /pj-rehearse more to run up to 10 rehearsals
Comment: /pj-rehearse max to run up to 25 rehearsals
Comment: /pj-rehearse auto-ack to run up to 5 rehearsals, and add the rehearsals-ack label on success
Comment: /pj-rehearse list to get an up-to-date list of affected jobs
Comment: /pj-rehearse abort to abort all active rehearsals
Comment: /pj-rehearse network-access-allowed to allow rehearsals of tests that have the restrict_network_access field set to false. This must be executed by an openshift org member who is not the PR author

Once you are satisfied with the results of the rehearsals, comment: /pj-rehearse ack to unblock merge. When the rehearsals-ack label is present on your PR, merge will no longer be blocked by rehearsals.
If you would like the rehearsals-ack label removed, comment: /pj-rehearse reject to re-block merging.

@krisnababu
Copy link
Copy Markdown
Contributor Author

/pj-rehearse periodic-ci-redhat-chaos-lp-chaos-main-ocp4.21-nightly--cnv-4.21-stable-cnvcases--aws-lp-chaos--cnv--node-outage

@openshift-merge-bot
Copy link
Copy Markdown
Contributor

@krisnababu: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 31, 2026

@krisnababu: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/rehearse/periodic-ci-redhat-chaos-lp-chaos-main-ocp4.21-nightly--cnv-4.21-stable-cnvcases--aws-lp-chaos--cnv--node-outage 1e76a20 link unknown /pj-rehearse periodic-ci-redhat-chaos-lp-chaos-main-ocp4.21-nightly--cnv-4.21-stable-cnvcases--aws-lp-chaos--cnv--node-outage

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@krisnababu
Copy link
Copy Markdown
Contributor Author

/pj-rehearse periodic-ci-redhat-chaos-lp-chaos-main-ocp4.21-nightly--cnv-4.21-stable-cnvcases--aws-lp-chaos--cnv--node-outage

@openshift-merge-bot
Copy link
Copy Markdown
Contributor

@krisnababu: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants