Create API to list Quota credits
Description
When credits are added to an account, an entry that represents the operation is created in cloud_usage.quota_credits. Some information about these entries is currently returned alongside an account's balance through the quotaBalance API. However, this is not consistent with the API's purpose (viewing an account's balance), and it is not possible to view the credit addition history for a whole domain with a single API call.
In order to make quotaBalance more consistent with its purpose, the credit addition history will be removed from its response alongside some enhancements to the API in the future. To list the credit addition history, this PR adds the quotaCreditsList, which has the following parameters:
accountid: ID of the account for which the credit statement will be generated.domainid: ID of the domain for which credit statement will be generated. Available only for administrators.isrecursive: Whether to generate the credit statement for the provided domain and its children. Defaults to false.startdate: Start date of the statement. If not provided, the first day of the current month will be considered as the start date.enddate: End date of the statement. If not provided, the current date will be considered as the end date.
No parameters are flagged as required, but at least accountid or domainid must be provided.
Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [X] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
- [ ] Major
- [X] Minor
How Has This Been Tested?
In my environment, there were the following domains and accounts:
ROOT: admin (root admin)ROOT/d1: d1 (domain admin), u1 (user)
First, in the root admin account, I added 100 credits to the three accounts; in the domain admin account, I added credits to d1 and u1. Then, I tested the behavior of the API for each account.
For the root admin account:
- I listed admin's credits history and verified that the response had the expected results (
accountidtested).
(admin) π’ > quota creditslist accountid=60b92e66-28b2-11ef-a04e-6ac09c2b3d6b
{
"count": 1,
"credit": [
{
"credit": 100,
"creditedon": "2024-08-18T22:45:39+0000",
"creditoruserid": "60ba931c-28b2-11ef-a04e-6ac09c2b3d6b",
"creditorusername": "admin",
"currency": "$"
}
]
}
- I listed d1's credits history and verified that the response had the expected results.
(admin) π’ > quota creditslist accountid=a6b8f3b0-0f4f-4762-86eb-df229fe602ed
{
"count": 2,
"credit": [
{
"credit": 100,
"creditedon": "2024-08-18T22:45:53+0000",
"creditoruserid": "60ba931c-28b2-11ef-a04e-6ac09c2b3d6b",
"creditorusername": "admin",
"currency": "$"
},
{
"credit": 200,
"creditedon": "2024-08-18T22:46:06+0000",
"creditoruserid": "5bd602d7-5d43-4d68-9619-d413d0458a2d",
"creditorusername": "d1",
"currency": "$"
}
]
}
- I listed u1's credits history and verified that the response had the expected results.
(admin) π’ > quota creditslist accountid=2560f486-38f1-42bf-aa0f-c68abe7ad15e
{
"count": 2,
"credit": [
{
"credit": 100,
"creditedon": "2024-08-18T22:45:58+0000",
"creditoruserid": "60ba931c-28b2-11ef-a04e-6ac09c2b3d6b",
"creditorusername": "admin",
"currency": "$"
},
{
"credit": 200,
"creditedon": "2024-08-18T22:46:10+0000",
"creditoruserid": "5bd602d7-5d43-4d68-9619-d413d0458a2d",
"creditorusername": "d1",
"currency": "$"
}
]
}
- I listed d1's credits specifying the interval so that only the first addition was shown (
enddatetested).
(admin) π’ > quota creditslist accountid=a6b8f3b0-0f4f-4762-86eb-df229fe602ed enddate=2024-08-18T22:46:00+0000
{
"count": 1,
"credit": [
{
"credit": 100,
"creditedon": "2024-08-18T22:45:53+0000",
"creditoruserid": "60ba931c-28b2-11ef-a04e-6ac09c2b3d6b",
"creditorusername": "admin",
"currency": "$"
}
]
}
- I listed d1's credits specifying the interval so that only the last addition was shown (
startdatetested).
(admin) π’ > quota creditslist accountid=a6b8f3b0-0f4f-4762-86eb-df229fe602ed startdate=2024-08-18T22:46:00+0000
{
"count": 1,
"credit": [
{
"credit": 200,
"creditedon": "2024-08-18T22:46:06+0000",
"creditoruserid": "5bd602d7-5d43-4d68-9619-d413d0458a2d",
"creditorusername": "d1",
"currency": "$"
}
]
}
- I listed
ROOT's credits and verified that the response had the expected results (domainidtested).
(admin) π’ > quota creditslist domainid=40815ad7-28b2-11ef-a04e-6ac09c2b3d6b
{
"count": 1,
"credit": [
{
"credit": 100,
"creditedon": "2024-08-18T22:45:39+0000",
"creditoruserid": "60ba931c-28b2-11ef-a04e-6ac09c2b3d6b",
"creditorusername": "admin",
"currency": "$"
}
]
}
- I listed
ROOT's credits recursively and verified that the response had the expected results (isrecursivetested).
(admin) π’ > quota creditslist domainid=40815ad7-28b2-11ef-a04e-6ac09c2b3d6b isrecursive=true
{
"count": 5,
"credit": [
{
"credit": 100,
"creditedon": "2024-08-18T22:45:39+0000",
"creditoruserid": "60ba931c-28b2-11ef-a04e-6ac09c2b3d6b",
"creditorusername": "admin",
"currency": "$"
},
{
"credit": 100,
"creditedon": "2024-08-18T22:45:53+0000",
"creditoruserid": "60ba931c-28b2-11ef-a04e-6ac09c2b3d6b",
"creditorusername": "admin",
"currency": "$"
},
{
"credit": 100,
"creditedon": "2024-08-18T22:45:58+0000",
"creditoruserid": "60ba931c-28b2-11ef-a04e-6ac09c2b3d6b",
"creditorusername": "admin",
"currency": "$"
},
{
"credit": 200,
"creditedon": "2024-08-18T22:46:06+0000",
"creditoruserid": "5bd602d7-5d43-4d68-9619-d413d0458a2d",
"creditorusername": "d1",
"currency": "$"
},
{
"credit": 200,
"creditedon": "2024-08-18T22:46:10+0000",
"creditoruserid": "5bd602d7-5d43-4d68-9619-d413d0458a2d",
"creditorusername": "d1",
"currency": "$"
}
]
}
For the domain admin account:
- I tried to list admin's credits history and verified that my permission was denied.
(d1) π’ > quota creditslist accountid=2
π Error: (HTTP 531, error code 4365) Caller does not have permission to operate with provided resource.
- I listed d1's credits history and verified that the response had the expected results.
(d1) π’ > quota creditslist accountid=a6b8f3b0-0f4f-4762-86eb-df229fe602ed
{
"count": 2,
"credit": [
{
"credit": 100,
"creditedon": "2024-08-18T22:45:53+0000",
"creditoruserid": "60ba931c-28b2-11ef-a04e-6ac09c2b3d6b",
"creditorusername": "admin",
"currency": "$"
},
{
"credit": 200,
"creditedon": "2024-08-18T22:46:06+0000",
"creditoruserid": "5bd602d7-5d43-4d68-9619-d413d0458a2d",
"creditorusername": "d1",
"currency": "$"
}
]
}
- I listed u1's credits history and verified that the response had the expected results.
(d1) π’ > quota creditslist accountid=2560f486-38f1-42bf-aa0f-c68abe7ad15e
{
"count": 2,
"credit": [
{
"credit": 100,
"creditedon": "2024-08-18T22:45:58+0000",
"creditoruserid": "60ba931c-28b2-11ef-a04e-6ac09c2b3d6b",
"creditorusername": "admin",
"currency": "$"
},
{
"credit": 200,
"creditedon": "2024-08-18T22:46:10+0000",
"creditoruserid": "5bd602d7-5d43-4d68-9619-d413d0458a2d",
"creditorusername": "d1",
"currency": "$"
}
]
}
- I listed
ROOT/d1's credits history and verified that the response had the expected results.
(d1) π’ > quota creditslist domainid=02da877c-615f-4a7d-a68d-bde6b6d5d01c
{
"count": 4,
"credit": [
{
"credit": 100,
"creditedon": "2024-08-18T22:45:53+0000",
"creditoruserid": "60ba931c-28b2-11ef-a04e-6ac09c2b3d6b",
"creditorusername": "admin",
"currency": "$"
},
{
"credit": 100,
"creditedon": "2024-08-18T22:45:58+0000",
"creditoruserid": "60ba931c-28b2-11ef-a04e-6ac09c2b3d6b",
"creditorusername": "admin",
"currency": "$"
},
{
"credit": 200,
"creditedon": "2024-08-18T22:46:06+0000",
"creditoruserid": "5bd602d7-5d43-4d68-9619-d413d0458a2d",
"creditorusername": "d1",
"currency": "$"
},
{
"credit": 200,
"creditedon": "2024-08-18T22:46:10+0000",
"creditoruserid": "5bd602d7-5d43-4d68-9619-d413d0458a2d",
"creditorusername": "d1",
"currency": "$"
}
]
}
For the user account:
- I tried to list admin's credits history and verified that my permission was denied.
(u1) π’ > quota creditslist accountid=2
π Error: (HTTP 531, error code 4365) Caller does not have permission to operate with provided resource.
- I tried to list d1's credits history and verified that my permission was denied.
(u1) π’ > quota creditslist accountid=a6b8f3b0-0f4f-4762-86eb-df229fe602ed
π Error: (HTTP 531, error code 4365) Caller does not have permission to operate with provided resource.
- I listed u1's credits history and verified that the response had the expected results.
(u1) π’ > quota creditslist accountid=2560f486-38f1-42bf-aa0f-c68abe7ad15e
{
"count": 2,
"credit": [
{
"credit": 100,
"creditedon": "2024-08-18T22:45:58+0000",
"creditoruserid": "60ba931c-28b2-11ef-a04e-6ac09c2b3d6b",
"creditorusername": "admin",
"currency": "$"
},
{
"credit": 200,
"creditedon": "2024-08-18T22:46:10+0000",
"creditoruserid": "5bd602d7-5d43-4d68-9619-d413d0458a2d",
"creditorusername": "d1",
"currency": "$"
}
]
}
- I tried to list the credits history for
ROOT/d1and verified that my permission was denied.
(u1) π’ > quota creditslist domainid=2
π Error: (HTTP 531, error code 4365) Regular users are not allowed to generate domain statements.
@blueorangutan package
@winterhazel 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.
Codecov Report
Attention: Patch coverage is 68.58974% with 49 lines in your changes missing coverage. Please review.
Project coverage is 16.08%. Comparing base (
d1cf453) to head (542c4c5). Report is 20 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #9590 +/- ##
============================================
+ Coverage 16.07% 16.08% +0.01%
- Complexity 12890 12914 +24
============================================
Files 5642 5643 +1
Lines 494087 494196 +109
Branches 59931 59940 +9
============================================
+ Hits 79425 79514 +89
- Misses 405844 405863 +19
- Partials 8818 8819 +1
| Flag | Coverage Ξ | |
|---|---|---|
| uitests | 4.01% <ΓΈ> (ΓΈ) |
|
| unittests | 16.93% <68.58%> (+0.01%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Packaging result [SF]: βοΈ el8 βοΈ el9 βοΈ debian βοΈ suse15. SL-JID 10810
@blueorangutan test keepEnv
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
[SF] Trillian test result (tid-11210) Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8 Total time taken: 48891 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9590-t11210-kvm-ol8.zip Smoke tests completed. 139 look OK, 0 have errors, 0 did not run Only failed and skipped tests results shown below:
| Test | Result | Time (s) | Test File |
|---|
@winterhazel , I think this is good to go safe one concern; removing response data from an API response is a backwards incompatibility. I can not judge the gravity (and haven't tested this PR and am not a quota user so just take my concern a little seriously ;) but the principle should only be broken after careful consideration. Can you give some stronger arguments than consistency for removing it, please?
Hey @DaanHoogland, my idea of removing the credits listing from quotaBalance was to make each API have a single responsibility and make credits-related code changes easier in the future (as we would not have to worry about checking and updating quotaBalance accordingly anymore when introducing these changes). I don't have a stronger argument for it π
Regarding the gravity of removing it: Quota users would be advised to use quotaCreditsList to list credit additions instead. However, scripts/automation tools/external software that use quotaBalance to get the credit addition history of accounts and were not updated before upgrading CloudStack would break. The Quota UI currently uses it to show a list containing the balance and credit addition entries at the same time, but this page will be reworked in the future to separate them, using quotaCreditsList for listing credit additions instead.
As it would definitely have an impact, I understand why we would want to keep this information in the response, so I'm okay with not removing it. The enhancements to quotaBalance will still be made without this change.
ok, @winterhazel . I know you and your are the most heavy users of quota so I am not going to -1 this by any means, but my advice would be to leave it in and shout about removing it before you do. you might get work mitigating any regressions if you don't.
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
@blueorangutan package
@winterhazel 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]: βοΈ el8 βοΈ el9 βοΈ debian βοΈ suse15. SL-JID 11731
@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]: βοΈ el8 βοΈ el9 βοΈ debian βοΈ suse15. SL-JID 12004
@blueorangutan test
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
[SF] Trillian test result (tid-12057) Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8 Total time taken: 64498 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9590-t12057-kvm-ol8.zip Smoke tests completed. 140 look OK, 1 have errors, 0 did not run Only failed and skipped tests results shown below:
| Test | Result | Time (s) | Test File |
|---|---|---|---|
| test_02_unsecure_vm_migration | Error |
450.11 | test_vm_life_cycle.py |
| test_03_secured_to_nonsecured_vm_migration | Error |
380.41 | test_vm_life_cycle.py |
| test_04_nonsecured_to_secured_vm_migration | Error |
359.40 | test_vm_life_cycle.py |
applying insanity
@blueorangutan test keepEnv
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
[SF] Trillian test result (tid-12058) Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8 Total time taken: 52614 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9590-t12058-kvm-ol8.zip Smoke tests completed. 140 look OK, 1 have errors, 0 did not run Only failed and skipped tests results shown below:
| Test | Result | Time (s) | Test File |
|---|---|---|---|
| test_01_secure_vm_migration | Error |
134.27 | test_vm_life_cycle.py |
| test_01_secure_vm_migration | Error |
134.27 | test_vm_life_cycle.py |
test_vm_life_cycle has a more generic problem especially in the secure chapter. The errors are different both times and probably environmental by nature.
Hey @DaanHoogland, my idea of removing the credits listing from
quotaBalancewas to make each API have a single responsibility and make credits-related code changes easier in the future (as we would not have to worry about checking and updatingquotaBalanceaccordingly anymore when introducing these changes). I don't have a stronger argument for it πRegarding the gravity of removing it: Quota users would be advised to use
quotaCreditsListto list credit additions instead. However, scripts/automation tools/external software that usequotaBalanceto get the credit addition history of accounts and were not updated before upgrading CloudStack would break. The Quota UI currently uses it to show a list containing the balance and credit addition entries at the same time, but this page will be reworked in the future to separate them, usingquotaCreditsListfor listing credit additions instead.As it would definitely have an impact, I understand why we would want to keep this information in the response, so I'm okay with not removing it. The enhancements to
quotaBalancewill still be made without this change.
@winterhazel , clear, your call..!
@winterhazel , clear, your call..!
This one should be good for merge as well π