USHIFT-7221: Automate Polarion test: Verify sosreport can be consumed by omg & omc support tools#6991
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@agullon: This pull request references USHIFT-7221 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughExtracts SOS report keywords into a shared Robot Framework resource, refactors an existing suite to reuse them, adds a new suite testing omc/omg consumption of SOS reports, adds omc/omg installers to fetch_tools.sh, and updates scenario scripts to fetch these tools and run the new suite. ChangesSOS Report Tooling and Testing
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Suite as sos-report-support-tools.robot
participant Host as MicroShift Host
participant Local as Local Test Runner
participant Omc as omc
participant Omg as omg
Suite->>Host: Create Sos Report
Host-->>Suite: sosreport tarball path
Suite->>Host: Extract Sos Report
Host-->>Suite: extracted sosreport dir
Suite->>Local: Download Must Gather Data
Local-->>Suite: LOCAL_SOS_DIR / MUST_GATHER_PATH
Suite->>Omc: use MUST_GATHER_PATH, get pods/nodes, logs
Suite->>Omg: use MUST_GATHER_PATH, get pods/nodes, logs, projects
Suite->>Local: Teardown removes downloaded SOS dir
Related Issues: None found in provided context. Related PRs: None found in provided context. Suggested labels: test, robot-framework Suggested reviewers: None determined from provided context. Poem: A rabbit hopped through sos-report land, 🐇 Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/suites/otp-workloads/sos-report-support-tools.robot (2)
86-99: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin the
omcrelease version and verify checksum.Downloading
latestfrom GitHub means the suite can silently start testing against a new/breakingomcrelease, and there's no integrity check on the binary beforechmod +x/execution.🔒 Pin version and verify checksum
+${OMC_VERSION} v0.x.y # pin to a known-good release ... - Command Should Work - ... curl -sSfL --connect-timeout 30 --max-time 120 -o /tmp/omc.tar.gz https://github.com/gmeghnag/omc/releases/latest/download/omc_Linux_aarch64.tar.gz + Command Should Work + ... curl -sSfL --connect-timeout 30 --max-time 120 -o /tmp/omc.tar.gz https://github.com/gmeghnag/omc/releases/download/${OMC_VERSION}/omc_Linux_aarch64.tar.gz🤖 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 `@test/suites/otp-workloads/sos-report-support-tools.robot` around lines 86 - 99, Update the Install Omc keyword to stop fetching the mutable GitHub latest asset and instead download a pinned omc release version for each supported architecture; use the existing Install Omc flow and OMC_BINARY path to keep the change localized. Add a checksum verification step after downloading the archive/binary and before chmod +x or execution, so the suite validates the artifact integrity for both the aarch64 tarball path and the x86_64 binary path. Ensure the version and checksum values are explicit and easy to update in one place.
106-110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin
o-must-gatherversion for reproducibility.Installing the latest pip release risks unexpected breakage/flakiness if a new upstream release changes CLI behavior.
♻️ Pin package version
- Command Should Work ${OMG_VENV}/bin/pip install o-must-gather + Command Should Work ${OMG_VENV}/bin/pip install o-must-gather==<pinned-version>🤖 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 `@test/suites/otp-workloads/sos-report-support-tools.robot` around lines 106 - 110, The Install Omg keyword currently installs the unpinned latest o-must-gather release, which can make the sos-report support tools test flaky. Update the Command Should Work call in Install Omg to install a specific o-must-gather version instead of the floating package name, so the behavior stays reproducible across runs. Keep the change localized to the Install Omg keyword and preserve the existing venv setup and OMG_BINARY help check.
🤖 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.
Nitpick comments:
In `@test/suites/otp-workloads/sos-report-support-tools.robot`:
- Around line 86-99: Update the Install Omc keyword to stop fetching the mutable
GitHub latest asset and instead download a pinned omc release version for each
supported architecture; use the existing Install Omc flow and OMC_BINARY path to
keep the change localized. Add a checksum verification step after downloading
the archive/binary and before chmod +x or execution, so the suite validates the
artifact integrity for both the aarch64 tarball path and the x86_64 binary path.
Ensure the version and checksum values are explicit and easy to update in one
place.
- Around line 106-110: The Install Omg keyword currently installs the unpinned
latest o-must-gather release, which can make the sos-report support tools test
flaky. Update the Command Should Work call in Install Omg to install a specific
o-must-gather version instead of the floating package name, so the behavior
stays reproducible across runs. Keep the change localized to the Install Omg
keyword and preserve the existing venv setup and OMG_BINARY help check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: d7bfe747-df73-4617-ac1e-b8e8420f2d45
📒 Files selected for processing (6)
test/resources/sos-report.resourcetest/scenarios-bootc/el10/releases/el102-lrel@otp-workloads.shtest/scenarios-bootc/el9/releases/el98-lrel@otp-workloads.shtest/scenarios/releases/el98-lrel@otp-workloads.shtest/suites/otp-workloads/sos-report-plugins.robottest/suites/otp-workloads/sos-report-support-tools.robot
agullon
changed the title
|
/label tide/merge-method-squash |
openshift-ci
Bot
added
the
tide/merge-method-squash
coderabbitai
Bot
added
the
ready-for-human-review
|
/retest |
1 similar comment
|
/retest |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/suites/otp-workloads/sos-report-support-tools.robot (1)
102-125: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDerive pod and container from the same lookup.
openshift-dnshas multiple workloads (dns-default,node-resolver), so picking the “first” pod and container in separate calls can pair mismatched names and breaklogs.🤖 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 `@test/suites/otp-workloads/sos-report-support-tools.robot` around lines 102 - 125, The log retrieval in Verify Tool Can Retrieve Logs is deriving pod and container separately, which can mismatch when a namespace has multiple workloads. Update Get Pod Name From Tool and Get Container Name From Tool (or replace them with a single helper) so both values come from the same pod lookup, and have Verify Tool Can Retrieve Logs use that paired result when building the logs command.
🧹 Nitpick comments (2)
test/suites/otp-workloads/sos-report-support-tools.robot (1)
18-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOMG_VERSION duplicated across files.
${OMG_VERSION}(1.2.6) is hardcoded here and independently hardcoded again inscripts/fetch_tools.sh'sgettool_omg. These two pins can silently drift since nothing keeps them in sync.🤖 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 `@test/suites/otp-workloads/sos-report-support-tools.robot` around lines 18 - 26, The OMG version pin is duplicated in this Robot suite variable block and in scripts/fetch_tools.sh’s gettool_omg, which can drift out of sync. Make the version source single-sourced by reusing one shared constant or variable reference for OMG_VERSION in both places, and update the test suite to consume that shared value instead of hardcoding 1.2.6 here.scripts/fetch_tools.sh (1)
303-311: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the unused
gettool_omgfetcher or wire the suite to reuse it.test/suites/otp-workloads/sos-report-support-tools.robotinstallso-must-gatherinto a remote/tmp/omg-venv, so the local${DEST_DIR}/omg-venvcreated byscripts/fetch_tools.shhas no in-tree consumer. That also leaves the1.2.6pin duplicated beside${OMG_VERSION}.🤖 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 `@scripts/fetch_tools.sh` around lines 303 - 311, The gettool_omg fetcher is currently unused, so either remove it from fetch_tools.sh or update the suite to call it instead of creating its own remote /tmp/omg-venv. If you keep it, make the o-must-gather install in test/suites/otp-workloads/sos-report-support-tools.robot reuse the existing gettool_omg logic and align the version source with ${OMG_VERSION} rather than duplicating the 1.2.6 pin. Use the gettool_omg function and OMG_VERSION symbol as the main points of reference when wiring this up.
🤖 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.
Outside diff comments:
In `@test/suites/otp-workloads/sos-report-support-tools.robot`:
- Around line 102-125: The log retrieval in Verify Tool Can Retrieve Logs is
deriving pod and container separately, which can mismatch when a namespace has
multiple workloads. Update Get Pod Name From Tool and Get Container Name From
Tool (or replace them with a single helper) so both values come from the same
pod lookup, and have Verify Tool Can Retrieve Logs use that paired result when
building the logs command.
---
Nitpick comments:
In `@scripts/fetch_tools.sh`:
- Around line 303-311: The gettool_omg fetcher is currently unused, so either
remove it from fetch_tools.sh or update the suite to call it instead of creating
its own remote /tmp/omg-venv. If you keep it, make the o-must-gather install in
test/suites/otp-workloads/sos-report-support-tools.robot reuse the existing
gettool_omg logic and align the version source with ${OMG_VERSION} rather than
duplicating the 1.2.6 pin. Use the gettool_omg function and OMG_VERSION symbol
as the main points of reference when wiring this up.
In `@test/suites/otp-workloads/sos-report-support-tools.robot`:
- Around line 18-26: The OMG version pin is duplicated in this Robot suite
variable block and in scripts/fetch_tools.sh’s gettool_omg, which can drift out
of sync. Make the version source single-sourced by reusing one shared constant
or variable reference for OMG_VERSION in both places, and update the test suite
to consume that shared value instead of hardcoding 1.2.6 here.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: a3909f35-0b90-43dc-83fd-822aa45b6b12
📒 Files selected for processing (2)
scripts/fetch_tools.shtest/suites/otp-workloads/sos-report-support-tools.robot
coderabbitai
Bot
removed
the
ready-for-human-review
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/suites/otp-workloads/sos-report-support-tools.robot (1)
93-99: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider a timeout on local process calls.
Local Command Should Workruns commands viaRun Processwith notimeout. Ifomc/omgever hangs (e.g., waiting on stdin or a stuck subprocess), the suite will block indefinitely rather than failing fast.♻️ Add a bounded timeout
Local Command Should Work [Documentation] Run a command locally and verify it succeeds. [Arguments] ${command} - ${result}= Process.Run Process bash -c ${command} + ${result}= Process.Run Process bash -c ${command} timeout=60s on_timeout=terminate Should Be Equal As Integers ${result.rc} 0 ... msg=Local command failed (rc=${result.rc}): ${command}\nstderr: ${result.stderr} RETURN ${result.stdout}🤖 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 `@test/suites/otp-workloads/sos-report-support-tools.robot` around lines 93 - 99, The Local Command Should Work keyword currently runs Process.Run Process without any timeout, so it can hang indefinitely if a command like omc or omg blocks. Update this keyword to pass a bounded timeout to Run Process, and keep the existing rc/stdout/stderr assertions so failures still report useful context. Use the Local Command Should Work and Process.Run Process symbols to locate and adjust the call.
🤖 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.
Nitpick comments:
In `@test/suites/otp-workloads/sos-report-support-tools.robot`:
- Around line 93-99: The Local Command Should Work keyword currently runs
Process.Run Process without any timeout, so it can hang indefinitely if a
command like omc or omg blocks. Update this keyword to pass a bounded timeout to
Run Process, and keep the existing rc/stdout/stderr assertions so failures still
report useful context. Use the Local Command Should Work and Process.Run Process
symbols to locate and adjust the call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: fd4360e1-2334-4b52-9af6-d1c0486d3411
📒 Files selected for processing (7)
scripts/fetch_tools.shtest/requirements.txttest/run.shtest/scenarios-bootc/el10/releases/el102-lrel@otp-workloads.shtest/scenarios-bootc/el9/releases/el98-lrel@otp-workloads.shtest/scenarios/releases/el98-lrel@otp-workloads.shtest/suites/otp-workloads/sos-report-support-tools.robot
💤 Files with no reviewable changes (1)
- scripts/fetch_tools.sh
✅ Files skipped from review due to trivial changes (1)
- test/run.sh
Add Robot Framework test (OCP-61971) that verifies MicroShift sosreport output can be consumed by the omc and omg support tools. Both tools are pre-installed locally via fetch_tools.sh with pinned versions and checksum verification (omc v3.15.0, omg v1.2.6), then run against a sosreport downloaded from the remote host. The test generates a sosreport, extracts it on the remote host, downloads the sos_commands/microshift directory locally via SCP, and validates that each tool can list pods, nodes, projects, and retrieve container logs. Also extract shared SOS report keywords (Create Sos Report, Extract Sos Report, Cleanup Sos Report Directory) into a new sos-report.resource to eliminate duplication between test suites. USHIFT-7221 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> pre-commit.check-secrets: ENABLED
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
/retest |
1 similar comment
|
/retest |
|
/override ci/prow/e2e-aws-tests-bootc-release-arm-el10 |
|
@agullon: Overrode contexts on behalf of agullon: ci/prow/e2e-aws-tests-bootc-release-arm-el10, ci/prow/e2e-aws-tests-bootc-release-el10, ci/prow/e2e-aws-tests-bootc-release-el9 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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: agullon, kasturinarra 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 |
|
/verified by CI |
openshift-ci-robot
added
the
verified
|
@agullon: This PR has been marked as verified by 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/override ci/prow/e2e-aws-tests-release because failures not related to the changes |
|
@agullon: Overrode contexts on behalf of agullon: ci/prow/e2e-aws-tests-bootc-release-arm-el10, ci/prow/e2e-aws-tests-bootc-release-el10, ci/prow/e2e-aws-tests-bootc-release-el9, ci/prow/e2e-aws-tests-release, ci/prow/e2e-aws-tests-release-arm 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. |
|
@agullon: all tests passed! 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. |
4 participants
Summary
Automate Polarion test case OCP-61971: verify that MicroShift sosreport
output is compatible with the omc and
omg support tools.
What changed
New test:
test/suites/otp-workloads/sos-report-support-tools.robot— generates a sosreport on the MicroShift host, downloads the
sos_commands/microshiftdirectory locally via SCP, and runs both omcand omg against it to verify they can list pods, nodes, projects, and
retrieve container logs.
New shared resource:
test/resources/sos-report.resource— extractsCreate Sos Report,Extract Sos Report, andCleanup Sos Report Directorykeywords fromsos-report-plugins.robotso both suites canreuse them.
Tool management via
fetch_tools.sh: addsgettool_omc(pinnedv3.15.0 with SHA256 checksum) and
gettool_omg(pinned v1.2.6 in adedicated Python venv). Both tools run on the test runner host, not on
the MicroShift cluster.
Scenario registration: the new test is added to all three
otp-workloads scenarios (el98 ostree, el98 bootc, el102 bootc), each
calling
fetch_tools.sh omc omgbeforerun_tests.Design decisions
being installed on the cluster. This avoids modifying the remote host
and sidesteps read-only filesystem issues on bootc/ostree deployments.
sos_commands/microshiftis downloaded (not the full sosreport)to minimize SCP transfer time.
_installhelper is not used for omc on aarch64 because its--wildcards "*/omc"pattern does not match root-level entries inGoReleaser tarballs. The aarch64 path does manual download, checksum
verification, and extraction instead.
Test plan
robocop checkandrobocop format --checkpassshellcheckpasses on all modified shell scripts