-
Notifications
You must be signed in to change notification settings - Fork 3
fix(helm): temporarily disable nvidia runtimeClassName #751
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
base: master
Are you sure you want to change the base?
Conversation
Temporarily comment out the nvidia runtimeClassName in single-node deployment to resolve runtime compatibility issues. This is a temporary patch that should be reverted once the underlying issue is resolved. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Summary of ChangesHello @dittops, 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 implements a temporary fix to resolve immediate runtime compatibility issues by commenting out the 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 temporarily disables the runtimeClassName: nvidia setting by commenting it out in the Helm template. While this is a quick fix, I've suggested a more maintainable approach using a feature flag in values.yaml. This aligns better with Helm best practices for managing configurable options and avoids leaving commented-out code in the codebase.
| # {{- if eq .node.type "cuda" }} | ||
| # runtimeClassName: nvidia | ||
| # {{- end }} |
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.
While commenting out this block achieves the goal, a more idiomatic and maintainable approach for Helm charts is to use a feature flag in values.yaml. This makes the configuration explicit and allows toggling the behavior without changing the template code. This is preferable to leaving commented-out code in the template.
You could add a new value like nvidiaRuntimeClassEnabled to your values.yaml (setting it to false to keep it disabled) and use it here. This would make re-enabling it in the future cleaner.
{{- if and (eq .node.type "cuda") .Values.nvidiaRuntimeClassEnabled }}
runtimeClassName: nvidia
{{- end }}…udEcosystem/bud-runtime into patch/disable-nvidia-runtime-class
…into patch/disable-nvidia-runtime-class
Summary
runtimeClassName: nvidiasetting in the single-node deployment templateChanges
runtimeClassName: nvidiafor CUDA nodes inservices/budcluster/budcluster/charts/bud_runtime_container/templates/single-node.yamlWhy This Is Needed
This patch addresses immediate runtime compatibility issues by disabling the nvidia runtime class specification. This should be considered a temporary workaround.
Test Plan
Follow-up
This is a temporary patch and should be reverted once the underlying compatibility issue is resolved. A more permanent solution should be investigated.
🤖 Generated with Claude Code