Repository: pravega/zookeeper-operator (go)

Issue:
allow to set a SeviceAccount to cluster Statefulset
We need to be able to define a Kubernetes `ServiceAccount` in the Statefulset, so we can use images from private repositories (with specific ImagePullSecrets).

Pull request:
Creating service account by operator
Signed-off-by: anisha.kj <anisha.kj@dell.com>

### Change log description

Made changes to create service account for statefulset in operator.

### Purpose of the change

 Fixes #78

### What the code does

If service account is set in zookeepercluster spec, operator will create the service account.

### How to verify it
Verified installation of operator and zookeeper cluster.
Verified upgrade of operator and cluster



Patch:
diff --git a/README.md b/README.md
index b8683c6..d2be551 100644
--- a/README.md
+++ b/README.md
@@ -15,6 +15,7 @@ The project is currently alpha. While no breaking API changes are currently plan
     * [Deploy a sample Zookeeper Cluster to a cluster using Istio](#deploy-a-sample-zookeeper-cluster-with-istio)
     * [Upgrade a Zookeeper Cluster](#upgrade-a-zookeeper-cluster)
     * [Uninstall the Zookeeper Cluster](#uninstall-the-zookeeper-cluster)
+    * [Upgrade the Zookeeper Operator](#upgrade-the-operator)
     * [Uninstall the Operator](#uninstall-the-operator)
  * [Development](#development)
     * [Build the Operator Image](#build-the-operator-image)
@@ -150,7 +151,17 @@ svc/zookeeper-client     ClusterIP   10.31.243.173   <none>        2181/TCP
 svc/zookeeper-headless   ClusterIP   None            <none>        2888/TCP,3888/TCP   2m
 ```
 
->Note: If you want to configure non deafult service accounts to zookeeper pods, refer to [this](doc/rbac.md).
+>Note: If you want to configure non default service accounts to zookeeper pods, set the service account inside pod.This support is added from zookeeper operator version `0.2.9` onwards.
+
+```
+apiVersion: "zookeeper.pravega.io/v1beta1"
+kind: "ZookeeperCluster"
+metadata:
+  name: "example"
+spec:
+  pod:
+    serviceAccountName: "zookeeper"
+```
 
 ### Deploy a sample Zookeeper cluster with Ephemeral storage
 
@@ -270,6 +281,10 @@ Status:
 ```
 >Note: The value of the tag field should not be modified while an upgrade is already in progress.
 
+### Upgrade the Operator
+
+For upgrading the zookeeper operator check the document [operator-upgrade](doc/operator-upgrade.md)
+
 ### Uninstall the Zookeeper cluster
 
 #### Uninstall via helm
diff --git a/charts/zookeeper-operator/templates/clusterrole.yaml b/charts/zookeeper-operator/templates/clusterrole.yaml
index 3ca159c..1d724c7 100644
--- a/charts/zookeeper-operator/templates/clusterrole.yaml
+++ b/charts/zookeeper-operator/templates/clusterrole.yaml
@@ -23,6 +23,7 @@ rules:
   - events
   - configmaps
   - secrets
+  - serviceaccounts
   verbs:
   - "*"
 - apiGroups:
diff --git a/charts/zookeeper/templates/clusterrole.yaml b/charts/zookeeper/templates/clusterrole.yaml
deleted file mode 100644
index bb0dc76..0000000
--- a/charts/zookeeper/templates/clusterrole.yaml
+++ /dev/null
@@ -1,14 +0,0 @@
-kind: ClusterRole
-apiVersion: rbac.authorization.k8s.io/v1
-metadata:
-  name: {{ default "zookeeper" .Values.pod.serviceAccountName }}
-  namespace: {{ .Release.Namespace }}
-  labels:
-{{ include "zookeeper.commonLabels" . | indent 4 }}
-rules:
-- apiGroups:
-  - ""
-  resources:
-  - nodes
-  verbs:
-  - get
diff --git a/charts/zookeeper/templates/clusterrolebinding.yaml b/charts/zookeeper/templates/clusterrolebinding.yaml
deleted file mode 100644
index 37c1308..0000000
--- a/charts/zookeeper/templates/clusterrolebinding.yaml
+++ /dev/null
@@ -1,15 +0,0 @@
-kind: ClusterRoleBinding
-apiVersion: rbac.authorization.k8s.io/v1beta1
-metadata:
-  name: {{ default "zookeeper" .Values.pod.serviceAccountName }}
-  namespace: {{ .Release.Namespace }}
-  labels:
-{{ include "zookeeper.commonLabels" . | indent 4 }}
-subjects:
-- kind: ServiceAccount
-  name: {{ default "zookeeper" .Values.pod.serviceAccountName }}
-  namespace: {{ .Release.Namespace }}
-roleRef:
-  kind: ClusterRole
-  name: {{ default "zookeeper" .Values.pod.serviceAccountName }}
-  apiGroup: rbac.authorization.k8s.io
diff --git a/charts/zookeeper/templates/role.yaml b/charts/zookeeper/templates/role.yaml
deleted file mode 100644
index c058fab..0000000
--- a/charts/zookeeper/templates/role.yaml
+++ /dev/null
@@ -1,21 +0,0 @@
-kind: Role
-apiVersion: rbac.authorization.k8s.io/v1
-metadata:
-  name: {{ default "zookeeper" .Values.pod.serviceAccountName }}
-  namespace: {{ .Release.Namespace }}
-  labels:
-{{ include "zookeeper.commonLabels" . | indent 4 }}
-rules:
-- apiGroups:
-  - zookeeper.pravega.io
-  resources:
-  - "*"
-  verbs:
-  - get
-- apiGroups:
-  - ""
-  resources:
-  - pods
-  - services
-  verbs:
-  - get
diff --git a/charts/zookeeper/templates/rolebinding.yaml b/charts/zookeeper/templates/rolebinding.yaml
deleted file mode 100644
index 91535ac..0000000
--- a/charts/zookeeper/templates/rolebinding.yaml
+++ /dev/null
@@ -1,15 +0,0 @@
-kind: RoleBinding
-apiVersion: rbac.authorization.k8s.io/v1
-metadata:
-  name: {{ default "zookeeper" .Values.pod.serviceAccountName }}
-  namespace: {{ .Release.Namespace }}
-  labels:
-{{ include "zookeeper.commonLabels" . | indent 4 }}
-subjects:
-- kind: ServiceAccount
-  name: {{ default "zookeeper" .Values.pod.serviceAccountName }}
-  namespace: {{ .Release.Namespace }}
-roleRef:
-  kind: Role
-  name: {{ default "zookeeper" .Values.pod.serviceAccountName }}
-  apiGroup: rbac.authorization.k8s.io
diff --git a/charts/zookeeper/templates/service_account.yaml b/charts/zookeeper/templates/service_account.yaml
deleted file mode 100644
index 09dfa7f..0000000
--- a/charts/zookeeper/templates/service_account.yaml
+++ /dev/null
@@ -1,7 +0,0 @@
-apiVersion: v1
-kind: ServiceAccount
-metadata:
-  name: {{ default "zookeeper" .Values.pod.serviceAccountName }}
-  namespace: {{ .Release.Namespace }}
-  labels:
-{{ include "zookeeper.commonLabels" . | indent 4 }}
diff --git a/doc/operator-upgrade.md b/doc/operator-upgrade.md
new file mode 100644
index 0000000..b4a640f
--- /dev/null
+++ b/doc/operator-upgrade.md
@@ -0,0 +1,27 @@
+# Upgrading Operator
+zookeeperoperator can be upgraded to a version **[VERSION]** via helm using the following command
+
+```
+$ helm upgrade [ZOOKEEPER_OPERATOR_RELEASE_NAME] pravega/zookeeper-operator --version=[VERSION]
+```
+The zookeeper operator with deployment name **[DEPLOYMENT_NAME]** can also be upgraded manually by modifying the image tag using kubectl edit, patch or apply
+```
+$ kubectl edit deploy [DEPLOYMENT_NAME]
+```
+> Note: If you are upgrading zookeeper operator version to 0.2.9 or above manually, clusterrole has to be updated to include serviceaccounts. After updating clusterroles, zookeeper operator pod has to be restarted for the changes to take effect.
+
+```
+- apiGroups:
+  - ""
+  resources:
+  - pods
+  - services
+  - endpoints
+  - persistentvolumeclaims
+  - events
+  - configmaps
+  - secrets
+  - serviceaccounts
+  verbs:
+  - "*"
+```
diff --git a/doc/rbac.md b/doc/rbac.md
deleted file mode 100644
index e3f58de..0000000
--- a/doc/rbac.md
+++ /dev/null
@@ -1,92 +0,0 @@
-## Setting up RBAC for zookeeper stateful set
-### Use non-default service accounts
-
-From zookeeper operator version `0.2.9` onwards, support is added to configure non deault service accounts for zookeeper pods.
-
-Below are the steps
-
-1. Create a service account in the namespace where you want to deploy zookeeper cluster
-
-```
-apiVersion: v1
-kind: ServiceAccount
-metadata:
-  name: zookeeper
-```
-
-2. Create `Role` and `ClusterRole` with the minimum required permissions. Make sure to update the `default` namespace if you are deploying zookeeper to a custom namespace.
-
-```
-kind: Role
-apiVersion: rbac.authorization.k8s.io/v1
-metadata:
-  name: zookeeper
-  namespace: "default"
-rules:
-- apiGroups: ["zookeeper.pravega.io"]
-  resources: ["*"]
-  verbs: ["get"]
-- apiGroups: [""]
-  resources: ["pods", "services"]
-  verbs: ["get"]
----
-kind: ClusterRole
-apiVersion: rbac.authorization.k8s.io/v1
-metadata:
-  name: zookeeper
-rules:
-- apiGroups: [""]
-  resources: ["nodes"]
-  verbs: ["get"]
-```
-3. Bind roles and cluster roles to the service account.
-
-```
-kind: RoleBinding
-apiVersion: rbac.authorization.k8s.io/v1
-metadata:
-  name: zookeeper
-subjects:
-- kind: ServiceAccount
-  name: zookeeper
-roleRef:
-  kind: Role
-  name: zookeeper
-  apiGroup: rbac.authorization.k8s.io
----
-kind: ClusterRoleBinding
-apiVersion: rbac.authorization.k8s.io/v1beta1
-metadata:
-  name: zookeeper
-subjects:
-- kind: ServiceAccount
-  name: zookeeper
-  namespace: default
-roleRef:
-  kind: ClusterRole
-  name: zookeeper
-  apiGroup: rbac.authorization.k8s.io
-```
-
-4. Create zookeeper cluster
-  ```
-  apiVersion: zookeeper.pravega.io/v1beta1
-  kind: ZookeeperCluster
-  metadata:
-    name: zookeeper
-  spec:
-    replicas: 3
-    image:
-      repository: pravega/zookeeper
-      tag: 0.2.9
-    pod:
-      serviceAccountName: zookeeper  
-    storageType: persistence
-    persistence:
-      reclaimPolicy: Delete
-      spec:
-        storageClassName: "standard"
-        resources:
-          requests:
-            storage: 20Gi
-```
diff --git a/pkg/controller/zookeepercluster/zookeepercluster_controller.go b/pkg/controller/zookeepercluster/zookeepercluster_controller.go
index 4bf432e..39e869d 100644
--- a/pkg/controller/zookeepercluster/zookeepercluster_controller.go
+++ b/pkg/controller/zookeepercluster/zookeepercluster_controller.go
@@ -169,7 +169,24 @@ func (r *ReconcileZookeeperCluster) reconcileStatefulSet(instance *zookeeperv1be
 	if instance.Status.IsClusterInUpgradeFailedState() {
 		return nil
 	}
-
+	if instance.Spec.Pod.ServiceAccountName != "default" {
+		serviceAccount := zk.MakeServiceAccount(instance)
+		if err = controllerutil.SetControllerReference(instance, serviceAccount, r.scheme); err != nil {
+			return err
+		}
+		// Check if this ServiceAccount already exists
+		foundServiceAccount := &corev1.ServiceAccount{}
+		err = r.client.Get(context.TODO(), types.NamespacedName{Name: serviceAccount.Name, Namespace: serviceAccount.Namespace}, foundServiceAccount)
+		if err != nil && errors.IsNotFound(err) {
+			r.log.Info("Creating a new ServiceAccount", "ServiceAccount.Namespace", serviceAccount.Namespace, "ServiceAccount.Name", serviceAccount.Name)
+			err = r.client.Create(context.TODO(), serviceAccount)
+			if err != nil {
+				return err
+			}
+		} else if err != nil {
+			return err
+		}
+	}
 	sts := zk.MakeStatefulSet(instance)
 	if err = controllerutil.SetControllerReference(instance, sts, r.scheme); err != nil {
 		return err
diff --git a/pkg/zk/generators.go b/pkg/zk/generators.go
index bda8703..2a8ef2d 100644
--- a/pkg/zk/generators.go
+++ b/pkg/zk/generators.go
@@ -318,3 +318,13 @@ func MakePodDisruptionBudget(z *v1beta1.ZookeeperCluster) *policyv1beta1.PodDisr
 		},
 	}
 }
+
+//MakeServiceAccount returns the service account for zookeeper Cluster
+func MakeServiceAccount(z *v1beta1.ZookeeperCluster) *v1.ServiceAccount {
+	return &v1.ServiceAccount{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      z.Spec.Pod.ServiceAccountName,
+			Namespace: z.Namespace,
+		},
+	}
+}


Test patch:
diff --git a/pkg/controller/zookeepercluster/zookeepercluster_controller_test.go b/pkg/controller/zookeepercluster/zookeepercluster_controller_test.go
index 66dc75b..13364fd 100644
--- a/pkg/controller/zookeepercluster/zookeepercluster_controller_test.go
+++ b/pkg/controller/zookeepercluster/zookeepercluster_controller_test.go
@@ -200,6 +200,7 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 
 			BeforeEach(func() {
 				z.WithDefaults()
+				z.Spec.Pod.ServiceAccountName = "zookeeper"
 				z.Status.Init()
 				next := z.DeepCopy()
 				st := zk.MakeStatefulSet(z)
diff --git a/pkg/zk/generators_test.go b/pkg/zk/generators_test.go
index a1a46f5..a19f932 100644
--- a/pkg/zk/generators_test.go
+++ b/pkg/zk/generators_test.go
@@ -176,6 +176,29 @@ var _ = Describe("Generators Spec", func() {
 			})
 		})
 	})
+	Context("#MakeStatefulSet with non default service account", func() {
+		var sts *appsv1.StatefulSet
+
+		Context("with defaults", func() {
+
+			BeforeEach(func() {
+				z := &v1beta1.ZookeeperCluster{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "example",
+						Namespace: "default",
+					},
+					Spec: v1beta1.ZookeeperClusterSpec{},
+				}
+				z.Spec.Pod.ServiceAccountName = "zookeeper"
+				z.WithDefaults()
+				zk.MakeServiceAccount(z)
+				sts = zk.MakeStatefulSet(z)
+			})
+			It("Checking the sts service account", func() {
+				Ω(sts.Spec.Template.Spec.ServiceAccountName).To(Equal("zookeeper"))
+			})
+		})
+	})
 
 	Context("#MakeClientService", func() {
 		var s *v1.Service
diff --git a/test/e2e/resources/rbac-operator.yaml b/test/e2e/resources/rbac-operator.yaml
index 5db8db9..cc2ed43 100644
--- a/test/e2e/resources/rbac-operator.yaml
+++ b/test/e2e/resources/rbac-operator.yaml
@@ -28,6 +28,7 @@ rules:
   - events
   - configmaps
   - secrets
+  - serviceaccounts
   verbs:
   - "*"
 - apiGroups: