Fix #3448 quota calculation for monthly tariffs

[jairov4]

Description

The quota calculation has been wrong for monthly tariffs (like Compute-Month, GB-Month, etc) for so much time but even when #3448 was reported it was closed without evaluation.

The wrong calculation was the price per hour, it was taking the monthly tariff and multiplying by the aggregation ratio which has no sense at all.

The right price per hour is the monthly tariff divided by the standard number of hours in a month.

Fixes #3448.

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• 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
• Minor

Bug Severity

• BLOCKER
• Critical
• Major
• Minor
• Trivial

How Has This Been Tested?

Partial execution of the component with a test vector.
Tracing the values and compared with spreadsheet.

[DaanHoogland]

Your code looks alright @jairov4 but only if the aggregation is always hourly. I think there is another problem with the code and that is that the onehour vars are not named correctly; BigDecimal onehourcostpercpu, onehourcostper100mhz, onehourcostper1mb, onehourcostforvmusage;. I think these should have been called 'oneaggregation*'.
Have you tested with other agrregation durations?
Care to comment @abhinandanprateek , @rhtyd , @rafaelweingartner ? (not sure who else to ask)

[jairov4]

@DaanHoogland yes, this calculation is independent of the aggregation ratio because the raw usage variable is the number of hours used, then we only need the hour cost to compute the final quota for the aggregation time slot. As you can see in src/main/java/com/cloud/usage/parser/VMInstanceUsageParser.java

[jairov4]

btw: should I rebase it to 4.15?

[DaanHoogland]

You can rebase it @jairov4 , if you want it on that branch. It is unlikely an new 4.15 version will come out. if you do it will be merged forward to 4.16 as well.

[yadvr]

@blueorangutan package

[blueorangutan]

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result: ✔️ el7 ✖️ el8 ✖️ debian ✖️ suse15. SL-JID 1729

[yadvr]

@blueorangutan package

[blueorangutan]

@rohityadavcloud a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2144

[sureshanaparti]

@blueorangutan test

[blueorangutan]

@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

[blueorangutan]

Trillian test result (tid-2838)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31162 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5517-t2838-kvm-centos7.zip
Smoke tests completed. 92 look OK, 0 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File

[DaanHoogland]

genrally good to go, but some instances of the parameter aggregationRotio need to be removed.

[DaanHoogland]

@jairov4 , please remove the parameter aggregationRatio from methods that no longer use it.

[sureshanaparti]

@jairov4 , please remove the parameter aggregationRatio from methods that no longer use it.

Hi @jairov4 Can you address the outstanding comments please. Thanks.

[jairov4]

@sureshanaparti done!

[sureshanaparti]

@blueorangutan package

[blueorangutan]

@sureshanaparti a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2461

[DaanHoogland]

@blueorangutan test

[blueorangutan]

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

[blueorangutan]

Trillian test result (tid-3164)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 32912 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5517-t3164-kvm-centos7.zip
Smoke tests completed. 92 look OK, 0 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File

[DaanHoogland]

cltgm

[sureshanaparti]

code LGTM

[DaanHoogland]

@jairov4 looks good no regressions. The test_quota.py integration test is not run in either the smoke tests or in travis, So I'll run thos manually to make triple sure.

[DaanHoogland]

I ran the quota tests and got all to succeed except for one that requires more than a single day of data to accumulate. recarding this as success and merging because of succesfull testing and multiple LTGM.