Sync kube-scheduler: Improve CSILimits plugin accuracy by using VolumeAttachments #4608
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @guoqinwill, 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 updates the Volcano scheduler to integrate with Kubernetes' recent enhancements for CSILimits calculations. By incorporating VolumeAttachment information, the scheduler can now make more accurate and informed decisions regarding volume-related constraints, ensuring better alignment with upstream Kubernetes behavior and improved scheduling performance.
Highlights
- RBAC Permissions Update: The scheduler's service account now has permissions to list and watch
VolumeAttachmentresources under thestorage.k8s.ioAPI group. This is crucial for the scheduler to access necessary information forCSILimitscalculations. - Scheduler Cache Enhancement: The Volcano scheduler's cache has been updated to include a
VolumeAttachmentinformer. This allows the scheduler to actively watch and maintain an up-to-date state ofVolumeAttachmentobjects, improving the accuracy of its scheduling decisions related to CSI volumes. - CSILimits Plugin Accuracy: This pull request aligns Volcano with recent Kubernetes updates (v1.32+) by enabling the
CSILimitsplugin to leverageVolumeAttachmentsfor more precise volume capacity calculations. This ensures better resource utilization and scheduling efficiency.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
volcano-sh-bot
added
the
size/XS
guoqinwill
force-pushed
the
add-volumeAttach
branch
from
97e0c5e to
685b3c5
Compare
There was a problem hiding this comment.
Code Review
This pull request aims to improve the accuracy of the CSILimits plugin by using VolumeAttachments, aligning Volcano with recent changes in kube-scheduler. The changes include adding the necessary RBAC permissions for volumeattachments in the Helm chart and setting up a VolumeAttachmentInformer in the scheduler cache. The implementation is straightforward and correct. The changes look good to me.
guoqinwill
force-pushed
the
add-volumeAttach
branch
from
685b3c5 to
7176b01
Compare
…Attachments Signed-off-by: g00673948 <[email protected]>
guoqinwill
force-pushed
the
add-volumeAttach
branch
from
7176b01 to
2e96f07
Compare
|
So there will be a rbac permission error in volcano when using pvc? |
| sc.scInformer = informerFactory.Storage().V1().StorageClasses() | ||
| sc.scInformer.Informer() | ||
| sc.vaInformer = informerFactory.Storage().V1().VolumeAttachments() | ||
| sc.vaInformer.Informer() |
There was a problem hiding this comment.
Where this sc.vaInformer been used?
There was a problem hiding this comment.
In Volcano scheduler’s predicate plugin, the Filter method from k8s.io/kubernetes/pkg/scheduler/framework/plugins/nodevolumelimits is already used for volume-related pre-filtering. As shown in [Kubernetes PR #127757](https://github.com/kubernetes/kubernetes/pull/127757/files), nodevolumelimits already includes validation for VolumeAttachments. Therefore, it seems this PR may not be necessary.
There was a problem hiding this comment.
So we need to know the accurate error.
There was a problem hiding this comment.
The InformerFactory stared first in volcano, it's late to register informer in kube-scheduler plugin, so we shoulld register it before infromerFactory start. @kingeasternsun
There was a problem hiding this comment.
The InformerFactory stared first in volcano, it's late to register informer in kube-scheduler plugin, so we shoulld register it before infromerFactory start. @kingeasternsun
Got it. This comment could be resolved.
Ignore my comment @guoqinwill .
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Monokaix 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 |
volcano-sh-bot
added
the
approved
|
/lgtm |
|
Please also cherryick to release1.12 and 1.11 |
|
5 participants
What type of PR is this?
/kind bug
What this PR does / why we need it:
Starting from version 1.32, kube-scheduler supports retrieving volumeattachment to accurately calculate CSILimits. Volcano also needs to be updated accordingly. For details, see the Kubernetes pr：kubernetes/kubernetes#127757
Which issue(s) this PR fixes:
Fixes # See the Kubernetes issue：kubernetes/kubernetes#126502
Special notes for your reviewer:
Does this PR introduce a user-facing change?