From 5d545023fcc4ea525012232182a250669975f24d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Thu, 1 Feb 2018 10:59:16 -0200 Subject: [PATCH 1/3] [CLOUDSTACK-9338] ACS not accounting resources of VMs with custom service offering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ACS is accounting the resources properly when deploying VMs with custom service offerings. However, there are other methods (such as updateResourceCount) that do not execute the resource accounting properly, and these methods update the resource count for an account in the database. Therefore, if a user deploys VMs with custom service offerings, and later this user calls the “updateResourceCount” method, it (the method) will only account for VMs with normal service offerings, and update this as the number of resources used by the account. This will result in a smaller number of resources to be accounted for the given account than the real used value. The problem becomes worse because if the user starts to delete these VMs, it is possible to reach negative values of resources allocated (breaking all of the resource limiting for accounts). This is a very serious attack vector for public cloud providers! --- .../configuration/dao/ResourceCountDao.java | 14 ++++++ .../dao/ResourceCountDaoImpl.java | 42 ++++++++++++++++++ .../ResourceLimitManagerImpl.java | 44 +------------------ 3 files changed, 58 insertions(+), 42 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java index d4695c3ff75..f5b76e3e7fc 100644 --- a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java +++ b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java @@ -57,4 +57,18 @@ public interface ResourceCountDao extends GenericDao { Set listRowsToUpdateForDomain(long domainId, ResourceType type); long removeEntriesByOwner(long ownerId, ResourceOwnerType ownerType); + + /** + * Counts the number of CPU cores allocated for the given account. + * + * Side note: This method is not using the "resource_count" table. It is executing the actual count instead. + */ + long countCpuNumberAllocatedToAccount(long accountId); + + /** + * Counts the amount of memory allocated for the given account. + * + * Side note: This method is not using the "resource_count" table. It is executing the actual count instead. + */ + long countMemoryAllocatedToAccount(long accountId); } diff --git a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java index f7cd3cbf86f..3705418f98d 100644 --- a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java @@ -16,6 +16,9 @@ // under the License. package com.cloud.configuration.dao; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -42,6 +45,7 @@ import com.cloud.utils.db.GenericDaoBase; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; import com.cloud.utils.db.TransactionLegacy; +import com.cloud.utils.exception.CloudRuntimeException; @Component public class ResourceCountDaoImpl extends GenericDaoBase implements ResourceCountDao { @@ -248,4 +252,42 @@ public class ResourceCountDaoImpl extends GenericDaoBase return 0; } + private String baseSqlCountComputingResourceAllocatedToAccount = "Select " + + " SUM((CASE " + + " WHEN so.%s is not null THEN so.%s " + + " ELSE CONVERT(vmd.value, UNSIGNED INTEGER) " + + " END)) as total " + + " from vm_instance vm " + + " join service_offering_view so on so.id = vm.service_offering_id " + + " left join user_vm_details vmd on vmd.vm_id = vm.id and vmd.name = '%s' " + + " where vm.type = 'User' and state not in ('Destroyed', 'Error', 'Expunging') and display_vm = true and account_id = ? "; + + @Override + public long countCpuNumberAllocatedToAccount(long accountId) { + String sqlCountCpuNumberAllocatedToAccount = String.format(baseSqlCountComputingResourceAllocatedToAccount, ResourceType.cpu, ResourceType.cpu, "cpuNumber"); + return executeSqlCountComputingResourcesForAccount(accountId, sqlCountCpuNumberAllocatedToAccount); + } + + @Override + public long countMemoryAllocatedToAccount(long accountId) { + String serviceOfferingRamSizeField = "ram_size"; + String sqlCountCpuNumberAllocatedToAccount = String.format(baseSqlCountComputingResourceAllocatedToAccount, serviceOfferingRamSizeField, serviceOfferingRamSizeField, "memory"); + return executeSqlCountComputingResourcesForAccount(accountId, sqlCountCpuNumberAllocatedToAccount); + } + + private long executeSqlCountComputingResourcesForAccount(long accountId, String sqlCountComputingResourcesAllocatedToAccount) { + try (TransactionLegacy tx = TransactionLegacy.currentTxn()) { + PreparedStatement pstmt = tx.prepareAutoCloseStatement(sqlCountComputingResourcesAllocatedToAccount); + pstmt.setLong(1, accountId); + + ResultSet rs = pstmt.executeQuery(); + if (!rs.next()) { + throw new CloudRuntimeException(String.format("An unexpected case happened while counting allocated computing resources for account: " + accountId)); + } + return rs.getLong("total"); + } catch (SQLException e) { + throw new CloudRuntimeException(e); + } + } + } \ No newline at end of file diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index 86fa46b6c26..df7276dcbe3 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -947,51 +947,11 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim } public long countCpusForAccount(long accountId) { - GenericSearchBuilder cpuSearch = _serviceOfferingDao.createSearchBuilder(SumCount.class); - cpuSearch.select("sum", Func.SUM, cpuSearch.entity().getCpu()); - SearchBuilder join1 = _userVmDao.createSearchBuilder(); - join1.and("accountId", join1.entity().getAccountId(), Op.EQ); - join1.and("type", join1.entity().getType(), Op.EQ); - join1.and("state", join1.entity().getState(), SearchCriteria.Op.NIN); - join1.and("displayVm", join1.entity().isDisplayVm(), Op.EQ); - cpuSearch.join("offerings", join1, cpuSearch.entity().getId(), join1.entity().getServiceOfferingId(), JoinBuilder.JoinType.INNER); - cpuSearch.done(); - - SearchCriteria sc = cpuSearch.create(); - sc.setJoinParameters("offerings", "accountId", accountId); - sc.setJoinParameters("offerings", "type", VirtualMachine.Type.User); - sc.setJoinParameters("offerings", "state", new Object[] {State.Destroyed, State.Error, State.Expunging}); - sc.setJoinParameters("offerings", "displayVm", 1); - List cpus = _serviceOfferingDao.customSearch(sc, null); - if (cpus != null) { - return cpus.get(0).sum; - } else { - return 0; - } + return _resourceCountDao.countCpuNumberAllocatedToAccount(accountId); } public long calculateMemoryForAccount(long accountId) { - GenericSearchBuilder memorySearch = _serviceOfferingDao.createSearchBuilder(SumCount.class); - memorySearch.select("sum", Func.SUM, memorySearch.entity().getRamSize()); - SearchBuilder join1 = _userVmDao.createSearchBuilder(); - join1.and("accountId", join1.entity().getAccountId(), Op.EQ); - join1.and("type", join1.entity().getType(), Op.EQ); - join1.and("state", join1.entity().getState(), SearchCriteria.Op.NIN); - join1.and("displayVm", join1.entity().isDisplayVm(), Op.EQ); - memorySearch.join("offerings", join1, memorySearch.entity().getId(), join1.entity().getServiceOfferingId(), JoinBuilder.JoinType.INNER); - memorySearch.done(); - - SearchCriteria sc = memorySearch.create(); - sc.setJoinParameters("offerings", "accountId", accountId); - sc.setJoinParameters("offerings", "type", VirtualMachine.Type.User); - sc.setJoinParameters("offerings", "state", new Object[] {State.Destroyed, State.Error, State.Expunging}); - sc.setJoinParameters("offerings", "displayVm", 1); - List memory = _serviceOfferingDao.customSearch(sc, null); - if (memory != null) { - return memory.get(0).sum; - } else { - return 0; - } + return _resourceCountDao.countMemoryAllocatedToAccount(accountId); } public long calculateSecondaryStorageForAccount(long accountId) { From 7f934c0e866c8f4aedad99a8a5ffe4c03118a656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Thu, 1 Feb 2018 11:00:30 -0200 Subject: [PATCH 2/3] Formatting to make checkstyle happy --- .../configuration/dao/ResourceCountDao.java | 8 +- .../dao/ResourceCountDaoImpl.java | 23 +- .../ResourceLimitManagerImpl.java | 255 ++++++++---------- 3 files changed, 132 insertions(+), 154 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java index f5b76e3e7fc..b5a75d196fa 100644 --- a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java +++ b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java @@ -57,17 +57,17 @@ public interface ResourceCountDao extends GenericDao { Set listRowsToUpdateForDomain(long domainId, ResourceType type); long removeEntriesByOwner(long ownerId, ResourceOwnerType ownerType); - + /** * Counts the number of CPU cores allocated for the given account. - * + * * Side note: This method is not using the "resource_count" table. It is executing the actual count instead. */ long countCpuNumberAllocatedToAccount(long accountId); - + /** * Counts the amount of memory allocated for the given account. - * + * * Side note: This method is not using the "resource_count" table. It is executing the actual count instead. */ long countMemoryAllocatedToAccount(long accountId); diff --git a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java index 3705418f98d..56261337faf 100644 --- a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java @@ -27,9 +27,6 @@ import java.util.Set; import javax.annotation.PostConstruct; import javax.inject.Inject; -import com.cloud.domain.DomainVO; -import com.cloud.user.AccountVO; -import com.cloud.utils.db.JoinBuilder; import org.springframework.stereotype.Component; import com.cloud.configuration.Resource; @@ -37,11 +34,14 @@ import com.cloud.configuration.Resource.ResourceOwnerType; import com.cloud.configuration.Resource.ResourceType; import com.cloud.configuration.ResourceCountVO; import com.cloud.configuration.ResourceLimit; +import com.cloud.domain.DomainVO; import com.cloud.domain.dao.DomainDao; import com.cloud.exception.UnsupportedServiceException; +import com.cloud.user.AccountVO; import com.cloud.user.dao.AccountDao; import com.cloud.utils.db.DB; import com.cloud.utils.db.GenericDaoBase; +import com.cloud.utils.db.JoinBuilder; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; import com.cloud.utils.db.TransactionLegacy; @@ -55,9 +55,9 @@ public class ResourceCountDaoImpl extends GenericDaoBase private final SearchBuilder DomainSearch; @Inject - protected DomainDao _domainDao; + private DomainDao _domainDao; @Inject - protected AccountDao _accountDao; + private AccountDao _accountDao; public ResourceCountDaoImpl() { TypeSearch = createSearchBuilder(); @@ -252,16 +252,15 @@ public class ResourceCountDaoImpl extends GenericDaoBase return 0; } - private String baseSqlCountComputingResourceAllocatedToAccount = "Select " - + " SUM((CASE " + private String baseSqlCountComputingResourceAllocatedToAccount = "Select " + + " SUM((CASE " + " WHEN so.%s is not null THEN so.%s " - + " ELSE CONVERT(vmd.value, UNSIGNED INTEGER) " + + " ELSE CONVERT(vmd.value, UNSIGNED INTEGER) " + " END)) as total " - + " from vm_instance vm " + + " from vm_instance vm " + " join service_offering_view so on so.id = vm.service_offering_id " + " left join user_vm_details vmd on vmd.vm_id = vm.id and vmd.name = '%s' " + " where vm.type = 'User' and state not in ('Destroyed', 'Error', 'Expunging') and display_vm = true and account_id = ? "; - @Override public long countCpuNumberAllocatedToAccount(long accountId) { String sqlCountCpuNumberAllocatedToAccount = String.format(baseSqlCountComputingResourceAllocatedToAccount, ResourceType.cpu, ResourceType.cpu, "cpuNumber"); @@ -279,10 +278,10 @@ public class ResourceCountDaoImpl extends GenericDaoBase try (TransactionLegacy tx = TransactionLegacy.currentTxn()) { PreparedStatement pstmt = tx.prepareAutoCloseStatement(sqlCountComputingResourcesAllocatedToAccount); pstmt.setLong(1, accountId); - + ResultSet rs = pstmt.executeQuery(); if (!rs.next()) { - throw new CloudRuntimeException(String.format("An unexpected case happened while counting allocated computing resources for account: " + accountId)); + throw new CloudRuntimeException(String.format("An unexpected case happened while counting allocated computing resources for account: " + accountId)); } return rs.getLong("total"); } catch (SQLException e) { diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index df7276dcbe3..ca9c7ec26fd 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -69,7 +69,6 @@ import com.cloud.projects.Project; import com.cloud.projects.ProjectAccount.Role; import com.cloud.projects.dao.ProjectAccountDao; import com.cloud.projects.dao.ProjectDao; -import com.cloud.service.ServiceOfferingVO; import com.cloud.service.dao.ServiceOfferingDao; import com.cloud.storage.DataStoreRole; import com.cloud.storage.SnapshotVO; @@ -100,9 +99,6 @@ import com.cloud.utils.db.TransactionCallback; import com.cloud.utils.db.TransactionCallbackWithExceptionNoReturn; import com.cloud.utils.db.TransactionStatus; import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.vm.UserVmVO; -import com.cloud.vm.VirtualMachine; -import com.cloud.vm.VirtualMachine.State; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; @@ -280,10 +276,8 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim long numToDecrement = (delta.length == 0) ? 1 : delta[0].longValue(); if (!updateResourceCountForAccount(accountId, type, false, numToDecrement)) { - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_UPDATE_RESOURCE_COUNT, 0L, 0L, "Failed to decrement resource count of type " + type + - " for account id=" + - accountId, "Failed to decrement resource count of type " + type + " for account id=" + accountId + - "; use updateResourceCount API to recalculate/fix the problem"); + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_UPDATE_RESOURCE_COUNT, 0L, 0L, "Failed to decrement resource count of type " + type + " for account id=" + accountId, + "Failed to decrement resource count of type " + type + " for account id=" + accountId + "; use updateResourceCount API to recalculate/fix the problem"); } } @@ -426,13 +420,10 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim long domainResourceLimit = findCorrectResourceLimitForDomain(domain, type); long currentDomainResourceCount = _resourceCountDao.getResourceCount(domainId, ResourceOwnerType.Domain, type); long requestedDomainResourceCount = currentDomainResourceCount + numResources; - String messageSuffix = " domain resource limits of Type '" + type + "'" + - " for Domain Id = " + domainId + - " is exceeded: Domain Resource Limit = " + domainResourceLimit + - ", Current Domain Resource Amount = " + currentDomainResourceCount + - ", Requested Resource Amount = " + numResources + "."; + String messageSuffix = " domain resource limits of Type '" + type + "'" + " for Domain Id = " + domainId + " is exceeded: Domain Resource Limit = " + domainResourceLimit + + ", Current Domain Resource Amount = " + currentDomainResourceCount + ", Requested Resource Amount = " + numResources + "."; - if(s_logger.isDebugEnabled()) { + if (s_logger.isDebugEnabled()) { s_logger.debug("Checking if" + messageSuffix); } @@ -452,14 +443,11 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim long accountResourceLimit = findCorrectResourceLimitForAccount(account, type); long currentResourceCount = _resourceCountDao.getResourceCount(account.getId(), ResourceOwnerType.Account, type); long requestedResourceCount = currentResourceCount + numResources; - String messageSuffix = " amount of resources of Type = '" + type + "' for " + - (project == null ? "Account Name = " + account.getAccountName() : "Project Name = " + project.getName()) + - " in Domain Id = " + account.getDomainId() + - " is exceeded: Account Resource Limit = " + accountResourceLimit + - ", Current Account Resource Amount = " + currentResourceCount + - ", Requested Resource Amount = " + numResources + "."; + String messageSuffix = " amount of resources of Type = '" + type + "' for " + (project == null ? "Account Name = " + account.getAccountName() : "Project Name = " + project.getName()) + + " in Domain Id = " + account.getDomainId() + " is exceeded: Account Resource Limit = " + accountResourceLimit + ", Current Account Resource Amount = " + currentResourceCount + + ", Requested Resource Amount = " + numResources + "."; - if(s_logger.isDebugEnabled()) { + if (s_logger.isDebugEnabled()) { s_logger.debug("Checking if" + messageSuffix); } @@ -485,6 +473,7 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim return _resourceCountDao.lockRows(sc, null, true); } + @Override public long findDefaultResourceLimitForDomain(ResourceType resourceType) { Long resourceLimit = null; resourceLimit = domainResourceLimitMap.get(resourceType); @@ -522,7 +511,7 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim // check all domains in the account's domain hierarchy checkDomainResourceLimit(account, projectFinal, type, numResources); } - }); + }); } @Override @@ -611,11 +600,9 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim if (resourceType != null) { if (foundLimits.isEmpty()) { if (isAccount) { - limits.add(new ResourceLimitVO(resourceType, findCorrectResourceLimitForAccount(_accountMgr.getAccount(accountId), resourceType), accountId, - ResourceOwnerType.Account)); + limits.add(new ResourceLimitVO(resourceType, findCorrectResourceLimitForAccount(_accountMgr.getAccount(accountId), resourceType), accountId, ResourceOwnerType.Account)); } else { - limits.add(new ResourceLimitVO(resourceType, findCorrectResourceLimitForDomain(_domainDao.findById(domainId), resourceType), domainId, - ResourceOwnerType.Domain)); + limits.add(new ResourceLimitVO(resourceType, findCorrectResourceLimitForDomain(_domainDao.findById(domainId), resourceType), domainId, ResourceOwnerType.Domain)); } } else { limits.addAll(foundLimits); @@ -641,8 +628,7 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim if (accountLimitStr.size() < resourceTypes.length) { for (ResourceType rt : resourceTypes) { if (!accountLimitStr.contains(rt.toString()) && rt.supportsOwner(ResourceOwnerType.Account)) { - limits.add(new ResourceLimitVO(rt, findCorrectResourceLimitForAccount(_accountMgr.getAccount(accountId), rt), accountId, - ResourceOwnerType.Account)); + limits.add(new ResourceLimitVO(rt, findCorrectResourceLimitForAccount(_accountMgr.getAccount(accountId), rt), accountId, ResourceOwnerType.Account)); } } } @@ -651,8 +637,7 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim if (domainLimitStr.size() < resourceTypes.length) { for (ResourceType rt : resourceTypes) { if (!domainLimitStr.contains(rt.toString()) && rt.supportsOwner(ResourceOwnerType.Domain)) { - limits.add(new ResourceLimitVO(rt, findCorrectResourceLimitForDomain(_domainDao.findById(domainId), rt), domainId, - ResourceOwnerType.Domain)); + limits.add(new ResourceLimitVO(rt, findCorrectResourceLimitForDomain(_domainDao.findById(domainId), rt), domainId, ResourceOwnerType.Domain)); } } } @@ -708,9 +693,7 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim throw new InvalidParameterValueException("Only " + Resource.RESOURCE_UNLIMITED + " limit is supported for Root Admin accounts"); } - if ((caller.getAccountId() == accountId.longValue()) && - (_accountMgr.isDomainAdmin(caller.getId()) || - caller.getType() == Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN)) { + if ((caller.getAccountId() == accountId.longValue()) && (_accountMgr.isDomainAdmin(caller.getId()) || caller.getType() == Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN)) { // If the admin is trying to update his own account, disallow. throw new PermissionDeniedException("Unable to update resource limit for his own account " + accountId + ", permission denied"); } @@ -733,8 +716,7 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim throw new PermissionDeniedException("Cannot update resource limit for ROOT domain " + domainId + ", permission denied"); } - if ((caller.getDomainId() == domainId.longValue()) && caller.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN || - caller.getType() == Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN) { + if ((caller.getDomainId() == domainId.longValue()) && caller.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN || caller.getType() == Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN) { // if the admin is trying to update their own domain, disallow... throw new PermissionDeniedException("Unable to update resource limit for domain " + domainId + ", permission denied"); } @@ -743,8 +725,8 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim DomainVO parentDomain = _domainDao.findById(parentDomainId); long parentMaximum = findCorrectResourceLimitForDomain(parentDomain, resourceType); if ((parentMaximum >= 0) && (max.longValue() > parentMaximum)) { - throw new InvalidParameterValueException("Domain " + domain.getName() + "(id: " + parentDomain.getId() + ") has maximum allowed resource limit " + - parentMaximum + " for " + resourceType + ", please specify a value less that or equal to " + parentMaximum); + throw new InvalidParameterValueException("Domain " + domain.getName() + "(id: " + parentDomain.getId() + ") has maximum allowed resource limit " + parentMaximum + " for " + + resourceType + ", please specify a value less that or equal to " + parentMaximum); } } ownerType = ResourceOwnerType.Domain; @@ -766,8 +748,7 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim } @Override - public List recalculateResourceCount(Long accountId, Long domainId, Integer typeId) throws InvalidParameterValueException, CloudRuntimeException, - PermissionDeniedException { + public List recalculateResourceCount(Long accountId, Long domainId, Integer typeId) throws InvalidParameterValueException, CloudRuntimeException, PermissionDeniedException { Account callerAccount = CallContext.current().getCallingAccount(); long count = 0; List counts = new ArrayList(); @@ -818,25 +799,24 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim @DB protected boolean updateResourceCountForAccount(final long accountId, final ResourceType type, final boolean increment, final long delta) { - if(s_logger.isDebugEnabled()) { - s_logger.debug("Updating resource Type = " + type + " count for Account = " + accountId + - " Operation = " + (increment ? "increasing" : "decreasing") + " Amount = " + delta); + if (s_logger.isDebugEnabled()) { + s_logger.debug("Updating resource Type = " + type + " count for Account = " + accountId + " Operation = " + (increment ? "increasing" : "decreasing") + " Amount = " + delta); } try { return Transaction.execute(new TransactionCallback() { - @Override - public Boolean doInTransaction(TransactionStatus status) { - boolean result = true; - List rowsToUpdate = lockAccountAndOwnerDomainRows(accountId, type); - for (ResourceCountVO rowToUpdate : rowsToUpdate) { - if (!_resourceCountDao.updateById(rowToUpdate.getId(), increment, delta)) { - s_logger.trace("Unable to update resource count for the row " + rowToUpdate); - result = false; - } + @Override + public Boolean doInTransaction(TransactionStatus status) { + boolean result = true; + List rowsToUpdate = lockAccountAndOwnerDomainRows(accountId, type); + for (ResourceCountVO rowToUpdate : rowsToUpdate) { + if (!_resourceCountDao.updateById(rowToUpdate.getId(), increment, delta)) { + s_logger.trace("Unable to update resource count for the row " + rowToUpdate); + result = false; } - return result; } - }); + return result; + } + }); } catch (Exception ex) { s_logger.error("Failed to update resource count for account id=" + accountId); return false; @@ -846,102 +826,101 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim @DB protected long recalculateDomainResourceCount(final long domainId, final ResourceType type) { return Transaction.execute(new TransactionCallback() { - @Override - public Long doInTransaction(TransactionStatus status) { - long newResourceCount = 0; - lockDomainRows(domainId, type); - ResourceCountVO domainRC = _resourceCountDao.findByOwnerAndType(domainId, ResourceOwnerType.Domain, type); - long oldResourceCount = domainRC.getCount(); + @Override + public Long doInTransaction(TransactionStatus status) { + long newResourceCount = 0; + lockDomainRows(domainId, type); + ResourceCountVO domainRC = _resourceCountDao.findByOwnerAndType(domainId, ResourceOwnerType.Domain, type); + long oldResourceCount = domainRC.getCount(); - List domainChildren = _domainDao.findImmediateChildrenForParent(domainId); - // for each child domain update the resource count - if (type.supportsOwner(ResourceOwnerType.Domain)) { + List domainChildren = _domainDao.findImmediateChildrenForParent(domainId); + // for each child domain update the resource count + if (type.supportsOwner(ResourceOwnerType.Domain)) { - // calculate project count here - if (type == ResourceType.project) { - newResourceCount += _projectDao.countProjectsForDomain(domainId); - } - - for (DomainVO childDomain : domainChildren) { - long childDomainResourceCount = recalculateDomainResourceCount(childDomain.getId(), type); - newResourceCount += childDomainResourceCount; // add the child domain count to parent domain count - } + // calculate project count here + if (type == ResourceType.project) { + newResourceCount += _projectDao.countProjectsForDomain(domainId); } - if (type.supportsOwner(ResourceOwnerType.Account)) { - List accounts = _accountDao.findActiveAccountsForDomain(domainId); - for (AccountVO account : accounts) { - long accountResourceCount = recalculateAccountResourceCount(account.getId(), type); - newResourceCount += accountResourceCount; // add account's resource count to parent domain count - } + for (DomainVO childDomain : domainChildren) { + long childDomainResourceCount = recalculateDomainResourceCount(childDomain.getId(), type); + newResourceCount += childDomainResourceCount; // add the child domain count to parent domain count } - _resourceCountDao.setResourceCount(domainId, ResourceOwnerType.Domain, type, newResourceCount); - - if (oldResourceCount != newResourceCount) { - s_logger.warn("Discrepency in the resource count has been detected " + "(original count = " + oldResourceCount + - " correct count = " + newResourceCount + ") for Type = " + type + - " for Domain ID = " + domainId + " is fixed during resource count recalculation."); - } - - return newResourceCount; } - }); + + if (type.supportsOwner(ResourceOwnerType.Account)) { + List accounts = _accountDao.findActiveAccountsForDomain(domainId); + for (AccountVO account : accounts) { + long accountResourceCount = recalculateAccountResourceCount(account.getId(), type); + newResourceCount += accountResourceCount; // add account's resource count to parent domain count + } + } + _resourceCountDao.setResourceCount(domainId, ResourceOwnerType.Domain, type, newResourceCount); + + if (oldResourceCount != newResourceCount) { + s_logger.warn("Discrepency in the resource count has been detected " + "(original count = " + oldResourceCount + " correct count = " + newResourceCount + ") for Type = " + type + + " for Domain ID = " + domainId + " is fixed during resource count recalculation."); + } + + return newResourceCount; + } + }); } @DB protected long recalculateAccountResourceCount(final long accountId, final ResourceType type) { Long newCount = Transaction.execute(new TransactionCallback() { - @Override - public Long doInTransaction(TransactionStatus status) { - Long newCount = null; - lockAccountAndOwnerDomainRows(accountId, type); - ResourceCountVO accountRC = _resourceCountDao.findByOwnerAndType(accountId, ResourceOwnerType.Account, type); - long oldCount = 0; - if (accountRC != null) - oldCount = accountRC.getCount(); - - if (type == Resource.ResourceType.user_vm) { - newCount = _userVmDao.countAllocatedVMsForAccount(accountId); - } else if (type == Resource.ResourceType.volume) { - newCount = _volumeDao.countAllocatedVolumesForAccount(accountId); - long virtualRouterCount = _vmDao.findIdsOfAllocatedVirtualRoutersForAccount(accountId).size(); - newCount = newCount - virtualRouterCount; // don't count the volumes of virtual router - } else if (type == Resource.ResourceType.snapshot) { - newCount = _snapshotDao.countSnapshotsForAccount(accountId); - } else if (type == Resource.ResourceType.public_ip) { - newCount = calculatePublicIpForAccount(accountId); - } else if (type == Resource.ResourceType.template) { - newCount = _vmTemplateDao.countTemplatesForAccount(accountId); - } else if (type == Resource.ResourceType.project) { - newCount = _projectAccountDao.countByAccountIdAndRole(accountId, Role.Admin); - } else if (type == Resource.ResourceType.network) { - newCount = _networkDao.countNetworksUserCanCreate(accountId); - } else if (type == Resource.ResourceType.vpc) { - newCount = _vpcDao.countByAccountId(accountId); - } else if (type == Resource.ResourceType.cpu) { - newCount = countCpusForAccount(accountId); - } else if (type == Resource.ResourceType.memory) { - newCount = calculateMemoryForAccount(accountId); - } else if (type == Resource.ResourceType.primary_storage) { - List virtualRouters = _vmDao.findIdsOfAllocatedVirtualRoutersForAccount(accountId); - newCount = _volumeDao.primaryStorageUsedForAccount(accountId, virtualRouters); - } else if (type == Resource.ResourceType.secondary_storage) { - newCount = calculateSecondaryStorageForAccount(accountId); - } else { - throw new InvalidParameterValueException("Unsupported resource type " + type); - } - _resourceCountDao.setResourceCount(accountId, ResourceOwnerType.Account, type, (newCount == null) ? 0 : newCount.longValue()); - - // No need to log message for primary and secondary storage because both are recalculating the - // resource count which will not lead to any discrepancy. - if (!Long.valueOf(oldCount).equals(newCount) && - (type != Resource.ResourceType.primary_storage && type != Resource.ResourceType.secondary_storage)) { - s_logger.warn("Discrepency in the resource count " + "(original count=" + oldCount + " correct count = " + newCount + ") for type " + type + - " for account ID " + accountId + " is fixed during resource count recalculation."); - } - return newCount; + @Override + public Long doInTransaction(TransactionStatus status) { + Long newCount = null; + lockAccountAndOwnerDomainRows(accountId, type); + ResourceCountVO accountRC = _resourceCountDao.findByOwnerAndType(accountId, ResourceOwnerType.Account, type); + long oldCount = 0; + if (accountRC != null) { + oldCount = accountRC.getCount(); } - }); + + if (type == Resource.ResourceType.user_vm) { + newCount = _userVmDao.countAllocatedVMsForAccount(accountId); + } else if (type == Resource.ResourceType.volume) { + newCount = _volumeDao.countAllocatedVolumesForAccount(accountId); + long virtualRouterCount = _vmDao.findIdsOfAllocatedVirtualRoutersForAccount(accountId).size(); + newCount = newCount - virtualRouterCount; // don't count the volumes of virtual router + } else if (type == Resource.ResourceType.snapshot) { + newCount = _snapshotDao.countSnapshotsForAccount(accountId); + } else if (type == Resource.ResourceType.public_ip) { + newCount = calculatePublicIpForAccount(accountId); + } else if (type == Resource.ResourceType.template) { + newCount = _vmTemplateDao.countTemplatesForAccount(accountId); + } else if (type == Resource.ResourceType.project) { + newCount = _projectAccountDao.countByAccountIdAndRole(accountId, Role.Admin); + } else if (type == Resource.ResourceType.network) { + newCount = _networkDao.countNetworksUserCanCreate(accountId); + } else if (type == Resource.ResourceType.vpc) { + newCount = _vpcDao.countByAccountId(accountId); + } else if (type == Resource.ResourceType.cpu) { + newCount = countCpusForAccount(accountId); + } else if (type == Resource.ResourceType.memory) { + newCount = calculateMemoryForAccount(accountId); + } else if (type == Resource.ResourceType.primary_storage) { + List virtualRouters = _vmDao.findIdsOfAllocatedVirtualRoutersForAccount(accountId); + newCount = _volumeDao.primaryStorageUsedForAccount(accountId, virtualRouters); + } else if (type == Resource.ResourceType.secondary_storage) { + newCount = calculateSecondaryStorageForAccount(accountId); + } else { + throw new InvalidParameterValueException("Unsupported resource type " + type); + } + _resourceCountDao.setResourceCount(accountId, ResourceOwnerType.Account, type, (newCount == null) ? 0 : newCount.longValue()); + + // No need to log message for primary and secondary storage because both are recalculating the + // resource count which will not lead to any discrepancy. + if (!Long.valueOf(oldCount).equals(newCount) && (type != Resource.ResourceType.primary_storage && type != Resource.ResourceType.secondary_storage)) { + s_logger.warn("Discrepency in the resource count " + "(original count=" + oldCount + " correct count = " + newCount + ") for type " + type + " for account ID " + accountId + + " is fixed during resource count recalculation."); + } + return newCount; + } + }); return (newCount == null) ? 0 : newCount.longValue(); } @@ -1001,7 +980,7 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim return _resourceCountDao.getResourceCount(account.getId(), ResourceOwnerType.Account, type); } - private boolean isDisplayFlagOn(Boolean displayResource){ + private boolean isDisplayFlagOn(Boolean displayResource) { // 1. If its null assume displayResource = 1 // 2. If its not null then send true if displayResource = 1 From 601d095d71423ea7a22aa7e19f2f646b61f61478 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Tue, 6 Feb 2018 21:08:07 -0200 Subject: [PATCH 3/3] Python automated test case for updateResourceCount API method --- .../component/test_updateResourceCount.py | 235 ++++++++++++++++++ 1 file changed, 235 insertions(+) create mode 100644 test/integration/component/test_updateResourceCount.py diff --git a/test/integration/component/test_updateResourceCount.py b/test/integration/component/test_updateResourceCount.py new file mode 100644 index 00000000000..c9bd6e6f183 --- /dev/null +++ b/test/integration/component/test_updateResourceCount.py @@ -0,0 +1,235 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" Test update resource count API method +""" +# Import Local Modules +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.lib.utils import (random_gen, + cleanup_resources) +from marvin.lib.base import (Domain, + Account, + ServiceOffering, + VirtualMachine, + Network, + User, + NATRule, + Template, + PublicIPAddress, + Resources) +from marvin.lib.common import (get_domain, + get_zone, + get_template, + list_accounts, + list_virtual_machines, + list_service_offering, + list_templates, + list_users, + get_builtin_template_info, + wait_for_cleanup) +from nose.plugins.attrib import attr +from marvin.cloudstackException import CloudstackAPIException +import time + + +class Services: + + """These are some configurations that will get implemented in ACS. They are put here to follow some sort of standard that seems to be in place. + """ + + def __init__(self): + self.services = { + "account": { + "email": "test@test.com", + "firstname": "Test", + "lastname": "Tester", + "username": "test", + "password": "acountFirstUserPass", + }, + "service_offering_custom": { + "name": "Custom service offering test", + "displaytext": "Custom service offering test", + "cpunumber": None, + "cpuspeed": None, + # in MHz + "memory": None, + # In MBs + }, + "service_offering_normal": { + "name": "Normal service offering", + "displaytext": "Normal service offering", + "cpunumber": 2, + "cpuspeed": 1000, + # in MHz + "memory": 512, + # In MBs + }, + "virtual_machine": { + "displayname": "Test VM", + "username": "root", + "password": "password", + "ssh_port": 22, + "hypervisor": 'XenServer', + # Hypervisor type should be same as + # hypervisor type of cluster + "privateport": 22, + "publicport": 22, + "protocol": 'TCP', + }, + "ostype": 'CentOS 5.3 (64-bit)', + "sleep": 60, + "timeout": 10 + } + + +class TestUpdateResourceCount(cloudstackTestCase): + + @classmethod + def setUpClass(cls): + cls.testClient = super(TestUpdateResourceCount, cls).getClsTestClient() + cls.api_client = cls.testClient.getApiClient() + + cls.services = Services().services + cls.zone = get_zone(cls.api_client, cls.testClient.getZoneForTests()) + cls.services['mode'] = cls.zone.networktype + cls.template = get_template( + cls.api_client, + cls.zone.id, + cls.services["ostype"] + ) + cls.services["virtual_machine"]["zoneid"] = cls.zone.id + cls.services["virtual_machine"]["template"] = cls.template.id + + cls.service_offering_custom = ServiceOffering.create( + cls.api_client, + cls.services["service_offering_custom"] + ) + cls.service_offering_normal = ServiceOffering.create( + cls.api_client, + cls.services["service_offering_normal"] + ) + cls._cleanup = [cls.service_offering_custom, cls.service_offering_normal] + return + + @classmethod + def tearDownClass(cls): + try: + # Cleanup resources used + cleanup_resources(cls.api_client, cls._cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + def setUp(self): + self.apiclient = self.testClient.getApiClient() + self.dbclient = self.testClient.getDbConnection() + self.cleanup = [] + self.account = Account.create( + self.apiclient, + self.services["account"] + ) + self.debug("Created account: %s" % self.account.name) + self.cleanup.append(self.account) + + return + + def tearDown(self): + try: + # Clean up, terminate the created accounts, domains etc + cleanup_resources(self.apiclient, self.cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + @attr( + tags=[ + "advanced", + "basic", + "eip", + "advancedns", + "sg"], + required_hardware="false") + def test_01_updateResourceCount(self): + """Test update resource count for an account using a custom service offering to deploy a VM. + """ + + # This test will execute the following steps to assure resource count update is working properly + # 1. Create an account. + # 2. Start 2 VMs; one with normal service offering and other with a custom service offering + # 3. Call the update resource count method and check the CPU and memory values. + # The two VMs will add up to 3 CPUs and 1024Mb of RAM. + # 4. If the return of updateResourceCount method matches with the expected one, the test passes; otherwise, it fails. + # 5. Remove everything created by deleting the account + + vm_1 = VirtualMachine.create( + self.apiclient, + self.services["virtual_machine"], + accountid=self.account.name, + domainid=self.account.domainid, + serviceofferingid=self.service_offering_custom.id, + customcpunumber = 1, + customcpuspeed = 1000, + custommemory = 512 + ) + + self.debug("Deployed VM 1 in account: %s, ID: %s" % ( + self.account.name, + vm_1.id + )) + + vm_2 = VirtualMachine.create( + self.apiclient, + self.services["virtual_machine"], + accountid=self.account.name, + domainid=self.account.domainid, + serviceofferingid=self.service_offering_normal.id + ) + self.debug("Deployed VM 2 in account: %s, ID: %s" % ( + self.account.name, + vm_2.id + )) + + resourceCountCpu = Resources.updateCount( + self.apiclient, + resourcetype=8, + account=self.account.name, + domainid=self.account.domainid + ) + + self.debug("ResourceCount for CPU: %s" % ( + resourceCountCpu[0].resourcecount + )) + self.assertEqual( + resourceCountCpu[0].resourcecount, + 3, + "The number of CPU cores does not seem to be right." + ) + resourceCountMemory = Resources.updateCount( + self.apiclient, + resourcetype=9, + account=self.account.name, + domainid=self.account.domainid + ) + + self.debug("ResourceCount for memory: %s" % ( + resourceCountMemory[0].resourcecount + )) + self.assertEqual( + resourceCountMemory[0].resourcecount, + 1024, + "The memory amount does not seem to be right." + ) + return \ No newline at end of file