Skip to content

Conversation

@Gauravtalreja1
Copy link
Member

Problem Statement

we got a few bugs where customer is running Libvirt CR on RHEL10 hypervisor, and its currently incompatible with existing Satellite release, so in Satellite 6.19 we introducing the support Libvirt CR on RHEL10 hypervisor in SAT-40266

Solution

Modify Libvirt tests to run on Libvirt-CR deployed on RHEL10 along with existing RHEL9 deployment.

Related Issues

@Gauravtalreja1 Gauravtalreja1 self-assigned this Dec 22, 2025
@Gauravtalreja1 Gauravtalreja1 requested review from a team as code owners December 22, 2025 16:48
@Gauravtalreja1 Gauravtalreja1 added QETestCoverage Issues and PRs relating to a Satellite bug No-CherryPick PR doesnt need CherryPick to previous branches Stream Introduced in or relating directly to Satellite Stream/Master labels Dec 22, 2025
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 2 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `tests/foreman/cli/test_computeresource_libvirt.py:190` </location>
<code_context>


-def test_positive_create_with_locs(libvirt_url, module_target_sat):
+def test_positive_create_with_locs(libvirt, module_target_sat):
     """Create Compute Resource with multiple locations

</code_context>

<issue_to_address>
**suggestion (testing):** Consider parametrizing `libvirt` here as well to exercise both libvirt9 and libvirt10 backends.

This test now depends on `libvirt.url`, but unlike `test_positive_crud_libvirt_cr` it doesn’t parametrize `libvirt` over `['libvirt9', 'libvirt10']`, so only the fixture default (currently libvirt9) is exercised. To validate behavior on both RHEL9 and RHEL10 Libvirt-CR, please add `@pytest.mark.parametrize('libvirt', ['libvirt9', 'libvirt10'], indirect=True)` here as well.

```suggestion
@pytest.mark.parametrize('libvirt', ['libvirt9', 'libvirt10'], indirect=True)
def test_positive_create_with_locs(libvirt, module_target_sat):
```
</issue_to_address>

### Comment 2
<location> `tests/foreman/cli/test_computeresource_libvirt.py:483-485` </location>
<code_context>


 def test_positive_crud_image_libvirt_with_name(
-    request, module_location, module_org, module_target_sat, module_os
+    request, module_location, module_org, module_target_sat, module_os, libvirt
 ):
     """Create, Read, Update and Delete images on the libvirt compute resource
</code_context>

<issue_to_address>
**suggestion (testing):** This image CRUD test only runs against the default libvirt backend; consider parametrizing to cover both libvirt9 and libvirt10.

Since this flow now depends on `libvirt.url` and `configure_libvirt_cr(server_fqdn=libvirt.fqdn)`, it should be exercised against both RHEL9 and RHEL10 Libvirt-CR. Please parametrize this test over libvirt versions (e.g. using `@pytest.mark.parametrize('libvirt', ['libvirt9', 'libvirt10'], indirect=True)` as in the other e2e tests).

```suggestion
@pytest.mark.parametrize('libvirt', ['libvirt9', 'libvirt10'], indirect=True)
def test_positive_crud_image_libvirt_with_name(
    request, module_location, module_org, module_target_sat, module_os, libvirt
):
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +13 to +14
'libvirt9': settings.libvirt.hostname.libvirt9,
'libvirt10': settings.libvirt.hostname.libvirt10,
Copy link
Contributor

Choose a reason for hiding this comment

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

libvirt9 and libvirt10 are not defined in the libvirt settings. Have you raised an MR for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Of course, yes, see satellite-jenkins MR#1838, where I'm validating this changing using its Jenkins MR

@Gauravtalreja1
Copy link
Member Author

Tested the changes with Jenkins-MR of satellite-jenkins MR#1838 and attached results there, so this is ready for review/merge now 🍏

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

Labels

No-CherryPick PR doesnt need CherryPick to previous branches QETestCoverage Issues and PRs relating to a Satellite bug Stream Introduced in or relating directly to Satellite Stream/Master

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants