-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Quota tariff order #8347
Quota tariff order #8347
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8347 +/- ##
============================================
+ Coverage 4.34% 31.04% +26.70%
- Complexity 0 33910 +33910
============================================
Files 361 5405 +5044
Lines 29004 380315 +351311
Branches 5092 55505 +50413
============================================
+ Hits 1260 118079 +116819
- Misses 27604 246497 +218893
- Partials 140 15739 +15599
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clgtm and well covered by tests, however as this is user facinng functionality, doe sit make sense to add an integration test @JoaoJandre ?
Sure, I'll try to add some integration tests |
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
@DaanHoogland I've created the integration test. However, while testing it, I found out that the test only works alongside #8581, thus, this PR now depends on #8581. |
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
@blueorangutan package |
@JoaoJandre a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8651 |
@blueorangutan test alma9 kvm-alma9 |
@DaanHoogland a [SL] Trillian-Jenkins test job (alma9 mgmt + kvm-alma9) has been kicked to run smoke tests |
...ins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffUpdateCmd.java
Outdated
Show resolved
Hide resolved
...ins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffCreateCmd.java
Outdated
Show resolved
Hide resolved
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
@blueorangutan package |
@JoaoJandre a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✖️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9864 |
@DaanHoogland @sureshanaparti @rohityadavcloud could we run the CI here? |
@JoaoJandre we are prioritising 4.19 now. /me postponing this. |
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
@blueorangutan package |
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10315 |
@blueorangutan test |
@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I manually tested the PR in a local environment.
Tests Descriptions
quotaTariffCreate
API
Firstly, I created a tariff without specifying a value for the position parameter.
quota tariffcreate usagetype=1 name="Running VM - Position 1" value=10
{
"quotatariff": {
"currency": "$",
"effectiveDate": "2024-07-11T22:03:45+0000",
"id": "7fcaf5f0-19e3-4211-8a2e-43c085db8cb7",
"name": "Running VM - Position 1",
"position": 1,
"tariffValue": 10,
"usageDiscriminator": "None",
"usageName": "RUNNING_VM",
"usageType": 1,
"usageTypeDescription": "Running Vm Usage",
"usageUnit": "Compute*Month"
}
}
The tariff has been created with the position set to 1
, which is the expected behavior.
I created another tariff, with the same usage type, but this time I specified a position.
quota tariffcreate usagetype=1 name="Running VM - Position 2" position=2 value=20
{
"quotatariff": {
"currency": "$",
"effectiveDate": "2024-07-11T22:04:30+0000",
"id": "eb815532-97ce-4e1d-81c5-d5872b732502",
"name": "Running VM - Position 2",
"position": 2,
"tariffValue": 20,
"usageDiscriminator": "None",
"usageName": "RUNNING_VM",
"usageType": 1,
"usageTypeDescription": "Running Vm Usage",
"usageUnit": "Compute*Month"
}
}
The tariff has been created, and the position attribute was set to 2
.
quotaTariffUpdate
API
I updated the position of the Running VM - Position 1
tariff to 3
.
quota tariffupdate name="Running VM - Position 1" position=3
{
"quotatariff": {
"currency": "$",
"effectiveDate": "2024-07-11T22:03:45+0000",
"id": "ef69dcba-51a6-462a-93f1-5e08178faeaf",
"name": "Running VM - Position 1",
"position": 3,
"tariffValue": 10,
"usageDiscriminator": "None",
"usageName": "RUNNING_VM",
"usageType": 1,
"usageTypeDescription": "Running Vm Usage",
"usageUnit": "Compute*Month"
}
}
The position attribute was updated accordingly to 3
.
I edited the same tariff again without entering a new position.
quota tariffupdate name="Running VM - Position 1" value=15
{
"quotatariff": {
"currency": "$",
"effectiveDate": "2024-07-11T22:03:45+0000",
"id": "a2215dac-9901-43aa-bf99-aa2f7a6d65a3",
"name": "Running VM - Position 1",
"position": 3,
"tariffValue": 15,
"usageDiscriminator": "None",
"usageName": "RUNNING_VM",
"usageType": 1,
"usageTypeDescription": "Running Vm Usage",
"usageUnit": "Compute*Month"
}
}
The position value remained unchanged, which is the expected behavior.
lastTariffs
preset variable
I updated the Running VM - Position 1
with its original value and position.
quota tariffupdate name="Running VM - Position 1" position=1 value=10
{
"quotatariff": {
"currency": "$",
"effectiveDate": "2024-07-11T22:03:45+0000",
"id": "24e25d39-9a4f-4915-8cf8-828b5c4fc9e3",
"name": "Running VM - Position 1",
"position": 1,
"tariffValue": 10,
"usageDiscriminator": "None",
"usageName": "RUNNING_VM",
"usageType": 1,
"usageTypeDescription": "Running Vm Usage",
"usageUnit": "Compute*Month"
}
}
I created a third tariff with a position equal to 3 and an activation rule that returns the sum of the values of the two previous tariffs increased by 20%
.
quota tariffcreate usagetype=1 name="Running VM - Position 3" value=0 position=3 activationrule="(lastTariffs[0].value + lastTariffs[1].value) * 1.2"
{
"quotatariff": {
"activationRule": "(lastTariffs[0].value + lastTariffs[1].value) * 1.2",
"currency": "$",
"effectiveDate": "2024-07-11T22:09:18+0000",
"id": "733e1df4-c3b9-4e31-b9eb-535fdc7311a3",
"name": "Running VM - Position 3",
"position": 3,
"tariffValue": 0,
"usageDiscriminator": "None",
"usageName": "RUNNING_VM",
"usageType": 1,
"usageTypeDescription": "Running Vm Usage",
"usageUnit": "Compute*Month"
}
}
I executed quota update
and checked the logs for the tariff values. The following log appeared, stating that the value of the third tariff is equal to 36 (10 + 20) * 1.2
:
DEBUG [o.a.c.q.QuotaManagerImpl] (qtp1404565079-27:[ctx-692ce99d, ctx-a2776cd3]) (logid:22a7aec5) The script [(lastTariffs[0].value + lastTariffs[1].value) * 1.2] of quota tariff [{"name":"Running VM - Position 3","usageName":"RUNNING_VM","uuid":"733e1df4-c3b9-4e31-b9eb-535fdc7311a3","startDate":"2024-07-11T22:09:18+0000","endDate":"null"}] had a numeric value [36.0], therefore we will use it in the calculation.
@JoaoJandre, once this PR gets merged, I intend to add UI support for the position parameter on #9225. |
[SF] Trillian test result (tid-10803)
|
... I'm guessing these are flukes, apllyining well tested insanity method |
@blueorangutan test |
@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
[SF] Trillian test result (tid-10811)
|
@blueorangutan test alma9 kvm-alma9 |
@DaanHoogland a [SL] Trillian-Jenkins test job (alma9 mgmt + kvm-alma9) has been kicked to run smoke tests |
[SF] Trillian test result (tid-10824)
|
Description
Quota tariffs have been extended to make it possible to define the order of their execution. To achieve this, the
position
parameter was added to thequotaTariffCreate
andquotaTariffUpdate
APIs. This parameter is used to define the tariff execution sequence of tariffs with the same usage type. This order is ascending, that is, a tariff withposition
equal to 1 will be executed before a tariff withposition
equal to 2. If two tariffs of the same usage type have equal positions, the most recent one will be executed first.During the processing of Quota tariffs, ACS will execute them in the defined order and inject the values into subsequent rates. To enable tariff injection, the preset variable
lastTariffs
was created. It will have a list of objects containing theid
andvalue
attributes of the tariffs executed before it. Below is an example of the variable value when executing 3 tariffs (A
,B
andC
) of the same usage type:A
, the value oflastTariffs
will be:B
,lastTariffs
will have the value:C
,lastTariffs
will have the values:Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
How Has This Been Tested?
About the
quotaTariffCreate
andquotaTariffUpdate
APIsAbout the
lastTariffs
preset variable:I created three tariffs with activation rules:
lastTariffs[0].value + 7
lastTariffs[0].value + lastTariffs[1].value
.When running
quotaUpdate
, I looked in the logs for which values were calculated according to the tariffs, the first returned 10, the second 17 and the third 27.