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 38289537bc6..6cc9f16d8c1 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.HashMap; 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,8 +264,13 @@ public class UpdateVMCmd extends BaseCustomIdCmd implements SecurityGroupAction, @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){ + UserVm result = null; + try { + result = _userVmService.updateVirtualMachine(this); + } catch (CloudRuntimeException e) { + 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); response.setResponseName(getCommandName()); setResponseObject(response); 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 c192f876aac..f02395e0d44 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 d45b92098d2..854d5a2107d 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1048,9 +1048,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac final HypervisorGuru hvGuru = _hvGuruMgr.getGuru(vm.getHypervisorType()); - // check resource count if ResoureCountRunningVMsonly.value() = true + // check resource count if ResourceCountRunningVMsonly.value() = true final Account owner = _entityMgr.findById(Account.class, vm.getAccountId()); - if (VirtualMachine.Type.User.equals(vm.type) && ResoureCountRunningVMsonly.value()) { + if (VirtualMachine.Type.User.equals(vm.type) && ResourceCountRunningVMsonly.value()) { resourceCountIncrement(owner.getAccountId(),new Long(offering.getCpu()), new Long(offering.getRamSize())); } @@ -1367,7 +1367,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } } finally { if (startedVm == null) { - if (VirtualMachine.Type.User.equals(vm.type) && ResoureCountRunningVMsonly.value()) { + if (VirtualMachine.Type.User.equals(vm.type) && ResourceCountRunningVMsonly.value()) { resourceCountDecrement(owner.getAccountId(),new Long(offering.getCpu()), new Long(offering.getRamSize())); } if (canRetry) { @@ -2133,7 +2133,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac boolean result = stateTransitTo(vm, Event.OperationSucceeded, null); if (result) { - if (VirtualMachine.Type.User.equals(vm.type) && ResoureCountRunningVMsonly.value()) { + if (VirtualMachine.Type.User.equals(vm.type) && ResourceCountRunningVMsonly.value()) { //update resource count if stop successfully ServiceOfferingVO offering = _offeringDao.findById(vm.getId(), vm.getServiceOfferingId()); resourceCountDecrement(vm.getAccountId(),new Long(offering.getCpu()), new Long(offering.getRamSize())); @@ -4829,7 +4829,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac 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 af7c2a2acb8..d2206d11c49 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 class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim 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 class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim 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 class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim 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 72cd6f3fdf5..e97526766ba 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -116,6 +116,7 @@ import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO; 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.springframework.beans.factory.annotation.Autowired; @@ -627,7 +628,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } 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); @@ -635,7 +636,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } 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); @@ -1112,7 +1113,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir 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); } @@ -1129,7 +1130,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir _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) { @@ -1233,7 +1234,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir 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); } @@ -1269,7 +1270,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir _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) { @@ -2178,7 +2179,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // 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())); } @@ -2584,6 +2585,53 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } } + private void verifyVmLimits(UserVmVO vmInstance, Map details) { + 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()); + ServiceOfferingVO svcOffering = _serviceOfferingDao.findById(vmInstance.getServiceOfferingId()); + boolean isDynamic = currentServiceOffering.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); + } + if (VirtualMachineManager.ResourceCountRunningVMsonly.value()) { + return; + } + long currentCpu = currentServiceOffering.getCpu(); + long currentMemory = currentServiceOffering.getRamSize(); + + try { + if (newCpu > currentCpu) { + _resourceLimitMgr.checkResourceLimit(owner, ResourceType.cpu, newCpu - currentCpu); + } + if (newMemory > currentMemory) { + _resourceLimitMgr.checkResourceLimit(owner, ResourceType.memory, newMemory - currentMemory); + } + } catch (ResourceAllocationException e) { + s_logger.error(String.format("Failed to updated VM due to: %s", e.getLocalizedMessage())); + throw new InvalidParameterValueException(e.getLocalizedMessage()); + } + + if (newCpu > currentCpu) { + _resourceLimitMgr.incrementResourceCount(owner.getAccountId(), ResourceType.cpu, newCpu - currentCpu); + } 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 (newMemory > 0 && currentMemory > newMemory){ + _resourceLimitMgr.decrementResourceCount(owner.getAccountId(), ResourceType.memory, currentMemory - newMemory); + } + } + @Override @ActionEvent(eventType = EventTypes.EVENT_VM_UPDATE, eventDescription = "updating Vm") public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableException, InsufficientCapacityException { @@ -2657,6 +2705,8 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } } } + + verifyVmLimits(vmInstance, details); vmInstance.setDetails(details); _vmDao.saveDetails(vmInstance); } @@ -2679,9 +2729,9 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // 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 @@ -3747,7 +3797,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } size += _diskOfferingDao.findById(diskOfferingId).getDiskSize(); } - if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) { + if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) { resourceLimitCheck(owner, isDisplayVm, new Long(offering.getCpu()), new Long(offering.getRamSize())); } @@ -4974,12 +5024,11 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir 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(), 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 if (_securityGroupMgr.isVmSecurityGroupEnabled(vmId) && _securityGroupMgr.getSecurityGroupsForVm(vmId).isEmpty() && !_securityGroupMgr.isVmMappedToDefaultSecurityGroup(vmId) && _networkModel.canAddDefaultSecurityGroup()) { @@ -6672,7 +6721,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir 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())); } @@ -6730,7 +6779,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } //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())); } diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index da59f07a3a8..a9821094f06 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 @@ package com.cloud.vm; 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,15 @@ import java.util.HashMap; 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; import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd; @@ -145,6 +149,12 @@ public class UserVmManagerImplTest { @Mock private VMTemplateDao templateDao; + @Mock + private AccountDao accountDao; + + @Mock + ResourceLimitService resourceLimitMgr; + private long vmId = 1l; private static final long GiB_TO_BYTES = 1024 * 1024 * 1024; @@ -167,6 +177,8 @@ public class UserVmManagerImplTest { 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 @@ -280,6 +292,7 @@ public class UserVmManagerImplTest { @Test public void updateVirtualMachineTestCleanUpFalseAndDetailsEmpty() throws ResourceUnavailableException, InsufficientCapacityException { + Mockito.when(accountDao.findById(Mockito.anyLong())).thenReturn(callerAccount); prepareAndExecuteMethodDealingWithDetails(false, false); } @@ -288,6 +301,10 @@ public class UserVmManagerImplTest { 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); @@ -303,7 +320,6 @@ public class UserVmManagerImplTest { } Mockito.when(updateVmCommand.getDetails()).thenReturn(details); Mockito.when(updateVmCommand.isCleanupDetails()).thenReturn(cleanUpDetails); - configureDoNothingForDetailsMethod(); userVmManagerImpl.updateVirtualMachine(updateVmCommand);