Skip to content

Commit 74bb806

Browse files
Pearl1594utchoang
Pearl1594
and
utchoang
authored
resource limit: Fix resource limit check on VM start (apache#5428)
* resource limit: Fix resource limit check on VM start * add check to validate if cpu/memory are within limits for custom offering + exception handling * unit tests Co-authored-by: utchoang <hoangnm@unitech.vn>
1 parent 6ba656b commit 74bb806

File tree

6 files changed

+98
-27
lines changed

6 files changed

+98
-27
lines changed

‎api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.List;
2222
import java.util.Map;
2323

24+
import com.cloud.utils.exception.CloudRuntimeException;
2425
import org.apache.log4j.Logger;
2526

2627
import org.apache.cloudstack.acl.RoleType;
@@ -263,8 +264,13 @@ public long getEntityOwnerId() {
263264
@Override
264265
public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException {
265266
CallContext.current().setEventDetails("Vm Id: " + this._uuidMgr.getUuid(VirtualMachine.class, getId()));
266-
UserVm result = _userVmService.updateVirtualMachine(this);
267-
if (result != null){
267+
UserVm result = null;
268+
try {
269+
result = _userVmService.updateVirtualMachine(this);
270+
} catch (CloudRuntimeException e) {
271+
throw new CloudRuntimeException(String.format("Failed to update VM, due to: %s", e.getLocalizedMessage()), e);
272+
}
273+
if (result != null) {
268274
UserVmResponse response = _responseGenerator.createUserVmResponse(getResponseView(), "virtualmachine", result).get(0);
269275
response.setResponseName(getCommandName());
270276
setResponseObject(response);

‎engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public interface VirtualMachineManager extends Manager {
7070
ConfigKey<Boolean> VmConfigDriveForceHostCacheUse = new ConfigKey<>("Advanced", Boolean.class, "vm.configdrive.force.host.cache.use", "false",
7171
"If true, config drive is forced to create on the host cache storage. Currently only supported for KVM.", true, ConfigKey.Scope.Zone);
7272

73-
ConfigKey<Boolean> ResoureCountRunningVMsonly = new ConfigKey<Boolean>("Advanced", Boolean.class, "resource.count.running.vms.only", "false",
73+
ConfigKey<Boolean> ResourceCountRunningVMsonly = new ConfigKey<Boolean>("Advanced", Boolean.class, "resource.count.running.vms.only", "false",
7474
"Count the resources of only running VMs in resource limitation.", true);
7575

7676
ConfigKey<Boolean> AllowExposeHypervisorHostnameAccountLevel = new ConfigKey<Boolean>("Advanced", Boolean.class, "account.allow.expose.host.hostname",

‎engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1048,9 +1048,9 @@ public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfil
10481048

10491049
final HypervisorGuru hvGuru = _hvGuruMgr.getGuru(vm.getHypervisorType());
10501050

1051-
// check resource count if ResoureCountRunningVMsonly.value() = true
1051+
// check resource count if ResourceCountRunningVMsonly.value() = true
10521052
final Account owner = _entityMgr.findById(Account.class, vm.getAccountId());
1053-
if (VirtualMachine.Type.User.equals(vm.type) && ResoureCountRunningVMsonly.value()) {
1053+
if (VirtualMachine.Type.User.equals(vm.type) && ResourceCountRunningVMsonly.value()) {
10541054
resourceCountIncrement(owner.getAccountId(),new Long(offering.getCpu()), new Long(offering.getRamSize()));
10551055
}
10561056

@@ -1367,7 +1367,7 @@ public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfil
13671367
}
13681368
} finally {
13691369
if (startedVm == null) {
1370-
if (VirtualMachine.Type.User.equals(vm.type) && ResoureCountRunningVMsonly.value()) {
1370+
if (VirtualMachine.Type.User.equals(vm.type) && ResourceCountRunningVMsonly.value()) {
13711371
resourceCountDecrement(owner.getAccountId(),new Long(offering.getCpu()), new Long(offering.getRamSize()));
13721372
}
13731373
if (canRetry) {
@@ -2133,7 +2133,7 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl
21332133

21342134
boolean result = stateTransitTo(vm, Event.OperationSucceeded, null);
21352135
if (result) {
2136-
if (VirtualMachine.Type.User.equals(vm.type) && ResoureCountRunningVMsonly.value()) {
2136+
if (VirtualMachine.Type.User.equals(vm.type) && ResourceCountRunningVMsonly.value()) {
21372137
//update resource count if stop successfully
21382138
ServiceOfferingVO offering = _offeringDao.findById(vm.getId(), vm.getServiceOfferingId());
21392139
resourceCountDecrement(vm.getAccountId(),new Long(offering.getCpu()), new Long(offering.getRamSize()));
@@ -4829,7 +4829,7 @@ public ConfigKey<?>[] getConfigKeys() {
48294829
return new ConfigKey<?>[] { ClusterDeltaSyncInterval, StartRetry, VmDestroyForcestop, VmOpCancelInterval, VmOpCleanupInterval, VmOpCleanupWait,
48304830
VmOpLockStateRetry, VmOpWaitInterval, ExecuteInSequence, VmJobCheckInterval, VmJobTimeout, VmJobStateReportInterval,
48314831
VmConfigDriveLabel, VmConfigDriveOnPrimaryPool, VmConfigDriveForceHostCacheUse, VmConfigDriveUseHostCacheOnUnsupportedPool,
4832-
HaVmRestartHostUp, ResoureCountRunningVMsonly, AllowExposeHypervisorHostname, AllowExposeHypervisorHostnameAccountLevel };
4832+
HaVmRestartHostUp, ResourceCountRunningVMsonly, AllowExposeHypervisorHostname, AllowExposeHypervisorHostnameAccountLevel };
48334833
}
48344834

48354835
public List<StoragePoolAllocator> getStoragePoolAllocators() {

‎server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -895,7 +895,7 @@ public Long doInTransaction(TransactionStatus status) {
895895
protected long recalculateAccountResourceCount(final long accountId, final ResourceType type) {
896896
final Long newCount;
897897
if (type == Resource.ResourceType.user_vm) {
898-
newCount = _userVmDao.countAllocatedVMsForAccount(accountId, VirtualMachineManager.ResoureCountRunningVMsonly.value());
898+
newCount = _userVmDao.countAllocatedVMsForAccount(accountId, VirtualMachineManager.ResourceCountRunningVMsonly.value());
899899
} else if (type == Resource.ResourceType.volume) {
900900
long virtualRouterCount = _vmDao.findIdsOfAllocatedVirtualRoutersForAccount(accountId).size();
901901
newCount = _volumeDao.countAllocatedVolumesForAccount(accountId) - virtualRouterCount; // don't count the volumes of virtual router
@@ -963,7 +963,7 @@ public long countCpusForAccount(long accountId) {
963963

964964
SearchCriteria<UserVmJoinVO> sc1 = userVmSearch.create();
965965
sc1.setParameters("accountId", accountId);
966-
if (VirtualMachineManager.ResoureCountRunningVMsonly.value())
966+
if (VirtualMachineManager.ResourceCountRunningVMsonly.value())
967967
sc1.setParameters("state", new Object[] {State.Destroyed, State.Error, State.Expunging, State.Stopped});
968968
else
969969
sc1.setParameters("state", new Object[] {State.Destroyed, State.Error, State.Expunging});
@@ -987,7 +987,7 @@ public long calculateMemoryForAccount(long accountId) {
987987

988988
SearchCriteria<UserVmJoinVO> sc1 = userVmSearch.create();
989989
sc1.setParameters("accountId", accountId);
990-
if (VirtualMachineManager.ResoureCountRunningVMsonly.value())
990+
if (VirtualMachineManager.ResourceCountRunningVMsonly.value())
991991
sc1.setParameters("state", new Object[] {State.Destroyed, State.Error, State.Expunging, State.Stopped});
992992
else
993993
sc1.setParameters("state", new Object[] {State.Destroyed, State.Error, State.Expunging});

‎server/src/main/java/com/cloud/vm/UserVmManagerImpl.java‎

Lines changed: 64 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@
116116
import org.apache.commons.codec.binary.Base64;
117117
import org.apache.commons.collections.CollectionUtils;
118118
import org.apache.commons.collections.MapUtils;
119+
import org.apache.commons.lang.math.NumberUtils;
119120
import org.apache.commons.lang3.StringUtils;
120121
import org.apache.log4j.Logger;
121122
import org.springframework.beans.factory.annotation.Autowired;
@@ -627,15 +628,15 @@ private void resourceLimitCheck(Account owner, Boolean displayVm, Long cpu, Long
627628
}
628629

629630
protected void resourceCountIncrement(long accountId, Boolean displayVm, Long cpu, Long memory) {
630-
if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) {
631+
if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) {
631632
_resourceLimitMgr.incrementResourceCount(accountId, ResourceType.user_vm, displayVm);
632633
_resourceLimitMgr.incrementResourceCount(accountId, ResourceType.cpu, displayVm, cpu);
633634
_resourceLimitMgr.incrementResourceCount(accountId, ResourceType.memory, displayVm, memory);
634635
}
635636
}
636637

637638
protected void resourceCountDecrement(long accountId, Boolean displayVm, Long cpu, Long memory) {
638-
if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) {
639+
if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) {
639640
_resourceLimitMgr.decrementResourceCount(accountId, ResourceType.user_vm, displayVm);
640641
_resourceLimitMgr.decrementResourceCount(accountId, ResourceType.cpu, displayVm, cpu);
641642
_resourceLimitMgr.decrementResourceCount(accountId, ResourceType.memory, displayVm, memory);
@@ -1112,7 +1113,7 @@ public UserVm upgradeVirtualMachine(UpgradeVMCmd cmd) throws ResourceAllocationE
11121113
int currentMemory = currentServiceOffering.getRamSize();
11131114

11141115
Account owner = _accountMgr.getActiveAccountById(vmInstance.getAccountId());
1115-
if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) {
1116+
if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) {
11161117
if (newCpu > currentCpu) {
11171118
_resourceLimitMgr.checkResourceLimit(owner, ResourceType.cpu, newCpu - currentCpu);
11181119
}
@@ -1129,7 +1130,7 @@ public UserVm upgradeVirtualMachine(UpgradeVMCmd cmd) throws ResourceAllocationE
11291130
_itMgr.upgradeVmDb(vmId, newServiceOffering, currentServiceOffering);
11301131

11311132
// Increment or decrement CPU and Memory count accordingly.
1132-
if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) {
1133+
if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) {
11331134
if (newCpu > currentCpu) {
11341135
_resourceLimitMgr.incrementResourceCount(owner.getAccountId(), ResourceType.cpu, new Long(newCpu - currentCpu));
11351136
} else if (currentCpu > newCpu) {
@@ -1233,7 +1234,7 @@ private UserVm upgradeStoppedVirtualMachine(Long vmId, Long svcOffId, Map<String
12331234
int currentMemory = currentServiceOffering.getRamSize();
12341235

12351236
Account owner = _accountMgr.getActiveAccountById(vmInstance.getAccountId());
1236-
if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) {
1237+
if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) {
12371238
if (newCpu > currentCpu) {
12381239
_resourceLimitMgr.checkResourceLimit(owner, ResourceType.cpu, newCpu - currentCpu);
12391240
}
@@ -1269,7 +1270,7 @@ private UserVm upgradeStoppedVirtualMachine(Long vmId, Long svcOffId, Map<String
12691270
_itMgr.upgradeVmDb(vmId, newServiceOffering, currentServiceOffering);
12701271

12711272
// Increment or decrement CPU and Memory count accordingly.
1272-
if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) {
1273+
if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) {
12731274
if (newCpu > currentCpu) {
12741275
_resourceLimitMgr.incrementResourceCount(owner.getAccountId(), ResourceType.cpu, new Long(newCpu - currentCpu));
12751276
} else if (currentCpu > newCpu) {
@@ -2178,7 +2179,7 @@ public UserVm recoverVirtualMachine(RecoverVMCmd cmd) throws ResourceAllocationE
21782179

21792180
// First check that the maximum number of UserVMs, CPU and Memory limit for the given
21802181
// accountId will not be exceeded
2181-
if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) {
2182+
if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) {
21822183
resourceLimitCheck(account, vm.isDisplayVm(), new Long(serviceOffering.getCpu()), new Long(serviceOffering.getRamSize()));
21832184
}
21842185

@@ -2584,6 +2585,53 @@ protected void runInContext() {
25842585
}
25852586
}
25862587

2588+
private void verifyVmLimits(UserVmVO vmInstance, Map<String, String> details) {
2589+
Account owner = _accountDao.findById(vmInstance.getAccountId());
2590+
if (owner == null) {
2591+
throw new InvalidParameterValueException("The owner of " + vmInstance + " does not exist: " + vmInstance.getAccountId());
2592+
}
2593+
2594+
long newCpu = NumberUtils.toLong(details.get(VmDetailConstants.CPU_NUMBER));
2595+
long newMemory = NumberUtils.toLong(details.get(VmDetailConstants.MEMORY));
2596+
ServiceOfferingVO currentServiceOffering = _serviceOfferingDao.findByIdIncludingRemoved(vmInstance.getId(), vmInstance.getServiceOfferingId());
2597+
ServiceOfferingVO svcOffering = _serviceOfferingDao.findById(vmInstance.getServiceOfferingId());
2598+
boolean isDynamic = currentServiceOffering.isDynamic();
2599+
if (isDynamic) {
2600+
Map<String, String> customParameters = new HashMap<>();
2601+
customParameters.put(VmDetailConstants.CPU_NUMBER, String.valueOf(newCpu));
2602+
customParameters.put(VmDetailConstants.MEMORY, String.valueOf(newMemory));
2603+
validateCustomParameters(svcOffering, customParameters);
2604+
}
2605+
if (VirtualMachineManager.ResourceCountRunningVMsonly.value()) {
2606+
return;
2607+
}
2608+
long currentCpu = currentServiceOffering.getCpu();
2609+
long currentMemory = currentServiceOffering.getRamSize();
2610+
2611+
try {
2612+
if (newCpu > currentCpu) {
2613+
_resourceLimitMgr.checkResourceLimit(owner, ResourceType.cpu, newCpu - currentCpu);
2614+
}
2615+
if (newMemory > currentMemory) {
2616+
_resourceLimitMgr.checkResourceLimit(owner, ResourceType.memory, newMemory - currentMemory);
2617+
}
2618+
} catch (ResourceAllocationException e) {
2619+
s_logger.error(String.format("Failed to updated VM due to: %s", e.getLocalizedMessage()));
2620+
throw new InvalidParameterValueException(e.getLocalizedMessage());
2621+
}
2622+
2623+
if (newCpu > currentCpu) {
2624+
_resourceLimitMgr.incrementResourceCount(owner.getAccountId(), ResourceType.cpu, newCpu - currentCpu);
2625+
} else if (newCpu > 0 && currentCpu > newCpu){
2626+
_resourceLimitMgr.decrementResourceCount(owner.getAccountId(), ResourceType.cpu, currentCpu - newCpu);
2627+
}
2628+
if (newMemory > currentMemory) {
2629+
_resourceLimitMgr.incrementResourceCount(owner.getAccountId(), ResourceType.memory, newMemory - currentMemory);
2630+
} else if (newMemory > 0 && currentMemory > newMemory){
2631+
_resourceLimitMgr.decrementResourceCount(owner.getAccountId(), ResourceType.memory, currentMemory - newMemory);
2632+
}
2633+
}
2634+
25872635
@Override
25882636
@ActionEvent(eventType = EventTypes.EVENT_VM_UPDATE, eventDescription = "updating Vm")
25892637
public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableException, InsufficientCapacityException {
@@ -2657,6 +2705,8 @@ public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableEx
26572705
}
26582706
}
26592707
}
2708+
2709+
verifyVmLimits(vmInstance, details);
26602710
vmInstance.setDetails(details);
26612711
_vmDao.saveDetails(vmInstance);
26622712
}
@@ -2679,9 +2729,9 @@ protected void updateDisplayVmFlag(Boolean isDisplayVm, Long id, UserVmVO vmInst
26792729
// Resource limit changes
26802730
ServiceOffering offering = _serviceOfferingDao.findByIdIncludingRemoved(vmInstance.getId(), vmInstance.getServiceOfferingId());
26812731
if (isDisplayVm) {
2682-
resourceCountIncrement(vmInstance.getAccountId(), true, new Long(offering.getCpu()), new Long(offering.getRamSize()));
2732+
resourceCountIncrement(vmInstance.getAccountId(), true, Long.valueOf(offering.getCpu()), Long.valueOf(offering.getRamSize()));
26832733
} else {
2684-
resourceCountDecrement(vmInstance.getAccountId(), true, new Long(offering.getCpu()), new Long(offering.getRamSize()));
2734+
resourceCountDecrement(vmInstance.getAccountId(), true, Long.valueOf(offering.getCpu()), Long.valueOf(offering.getRamSize()));
26852735
}
26862736

26872737
// Usage
@@ -3747,7 +3797,7 @@ private UserVm createVirtualMachine(DataCenter zone, ServiceOffering serviceOffe
37473797
}
37483798
size += _diskOfferingDao.findById(diskOfferingId).getDiskSize();
37493799
}
3750-
if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) {
3800+
if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) {
37513801
resourceLimitCheck(owner, isDisplayVm, new Long(offering.getCpu()), new Long(offering.getRamSize()));
37523802
}
37533803

@@ -4974,12 +5024,11 @@ public Pair<UserVmVO, Map<VirtualMachineProfile.Param, Object>> startVirtualMach
49745024
if (owner.getState() == Account.State.disabled) {
49755025
throw new PermissionDeniedException("The owner of " + vm + " is disabled: " + vm.getAccountId());
49765026
}
4977-
if (VirtualMachineManager.ResoureCountRunningVMsonly.value()) {
5027+
if (VirtualMachineManager.ResourceCountRunningVMsonly.value()) {
49785028
// check if account/domain is with in resource limits to start a new vm
49795029
ServiceOfferingVO offering = _serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId());
4980-
resourceLimitCheck(owner, vm.isDisplayVm(), new Long(offering.getCpu()), new Long(offering.getRamSize()));
5030+
resourceLimitCheck(owner, vm.isDisplayVm(), Long.valueOf(offering.getCpu()), Long.valueOf(offering.getRamSize()));
49815031
}
4982-
49835032
// check if vm is security group enabled
49845033
if (_securityGroupMgr.isVmSecurityGroupEnabled(vmId) && _securityGroupMgr.getSecurityGroupsForVm(vmId).isEmpty()
49855034
&& !_securityGroupMgr.isVmMappedToDefaultSecurityGroup(vmId) && _networkModel.canAddDefaultSecurityGroup()) {
@@ -6672,7 +6721,7 @@ public UserVm moveVMToUser(final AssignVMCmd cmd) throws ResourceAllocationExcep
66726721
removeInstanceFromInstanceGroup(cmd.getVmId());
66736722

66746723
// VV 2: check if account/domain is with in resource limits to create a new vm
6675-
if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) {
6724+
if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) {
66766725
resourceLimitCheck(newAccount, vm.isDisplayVm(), new Long(offering.getCpu()), new Long(offering.getRamSize()));
66776726
}
66786727

@@ -6730,7 +6779,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
67306779
}
67316780

67326781
//update resource count of new account
6733-
if (! VirtualMachineManager.ResoureCountRunningVMsonly.value()) {
6782+
if (! VirtualMachineManager.ResourceCountRunningVMsonly.value()) {
67346783
resourceCountIncrement(newAccount.getAccountId(), vm.isDisplayVm(), new Long(offering.getCpu()), new Long(offering.getRamSize()));
67356784
}
67366785

0 commit comments

Comments
 (0)