From b301e1479968c2b7799d296a3fe4d1d136a57ac9 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 9 Sep 2021 14:12:40 +0530 Subject: [PATCH 01/14] resource limit: Fix resource limit check on VM start --- server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index d3a67a08865c..d6d9fc710904 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -4927,10 +4927,10 @@ public Pair> startVirtualMach if (owner.getState() == Account.State.disabled) { throw new PermissionDeniedException("The owner of " + vm + " is disabled: " + vm.getAccountId()); } - if (VirtualMachineManager.ResoureCountRunningVMsonly.value()) { + if (!VirtualMachineManager.ResoureCountRunningVMsonly.value()) { // check if account/domain is with in resource limits to start a new vm ServiceOfferingVO offering = _serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId()); - resourceLimitCheck(owner, vm.isDisplayVm(), new Long(offering.getCpu()), new Long(offering.getRamSize())); + resourceLimitCheck(owner, vm.isDisplayVm(), Long.valueOf(offering.getCpu()), Long.valueOf(offering.getRamSize())); } // check if vm is security group enabled From 729f14bfd389f262f38a1d41389ab391c573ba18 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 9 Sep 2021 17:47:05 +0530 Subject: [PATCH 02/14] refactor --- server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index d6d9fc710904..6a223a54430b 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -4927,11 +4927,10 @@ public Pair> startVirtualMach if (owner.getState() == Account.State.disabled) { throw new PermissionDeniedException("The owner of " + vm + " is disabled: " + vm.getAccountId()); } - if (!VirtualMachineManager.ResoureCountRunningVMsonly.value()) { - // check if account/domain is with in resource limits to start a new vm - ServiceOfferingVO offering = _serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId()); - resourceLimitCheck(owner, vm.isDisplayVm(), Long.valueOf(offering.getCpu()), Long.valueOf(offering.getRamSize())); - } + + // check if account/domain is with in resource limits to start a new vm + ServiceOfferingVO offering = _serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId()); + resourceLimitCheck(owner, vm.isDisplayVm(), Long.valueOf(offering.getCpu()), Long.valueOf(offering.getRamSize())); // check if vm is security group enabled if (_securityGroupMgr.isVmSecurityGroupEnabled(vmId) && _securityGroupMgr.getSecurityGroupsForVm(vmId).isEmpty() From 768d5d91af077e9946664a9a3d495cd79f39f635 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Mon, 13 Sep 2021 14:23:56 +0530 Subject: [PATCH 03/14] refactor logic --- .../api/command/user/vm/UpdateVMCmd.java | 19 ++++--- .../java/com/cloud/vm/UserVmManagerImpl.java | 49 ++++++++++++++++--- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java index 38289537bc64..6df85d03aa5f 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java @@ -21,6 +21,7 @@ import java.util.List; import java.util.Map; +import com.cloud.utils.exception.CloudRuntimeException; import org.apache.log4j.Logger; import org.apache.cloudstack.acl.RoleType; @@ -263,13 +264,17 @@ public long getEntityOwnerId() { @Override public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException { CallContext.current().setEventDetails("Vm Id: " + this._uuidMgr.getUuid(VirtualMachine.class, getId())); - UserVm result = _userVmService.updateVirtualMachine(this); - if (result != null){ - UserVmResponse response = _responseGenerator.createUserVmResponse(getResponseView(), "virtualmachine", result).get(0); - response.setResponseName(getCommandName()); - setResponseObject(response); - } else { - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to update vm"); + try { + UserVm result = _userVmService.updateVirtualMachine(this); + if (result != null) { + UserVmResponse response = _responseGenerator.createUserVmResponse(getResponseView(), "virtualmachine", result).get(0); + response.setResponseName(getCommandName()); + setResponseObject(response); + } else { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to update vm"); + } + } catch (Exception e) { + throw new CloudRuntimeException("Failed to update VM", e); } } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 6a223a54430b..0e3e6a23e88e 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -633,6 +633,12 @@ protected void resourceCountDecrement(long accountId, Boolean displayVm, Long cp } } + protected void updateResourceCount(long accountId, long domainId) { + _resourceLimitMgr.recalculateResourceCount(accountId, domainId, ResourceType.user_vm.getOrdinal()); + _resourceLimitMgr.recalculateResourceCount(accountId, domainId, ResourceType.cpu.getOrdinal()); + _resourceLimitMgr.recalculateResourceCount(accountId, domainId, ResourceType.memory.getOrdinal()); + } + public class VmAndCountDetails { long vmId; int retrievalCount = VmIpFetchTrialMax.value(); @@ -2575,6 +2581,38 @@ protected void runInContext() { } } + private void verifyVmLimits(UserVmVO vmInstance, Map details) { + Long newCpu = Long.valueOf(details.get(VmDetailConstants.CPU_NUMBER)); + Long newMemory = Long.valueOf(details.get(VmDetailConstants.MEMORY)); + long currentCpu = 0L; + long currentMemory = 0L; + List vmDetails = userVmDetailsDao.listDetails(vmInstance.getId()); + for (UserVmDetailVO detailVO : vmDetails) { + if (VmDetailConstants.CPU_NUMBER.equals(detailVO.getName())) { + currentCpu = Long.parseLong(detailVO.getValue()); + } + if (VmDetailConstants.MEMORY.equals(detailVO.getName())) { + currentMemory = Long.parseLong(detailVO.getValue()); + } + } + Account owner = _accountDao.findById(vmInstance.getAccountId()); + if (owner == null) { + throw new InvalidParameterValueException("The owner of " + vmInstance + " does not exist: " + vmInstance.getAccountId()); + } + if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) { + try { + if (newCpu > currentCpu) { + _resourceLimitMgr.checkResourceLimit(owner, ResourceType.cpu, newCpu - currentCpu); + } + if (newMemory > currentMemory) { + _resourceLimitMgr.checkResourceLimit(owner, ResourceType.memory, newMemory - currentMemory); + } + } catch (ResourceAllocationException e) { + throw new CloudRuntimeException("Failed to update VM settings", e); + } + } + } + @Override @ActionEvent(eventType = EventTypes.EVENT_VM_UPDATE, eventDescription = "updating Vm") public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableException, InsufficientCapacityException { @@ -2648,6 +2686,8 @@ public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableEx } } } + + verifyVmLimits(vmInstance, details); vmInstance.setDetails(details); _vmDao.saveDetails(vmInstance); } @@ -2670,9 +2710,9 @@ protected void updateDisplayVmFlag(Boolean isDisplayVm, Long id, UserVmVO vmInst // Resource limit changes ServiceOffering offering = _serviceOfferingDao.findByIdIncludingRemoved(vmInstance.getId(), vmInstance.getServiceOfferingId()); if (isDisplayVm) { - resourceCountIncrement(vmInstance.getAccountId(), true, new Long(offering.getCpu()), new Long(offering.getRamSize())); + resourceCountIncrement(vmInstance.getAccountId(), true, Long.valueOf(offering.getCpu()), Long.valueOf(offering.getRamSize())); } else { - resourceCountDecrement(vmInstance.getAccountId(), true, new Long(offering.getCpu()), new Long(offering.getRamSize())); + resourceCountDecrement(vmInstance.getAccountId(), true, Long.valueOf(offering.getCpu()), Long.valueOf(offering.getRamSize())); } // Usage @@ -4928,10 +4968,6 @@ public Pair> startVirtualMach throw new PermissionDeniedException("The owner of " + vm + " is disabled: " + vm.getAccountId()); } - // check if account/domain is with in resource limits to start a new vm - ServiceOfferingVO offering = _serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId()); - resourceLimitCheck(owner, vm.isDisplayVm(), Long.valueOf(offering.getCpu()), Long.valueOf(offering.getRamSize())); - // check if vm is security group enabled if (_securityGroupMgr.isVmSecurityGroupEnabled(vmId) && _securityGroupMgr.getSecurityGroupsForVm(vmId).isEmpty() && !_securityGroupMgr.isVmMappedToDefaultSecurityGroup(vmId) && _networkModel.canAddDefaultSecurityGroup()) { @@ -5055,6 +5091,7 @@ public Pair> startVirtualMach } } + updateResourceCount(owner.getAccountId(), owner.getDomainId()); return vmParamPair; } From 9a0ebda31b5b8a1406fd970ee53a1d5d21f7fa57 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Mon, 13 Sep 2021 17:55:08 +0530 Subject: [PATCH 04/14] address comments --- .../entity/api/VirtualMachineEntityImpl.java | 2 +- .../java/com/cloud/vm/UserVmManagerImpl.java | 52 ++++++++++++------- .../com/cloud/vm/UserVmManagerImplTest.java | 5 ++ 3 files changed, 38 insertions(+), 21 deletions(-) diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntityImpl.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntityImpl.java index 3e5f910fa8a1..a709173a072a 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntityImpl.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntityImpl.java @@ -197,7 +197,7 @@ public void delTag() { } @Override - public String reserve(DeploymentPlanner plannerToUse, DeploymentPlan plan, ExcludeList exclude, String caller) throws InsufficientCapacityException, + public String reserve(DeploymentPlanner plannerToUse, DeploymentPlan plan, ExcludeList exclude, String caller) throws InsufficientCapacityException, ResourceUnavailableException { return manager.reserveVirtualMachine(this.vmEntityVO, plannerToUse, plan, exclude); } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 0e3e6a23e88e..e39026da65d9 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -633,12 +633,6 @@ protected void resourceCountDecrement(long accountId, Boolean displayVm, Long cp } } - protected void updateResourceCount(long accountId, long domainId) { - _resourceLimitMgr.recalculateResourceCount(accountId, domainId, ResourceType.user_vm.getOrdinal()); - _resourceLimitMgr.recalculateResourceCount(accountId, domainId, ResourceType.cpu.getOrdinal()); - _resourceLimitMgr.recalculateResourceCount(accountId, domainId, ResourceType.memory.getOrdinal()); - } - public class VmAndCountDetails { long vmId; int retrievalCount = VmIpFetchTrialMax.value(); @@ -2582,8 +2576,8 @@ protected void runInContext() { } private void verifyVmLimits(UserVmVO vmInstance, Map details) { - Long newCpu = Long.valueOf(details.get(VmDetailConstants.CPU_NUMBER)); - Long newMemory = Long.valueOf(details.get(VmDetailConstants.MEMORY)); + Long newCpu = Long.valueOf(details.get(VmDetailConstants.CPU_NUMBER) != null ? details.get(VmDetailConstants.CPU_NUMBER) : "0"); + Long newMemory = Long.valueOf(details.get(VmDetailConstants.MEMORY) != null ? details.get(VmDetailConstants.MEMORY) : "0"); long currentCpu = 0L; long currentMemory = 0L; List vmDetails = userVmDetailsDao.listDetails(vmInstance.getId()); @@ -2599,18 +2593,33 @@ private void verifyVmLimits(UserVmVO vmInstance, Map details) { if (owner == null) { throw new InvalidParameterValueException("The owner of " + vmInstance + " does not exist: " + vmInstance.getAccountId()); } - if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) { - try { - if (newCpu > currentCpu) { - _resourceLimitMgr.checkResourceLimit(owner, ResourceType.cpu, newCpu - currentCpu); - } - if (newMemory > currentMemory) { - _resourceLimitMgr.checkResourceLimit(owner, ResourceType.memory, newMemory - currentMemory); - } - } catch (ResourceAllocationException e) { - throw new CloudRuntimeException("Failed to update VM settings", e); + + if (VirtualMachineManager.ResoureCountRunningVMsonly.value()) { + return; + } + + try { + if (newCpu > currentCpu) { + _resourceLimitMgr.checkResourceLimit(owner, ResourceType.cpu, newCpu - currentCpu); } + if (newMemory > currentMemory) { + _resourceLimitMgr.checkResourceLimit(owner, ResourceType.memory, newMemory - currentMemory); + } + } catch (ResourceAllocationException e) { + throw new CloudRuntimeException("Failed to update VM settings", e); + } + + if (newCpu > currentCpu) { + _resourceLimitMgr.incrementResourceCount(owner.getAccountId(), ResourceType.cpu, newCpu - currentCpu); + } else { + _resourceLimitMgr.decrementResourceCount(owner.getAccountId(), ResourceType.cpu, currentCpu - newCpu); } + if (newMemory > currentMemory) { + _resourceLimitMgr.incrementResourceCount(owner.getAccountId(), ResourceType.memory, newMemory - currentMemory); + } else { + _resourceLimitMgr.decrementResourceCount(owner.getAccountId(), ResourceType.memory, currentMemory - newMemory); + } + } @Override @@ -4967,7 +4976,11 @@ public Pair> startVirtualMach if (owner.getState() == Account.State.disabled) { throw new PermissionDeniedException("The owner of " + vm + " is disabled: " + vm.getAccountId()); } - + if (VirtualMachineManager.ResoureCountRunningVMsonly.value()) { + // check if account/domain is with in resource limits to start a new vm + ServiceOfferingVO offering = _serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId()); + resourceLimitCheck(owner, vm.isDisplayVm(), Long.valueOf(offering.getCpu()), Long.valueOf(offering.getRamSize())); + } // check if vm is security group enabled if (_securityGroupMgr.isVmSecurityGroupEnabled(vmId) && _securityGroupMgr.getSecurityGroupsForVm(vmId).isEmpty() && !_securityGroupMgr.isVmMappedToDefaultSecurityGroup(vmId) && _networkModel.canAddDefaultSecurityGroup()) { @@ -5091,7 +5104,6 @@ public Pair> startVirtualMach } } - updateResourceCount(owner.getAccountId(), owner.getDomainId()); return vmParamPair; } diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index da59f07a3a86..7a57d0408071 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -38,6 +38,7 @@ import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.VMTemplateDao; +import com.cloud.user.dao.AccountDao; import org.apache.cloudstack.api.BaseCmd.HTTPMethod; import org.apache.cloudstack.api.command.user.vm.UpdateVMCmd; import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd; @@ -145,6 +146,9 @@ public class UserVmManagerImplTest { @Mock private VMTemplateDao templateDao; + @Mock + private AccountDao accountDao; + private long vmId = 1l; private static final long GiB_TO_BYTES = 1024 * 1024 * 1024; @@ -280,6 +284,7 @@ public void updateVirtualMachineTestCleanUpFalseAndDetailsNotEmpty() throws Reso @Test public void updateVirtualMachineTestCleanUpFalseAndDetailsEmpty() throws ResourceUnavailableException, InsufficientCapacityException { + Mockito.when(accountDao.findById(Mockito.anyLong())).thenReturn(callerAccount); prepareAndExecuteMethodDealingWithDetails(false, false); } From 89cbd9b1e7c0980b442b7ddf7adcc7e278f18b15 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Mon, 13 Sep 2021 19:12:14 +0530 Subject: [PATCH 05/14] address comments --- .../entity/api/VirtualMachineEntityImpl.java | 2 +- .../java/com/cloud/vm/UserVmManagerImpl.java | 27 +++++++------------ .../com/cloud/vm/UserVmManagerImplTest.java | 9 ++++++- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntityImpl.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntityImpl.java index a709173a072a..3e5f910fa8a1 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntityImpl.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntityImpl.java @@ -197,7 +197,7 @@ public void delTag() { } @Override - public String reserve(DeploymentPlanner plannerToUse, DeploymentPlan plan, ExcludeList exclude, String caller) throws InsufficientCapacityException, + public String reserve(DeploymentPlanner plannerToUse, DeploymentPlan plan, ExcludeList exclude, String caller) throws InsufficientCapacityException, ResourceUnavailableException { return manager.reserveVirtualMachine(this.vmEntityVO, plannerToUse, plan, exclude); } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index e39026da65d9..5a5c76520c62 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -114,6 +114,7 @@ import org.apache.commons.codec.binary.Base64; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; +import org.apache.commons.lang.math.NumberUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.w3c.dom.Document; @@ -2576,28 +2577,20 @@ protected void runInContext() { } private void verifyVmLimits(UserVmVO vmInstance, Map details) { - Long newCpu = Long.valueOf(details.get(VmDetailConstants.CPU_NUMBER) != null ? details.get(VmDetailConstants.CPU_NUMBER) : "0"); - Long newMemory = Long.valueOf(details.get(VmDetailConstants.MEMORY) != null ? details.get(VmDetailConstants.MEMORY) : "0"); - long currentCpu = 0L; - long currentMemory = 0L; - List vmDetails = userVmDetailsDao.listDetails(vmInstance.getId()); - for (UserVmDetailVO detailVO : vmDetails) { - if (VmDetailConstants.CPU_NUMBER.equals(detailVO.getName())) { - currentCpu = Long.parseLong(detailVO.getValue()); - } - if (VmDetailConstants.MEMORY.equals(detailVO.getName())) { - currentMemory = Long.parseLong(detailVO.getValue()); - } + if (VirtualMachineManager.ResoureCountRunningVMsonly.value()) { + return; } + + long newCpu = NumberUtils.toLong(details.get(VmDetailConstants.CPU_NUMBER)); + long newMemory = NumberUtils.toLong(details.get(VmDetailConstants.MEMORY)); + ServiceOfferingVO currentServiceOffering = _offeringDao.findByIdIncludingRemoved(vmInstance.getId(), vmInstance.getServiceOfferingId()); + long currentCpu = currentServiceOffering.getCpu(); + long currentMemory = currentServiceOffering.getRamSize(); + Account owner = _accountDao.findById(vmInstance.getAccountId()); if (owner == null) { throw new InvalidParameterValueException("The owner of " + vmInstance + " does not exist: " + vmInstance.getAccountId()); } - - if (VirtualMachineManager.ResoureCountRunningVMsonly.value()) { - return; - } - try { if (newCpu > currentCpu) { _resourceLimitMgr.checkResourceLimit(owner, ResourceType.cpu, newCpu - currentCpu); diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 7a57d0408071..4c30682cdd49 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.anyString; @@ -32,12 +33,14 @@ import java.util.List; import java.util.Map; +import com.cloud.configuration.Resource; import com.cloud.hypervisor.Hypervisor; import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.VMTemplateDao; +import com.cloud.user.ResourceLimitService; import com.cloud.user.dao.AccountDao; import org.apache.cloudstack.api.BaseCmd.HTTPMethod; import org.apache.cloudstack.api.command.user.vm.UpdateVMCmd; @@ -149,6 +152,9 @@ public class UserVmManagerImplTest { @Mock private AccountDao accountDao; + @Mock + ResourceLimitService resourceLimitMgr; + private long vmId = 1l; private static final long GiB_TO_BYTES = 1024 * 1024 * 1024; @@ -171,6 +177,8 @@ public void beforeTest() { CallContext.register(callerUser, callerAccount); customParameters.put(VmDetailConstants.ROOT_DISK_SIZE, "123"); + lenient().doNothing().when(resourceLimitMgr).incrementResourceCount(anyLong(), any(Resource.ResourceType.class)); + lenient().doNothing().when(resourceLimitMgr).decrementResourceCount(anyLong(), any(Resource.ResourceType.class), anyLong()); } @After @@ -308,7 +316,6 @@ private void prepareAndExecuteMethodDealingWithDetails(boolean cleanUpDetails, b } Mockito.when(updateVmCommand.getDetails()).thenReturn(details); Mockito.when(updateVmCommand.isCleanupDetails()).thenReturn(cleanUpDetails); - configureDoNothingForDetailsMethod(); userVmManagerImpl.updateVirtualMachine(updateVmCommand); From 960f31f49f5428752c0eedf1c13c2ef7d66fdb92 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Mon, 13 Sep 2021 21:21:06 +0530 Subject: [PATCH 06/14] address comments --- server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 7 +++---- .../src/test/java/com/cloud/vm/UserVmManagerImplTest.java | 4 ++++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 5a5c76520c62..4661ddcfea66 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -2583,7 +2583,7 @@ private void verifyVmLimits(UserVmVO vmInstance, Map details) { long newCpu = NumberUtils.toLong(details.get(VmDetailConstants.CPU_NUMBER)); long newMemory = NumberUtils.toLong(details.get(VmDetailConstants.MEMORY)); - ServiceOfferingVO currentServiceOffering = _offeringDao.findByIdIncludingRemoved(vmInstance.getId(), vmInstance.getServiceOfferingId()); + ServiceOfferingVO currentServiceOffering = _serviceOfferingDao.findByIdIncludingRemoved(vmInstance.getId(), vmInstance.getServiceOfferingId()); long currentCpu = currentServiceOffering.getCpu(); long currentMemory = currentServiceOffering.getRamSize(); @@ -2604,15 +2604,14 @@ private void verifyVmLimits(UserVmVO vmInstance, Map details) { if (newCpu > currentCpu) { _resourceLimitMgr.incrementResourceCount(owner.getAccountId(), ResourceType.cpu, newCpu - currentCpu); - } else { + } else if (currentCpu > newCpu){ _resourceLimitMgr.decrementResourceCount(owner.getAccountId(), ResourceType.cpu, currentCpu - newCpu); } if (newMemory > currentMemory) { _resourceLimitMgr.incrementResourceCount(owner.getAccountId(), ResourceType.memory, newMemory - currentMemory); - } else { + } else if (currentMemory > newMemory){ _resourceLimitMgr.decrementResourceCount(owner.getAccountId(), ResourceType.memory, currentMemory - newMemory); } - } @Override diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 4c30682cdd49..a9821094f067 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -301,6 +301,10 @@ private void prepareAndExecuteMethodDealingWithDetails(boolean cleanUpDetails, b ServiceOffering offering = getSvcoffering(512); Mockito.when(_serviceOfferingDao.findById(Mockito.anyLong(), Mockito.anyLong())).thenReturn((ServiceOfferingVO) offering); + Mockito.when(_serviceOfferingDao.findByIdIncludingRemoved(Mockito.anyLong(), Mockito.anyLong())).thenReturn((ServiceOfferingVO) offering); + ServiceOfferingVO currentServiceOffering = Mockito.mock(ServiceOfferingVO.class); + Mockito.lenient().when(currentServiceOffering.getCpu()).thenReturn(1); + Mockito.lenient().when(currentServiceOffering.getRamSize()).thenReturn(512); List nics = new ArrayList<>(); NicVO nic1 = mock(NicVO.class); From 01c39ef863066ad27c0488d9cd23a86fe1bad66f Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Fri, 17 Sep 2021 13:40:24 +0530 Subject: [PATCH 07/14] add check to validate if cpu/memory are within limits for custom offering + exception handling --- .../api/command/user/vm/UpdateVMCmd.java | 21 ++++++++++--------- .../java/com/cloud/vm/UserVmManagerImpl.java | 8 +++++++ 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java index 6df85d03aa5f..60e2f076c3ae 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java @@ -264,17 +264,18 @@ public long getEntityOwnerId() { @Override public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException { CallContext.current().setEventDetails("Vm Id: " + this._uuidMgr.getUuid(VirtualMachine.class, getId())); + UserVm result = null; try { - UserVm result = _userVmService.updateVirtualMachine(this); - if (result != null) { - UserVmResponse response = _responseGenerator.createUserVmResponse(getResponseView(), "virtualmachine", result).get(0); - response.setResponseName(getCommandName()); - setResponseObject(response); - } else { - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to update vm"); - } - } catch (Exception e) { - throw new CloudRuntimeException("Failed to update VM", e); + result = _userVmService.updateVirtualMachine(this); + } catch (CloudRuntimeException e) { + throw new CloudRuntimeException(String.format("Failed to update VM, due to %s: ", e.getLocalizedMessage())); + } + if (result != null) { + UserVmResponse response = _responseGenerator.createUserVmResponse(getResponseView(), "virtualmachine", result).get(0); + response.setResponseName(getCommandName()); + setResponseObject(response); + } else { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to update vm"); } } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 1c967a7ed361..68e6cd618e01 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -2593,6 +2593,14 @@ private void verifyVmLimits(UserVmVO vmInstance, Map details) { long newCpu = NumberUtils.toLong(details.get(VmDetailConstants.CPU_NUMBER)); long newMemory = NumberUtils.toLong(details.get(VmDetailConstants.MEMORY)); ServiceOfferingVO currentServiceOffering = _serviceOfferingDao.findByIdIncludingRemoved(vmInstance.getId(), vmInstance.getServiceOfferingId()); + ServiceOfferingVO svcOffering = _serviceOfferingDao.findById(vmInstance.getServiceOfferingId()); + boolean isDynamic = svcOffering.isDynamic(); + if (isDynamic) { + Map customParameters = new HashMap<>(); + customParameters.put(VmDetailConstants.CPU_NUMBER, String.valueOf(newCpu)); + customParameters.put(VmDetailConstants.MEMORY, String.valueOf(newMemory)); + validateCustomParameters(svcOffering, customParameters); + } long currentCpu = currentServiceOffering.getCpu(); long currentMemory = currentServiceOffering.getRamSize(); From 3f5959133565a1ece237f7249bf1ceda62375ea3 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Fri, 17 Sep 2021 14:45:52 +0530 Subject: [PATCH 08/14] unit test fix --- .../apache/cloudstack/api/command/user/vm/UpdateVMCmd.java | 2 +- server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java index 60e2f076c3ae..fc29268e775e 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java @@ -268,7 +268,7 @@ public void execute() throws ResourceUnavailableException, InsufficientCapacityE try { result = _userVmService.updateVirtualMachine(this); } catch (CloudRuntimeException e) { - throw new CloudRuntimeException(String.format("Failed to update VM, due to %s: ", e.getLocalizedMessage())); + throw new CloudRuntimeException(String.format("Failed to update VM, due to: %s", e.getLocalizedMessage())); } if (result != null) { UserVmResponse response = _responseGenerator.createUserVmResponse(getResponseView(), "virtualmachine", result).get(0); diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 68e6cd618e01..6784ffdbaf1f 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -2594,7 +2594,7 @@ private void verifyVmLimits(UserVmVO vmInstance, Map details) { long newMemory = NumberUtils.toLong(details.get(VmDetailConstants.MEMORY)); ServiceOfferingVO currentServiceOffering = _serviceOfferingDao.findByIdIncludingRemoved(vmInstance.getId(), vmInstance.getServiceOfferingId()); ServiceOfferingVO svcOffering = _serviceOfferingDao.findById(vmInstance.getServiceOfferingId()); - boolean isDynamic = svcOffering.isDynamic(); + boolean isDynamic = currentServiceOffering.isDynamic(); if (isDynamic) { Map customParameters = new HashMap<>(); customParameters.put(VmDetailConstants.CPU_NUMBER, String.valueOf(newCpu)); @@ -2616,7 +2616,8 @@ private void verifyVmLimits(UserVmVO vmInstance, Map details) { _resourceLimitMgr.checkResourceLimit(owner, ResourceType.memory, newMemory - currentMemory); } } catch (ResourceAllocationException e) { - throw new CloudRuntimeException("Failed to update VM settings", e); + s_logger.error(String.format("Failed to updated VM due to: %s", e.getLocalizedMessage())); + throw new InvalidParameterValueException(e.getLocalizedMessage()); } if (newCpu > currentCpu) { From 77f7e7277971f8ca006891d363e6ef794798c572 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Fri, 17 Sep 2021 16:56:32 +0530 Subject: [PATCH 09/14] fix type in global setting name --- .../com/cloud/vm/VirtualMachineManager.java | 2 +- .../cloud/vm/VirtualMachineManagerImpl.java | 10 +++--- .../ResourceLimitManagerImpl.java | 6 ++-- .../java/com/cloud/vm/UserVmManagerImpl.java | 35 ++++++++++--------- 4 files changed, 27 insertions(+), 26 deletions(-) diff --git a/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java b/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java index c192f876aac5..f02395e0d44e 100644 --- a/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java +++ b/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java @@ -70,7 +70,7 @@ public interface VirtualMachineManager extends Manager { ConfigKey VmConfigDriveForceHostCacheUse = new ConfigKey<>("Advanced", Boolean.class, "vm.configdrive.force.host.cache.use", "false", "If true, config drive is forced to create on the host cache storage. Currently only supported for KVM.", true, ConfigKey.Scope.Zone); - ConfigKey ResoureCountRunningVMsonly = new ConfigKey("Advanced", Boolean.class, "resource.count.running.vms.only", "false", + ConfigKey ResourceCountRunningVMsonly = new ConfigKey("Advanced", Boolean.class, "resource.count.running.vms.only", "false", "Count the resources of only running VMs in resource limitation.", true); ConfigKey AllowExposeHypervisorHostnameAccountLevel = new ConfigKey("Advanced", Boolean.class, "account.allow.expose.host.hostname", diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 69c5cc6e2788..6314c448b7da 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1043,9 +1043,9 @@ public void orchestrateStart(final String vmUuid, final Map[] getConfigKeys() { return new ConfigKey[] { ClusterDeltaSyncInterval, StartRetry, VmDestroyForcestop, VmOpCancelInterval, VmOpCleanupInterval, VmOpCleanupWait, VmOpLockStateRetry, VmOpWaitInterval, ExecuteInSequence, VmJobCheckInterval, VmJobTimeout, VmJobStateReportInterval, VmConfigDriveLabel, VmConfigDriveOnPrimaryPool, VmConfigDriveForceHostCacheUse, VmConfigDriveUseHostCacheOnUnsupportedPool, - HaVmRestartHostUp, ResoureCountRunningVMsonly, AllowExposeHypervisorHostname, AllowExposeHypervisorHostnameAccountLevel }; + HaVmRestartHostUp, ResourceCountRunningVMsonly, AllowExposeHypervisorHostname, AllowExposeHypervisorHostnameAccountLevel }; } public List getStoragePoolAllocators() { diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index af7c2a2acb85..d2206d11c49e 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -895,7 +895,7 @@ public Long doInTransaction(TransactionStatus status) { protected long recalculateAccountResourceCount(final long accountId, final ResourceType type) { final Long newCount; if (type == Resource.ResourceType.user_vm) { - newCount = _userVmDao.countAllocatedVMsForAccount(accountId, VirtualMachineManager.ResoureCountRunningVMsonly.value()); + newCount = _userVmDao.countAllocatedVMsForAccount(accountId, VirtualMachineManager.ResourceCountRunningVMsonly.value()); } else if (type == Resource.ResourceType.volume) { long virtualRouterCount = _vmDao.findIdsOfAllocatedVirtualRoutersForAccount(accountId).size(); newCount = _volumeDao.countAllocatedVolumesForAccount(accountId) - virtualRouterCount; // don't count the volumes of virtual router @@ -963,7 +963,7 @@ public long countCpusForAccount(long accountId) { SearchCriteria sc1 = userVmSearch.create(); sc1.setParameters("accountId", accountId); - if (VirtualMachineManager.ResoureCountRunningVMsonly.value()) + if (VirtualMachineManager.ResourceCountRunningVMsonly.value()) sc1.setParameters("state", new Object[] {State.Destroyed, State.Error, State.Expunging, State.Stopped}); else sc1.setParameters("state", new Object[] {State.Destroyed, State.Error, State.Expunging}); @@ -987,7 +987,7 @@ public long calculateMemoryForAccount(long accountId) { SearchCriteria sc1 = userVmSearch.create(); sc1.setParameters("accountId", accountId); - if (VirtualMachineManager.ResoureCountRunningVMsonly.value()) + if (VirtualMachineManager.ResourceCountRunningVMsonly.value()) sc1.setParameters("state", new Object[] {State.Destroyed, State.Error, State.Expunging, State.Stopped}); else sc1.setParameters("state", new Object[] {State.Destroyed, State.Error, State.Expunging}); diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 6784ffdbaf1f..5c6508f91ab0 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -628,7 +628,7 @@ private void resourceLimitCheck(Account owner, Boolean displayVm, Long cpu, Long } protected void resourceCountIncrement(long accountId, Boolean displayVm, Long cpu, Long memory) { - if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) { + if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) { _resourceLimitMgr.incrementResourceCount(accountId, ResourceType.user_vm, displayVm); _resourceLimitMgr.incrementResourceCount(accountId, ResourceType.cpu, displayVm, cpu); _resourceLimitMgr.incrementResourceCount(accountId, ResourceType.memory, displayVm, memory); @@ -636,7 +636,7 @@ protected void resourceCountIncrement(long accountId, Boolean displayVm, Long cp } protected void resourceCountDecrement(long accountId, Boolean displayVm, Long cpu, Long memory) { - if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) { + if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) { _resourceLimitMgr.decrementResourceCount(accountId, ResourceType.user_vm, displayVm); _resourceLimitMgr.decrementResourceCount(accountId, ResourceType.cpu, displayVm, cpu); _resourceLimitMgr.decrementResourceCount(accountId, ResourceType.memory, displayVm, memory); @@ -1113,7 +1113,7 @@ public UserVm upgradeVirtualMachine(UpgradeVMCmd cmd) throws ResourceAllocationE int currentMemory = currentServiceOffering.getRamSize(); Account owner = _accountMgr.getActiveAccountById(vmInstance.getAccountId()); - if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) { + if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) { if (newCpu > currentCpu) { _resourceLimitMgr.checkResourceLimit(owner, ResourceType.cpu, newCpu - currentCpu); } @@ -1130,7 +1130,7 @@ public UserVm upgradeVirtualMachine(UpgradeVMCmd cmd) throws ResourceAllocationE _itMgr.upgradeVmDb(vmId, newServiceOffering, currentServiceOffering); // Increment or decrement CPU and Memory count accordingly. - if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) { + if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) { if (newCpu > currentCpu) { _resourceLimitMgr.incrementResourceCount(owner.getAccountId(), ResourceType.cpu, new Long(newCpu - currentCpu)); } else if (currentCpu > newCpu) { @@ -1234,7 +1234,7 @@ private UserVm upgradeStoppedVirtualMachine(Long vmId, Long svcOffId, Map currentCpu) { _resourceLimitMgr.checkResourceLimit(owner, ResourceType.cpu, newCpu - currentCpu); } @@ -1270,7 +1270,7 @@ private UserVm upgradeStoppedVirtualMachine(Long vmId, Long svcOffId, Map currentCpu) { _resourceLimitMgr.incrementResourceCount(owner.getAccountId(), ResourceType.cpu, new Long(newCpu - currentCpu)); } else if (currentCpu > newCpu) { @@ -2179,7 +2179,7 @@ public UserVm recoverVirtualMachine(RecoverVMCmd cmd) throws ResourceAllocationE // First check that the maximum number of UserVMs, CPU and Memory limit for the given // accountId will not be exceeded - if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) { + if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) { resourceLimitCheck(account, vm.isDisplayVm(), new Long(serviceOffering.getCpu()), new Long(serviceOffering.getRamSize())); } @@ -2585,11 +2585,16 @@ protected void runInContext() { } } - private void verifyVmLimits(UserVmVO vmInstance, Map details) { - if (VirtualMachineManager.ResoureCountRunningVMsonly.value()) { + private void verifyVmLimits(UserVmVO vmInstance, Map details) { + if (VirtualMachineManager.ResourceCountRunningVMsonly.value()) { return; } + Account owner = _accountDao.findById(vmInstance.getAccountId()); + if (owner == null) { + throw new InvalidParameterValueException("The owner of " + vmInstance + " does not exist: " + vmInstance.getAccountId()); + } + long newCpu = NumberUtils.toLong(details.get(VmDetailConstants.CPU_NUMBER)); long newMemory = NumberUtils.toLong(details.get(VmDetailConstants.MEMORY)); ServiceOfferingVO currentServiceOffering = _serviceOfferingDao.findByIdIncludingRemoved(vmInstance.getId(), vmInstance.getServiceOfferingId()); @@ -2604,10 +2609,6 @@ private void verifyVmLimits(UserVmVO vmInstance, Map details) { long currentCpu = currentServiceOffering.getCpu(); long currentMemory = currentServiceOffering.getRamSize(); - Account owner = _accountDao.findById(vmInstance.getAccountId()); - if (owner == null) { - throw new InvalidParameterValueException("The owner of " + vmInstance + " does not exist: " + vmInstance.getAccountId()); - } try { if (newCpu > currentCpu) { _resourceLimitMgr.checkResourceLimit(owner, ResourceType.cpu, newCpu - currentCpu); @@ -3797,7 +3798,7 @@ private UserVm createVirtualMachine(DataCenter zone, ServiceOffering serviceOffe } size += _diskOfferingDao.findById(diskOfferingId).getDiskSize(); } - if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) { + if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) { resourceLimitCheck(owner, isDisplayVm, new Long(offering.getCpu()), new Long(offering.getRamSize())); } @@ -5024,7 +5025,7 @@ public Pair> startVirtualMach if (owner.getState() == Account.State.disabled) { throw new PermissionDeniedException("The owner of " + vm + " is disabled: " + vm.getAccountId()); } - if (VirtualMachineManager.ResoureCountRunningVMsonly.value()) { + if (VirtualMachineManager.ResourceCountRunningVMsonly.value()) { // check if account/domain is with in resource limits to start a new vm ServiceOfferingVO offering = _serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId()); resourceLimitCheck(owner, vm.isDisplayVm(), Long.valueOf(offering.getCpu()), Long.valueOf(offering.getRamSize())); @@ -6721,7 +6722,7 @@ public UserVm moveVMToUser(final AssignVMCmd cmd) throws ResourceAllocationExcep removeInstanceFromInstanceGroup(cmd.getVmId()); // VV 2: check if account/domain is with in resource limits to create a new vm - if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) { + if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) { resourceLimitCheck(newAccount, vm.isDisplayVm(), new Long(offering.getCpu()), new Long(offering.getRamSize())); } @@ -6779,7 +6780,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) { } //update resource count of new account - if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) { + if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) { resourceCountIncrement(newAccount.getAccountId(), vm.isDisplayVm(), new Long(offering.getCpu()), new Long(offering.getRamSize())); } From 8d7cef4dad77afec6f3530b60661dc1ff7a46b16 Mon Sep 17 00:00:00 2001 From: utchoang Date: Mon, 20 Sep 2021 13:16:55 +0700 Subject: [PATCH 10/14] fix search select options --- ui/src/components/view/SearchView.vue | 5 +-- ui/src/views/AutogenView.vue | 4 +-- .../views/compute/CreateKubernetesCluster.vue | 12 ++++--- ui/src/views/compute/DeployVM.vue | 21 ++++++----- ui/src/views/compute/InstanceTab.vue | 13 ++++--- ui/src/views/iam/AddUser.vue | 24 +++++++------ ui/src/views/image/TemplateZones.vue | 14 ++++---- .../views/infra/network/DedicatedVLANTab.vue | 35 ++++++++++-------- .../network/CreateIsolatedNetworkForm.vue | 12 ++++--- .../views/network/CreateSharedNetworkForm.vue | 36 +++++++++++-------- 10 files changed, 104 insertions(+), 72 deletions(-) diff --git a/ui/src/components/view/SearchView.vue b/ui/src/components/view/SearchView.vue index ad6a48b3abf7..67ba37bb1400 100644 --- a/ui/src/components/view/SearchView.vue +++ b/ui/src/components/view/SearchView.vue @@ -61,13 +61,14 @@ showSearch optionFilterProp="children" :filterOption="(input, option) => { - return option.componentOptions.children[0].text.toLowerCase().indexOf(input.toLowerCase()) >= 0 + return option.componentOptions.propsData.label.toLowerCase().indexOf(input.toLowerCase()) >= 0 }" :loading="field.loading"> + :value="opt.id" + :label="$t(opt.name)"> diff --git a/ui/src/views/AutogenView.vue b/ui/src/views/AutogenView.vue index c924486eff30..0467ecf2031e 100644 --- a/ui/src/views/AutogenView.vue +++ b/ui/src/views/AutogenView.vue @@ -265,12 +265,12 @@ :loading="field.loading" :placeholder="field.description" :filterOption="(input, option) => { - return option.componentOptions.children[0].text.toLowerCase().indexOf(input.toLowerCase()) >= 0 + return option.componentOptions.propsData.label.toLowerCase().indexOf(input.toLowerCase()) >= 0 }" :autoFocus="fieldIndex === firstIndex" > {{ }} - + diff --git a/ui/src/views/compute/CreateKubernetesCluster.vue b/ui/src/views/compute/CreateKubernetesCluster.vue index 0749da1ead27..a1590609c72c 100644 --- a/ui/src/views/compute/CreateKubernetesCluster.vue +++ b/ui/src/views/compute/CreateKubernetesCluster.vue @@ -49,15 +49,17 @@ showSearch optionFilterProp="children" :filterOption="(input, option) => { - return option.componentOptions.children[0].text.toLowerCase().indexOf(input.toLowerCase()) >= 0 + return option.componentOptions.propsData.label.toLowerCase().indexOf(input.toLowerCase()) >= 0 }" :loading="zoneLoading" :placeholder="apiParams.zoneid.description" @change="val => { this.handleZoneChange(this.zones[val]) }"> - - - - {{ opt.name || opt.description }} + + + + + {{ opt.name || opt.description }} + diff --git a/ui/src/views/compute/DeployVM.vue b/ui/src/views/compute/DeployVM.vue index d7db27557161..00ed65f6c20e 100644 --- a/ui/src/views/compute/DeployVM.vue +++ b/ui/src/views/compute/DeployVM.vue @@ -68,15 +68,19 @@ }]" showSearch optionFilterProp="children" - :filterOption="filterOption" + :filterOption="(input, option) => { + return option.componentOptions.propsData.label.toLowerCase().indexOf(input.toLowerCase()) >= 0 + }" @change="onSelectZoneId" :loading="loading.zones" autoFocus > - - - - {{ zone1.name }} + + + + + {{ zone1.name }} + @@ -1878,9 +1882,10 @@ export default { }) }, filterOption (input, option) { - return ( - option.componentOptions.children[0].text.toUpperCase().indexOf(input.toUpperCase()) >= 0 - ) + console.log(option) + // return ( + // option.componentOptions.children[0].text.toUpperCase().indexOf(input.toUpperCase()) >= 0 + // ) }, onSelectZoneId (value) { this.dataPreFill = {} diff --git a/ui/src/views/compute/InstanceTab.vue b/ui/src/views/compute/InstanceTab.vue index b3b3f0ce7a44..437410a092d4 100644 --- a/ui/src/views/compute/InstanceTab.vue +++ b/ui/src/views/compute/InstanceTab.vue @@ -165,15 +165,18 @@ showSearch optionFilterProp="children" :filterOption="(input, option) => { - return option.componentOptions.children[0].text.toLowerCase().indexOf(input.toLowerCase()) >= 0 + return option.componentOptions.propsData.label.toLowerCase().indexOf(input.toLowerCase()) >= 0 }" > - - - {{ network.name }} + :value="network.id" + :label="network.name"> + + + + {{ network.name }} +

{{ $t('label.publicip') }}:

diff --git a/ui/src/views/iam/AddUser.vue b/ui/src/views/iam/AddUser.vue index a2838a39c630..2c9fc8c63a5e 100644 --- a/ui/src/views/iam/AddUser.vue +++ b/ui/src/views/iam/AddUser.vue @@ -92,12 +92,14 @@ showSearch optionFilterProp="children" :filterOption="(input, option) => { - return option.componentOptions.children[0].text.toLowerCase().indexOf(input.toLowerCase()) >= 0 + return option.componentOptions.propsData.label.toLowerCase().indexOf(input.toLowerCase()) >= 0 }" > - - - - {{ domain.path || domain.name || domain.description }} + + + + + {{ domain.path || domain.name || domain.description }} + @@ -112,12 +114,14 @@ showSearch optionFilterProp="children" :filterOption="(input, option) => { - return option.componentOptions.children[0].text.toLowerCase().indexOf(input.toLowerCase()) >= 0 + return option.componentOptions.propsData.label.toLowerCase().indexOf(input.toLowerCase()) >= 0 }" > - - - - {{ item.name }} + + + + + {{ item.name }} + diff --git a/ui/src/views/image/TemplateZones.vue b/ui/src/views/image/TemplateZones.vue index 397af47a3635..e874b58931ca 100644 --- a/ui/src/views/image/TemplateZones.vue +++ b/ui/src/views/image/TemplateZones.vue @@ -122,16 +122,18 @@ showSearch optionFilterProp="children" :filterOption="(input, option) => { - return option.componentOptions.children[0].text.toLowerCase().indexOf(input.toLowerCase()) >= 0 + return option.componentOptions.propsData.label.toLowerCase().indexOf(input.toLowerCase()) >= 0 }" :loading="zoneLoading" autoFocus> - - - + + + + + + + {{ zone.name }} - - {{ zone.name }} diff --git a/ui/src/views/infra/network/DedicatedVLANTab.vue b/ui/src/views/infra/network/DedicatedVLANTab.vue index ed7be5f86513..74fd444956d5 100644 --- a/ui/src/views/infra/network/DedicatedVLANTab.vue +++ b/ui/src/views/infra/network/DedicatedVLANTab.vue @@ -104,12 +104,14 @@ showSearch optionFilterProp="children" :filterOption="(input, option) => { - return option.componentOptions.children[0].text.toLowerCase().indexOf(input.toLowerCase()) >= 0 + return option.componentOptions.propsData.label.toLowerCase().indexOf(input.toLowerCase()) >= 0 }" > - - - - {{ domain.path || domain.name || domain.description }} + + + + + {{ domain.path || domain.name || domain.description }} + @@ -122,15 +124,17 @@ showSearch optionFilterProp="children" :filterOption="(input, option) => { - return option.componentOptions.children[0].text.toLowerCase().indexOf(input.toLowerCase()) >= 0 + return option.componentOptions.propsData.value.toLowerCase().indexOf(input.toLowerCase()) >= 0 }" > - - - {{ account.name }} + + + + {{ account.name }} + @@ -143,15 +147,18 @@ showSearch optionFilterProp="children" :filterOption="(input, option) => { - return option.componentOptions.children[0].text.toLowerCase().indexOf(input.toLowerCase()) >= 0 + return option.componentOptions.propsData.label.toLowerCase().indexOf(input.toLowerCase()) >= 0 }" > - - - {{ project.name }} + :value="project.id" + :label="project.name"> + + + + {{ project.name }} + diff --git a/ui/src/views/network/CreateIsolatedNetworkForm.vue b/ui/src/views/network/CreateIsolatedNetworkForm.vue index 462b581b1636..e421ca6db666 100644 --- a/ui/src/views/network/CreateIsolatedNetworkForm.vue +++ b/ui/src/views/network/CreateIsolatedNetworkForm.vue @@ -54,15 +54,17 @@ showSearch optionFilterProp="children" :filterOption="(input, option) => { - return option.componentOptions.children[0].text.toLowerCase().indexOf(input.toLowerCase()) >= 0 + return option.componentOptions.propsData.label.toLowerCase().indexOf(input.toLowerCase()) >= 0 }" :loading="zoneLoading" :placeholder="this.$t('label.zoneid')" @change="val => { this.handleZoneChange(this.zones[val]) }"> - - - - {{ opt.name || opt.description }} + + + + + {{ opt.name || opt.description }} + diff --git a/ui/src/views/network/CreateSharedNetworkForm.vue b/ui/src/views/network/CreateSharedNetworkForm.vue index 83373410c643..5e6b02152149 100644 --- a/ui/src/views/network/CreateSharedNetworkForm.vue +++ b/ui/src/views/network/CreateSharedNetworkForm.vue @@ -54,15 +54,17 @@ showSearch optionFilterProp="children" :filterOption="(input, option) => { - return option.componentOptions.children[0].text.toLowerCase().indexOf(input.toLowerCase()) >= 0 + return option.componentOptions.propsData.label.toLowerCase().indexOf(input.toLowerCase()) >= 0 }" :loading="zoneLoading" :placeholder="this.$t('label.zoneid')" @change="val => { this.handleZoneChange(this.zones[val]) }"> - - - - {{ opt.name || opt.description }} + + + + + {{ opt.name || opt.description }} + @@ -158,15 +160,17 @@ showSearch optionFilterProp="children" :filterOption="(input, option) => { - return option.componentOptions.children[0].text.toLowerCase().indexOf(input.toLowerCase()) >= 0 + return option.componentOptions.propsData.label.toLowerCase().indexOf(input.toLowerCase()) >= 0 }" :loading="domainLoading" :placeholder="this.$t('label.domainid')" @change="val => { this.handleDomainChange(this.domains[val]) }"> - - - - {{ opt.path || opt.name || opt.description }} + + + + + {{ opt.path || opt.name || opt.description }} + @@ -194,15 +198,17 @@ showSearch optionFilterProp="children" :filterOption="(input, option) => { - return option.componentOptions.children[0].text.toLowerCase().indexOf(input.toLowerCase()) >= 0 + return option.componentOptions.propsData.label.toLowerCase().indexOf(input.toLowerCase()) >= 0 }" :loading="projectLoading" :placeholder="this.$t('label.projectid')" @change="val => { this.handleProjectChange(this.projects[val]) }"> - - - - {{ opt.name || opt.description }} + + + + + {{ opt.name || opt.description }} + From 1164f3b71a602254c3e3a53ba5a27f902191b378 Mon Sep 17 00:00:00 2001 From: utchoang Date: Mon, 20 Sep 2021 13:27:35 +0700 Subject: [PATCH 11/14] fix error when selected zone --- ui/src/views/compute/wizard/NetworkSelection.vue | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ui/src/views/compute/wizard/NetworkSelection.vue b/ui/src/views/compute/wizard/NetworkSelection.vue index c5f895ba0ea1..494bddbd5db3 100644 --- a/ui/src/views/compute/wizard/NetworkSelection.vue +++ b/ui/src/views/compute/wizard/NetworkSelection.vue @@ -267,8 +267,11 @@ export default { this.form = this.$form.createForm(this) }, created () { + this.vpcs = [] + const projectId = store?.getters?.project?.id || null + if (!projectId) return api('listVPCs', { - projectid: store.getters.project.id + projectid: projectId }).then((response) => { this.vpcs = _.get(response, 'listvpcsresponse.vpc') }) From 32350ce5a80fad390d0b0ca088d0d6d6d90b2bf0 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Mon, 20 Sep 2021 11:58:56 +0530 Subject: [PATCH 12/14] enhance logging --- .../org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java index fc29268e775e..6cc9f16d8c1f 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java @@ -268,7 +268,7 @@ public void execute() throws ResourceUnavailableException, InsufficientCapacityE try { result = _userVmService.updateVirtualMachine(this); } catch (CloudRuntimeException e) { - throw new CloudRuntimeException(String.format("Failed to update VM, due to: %s", e.getLocalizedMessage())); + throw new CloudRuntimeException(String.format("Failed to update VM, due to: %s", e.getLocalizedMessage()), e); } if (result != null) { UserVmResponse response = _responseGenerator.createUserVmResponse(getResponseView(), "virtualmachine", result).get(0); From aa027a2075e09eaf18723bab7a360c90d9e57a17 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Mon, 20 Sep 2021 15:35:15 +0530 Subject: [PATCH 13/14] intensify check --- server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 5c6508f91ab0..051046f731c0 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -2623,12 +2623,12 @@ private void verifyVmLimits(UserVmVO vmInstance, Map details) { if (newCpu > currentCpu) { _resourceLimitMgr.incrementResourceCount(owner.getAccountId(), ResourceType.cpu, newCpu - currentCpu); - } else if (currentCpu > newCpu){ + } else if (newCpu > 0 && currentCpu > newCpu){ _resourceLimitMgr.decrementResourceCount(owner.getAccountId(), ResourceType.cpu, currentCpu - newCpu); } if (newMemory > currentMemory) { _resourceLimitMgr.incrementResourceCount(owner.getAccountId(), ResourceType.memory, newMemory - currentMemory); - } else if (currentMemory > newMemory){ + } else if (newMemory > 0 && currentMemory > newMemory){ _resourceLimitMgr.decrementResourceCount(owner.getAccountId(), ResourceType.memory, currentMemory - newMemory); } } From e9e3dc88aaa5590a76e28b0ca6fd3d862908595e Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 23 Sep 2021 11:25:15 +0530 Subject: [PATCH 14/14] move check --- server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 051046f731c0..e97526766ba5 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -2586,10 +2586,6 @@ protected void runInContext() { } private void verifyVmLimits(UserVmVO vmInstance, Map details) { - if (VirtualMachineManager.ResourceCountRunningVMsonly.value()) { - return; - } - Account owner = _accountDao.findById(vmInstance.getAccountId()); if (owner == null) { throw new InvalidParameterValueException("The owner of " + vmInstance + " does not exist: " + vmInstance.getAccountId()); @@ -2606,6 +2602,9 @@ private void verifyVmLimits(UserVmVO vmInstance, Map details) { customParameters.put(VmDetailConstants.MEMORY, String.valueOf(newMemory)); validateCustomParameters(svcOffering, customParameters); } + if (VirtualMachineManager.ResourceCountRunningVMsonly.value()) { + return; + } long currentCpu = currentServiceOffering.getCpu(); long currentMemory = currentServiceOffering.getRamSize();