Do not retrieve VM's stats on normal VM listing (#8782)

* Do not retrieve VM's stats on normal VM listing * Add config to control the behavior * address reviews
2025-10-26 08:42:29 +01:00 · 2024-06-05 09:15:28 -03:00 · 2024-06-05 09:15:28 -03:00 · 631d6ad09b
commit 631d6ad09b
parent 68a231aaee
11 changed files with 63 additions and 39 deletions
--- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/ListVMsCmd.java
+++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/ListVMsCmd.java
@ -16,9 +16,10 @@
 // under the License.
 package org.apache.cloudstack.api.command.user.vm;

-import java.util.ArrayList;
 import java.util.EnumSet;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;

 import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.affinity.AffinityGroupResponse;
@ -45,6 +46,7 @@ import org.apache.cloudstack.api.response.UserVmResponse;
 import org.apache.cloudstack.api.response.VpcResponse;
 import org.apache.cloudstack.api.response.ZoneResponse;
 import org.apache.commons.lang3.BooleanUtils;
+import org.apache.commons.collections.CollectionUtils;
 import org.apache.log4j.Logger;

 import com.cloud.exception.InvalidParameterValueException;
@ -58,7 +60,6 @@ import com.cloud.vm.VirtualMachine;
 public class ListVMsCmd extends BaseListRetrieveOnlyResourceCountCmd implements UserCmd {
    public static final Logger s_logger = Logger.getLogger(ListVMsCmd.class.getName());

-    private static final String s_name = "listvirtualmachinesresponse";

    /////////////////////////////////////////////////////
    //////////////// API parameters /////////////////////
@ -98,7 +99,8 @@ public class ListVMsCmd extends BaseListRetrieveOnlyResourceCountCmd implements
               collectionType = CommandType.STRING,
               description = "comma separated list of vm details requested, "
                   + "value can be a list of [all, group, nics, stats, secgrp, tmpl, servoff, diskoff, backoff, iso, volume, min, affgrp]."
-                   + " If no parameter is passed in, the details will be defaulted to all")
+                   + " If no parameter is passed in, the details will be defaulted to all. When return.vm.stats.on.vm.list is true, the default" +
+                       "details change to [group, nics, secgrp, tmpl, servoff, diskoff, backoff, iso, volume, min, affgrp], thus the stats will not be returned. ")
    private List<String> viewDetails;

    @Parameter(name = ApiConstants.TEMPLATE_ID, type = CommandType.UUID, entityType = TemplateResponse.class, description = "list vms by template")
@ -239,22 +241,32 @@ public class ListVMsCmd extends BaseListRetrieveOnlyResourceCountCmd implements
        return autoScaleVmGroupId;
    }

+    protected boolean isViewDetailsEmpty() {
+        return CollectionUtils.isEmpty(viewDetails);
+    }
+
    public EnumSet<VMDetails> getDetails() throws InvalidParameterValueException {
-        EnumSet<VMDetails> dv;
-        if (viewDetails == null || viewDetails.size() <= 0) {
-            dv = EnumSet.of(VMDetails.all);
-        } else {
-            try {
-                ArrayList<VMDetails> dc = new ArrayList<VMDetails>();
-                for (String detail : viewDetails) {
-                    dc.add(VMDetails.valueOf(detail));
-                }
-                dv = EnumSet.copyOf(dc);
-            } catch (IllegalArgumentException e) {
-                throw new InvalidParameterValueException("The details parameter contains a non permitted value. The allowed values are " + EnumSet.allOf(VMDetails.class));
+        if (isViewDetailsEmpty()) {
+            if (_queryService.ReturnVmStatsOnVmList.value()) {
+                return EnumSet.of(VMDetails.all);
            }
+
+            Set<VMDetails> allDetails = new HashSet<>(Set.of(VMDetails.values()));
+            allDetails.remove(VMDetails.stats);
+            allDetails.remove(VMDetails.all);
+            return EnumSet.copyOf(allDetails);
+        }
+
+        try {
+            Set<VMDetails> dc = new HashSet<>();
+            for (String detail : viewDetails) {
+                dc.add(VMDetails.valueOf(detail));
+            }
+
+            return EnumSet.copyOf(dc);
+        } catch (IllegalArgumentException e) {
+            throw new InvalidParameterValueException("The details parameter contains a non permitted value. The allowed values are " + EnumSet.allOf(VMDetails.class));
        }
-        return dv;
    }

    @Override
@ -277,10 +289,6 @@ public class ListVMsCmd extends BaseListRetrieveOnlyResourceCountCmd implements
    /////////////////////////////////////////////////////
    /////////////// API Implementation///////////////////
    /////////////////////////////////////////////////////
-    @Override
-    public String getCommandName() {
-        return s_name;
-    }

    @Override
    public ApiCommandResourceType getApiResourceType() {
--- a/api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java
+++ b/api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java
@ -37,6 +37,7 @@ import com.cloud.serializer.Param;
 import com.cloud.uservm.UserVm;
 import com.cloud.vm.VirtualMachine;
 import com.google.gson.annotations.SerializedName;
+import org.apache.commons.collections.CollectionUtils;

@SuppressWarnings("unused")
@EntityReference(value = {VirtualMachine.class, UserVm.class, VirtualRouter.class})
@ -273,6 +274,10 @@ public class UserVmResponse extends BaseResponseWithTagInformation implements Co
    @Param(description = "the hypervisor on which the template runs")
    private String hypervisor;

+    @SerializedName(ApiConstants.IP_ADDRESS)
+    @Param(description = "the VM's primary IP address")
+    private String ipAddress;
+
    @SerializedName(ApiConstants.PUBLIC_IP_ID)
    @Param(description = "public IP address id associated with vm via Static nat rule")
    private String publicIpId;
@ -627,6 +632,10 @@ public class UserVmResponse extends BaseResponseWithTagInformation implements Co
        return hypervisor;
    }

+    public String getIpAddress() {
+        return ipAddress;
+    }
+
    public String getPublicIpId() {
        return publicIpId;
    }
@ -863,6 +872,13 @@ public class UserVmResponse extends BaseResponseWithTagInformation implements Co

    public void setNics(Set<NicResponse> nics) {
        this.nics = nics;
+        setIpAddress(nics);
+    }
+
+    public void setIpAddress(final Set<NicResponse> nics) {
+        if (CollectionUtils.isNotEmpty(nics)) {
+            this.ipAddress = nics.iterator().next().getIpaddress();
+        }
    }

    public void addNic(NicResponse nic) {
--- a/api/src/main/java/org/apache/cloudstack/query/QueryService.java
+++ b/api/src/main/java/org/apache/cloudstack/query/QueryService.java
@ -125,6 +125,10 @@ public interface QueryService {
    static final ConfigKey<Boolean> SharePublicTemplatesWithOtherDomains = new ConfigKey<>("Advanced", Boolean.class, "share.public.templates.with.other.domains", "true",
            "If false, templates of this domain will not show up in the list templates of other domains.", true, ConfigKey.Scope.Domain);

+    ConfigKey<Boolean> ReturnVmStatsOnVmList = new ConfigKey<>("Advanced", Boolean.class, "return.vm.stats.on.vm.list", "true",
+            "If false, changes the listVirtualMachines default details to [group, nics, secgrp, tmpl, servoff, diskoff, backoff, iso, volume, min, affgrp], so that the VMs' stats" +
+                    " are not returned by default when listing VMs; only when the 'stats' or 'all' detail is informed.", true, ConfigKey.Scope.Global);
+
    ListResponse<UserResponse> searchForUsers(ListUsersCmd cmd) throws PermissionDeniedException;

    ListResponse<UserResponse> searchForUsers(Long domainId, boolean recursive) throws PermissionDeniedException;
--- a/plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVMsMetricsCmd.java
+++ b/plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVMsMetricsCmd.java
@ -17,10 +17,12 @@

 package org.apache.cloudstack.api;

+import java.util.EnumSet;
 import java.util.List;

 import javax.inject.Inject;

+import com.cloud.exception.InvalidParameterValueException;
 import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.api.command.user.UserCmd;
 import org.apache.cloudstack.api.command.user.vm.ListVMsCmd;
@ -42,18 +44,21 @@ import org.apache.cloudstack.response.VmMetricsResponse;
 *     although most of it is not suitable/useful for the API purpose.</li>
 * </ul>
 */
-@APICommand(name = ListVMsMetricsCmd.APINAME, description = "Lists VM metrics", responseObject = VmMetricsResponse.class,
+@APICommand(name = "listVirtualMachinesMetrics", description = "Lists VM metrics", responseObject = VmMetricsResponse.class,
        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,  responseView = ResponseObject.ResponseView.Restricted,
        since = "4.9.3", authorized = {RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User})
 public class ListVMsMetricsCmd extends ListVMsCmd implements UserCmd {
-    public static final String APINAME = "listVirtualMachinesMetrics";

    @Inject
    private MetricsService metricsService;

    @Override
-    public String getCommandName() {
-        return APINAME.toLowerCase() + BaseCmd.RESPONSE_SUFFIX;
+    public EnumSet<ApiConstants.VMDetails> getDetails() throws InvalidParameterValueException {
+        if (isViewDetailsEmpty()) {
+            return EnumSet.of(ApiConstants.VMDetails.all);
+        }
+
+        return super.getDetails();
    }

    @Override
--- a/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java
+++ b/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java
@ -619,7 +619,6 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements
            }

            metricsResponse.setHasAnnotation(vmResponse.hasAnnotation());
-            metricsResponse.setIpAddress(vmResponse.getNics());
            metricsResponse.setCpuTotal(vmResponse.getCpuNumber(), vmResponse.getCpuSpeed());
            metricsResponse.setMemTotal(vmResponse.getMemory());
            metricsResponse.setNetworkRead(vmResponse.getNetworkKbsRead());
--- a/plugins/metrics/src/main/java/org/apache/cloudstack/response/VmMetricsResponse.java
+++ b/plugins/metrics/src/main/java/org/apache/cloudstack/response/VmMetricsResponse.java
@ -17,19 +17,13 @@

 package org.apache.cloudstack.response;

-import java.util.Set;
-
 import org.apache.cloudstack.api.ApiConstants;
-import org.apache.cloudstack.api.response.NicResponse;
 import org.apache.cloudstack.api.response.UserVmResponse;

 import com.cloud.serializer.Param;
 import com.google.gson.annotations.SerializedName;

 public class VmMetricsResponse extends UserVmResponse {
-    @SerializedName(ApiConstants.IP_ADDRESS)
-    @Param(description = "the VM's primary IP address")
-    private String ipAddress;

    @SerializedName("cputotal")
    @Param(description = "the total cpu capacity in Ghz")
@ -59,11 +53,6 @@ public class VmMetricsResponse extends UserVmResponse {
    @Param(description = "the total disk iops")
    private Long diskIopsTotal;

-    public void setIpAddress(final Set<NicResponse> nics) {
-        if (nics != null && nics.size() > 0) {
-            this.ipAddress = nics.iterator().next().getIpaddress();
-        }
-    }

    public void setCpuTotal(final Integer cpuNumber, final Integer cpuSpeed) {
        if (cpuNumber != null && cpuSpeed != null) {
--- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
+++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
@ -5686,6 +5686,6 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q
    @Override
    public ConfigKey<?>[] getConfigKeys() {
        return new ConfigKey<?>[] {AllowUserViewDestroyedVM, UserVMDeniedDetails, UserVMReadOnlyDetails, SortKeyAscending,
-                AllowUserViewAllDomainAccounts, SharePublicTemplatesWithOtherDomains};
+                AllowUserViewAllDomainAccounts, SharePublicTemplatesWithOtherDomains, ReturnVmStatsOnVmList};
    }
 }
--- a/server/src/main/java/com/cloud/api/query/ViewResponseHelper.java
+++ b/server/src/main/java/com/cloud/api/query/ViewResponseHelper.java
@ -166,6 +166,7 @@ public class ViewResponseHelper {
                // update nics, securitygroups, tags, affinitygroups for 1 to many mapping fields
                userVmData = ApiDBUtils.fillVmDetails(view, userVmData, userVm);
            }
+            userVmData.setIpAddress(userVmData.getNics());
            vmDataList.put(userVm.getId(), userVmData);
        }
        return new ArrayList<UserVmResponse>(vmDataList.values());
--- a/ui/src/config/router.js
+++ b/ui/src/config/router.js
@ -75,6 +75,7 @@ function generateRouterMap (section) {
          icon: child.icon,
          docHelp: vueProps.$applyDocHelpMappings(child.docHelp),
          permission: child.permission,
+          getApiToCall: child.getApiToCall,
          resourceType: child.resourceType,
          filters: child.filters,
          params: child.params ? child.params : {},
--- a/ui/src/config/section/compute.js
+++ b/ui/src/config/section/compute.js
@ -28,7 +28,8 @@ export default {
      title: 'label.instances',
      icon: 'cloud-server-outlined',
      docHelp: 'adminguide/virtual_machines.html',
-      permission: ['listVirtualMachinesMetrics'],
+      permission: ['listVirtualMachines', 'listVirtualMachinesMetrics'],
+      getApiToCall: () => store.getters.metrics ? 'listVirtualMachinesMetrics' : 'listVirtualMachines',
      resourceType: 'UserVm',
      params: () => {
        var params = { details: 'servoff,tmpl,iso,nics,backoff' }
--- a/ui/src/views/AutogenView.vue
+++ b/ui/src/views/AutogenView.vue
@ -819,7 +819,7 @@ export default {
      }

      if (this.$route && this.$route.meta && this.$route.meta.permission) {
-        this.apiName = this.$route.meta.permission[0]
+        this.apiName = (this.$route.meta.getApiToCall && this.$route.meta.getApiToCall()) || this.$route.meta.permission[0]
        if (this.$route.meta.columns) {
          const columns = this.$route.meta.columns
          if (columns && typeof columns === 'function') {