Skip to content

Set number of hours per month with actual number in Quota - #7210

Merged
DaanHoogland merged 2 commits into
apache:mainfrom
scclouds:fix-quota-hours-per-month-total
Nov 17, 2023
Merged

Set number of hours per month with actual number in Quota#7210
DaanHoogland merged 2 commits into
apache:mainfrom
scclouds:fix-quota-hours-per-month-total

Conversation

@stephankruggg

Copy link
Copy Markdown
Contributor

Description

The Quota usage value for monthly usage types uses the cost per hour to calculate the total tariff value in a particular period. In order to define the cost per hour, it takes into account the total number of hours in each month. However, this value is currently fixed at 720 hours for every month.

This causes incorrect values when calculating the cost per hour, resulting in costs being higher than expected. Therefore, this PR aims to calculate the number of hours in each month based on the actual number of days of each month.

For example, if the tariff value is 100 per month, the current cost per hour is 0.138888889 for every month. For 1 hour per month the total cost in a year would be 1.666666668.
With the adjustments to the cost per hour of this PR, the costs per hour would be: 0.134408602 (January, March, May, July, August, October, December), 0.148809524 (February of a non-leap year), 0.143678161 (February of a leap year), and 0.138888889 (April, June, September, November). This results in 1.645225294 for a non-leap year and 1.640093931 for a leap year at 1 hour per month, which is a difference of about 1.3% in costs to the current method.

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

How Has This Been Tested?

In a local lab I applied the changes, and manually changed the current month of the MS to every month in a year (including February of a leap year) and verified if the cost per hour was calculated correctly using the number of hours of each month when the quotaUpdate API was called.

@codecov

codecov Bot commented Feb 13, 2023

Copy link
Copy Markdown

Codecov Report

Merging #7210 (d419dcd) into main (235e4fe) will increase coverage by 1.84%.
Report is 23 commits behind head on main.
The diff coverage is 80.00%.

@@             Coverage Diff              @@
##               main    #7210      +/-   ##
============================================
+ Coverage     29.15%   30.99%   +1.84%     
+ Complexity    30909    30723     -186     
============================================
  Files          5165     4827     -338     
  Lines        364111   337386   -26725     
  Branches      53306    48559    -4747     
============================================
- Hits         106153   104582    -1571     
+ Misses       243397   218410   -24987     
+ Partials      14561    14394     -167     
Flag Coverage Δ
simulator-marvin-tests 24.82% <0.00%> (-0.31%) ⬇️
uitests ?
unit-tests 14.80% <100.00%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
.../org/apache/cloudstack/quota/QuotaManagerImpl.java 50.39% <100.00%> (+0.39%) ⬆️
utils/src/main/java/com/cloud/utils/DateUtil.java 12.90% <0.00%> (-11.13%) ⬇️

... and 523 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@sonarqubecloud

Copy link
Copy Markdown

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell B 1 Code Smell

11.1% 11.1% Coverage
0.0% 0.0% Duplication

Comment thread framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java Outdated
@DaanHoogland DaanHoogland added this to the 4.19.0.0 milestone Jun 22, 2023

@JoaoJandre JoaoJandre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CLTM, didn't test it

@shwstppr

shwstppr commented Oct 3, 2023

Copy link
Copy Markdown
Contributor

@stephankruggg cc @GaOrtiga can you please check the outstanding review comments?

@DaanHoogland

Copy link
Copy Markdown
Contributor

@stephankruggg can you answer my question #7210 (comment) ?
Any daylight saving or leap year/ - second issues possible there?

@GaOrtiga
GaOrtiga force-pushed the fix-quota-hours-per-month-total branch from b62e20e to d419dcd Compare November 3, 2023 14:57
@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@DaanHoogland

Copy link
Copy Markdown
Contributor

ui build error is in codecov push, not in code.

@DaanHoogland DaanHoogland self-assigned this Nov 15, 2023
@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 7763

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian test result (tid-8331)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 45428 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7210-t8331-kvm-centos7.zip
Smoke tests completed. 117 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_08_migrate_vm Error 0.06 test_vm_life_cycle.py

@DaanHoogland DaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clgtm

@DaanHoogland
DaanHoogland merged commit db27c0a into apache:main Nov 17, 2023
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Nov 29, 2023
Co-authored-by: Stephan Krug <stephan.krug@scclouds.com.br>
Co-authored-by: Gabriel <gabriel.fernandes@scclouds.com.br>
@DaanHoogland DaanHoogland removed their assignment Dec 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants