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 d4695c3ff751..f5b76e3e7fcb 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 f7cd3cbf86f6..3705418f98dd 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.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 long removeEntriesByOwner(long ownerId, ResourceOwnerType ownerType) { 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 86fa46b6c265..df7276dcbe37 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 Long doInTransaction(TransactionStatus status) { } 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 | 253 ++++++++---------- 3 files changed, 131 insertions(+), 153 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 f5b76e3e7fcb..b5a75d196fa5 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 3705418f98dd..56261337faf1 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 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.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 long removeEntriesByOwner(long ownerId, ResourceOwnerType ownerType) { 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 @@ private long executeSqlCountComputingResourcesForAccount(long accountId, String 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 df7276dcbe37..ca9c7ec26fdc 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.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.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 void decrementResourceCount(long accountId, ResourceType type, Long... de 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 @@ private void checkDomainResourceLimit(final Account account, final Project proje 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 @@ private void checkAccountResourceLimit(final Account account, final Project proj 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 + "."; - - if(s_logger.isDebugEnabled()) { + 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()) { s_logger.debug("Checking if" + messageSuffix); } @@ -485,6 +473,7 @@ private List lockDomainRows(long domainId, final ResourceType t return _resourceCountDao.lockRows(sc, null, true); } + @Override public long findDefaultResourceLimitForDomain(ResourceType resourceType) { Long resourceLimit = null; resourceLimit = domainResourceLimitMap.get(resourceType); @@ -522,7 +511,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) throws Resour // check all domains in the account's domain hierarchy checkDomainResourceLimit(account, projectFinal, type, numResources); } - }); + }); } @Override @@ -611,11 +600,9 @@ public List searchForLimits(Long id, Long accountId, Long domai 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 List searchForLimits(Long id, Long accountId, Long domai 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 List searchForLimits(Long id, Long accountId, Long domai 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 ResourceLimitVO updateResourceLimit(Long accountId, Long domainId, Intege 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 ResourceLimitVO updateResourceLimit(Long accountId, Long domainId, Intege 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 ResourceLimitVO updateResourceLimit(Long accountId, Long domainId, Intege 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 ResourceLimitVO updateResourceLimit(Long accountId, Long domainId, Intege } @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 List recalculateResourceCount(Long accountId, Long domai @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 Boolean doInTransaction(TransactionStatus status) { @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(); - - 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); - } + @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)) { - 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."); + 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); - return 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 long getResourceCount(Account account, ResourceType type) { 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 000000000000..c9bd6e6f183a --- /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