Skip to content

Commit 885f0dc

Browse files
BryanMLimadhslove
authored andcommitted
[Quota] Improve Quota balance calculation flow (apache#8581)
1 parent 309a564 commit 885f0dc

3 files changed

Lines changed: 250 additions & 50 deletions

File tree

framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java

Lines changed: 53 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.Arrays;
2323
import java.util.Date;
2424
import java.util.HashMap;
25+
import java.util.LinkedHashSet;
2526
import java.util.List;
2627
import java.util.Map;
2728
import java.util.TimeZone;
@@ -54,6 +55,7 @@
5455
import org.apache.commons.lang3.ObjectUtils;
5556
import org.apache.commons.lang3.StringUtils;
5657
import org.apache.commons.lang3.math.NumberUtils;
58+
import org.apache.commons.lang3.time.DateUtils;
5759
import org.springframework.stereotype.Component;
5860

5961
import com.cloud.usage.UsageVO;
@@ -145,79 +147,81 @@ protected void processQuotaBalanceForAccount(AccountVO accountVo, List<QuotaUsag
145147
return;
146148
}
147149

148-
QuotaUsageVO firstQuotaUsage = accountQuotaUsages.get(0);
149-
Date startDate = firstQuotaUsage.getStartDate();
150-
Date endDate = firstQuotaUsage.getStartDate();
150+
Date startDate = accountQuotaUsages.get(0).getStartDate();
151+
Date endDate = accountQuotaUsages.get(0).getEndDate();
152+
Date lastQuotaUsageEndDate = accountQuotaUsages.get(accountQuotaUsages.size() - 1).getEndDate();
151153

152-
logger.info("Processing quota balance for account [{}] between [{}] and [{}].", accountToString,
153-
DateUtil.displayDateInTimezone(usageAggregationTimeZone, startDate),
154-
DateUtil.displayDateInTimezone(usageAggregationTimeZone, accountQuotaUsages.get(accountQuotaUsages.size() - 1).getEndDate()));
154+
LinkedHashSet<Pair<Date, Date>> periods = accountQuotaUsages.stream()
155+
.map(quotaUsageVO -> new Pair<>(quotaUsageVO.getStartDate(), quotaUsageVO.getEndDate()))
156+
.collect(Collectors.toCollection(LinkedHashSet::new));
157+
158+
logger.info(String.format("Processing quota balance for account[%s] between [%s] and [%s].", accountToString, startDate, lastQuotaUsageEndDate));
155159

156-
BigDecimal aggregatedUsage = BigDecimal.ZERO;
157160
long accountId = accountVo.getAccountId();
158161
long domainId = accountVo.getDomainId();
162+
BigDecimal accountBalance = retrieveBalanceForUsageCalculation(accountId, domainId, startDate, accountToString);
159163

160-
aggregatedUsage = getUsageValueAccordingToLastQuotaUsageEntryAndLastQuotaBalance(accountId, domainId, startDate, endDate, aggregatedUsage, accountToString);
161-
162-
for (QuotaUsageVO quotaUsage : accountQuotaUsages) {
163-
Date quotaUsageStartDate = quotaUsage.getStartDate();
164-
Date quotaUsageEndDate = quotaUsage.getEndDate();
165-
BigDecimal quotaUsed = quotaUsage.getQuotaUsed();
166-
167-
if (quotaUsed.equals(BigDecimal.ZERO)) {
168-
aggregatedUsage = aggregatedUsage.add(aggregateCreditBetweenDates(accountId, domainId, quotaUsageStartDate, quotaUsageEndDate, accountToString));
169-
continue;
170-
}
171-
172-
if (startDate.compareTo(quotaUsageStartDate) == 0) {
173-
aggregatedUsage = aggregatedUsage.subtract(quotaUsed);
174-
continue;
175-
}
176-
177-
_quotaBalanceDao.saveQuotaBalance(new QuotaBalanceVO(accountId, domainId, aggregatedUsage, endDate));
164+
for (Pair<Date, Date> period : periods) {
165+
startDate = period.first();
166+
endDate = period.second();
178167

179-
aggregatedUsage = BigDecimal.ZERO;
180-
startDate = quotaUsageStartDate;
181-
endDate = quotaUsageEndDate;
168+
accountBalance = calculateBalanceConsideringCreditsAddedAndQuotaUsed(accountBalance, accountQuotaUsages, accountId, domainId, startDate, endDate, accountToString);
169+
_quotaBalanceDao.saveQuotaBalance(new QuotaBalanceVO(accountId, domainId, accountBalance, endDate));
170+
}
171+
saveQuotaAccount(accountId, accountBalance, endDate);
172+
}
182173

183-
QuotaBalanceVO lastRealBalanceEntry = _quotaBalanceDao.findLastBalanceEntry(accountId, domainId, endDate);
184-
Date lastBalanceDate = new Date(0);
174+
/**
175+
* Calculates the balance for the given account considering the specified period. The balance is calculated as follows:
176+
* <ol>
177+
* <li>The credits added in this period are added to the balance.</li>
178+
* <li>All quota consumed in this period are subtracted from the account balance.</li>
179+
* </ol>
180+
*/
181+
protected BigDecimal calculateBalanceConsideringCreditsAddedAndQuotaUsed(BigDecimal accountBalance, List<QuotaUsageVO> accountQuotaUsages, long accountId, long domainId,
182+
Date startDate, Date endDate, String accountToString) {
183+
accountBalance = accountBalance.add(aggregateCreditBetweenDates(accountId, domainId, startDate, endDate, accountToString));
185184

186-
if (lastRealBalanceEntry != null) {
187-
lastBalanceDate = lastRealBalanceEntry.getUpdatedOn();
188-
aggregatedUsage = aggregatedUsage.add(lastRealBalanceEntry.getCreditBalance());
185+
for (QuotaUsageVO quotaUsageVO : accountQuotaUsages) {
186+
if (DateUtils.isSameInstant(quotaUsageVO.getStartDate(), startDate)) {
187+
accountBalance = accountBalance.subtract(quotaUsageVO.getQuotaUsed());
189188
}
190-
191-
aggregatedUsage = aggregatedUsage.add(aggregateCreditBetweenDates(accountId, domainId, lastBalanceDate, endDate, accountToString));
192-
aggregatedUsage = aggregatedUsage.subtract(quotaUsed);
193189
}
194-
195-
_quotaBalanceDao.saveQuotaBalance(new QuotaBalanceVO(accountId, domainId, aggregatedUsage, endDate));
196-
saveQuotaAccount(accountId, aggregatedUsage, endDate);
190+
return accountBalance;
197191
}
198192

199-
protected BigDecimal getUsageValueAccordingToLastQuotaUsageEntryAndLastQuotaBalance(long accountId, long domainId, Date startDate, Date endDate, BigDecimal aggregatedUsage,
200-
String accountToString) {
193+
/**
194+
* Retrieves the initial balance prior to the period of the quota processing.
195+
* <ul>
196+
* <li>
197+
* If it is the first time of processing for the account, the credits prior to the quota processing are added, and the first balance is persisted in the DB.
198+
* </li>
199+
* <li>
200+
* Otherwise, the last real balance of the account is retrieved.
201+
* </li>
202+
* </ul>
203+
*/
204+
protected BigDecimal retrieveBalanceForUsageCalculation(long accountId, long domainId, Date startDate, String accountToString) {
205+
BigDecimal accountBalance = BigDecimal.ZERO;
201206
QuotaUsageVO lastQuotaUsage = _quotaUsageDao.findLastQuotaUsageEntry(accountId, domainId, startDate);
202207

203208
if (lastQuotaUsage == null) {
204-
aggregatedUsage = aggregatedUsage.add(aggregateCreditBetweenDates(accountId, domainId, new Date(0), startDate, accountToString));
205-
QuotaBalanceVO firstBalance = new QuotaBalanceVO(accountId, domainId, aggregatedUsage, startDate);
209+
accountBalance = accountBalance.add(aggregateCreditBetweenDates(accountId, domainId, new Date(0), startDate, accountToString));
210+
QuotaBalanceVO firstBalance = new QuotaBalanceVO(accountId, domainId, accountBalance, startDate);
206211

207212
logger.debug(String.format("Persisting the first quota balance [%s] for account [%s].", firstBalance, accountToString));
208213
_quotaBalanceDao.saveQuotaBalance(firstBalance);
209214
} else {
210-
QuotaBalanceVO lastRealBalance = _quotaBalanceDao.findLastBalanceEntry(accountId, domainId, endDate);
215+
QuotaBalanceVO lastRealBalance = _quotaBalanceDao.findLastBalanceEntry(accountId, domainId, startDate);
211216

212-
if (lastRealBalance != null) {
213-
aggregatedUsage = aggregatedUsage.add(lastRealBalance.getCreditBalance());
214-
aggregatedUsage = aggregatedUsage.add(aggregateCreditBetweenDates(accountId, domainId, lastRealBalance.getUpdatedOn(), endDate, accountToString));
215-
} else {
217+
if (lastRealBalance == null) {
216218
logger.warn(String.format("Account [%s] has quota usage entries, however it does not have a quota balance.", accountToString));
219+
} else {
220+
accountBalance = accountBalance.add(lastRealBalance.getCreditBalance());
217221
}
218222
}
219223

220-
return aggregatedUsage;
224+
return accountBalance;
221225
}
222226

223227
protected void saveQuotaAccount(long accountId, BigDecimal aggregatedUsage, Date endDate) {
Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing,
12+
# software distributed under the License is distributed on an
13+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
# KIND, either express or implied. See the License for the
15+
# specific language governing permissions and limitations
16+
# under the License.
17+
"""
18+
Test cases for validating the Quota balance of accounts
19+
"""
20+
21+
from marvin.cloudstackTestCase import *
22+
from marvin.lib.utils import *
23+
from marvin.lib.base import *
24+
from marvin.lib.common import *
25+
from nose.plugins.attrib import attr
26+
27+
28+
class TestQuotaBalance(cloudstackTestCase):
29+
30+
@classmethod
31+
def setUpClass(cls):
32+
testClient = super(TestQuotaBalance, cls).getClsTestClient()
33+
cls.apiclient = testClient.getApiClient()
34+
cls.services = testClient.getParsedTestDataConfig()
35+
cls.mgtSvrDetails = cls.config.__dict__["mgtSvr"][0].__dict__
36+
37+
# Get Zone, Domain and templates
38+
cls.domain = get_domain(cls.apiclient)
39+
cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
40+
cls.zone
41+
42+
# Create Account
43+
cls.account = Account.create(
44+
cls.apiclient,
45+
cls.services["account"],
46+
domainid=cls.domain.id
47+
)
48+
cls._cleanup = [
49+
cls.account,
50+
]
51+
52+
cls.services["account"] = cls.account.name
53+
54+
if not is_config_suitable(apiclient=cls.apiclient, name='quota.enable.service', value='true'):
55+
cls.debug("Quota service is not enabled, therefore the configuration `quota.enable.service` will be set to `true` and the management server will be restarted.")
56+
Configurations.update(cls.apiclient, "quota.enable.service", "true")
57+
cls.restartServer()
58+
59+
return
60+
61+
@classmethod
62+
def restartServer(cls):
63+
"""Restart management server"""
64+
65+
cls.debug("Restarting management server")
66+
sshClient = SshClient(
67+
cls.mgtSvrDetails["mgtSvrIp"],
68+
22,
69+
cls.mgtSvrDetails["user"],
70+
cls.mgtSvrDetails["passwd"]
71+
)
72+
73+
command = "service cloudstack-management restart"
74+
sshClient.execute(command)
75+
76+
# Waits for management to come up in 5 mins, when it's up it will continue
77+
timeout = time.time() + 300
78+
while time.time() < timeout:
79+
if cls.isManagementUp() is True:
80+
time.sleep(30)
81+
return
82+
time.sleep(5)
83+
return cls.fail("Management server did not come up, failing")
84+
85+
@classmethod
86+
def isManagementUp(cls):
87+
try:
88+
cls.apiclient.listInfrastructure(listInfrastructure.listInfrastructureCmd())
89+
return True
90+
except Exception:
91+
return False
92+
93+
@classmethod
94+
def tearDownClass(cls):
95+
try:
96+
# Cleanup resources used
97+
cleanup_resources(cls.apiclient, cls._cleanup)
98+
except Exception as e:
99+
raise Exception("Warning: Exception during cleanup : %s" % e)
100+
return
101+
102+
def setUp(self):
103+
self.apiclient = self.testClient.getApiClient()
104+
self.dbclient = self.testClient.getDbConnection()
105+
self.cleanup = []
106+
self.tariffs = []
107+
return
108+
109+
def tearDown(self):
110+
try:
111+
cleanup_resources(self.apiclient, self.cleanup)
112+
self.delete_tariffs()
113+
except Exception as e:
114+
raise Exception("Warning: Exception during cleanup : %s" % e)
115+
return
116+
117+
def delete_tariffs(self):
118+
for tariff in self.tariffs:
119+
cmd = quotaTariffDelete.quotaTariffDeleteCmd()
120+
cmd.id = tariff.uuid
121+
self.apiclient.quotaTariffDelete(cmd)
122+
123+
@attr(tags=["advanced", "smoke", "quota"], required_hardware="false")
124+
def test_quota_balance(self):
125+
"""
126+
Test Quota balance
127+
128+
Validate the following
129+
1. Add credits to an account
130+
2. Create Quota tariff for the usage type 21 (VM_DISK_IO_READ)
131+
3. Simulate quota usage by inserting a row in the `cloud_usage` table
132+
4. Update the balance of the account by calling the API quotaUpdate
133+
5. Verify the balance of the account according to the tariff created
134+
"""
135+
136+
# Create quota tariff for the usage type 21 (VM_DISK_IO_READ)
137+
cmd = quotaTariffCreate.quotaTariffCreateCmd()
138+
cmd.name = 'Tariff'
139+
cmd.value = '10'
140+
cmd.usagetype = '21'
141+
self.tariffs.append(self.apiclient.quotaTariffCreate(cmd))
142+
143+
# Add credits to the account
144+
cmd = quotaCredits.quotaCreditsCmd()
145+
cmd.account = self.account.name
146+
cmd.domainid = self.domain.id
147+
cmd.value = 100
148+
self.apiclient.quotaCredits(cmd)
149+
150+
# Fetch account ID from account_uuid
151+
account_id_select = f"SELECT id FROM account WHERE uuid = '{self.account.id}';"
152+
self.debug(account_id_select)
153+
qresultset = self.dbclient.execute(account_id_select)
154+
account_id = qresultset[0][0]
155+
156+
# Fetch domain ID from domain_uuid
157+
domain_id_select = f"SELECT id FROM `domain` d WHERE uuid = '{self.domain.id}';"
158+
self.debug(domain_id_select)
159+
qresultset = self.dbclient.execute(domain_id_select)
160+
domain_id = qresultset[0][0]
161+
162+
# Fetch zone ID from zone_uuid
163+
zone_id_select = f"SELECT id from data_center dc where dc.uuid = '{self.zone.id}';"
164+
self.debug(zone_id_select)
165+
qresultset = self.dbclient.execute(zone_id_select)
166+
zone_id = qresultset[0][0]
167+
168+
start_date = datetime.datetime.now() + datetime.timedelta(seconds=1)
169+
end_date = datetime.datetime.now() + datetime.timedelta(hours=1)
170+
171+
# Manually insert a usage regarding the usage type 21 (VM_DISK_IO_READ)
172+
sql_query = (f"INSERT INTO cloud_usage.cloud_usage (zone_id,account_id,domain_id,description,usage_display,usage_type,raw_usage,vm_instance_id,vm_name,offering_id,template_id,"
173+
f"usage_id,`type`,`size`,network_id,start_date,end_date,virtual_size,cpu_speed,cpu_cores,memory,quota_calculated,is_hidden,state)"
174+
f" VALUES ('{zone_id}','{account_id}','{domain_id}','Test','1 Hrs',21,1,NULL,NULL,NULL,NULL,NULL,'VirtualMachine',NULL,NULL,'{start_date}','{end_date}',NULL,NULL,NULL,NULL,0,0,NULL);")
175+
self.debug(sql_query)
176+
self.dbclient.execute(sql_query)
177+
178+
# Update quota to calculate the balance of the account
179+
cmd = quotaUpdate.quotaUpdateCmd()
180+
self.apiclient.quotaUpdate(cmd)
181+
182+
# Retrieve the quota balance of the account
183+
cmd = quotaBalance.quotaBalanceCmd()
184+
cmd.domainid = self.account.domainid
185+
cmd.account = self.account.name
186+
response = self.apiclient.quotaBalance(cmd)
187+
188+
self.debug(f"The quota balance for the account {self.account.name} is {response.balance}.")
189+
self.assertEqual(response.balance.startquota, 90, f"The `startQuota` response field is supposed to be 90 but was {response.balance.startquota}.")
190+
191+
return

usage/src/main/java/com/cloud/usage/UsageManagerImpl.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,9 +429,14 @@ protected void runInContextInternal() {
429429
cal.add(Calendar.MILLISECOND, -1);
430430
endDate = cal.getTime().getTime();
431431
} else {
432-
endDate = cal.getTime().getTime(); // current time
433432
cal.add(Calendar.MINUTE, -1 * _aggregationDuration);
433+
cal.set(Calendar.SECOND, 0);
434+
cal.set(Calendar.MILLISECOND, 0);
434435
startDate = cal.getTime().getTime();
436+
437+
cal.add(Calendar.MINUTE, _aggregationDuration);
438+
cal.add(Calendar.MILLISECOND, -1);
439+
endDate = cal.getTime().getTime();
435440
}
436441

437442
parse(job, startDate, endDate);

0 commit comments

Comments
 (0)