You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2018/02/23 14:07:43 UTC

[GitHub] rafaelweingartner closed pull request #2443: [CLOUDSTACK-9338] ACS is not accounting resources of VMs with custom service offering properly

rafaelweingartner closed pull request #2443:  [CLOUDSTACK-9338] ACS is not accounting resources of VMs with custom service offering properly
URL: https://github.com/apache/cloudstack/pull/2443
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

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..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,4 +57,18 @@
     Set<Long> 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..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
@@ -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;
@@ -24,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;
@@ -34,14 +34,18 @@
 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;
+import com.cloud.utils.exception.CloudRuntimeException;
 
 @Component
 public class ResourceCountDaoImpl extends GenericDaoBase<ResourceCountVO, Long> implements ResourceCountDao {
@@ -51,9 +55,9 @@
     private final SearchBuilder<ResourceCountVO> DomainSearch;
 
     @Inject
-    protected DomainDao _domainDao;
+    private DomainDao _domainDao;
     @Inject
-    protected AccountDao _accountDao;
+    private AccountDao _accountDao;
 
     public ResourceCountDaoImpl() {
         TypeSearch = createSearchBuilder();
@@ -248,4 +252,41 @@ 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 86fa46b6c26..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.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 void checkAccountResourceLimit(final Account account, final Project proj
         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 void doInTransactionWithoutResult(TransactionStatus status) throws Resour
         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 void doInTransactionWithoutResult(TransactionStatus status) throws Resour
                     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 void doInTransactionWithoutResult(TransactionStatus status) throws Resour
                     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<ResourceCountVO> recalculateResourceCount(Long accountId, Long domainId, Integer typeId) throws InvalidParameterValueException, CloudRuntimeException,
-        PermissionDeniedException {
+    public List<ResourceCountVO> recalculateResourceCount(Long accountId, Long domainId, Integer typeId) throws InvalidParameterValueException, CloudRuntimeException, PermissionDeniedException {
         Account callerAccount = CallContext.current().getCallingAccount();
         long count = 0;
         List<ResourceCountVO> counts = new ArrayList<ResourceCountVO>();
@@ -818,25 +799,24 @@ public ResourceLimitVO updateResourceLimit(Long accountId, Long domainId, Intege
 
     @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<Boolean>() {
-                    @Override
-                    public Boolean doInTransaction(TransactionStatus status) {
-                        boolean result = true;
-                        List<ResourceCountVO> 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<ResourceCountVO> 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,152 +826,111 @@ public Boolean doInTransaction(TransactionStatus status) {
     @DB
     protected long recalculateDomainResourceCount(final long domainId, final ResourceType type) {
         return Transaction.execute(new TransactionCallback<Long>() {
-                @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<DomainVO> 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<DomainVO> 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<AccountVO> 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<AccountVO> 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<Long>() {
-                @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<Long> 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<Long> 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();
     }
 
     public long countCpusForAccount(long accountId) {
-        GenericSearchBuilder<ServiceOfferingVO, SumCount> cpuSearch = _serviceOfferingDao.createSearchBuilder(SumCount.class);
-        cpuSearch.select("sum", Func.SUM, cpuSearch.entity().getCpu());
-        SearchBuilder<UserVmVO> 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<SumCount> 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<SumCount> 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<ServiceOfferingVO, SumCount> memorySearch = _serviceOfferingDao.createSearchBuilder(SumCount.class);
-        memorySearch.select("sum", Func.SUM, memorySearch.entity().getRamSize());
-        SearchBuilder<UserVmVO> 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<SumCount> 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<SumCount> 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) {
@@ -1041,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
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


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services