[6.19.z] Test the VM power on/off functionality and verify memory usage - #20974
Conversation
Reviewer's GuideAdds a new end-to-end UI test that provisions an OpenShift Virtualization VM via image-based provisioning and validates VM power state transitions, available actions, and memory usage reporting in the compute resource view, including userdata template setup via API. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
wait_forandsearchcalls assume at least one host is always returned (e.g....search(...)[0]), which can raiseIndexErrorand make the test flaky; consider asserting the list is non-empty or usingwait_forto guard against empty responses before indexing. - After calling
vm_poweroff, the test immediately asserts the new power state without any retry or wait; wrapping thesearch_virtual_machinefor the power-off verification in await_forwould make the test more robust against UI/API propagation delays. - The memory check uses
round(memory_in_gb)and string comparison to'6 GB'; tying the assertion directly to the configured MB value (e.g. computing the expected display format from API/CR settings) would reduce brittleness if the UI changes its rounding or unit display.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `wait_for` and `search` calls assume at least one host is always returned (e.g. `...search(...)[0]`), which can raise `IndexError` and make the test flaky; consider asserting the list is non-empty or using `wait_for` to guard against empty responses before indexing.
- After calling `vm_poweroff`, the test immediately asserts the new power state without any retry or wait; wrapping the `search_virtual_machine` for the power-off verification in a `wait_for` would make the test more robust against UI/API propagation delays.
- The memory check uses `round(memory_in_gb)` and string comparison to `'6 GB'`; tying the assertion directly to the configured MB value (e.g. computing the expected display format from API/CR settings) would reduce brittleness if the UI changes its rounding or unit display.
## Individual Comments
### Comment 1
<location path="tests/foreman/ui/test_computeresource_ocpv.py" line_range="117-120" />
<code_context>
+ if host_api:
+ request.addfinalizer(host_api[0].delete)
+
+ wait_for(
+ lambda: (
+ sat.api.Host().search(query={'search': f'name="{host_fqdn}"'})[0].build_status_label
+ != 'Pending installation'
+ ),
+ timeout=1500,
</code_context>
<issue_to_address>
**issue (bug_risk):** Make the wait_for condition resilient to missing host records to avoid IndexError flakiness
The `wait_for` lambda assumes `sat.api.Host().search(...)[0]` always returns a host. If the record isn’t present yet, this raises `IndexError` and fails the test instead of retrying. Please make this more defensive by checking that the search result is non-empty before accessing `[0]`, or by catching `IndexError` and returning `False`, so `wait_for` can keep polling instead of causing flaky failures when host creation is slow.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| wait_for( | ||
| lambda: ( | ||
| sat.api.Host().search(query={'search': f'name="{host_fqdn}"'})[0].build_status_label | ||
| != 'Pending installation' |
There was a problem hiding this comment.
issue (bug_risk): Make the wait_for condition resilient to missing host records to avoid IndexError flakiness
The wait_for lambda assumes sat.api.Host().search(...)[0] always returns a host. If the record isn’t present yet, this raises IndexError and fails the test instead of retrying. Please make this more defensive by checking that the search result is non-empty before accessing [0], or by catching IndexError and returning False, so wait_for can keep polling instead of causing flaky failures when host creation is slow.
|
PRT Result |
Test the VM power on/off functionality and verify memory usage during the operation. (cherry picked from commit 9d843f7)
fe0708f to
72742ef
Compare
|
PRT Result |
|
|
PRT Result |
Cherrypick of PR: #20850
Problem Statement
The VM was not displaying the correct power state (on/off), and the memory value was also not shown correctly.
Solution
After the fix, the VMs are showing the correct state. When the VM is powered on, it displays the correct memory value. However, when the VM is powered off, the memory value is not shown correctly. This behavior was already mentioned in the PR, and the developer is aware of the issue. At the moment, this is not a blocker for us.
Also adding end-to-end OCP image-based provisioning.
Summary by Sourcery
Extend OpenShift Virtualization end-to-end userdata image provisioning test to validate VM power state and memory reporting in the compute resource UI.
New Features:
Tests:
Summary by Sourcery
Tests: