Skip to content

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

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.

Sign up for GitHub

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

Merged
yadvr merged 3 commits into from
Jun 5, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c711a39
Do not retrieve VM's stats on normal VM listing
JoaoJandre Mar 13, 2024
3fe1a0b
Add config to control the behavior
JoaoJandre May 3, 2024
e0f7462
address reviews
JoaoJandre May 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 28 additions & 20 deletions api/src/main/java/org/apache/cloudstack/api/command/user/vm/ListVMsCmd.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand All@@ -45,6 +46,7 @@
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;
Expand All@@ -58,7 +60,6 @@
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 /////////////////////
Expand DownExpand Up@@ -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")
Expand DownExpand Up@@ -239,22 +241,32 @@ public Long getAutoScaleVmGroupId() {
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()));
Comment thread
yadvr marked this conversation as resolved.
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
Expand All@@ -277,10 +289,6 @@ public Boolean getVnf() {
/////////////////////////////////////////////////////
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////
@Override
public String getCommandName() {
return s_name;
}

@Override
public ApiCommandResourceType getApiResourceType() {
Expand Down
16 changes: 16 additions & 0 deletions api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -37,6 +37,7 @@
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})
Expand DownExpand Up@@ -269,6 +270,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;
Comment thread
yadvr marked this conversation as resolved.

@SerializedName(ApiConstants.PUBLIC_IP_ID)
@Param(description = "public IP address id associated with vm via Static nat rule")
private String publicIpId;
Expand DownExpand Up@@ -623,6 +628,10 @@ public String getHypervisor() {
return hypervisor;
}

public String getIpAddress() {
return ipAddress;
}

public String getPublicIpId() {
return publicIpId;
}
Expand DownExpand Up@@ -859,6 +868,13 @@ public void setForVirtualNetwork(Boolean forVirtualNetwork) {

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) {
Expand Down
4 changes: 4 additions & 0 deletions api/src/main/java/org/apache/cloudstack/query/QueryService.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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",
Comment thread
yadvr marked this conversation as resolved.
Comment thread
yadvr marked this conversation as resolved.
"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;
Expand Down
13 changes: 9 additions & 4 deletions plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVMsMetricsCmd.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand All@@ -42,18 +44,21 @@
* 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
Expand Down
1 change: 0 additions & 1 deletion plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -617,7 +617,6 @@ public List<VmMetricsResponse> listVmMetrics(List<UserVmResponse> vmResponses) {
}

metricsResponse.setHasAnnotation(vmResponse.hasAnnotation());
metricsResponse.setIpAddress(vmResponse.getNics());
Comment thread
JoaoJandre marked this conversation as resolved.
metricsResponse.setCpuTotal(vmResponse.getCpuNumber(), vmResponse.getCpuSpeed());
metricsResponse.setMemTotal(vmResponse.getMemory());
metricsResponse.setNetworkRead(vmResponse.getNetworkKbsRead());
Expand Down
11 changes: 0 additions & 11 deletions plugins/metrics/src/main/java/org/apache/cloudstack/response/VmMetricsResponse.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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")
Expand DownExpand Up@@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -5250,6 +5250,6 @@ public String getConfigComponentName() {
@Override
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[] {AllowUserViewDestroyedVM, UserVMDeniedDetails, UserVMReadOnlyDetails, SortKeyAscending,
AllowUserViewAllDomainAccounts, SharePublicTemplatesWithOtherDomains};
AllowUserViewAllDomainAccounts, SharePublicTemplatesWithOtherDomains, ReturnVmStatsOnVmList};
}
}
1 change: 1 addition & 0 deletions server/src/main/java/com/cloud/api/query/ViewResponseHelper.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -166,6 +166,7 @@ public static List<UserVmResponse> createUserVmResponse(ResponseView view, Strin
// 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());
Expand Down
1 change: 1 addition & 0 deletions ui/src/config/router.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 : {},
Expand Down
3 changes: 2 additions & 1 deletion ui/src/config/section/compute.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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',
Comment thread
yadvr marked this conversation as resolved.
resourceType: 'UserVm',
params: () => {
var params = { details: 'servoff,tmpl,nics,backoff' }
Expand Down
2 changes: 1 addition & 1 deletion ui/src/views/AutogenView.vue
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -816,7 +816,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') {
Expand Down