Conversation
WalkthroughUpdated test configuration in AWS global config to add CEPH_RBD_VIRTUALIZATION storage class, remove default flag from TRIDENT_CSI_FSX entry, and replace storage class references from IO2_CSI to CEPH_RBD_VIRTUALIZATION. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
|
/verified |
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Your deliverable: Test Execution Plan
Guidelines:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL: Before setting "Run smoke tests: True", you MUST use shell scripts to: |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/global_config_aws.py (1)
14-23: Consider documenting the rationale for the wffc setting.The PR description is empty, making it difficult to understand the reasoning behind choosing
wffc: Falsefor CEPH_RBD_VIRTUALIZATION while IO2_CSI useswffc: True. Consider adding a brief inline comment or updating the PR description to explain this configuration choice, especially if it's related to specific OCS/AWS characteristics.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/global_config_aws.py
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 2469
File: utilities/sanity.py:139-142
Timestamp: 2025-11-08T07:36:57.616Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers to keep refactoring PRs (like PR #2469) strictly focused on moving/organizing code into more granular modules without adding new functionality, error handling, or behavioral changes. Such improvements should be handled in separate PRs.
Learnt from: vamsikrishna-siddu
Repo: RedHatQE/openshift-virtualization-tests PR: 2199
File: tests/storage/test_online_resize.py:108-113
Timestamp: 2025-09-28T14:43:07.181Z
Learning: In the openshift-virtualization-tests repo, PR #2199 depends on PR #2139 which adds architecture-specific OS_FLAVOR attributes to the Images.Cirros class (OS_FLAVOR_CIRROS for x86_64/ARM64, OS_FLAVOR_FEDORA for s390x), enabling conditional logic based on the underlying OS flavor in tests.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T14:00:59.076Z
Learning: In the openshift-virtualization-tests repository, when responding to test execution plan requests from openshift-virtualization-qe-bot-3, CodeRabbit must post ONLY an inline review comment on the Files Changed tab and then stop immediately without generating any follow-up comments in the PR discussion thread. No acknowledgment messages, no confirmation of posting, no explanation - silence after posting the inline review equals success. Additional comments create empty/meaningless reviews that clutter the PR.
Learnt from: vamsikrishna-siddu
Repo: RedHatQE/openshift-virtualization-tests PR: 2199
File: tests/storage/test_online_resize.py:108-113
Timestamp: 2025-09-28T14:43:07.181Z
Learning: In the openshift-virtualization-tests repo, PR #2199 depends on PR #2139 which adds the OS_FLAVOR attribute to the Images.Cirros class, making Images.Cirros.OS_FLAVOR available for conditional logic in tests.
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 2356
File: tests/global_config_gcp.py:33-34
Timestamp: 2025-10-22T13:58:31.219Z
Learning: In tests/global_config_gcp.py, both storage_class_a and storage_class_b should be set to StorageClassNames.GCNV (not GCP), as the intent is to use GCNV for both storage class variables in this configuration.
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1641
File: tests/infrastructure/golden_images/test_common_templates_data_volumes.py:47-48
Timestamp: 2025-08-13T06:27:29.727Z
Learning: In tests/infrastructure/golden_images/test_common_templates_data_volumes.py, when set_storage_class is True but get_diff_storage_class_from_matrix returns None (indicating only one storage class is available), the test should continue using the default storage class rather than being skipped. This covers special cluster architectures that only have a single storage class available.
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 2014
File: tests/storage/cdi_clone/test_clone.py:0-0
Timestamp: 2025-09-10T13:25:59.799Z
Learning: In the openshift-virtualization-tests repository, when creating DataVolume objects using ocp_resources, use `api_name="storage"` rather than the default `api_name="pvc"`.
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 1160
File: tests/storage/storage_migration/test_mtc_storage_class_migration.py:165-176
Timestamp: 2025-06-17T07:45:37.776Z
Learning: In the openshift-virtualization-tests repository, user jpeimer prefers explicit fixture parameters over composite fixtures in test methods, even when there are many parameters, as they find this approach more readable and maintainable for understanding test dependencies.
📚 Learning: 2025-10-22T13:58:31.219Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 2356
File: tests/global_config_gcp.py:33-34
Timestamp: 2025-10-22T13:58:31.219Z
Learning: In tests/global_config_gcp.py, both storage_class_a and storage_class_b should be set to StorageClassNames.GCNV (not GCP), as the intent is to use GCNV for both storage class variables in this configuration.
Applied to files:
tests/global_config_aws.py
📚 Learning: 2025-08-13T06:27:29.727Z
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1641
File: tests/infrastructure/golden_images/test_common_templates_data_volumes.py:47-48
Timestamp: 2025-08-13T06:27:29.727Z
Learning: In tests/infrastructure/golden_images/test_common_templates_data_volumes.py, when set_storage_class is True but get_diff_storage_class_from_matrix returns None (indicating only one storage class is available), the test should continue using the default storage class rather than being skipped. This covers special cluster architectures that only have a single storage class available.
Applied to files:
tests/global_config_aws.py
📚 Learning: 2025-08-27T12:38:26.496Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1723
File: conftest.py:496-502
Timestamp: 2025-08-27T12:38:26.496Z
Learning: In PR #1723, the user rnetser clarified that updating downstream string comparisons to handle DataVolume enums from StorageClassConfig is not part of the current PR scope, which focuses only on adding conformance storage class support.
Applied to files:
tests/global_config_aws.py
📚 Learning: 2025-05-21T09:41:46.532Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 954
File: tests/storage/storage_migration/conftest.py:156-176
Timestamp: 2025-05-21T09:41:46.532Z
Learning: In the RedHatQE/openshift-virtualization-tests codebase, the pattern of calling to_dict() on a DataVolume object and then accessing dv.res to populate data_volume_template is the preferred approach rather than building the template dict explicitly.
Applied to files:
tests/global_config_aws.py
📚 Learning: 2025-09-10T13:25:59.799Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 2014
File: tests/storage/cdi_clone/test_clone.py:0-0
Timestamp: 2025-09-10T13:25:59.799Z
Learning: In the openshift-virtualization-tests repository, when creating DataVolume objects using ocp_resources, use `api_name="storage"` rather than the default `api_name="pvc"`.
Applied to files:
tests/global_config_aws.py
📚 Learning: 2025-12-15T12:33:06.686Z
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests PR: 3024
File: tests/network/connectivity/utils.py:17-17
Timestamp: 2025-12-15T12:33:06.686Z
Learning: In the test suite, ensure the ipv6_network_data fixture returns a factory function (Callable) and that all call sites invoke it to obtain the actual data dict, i.e., use ipv6_network_data() at call sites. This enables future extensibility for configuring secondary interfaces' IP addresses without changing call sites. Apply this pattern to all Python test files under tests.
Applied to files:
tests/global_config_aws.py
📚 Learning: 2025-12-22T16:27:40.244Z
Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests PR: 3196
File: tests/network/upgrade/test_upgrade_network.py:4-4
Timestamp: 2025-12-22T16:27:40.244Z
Learning: For PRs that remove tests, rely on pytest --collect-only to verify the test discovery results (which tests are selected/deselected) and ensure the removal is clean and the test module remains functional. Full test execution is not required for test deletion PRs. This guideline applies to test files anywhere under the tests/ directory (e.g., tests/network/upgrade/test_upgrade_network.py) and should be used for similar test-deletion scenarios across the repository.
Applied to files:
tests/global_config_aws.py
🧬 Code graph analysis (1)
tests/global_config_aws.py (1)
utilities/constants.py (1)
StorageClassNames(706-722)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: tox
- GitHub Check: build-container
🔇 Additional comments (2)
tests/global_config_aws.py (2)
53-54: No issues found. Setting bothstorage_class_aandstorage_class_btoStorageClassNames.CEPH_RBD_VIRTUALIZATIONis consistent with other cloud configuration files (NFS, ARO, OCI, GCP) where both variables reference the same default storage class for the platform.
14-23: Verify storage class assignment intent and CEPH_RBD_VIRTUALIZATION AWS deployment requirement.Both
storage_class_aandstorage_class_bare now set toCEPH_RBD_VIRTUALIZATION(changed fromIO2_CSI). This means storage migration tests will migrate between the same storage class, which may not exercise the intended code paths if the tests expect source and target classes to differ. Confirm this is intentional for AWS's storage architecture.Additionally, the
wffcchange fromTrue(IO2_CSI) toFalse(CEPH_RBD_VIRTUALIZATION) affects test matrix filtering—tests usingwffc_matrix()will exclude this storage class while tests usingimmediate_matrix()will include it. Ensure CEPH_RBD_VIRTUALIZATION (OCS-backed) is deployed in your AWS test clusters before merging.⛔ Skipped due to learnings
Learnt from: RoniKishner Repo: RedHatQE/openshift-virtualization-tests PR: 1641 File: tests/infrastructure/golden_images/test_common_templates_data_volumes.py:47-48 Timestamp: 2025-08-13T06:27:29.727Z Learning: In tests/infrastructure/golden_images/test_common_templates_data_volumes.py, when set_storage_class is True but get_diff_storage_class_from_matrix returns None (indicating only one storage class is available), the test should continue using the default storage class rather than being skipped. This covers special cluster architectures that only have a single storage class available.Learnt from: rnetser Repo: RedHatQE/openshift-virtualization-tests PR: 1723 File: conftest.py:496-502 Timestamp: 2025-08-27T12:38:26.496Z Learning: In PR #1723, the user rnetser clarified that updating downstream string comparisons to handle DataVolume enums from StorageClassConfig is not part of the current PR scope, which focuses only on adding conformance storage class support.Learnt from: jpeimer Repo: RedHatQE/openshift-virtualization-tests PR: 2356 File: tests/global_config_gcp.py:33-34 Timestamp: 2025-10-22T13:58:31.219Z Learning: In tests/global_config_gcp.py, both storage_class_a and storage_class_b should be set to StorageClassNames.GCNV (not GCP), as the intent is to use GCNV for both storage class variables in this configuration.Learnt from: jpeimer Repo: RedHatQE/openshift-virtualization-tests PR: 0 File: :0-0 Timestamp: 2025-12-22T15:56:00.157Z Learning: In the openshift-virtualization-tests repository, when responding to test execution plan requests from openshift-virtualization-qe-bot-3, do NOT use "REQUEST_CHANGES" review type if the PR author has already marked the PR as verified (e.g., with `/verified` command). Test execution plans are informational guides, not blocking requirements. Use COMMENT event for informational test plans, or only REQUEST_CHANGES if there are actual code issues that need to be addressed before merging.Learnt from: hmeir Repo: RedHatQE/openshift-virtualization-tests PR: 3081 File: tests/install_upgrade_operators/product_install/test_install_openshift_virtualization.py:189-191 Timestamp: 2025-12-29T10:07:12.804Z Learning: In test_ocs_virt_default_storage_class in tests/install_upgrade_operators/product_install/test_install_openshift_virtualization.py, consecutive_checks_count=3 is sufficient for verify_boot_sources_reimported because datasources update quickly in the installation lane scenario after storage class changes.Learnt from: jpeimer Repo: RedHatQE/openshift-virtualization-tests PR: 2014 File: tests/storage/cdi_clone/test_clone.py:0-0 Timestamp: 2025-09-10T13:25:59.799Z Learning: In the openshift-virtualization-tests repository, when creating DataVolume objects using ocp_resources, use `api_name="storage"` rather than the default `api_name="pvc"`.Learnt from: rnetser Repo: RedHatQE/openshift-virtualization-tests PR: 3062 File: conftest.py:333-333 Timestamp: 2025-12-16T20:11:03.645Z Learning: In the openshift-virtualization-tests repository, when determining smoke test impact for changes affecting py_config["os_login_param"], follow this verification methodology: (1) Find all smoke tests: `rg "pytest.mark.smoke" --type=py -B2 | grep "def test_"`, (2) For each smoke test file, search for VM creation patterns: `rg "VirtualMachineForTests|running_vm|VirtualMachineForTestsFromTemplate|wait_for_ssh|check_ssh_connectivity"`, (3) Trace the dependency chain: smoke test → VirtualMachineForTests/running_vm() → wait_for_ssh_connectivity() (default enabled) → vm.login_params property → py_config["os_login_param"][vm.os_flavor], (4) Check utilities/virt.py for login_params usage: `rg "os_login_param|login_params" utilities/virt.py -C3`. Any smoke test creating VMs with default SSH connectivity checks (running_vm with check_ssh_connectivity=True) depends on os_login_param, even if the test doesn't directly reference it.Learnt from: rnetser Repo: RedHatQE/openshift-virtualization-tests PR: 0 File: :0-0 Timestamp: 2025-12-16T10:28:54.212Z Learning: In the openshift-virtualization-tests repository, when determining smoke test impact for test execution plans, only set "Run smoke tests: True" if there is a verified, traceable dependency path from smoke tests to the changed code. Session-scoped fixtures or infrastructure-sounding changes do NOT automatically imply smoke test impact - the dependency chain must be explicitly verified using shell scripts before recommending smoke test execution.Learnt from: jpeimer Repo: RedHatQE/openshift-virtualization-tests PR: 954 File: tests/storage/storage_migration/conftest.py:156-176 Timestamp: 2025-05-21T09:41:46.532Z Learning: In the RedHatQE/openshift-virtualization-tests codebase, the pattern of calling to_dict() on a DataVolume object and then accessing dv.res to populate data_volume_template is the preferred approach rather than building the template dict explicitly.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1157 🏁 Script executed: Length of output: 26 🏁 Script executed: Length of output: 53 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 46503 🏁 Script executed: Length of output: 5907 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 4657 🏁 Script executed: Length of output: 281 🏁 Script executed: Length of output: 872 🏁 Script executed: Length of output: 67 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 1006 🏁 Script executed: Length of output: 827 🏁 Script executed: Length of output: 4148 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 424 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 469 🏁 Script executed: Length of output: 2510 🏁 Script executed: Length of output: 1686 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 92 🏁 Script executed: Length of output: 101 🏁 Script executed: Length of output: 272 🏁 Script executed: Length of output: 1835 🏁 Script executed: Length of output: 1728 🧠 Learnings used |
|
/approve |
|
/verified |
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Your deliverable: Test Execution Plan
Guidelines:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL: Before setting "Run smoke tests: True", you MUST use shell scripts to: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1159 🏁 Script executed: Length of output: 1300 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 1841 🏁 Script executed: Length of output: 1649 🏁 Script executed: Length of output: 2714 🏁 Script executed: Length of output: 2293 🏁 Script executed: Length of output: 218 🏁 Script executed: Length of output: 669 🏁 Script executed: Length of output: 475 🏁 Script executed: Length of output: 3024 🏁 Script executed: Length of output: 6167 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 482 🏁 Script executed: Length of output: 1728 🧠 Learnings used |
|
/approve |
|
/lgtm |
|
/lgtm |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
##### Short description: ##### More details: ##### What this PR does / why we need it: ##### Which issue(s) this PR fixes: ##### Special notes for reviewer: ##### jira-ticket: <!-- full-ticket-url needs to be provided. This would add a link to the pull request to the jira and close it when the pull request is merged If the task is not tracked by a Jira ticket, just write "NONE". --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated storage configuration tests to support CEPH RBD virtualization with block volume mode and read-write-many access capabilities. * Adjusted default storage class configurations for improved test coverage. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
##### Short description: ##### More details: ##### What this PR does / why we need it: ##### Which issue(s) this PR fixes: ##### Special notes for reviewer: ##### jira-ticket: <!-- full-ticket-url needs to be provided. This would add a link to the pull request to the jira and close it when the pull request is merged If the task is not tracked by a Jira ticket, just write "NONE". --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated storage configuration tests to support CEPH RBD virtualization with block volume mode and read-write-many access capabilities. * Adjusted default storage class configurations for improved test coverage. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Short description:
More details:
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.