Skip to content

USHIFT-6744: Migrate workload gingko tests from OTP#6631

Open
kasturinarra wants to merge 1 commit into
openshift:mainfrom
kasturinarra:port_wldsc_rf
Open

USHIFT-6744: Migrate workload gingko tests from OTP#6631
kasturinarra wants to merge 1 commit into
openshift:mainfrom
kasturinarra:port_wldsc_rf

Conversation

@kasturinarra
Copy link
Copy Markdown
Contributor

@kasturinarra kasturinarra commented May 6, 2026

Summary by CodeRabbit

  • Tests
    • Introduced new test scenarios for OTP workloads across multiple system versions and release types
    • Added validation test suites for OpenShift CLI operations, controller manager configuration, system diagnostics collection, and persistent storage functionality
    • Extended existing test scenarios with additional workload coverage to improve testing robustness

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds OTP workload test scenarios and suite validations across RHEL 9.8 and 10.2 platforms in both periodic and release pipelines. New scenario scripts define VMs, environment setup, and test execution for OTP workload tests. Existing scenario files are extended to include OTP test suites alongside their primary test jobs. Four new Robot Framework test suites validate KCM flags, OC CLI behavior, SOS report plugins, and StatefulSet with PVCs.

Changes

OTP Workloads Test Scenarios

Layer / File(s) Summary
Scenario definition for EL9/EL10 OTP workloads
test/scenarios/releases/el98-lrel@otp-workloads.sh, test/scenarios-bootc/el9/releases/el98-lrel@otp-workloads.sh, test/scenarios-bootc/el10/releases/el102-lrel@otp-workloads.sh
Three new scenario scripts define start_image references and implement scenario_create_vms, scenario_remove_vms, and scenario_run_tests hooks to prepare kickstart configs, launch single-VM environments, and execute OTP workload test suites (sos-report-plugins.robot and kcm-flags.robot).
Existing scenario integration with OTP test suites
test/scenarios/releases/el98-lrel@osconfig.sh, test/scenarios/periodics/el98-src@osconfig-lifecycle.sh, test/scenarios-bootc/el9/releases/el98-lrel@osconfig-router.sh, test/scenarios-bootc/el9/periodics/el98-src@osconfig-lifecycle.sh, test/scenarios-bootc/el10/releases/el102-lrel@osconfig-router.sh, test/scenarios-bootc/el10/periodics/el102-src@osconfig-lifecycle.sh
Six existing scenario run_tests functions extend their run_tests host1 invocations to include suites/otp-workloads/oc-cli.robot and suites/otp-workloads/statefulset-pvc.robot alongside their primary test suites.

OTP Workloads Test Suites

Layer / File(s) Summary
KCM flag validation suite
test/suites/otp-workloads/kcm-flags.robot
New test case verifies kube-controller-manager starts with OpenShift flavor flags by injecting a drop-in config to enable verbose logging, restarting MicroShift, and asserting expected --controllers flags and --openshift-config="" in journal output.
OC CLI behavior validation suite
test/suites/otp-workloads/oc-cli.robot
New test suite with ten test cases validates: oc version clean git state; --idms-file present and --icsp-file absent in help for oc image extract and oc adm release info; mutual exclusion of IDMS and ICSP flags; oc get events timestamp formatting; oc set data --dry-run=server non-persistence; oc debug startupProbe removal; and oc explain coverage for all API resources.
SOS report plugin validation suite
test/suites/otp-workloads/sos-report-plugins.robot
New test suite validates SOS plugin enablement, report content (oc adm inspect output and /etc/microshift equivalence), behavior after MicroShift stop, and greenboot artifacts under system profile. Includes helper keywords for report creation, extraction, and plugin verification.
StatefulSet PVC label validation suite
test/suites/otp-workloads/statefulset-pvc.robot
New test case deploys a StatefulSet from YAML asset in a unique namespace, waits for pod readiness, and verifies custom labels on PVCs from volumeClaimTemplates. Suite setup checks for default StorageClass availability; teardown removes resources.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 11 | ❌ 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 (11 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: migrating workload Ginkgo tests from OTP to Robot Framework test suites, which is the core objective of the PR.
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 All 16 Robot test cases have stable, deterministic names with no dynamic content. Test names are descriptive strings without variables, UUIDs, timestamps, pod/node names, or generated suffixes.
Test Structure And Quality ✅ Passed PR contains Robot Framework tests and Bash scripts, not Ginkgo tests. Custom check targets Ginkgo code, making it not applicable to this PR.
Microshift Test Compatibility ✅ Passed No Ginkgo tests were added. This PR adds Robot Framework test suites, not Ginkgo e2e tests, so the custom check does not apply.
Single Node Openshift (Sno) Test Compatibility ✅ Passed PR contains only Robot Framework test suites and shell scenario scripts. The SNO compatibility check applies only to Ginkgo e2e tests, which are not present in this PR.
Topology-Aware Scheduling Compatibility ✅ Passed PR contains only test files (Robot Framework suites and bash test orchestration scripts). No deployment manifests, operator code, or controllers are added/modified. No scheduling constraints found.
Ote Binary Stdout Contract ✅ Passed PR adds only Robot Framework tests and shell scripts. No OTE binaries, Go code, Ginkgo test code, or process-level code (main/init/TestMain/suite setup) present. Custom check not applicable.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed Custom check does not apply. PR contains only Robot Framework tests and Bash scripts, not Ginkgo Go tests which the check specifically targets.

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

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

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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

@openshift-ci openshift-ci Bot requested review from ggiguash and pmtk May 6, 2026 17:02
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 6, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kasturinarra

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

The pull request process is described 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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 6, 2026
@kasturinarra kasturinarra changed the title Add sosreport and kcm related tests from OTP USHIFT-6744: Add sosreport and kcm related tests from OTP May 6, 2026
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 6, 2026
@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented May 6, 2026

@kasturinarra: This pull request references USHIFT-6744 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.

Details

In 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.

@kasturinarra
Copy link
Copy Markdown
Contributor Author

@CodeRabbit help

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 2026

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

CodeRabbit commands

These commands are invoked using PR/Issue comments.

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai rate limit to show your current review rate limit status.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai evaluate custom pre-merge check --instructions <custom-checks-instructions> --name <custom-checks-title> [--mode <error|warning>] to validate the custom pre-merge checks instructions. Defaults to error when --mode is omitted.
  • @coderabbitai ignore pre-merge checks to override pre-merge checks and get an approval on PR.
  • @coderabbitai run pre-merge checks to run pre-merge checks on the pull request.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve merge conflict to automatically resolve merge conflicts.
  • @coderabbitai autofix to automatically fix issues identified in unresolved review comments.
  • @coderabbitai resolve to resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai generate configuration to create a PR that adds the current resolved configuration as .coderabbit.yaml (or show it if already present).
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, support, documentation and community

  • Visit our status page to check the current availability of CodeRabbit.
  • Create a ticket on our support page for assistance with any issues or questions.
  • Visit our documentation site for detailed information on how to use CodeRabbit.
  • Join our Discord community to connect with other users and get help from the community.
  • Follow us on X/Twitter for updates and announcements.

@kasturinarra
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@kasturinarra
Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 2026

✅ Actions performed

Full review triggered.

@kasturinarra
Copy link
Copy Markdown
Contributor Author

/retest

@kasturinarra
Copy link
Copy Markdown
Contributor Author

/test e2e-aws-tests-release e2e-aws-tests-bootc-el9 e2e-aws-tests-release-arm e2e-aws-tests e2e-aws-tests-bootc-el10

@kasturinarra
Copy link
Copy Markdown
Contributor Author

/test e2e-aws-tests-release e2e-aws-tests-release-arm

@kasturinarra
Copy link
Copy Markdown
Contributor Author

@agullon could you please help review ?

suites/osconfig/clusterid.robot \
suites/osconfig/systemd-resolved.robot
suites/osconfig/systemd-resolved.robot \
suites/osconfig/sos-report-plugins.robot
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why you only add it in scenarios?
I think this should be add it in all the scenario where suites/osconfig/clusterid.robot is.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The reason i did not add is, in general we are running these gingko cases only in release scenarios right, so i thought adding there alone would be more appropriate than expanding it to other scenarios

@kasturinarra kasturinarra force-pushed the port_wldsc_rf branch 2 times, most recently from 290fee5 to 31663a5 Compare May 12, 2026 14:11
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: 6

🧹 Nitpick comments (2)
test/suites/standard2/dns-custom-config-validation.robot (1)

23-23: ⚡ Quick win

Prefer a TEST-NET IP for deterministic test isolation

Use RFC 5737 ranges (for example 198.51.100.97) instead of a globally routable address.

🤖 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/standard2/dns-custom-config-validation.robot` at line 23, The
test sets ${FAKE_LISTEN_IP} to a globally routable address (99.99.99.97); update
the variable to use an RFC 5737 TEST-NET address (e.g., 198.51.100.97) so tests
are deterministic and isolated—replace the value assigned to ${FAKE_LISTEN_IP}
in dns-custom-config-validation.robot accordingly.
test/suites/otp-workloads/kcm-flags.robot (1)

35-36: ⚡ Quick win

Scope journal queries to the current boot to reduce stale-log false positives.

Line 35 and Line 59 read the full unit journal. On long-lived hosts this can match old kube-controller-manager starts and make the assertion pass for the wrong run.

Suggested patch
-    ...    journalctl -u microshift --no-pager | grep kube-controller-manager | grep FLAG
+    ...    journalctl -u microshift -b --no-pager | grep kube-controller-manager | grep FLAG
...
-    ...    journalctl -u microshift --no-pager | grep kube-controller-manager | grep openshift-config
+    ...    journalctl -u microshift -b --no-pager | grep kube-controller-manager | grep openshift-config

Also applies to: 59-60

🤖 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/kcm-flags.robot` around lines 35 - 36, The
journalctl calls in the test (the command lines that currently use "journalctl
-u microshift --no-pager | grep kube-controller-manager | grep FLAG") are
reading the entire journal and can match entries from previous boots; update
those invocations (lines referencing the kube-controller-manager grep) to scope
to the current boot by adding the boot filter (e.g., use "journalctl -u
microshift -b --no-pager ..." or "--boot" / "-b 0") so the grep checks only logs
from the current boot; apply the same change to both occurrences (the commands
around the kube-controller-manager grep on the lines mentioned).
🤖 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 `@test/scenarios/releases/el98-lrel`@osconfig.sh:
- Line 21: The commit commented out the run-stage image guard by disabling the
call to exit_if_commit_not_found; restore that validation by re-enabling the
call to exit_if_commit_not_found "${start_image}" (remove the leading comment)
so the scenario exits early when the start image cannot be resolved; ensure no
alternate bypass logic was added and run the scenario tests to confirm the guard
triggers as expected.

In `@test/scenarios/releases/el98-lrel`@otp-workloads.sh:
- Line 21: The commented-out guard exit_if_commit_not_found "${start_image}"
should not be left as dead code; either restore it by uncommenting the call to
exit_if_commit_not_found with the "${start_image}" argument so the scenario
enforces the commit check, or remove the commented line entirely to keep the
script consistent with other scenario flows; locate the commented string
"exit_if_commit_not_found \"${start_image}\"" and perform one of those two
actions.

In `@test/suites/otp-workloads/sos-report-plugins.robot`:
- Around line 57-67: The current comparisons use raw `ls -l` output (variables
${system_manifests}/${report_manifests} and
${system_manifests_d}/${report_manifests_d}) which is brittle; replace the `ls
-l` commands with deterministic listings or checksums — e.g., run a normalized
command that lists filenames sorted (or computes checksums like md5sum) under
/etc/microshift/manifests and /etc/microshift/manifests.d in both system and
${sos_report_extracted}, store those into the same variables, and then use
Should Be Equal As Strings to compare the normalized/sorted filename lists or
checksum outputs so comparisons are stable across metadata formatting
differences.

In `@test/suites/otp-workloads/statefulset-pvc.robot`:
- Around line 73-75: Replace the format-dependent labels fetch with a direct
jsonpath query of the specific label key: instead of calling Run With Kubeconfig
to get {.metadata.labels} into ${labels} and using Should Contain, call oc get
pvc ${pvc_name} -n ${namespace} -o jsonpath='{.metadata.labels["${label_key}"]}'
(or {.metadata.labels.${label_key}}) into a variable like ${label_value_actual}
and then assert equality with Should Be Equal ${label_value_actual}
${label_value}; update references to ${labels}, Should Contain, and the jsonpath
string accordingly so the test compares the label value exactly.

In `@test/suites/standard2/dns-custom-config-validation.robot`:
- Around line 141-144: The journal cursor is captured too late which can miss
the corefile removal event; move the Get Journal Cursor call so it happens
before the step that deletes the Corefile (i.e., call Get Journal Cursor
immediately prior to the "Remove Custom Corefile" action), then keep the
subsequent "Pattern Should Appear In Log Output" that uses that cursor to assert
the "watched file.*was removed, keeping last known ConfigMap content" message.
- Around line 130-132: The teardown currently runs "Command Should Work    rm
-rf /etc/microshift/dns" which can remove unrelated production data; change the
teardown to a safe removal that only deletes test-created files or verifies the
directory is a test sandbox before removing it. Replace the direct rm -rf call
in the [Teardown] Run Keywords sequence (the "Command Should Work" invocation)
with a guarded approach: either remove only a known test subdirectory or
specific files, or first check a marker/ownership (e.g. test-created flag or
unique temp path) before calling removal, or use Robot keywords like "Directory
Should Exist" + "Remove Directory" on a test-scoped path; keep "Cleanup
Validation Test" as the final teardown step.

---

Nitpick comments:
In `@test/suites/otp-workloads/kcm-flags.robot`:
- Around line 35-36: The journalctl calls in the test (the command lines that
currently use "journalctl -u microshift --no-pager | grep
kube-controller-manager | grep FLAG") are reading the entire journal and can
match entries from previous boots; update those invocations (lines referencing
the kube-controller-manager grep) to scope to the current boot by adding the
boot filter (e.g., use "journalctl -u microshift -b --no-pager ..." or "--boot"
/ "-b 0") so the grep checks only logs from the current boot; apply the same
change to both occurrences (the commands around the kube-controller-manager grep
on the lines mentioned).

In `@test/suites/standard2/dns-custom-config-validation.robot`:
- Line 23: The test sets ${FAKE_LISTEN_IP} to a globally routable address
(99.99.99.97); update the variable to use an RFC 5737 TEST-NET address (e.g.,
198.51.100.97) so tests are deterministic and isolated—replace the value
assigned to ${FAKE_LISTEN_IP} in dns-custom-config-validation.robot accordingly.
🪄 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: 9e51e208-8ecc-4b72-8dd9-96794339df61

📥 Commits

Reviewing files that changed from the base of the PR and between aaba151 and 290fee5.

📒 Files selected for processing (15)
  • test/scenarios-bootc/el10/periodics/el102-src@osconfig-lifecycle.sh
  • test/scenarios-bootc/el10/releases/el102-lrel@osconfig-router.sh
  • test/scenarios-bootc/el10/releases/el102-lrel@otp-workloads.sh
  • test/scenarios-bootc/el9/periodics/el98-src@osconfig-lifecycle.sh
  • test/scenarios-bootc/el9/releases/el98-lrel@osconfig-router.sh
  • test/scenarios-bootc/el9/releases/el98-lrel@otp-workloads.sh
  • test/scenarios/periodics/el98-src@osconfig-lifecycle.sh
  • test/scenarios/releases/el98-lrel@osconfig.sh
  • test/scenarios/releases/el98-lrel@otp-workloads.sh
  • test/suites/otp-workloads/kcm-flags.robot
  • test/suites/otp-workloads/oc-cli.robot
  • test/suites/otp-workloads/sos-report-plugins.robot
  • test/suites/otp-workloads/statefulset-pvc.robot
  • test/suites/standard2/dns-custom-config-validation.robot
  • test_cases_USHIFT-6900.md
✅ Files skipped from review due to trivial changes (2)

Comment thread test/scenarios/releases/el98-lrel@osconfig.sh Outdated
Comment thread test/scenarios/releases/el98-lrel@otp-workloads.sh Outdated
Comment on lines +57 to +67
${system_manifests}= Command Should Work ls -l /etc/microshift/manifests
${report_manifests}= Command Should Work
... ls -l ${sos_report_extracted}/etc/microshift/manifests
Should Be Equal As Strings ${report_manifests} ${system_manifests}
... msg=Files inside /etc/microshift/manifests directory does not match between system and sosreport

${system_manifests_d}= Command Should Work ls -l /etc/microshift/manifests.d/
${report_manifests_d}= Command Should Work
... ls -l ${sos_report_extracted}/etc/microshift/manifests.d/
Should Be Equal As Strings ${report_manifests_d} ${system_manifests_d}
... msg=Files inside /etc/microshift/manifests.d directory does not match between system and sosreport
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use deterministic comparisons instead of raw ls -l output.

These assertions are brittle and can fail on metadata formatting differences. Compare normalized filenames (or checksums) to avoid flaky results.

Suggested fix
-    ${system_manifests}=    Command Should Work    ls -l /etc/microshift/manifests
+    ${system_manifests}=    Command Should Work
+    ...    find /etc/microshift/manifests -mindepth 1 -maxdepth 1 -printf '%f\n' | sort
     ${report_manifests}=    Command Should Work
-    ...    ls -l ${sos_report_extracted}/etc/microshift/manifests
+    ...    find ${sos_report_extracted}/etc/microshift/manifests -mindepth 1 -maxdepth 1 -printf '%f\n' | sort
     Should Be Equal As Strings    ${report_manifests}    ${system_manifests}
     ...    msg=Files inside /etc/microshift/manifests directory does not match between system and sosreport

-    ${system_manifests_d}=    Command Should Work    ls -l /etc/microshift/manifests.d/
+    ${system_manifests_d}=    Command Should Work
+    ...    find /etc/microshift/manifests.d -mindepth 1 -maxdepth 1 -printf '%f\n' | sort
     ${report_manifests_d}=    Command Should Work
-    ...    ls -l ${sos_report_extracted}/etc/microshift/manifests.d/
+    ...    find ${sos_report_extracted}/etc/microshift/manifests.d -mindepth 1 -maxdepth 1 -printf '%f\n' | sort
     Should Be Equal As Strings    ${report_manifests_d}    ${system_manifests_d}
     ...    msg=Files inside /etc/microshift/manifests.d directory does not match between system and sosreport
🤖 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-plugins.robot` around lines 57 - 67, The
current comparisons use raw `ls -l` output (variables
${system_manifests}/${report_manifests} and
${system_manifests_d}/${report_manifests_d}) which is brittle; replace the `ls
-l` commands with deterministic listings or checksums — e.g., run a normalized
command that lists filenames sorted (or computes checksums like md5sum) under
/etc/microshift/manifests and /etc/microshift/manifests.d in both system and
${sos_report_extracted}, store those into the same variables, and then use
Should Be Equal As Strings to compare the normalized/sorted filename lists or
checksum outputs so comparisons are stable across metadata formatting
differences.

Comment thread test/suites/otp-workloads/statefulset-pvc.robot Outdated
Comment thread test/suites/standard2/dns-custom-config-validation.robot Outdated
Comment thread test/suites/standard2/dns-custom-config-validation.robot Outdated
@kasturinarra
Copy link
Copy Markdown
Contributor Author

kasturinarra commented May 12, 2026

/retitle USHIFT-6744: Migrate workload gingko tests from OTP

@openshift-ci openshift-ci Bot changed the title USHIFT-6744: Add sosreport and kcm related tests from OTP USHIFT-6744: Migrate workload gingko tests from OTP May 12, 2026
@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label May 12, 2026
Verify Default StorageClass Exists
[Documentation] Skip test if no default StorageClass is available.
${output} ${rc}= Run With Kubeconfig
... oc get sc -o jsonpath\='{.items[?(@.metadata.annotations.storageclass\\.kubernetes\\.io/is-default-class=="true")].metadata.name}'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you reuse the Oc Get and other keywords from oc.resource files? You can tell Claude to do it

@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label May 12, 2026
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 12, 2026

New changes are detected. LGTM label has been removed.

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

♻️ Duplicate comments (1)
test/suites/standard2/dns-custom-config-validation.robot (1)

131-133: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid rm -rf on /etc/microshift/dns in teardown

Line 132 is still unsafe: recursive delete on a host path can remove non-test data. Prefer removing only the test file and optionally removing the directory if empty.

Safer teardown change
-    [Teardown]    Run Keywords
-    ...    Command Should Work    rm -rf /etc/microshift/dns
-    ...    AND    Cleanup Validation Test
+    [Teardown]    Run Keywords
+    ...    Remove Custom Corefile
+    ...    AND    Command Should Work    rmdir /etc/microshift/dns || true
+    ...    AND    Cleanup Validation Test
🤖 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/standard2/dns-custom-config-validation.robot` around lines 131 -
133, The teardown is using a dangerous recursive delete (the "Command Should
Work    rm -rf /etc/microshift/dns" call under the [Teardown] / Run Keywords
block); replace it with a safe two-step cleanup that only removes the specific
test artifact(s) and then removes the directory only if empty. Concretely,
update the teardown to call "Command Should Work" (or the Robot "Remove
File"/"Run" variant) to delete the exact test file(s) you created in
/etc/microshift/dns, and then run an idempotent directory removal (e.g., "rmdir
--ignore-fail-on-non-empty /etc/microshift/dns" or perform a check that the
directory is empty before removing) instead of using rm -rf, keeping the rest of
the teardown (including the "Cleanup Validation Test" keyword) intact.
🧹 Nitpick comments (1)
test/suites/standard2/dns-custom-config-validation.robot (1)

240-243: ⚡ Quick win

Restore service state in this teardown too

This teardown removes config artifacts but does not restart MicroShift. Add restart for deterministic cleanup, consistent with Cleanup Validation Test.

Teardown consistency fix
     [Teardown]    Run Keywords
     ...    Remove Custom Corefile
     ...    AND    Remove Drop In MicroShift Config    20-dns-custom
+    ...    AND    Restart MicroShift
🤖 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/standard2/dns-custom-config-validation.robot` around lines 240 -
243, The teardown currently calls the keywords Remove Custom Corefile and Remove
Drop In MicroShift Config 20-dns-custom but does not restart MicroShift; update
the [Teardown] sequence in dns-custom-config-validation.robot to include the
same restart step used in the Cleanup Validation Test (i.e., add the keyword
that restarts MicroShift after Remove Drop In MicroShift Config) so the service
is deterministically restored during cleanup.
🤖 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 `@test/suites/standard2/dns-custom-config-validation.robot`:
- Around line 376-384: The test "CoreDNS Pod Should Not Be Ready" uses container
restartCount > 0 which can reflect past restarts; instead query the pod's
current readiness flag (e.g., use jsonpath
{.items[0].status.containerStatuses[0].ready} or
{.items[0].status.conditions[?(@.type=="Ready")].status}) and assert it is
"False" (or boolean false) to detect an unhealthy CoreDNS pod; update the Run
With Kubeconfig call in that test to fetch the readiness field and replace the
restartCount-based assertions with a check that readiness == False and an
explanatory failure message referencing the pod readiness state.

---

Duplicate comments:
In `@test/suites/standard2/dns-custom-config-validation.robot`:
- Around line 131-133: The teardown is using a dangerous recursive delete (the
"Command Should Work    rm -rf /etc/microshift/dns" call under the [Teardown] /
Run Keywords block); replace it with a safe two-step cleanup that only removes
the specific test artifact(s) and then removes the directory only if empty.
Concretely, update the teardown to call "Command Should Work" (or the Robot
"Remove File"/"Run" variant) to delete the exact test file(s) you created in
/etc/microshift/dns, and then run an idempotent directory removal (e.g., "rmdir
--ignore-fail-on-non-empty /etc/microshift/dns" or perform a check that the
directory is empty before removing) instead of using rm -rf, keeping the rest of
the teardown (including the "Cleanup Validation Test" keyword) intact.

---

Nitpick comments:
In `@test/suites/standard2/dns-custom-config-validation.robot`:
- Around line 240-243: The teardown currently calls the keywords Remove Custom
Corefile and Remove Drop In MicroShift Config 20-dns-custom but does not restart
MicroShift; update the [Teardown] sequence in dns-custom-config-validation.robot
to include the same restart step used in the Cleanup Validation Test (i.e., add
the keyword that restarts MicroShift after Remove Drop In MicroShift Config) so
the service is deterministically restored during cleanup.
🪄 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: 0f14fa01-1db6-4fc1-9ed2-558967ca25d5

📥 Commits

Reviewing files that changed from the base of the PR and between e3dd561 and 94d6a08.

📒 Files selected for processing (15)
  • test/scenarios-bootc/el10/periodics/el102-src@osconfig-lifecycle.sh
  • test/scenarios-bootc/el10/releases/el102-lrel@osconfig-router.sh
  • test/scenarios-bootc/el10/releases/el102-lrel@otp-workloads.sh
  • test/scenarios-bootc/el9/periodics/el98-src@osconfig-lifecycle.sh
  • test/scenarios-bootc/el9/releases/el98-lrel@osconfig-router.sh
  • test/scenarios-bootc/el9/releases/el98-lrel@otp-workloads.sh
  • test/scenarios/periodics/el98-src@osconfig-lifecycle.sh
  • test/scenarios/releases/el98-lrel@osconfig.sh
  • test/scenarios/releases/el98-lrel@otp-workloads.sh
  • test/suites/otp-workloads/kcm-flags.robot
  • test/suites/otp-workloads/oc-cli.robot
  • test/suites/otp-workloads/sos-report-plugins.robot
  • test/suites/otp-workloads/statefulset-pvc.robot
  • test/suites/standard2/dns-custom-config-validation.robot
  • test_cases_USHIFT-6900.md
✅ Files skipped from review due to trivial changes (1)
  • test_cases_USHIFT-6900.md
🚧 Files skipped from review as they are similar to previous changes (12)

Comment thread test/suites/standard2/dns-custom-config-validation.robot Outdated
@kasturinarra
Copy link
Copy Markdown
Contributor Author

/retest

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 13, 2026

@kasturinarra: The following tests 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/prow/e2e-aws-tests-periodic 3201c52 link true /test e2e-aws-tests-periodic
ci/prow/e2e-aws-tests-release-arm 3201c52 link true /test e2e-aws-tests-release-arm
ci/prow/e2e-aws-tests-release 3201c52 link true /test e2e-aws-tests-release
ci/prow/e2e-aws-tests-periodic-arm 3201c52 link true /test e2e-aws-tests-periodic-arm

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.

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants