-
Notifications
You must be signed in to change notification settings - Fork 517
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
Add NVIDIA APIs #4182
Add NVIDIA APIs #4182
Conversation
sources/settings-migrations/v1.23.0/kubelet-device-plugin-metadata/Cargo.toml
Outdated
Show resolved
Hide resolved
sources/settings-migrations/v1.23.0/kubelet-device-plugin-settings/Cargo.toml
Outdated
Show resolved
Hide resolved
sources/settings-migrations/v1.23.0/nvidia-container-runtime-metadata/Cargo.toml
Outdated
Show resolved
Hide resolved
sources/settings-migrations/v1.23.0/nvidia-container-runtime-settings/Cargo.toml
Outdated
Show resolved
Hide resolved
cc357d4
to
d8b052f
Compare
(forced push includes rebase) |
d8b052f
to
b6363e7
Compare
Forced push includes:
|
b6363e7
to
88e76ab
Compare
(rebased develop and drop hack commit) |
Signed-off-by: Arnaldo Garcia Rincon <[email protected]>
Add settings defaults for nvidia-container-runtime and kubelet-device-plugin Signed-off-by: Arnaldo Garcia Rincon <[email protected]>
Signed-off-by: Arnaldo Garcia Rincon <[email protected]>
Signed-off-by: Arnaldo Garcia Rincon <[email protected]>
88e76ab
to
d3d8b35
Compare
Forced push fixes the settings plugins definitions for |
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.
just some small nits
@@ -0,0 +1,19 @@ | |||
[package] | |||
name = "settings-plugin-aws-k8s-nvidia" | |||
version = "0.1.0" |
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.
should we add authors here?
[package] | ||
name = "nvidia-container-runtime-settings" | ||
version = "0.1.0" | ||
authors = ["Monirul Islam <[email protected]>"] |
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.
slight nit, should @arnaldo2792 be a listed author as well? (and for other crates added in this PR)
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.
I wanted to keep the ownership of the work since the commits will have my name. My changes to that code were minimal 👍
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!
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!
Issue number:
N / A
Description of changes:
This includes two new APIs to all the NVIDIA variants:
settings.nvidia-container-runtime
: used to configure the NVIDIA container toolkit behaviorsettings.kubelet-device-plugin.nvidia
: used to expose configurations for the NVIDIA k8s device pluginTesting done:
As part of bottlerocket-os/bottlerocket-settings-sdk#60 and bottlerocket-os/bottlerocket-core-kit#132
NVIDIA_VISIBLE_DEVICES=all
can access all GPUs when the configurations to allow this are set.Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.