Test the VM power on/off functionality and verify memory usage - #20850
Conversation
Reviewer's GuideAdds a new UI end-to-end test for OpenShift Virtualization image-based provisioning that creates a host via the Foreman UI, verifies successful installation, and validates VM power state transitions, available power actions, and reported memory in the compute resource view, while slightly cleaning up the existing API test docstring spacing. 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 left some high level feedback:
- You call
session.computeresource.search_virtual_machine(cr.name, host.name)[0]multiple times in succession; consider assigning this to a local variable to avoid repeated lookups and make the assertions easier to read. - The test relies on hardcoded UI text values like
'On','Off','Power On', and'Power Off'; consider centralizing these as constants or using higher-level helpers so the test is less brittle to minor UI wording changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You call `session.computeresource.search_virtual_machine(cr.name, host.name)[0]` multiple times in succession; consider assigning this to a local variable to avoid repeated lookups and make the assertions easier to read.
- The test relies on hardcoded UI text values like `'On'`, `'Off'`, `'Power On'`, and `'Power Off'`; consider centralizing these as constants or using higher-level helpers so the test is less brittle to minor UI wording changes.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| with session: | ||
| session.organization.select(org_name=module_org.name) | ||
| session.location.select(loc_name=module_location.name) | ||
| # The VM appears in the compute resource with correct power state (On) |
There was a problem hiding this comment.
This is API test module, so I feel it should be wise to create UI test module to cover this scenario along with provisioning
There was a problem hiding this comment.
I tried using the API, but it appears that the API call is not working:
PUT /api/compute_resources/:id/available_virtual_machines/:vm_id/power
There was a problem hiding this comment.
Which compute resource is it? If it's not working we should fix it.
There was a problem hiding this comment.
You don't say what "is not working", but I have a hunch that theforeman/foreman#10889 will fix the issue if the API was returning a 404 on that PUT request you tried.
There was a problem hiding this comment.
@evgeni I tested your patch. It is now displaying the VM information correctly, which is good. However, the power action is still not being executed.
There was a problem hiding this comment.
I used hammer compute-resource virtual-machine power --id 1 --vm-id <id> and it worked on my instance.
What was the error you were getting?
There was a problem hiding this comment.
It is working now. I think I was trying it on an older machine earlier.
|
PRT Result |
e296e90 to
be6739b
Compare
|
PRT Result |
be6739b to
8b9b307
Compare
|
PRT Result |
|
PRT Result |
593da29 to
8acb372
Compare
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The test calls
session.computeresource.search_virtual_machine(...)multiple times with identical arguments; consider querying once per state (initial, after poweroff) and reusing the result to simplify the assertions and reduce UI calls. - The hard-coded
storage_classvalue ('trident-nfs (csi.trident.netapp.io)') makes the test tightly coupled to a specific environment; consider sourcing this from configuration or a fixture so the test can run against different OCPV setups.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The test calls `session.computeresource.search_virtual_machine(...)` multiple times with identical arguments; consider querying once per state (initial, after poweroff) and reusing the result to simplify the assertions and reduce UI calls.
- The hard-coded `storage_class` value (`'trident-nfs (csi.trident.netapp.io)'`) makes the test tightly coupled to a specific environment; consider sourcing this from configuration or a fixture so the test can run against different OCPV setups.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
PRT Result |
8acb372 to
f6c8605
Compare
|
PRT Result |
|
PRT Result |
|
PRT Result |
|
PRT Result |
f6c8605 to
a6dea7c
Compare
|
PRT Result |
e570f1f to
c81f868
Compare
| host_api = sat.api.Host().search(query={'search': f'name="{host_fqdn}"'}) | ||
| if host_api: | ||
| request.addfinalizer(host_api[0].delete) |
There was a problem hiding this comment.
You can just use existing provisioning cleanup finalizer which does the same
request.addfinalizer(lambda: sat.provisioning_cleanup(host.name))
There was a problem hiding this comment.
I removed above part again because the tests were failing. With the previous change, the tests are now passing. ✅
c81f868 to
d7529ab
Compare
shubhamsg199
left a comment
There was a problem hiding this comment.
Ack, pending PRT results
|
PRT Result |
|
PRT Result |
|
sambible
left a comment
There was a problem hiding this comment.
ACK, pending PRT. I re-kicked it.
|
PRT Result |
|
PRT Result |
|
PRT Result |
d7529ab to
f071913
Compare
|
|
PRT Result |
Test the VM power on/off functionality and verify memory usage during the operation. (cherry picked from commit 9d843f7)
Test the VM power on/off functionality and verify memory usage during the operation. (cherry picked from commit 9d843f7)
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
Add a UI-based end-to-end test that provisions a host on OpenShift Virtualization and validates VM power state and memory reporting in the compute resource view.
New Features:
Tests: