extension: improve host vm power reporting #11619
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.22 #11619 +/- ##
============================================
+ Coverage 17.55% 17.56% +0.01%
- Complexity 15535 15547 +12
============================================
Files 5909 5909
Lines 529053 529140 +87
Branches 64619 64629 +10
============================================
+ Hits 92879 92952 +73
- Misses 425719 425731 +12
- Partials 10455 10457 +2
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:
|
shwstppr
changed the title
There was a problem hiding this comment.
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
statusesaction 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.
| } | ||
| try { | ||
| JsonObject jsonObj = new JsonParser().parse(response).getAsJsonObject(); | ||
| JsonObject jsonObj = JsonParser.parseString(response).getAsJsonObject(); |
There was a problem hiding this comment.
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.
|
@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)
|
There was a problem hiding this comment.
@shwstppr unable to deploy vm on proxmox host, during the vm deployment provided only vm_name in external details
Getting the following exception
2025-09-22 08:34:49,095 WARN [o.a.c.h.e.p.ExternalPathPayloadProvisioner] (DirectAgent-10:[ctx-c16a7629]) (logid:73cf7b1d) Failed to create the instance daf26bd7-2b9e-4480-acd6-ce2edce8f430 on external system: External API execution failed with exit code 1
2025-09-22 08:34:49,095 DEBUG [o.a.c.h.e.p.ExternalPathPayloadProvisioner] (DirectAgent-10:[ctx-c16a7629]) (logid:73cf7b1d) Deploying VM daf26bd7-2b9e-4480-acd6-ce2edce8f430 on the external system failed: Exit code: 1, Output: {"error":"Missing required fields: vmid"}
The issue was not present before
|
@kiranchavala are you able to deploy VMs when not using PR changes? |
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
@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 15696 |
shwstppr
force-pushed
the
improve-ext-powerreport
branch
from
0b5aa17 to
2129c80
Compare
Add `statuses` action in extensions to report VM power states This 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 existing `status` action. This helps with updating the host in CloudStack after out-of-band migrations for the VM. Signed-off-by: Abhishek Kumar <[email protected]>
shwstppr
force-pushed
the
improve-ext-powerreport
branch
from
2129c80 to
bc826b8
Compare
|
@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 15705 |
|
I have started looking. Will finish the review by tomorrow. |
abh1sar
left a comment
abh1sar
left a comment
There was a problem hiding this comment.
code LGTM with 2 minor comments.
extensions/Proxmox/proxmox.sh
Outdated
| }' | ||
| } | ||
| get_console() { | ||
| check_required_fields node vmid |
There was a problem hiding this comment.
indentation is changed from 4 to 3 spaces.
| doReturn(new Pair<>(false, "Error")).when(provisioner) | ||
| .getInstanceStatusesOnExternalSystem(anyString(), anyString(), anyString(), anyMap(), anyInt()); | ||
|
|
||
| Map<String, HostVmStateReportEntry> result = provisioner.getVmPowerStates(host, accessDetails, "test-extension", "test-path"); |
Signed-off-by: Abhishek Kumar <[email protected]>
5 participants
Description
Add
statusesaction 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
statusesaction.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 existingstatusaction.This helps with updating the host in CloudStack after out-of-band migrations for the VM.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
extension-powersync-oobmigrate.mp4
How Has This Been Tested?
Tested with an env having 2x Proxmox hosts. Power sync worked after oob power operations on the VM. Also, host gets updated in CloudStack after oob migration from Proxmox (video above).
How did you try to break this feature and the system with this change?