Skip to content
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

rename nvidia-gpu-device-plugin to kvm-nvidia-gpu #19133

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions deploy/addons/aliyun_mirror.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
"registry.k8s.io/mongodb-install": "registry.cn-hangzhou.aliyuncs.com/google_containers/mongodb-install",
"registry.k8s.io/nginx-slim": "registry.cn-hangzhou.aliyuncs.com/google_containers/nginx-slim",
"registry.k8s.io/nvidia-gpu-device-plugin": "registry.cn-hangzhou.aliyuncs.com/google_containers/nvidia-gpu-device-plugin",
"registry.k8s.io/kvm-nvidia-gpu": "registry.cn-hangzhou.aliyuncs.com/google_containers/nvidia-gpu-device-plugin",
"registry.k8s.io/pause": "registry.cn-hangzhou.aliyuncs.com/google_containers/pause",
"registry.k8s.io/pause-amd64": "registry.cn-hangzhou.aliyuncs.com/google_containers/pause-amd64",
"registry.k8s.io/spark": "registry.cn-hangzhou.aliyuncs.com/google_containers/spark",
Expand Down
4 changes: 4 additions & 0 deletions deploy/addons/assets.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ var (
//go:embed gpu/nvidia-gpu-device-plugin.yaml.tmpl
NvidiaGpuDevicePluginAssets embed.FS

// KvmNvidiaGpuAssets assets for kvm-nvidia-gpu addon
//go:embed gpu/kvm-nvidia-gpu.yaml.tmpl
KvmNvidiaGpuAssets embed.FS

// LogviewerAssets assets for logviewer addon
//go:embed logviewer/*.tmpl logviewer/*.yaml
LogviewerAssets embed.FS
Expand Down
64 changes: 64 additions & 0 deletions deploy/addons/gpu/kvm-nvidia-gpu.yaml.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# Copyright 2018 The Kubernetes Authors All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pick up this task,
I dont think we need to create a new Addon called KVM-nvidia...we should keep the addon intact, so we dont have to Maintain Two Copy of it.... I was hoping we have a solution that the Old Name still Works

what I had in mind was depreciation in the Flag Name, we already have an example of that

for example,

minikube start --help 
     --vm-driver='':
	DEPRECATED, use `driver` instead.

so we might have to refactor our Adddons package to allow an addon to have Two Names, or allowed it to have a depricated Name and when the user uses the depricated name we encourage them to use the new name.

and by the way this PR should probably Advice the users on the Docker Driver to use the --gpus flag instead

Copy link
Author

@ilyesAj ilyesAj Jun 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@medyagh , thank you for your review, i misunderstood the need. i will try to refactor the work to:

  • make both names working ( the same addon pointing on )
  • deprecate the old one with just notification

for advising users on the docker driver, i double checked and it seems that on the documentation you already recommend that flag : https://minikube.sigs.k8s.io/docs/tutorials/nvidia/#docker

#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

apiVersion: apps/v1
kind: DaemonSet
metadata:
name: kvm-nvidia-gpu
namespace: kube-system
labels:
k8s-app: kvm-nvidia-gpu
kubernetes.io/minikube-addons: kvm-nvidia-gpu
addonmanager.kubernetes.io/mode: Reconcile
spec:
selector:
matchLabels:
k8s-app: kvm-nvidia-gpu
template:
metadata:
labels:
k8s-app: kvm-nvidia-gpu
spec:
priorityClassName: system-node-critical
tolerations:
- operator: "Exists"
effect: "NoExecute"
- operator: "Exists"
effect: "NoSchedule"
volumes:
- name: device-plugin
hostPath:
path: /var/lib/kubelet/device-plugins
- name: dev
hostPath:
path: /dev
containers:
- image: {{.CustomRegistries.NvidiaDevicePlugin | default .ImageRepository | default .Registries.NvidiaDevicePlugin }}{{.Images.NvidiaDevicePlugin}}
command: ["/usr/bin/nvidia-gpu-device-plugin", "-logtostderr"]
name: kvm-nvidia-gpu
resources:
requests:
cpu: 50m
memory: 10Mi
securityContext:
allowPrivilegeEscalation: false
capabilities:
drop: ["ALL"]
volumeMounts:
- name: device-plugin
mountPath: /device-plugin
- name: dev
mountPath: /dev
updateStrategy:
type: RollingUpdate
4 changes: 4 additions & 0 deletions pkg/addons/addons.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ func preStartMessages(name, value string) {
out.Styled(style.Warning, "The ambassador addon has stopped working as of v1.23.0, for more details visit: https://github.com/datawire/ambassador-operator/issues/73")
case "olm":
out.Styled(style.Warning, "The OLM addon has stopped working, for more details visit: https://github.com/operator-framework/operator-lifecycle-manager/issues/2534")
case "nvidia-gpu-device-plugin":
out.Styled(style.Warning, "The nvidia-gpu-device-plugin addon is deprecated and will be removed in a future release. Please use the kvm-nvidia-gpu addon instead.for more details visit: https://github.com/kubernetes/minikube/issues/19114")
}
}

Expand Down Expand Up @@ -163,6 +165,8 @@ func Deprecations(name string) (bool, string, string) {
return true, "metrics-server", "using metrics-server addon, heapster is deprecated"
case "efk":
return true, "", "The current images used in the efk addon contain Log4j vulnerabilities, the addon will be disabled until images are updated, see: https://github.com/kubernetes/minikube/issues/15280"
case "nvidia-gpu-device-plugin":
return true, "kvm-nvidia-gpu", "The nvidia-gpu-device-plugin addon is deprecated and will be removed in a future release. Please use the kvm-nvidia-gpu addon instead.for more details visit:https://github.com/kubernetes/minikube/issues/19114"
}
return false, "", ""
}
Expand Down
6 changes: 6 additions & 0 deletions pkg/addons/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,16 @@ var Addons = []*Addon{
callbacks: []setFn{EnableOrDisableAddon},
},
{
// Deprecated, will be removed in a future release in favor of kvm-nvidia-gpu
name: "nvidia-gpu-device-plugin",
set: SetBool,
callbacks: []setFn{EnableOrDisableAddon},
},
{
name: "kvm-nvidia-gpu",
set: SetBool,
callbacks: []setFn{EnableOrDisableAddon},
},
{
name: "olm",
set: SetBool,
Expand Down
11 changes: 11 additions & 0 deletions pkg/minikube/assets/addons.go
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,17 @@ var Addons = map[string]*Addon{
}, map[string]string{
"NvidiaDevicePlugin": "registry.k8s.io",
}),
"kvm-nvidia-gpu": NewAddon([]*BinAsset{
MustBinAsset(addons.KvmNvidiaGpuAssets,
"gpu/kvm-nvidia-gpu.yaml.tmpl",
vmpath.GuestAddonsDir,
"kvm-nvidia-gpu.yaml",
"0640"),
}, false, "kvm-nvidia-gpu", "3rd party (NVIDIA)", "", "https://minikube.sigs.k8s.io/docs/tutorials/nvidia/", map[string]string{
"NvidiaDevicePlugin": "nvidia-gpu-device-plugin@sha256:4b036e8844920336fa48f36edeb7d4398f426d6a934ba022848deed2edbf09aa",
}, map[string]string{
"NvidiaDevicePlugin": "registry.k8s.io",
}),
"logviewer": NewAddon([]*BinAsset{
MustBinAsset(addons.LogviewerAssets,
"logviewer/logviewer-dp-and-svc.yaml.tmpl",
Expand Down
6 changes: 4 additions & 2 deletions site/content/en/docs/tutorials/nvidia.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,9 @@ to expose GPUs with `--driver=kvm`. Please don't mix these instructions.

- Install NVIDIA's device plugin:
```shell
minikube addons enable nvidia-device-plugin
minikube addons enable kvm-nvidia-gpu
```
NOTE: `nvidia-gpu-device-plugin` has been deprecated in favor of `kvm-nvidia-gpu`.
{{% /tab %}}
{{% tab kvm %}}
## Using the kvm driver
Expand Down Expand Up @@ -102,9 +103,10 @@ host to the minikube VM. Doing so has a few prerequisites:

If this succeeded, run the following commands:
```shell
minikube addons enable nvidia-gpu-device-plugin
minikube addons enable kvm-nvidia-gpu
minikube addons enable nvidia-driver-installer
```
NOTE: `nvidia-gpu-device-plugin` has been deprecated in favor of `kvm-nvidia-gpu`.

This will install the NVIDIA driver (that works for GeForce/Quadro cards)
on the VM.
Expand Down