Skip to content

Commit

Permalink
remove redundant finalizers in user functionality
Browse files Browse the repository at this point in the history
  • Loading branch information
ribaraka committed Aug 23, 2023
1 parent 1fd8cfd commit 462c584
Show file tree
Hide file tree
Showing 13 changed files with 111 additions and 274 deletions.
4 changes: 0 additions & 4 deletions apis/clusterresources/v1beta1/redisuser_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 0 additions & 4 deletions apis/kafkamanagement/v1beta1/kafkauser_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
39 changes: 2 additions & 37 deletions controllers/clusterresources/cassandrauser_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand All @@ -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)
Expand All @@ -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, msgDeleteUser)
r.EventRecorder.Event(u, models.Warning, models.DeletingEvent, msgDeleteUser)

return models.ExitReconcile, nil
}
Expand Down
4 changes: 1 addition & 3 deletions controllers/clusterresources/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,4 @@ 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
}
const msgDeleteUser = "If you want to delete the user, first remove the user from cluster specification"
51 changes: 2 additions & 49 deletions controllers/clusterresources/opensearchuser_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, msgDeleteUser)
r.EventRecorder.Event(user, models.Warning, models.DeletingEvent, msgDeleteUser)

return models.ExitReconcile, nil
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down
98 changes: 42 additions & 56 deletions controllers/clusterresources/redisuser_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,30 @@ 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
}
}

username, password, err := getUserCreds(secret)
if err != nil {
l.Error(err, "Cannot get user credentials", "user", username)
Expand All @@ -109,8 +133,6 @@ func (r *RedisUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (

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",
Expand All @@ -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)
Expand All @@ -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)

Expand All @@ -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
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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",
Expand All @@ -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(
Expand All @@ -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, 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, msgDeleteUser)

return models.ErrUserStillExist
}
Expand All @@ -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
}
Expand Down
2 changes: 2 additions & 0 deletions controllers/clusters/cassandra_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
6 changes: 6 additions & 0 deletions controllers/clusters/kafka_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,8 @@ func (r *KafkaReconciler) handleCreateUser(
return err
}

l.Info("User has been added to the queue for creation", "username", u.Name)

return nil
}

Expand Down Expand Up @@ -390,6 +392,8 @@ func (r *KafkaReconciler) handleDeleteUser(
return err
}

l.Info("User has been added to the queue for deletion", "username", u.Name)

return nil
}

Expand Down Expand Up @@ -439,6 +443,8 @@ func (r *KafkaReconciler) detachUser(
return err
}

l.Info("User has been added to the queue for detaching", "username", u.Name)

return nil
}

Expand Down
Loading

0 comments on commit 462c584

Please sign in to comment.