search

apache / cloudstack

Apache CloudStack is an opensource Infrastructure as a Service (IaaS) cloud computing platform
https://cloudstack.apache.org/
Apache License 2.0
2.1k stars 1.11k forks source link

Create API to list Quota credits #9590

Open winterhazel opened 2 months ago

winterhazel commented 2 months ago

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:

No parameters are flagged as required, but at least accountid or domainid must be provided.

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

How Has This Been Tested?

In my environment, there were the following domains and accounts:

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:

(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": "$"
    }
  ]
}
(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": "$"
    }
  ]
}
(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": "$"
    }
  ]
}
(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": "$"
    }
  ]
}
(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": "$"
    }
  ]
}
(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": "$"
    }
  ]
}
(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:

(d1) 🐒 > quota creditslist accountid=2
πŸ™ˆ Error: (HTTP 531, error code 4365) Caller does not have permission to operate with provided resource.
(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": "$"
    }
  ]
}
(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": "$"
    }
  ]
}
(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:

(u1) 🐒 > quota creditslist accountid=2
πŸ™ˆ Error: (HTTP 531, error code 4365) Caller does not have permission to operate with provided resource.
(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.
(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": "$"
    }
  ]
}
(u1) 🐒 > quota creditslist domainid=2
πŸ™ˆ Error: (HTTP 531, error code 4365) Regular users are not allowed to generate domain statements.
winterhazel commented 2 months ago

@blueorangutan package

blueorangutan commented 2 months ago

@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[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 68.58974% with 49 lines in your changes missing coverage. Please review.

Project coverage is 15.59%. Comparing base (8ca1843) to head (a38619c). Report is 37 commits behind head on main.

Files with missing lines Patch % Lines
...ache/cloudstack/quota/dao/QuotaCreditsDaoImpl.java 0.00% 18 Missing :warning:
...he/cloudstack/api/command/QuotaCreditsListCmd.java 69.76% 13 Missing :warning:
.../main/java/com/cloud/domain/dao/DomainDaoImpl.java 0.00% 6 Missing :warning:
...udstack/api/response/QuotaResponseBuilderImpl.java 91.04% 2 Missing and 4 partials :warning:
...org/apache/cloudstack/quota/vo/QuotaCreditsVO.java 0.00% 3 Missing :warning:
.../cloudstack/api/response/QuotaBalanceResponse.java 0.00% 2 Missing :warning:
.../org/apache/cloudstack/quota/QuotaServiceImpl.java 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #9590 +/- ## ============================================ + Coverage 15.54% 15.59% +0.04% - Complexity 11996 12058 +62 ============================================ Files 5499 5502 +3 Lines 481706 482180 +474 Branches 62623 62014 -609 ============================================ + Hits 74884 75185 +301 - Misses 398536 398691 +155 - Partials 8286 8304 +18 ``` | [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/9590/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ” | | |---|---|---| | [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/9590/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.17% <ΓΈ> (-0.01%)` | :arrow_down: | | [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/9590/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `16.37% <68.58%> (+0.05%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

blueorangutan commented 2 months ago

Packaging result [SF]: βœ”οΈ el8 βœ”οΈ el9 βœ”οΈ debian βœ”οΈ suse15. SL-JID 10810

DaanHoogland commented 2 months ago

@blueorangutan test keepEnv

blueorangutan commented 2 months ago

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

blueorangutan commented 2 months ago

[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
DaanHoogland commented 2 months ago

@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?

winterhazel commented 2 months ago

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.

DaanHoogland commented 2 months ago

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.