remove redundant finalizers in user functionality

instaclustr · Aug 28, 2023 · 3c1de94 · 3c1de94
1 parent f0ee5c1
commit 3c1de94
Show file tree

Hide file tree

Showing 14 changed files with 116 additions and 285 deletions.
diff --git a/apis/clusterresources/v1beta1/redisuser_types.go b/apis/clusterresources/v1beta1/redisuser_types.go
@@ -69,10 +69,6 @@ func (r *RedisUser) GetDeletionFinalizer() string {
 	return models.DeletionFinalizer + "_" + r.Namespace + "_" + r.Name
 }
 
-func (r *RedisUser) DeletionUserFinalizer(clusterID, namespace string) string {
-	return models.DeletionUserFinalizer + clusterID + "_" + namespace
-}
-
 func (r *RedisUser) ToInstAPIUpdate(password, id string) *models.RedisUserUpdate {
 	return &models.RedisUserUpdate{
 		ID:       id,

diff --git a/apis/kafkamanagement/v1beta1/kafkauser_types.go b/apis/kafkamanagement/v1beta1/kafkauser_types.go
@@ -75,10 +75,6 @@ func (ku *KafkaUser) GetDeletionFinalizer() string {
 	return models.DeletionFinalizer + "_" + ku.Namespace + "_" + ku.Name
 }
 
-func (ku *KafkaUser) GetDeletionUserFinalizer(clusterID string) string {
-	return models.DeletionUserFinalizer + clusterID
-}
-
 func (ku *KafkaUser) GetID(clusterID, name string) string {
 	return clusterID + "_" + name
 }

diff --git a/controllers/clusterresources/cassandrauser_controller.go b/controllers/clusterresources/cassandrauser_controller.go
@@ -88,6 +88,17 @@ func (r *CassandraUserReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		return models.ReconcileRequeue, nil
 	}
 
+	username, password, err := getUserCreds(s)
+	if err != nil {
+		l.Error(err, "Cannot get the Cassandra user credentials from the secret",
+			"secret name", s.Name,
+			"secret namespace", s.Namespace)
+		r.EventRecorder.Eventf(u, models.Warning, models.CreatingEvent,
+			"Cannot get the Cassandra user credentials from the secret. Reason: %v", err)
+
+		return models.ReconcileRequeue, nil
+	}
+
 	if controllerutil.AddFinalizer(s, u.GetDeletionFinalizer()) {
 		err = r.Update(ctx, s)
 		if err != nil {
@@ -110,17 +121,6 @@ func (r *CassandraUserReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		}
 	}
 
-	username, password, err := getUserCreds(s)
-	if err != nil {
-		l.Error(err, "Cannot get the Cassandra user credentials from the secret",
-			"secret name", s.Name,
-			"secret namespace", s.Namespace)
-		r.EventRecorder.Eventf(u, models.Warning, models.CreatingEvent,
-			"Cannot get the Cassandra user credentials from the secret. Reason: %v", err)
-
-		return models.ReconcileRequeue, nil
-	}
-
 	for clusterID, event := range u.Status.ClustersEvents {
 		if event == models.CreatingEvent {
 			l.Info("Creating user", "user", u, "cluster ID", clusterID)
@@ -153,18 +153,6 @@ func (r *CassandraUserReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 				"User has been created for a cluster. Cluster ID: %s, username: %s",
 				clusterID, username)
 
-			controllerutil.AddFinalizer(u, getDeletionUserFinalizer(clusterID))
-			err = r.Patch(ctx, u, patch)
-			if err != nil {
-				l.Error(err, "Cannot patch Cassandra user resource",
-					"secret name", s.Name,
-					"secret namespace", s.Namespace)
-				r.EventRecorder.Eventf(u, models.Warning, models.PatchFailed,
-					"Resource patch is failed. Reason: %v", err)
-
-				return models.ReconcileRequeue, nil
-			}
-
 			continue
 		}
 
@@ -197,18 +185,6 @@ func (r *CassandraUserReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 				return models.ReconcileRequeue, nil
 			}
 
-			controllerutil.RemoveFinalizer(u, getDeletionUserFinalizer(clusterID))
-			err = r.Patch(ctx, u, patch)
-			if err != nil {
-				l.Error(err, "Cannot patch Cassandra user resource",
-					"secret name", s.Name,
-					"secret namespace", s.Namespace)
-				r.EventRecorder.Eventf(u, models.Warning, models.PatchFailed,
-					"Resource patch is failed. Reason: %v", err)
-
-				return models.ReconcileRequeue, nil
-			}
-
 			continue
 		}
 
@@ -223,16 +199,6 @@ func (r *CassandraUserReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 				return models.ReconcileRequeue, nil
 			}
 
-			controllerutil.RemoveFinalizer(u, getDeletionUserFinalizer(clusterID))
-			err = r.Patch(ctx, u, patch)
-			if err != nil {
-				l.Error(err, "Cannot delete finalizer from the Cassandra user resource",
-					"cluster ID", clusterID)
-				r.EventRecorder.Eventf(u, models.Warning, models.PatchFailed,
-					"Deleting finalizer from the Cassandra user resource has been failed. Reason: %v", err)
-				return models.ReconcileRequeue, nil
-			}
-
 			l.Info("Cassandra user has been detached from the cluster", "cluster ID", clusterID)
 			r.EventRecorder.Eventf(u, models.Normal, models.Deleted,
 				"User has been detached from the cluster. ClusterID: %v", clusterID)
@@ -241,9 +207,8 @@ func (r *CassandraUserReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 
 	if u.DeletionTimestamp != nil {
 		if u.Status.ClustersEvents != nil {
-			l.Error(models.ErrUserStillExist, "Please remove the user from the cluster specification")
-			r.EventRecorder.Event(u, models.Warning, models.DeletingEvent,
-				"The user is still attached to cluster, please remove the user from the cluster specification.")
+			l.Error(models.ErrUserStillExist, instaclustr.MsgDeleteUser)
+			r.EventRecorder.Event(u, models.Warning, models.DeletingEvent, instaclustr.MsgDeleteUser)
 
 			return models.ExitReconcile, nil
 		}

diff --git a/controllers/clusterresources/helpers.go b/controllers/clusterresources/helpers.go
@@ -75,7 +75,3 @@ func getUserCreds(secret *k8sCore.Secret) (username, password string, err error)
 
 	return username[:len(username)-1], password[:len(password)-1], nil
 }
-
-func getDeletionUserFinalizer(clusterID string) string {
-	return models.DeletionUserFinalizer + clusterID
-}
diff --git a/controllers/clusterresources/opensearchuser_controller.go b/controllers/clusterresources/opensearchuser_controller.go
@@ -152,10 +152,8 @@ func (r *OpenSearchUserReconciler) Reconcile(
 
 	if user.DeletionTimestamp != nil {
 		if user.Status.ClustersEvents != nil {
-			logger.Error(models.ErrUserStillExist, "please remove the user from the cluster specification")
-			r.EventRecorder.Event(user, models.Warning, models.DeletingEvent,
-				"The user is still attached to cluster, please remove the user from the cluster specification.",
-			)
+			logger.Error(models.ErrUserStillExist, instaclustr.MsgDeleteUser)
+			r.EventRecorder.Event(user, models.Warning, models.DeletingEvent, instaclustr.MsgDeleteUser)
 
 			return models.ExitReconcile, nil
 		}
@@ -221,22 +219,7 @@ func (r *OpenSearchUserReconciler) createUser(
 	}
 
 	patch := user.NewPatch()
-	if controllerutil.AddFinalizer(user, getDeletionUserFinalizer(clusterID)) {
-		err = r.Patch(ctx, user, patch)
-		if err != nil {
-			logger.Error(err, "Cannot patch OpenSearch user resource with deletion finalizer",
-				"cluster ID", clusterID,
-			)
-			r.EventRecorder.Eventf(
-				user, models.Warning, models.PatchFailed,
-				"Resource patching with deletion finalizer has been failed ). Reason: %v",
-				err,
-			)
-			return err
-		}
-	}
 
-	patch = user.NewPatch()
 	user.Status.ClustersEvents[clusterID] = models.Created
 	err = r.Status().Patch(ctx, user, patch)
 	if err != nil {
@@ -311,21 +294,6 @@ func (r *OpenSearchUserReconciler) deleteUser(
 		return err
 	}
 
-	patch = user.NewPatch()
-	controllerutil.RemoveFinalizer(user, getDeletionUserFinalizer(clusterID))
-	err = r.Patch(ctx, user, patch)
-	if err != nil {
-		logger.Error(err, "Cannot delete finalizer from the OpenSearch user resource",
-			"cluster ID", clusterID,
-		)
-		r.EventRecorder.Eventf(
-			user, models.Warning, models.PatchFailed,
-			"Deleting finalizer from the OpenSearch user resource has been failed. Reason: %v",
-			err,
-		)
-		return err
-	}
-
 	logger.Info("OpenSearch user has been deleted from the cluster",
 		"cluster ID", clusterID,
 	)
@@ -358,21 +326,6 @@ func (r *OpenSearchUserReconciler) detachUserFromDeletedCluster(
 		return err
 	}
 
-	patch = user.NewPatch()
-	controllerutil.RemoveFinalizer(user, getDeletionUserFinalizer(clusterID))
-	err = r.Patch(ctx, user, patch)
-	if err != nil {
-		logger.Error(err, "Cannot delete finalizer from the OpenSearch user resource",
-			"cluster ID", clusterID,
-		)
-		r.EventRecorder.Eventf(
-			user, models.Warning, models.PatchFailed,
-			"Deleting finalizer from the OpenSearch user resource has been failed. Reason: %v",
-			err,
-		)
-		return err
-	}
-
 	logger.Info("OpenSearch user has been deleted from the cluster",
 		"cluster ID", clusterID,
 	)

diff --git a/controllers/clusterresources/redisuser_controller.go b/controllers/clusterresources/redisuser_controller.go
@@ -107,10 +107,32 @@ func (r *RedisUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		return models.ReconcileRequeue, nil
 	}
 
+	if controllerutil.AddFinalizer(secret, user.GetDeletionFinalizer()) {
+		err = r.Update(ctx, secret)
+		if err != nil {
+			l.Error(err, "Cannot update Redis user secret with deletion finalizer",
+				"secret name", secret.Name,
+				"secret namespace", secret.Namespace)
+			r.EventRecorder.Eventf(user, models.Warning, models.UpdatedEvent,
+				"Cannot assign k8s secret to a Redis user. Reason: %v", err)
+
+			return models.ReconcileRequeue, nil
+		}
+	}
+
+	patch := user.NewPatch()
+	if controllerutil.AddFinalizer(user, user.GetDeletionFinalizer()) {
+		err = r.Patch(ctx, user, patch)
+		if err != nil {
+			l.Error(err, "Patch is failed. Cannot set finalizer to the user")
+			r.EventRecorder.Eventf(user, models.Warning, models.PatchFailed,
+				"Patch is failed. Cannot set finalizer to the user. Reason: %v", err)
+			return models.ReconcileRequeue, nil
+		}
+	}
+
 	for clusterID, event := range user.Status.ClustersEvents {
 		if event == models.CreatingEvent {
-			patch := user.NewPatch()
-
 			_, err = r.API.CreateRedisUser(user.Spec.ToInstAPI(password, clusterID, username))
 			if err != nil {
 				l.Error(err, "Cannot create a user for the Redis cluster",
@@ -122,9 +144,7 @@ func (r *RedisUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 				return models.ReconcileRequeue, nil
 			}
 
-			event = models.Created
-			user.Status.ClustersEvents[clusterID] = event
-
+			user.Status.ClustersEvents[clusterID] = models.Created
 			err = r.Status().Patch(ctx, user, patch)
 			if err != nil {
 				l.Error(err, "Cannot patch Redis user status", "user", user.Name)
@@ -139,42 +159,15 @@ func (r *RedisUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 				"User has been created for a Redis cluster. Cluster ID: %s, username: %s",
 				clusterID, username)
 
-			finalizerNeeded := controllerutil.AddFinalizer(secret, user.GetDeletionFinalizer())
-			if finalizerNeeded {
-				err = r.Update(ctx, secret)
-				if err != nil {
-					l.Error(err, "Cannot update Redis user secret with deletion finalizer",
-						"secret name", secret.Name,
-						"secret namespace", secret.Namespace)
-					r.EventRecorder.Eventf(user, models.Warning, models.UpdatedEvent,
-						"Cannot assign k8s secret to a Redis user. Reason: %v", err)
-
-					return models.ReconcileRequeue, nil
-				}
-			}
-
-			controllerutil.AddFinalizer(user, user.DeletionUserFinalizer(clusterID, user.Namespace))
-			err = r.Patch(ctx, user, patch)
-			if err != nil {
-				l.Error(err, "Cannot patch Redis user resource with deletion finalizer",
-					"secret name", secret.Name,
-					"secret namespace", secret.Namespace)
-				r.EventRecorder.Eventf(user, models.Warning, models.PatchFailed,
-					"Resource patch with deletion user finalizer is failed. Reason: %v", err)
-
-				return models.ReconcileRequeue, nil
-			}
-
 			continue
 		}
 
 		if event == models.DeletingEvent {
-			patch := user.NewPatch()
-
-			userID := fmt.Sprintf(instaclustr.RedisUserIDFmt, clusterID, user.Namespace)
+			userID := fmt.Sprintf(instaclustr.RedisUserIDFmt, clusterID, username)
 			err = r.API.DeleteRedisUser(userID)
 			if err != nil {
-				l.Error(err, "Cannot delete Redis user from the cluster. Cluster ID: %s.", "user", clusterID, user.Name)
+				l.Error(err, "Cannot delete Redis user from the cluster.",
+					"cluster ID", clusterID, "user ID", userID)
 				r.EventRecorder.Eventf(user, models.Warning, models.DeletingEvent,
 					"Cannot delete Redis user from the cluster. Cluster ID: %s. Reason: %v", clusterID, err)
 
@@ -198,18 +191,6 @@ func (r *RedisUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 				return models.ReconcileRequeue, nil
 			}
 
-			controllerutil.RemoveFinalizer(user, user.DeletionUserFinalizer(clusterID, user.Namespace))
-			err = r.Patch(ctx, user, patch)
-			if err != nil {
-				l.Error(err, "Cannot patch Redis user resource",
-					"secret name", secret.Name,
-					"secret namespace", secret.Namespace)
-				r.EventRecorder.Eventf(user, models.Warning, models.PatchFailed,
-					"Resource patch is failed. Reason: %v", err)
-
-				return models.ReconcileRequeue, nil
-			}
-
 			continue
 		}
 
@@ -253,8 +234,6 @@ func (r *RedisUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 			"cluster ID", clusterID,
 			"user ID", userID,
 		)
-
-		return models.ExitReconcile, nil
 	}
 
 	if user.DeletionTimestamp != nil {
@@ -290,8 +269,6 @@ func (r *RedisUserReconciler) detachUserFromDeletedCluster(
 		return err
 	}
 
-	controllerutil.RemoveFinalizer(user, user.DeletionUserFinalizer(clusterID, user.Namespace))
-
 	err = r.Patch(ctx, user, patch)
 	if err != nil {
 		logger.Error(err, "Cannot delete finalizer from the Redis user resource",
@@ -313,7 +290,7 @@ func (r *RedisUserReconciler) detachUserFromDeletedCluster(
 		"Redis user resource has been deleted from the cluster (clusterID: %v)", clusterID,
 	)
 
-	return err
+	return nil
 }
 
 func (r *RedisUserReconciler) handleDeleteUser(
@@ -333,10 +310,9 @@ func (r *RedisUserReconciler) handleDeleteUser(
 
 	for clusterID, event := range u.Status.ClustersEvents {
 		if event == models.Created || event == models.CreatingEvent {
-			l.Error(models.ErrUserStillExist, "please remove the user from the cluster specification",
+			l.Error(models.ErrUserStillExist, instaclustr.MsgDeleteUser,
 				"username", username, "cluster ID", clusterID)
-			r.EventRecorder.Event(u, models.Warning, models.DeletingEvent,
-				"The user is still attached to cluster, please remove the user from the cluster specification.")
+			r.EventRecorder.Event(u, models.Warning, models.DeletingEvent, instaclustr.MsgDeleteUser)
 
 			return models.ErrUserStillExist
 		}
@@ -346,9 +322,19 @@ func (r *RedisUserReconciler) handleDeleteUser(
 	err = r.Update(ctx, s)
 	if err != nil {
 		l.Error(err, "Cannot remove finalizer from secret", "secret name", s.Name)
+		r.EventRecorder.Eventf(u, models.Warning, models.PatchFailed,
+			"Patch is failed. Cannot remove finalizer from secret. Reason: %v", err)
 
+		return err
+	}
+
+	patch := u.NewPatch()
+	controllerutil.RemoveFinalizer(u, u.GetDeletionFinalizer())
+	err = r.Patch(ctx, u, patch)
+	if err != nil {
+		l.Error(err, "Cannot remove finalizer from user", "user finalizer", u.Finalizers)
 		r.EventRecorder.Eventf(u, models.Warning, models.PatchFailed,
-			"Resource patch is failed. Reason: %v", err)
+			"Patch is failed. Cannot remove finalizer from user. Reason: %v", err)
 
 		return err
 	}

diff --git a/controllers/clusters/cassandra_controller.go b/controllers/clusters/cassandra_controller.go
@@ -772,6 +772,8 @@ func (r *CassandraReconciler) handleUsersDetach(
 		return err
 	}
 
+	l.Info("User has been added to the queue for detaching", "username", u.Name)
+
 	return nil
 }