Task/odf 564 delete cms (#151)

* move delete cm function into utils * add deletion when removing odf-operator pod * add cm permissions * add ownership to CMs * add new env variable for operator pod name * add new env variable for operator pod name * fix typo * add permissions for pods * add permissions for pods * fix unitest - create pod manually * change by review --------- Signed-off-by: 662962756 <[email protected]>
IBM · Jul 16, 2023 · 649561b · 649561b
1 parent 5e30685
commit 649561b
Show file tree

Hide file tree

Showing 12 changed files with 144 additions and 33 deletions.
diff --git a/bundle/manifests/ibm-storage-odf-operator.clusterserviceversion.yaml b/bundle/manifests/ibm-storage-odf-operator.clusterserviceversion.yaml
@@ -73,6 +73,14 @@ spec:
                 - patch
                 - update
                 - watch
+            - apiGroups:
+              - ""
+              resources:
+                - pods
+              verbs:
+                - get
+                - list
+                - watch
             - apiGroups:
                 - ""
               resources:
@@ -327,6 +335,10 @@ spec:
                         valueFrom:
                           fieldRef:
                             fieldPath: metadata.annotations['olm.operatorNamespace']
+                      - name: OPERATOR_POD_NAME
+                        valueFrom:
+                          fieldRef:
+                            fieldPath: metadata.name
                     image: registry.connect.redhat.com/ibm/ibm-storage-odf-operator:1.5.0
                     imagePullPolicy: Always
                     livenessProbe:

diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml
@@ -42,6 +42,10 @@ spec:
             valueFrom:
               fieldRef:
                 fieldPath: metadata.annotations['olm.operatorNamespace']
+          - name: OPERATOR_POD_NAME
+            valueFrom:
+              fieldRef:
+                fieldPath: metadata.name
           - name: EXPORTER_IMAGE
             value: docker.io/ibmcom/ibm-storage-odf-block-driver:v0.0.22
         securityContext:

diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml
@@ -117,6 +117,14 @@ rules:
   verbs:
   - get
   - list
+- apiGroups:
+  - ""
+  resources:
+  - pods
+  verbs:
+  - get
+  - list
+  - watch
 - apiGroups:
   - ""
   resources:

diff --git a/controllers/exporter.go b/controllers/exporter.go
@@ -77,7 +77,7 @@ func getExporterMetricsServiceMonitorName() string {
 	return fsServiceMonitorName
 }
 
-func isOwnerExist(ownerReferences []metav1.OwnerReference, newOwner metav1.OwnerReference) bool {
+func IsOwnerExist(ownerReferences []metav1.OwnerReference, newOwner metav1.OwnerReference) bool {
 	for _, owner := range ownerReferences {
 		if owner.UID == newOwner.UID {
 			return true
@@ -135,7 +135,7 @@ func updateExporterMetricsService(foundService *corev1.Service, expectedService
 		isChanged = true
 	}
 
-	if !isOwnerExist(foundService.GetOwnerReferences(), newOwner) {
+	if !IsOwnerExist(foundService.GetOwnerReferences(), newOwner) {
 		foundService.SetOwnerReferences(append(foundService.GetOwnerReferences(), newOwner))
 		isChanged = true
 	}
@@ -300,7 +300,7 @@ func InitExporterDeployment(
 }
 
 func updateExporterDeployment(found *appsv1.Deployment, expected *appsv1.Deployment, newOwner metav1.OwnerReference) *appsv1.Deployment {
-	isExist := isOwnerExist(found.GetOwnerReferences(), newOwner)
+	isExist := IsOwnerExist(found.GetOwnerReferences(), newOwner)
 	if !isExist {
 		found.SetOwnerReferences(append(found.GetOwnerReferences(), newOwner))
 	}
@@ -351,7 +351,7 @@ func InitExporterMetricsServiceMonitor(instance *odfv1alpha1.FlashSystemCluster)
 func updateExporterMetricsServiceMonitor(foundServiceMonitor *monitoringv1.ServiceMonitor,
 	expectedServiceMonitor *monitoringv1.ServiceMonitor, newOwner metav1.OwnerReference) *monitoringv1.ServiceMonitor {
 
-	isExist := isOwnerExist(foundServiceMonitor.GetOwnerReferences(), newOwner)
+	isExist := IsOwnerExist(foundServiceMonitor.GetOwnerReferences(), newOwner)
 	if !isExist {
 		foundServiceMonitor.SetOwnerReferences(append(foundServiceMonitor.GetOwnerReferences(), newOwner))
 	}

diff --git a/controllers/flashsystemcluster_controller.go b/controllers/flashsystemcluster_controller.go
@@ -59,8 +59,7 @@ type ReconcileMapper struct {
 
 func (s *ReconcileMapper) DefaultStorageClassToClusterMapperFunc(object client.Object) []reconcile.Request {
 	clusters := &odfv1alpha1.FlashSystemClusterList{}
-	err := s.reconciler.Client.List(context.TODO(), clusters)
-	if err != nil {
+	if err := s.reconciler.Client.List(context.TODO(), clusters); err != nil {
 		s.reconciler.Log.Error(err, "failed to list FlashSystemCluster", "DefaultStorageClassToClusterMapperFunc", s)
 		return nil
 	}
@@ -90,30 +89,16 @@ func (s *ReconcileMapper) ConfigMapToClusterMapFunc(object client.Object) []reco
 
 	if object.GetName() == util.FscCmName {
 		s.reconciler.Log.Info("Discovered ODF-FS configMap deletion. Deleting Pools ConfigMap", "ConfigMapToClusterMapFunc", s)
-		foundPoolsCm, err := util.GetCreateConfigmap(s.reconciler.Client, s.reconciler.Log, object.GetNamespace(), false, util.PoolsCmName)
-		if err != nil {
-			if errors.IsNotFound(err) {
-				s.reconciler.Log.Info("ConfigMap is already deleted", "ConfigMap", util.PoolsCmName)
-			} else {
-				return nil // Error reading the object - requeue the request.
-			}
-		} else {
-			err = s.reconciler.Client.Delete(
-				context.TODO(),
-				foundPoolsCm)
-			if err != nil {
-				s.reconciler.Log.Error(err, "failed to delete ConfigMap", "ConfigMap", util.PoolsCmName)
-				return nil
-			}
+		if err := util.DeleteConfigMap(s.reconciler.Log, s.reconciler.Client, util.PoolsCmName, object.GetNamespace()); err != nil {
+			s.reconciler.Log.Error(err, "failed to delete ConfigMap", "ConfigMap", util.PoolsCmName)
+			return nil
 		}
 	}
 
 	if object.GetName() == util.PoolsCmName {
 		s.reconciler.Log.Info("Discovered Pools ConfigMap deletion. Reconciling all FlashSystemClusters", "ConfigMapToClusterMapFunc", s)
-
 		clusters := &odfv1alpha1.FlashSystemClusterList{}
-		err := s.reconciler.Client.List(context.TODO(), clusters)
-		if err != nil {
+		if err := s.reconciler.Client.List(context.TODO(), clusters); err != nil {
 			s.reconciler.Log.Error(err, "failed to list FlashSystemCluster", "ConfigMapToClusterMapFunc", s)
 			return nil
 		}
@@ -135,8 +120,7 @@ func (s *ReconcileMapper) CSIToClusterMapFunc(_ client.Object) []reconcile.Reque
 	s.reconciler.IsCSICRCreated = false
 
 	clusters := &odfv1alpha1.FlashSystemClusterList{}
-	err := s.reconciler.Client.List(context.TODO(), clusters)
-	if err != nil {
+	if err := s.reconciler.Client.List(context.TODO(), clusters); err != nil {
 		s.reconciler.Log.Error(err, "failed to list FlashSystemCluster", "CSIToClusterMapFunc", s)
 		return nil
 	}
@@ -161,8 +145,7 @@ func (s *ReconcileMapper) CSIToClusterMapFunc(_ client.Object) []reconcile.Reque
 func (s *ReconcileMapper) SecretToClusterMapFunc(object client.Object) []reconcile.Request {
 	clusters := &odfv1alpha1.FlashSystemClusterList{}
 
-	err := s.reconciler.Client.List(context.TODO(), clusters)
-	if err != nil {
+	if err := s.reconciler.Client.List(context.TODO(), clusters); err != nil {
 		s.reconciler.Log.Error(err, "failed to list FlashSystemCluster", "SecretToClusterMapFunc", s)
 		return nil
 	}
@@ -179,7 +162,6 @@ func (s *ReconcileMapper) SecretToClusterMapFunc(object client.Object) []reconci
 			}
 			requests = append(requests, req)
 		}
-
 	}
 
 	if len(requests) > 0 {
@@ -207,6 +189,7 @@ type FlashSystemClusterReconciler struct {
 //+kubebuilder:rbac:groups=core,resources=namespaces,verbs=get;list
 //+kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete
 //+kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch;create;update;patch;delete
+//+kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;watch
 //+kubebuilder:rbac:groups=core,resources=events,verbs=get;list;watch;create;update;patch
 //+kubebuilder:rbac:groups=core,resources=services,verbs=get;list;watch;create;update;patch;delete
 //+kubebuilder:rbac:groups=core,resources=configmaps,verbs=get;list;watch;create;update;patch;delete
@@ -285,6 +268,12 @@ func (r *FlashSystemClusterReconciler) reconcile(instance *odfv1alpha1.FlashSyst
 		UID:        instance.UID,
 	}
 
+	newCMOwnerDetails, err := r.getCmOwnerDetails(instance.Namespace)
+	if err != nil {
+		r.Log.Error(err, "failed to get operator pod details", "ConfigMap", util.FscCmName)
+		return reconcile.Result{}, err
+	}
+
 	// Check GetDeletionTimestamp to determine if the object is under deletion
 	if instance.GetDeletionTimestamp().IsZero() {
 		if !util.IsContain(instance.GetFinalizers(), flashSystemClusterFinalizer) {
@@ -344,7 +333,7 @@ func (r *FlashSystemClusterReconciler) reconcile(instance *odfv1alpha1.FlashSyst
 	}
 
 	r.Log.Info("step: ensureFscConfigMap")
-	if err = r.ensureFscConfigMap(instance); err != nil {
+	if err = r.ensureFscConfigMap(instance, newCMOwnerDetails); err != nil {
 		reason := odfv1alpha1.ReasonReconcileFailed
 		message := fmt.Sprintf("failed to ensureFscConfigMap: %v", err)
 		util.SetReconcileErrorCondition(&instance.Status.Conditions, reason, message)
@@ -356,7 +345,7 @@ func (r *FlashSystemClusterReconciler) reconcile(instance *odfv1alpha1.FlashSyst
 	}
 
 	r.Log.Info("step: ensurePoolsConfigMap")
-	if err = r.ensurePoolsConfigMap(instance); err != nil {
+	if err = r.ensurePoolsConfigMap(instance, newCMOwnerDetails); err != nil {
 		reason := odfv1alpha1.ReasonReconcileFailed
 		message := fmt.Sprintf("failed to ensurePoolsConfigMap: %v", err)
 		util.SetReconcileErrorCondition(&instance.Status.Conditions, reason, message)
@@ -544,12 +533,16 @@ func (r *FlashSystemClusterReconciler) createEvent(instance *odfv1alpha1.FlashSy
 }
 
 // this object will not bind with instance
-func (r *FlashSystemClusterReconciler) ensureFscConfigMap(instance *odfv1alpha1.FlashSystemCluster) error {
+func (r *FlashSystemClusterReconciler) ensureFscConfigMap(instance *odfv1alpha1.FlashSystemCluster, newOwnerDetails v1.OwnerReference) error {
 	configmap, err := util.GetCreateConfigmap(r.Client, r.Log, watchNamespace, true, util.FscCmName)
 	if err != nil {
 		return err
 	}
 
+	if !IsOwnerExist(configmap.GetOwnerReferences(), newOwnerDetails) {
+		configmap.SetOwnerReferences(append(configmap.GetOwnerReferences(), newOwnerDetails))
+	}
+
 	if configmap.Data == nil {
 		configmap.Data = make(map[string]string)
 	}
@@ -575,12 +568,16 @@ func (r *FlashSystemClusterReconciler) ensureFscConfigMap(instance *odfv1alpha1.
 	return nil
 }
 
-func (r *FlashSystemClusterReconciler) ensurePoolsConfigMap(instance *odfv1alpha1.FlashSystemCluster) error {
+func (r *FlashSystemClusterReconciler) ensurePoolsConfigMap(instance *odfv1alpha1.FlashSystemCluster, newOwnerDetails v1.OwnerReference) error {
 	configmap, err := util.GetCreateConfigmap(r.Client, r.Log, watchNamespace, true, util.PoolsCmName)
 	if err != nil {
 		return err
 	}
 
+	if !IsOwnerExist(configmap.GetOwnerReferences(), newOwnerDetails) {
+		configmap.SetOwnerReferences(append(configmap.GetOwnerReferences(), newOwnerDetails))
+	}
+
 	if configmap.Data == nil {
 		configmap.Data = make(map[string]string)
 	}
@@ -950,3 +947,26 @@ func (r *FlashSystemClusterReconciler) ensureFlashSystemCSICR(instance *odfv1alp
 	r.IsCSICRCreated = true
 	return nil
 }
+
+func (r *FlashSystemClusterReconciler) getCmOwnerDetails(namespace string) (v1.OwnerReference, error) {
+	newOwnerDetails := v1.OwnerReference{}
+	operatorPodName, err := util.GetOperatorPodName()
+	if err != nil {
+		return newOwnerDetails, err
+	}
+	operatorPod := &corev1.Pod{}
+	if err := r.Client.Get(
+		context.TODO(),
+		types.NamespacedName{Name: operatorPodName, Namespace: namespace},
+		operatorPod); err != nil {
+		return newOwnerDetails, err
+	}
+
+	newOwnerDetails = v1.OwnerReference{
+		Name:       operatorPod.Name,
+		Kind:       operatorPod.Kind,
+		APIVersion: operatorPod.APIVersion,
+		UID:        operatorPod.UID,
+	}
+	return newOwnerDetails, nil
+}
diff --git a/controllers/flashsystemcluster_controller_test.go b/controllers/flashsystemcluster_controller_test.go
@@ -41,6 +41,7 @@ var _ = Describe("FlashSystemClusterReconciler", func() {
 		FlashSystemName  = "flashsystemcluster-sample"
 		namespace        = "openshift-storage"
 		secretName       = "fs-secret-sample"
+		operatorPodName  = "ibm-storage-odf-operator"
 		storageClassName = "odf-flashsystemcluster-sample"
 		poolName         = "Pool0"
 		fsType           = "ext4"
@@ -75,6 +76,39 @@ var _ = Describe("FlashSystemClusterReconciler", func() {
 			}, timeout, interval).Should(BeTrue())
 
 		})
+		It("should create operator pod successfully", func() {
+			By("By creating a new pod")
+			ctx := context.TODO()
+
+			pod := &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      operatorPodName,
+					Namespace: namespace,
+				},
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Name:  operatorPodName,
+							Image: "nginx",
+						},
+					},
+				},
+			}
+
+			Expect(k8sClient.Create(ctx, pod)).Should(Succeed())
+
+			By("By querying the created pod")
+			podLookupKey := types.NamespacedName{
+				Name:      operatorPodName,
+				Namespace: namespace,
+			}
+			createdPod := &corev1.Pod{}
+
+			Eventually(func() bool {
+				err := k8sClient.Get(ctx, podLookupKey, createdPod)
+				return err == nil
+			}, timeout, interval).Should(BeTrue())
+		})
 
 		It("should create secret successfully", func() {
 			By("By creating a new secret")

diff --git a/controllers/persistentvolume/suite_test.go b/controllers/persistentvolume/suite_test.go
@@ -103,6 +103,8 @@ var _ = BeforeSuite(func() {
 	Expect(err).NotTo(HaveOccurred())
 	err = os.Setenv(util.ExporterImageEnvVar, "docker.io/ibmcom/ibm-storage-odf-block-driver:v0.0.22")
 	Expect(err).NotTo(HaveOccurred())
+	err = os.Setenv(util.OperatorPodNameEnvVar, "ibm-storage-odf-operator")
+	Expect(err).NotTo(HaveOccurred())
 
 	// create manager and register controller for tset
 	ns, err := util.GetWatchNamespace()

diff --git a/controllers/storageclass/suite_test.go b/controllers/storageclass/suite_test.go
@@ -104,6 +104,8 @@ var _ = BeforeSuite(func() {
 	Expect(err).NotTo(HaveOccurred())
 	err = os.Setenv(util.ExporterImageEnvVar, "docker.io/ibmcom/ibm-storage-odf-block-driver:v0.0.22")
 	Expect(err).NotTo(HaveOccurred())
+	err = os.Setenv(util.OperatorPodNameEnvVar, "ibm-storage-odf-operator")
+	Expect(err).NotTo(HaveOccurred())
 
 	// create manager and register controller for tset
 	ns, err := util.GetWatchNamespace()

diff --git a/controllers/suite_test.go b/controllers/suite_test.go
@@ -114,6 +114,8 @@ var _ = BeforeSuite(func() {
 	Expect(err).NotTo(HaveOccurred())
 	err = os.Setenv(util.ExporterImageEnvVar, "docker.io/ibmcom/ibm-storage-odf-block-driver:v0.0.22")
 	Expect(err).NotTo(HaveOccurred())
+	err = os.Setenv(util.OperatorPodNameEnvVar, "ibm-storage-odf-operator")
+	Expect(err).NotTo(HaveOccurred())
 
 	// create manager and register controller for test
 	ns, err := util.GetWatchNamespace()

diff --git a/controllers/util/k8sutil.go b/controllers/util/k8sutil.go
@@ -34,6 +34,7 @@ type Label struct {
 // this value is empty if the operator is running with clusterScope.
 const WatchNamespaceEnvVar = "RESOURCES_NAMESPACE"
 const ExporterImageEnvVar = "EXPORTER_IMAGE"
+const OperatorPodNameEnvVar = "OPERATOR_POD_NAME"
 
 const OdfFsStorageSystemLabelKey = "odf-fs-storage-system"
 
@@ -59,6 +60,15 @@ func GetExporterImage() (string, error) {
 	return image, nil
 }
 
+// GetOperatorPodName returns the operator pod name
+func GetOperatorPodName() (string, error) {
+	podName, found := os.LookupEnv(OperatorPodNameEnvVar)
+	if !found {
+		return "", fmt.Errorf("%s must be set", OperatorPodNameEnvVar)
+	}
+	return podName, nil
+}
+
 // GetLabels returns the labels with cluster name
 func GetLabels() map[string]string {
 	return map[string]string{

diff --git a/controllers/util/poolutil.go b/controllers/util/poolutil.go
@@ -309,6 +309,22 @@ func initConfigMap(ns string, configMapName string) *corev1.ConfigMap {
 	return configMap
 }
 
+func DeleteConfigMap(log logr.Logger, client client.Client, configMapName string, ns string) error {
+	foundPoolsCm, err := GetCreateConfigmap(client, log, ns, false, configMapName)
+	if err != nil {
+		if errors.IsNotFound(err) {
+			log.Info("ConfigMap is already deleted", "ConfigMap", configMapName)
+		} else {
+			return err // Error reading the object - requeue the request.
+		}
+	} else {
+		if err := client.Delete(context.TODO(), foundPoolsCm); err != nil {
+			log.Error(err, "failed to delete ConfigMap", "ConfigMap", configMapName)
+			return err
+		}
+	}
+	return nil
+}
 func MapClustersByMgmtAddress(client client.Client, logger logr.Logger) (map[string]v1alpha1.FlashSystemCluster, error) {
 	clusters := &v1alpha1.FlashSystemClusterList{}
 	if err := client.List(context.Background(), clusters); err != nil {

diff --git a/start-operator.sh b/start-operator.sh
@@ -18,6 +18,7 @@
 
 export RESOURCES_NAMESPACE=openshift-storage
 export EXPORTER_IMAGE=docker.io/ibmcom/ibm-storage-odf-block-driver:v0.0.22
+export OPERATOR_POD_NAME=ibm-storage-odf-operator
 export TEST_FS_CR_FILEPATH="$(pwd)/config/samples/csi.ibm.com_v1_ibmblockcsi_cr.yaml"
 export TEST_FS_PROM_RULE_FILE="$(pwd)/rules/prometheus-flashsystem-rules.yaml"