From 8fe389125c44bba154385f7af83c83ee75ddd14a Mon Sep 17 00:00:00 2001 From: Kai-Hsun Chen Date: Mon, 5 Jun 2023 11:56:37 -0700 Subject: [PATCH] [Feature] Watch CR in multiple namespaces with namespaced RBAC resources (#1106) Watch CR in multiple namespaces with namespaced RBAC resources --- .../templates/deployment.yaml | 4 +- .../templates/leader_election_role.yaml | 12 +- .../leader_election_role_binding.yaml | 3 +- .../templates/multiple_namespaces_role.yaml | 240 ++++++++++++++++++ .../multiple_namespaces_rolebinding.yaml | 22 ++ .../templates/ray_rayjob_editor_role.yaml | 9 +- .../templates/ray_rayjob_viewer_role.yaml | 9 +- .../templates/ray_rayservice_editor_role.yaml | 7 +- .../templates/ray_rayservice_viewer_role.yaml | 7 +- .../kuberay-operator/templates/role.yaml | 6 +- .../templates/rolebinding.yaml | 8 - helm-chart/kuberay-operator/values.yaml | 16 +- ray-operator/DEVELOPMENT.md | 3 +- .../config/rbac/leader_election_role.yaml | 9 + ray-operator/main.go | 24 +- 15 files changed, 321 insertions(+), 58 deletions(-) create mode 100644 helm-chart/kuberay-operator/templates/multiple_namespaces_role.yaml create mode 100644 helm-chart/kuberay-operator/templates/multiple_namespaces_rolebinding.yaml diff --git a/helm-chart/kuberay-operator/templates/deployment.yaml b/helm-chart/kuberay-operator/templates/deployment.yaml index 6f1f12092ff..0f5ca0e2c04 100644 --- a/helm-chart/kuberay-operator/templates/deployment.yaml +++ b/helm-chart/kuberay-operator/templates/deployment.yaml @@ -42,10 +42,10 @@ spec: {{- $argList = append $argList "--enable-batch-scheduler" -}} {{- end -}} {{- $watchNamespace := "" -}} - {{- if .Values.singleNamespaceInstall -}} + {{- if and .Values.singleNamespaceInstall (not .Values.watchNamespace) -}} {{- $watchNamespace = .Release.Namespace -}} {{- else if .Values.watchNamespace -}} - {{- $watchNamespace = .Values.watchNamespace -}} + {{- $watchNamespace = join "," .Values.watchNamespace -}} {{- end -}} {{- if $watchNamespace -}} {{- $argList = append $argList "--watch-namespace" -}} diff --git a/helm-chart/kuberay-operator/templates/leader_election_role.yaml b/helm-chart/kuberay-operator/templates/leader_election_role.yaml index efa2d852e6f..9049e4d4b76 100644 --- a/helm-chart/kuberay-operator/templates/leader_election_role.yaml +++ b/helm-chart/kuberay-operator/templates/leader_election_role.yaml @@ -2,8 +2,7 @@ kind: Role apiVersion: rbac.authorization.k8s.io/v1 metadata: - labels: -{{ include "kuberay-operator.labels" . | indent 4 }} + labels: {{ include "kuberay-operator.labels" . | nindent 4 }} name: {{ include "kuberay-operator.fullname" . }}-leader-election rules: - apiGroups: @@ -32,4 +31,13 @@ rules: - events verbs: - create +- apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - create + - get + - list + - update {{- end }} diff --git a/helm-chart/kuberay-operator/templates/leader_election_role_binding.yaml b/helm-chart/kuberay-operator/templates/leader_election_role_binding.yaml index 4092fc0983b..c2abffd48f3 100644 --- a/helm-chart/kuberay-operator/templates/leader_election_role_binding.yaml +++ b/helm-chart/kuberay-operator/templates/leader_election_role_binding.yaml @@ -2,8 +2,7 @@ kind: RoleBinding apiVersion: rbac.authorization.k8s.io/v1 metadata: - labels: -{{ include "kuberay-operator.labels" . | indent 4 }} + labels: {{ include "kuberay-operator.labels" . | nindent 4 }} name: {{ include "kuberay-operator.fullname" . }}-leader-election subjects: - kind: ServiceAccount diff --git a/helm-chart/kuberay-operator/templates/multiple_namespaces_role.yaml b/helm-chart/kuberay-operator/templates/multiple_namespaces_role.yaml new file mode 100644 index 00000000000..7d4928b0fa8 --- /dev/null +++ b/helm-chart/kuberay-operator/templates/multiple_namespaces_role.yaml @@ -0,0 +1,240 @@ +# Install Role for namespaces listed in watchNamespace. +# This should be consistent with `role.yaml`, except for the `kind` field. +{{- if and .Values.rbacEnable .Values.singleNamespaceInstall }} +{{- $watchNamespaces := default (list .Release.Namespace) .Values.watchNamespace }} +{{- range $namespace := $watchNamespaces }} +--- +kind: Role +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + labels: {{ include "kuberay-operator.labels" $ | nindent 4 }} + name: {{ include "kuberay-operator.fullname" $ }} + namespace: {{ $namespace }} +rules: +- apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - create + - get + - list + - update +- apiGroups: + - "" + resources: + - events + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - "" + resources: + - pods + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - "" + resources: + - pods/status + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - "" + resources: + - serviceaccounts + verbs: + - create + - delete + - get + - list + - watch +- apiGroups: + - "" + resources: + - services + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - "" + resources: + - services/status + verbs: + - get + - patch + - update +- apiGroups: + - extensions + resources: + - ingresses + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - networking.k8s.io + resources: + - ingressclasses + verbs: + - get + - list + - watch +- apiGroups: + - networking.k8s.io + resources: + - ingresses + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - ray.io + resources: + - rayclusters + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - ray.io + resources: + - rayclusters/finalizers + verbs: + - update +- apiGroups: + - ray.io + resources: + - rayclusters/status + verbs: + - get + - patch + - update +- apiGroups: + - ray.io + resources: + - rayjobs + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - ray.io + resources: + - rayjobs/finalizers + verbs: + - update +- apiGroups: + - ray.io + resources: + - rayjobs/status + verbs: + - get + - patch + - update +- apiGroups: + - ray.io + resources: + - rayservices + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - ray.io + resources: + - rayservices/finalizers + verbs: + - update +- apiGroups: + - ray.io + resources: + - rayservices/status + verbs: + - get + - patch + - update +- apiGroups: + - rbac.authorization.k8s.io + resources: + - rolebindings + verbs: + - create + - delete + - get + - list + - watch +- apiGroups: + - rbac.authorization.k8s.io + resources: + - roles + verbs: + - create + - delete + - get + - list + - update + - watch +{{- if $.Values.batchScheduler.enabled }} +- apiGroups: + - scheduling.volcano.sh + resources: + - podgroups + verbs: + - create + - delete + - get + - list + - update + - watch +- apiGroups: + - apiextensions.k8s.io + resources: + - customresourcedefinitions + verbs: + - get +{{- end }} +{{- end }} +{{- end }} diff --git a/helm-chart/kuberay-operator/templates/multiple_namespaces_rolebinding.yaml b/helm-chart/kuberay-operator/templates/multiple_namespaces_rolebinding.yaml new file mode 100644 index 00000000000..2acc74cce5b --- /dev/null +++ b/helm-chart/kuberay-operator/templates/multiple_namespaces_rolebinding.yaml @@ -0,0 +1,22 @@ +# Install RoleBinding for namespaces listed in watchNamespace. +# This should be consistent with `rolebinding.yaml`, except for the `kind` field. +{{- if and .Values.rbacEnable .Values.singleNamespaceInstall }} +{{- $watchNamespaces := default (list .Release.Namespace) .Values.watchNamespace }} +{{- range $namespace := $watchNamespaces }} +--- +kind: RoleBinding +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + labels: {{ include "kuberay-operator.labels" $ | nindent 4 }} + name: {{ include "kuberay-operator.fullname" $ }} + namespace: {{ $namespace }} +subjects: +- kind: ServiceAccount + name: {{ $.Values.serviceAccount.name }} + namespace: {{ $.Release.Namespace }} +roleRef: + kind: Role + name: {{ include "kuberay-operator.fullname" $ }} + apiGroup: rbac.authorization.k8s.io +{{- end }} +{{- end }} diff --git a/helm-chart/kuberay-operator/templates/ray_rayjob_editor_role.yaml b/helm-chart/kuberay-operator/templates/ray_rayjob_editor_role.yaml index fd8aafa8849..0de7282a836 100644 --- a/helm-chart/kuberay-operator/templates/ray_rayjob_editor_role.yaml +++ b/helm-chart/kuberay-operator/templates/ray_rayjob_editor_role.yaml @@ -1,15 +1,10 @@ # permissions for end users to edit rayjobs. -{{- if .Values.rbacEnable }} +{{- if and .Values.rbacEnable (not .Values.singleNamespaceInstall) }} -{{- if .Values.singleNamespaceInstall }} -kind: Role -{{- else }} kind: ClusterRole -{{- end }} apiVersion: rbac.authorization.k8s.io/v1 metadata: - labels: -{{ include "kuberay-operator.labels" . | indent 4 }} + labels: {{ include "kuberay-operator.labels" . | nindent 4 }} name: rayjob-editor-role rules: - apiGroups: diff --git a/helm-chart/kuberay-operator/templates/ray_rayjob_viewer_role.yaml b/helm-chart/kuberay-operator/templates/ray_rayjob_viewer_role.yaml index 5904cf3cb38..26906b0dc7c 100644 --- a/helm-chart/kuberay-operator/templates/ray_rayjob_viewer_role.yaml +++ b/helm-chart/kuberay-operator/templates/ray_rayjob_viewer_role.yaml @@ -1,15 +1,10 @@ # permissions for end users to view rayjobs. -{{- if .Values.rbacEnable }} +{{- if and .Values.rbacEnable (not .Values.singleNamespaceInstall) }} -{{- if .Values.singleNamespaceInstall }} -kind: Role -{{- else }} kind: ClusterRole -{{- end }} apiVersion: rbac.authorization.k8s.io/v1 metadata: - labels: -{{ include "kuberay-operator.labels" . | indent 4 }} + labels: {{ include "kuberay-operator.labels" . | nindent 4 }} name: rayjob-viewer-role rules: - apiGroups: diff --git a/helm-chart/kuberay-operator/templates/ray_rayservice_editor_role.yaml b/helm-chart/kuberay-operator/templates/ray_rayservice_editor_role.yaml index 9002967f74c..2e2c0fa4401 100644 --- a/helm-chart/kuberay-operator/templates/ray_rayservice_editor_role.yaml +++ b/helm-chart/kuberay-operator/templates/ray_rayservice_editor_role.yaml @@ -1,12 +1,7 @@ # permissions for end users to edit rayservices. -{{- if .Values.rbacEnable }} +{{- if and .Values.rbacEnable (not .Values.singleNamespaceInstall) }} apiVersion: rbac.authorization.k8s.io/v1 - -{{- if .Values.singleNamespaceInstall }} -kind: Role -{{- else }} kind: ClusterRole -{{- end }} metadata: name: rayservice-editor-role rules: diff --git a/helm-chart/kuberay-operator/templates/ray_rayservice_viewer_role.yaml b/helm-chart/kuberay-operator/templates/ray_rayservice_viewer_role.yaml index 7a993c722b5..9641dbb83d9 100644 --- a/helm-chart/kuberay-operator/templates/ray_rayservice_viewer_role.yaml +++ b/helm-chart/kuberay-operator/templates/ray_rayservice_viewer_role.yaml @@ -1,12 +1,7 @@ # permissions for end users to view rayservices. -{{- if .Values.rbacEnable }} +{{- if and .Values.rbacEnable (not .Values.singleNamespaceInstall) }} apiVersion: rbac.authorization.k8s.io/v1 - -{{- if .Values.singleNamespaceInstall }} -kind: Role -{{- else }} kind: ClusterRole -{{- end }} metadata: name: rayservice-viewer-role rules: diff --git a/helm-chart/kuberay-operator/templates/role.yaml b/helm-chart/kuberay-operator/templates/role.yaml index 07d87858b61..904b565b6cd 100644 --- a/helm-chart/kuberay-operator/templates/role.yaml +++ b/helm-chart/kuberay-operator/templates/role.yaml @@ -1,10 +1,6 @@ -{{- if .Values.rbacEnable }} +{{- if and .Values.rbacEnable (not .Values.singleNamespaceInstall) }} -{{- if .Values.singleNamespaceInstall }} -kind: Role -{{- else }} kind: ClusterRole -{{- end }} apiVersion: rbac.authorization.k8s.io/v1 metadata: labels: diff --git a/helm-chart/kuberay-operator/templates/rolebinding.yaml b/helm-chart/kuberay-operator/templates/rolebinding.yaml index d2de61207e6..cfa1d0cf80c 100644 --- a/helm-chart/kuberay-operator/templates/rolebinding.yaml +++ b/helm-chart/kuberay-operator/templates/rolebinding.yaml @@ -1,9 +1,5 @@ {{- if .Values.rbacEnable }} -{{- if .Values.singleNamespaceInstall }} -kind: RoleBinding -{{- else }} kind: ClusterRoleBinding -{{- end }} apiVersion: rbac.authorization.k8s.io/v1 metadata: labels: @@ -14,11 +10,7 @@ subjects: name: {{ .Values.serviceAccount.name }} namespace: {{ .Release.Namespace }} roleRef: -{{- if .Values.singleNamespaceInstall }} - kind: Role -{{- else }} kind: ClusterRole -{{- end }} name: {{ include "kuberay-operator.fullname" . }} apiGroup: rbac.authorization.k8s.io {{- end }} diff --git a/helm-chart/kuberay-operator/values.yaml b/helm-chart/kuberay-operator/values.yaml index d56578cf1c8..066f232c6c7 100644 --- a/helm-chart/kuberay-operator/values.yaml +++ b/helm-chart/kuberay-operator/values.yaml @@ -55,16 +55,16 @@ batchScheduler: securityContext: {} # When singleNamespaceInstall is true: -# - the chart can be installed by users with permissions to a single namespace only -# (this excludes the CRDs which can only be installed at cluster-scope) -# - the KubeRay operator will only listen to the resource -# events from the operator's namespace by default +# - Install namespaced RBAC resources instead of cluster-scoped ones so that the chart can be installed by users +# with permissions restricted to a single namespace. (Please note that this excludes the CRDs, which can only be installed at the cluster scope.) +# - If "watchNamespace" is not set, the KubeRay operator will, by default, only listen +# to resource events within its own namespace. singleNamespaceInstall: false -# kuberay operator will only watch the resource events from the "watchNamespace" namespace. -# this option has no effect if singleNamespaceInstall is true, because we assume there are no -# permissions outside of the current namespace -# watchNamespace: ray-user-namespace +# The KubeRay operator will watch the custom resources in the namespaces listed in the "watchNamespace" parameter. +# watchNamespace: +# - n1 +# - n2 # Environment variables env: diff --git a/ray-operator/DEVELOPMENT.md b/ray-operator/DEVELOPMENT.md index 88c1d7d5c5d..c5bbf8a3a0d 100644 --- a/ray-operator/DEVELOPMENT.md +++ b/ray-operator/DEVELOPMENT.md @@ -134,6 +134,7 @@ We have several [consistency checks](https://github.com/ray-project/kuberay/blob 3. CRD YAML files in `ray-operator/config/crd/bases/` and `helm-chart/kuberay-operator/crds/` should be the same. 4. Kubebuilder markers in `ray-operator/controllers/ray/*_controller.go` should be synchronized with RBAC YAML files in `ray-operator/config/rbac`. 5. RBAC YAML files in `helm-chart/kuberay-operator/templates` and `ray-operator/config/rbac` should be synchronized. **Currently, we need to synchronize this manually.** See [#631](https://github.com/ray-project/kuberay/pull/631) as an example. +6. `multiple_namespaces_role.yaml` and `multiple_namespaces_rolebinding.yaml` should be synchronized with `role.yaml` and `rolebinding.yaml` in the `helm-chart/kuberay-operator/templates` directory. The only difference is that the former creates namespaced RBAC resources, while the latter creates cluster-scoped RBAC resources. ```bash # Synchronize consistency 1 and 4: @@ -146,7 +147,7 @@ make manifests make helm # Synchronize 1, 2, 3, and 4 in one command -# [Note]: Currently, we need to synchronize consistency 5 manually. +# [Note]: Currently, we need to synchronize consistency 5 and 6 manually. make sync # Reproduce CI error for job "helm-chart-verify-rbac" (consistency 5) diff --git a/ray-operator/config/rbac/leader_election_role.yaml b/ray-operator/config/rbac/leader_election_role.yaml index 01f93bbf888..81132016932 100644 --- a/ray-operator/config/rbac/leader_election_role.yaml +++ b/ray-operator/config/rbac/leader_election_role.yaml @@ -30,3 +30,12 @@ rules: - events verbs: - create +- apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - create + - get + - list + - update diff --git a/ray-operator/main.go b/ray-operator/main.go index 4cd26994df5..a86468d0487 100644 --- a/ray-operator/main.go +++ b/ray-operator/main.go @@ -4,6 +4,7 @@ import ( "flag" "fmt" "os" + "strings" "github.com/go-logr/zapr" "go.uber.org/zap" @@ -13,6 +14,7 @@ import ( "github.com/ray-project/kuberay/ray-operator/controllers/ray/batchscheduler" "go.uber.org/zap/zapcore" + "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/healthz" "k8s.io/apimachinery/pkg/runtime" @@ -58,7 +60,7 @@ func main() { &watchNamespace, "watch-namespace", "", - "Watch custom resources in the namespace, ignore other namespaces. If empty, all namespaces will be watched.") + "Specify a list of namespaces to watch for custom resources, separated by commas. If left empty, all namespaces will be watched.") flag.BoolVar(&ray.PrioritizeWorkersToDelete, "prioritize-workers-to-delete", true, "Temporary feature flag - to be deleted after testing") flag.BoolVar(&ray.ForcedClusterUpgrade, "forced-cluster-upgrade", false, @@ -117,15 +119,29 @@ func main() { setupLog.Info("Feature flag enable-batch-scheduler is enabled.") } - mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ + watchNamespaces := strings.Split(watchNamespace, ",") + options := ctrl.Options{ Scheme: scheme, MetricsBindAddress: metricsAddr, Port: 9443, HealthProbeBindAddress: probeAddr, LeaderElection: enableLeaderElection, LeaderElectionID: "ray-operator-leader", - Namespace: watchNamespace, - }) + } + + if len(watchNamespaces) == 1 { // It is not possible for len(watchNamespaces) == 0 to be true. The length of `strings.Split("", ",")`` is still 1. + options.Namespace = watchNamespaces[0] + if watchNamespaces[0] == "" { + setupLog.Info("Flag watchNamespace is not set. Watch custom resources in all namespaces.") + } else { + setupLog.Info(fmt.Sprintf("Only watch custom resources in the namespace: %s", watchNamespaces[0])) + } + } else { + options.NewCache = cache.MultiNamespacedCacheBuilder(watchNamespaces) + setupLog.Info(fmt.Sprintf("Only watch custom resources in multiple namespaces: %v", watchNamespaces)) + } + setupLog.Info("Setup manager") + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), options) if err != nil { setupLog.Error(err, "unable to start manager") os.Exit(1)