Skip to content
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

webhook: support elasticquota enable update resource key #2323

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lijunxin559
Copy link
Contributor

@lijunxin559 lijunxin559 commented Jan 17, 2025

Ⅰ. Describe what this PR does

Currently, the consistency check for resource types in elastic quota's add, delete and update is insufficient and too rigid. For example, maxResourceKey requires parent-child consistency, which can lead to the following problems:

  1. There is no unified logic to check the various resource types of min/max, which can easily lead to inconsistencies
  2. Unable to flexibly support changing resource types, especially when adding new resources, limited by the initial creation status

However, considering the original intention of quota design and current computing logic, it does not affect related calculations while supporting resource type checks(also already added more unitTest cases). Among them:

  1. The setting intentions of min and max are different and the calculation logic is not dependent. The resource types need to be checked separately
  2. Guaranteed is affected by min calculation, so there is no need to check separately

Ⅱ. Does this pull request fix one issue?

I propose to add a feature called ElasticQuotaEnableUpdateResourceKey to support changes to ElasticQuota Resource Key, with the following modifications:

  1. Support min and max resourceKey checks during elasticQuota add and update: checkSubAndParentGroupQuotaKey()
  2. Support AnnotationSharedWeight updated: when SharedWeight has a key which is not included in max, delete it; when SharedWeight does not have the key of max, it defaults to be filled in the quantity of max
  3. Maintain compatibility of resourceKeys update logic: updateResourceKeyNoLock()
  4. Propose standard process operations for elastic quota resourceKey: add resourceKey from parent node to child node, delete resourceKey from child node to parent node

Ⅲ. Describe how to verify it

Ⅳ. Special notes for reviews

V. Checklist

  • I have written necessary docs and comments
  • I have added necessary unit tests and integration tests
  • All checks passed in make test

Copy link

codecov bot commented Jan 17, 2025

Codecov Report

Attention: Patch coverage is 86.58537% with 11 lines in your changes missing coverage. Please review.

Project coverage is 66.07%. Comparing base (6f6ef82) to head (f44afa6).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
pkg/webhook/elasticquota/quota_topology.go 79.31% 4 Missing and 2 partials ⚠️
pkg/webhook/elasticquota/quota_topology_check.go 88.09% 3 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2323      +/-   ##
==========================================
- Coverage   66.09%   66.07%   -0.02%     
==========================================
  Files         458      458              
  Lines       54197    54334     +137     
==========================================
+ Hits        35823    35903      +80     
- Misses      15801    15844      +43     
- Partials     2573     2587      +14     
Flag Coverage Δ
unittests 66.07% <86.58%> (-0.02%) ⬇️

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

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

@lijunxin559 lijunxin559 force-pushed the support-elasticquota-enable-update-resourceKey branch from 01bf3f6 to 081511d Compare January 21, 2025 09:38
@saintube saintube requested a review from shaloulcy January 22, 2025 01:44
@shaloulcy
Copy link
Contributor

group_quota_manager.go里面的UpdateQuota、RefreshRuntime 需要加单侧覆盖这种情况

@lijunxin559 lijunxin559 force-pushed the support-elasticquota-enable-update-resourceKey branch from 081511d to 3a72f52 Compare January 23, 2025 06:42
@lijunxin559
Copy link
Contributor Author

group_quota_manager.go里面的UpdateQuota、RefreshRuntime 需要加单侧覆盖这种情况

added

@lijunxin559 lijunxin559 force-pushed the support-elasticquota-enable-update-resourceKey branch from 3a72f52 to f44afa6 Compare January 23, 2025 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants