Skip to content

Adding Count Resource Domain #7274

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

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

CharlesQueiroz
Copy link
Contributor

Description

Currently, the virtual router resources are not counted
towards domain resource count. Only the CPU/MEM of user
virtual machine are counted. If some customers have
VR's with higher cpu/memory values than the default value of
1 core, 256MB RAM then they are consuming extra resources
which are not counted towards the resource calculation
of the domain/account.

So two global settings are added to this change

"resource.count.running.routers" and "resource.count.running.routers.type"

"resource.count.running.routers" can have either true of false
"resource.count.running.routers.type" can have either "all" or "delta"
The default value is "all"

If "resource.count.running.routers" is true, then if
"resource.count.running.routers.type" is "all", then all VR resources
are counted towards the domain resource consumption

For example: If VR is running with 4 cores, and 4GB ram, then all 4 cores
and 4 GB are added to domain resource consumption

If "resource.count.running.routers.type" is "delta" then the diff
between the current VR CPU/ram and the default VR offering CPU/ram
is considered for domain resource calculation.

For example: If VR is running with 4 cores and 4Gb ram, then
3 cores and 4gb-0.25Gb will be counted for resource consumption

If "resource.count.running.routers" is false, then even if VR
has higher CPU/ram, then it's not counted towards domain resource
consumption

Test cases:

It can be tested using.

nosetests --with-marvin --marvin-config=<your test config file>.cfg test_router_resources.py

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)

@weizhouapache
Copy link
Member

@CharlesQueiroz
You have created #6899 which looks same as this. Can you close one ?

@codecov
Copy link

codecov bot commented Feb 22, 2023

Codecov Report

Attention: Patch coverage is 11.41553% with 194 lines in your changes are missing coverage. Please review.

Project coverage is 12.69%. Comparing base (2f309b5) to head (c51c6ae).
Report is 679 commits behind head on main.

❗ Current head c51c6ae differs from pull request most recent head bb7cee5. Consider uploading reports for the commit bb7cee5 to get more accurate results

Files Patch % Lines
.../cloud/resourcelimit/ResourceLimitManagerImpl.java 2.60% 112 Missing ⚠️
...n/java/com/cloud/vm/VirtualMachineManagerImpl.java 10.29% 60 Missing and 1 partial ⚠️
.../cloud/configuration/ConfigurationManagerImpl.java 33.33% 14 Missing ⚠️
...va/com/cloud/network/router/NetworkHelperImpl.java 46.15% 7 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #7274      +/-   ##
============================================
- Coverage     12.95%   12.69%   -0.27%     
+ Complexity     8986     8682     -304     
============================================
  Files          2728     2729       +1     
  Lines        256647   256732      +85     
  Branches      40024    40011      -13     
============================================
- Hits          33256    32595     -661     
- Misses       219214   219991     +777     
+ Partials       4177     4146      -31     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@CharlesQueiroz
Copy link
Contributor Author

@weizhouapache done! Sorry about that. I closed the previous one.

@weizhouapache
Copy link
Member

@weizhouapache done! Sorry about that. I closed the previous one.

You can use "git push -f" to force push a branch

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 8 Code Smells

1.5% 1.5% Coverage
2.1% 2.1% Duplication

@DaanHoogland DaanHoogland added this to the 4.18.1.0 milestone Feb 24, 2023
@GutoVeronezi GutoVeronezi self-requested a review February 26, 2023 19:39
@CharlesQueiroz CharlesQueiroz requested review from DaanHoogland and removed request for GutoVeronezi March 4, 2023 01:56
@DaanHoogland
Copy link
Contributor

I skimmed your code and looks good but I have no time for an intesive review now @CharlesQueiroz . Can you add your integration test to the test matrix (if that makes sense) and maybe add unittests where applicable as well?
thanks for your enhancement

@CharlesQueiroz
Copy link
Contributor Author

@DaanHoogland ... for sure. I'm trying to do that right now. I'll back with notices soon.

@github-actions
Copy link

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

@weizhouapache weizhouapache removed this from the 4.18.1.0 milestone May 12, 2023
@github-actions
Copy link

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

@DaanHoogland DaanHoogland added this to the 4.19.0.0 milestone Jun 22, 2023
@shwstppr
Copy link
Contributor

shwstppr commented Oct 9, 2023

Hi @CharlesQueiroz, can you please address merge conflicts and check review comments?

@CharlesQueiroz
Copy link
Contributor Author

Hi @CharlesQueiroz, can you please address merge conflicts and check review comments?

yes. I'm working to fix it.

@DaanHoogland
Copy link
Contributor

ping @CharlesQueiroz

@sureshanaparti
Copy link
Contributor

Hi @CharlesQueiroz Please resolve the conflicts, and let me know if this PR can be targeted for 4.19.1?

@sureshanaparti sureshanaparti modified the milestones: 4.19.1.0, 4.19.2 Jun 24, 2024
@DaanHoogland DaanHoogland marked this pull request as draft September 18, 2024 10:41
@DaanHoogland
Copy link
Contributor

@CharlesQueiroz we are nearing 4.19.2 . Do you plan to work on this?
cc @benj-n

@DaanHoogland
Copy link
Contributor

moving forward , due to inactivity

@DaanHoogland DaanHoogland modified the milestones: 4.19.2, 4.20.1 Feb 3, 2025
@Pearl1594
Copy link
Contributor

@CharlesQueiroz Would you be working on this?

@DaanHoogland
Copy link
Contributor

@Pearl1594 this is almost two years old and has over a year of inactivity. I propose we move this to "unplanned" until someone adopts the PR.

@Pearl1594 Pearl1594 moved this to In Progress in ACS 4.20.1 Mar 17, 2025
@Pearl1594 Pearl1594 modified the milestones: 4.20.1, unplanned Mar 17, 2025
@Pearl1594 Pearl1594 removed this from ACS 4.20.1 Mar 17, 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.

9 participants