From df2282584dbd1a5cb09ba3bb8feda3292320ed33 Mon Sep 17 00:00:00 2001
From: "R. P. Taylor" <1686627+rptaylor@users.noreply.github.com>
Date: Thu, 9 May 2024 11:32:03 -0700
Subject: [PATCH] update use of namespaces in chart (#62)

To ensure Helm releases are installed in the desired namespace it is important to always specify the release namespace: helm install -n <namespace>.
This MR fixes the chart so it will follow this approach (which is the standard way since I think Helm 3 at least), instead of using a value to control the namespace.

If you helm install without -n it is the same as using kubectl without -n, it should use the default namespace configured in the current context of your kubeconfig. However the current non-standard behaviour of the chart (which I would argue is a bug) overrides this to end up in kube-system instead of whatever your current default is - IF the user doesn't specify it with -n (but you really should).

Actually the more serious bug is that the current values-based namespace selection could potentially prevent the standard release-based namespace selection mechanism from working at all, so a user doing the right thing (helm install -n mynamespace) would potentially end up with the helm release details going in the right namespace and the k8s objects going in the wrong namespace, I am not sure how that would work.

fix #60

Also:
    There is no such thing as a namespace for a ClusterRoleBinding, k8s ignores it. Remove 'default'
    There is no reason to be granting RBAC privileges to the default service account in the default namespace, removed
---
 helm/amd-gpu/README.md                             | 1 -
 helm/amd-gpu/templates/NOTES.txt                   | 4 ++--
 helm/amd-gpu/templates/deviceplugin-daemonset.yaml | 1 -
 helm/amd-gpu/templates/labeller.yaml               | 7 +------
 helm/amd-gpu/values.yaml                           | 2 --
 5 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/helm/amd-gpu/README.md b/helm/amd-gpu/README.md
index 7c8523a7..0db7fca3 100644
--- a/helm/amd-gpu/README.md
+++ b/helm/amd-gpu/README.md
@@ -24,7 +24,6 @@ Kubernetes: `>= 1.18.0`
 | labeller.enabled | bool | `false` |  |
 | lbl.image.repository | string | `"docker.io/rocm/k8s-device-plugin"` |  |
 | lbl.image.tag | string | `"labeller-latest"` |  |
-| namespace | string | `"kube-system"` |  |
 | nfd.enabled | bool | `false` |  |
 | node_selector_enabled | bool | `false` |  |
 | node_selector."feature.node.kubernetes.io/pci-0300_1002.present" | string | `"true"` |  |
diff --git a/helm/amd-gpu/templates/NOTES.txt b/helm/amd-gpu/templates/NOTES.txt
index 9a3bf4ba..9249ef40 100644
--- a/helm/amd-gpu/templates/NOTES.txt
+++ b/helm/amd-gpu/templates/NOTES.txt
@@ -1,4 +1,4 @@
-{{ .Chart.Name }}-device-plugin-daemonset deployed in namespace '{{ .Values.namespace }}'
+{{ .Chart.Name }}-device-plugin-daemonset deployed in namespace '{{ .Release.Namespace }}'
 {{- if .Values.labeller.enabled }}
-{{ .Chart.Name }}-labeller-daemonset deployed in namespace '{{ .Values.namespace }}'
+{{ .Chart.Name }}-labeller-daemonset deployed in namespace '{{ .Release.Namespace }}'
 {{- end }}
diff --git a/helm/amd-gpu/templates/deviceplugin-daemonset.yaml b/helm/amd-gpu/templates/deviceplugin-daemonset.yaml
index bf6a181d..d36ddb4a 100644
--- a/helm/amd-gpu/templates/deviceplugin-daemonset.yaml
+++ b/helm/amd-gpu/templates/deviceplugin-daemonset.yaml
@@ -2,7 +2,6 @@ apiVersion: apps/v1
 kind: DaemonSet
 metadata:
   name: {{ .Chart.Name }}-device-plugin-daemonset
-  namespace: {{ .Values.namespace }}
 spec:
   selector:
     matchLabels:
diff --git a/helm/amd-gpu/templates/labeller.yaml b/helm/amd-gpu/templates/labeller.yaml
index 6e0a2a76..8a31b957 100644
--- a/helm/amd-gpu/templates/labeller.yaml
+++ b/helm/amd-gpu/templates/labeller.yaml
@@ -12,7 +12,6 @@ apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRoleBinding
 metadata:
   name: crb-{{ .Chart.Name }}-labeller
-  namespace: default
 roleRef:
   apiGroup: rbac.authorization.k8s.io
   kind: ClusterRole
@@ -20,16 +19,12 @@ roleRef:
 subjects:
 - kind: ServiceAccount
   name: default
-  namespace: default
-- kind: ServiceAccount
-  name: default
-  namespace: {{ .Values.namespace }}
+  namespace: {{ .Release.Namespace }}
 ---
 apiVersion: apps/v1
 kind: DaemonSet
 metadata:
   name: {{ .Chart.Name }}-labeller-daemonset
-  namespace: {{ .Values.namespace }}
 spec:
   selector:
     matchLabels:
diff --git a/helm/amd-gpu/values.yaml b/helm/amd-gpu/values.yaml
index 56cc0dc5..84370066 100644
--- a/helm/amd-gpu/values.yaml
+++ b/helm/amd-gpu/values.yaml
@@ -4,8 +4,6 @@ nfd:
 labeller:
   enabled: false
 
-namespace: kube-system
-
 dp:
   image:
     repository: docker.io/rocm/k8s-device-plugin
-- 
GitLab