From 4d911fc3b7f285d69f0b773f78ecaa5ced97dffb Mon Sep 17 00:00:00 2001 From: Abhinandan Prateek Date: Fri, 18 Dec 2015 18:32:35 +0530 Subject: [PATCH] QUOTA: fix admin account check, add debug info for serv ice offering, mv credit entry check to a boolena method --- .../cloudstack/quota/QuotaManagerImpl.java | 3 + .../cloudstack/quota/vo/QuotaBalanceVO.java | 10 ++- .../quota/vo/ServiceOfferingVO.java | 81 ++++++------------- .../api/command/QuotaSummaryCmd.java | 2 +- .../response/QuotaResponseBuilderImpl.java | 2 +- .../{smoke => plugins}/test_quota.py | 47 +++++++++-- tools/marvin/marvin/config/setup.cfg | 4 + 7 files changed, 81 insertions(+), 68 deletions(-) rename test/integration/{smoke => plugins}/test_quota.py (82%) diff --git a/framework/quota/src/org/apache/cloudstack/quota/QuotaManagerImpl.java b/framework/quota/src/org/apache/cloudstack/quota/QuotaManagerImpl.java index 0a59fa20fd3..5e112243ba9 100644 --- a/framework/quota/src/org/apache/cloudstack/quota/QuotaManagerImpl.java +++ b/framework/quota/src/org/apache/cloudstack/quota/QuotaManagerImpl.java @@ -360,6 +360,9 @@ public class QuotaManagerImpl extends ManagerBase implements QuotaManager { BigDecimal rawusage; // get service offering details ServiceOfferingVO serviceoffering = _serviceOfferingDao.findServiceOffering(usageRecord.getVmInstanceId(), usageRecord.getOfferingId()); + if (s_logger.isDebugEnabled()) { + s_logger.debug(serviceoffering); + } if (serviceoffering == null) return quotalist; rawusage = new BigDecimal(usageRecord.getRawUsage()); diff --git a/framework/quota/src/org/apache/cloudstack/quota/vo/QuotaBalanceVO.java b/framework/quota/src/org/apache/cloudstack/quota/vo/QuotaBalanceVO.java index b454a14b925..509702a8671 100644 --- a/framework/quota/src/org/apache/cloudstack/quota/vo/QuotaBalanceVO.java +++ b/framework/quota/src/org/apache/cloudstack/quota/vo/QuotaBalanceVO.java @@ -62,7 +62,7 @@ public class QuotaBalanceVO implements InternalIdentity { this.accountId = credit.getAccountId(); this.domainId = credit.getDomainId(); this.creditBalance = credit.getCredit(); - this.updatedOn = credit.getUpdatedOn() == null ? null : new Date(credit.getUpdatedOn().getTime()); + this.updatedOn = credit.getUpdatedOn() == null ? null : new Date(credit.getUpdatedOn().getTime()); this.creditsId = credit.getId(); } @@ -72,7 +72,7 @@ public class QuotaBalanceVO implements InternalIdentity { this.domainId = domainId; this.creditBalance = creditBalance; this.creditsId = 0L; - this.updatedOn = updatedOn == null ? null : new Date(updatedOn.getTime()); + this.updatedOn = updatedOn == null ? null : new Date(updatedOn.getTime()); } @Override @@ -108,6 +108,10 @@ public class QuotaBalanceVO implements InternalIdentity { this.creditsId = creditsId; } + public boolean isBalanceEntry(){ + return creditsId==0; + } + public BigDecimal getCreditBalance() { return creditBalance; } @@ -121,7 +125,7 @@ public class QuotaBalanceVO implements InternalIdentity { } public void setUpdatedOn(Date updatedOn) { - this.updatedOn = updatedOn == null ? null : new Date(updatedOn.getTime()); + this.updatedOn = updatedOn == null ? null : new Date(updatedOn.getTime()); } @Override diff --git a/framework/quota/src/org/apache/cloudstack/quota/vo/ServiceOfferingVO.java b/framework/quota/src/org/apache/cloudstack/quota/vo/ServiceOfferingVO.java index ffba453352b..d62dad49553 100644 --- a/framework/quota/src/org/apache/cloudstack/quota/vo/ServiceOfferingVO.java +++ b/framework/quota/src/org/apache/cloudstack/quota/vo/ServiceOfferingVO.java @@ -90,7 +90,7 @@ public class ServiceOfferingVO extends DiskOfferingVO implements ServiceOffering } public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer speed, Integer rateMbps, Integer multicastRateMbps, boolean offerHA, String displayText, - ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse, VirtualMachine.Type vmType, boolean defaultUse) { + ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse, VirtualMachine.Type vmType, boolean defaultUse) { super(name, displayText, provisioningType, false, tags, recreatable, useLocalStorage, systemUse, true); this.cpu = cpu; this.ramSize = ramSize; @@ -105,7 +105,8 @@ public class ServiceOfferingVO extends DiskOfferingVO implements ServiceOffering } public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer speed, Integer rateMbps, Integer multicastRateMbps, boolean offerHA, boolean limitCpuUse, - boolean volatileVm, String displayText, ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse, VirtualMachine.Type vmType, Long domainId) { + boolean volatileVm, String displayText, ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse, + VirtualMachine.Type vmType, Long domainId) { super(name, displayText, provisioningType, false, tags, recreatable, useLocalStorage, systemUse, true, domainId); this.cpu = cpu; this.ramSize = ramSize; @@ -118,68 +119,26 @@ public class ServiceOfferingVO extends DiskOfferingVO implements ServiceOffering this.vmType = vmType == null ? null : vmType.toString().toLowerCase(); } - public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer speed, Integer rateMbps, Integer multicastRateMbps, boolean offerHA, - boolean limitResourceUse, boolean volatileVm, String displayText, ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse, - VirtualMachine.Type vmType, Long domainId, String hostTag) { - this(name, - cpu, - ramSize, - speed, - rateMbps, - multicastRateMbps, - offerHA, - limitResourceUse, - volatileVm, - displayText, - provisioningType, - useLocalStorage, - recreatable, - tags, - systemUse, - vmType, - domainId); + public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer speed, Integer rateMbps, Integer multicastRateMbps, boolean offerHA, boolean limitResourceUse, + boolean volatileVm, String displayText, ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse, + VirtualMachine.Type vmType, Long domainId, String hostTag) { + this(name, cpu, ramSize, speed, rateMbps, multicastRateMbps, offerHA, limitResourceUse, volatileVm, displayText, provisioningType, useLocalStorage, recreatable, tags, + systemUse, vmType, domainId); this.hostTag = hostTag; } - public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer speed, Integer rateMbps, Integer multicastRateMbps, boolean offerHA, - boolean limitResourceUse, boolean volatileVm, String displayText, ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse, - VirtualMachine.Type vmType, Long domainId, String hostTag, String deploymentPlanner) { - this(name, - cpu, - ramSize, - speed, - rateMbps, - multicastRateMbps, - offerHA, - limitResourceUse, - volatileVm, - displayText, - provisioningType, - useLocalStorage, - recreatable, - tags, - systemUse, - vmType, - domainId, - hostTag); + public ServiceOfferingVO(String name, Integer cpu, Integer ramSize, Integer speed, Integer rateMbps, Integer multicastRateMbps, boolean offerHA, boolean limitResourceUse, + boolean volatileVm, String displayText, ProvisioningType provisioningType, boolean useLocalStorage, boolean recreatable, String tags, boolean systemUse, + VirtualMachine.Type vmType, Long domainId, String hostTag, String deploymentPlanner) { + this(name, cpu, ramSize, speed, rateMbps, multicastRateMbps, offerHA, limitResourceUse, volatileVm, displayText, provisioningType, useLocalStorage, recreatable, tags, + systemUse, vmType, domainId, hostTag); this.deploymentPlanner = deploymentPlanner; } public ServiceOfferingVO(ServiceOfferingVO offering) { - super(offering.getId(), - offering.getName(), - offering.getDisplayText(), - offering.getProvisioningType(), - false, - offering.getTags(), - offering.isRecreatable(), - offering.getUseLocalStorage(), - offering.getSystemUse(), - true, - offering.isCustomizedIops()== null ? false:offering.isCustomizedIops(), - offering.getDomainId(), - offering.getMinIops(), - offering.getMaxIops()); + super(offering.getId(), offering.getName(), offering.getDisplayText(), offering.getProvisioningType(), false, offering.getTags(), offering.isRecreatable(), + offering.getUseLocalStorage(), offering.getSystemUse(), true, offering.isCustomizedIops() == null ? false : offering.isCustomizedIops(), offering.getDomainId(), + offering.getMinIops(), offering.getMaxIops()); cpu = offering.getCpu(); ramSize = offering.getRamSize(); speed = offering.getSpeed(); @@ -333,5 +292,11 @@ public class ServiceOfferingVO extends DiskOfferingVO implements ServiceOffering public void setDynamicFlag(boolean isdynamic) { isDynamic = isdynamic; } -} + @Override + public String toString() { + return "ServiceOfferingVO [cpu=" + cpu + ", speed=" + speed + ", ramSize=" + ramSize + ", rateMbps=" + rateMbps + ", multicastRateMbps=" + multicastRateMbps + ", offerHA=" + + offerHA + ", limitCpuUse=" + limitCpuUse + ", volatileVm=" + volatileVm + ", hostTag=" + hostTag + ", defaultUse=" + defaultUse + ", vmType=" + vmType + + ", sortKey=" + sortKey + ", deploymentPlanner=" + deploymentPlanner + ", details=" + details + ", isDynamic=" + isDynamic + "]"; + } +} diff --git a/plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaSummaryCmd.java b/plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaSummaryCmd.java index 7dd4dfee03e..0c5c0f2c42b 100644 --- a/plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaSummaryCmd.java +++ b/plugins/database/quota/src/org/apache/cloudstack/api/command/QuotaSummaryCmd.java @@ -59,7 +59,7 @@ public class QuotaSummaryCmd extends BaseListCmd { public void execute() { Account caller = CallContext.current().getCallingAccount(); List responses; - if (caller.getAccountId() <= 2) { // root admin or system + if (caller.getType() == Account.ACCOUNT_TYPE_ADMIN) { // root admin or system if (getAccountName() != null && getDomainId() != null) responses = _responseBuilder.createQuotaSummaryResponse(caller.getAccountName(), caller.getDomainId()); else diff --git a/plugins/database/quota/src/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java b/plugins/database/quota/src/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java index 6fd3aac3cd6..01640d585df 100644 --- a/plugins/database/quota/src/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java +++ b/plugins/database/quota/src/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java @@ -187,7 +187,7 @@ public class QuotaResponseBuilderImpl implements QuotaResponseBuilder { //check that there is at least one balance entry for (Iterator it = quotaBalance.iterator(); it.hasNext();) { QuotaBalanceVO entry = it.next(); - if (entry.getCreditsId() == 0) { + if (entry.isBalanceEntry()) { have_balance_entries = true; break; } diff --git a/test/integration/smoke/test_quota.py b/test/integration/plugins/test_quota.py similarity index 82% rename from test/integration/smoke/test_quota.py rename to test/integration/plugins/test_quota.py index e399d6337b5..1ae6790597a 100644 --- a/test/integration/smoke/test_quota.py +++ b/test/integration/plugins/test_quota.py @@ -30,8 +30,6 @@ from nose.plugins.attrib import attr #Import System modules import time -#ENABLE THE QUOTA PLUGIN AND RESTART THE MANAGEMENT SERVER TO RUN QUOTA TESTS - class TestQuota(cloudstackTestCase): @classmethod @@ -54,9 +52,6 @@ class TestQuota(cloudstackTestCase): cls._cleanup = [ cls.account, ] - cls._cleanup = [ - cls.account, - ] return def setUp(self): @@ -80,6 +75,12 @@ class TestQuota(cloudstackTestCase): #Check quotaTariffList API returning 22 items @attr(tags=["smoke", "advanced"], required_hardware="false") def test_01_quota(self): + if not is_config_suitable( + apiclient=self.apiclient, + name='quota.enable.service', + value='true'): + self.skipTest('quota.enable.service should be true. skipping') + cmd = quotaTariffList.quotaTariffListCmd() response = self.apiclient.quotaTariffList(cmd) @@ -100,6 +101,12 @@ class TestQuota(cloudstackTestCase): #Check quota tariff on a particualr day @attr(tags=["smoke", "advanced"], required_hardware="false") def test_02_quota(self): + if not is_config_suitable( + apiclient=self.apiclient, + name='quota.enable.service', + value='true'): + self.skipTest('quota.enable.service should be true. skipping') + cmd = quotaTariffList.quotaTariffListCmd() cmd.startdate='2015-07-06' response = self.apiclient.quotaTariffList(cmd) @@ -114,6 +121,12 @@ class TestQuota(cloudstackTestCase): #check quota tariff of a particular item @attr(tags=["smoke", "advanced"], required_hardware="false") def test_03_quota(self): + if not is_config_suitable( + apiclient=self.apiclient, + name='quota.enable.service', + value='true'): + self.skipTest('quota.enable.service should be true. skipping') + cmd = quotaTariffList.quotaTariffListCmd() cmd.startdate='2015-07-06' cmd.usagetype='10' @@ -132,6 +145,12 @@ class TestQuota(cloudstackTestCase): #check the old tariff it should be same @attr(tags=["smoke", "advanced"], required_hardware="false") def test_04_quota(self): + if not is_config_suitable( + apiclient=self.apiclient, + name='quota.enable.service', + value='true'): + self.skipTest('quota.enable.service should be true. skipping') + cmd = quotaTariffList.quotaTariffListCmd() cmd.startdate='2015-07-06' cmd.usagetype='10' @@ -182,6 +201,12 @@ class TestQuota(cloudstackTestCase): #Make credit deposit @attr(tags=["smoke", "advanced"], required_hardware="false") def test_05_quota(self): + if not is_config_suitable( + apiclient=self.apiclient, + name='quota.enable.service', + value='true'): + self.skipTest('quota.enable.service should be true. skipping') + cmd = quotaCredits.quotaCreditsCmd() cmd.domainid = self.account.domainid cmd.account = self.account.name @@ -198,6 +223,12 @@ class TestQuota(cloudstackTestCase): #Make credit deposit and check today balance @attr(tags=["smoke", "advanced"], required_hardware="false") def test_06_quota(self): + if not is_config_suitable( + apiclient=self.apiclient, + name='quota.enable.service', + value='true'): + self.skipTest('quota.enable.service should be true. skipping') + cmd = quotaBalance.quotaBalanceCmd() today = datetime.date.today() cmd.domainid = self.account.domainid @@ -214,6 +245,12 @@ class TestQuota(cloudstackTestCase): #make credit deposit and check start and end date balances @attr(tags=["smoke", "advanced"], required_hardware="false") def test_07_quota(self): + if not is_config_suitable( + apiclient=self.apiclient, + name='quota.enable.service', + value='true'): + self.skipTest('quota.enable.service should be true. skipping') + cmd = quotaBalance.quotaBalanceCmd() today = datetime.date.today() cmd.domainid = self.account.domainid diff --git a/tools/marvin/marvin/config/setup.cfg b/tools/marvin/marvin/config/setup.cfg index 23981f0f34d..f694bbadaec 100644 --- a/tools/marvin/marvin/config/setup.cfg +++ b/tools/marvin/marvin/config/setup.cfg @@ -142,6 +142,10 @@ "LogFolderPath": "/tmp/" }, "globalConfig": [ + { + "name": "quota.enable.service", + "value": "true" + }, { "name": "network.gc.wait", "value": "60"