karmada-io
Use rbSpec.clusters and rbSpec.ReplicaRequirements to calculate rb' resource usage
What type of PR is this? /kind cleanup
What this PR does / why we need it:
As stated in #6467, the workload replicas of member clusters are consistent with rb.Clusters rather than rb.replicas. Therefore, when the federated-resource-quota-enforcement-controller periodically refreshes overallUsage based on rb, it should only use rbSpec.clusters and rbSpec.ReplicaRequirements for calculation.
For example, when a Deployment is scaled up and intercepted by the federated resource quota, its rb might be:
spec:
clusters:
- name: member1
replicas: 2
- name: member2
replicas: 2
replicas: 3
- If calculated based on
rbSpec.replicas, the total replicas = 3 * len(clusters) = 6 - If calculated based on
rbSpec.clusters, the total replicas = 2 + 2 = 4
The two results are inconsistent.
The purpose of this PR is to modify the algorithm for calculating rb resource usage to more accurately reflect the actual usage.
Which issue(s) this PR fixes: Parts of #6467
Special notes for your reviewer:
The method for calculating RB resource usage selected in this PR is consistent with that of the validation webhook. A common function could be extracted to implement this logic. However, considering that the purpose of this PR is to fix a bug rather than to refactor, I decided to fix the issue first and extract the common function in a follow-up change. This approach also makes the code review and code backport easier. Of course, if you believe it's better to extract the common function right away, please let me know your thoughts.
Does this PR introduce a user-facing change?:
:warning: Please install the to ensure uploads and comments are reliably processed by Codecov.
Codecov Report
Attention: Patch coverage is 80.76923% with 5 lines in your changes missing coverage. Please review.
Project coverage is 49.00%. Comparing base (
312fbd0) to head (3ed8f15). Report is 7 commits behind head on master.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| ...federated_resource_quota_enforcement_controller.go | 80.76% | 3 Missing and 2 partials :warning: |
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@ Coverage Diff @@
## master #6477 +/- ##
==========================================
- Coverage 49.02% 49.00% -0.03%
==========================================
Files 687 687
Lines 56043 56091 +48
==========================================
+ Hits 27476 27485 +9
- Misses 26787 26824 +37
- Partials 1780 1782 +2
| Flag | Coverage Δ | |
|---|---|---|
| unittests | 49.00% <80.76%> (-0.03%) |
:arrow_down: |
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.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
/cc @seanlaii @RainbowMango #6474 is merged, and we can push this forward
Just curious if we could also do this task, Extract common resources usage calculation function to binding utils, in this PR?
Since this PR will be backported to release-1.14, I don't recommend doing refactoring work in this PR.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: mszacillo, seanlaii
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~pkg/controllers/OWNERS~~ [mszacillo,seanlaii]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Just curious if we could also do this task, Extract common resources usage calculation function to binding utils, in this PR?
Since this PR will be backported to release-1.14, I don't recommend doing refactoring work in this PR.
So, shall we have a release note?
for waiting release note
Done~ If the release note is not suitable, please let me know.