Uh oh!
There was an error while loading. Please reload this page.
listHosts: add 'core' details - #13444
Conversation
nvazquez
commented
Jun 18, 2026
@blueorangutan package |
blueorangutan
commented
Jun 18, 2026
@nvazquez 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. |
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@## 4.22 #13444 +/- ##
=============================================
- Coverage 17.67% 3.69% -13.98%
=============================================
Files 5922 448 -5474 Lines 533167 38107 -495060 Branches 65210 7061 -58149 =============================================
- Hits 94246 1409 -92837 + Misses 428276 36511 -391765 + Partials 10645 187 -10458
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
blueorangutan
commented
Jun 18, 2026
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18291 |
weizhouapache
commented
Jun 18, 2026
@nvazquez |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new opt-in details=core option for the listHosts API, intended to return a lightweight “core” host response and avoid per-host (N+1) lookups performed by the existing response builder.
Changes:
- Added
coretoApiConstants.HostDetailssolistHostscan acceptdetails=core. - Implemented a new “core” host response path in
HostJoinDaoImpl.newHostResponse(...)viasetNewCoreHostResponse(...).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
server/src/main/java/com/cloud/api/query/dao/HostJoinDaoImpl.java | Adds a new lightweight host response builder and routes details=core requests to it. |
api/src/main/java/org/apache/cloudstack/api/ApiConstants.java | Adds core to the HostDetails enum (allowed values for details). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
@nvazquez in the description you mentioned avoiding requests when getting details like cpu usage, but I don't see this parameter in specific being added to the response, am I missing something? |
Uh oh!
There was an error while loading. Please reload this page.
GaOrtiga
commented
Jun 23, 2026
Also, the copilot left a review regarding the MS ID that I kind of agree. This API is admin only, so shouldn't be a big deal returning the internal ID, however, this same api returns MS UUID when other filters are passed, having this one be different may cause confusion |
nvazquez
commented
Jun 23, 2026
Thanks @RosiKyu@GaOrtiga@weizhouapache - I have refactored the code according to your review comments and Copilot comments. The only pending item is the msid return value, I'm keeping this PR in draft while this is not addressed and will put back to review state once its sorted out |
nvazquez
commented
Jun 23, 2026
@blueorangutan package |
blueorangutan
commented
Jun 23, 2026
@nvazquez 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. |
blueorangutan
commented
Jun 23, 2026
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18351 |
nvazquez
commented
Jun 26, 2026
@GaOrtiga@RosiKyu please find the PR ready for review again. I have introduced a new response field @blueorangutan package |
blueorangutan
commented
Jun 26, 2026
@nvazquez 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. |
blueorangutan
commented
Jun 26, 2026
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18395 |
nvazquez
commented
Jun 26, 2026
@blueorangutan test |
blueorangutan
commented
Jun 26, 2026
@nvazquez a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
blueorangutan
commented
Jun 27, 2026
[SF] Trillian test result (tid-16445)
|
RosiKyu
left a comment
There was a problem hiding this comment.
LGTM
Tested on 4.22.2.0
listHosts details=corereturns the lightweight host set as intended; default listing unchanged.- Management server returned as numeric msid under core - confirmed intentional (avoids a per-host DB lookup).
listHostsMetrics details=corereturns HTTP 530- External hypervisor hosts don't return extensionid/extensionname under core- accepted as won't-fix (fetching it would add a lookup and reduce the performance core exists for).
Follow-up: doc/API-docs note for the core detail.
Description
This PR adds a detail
corefor thelistHostsAPI, which avoids making extra DB queries per host listed.This will drastically reduce the time it takes to listHosts and allows us to get a list of hosts & their state, which can take lot of time for the API to complete if the core detail is not passed on large environments.
This is strictly opt-in with
listHosts details=coreand the default bahaviour is unchanged.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?