Copilot commented on code in PR #13444:
URL: https://github.com/apache/cloudstack/pull/13444#discussion_r3434105187
##########
server/src/main/java/com/cloud/api/query/dao/HostJoinDaoImpl.java:
##########
@@ -117,6 +117,51 @@ private boolean containsHostHATag(final String tags) {
return result;
}
+ private void setNewCoreHostResponse(HostJoinVO host, HostResponse
hostResponse) {
+ hostResponse.setId(host.getUuid());
+ hostResponse.setName(host.getName());
+ hostResponse.setState(host.getStatus());
+ hostResponse.setResourceState(host.getResourceState().toString());
+ hostResponse.setDisconnectedOn(host.getDisconnectedOn());
+ hostResponse.setHostType(host.getType());
+ hostResponse.setIpAddress(host.getPrivateIpAddress());
+ hostResponse.setVersion(host.getVersion());
+ hostResponse.setCreated(host.getCreated());
+ hostResponse.setRemoved(host.getRemoved());
+ hostResponse.setLastPinged(new Date(host.getLastPinged()));
+ if (host.getHypervisorType() != null) {
+
hostResponse.setHypervisor(host.getHypervisorType().getHypervisorDisplayName());
+ }
+
+ hostResponse.setZoneId(host.getZoneUuid());
+ hostResponse.setZoneName(host.getZoneName());
+ hostResponse.setPodId(host.getPodUuid());
+ hostResponse.setPodName(host.getPodName());
+ if (host.getClusterId() > 0) {
Review Comment:
`details=core` responses don’t set `clusterInternalId`.
`QueryManagerImpl.updateHostsExtensions()` relies on
`HostResponse.getClusterInternalId()` for External hypervisor hosts; leaving it
at the default 0 can prevent extension enrichment (or look up the wrong
cluster).
##########
server/src/main/java/com/cloud/api/query/dao/HostJoinDaoImpl.java:
##########
@@ -117,6 +117,51 @@ private boolean containsHostHATag(final String tags) {
return result;
}
+ private void setNewCoreHostResponse(HostJoinVO host, HostResponse
hostResponse) {
+ hostResponse.setId(host.getUuid());
+ hostResponse.setName(host.getName());
+ hostResponse.setState(host.getStatus());
+ hostResponse.setResourceState(host.getResourceState().toString());
+ hostResponse.setDisconnectedOn(host.getDisconnectedOn());
+ hostResponse.setHostType(host.getType());
+ hostResponse.setIpAddress(host.getPrivateIpAddress());
+ hostResponse.setVersion(host.getVersion());
+ hostResponse.setCreated(host.getCreated());
+ hostResponse.setRemoved(host.getRemoved());
+ hostResponse.setLastPinged(new Date(host.getLastPinged()));
+ if (host.getHypervisorType() != null) {
+
hostResponse.setHypervisor(host.getHypervisorType().getHypervisorDisplayName());
+ }
+
+ hostResponse.setZoneId(host.getZoneUuid());
+ hostResponse.setZoneName(host.getZoneName());
+ hostResponse.setPodId(host.getPodUuid());
+ hostResponse.setPodName(host.getPodName());
+ if (host.getClusterId() > 0) {
+ hostResponse.setClusterId(host.getClusterUuid());
+ hostResponse.setClusterName(host.getClusterName());
+ if (host.getClusterType() != null) {
+ hostResponse.setClusterType(host.getClusterType().toString());
+ }
+ }
+
+ String hostTags = host.getTag();
+ hostResponse.setHostTags(hostTags);
+ hostResponse.setExplicitHostTags(host.getExplicitTag());
+ hostResponse.setImplicitHostTags(host.getImplicitTag());
+ hostResponse.setStorageAccessGroups(host.getStorageAccessGroups());
+
hostResponse.setClusterStorageAccessGroups(host.getClusterStorageAccessGroups());
+
hostResponse.setPodStorageAccessGroups(host.getPodStorageAccessGroups());
+
hostResponse.setZoneStorageAccessGroups(host.getZoneStorageAccessGroups());
+
+ // msid is returned as-is; callers resolve it to avoid a per-host
lookup
+ if (host.getManagementServerId() != null) {
+
hostResponse.setManagementServerId(host.getManagementServerId().toString());
+ }
Review Comment:
In `details=core` you’re setting `managementserverid` to the host’s raw MSID
(`mgmt_server_id`) which is not the UUID normally returned by `listHosts` (the
non-core path resolves it via `mshost.uuid`). Returning a different identifier
type under an optional detail flag is likely to confuse API consumers; better
to omit this field in core mode unless you can return the UUID without per-host
lookups.
##########
server/src/main/java/com/cloud/api/query/dao/HostJoinDaoImpl.java:
##########
@@ -334,7 +379,11 @@ public HostResponse newMinimalHostResponse(HostJoinVO
host) {
@Override
public HostResponse newHostResponse(HostJoinVO host, EnumSet<HostDetails>
details) {
HostResponse hostResponse = new HostResponse();
- setNewHostResponseBase(host, details, hostResponse);
+ if (details.contains(HostDetails.core)) {
+ setNewCoreHostResponse(host, hostResponse);
+ } else {
+ setNewHostResponseBase(host, details, hostResponse);
+ }
Review Comment:
`HostDetails` is a multi-valued parameter (e.g. `details=capacity,stats`).
With the current check, any request that includes `core` alongside other flags
(including `all`) will silently drop the additional requested details because
the core path bypasses `setNewHostResponseBase`. Consider only using the
core-only response when `core` is the sole requested value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
