-
Notifications
You must be signed in to change notification settings - Fork 1.2k
extension: improve host vm power reporting #11619
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This PR adds change to get power state report of all VMs on the host from the extension itself. Signed-off-by: Abhishek Kumar <[email protected]>
Signed-off-by: Abhishek Kumar <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11619 +/- ##
============================================
+ Coverage 17.36% 17.40% +0.04%
- Complexity 15245 15291 +46
============================================
Files 5888 5888
Lines 525831 526171 +340
Branches 64183 64238 +55
============================================
+ Hits 91298 91580 +282
- Misses 424227 424256 +29
- Partials 10306 10335 +29
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Abhishek Kumar <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new statuses
action to extensions that allows querying the power state of all VMs on a host with a single call, improving efficiency over individual VM queries. The server gracefully falls back to the existing status
action when the extension doesn't support the new functionality.
Key changes:
- Add
statuses
action implementation to existing extensions (Proxmox, HyperV, sample provisioner) - Implement Java logic to use batch VM status retrieval with fallback to individual calls
- Add comprehensive test coverage for the new functionality
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
scripts/vm/hypervisor/external/provisioner/provisioner.sh | Adds sample statuses action implementation that returns "Not supported" error |
plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java | Implements batch VM status retrieval with fallback logic and modernizes JSON parsing |
plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java | Adds comprehensive test coverage for new batch status functionality |
extensions/Proxmox/proxmox.sh | Implements statuses action to query all VMs via Proxmox API |
extensions/HyperV/hyperv.py | Implements statuses action using PowerShell Get-VM command |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@@ -647,7 +654,7 @@ protected VirtualMachine.PowerState parsePowerStateFromResponse(UserVmVO userVmV | |||
return getPowerStateFromString(response); | |||
} | |||
try { | |||
JsonObject jsonObj = new JsonParser().parse(response).getAsJsonObject(); | |||
JsonObject jsonObj = JsonParser.parseString(response).getAsJsonObject(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The deprecated new JsonParser().parse()
method has been replaced with JsonParser.parseString()
, but this change should be consistent throughout the file. The same pattern should be applied to line 702 where JsonParser.parseString()
is used in the new code.
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
@blueorangutan package |
@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 14967 |
@blueorangutan test |
@shwstppr a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
[SF] Trillian test result (tid-14326)
|
Description
Add
statuses
action in extensions to report VM power statesThis PR introduces support for retrieving the power state of all VMs on a host directly from an extension using the new
statuses
action.When available, this provides a single aggregated response, reducing the need for multiple calls.
If the extension does not implement
statuses
, the server will gracefully fall back to querying individual VMs using the existingstatus
action.Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?