feat(binder): GPU_MEMORY_LIMIT binder plugin for shared GPU workloads#1504
feat(binder): GPU_MEMORY_LIMIT binder plugin for shared GPU workloads#1504FouoF wants to merge 4 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@FouoF Can you please rebase this on top of origin/main removing the commits already merged? |
9a689bd to
bd03290
Compare
|
@FouoF Can you please implement it as a new plugin to the binder that will be "opt-in" - ie not enabled by default? |
Merging this branch will decrease overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
b88c57a to
a5ee0a6
Compare
Thanks, I agree with the goal of keeping GPU virtualization separate from the GPU sharing plugin. After looking deeper, I don't think making this a binder-only standalone plugin is actually independent in the current architecture. The virtualization logic still relies on several pieces owned by GPU sharing: the admission mutation that injects the env/configmap references, the So an opt-in binder plugin would currently be a hidden extension of I think there are two reasonable options:
Given that, I’d prefer not to present this as an independent binder plugin in this PR unless we also split the admission/configmap pieces. |
|
@FouoF Lets present it as a dependent binder plugin then - it will only function well if the gpusharing plugin is also enabled. |
|
@enoodle I have made it an independent plugin and it will check if the gpusharing plugin is enabled. |
Signed-off-by: Jifei Wang <poff2001@outlook.com>
Signed-off-by: Jifei Wang <jifei.wang@dynamia.ai>
Signed-off-by: Jifei Wang <jifei.wang@dynamia.ai>
Signed-off-by: Jifei Wang <jifei.wang@dynamia.ai>
089af4b to
ef6037c
Compare
Description
As the issue described.
Related Issues
Fixes #1367
Checklist
Breaking Changes
No
Additional Notes