Remove rabbitmqv1.RabbitMQUserCleanupBlockedFinalizer on cleanup

Author: lmiccini

internal/controller/dataplane/manage_service_finalizers.go

@@ -224,6 +224,10 @@ func (r *OpenStackDataPlaneNodeSetReconciler) manageServiceFinalizers(
 		isCurrentlyInUse := currentUsernames[rabbitmqUser.Name] || currentUsernames[rabbitmqUser.Status.Username]
 
 		hasFinalizer := slices.Contains(rabbitmqUser.Finalizers, finalizerName)
+		hasCleanupBlockedFinalizer := slices.Contains(rabbitmqUser.Finalizers, rabbitmqv1.RabbitMQUserCleanupBlockedFinalizer)
+
+		needsUpdate := false
+		newFinalizers := make([]string, 0, len(rabbitmqUser.Finalizers))
 
 		if isCurrentlyInUse && !hasFinalizer {
 			// Add finalizer to this RabbitMqUser (this is the current user)
@@ -232,14 +236,9 @@ func (r *OpenStackDataPlaneNodeSetReconciler) manageServiceFinalizers(
 				"service", serviceName,
 				"user", rabbitmqUser.Name,
 				"finalizer", finalizerName)
-			rabbitmqUser.Finalizers = append(rabbitmqUser.Finalizers, finalizerName)
-			err = r.Update(ctx, rabbitmqUser)
-			if err != nil {
-				Log.Error(err, "Failed to add finalizer to RabbitMQUser",
-					"service", serviceName,
-					"user", rabbitmqUser.Name)
-				// Don't fail reconciliation, just log the error
-			}
+			newFinalizers = append(newFinalizers, rabbitmqUser.Finalizers...)
+			newFinalizers = append(newFinalizers, finalizerName)
+			needsUpdate = true
 		} else if !isCurrentlyInUse && hasFinalizer && shouldRemoveOldFinalizers {
 			// Remove finalizer from this RabbitMqUser (no longer in use)
 			// Safe to remove because we only reach here when:
@@ -252,20 +251,12 @@ func (r *OpenStackDataPlaneNodeSetReconciler) manageServiceFinalizers(
 				"service", serviceName,
 				"user", rabbitmqUser.Name,
 				"finalizer", finalizerName)
-			var newFinalizers []string
 			for _, f := range rabbitmqUser.Finalizers {
 				if f != finalizerName {
 					newFinalizers = append(newFinalizers, f)
 				}
 			}
-			rabbitmqUser.Finalizers = newFinalizers
-			err = r.Update(ctx, rabbitmqUser)
-			if err != nil {
-				Log.Error(err, "Failed to remove finalizer from RabbitMQUser",
-					"service", serviceName,
-					"user", rabbitmqUser.Name)
-				// Don't fail reconciliation, just log the error
-			}
+			needsUpdate = true
 		} else if !isCurrentlyInUse && hasFinalizer && !shouldRemoveOldFinalizers {
 			// This user is no longer in use but we can't remove the finalizer yet
 			// because not all nodes/nodesets have been updated
@@ -275,6 +266,39 @@ func (r *OpenStackDataPlaneNodeSetReconciler) manageServiceFinalizers(
 				"finalizer", finalizerName,
 				"updatedNodes", len(tracking.UpdatedNodes),
 				"totalNodes", len(allNodeNames))
+			newFinalizers = append(newFinalizers, rabbitmqUser.Finalizers...)
+		} else {
+			// No change to nodeset finalizers
+			newFinalizers = append(newFinalizers, rabbitmqUser.Finalizers...)
+		}
+
+		// Remove the temporary cleanup-blocked finalizer if present
+		// This finalizer was added by infra-operator as a temporary safeguard during
+		// credential rotation, but should be removed now that proper finalizer management is in place
+		if hasCleanupBlockedFinalizer {
+			Log.Info("Removing temporary cleanup-blocked finalizer from RabbitMqUser",
+				"user", rabbitmqUser.Name,
+				"finalizer", rabbitmqv1.RabbitMQUserCleanupBlockedFinalizer)
+			filteredFinalizers := make([]string, 0, len(newFinalizers))
+			for _, f := range newFinalizers {
+				if f != rabbitmqv1.RabbitMQUserCleanupBlockedFinalizer {
+					filteredFinalizers = append(filteredFinalizers, f)
+				}
+			}
+			newFinalizers = filteredFinalizers
+			needsUpdate = true
+		}
+
+		// Apply all finalizer changes in a single update
+		if needsUpdate {
+			rabbitmqUser.Finalizers = newFinalizers
+			err = r.Update(ctx, rabbitmqUser)
+			if err != nil {
+				Log.Error(err, "Failed to update finalizers on RabbitMQUser",
+					"service", serviceName,
+					"user", rabbitmqUser.Name)
+				// Don't fail reconciliation, just log the error
+			}
 		}
 	}
 }

internal/controller/dataplane/openstackdataplanenodeset_controller.go

@@ -51,6 +51,7 @@ import (
 
 	"github.com/go-logr/logr"
 	infranetworkv1 "github.com/openstack-k8s-operators/infra-operator/apis/network/v1beta1"
+	rabbitmqv1 "github.com/openstack-k8s-operators/infra-operator/apis/rabbitmq/v1beta1"
 	condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/rolebinding"
@@ -1095,7 +1096,7 @@ func (r *OpenStackDataPlaneNodeSetReconciler) isNodesetFullyUpdated(
 	helper *helper.Helper,
 	nodeset *dataplanev1.OpenStackDataPlaneNodeSet,
 	serviceName string,
-	secretsLastModified map[string]time.Time,
+	_ map[string]time.Time,
 ) (bool, error) {
 	Log := r.GetLogger(ctx)
 
@@ -1269,13 +1270,18 @@ func (r *OpenStackDataPlaneNodeSetReconciler) reconcileDelete(
 		updatedFinalizers := []string{}
 
 		// Keep only finalizers that don't belong to this nodeset
+		// Also remove the temporary cleanup-blocked finalizer if present
 		for _, f := range originalFinalizers {
-			if !strings.HasPrefix(f, finalizerPrefix) {
-				updatedFinalizers = append(updatedFinalizers, f)
-			} else {
+			if strings.HasPrefix(f, finalizerPrefix) {
 				Log.Info("Removing finalizer from RabbitMQ user during nodeset deletion",
 					"user", user.GetName(),
 					"finalizer", f)
+			} else if f == rabbitmqv1.RabbitMQUserCleanupBlockedFinalizer {
+				Log.Info("Removing temporary cleanup-blocked finalizer from RabbitMQ user during nodeset deletion",
+					"user", user.GetName(),
+					"finalizer", f)
+			} else {
+				updatedFinalizers = append(updatedFinalizers, f)
 			}
 		}
 

internal/dataplane/rabbitmq_test.go

@@ -18,7 +18,6 @@ package deployment
 
 import (
 	"context"
-	"fmt"
 	"testing"
 
 	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
@@ -432,11 +431,11 @@ func TestGetNovaCellRabbitMqUserFromSecret_EdgeCases(t *testing.T) {
 				Namespace: namespace,
 			},
 			Data: map[string][]byte{
-				"01-nova.conf": []byte(fmt.Sprintf(`[DEFAULT]
+				"01-nova.conf": []byte(`[DEFAULT]
 debug = true
 transport_url = rabbit://complex-user:p@ssw0rd@host1:5672,host2:5672,host3:5672/cell1
 log_dir = /var/log/nova
-`)),
+`),
 			},
 		}