-
Notifications
You must be signed in to change notification settings - Fork 1k
federatedresourcequota add the support to multi-components scheduling #6934
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
Conversation
Summary of ChangesHello @zhzhuang-zju, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request extends the federated resource quota mechanism to properly account for multi-component applications during scheduling. It addresses a gap where resource quota calculations might be skipped for multi-component resources if replica information was not explicitly present in the scheduling result, or if the scheduled clusters changed. The changes ensure that resource usage is accurately tracked for complex, distributed applications managed by Karmada. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds support for federated resource quota calculation for multi-component scheduling. The main change correctly triggers quota validation when scheduled clusters change for a resource with multiple components, which is necessary as replica counts are not used for them in this scenario. The implementation appears correct, and the addition of a new e2e test to verify this behavior is a great improvement. I have one suggestion to make the new test code more robust.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6934 +/- ##
==========================================
+ Coverage 46.43% 46.45% +0.02%
==========================================
Files 698 698
Lines 47787 47808 +21
==========================================
+ Hits 22189 22211 +22
+ Misses 23929 23928 -1
Partials 1669 1669
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
0dfbc60 to
83efc6c
Compare
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds support for federated resource quota calculation for multi-component scheduling. This is achieved by introducing a check for changes in scheduled clusters for multi-component workloads, which then triggers a quota recalculation. A comprehensive e2e test is also added to validate the quota usage calculation for a FlinkDeployment custom resource. The overall approach is sound, but I've identified a potential bug in how cluster changes are detected and provided a suggestion for a more robust implementation.
RainbowMango
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/assign
Signed-off-by: zhzhuang-zju <[email protected]>
83efc6c to
c012888
Compare
RainbowMango
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: RainbowMango The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind feature
What this PR does / why we need it:
We introduced support for resource quota calculation for multi-components in #6701. However, further adaptation is needed for multi-components scheduling. For example, the current scheduling result for multi-components does not include
replicas, so the existing logic skips quota calculation entirely—this needs to be updated.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: