Remove rabbitmqv1.RabbitMQUserCleanupBlockedFinalizer on cleanup

Author: lmiccini

internal/controller/dataplane/manage_service_finalizers.go

@@ -276,5 +276,27 @@ func (r *OpenStackDataPlaneNodeSetReconciler) manageServiceFinalizers(
 				"updatedNodes", len(tracking.UpdatedNodes),
 				"totalNodes", len(allNodeNames))
 		}
+
+		// 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 slices.Contains(rabbitmqUser.Finalizers, rabbitmqv1.RabbitMQUserCleanupBlockedFinalizer) {
+			Log.Info("Removing temporary cleanup-blocked finalizer from RabbitMqUser",
+				"user", rabbitmqUser.Name,
+				"finalizer", rabbitmqv1.RabbitMQUserCleanupBlockedFinalizer)
+			var newFinalizers []string
+			for _, f := range rabbitmqUser.Finalizers {
+				if f != rabbitmqv1.RabbitMQUserCleanupBlockedFinalizer {
+					newFinalizers = append(newFinalizers, f)
+				}
+			}
+			rabbitmqUser.Finalizers = newFinalizers
+			err = r.Update(ctx, rabbitmqUser)
+			if err != nil {
+				Log.Error(err, "Failed to remove cleanup-blocked finalizer from RabbitMQUser",
+					"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
-`)),
+`),
 			},
 		}
 

test/functional/dataplane/openstackdataplanenodeset_rabbitmq_finalizer_test.go

@@ -136,7 +136,7 @@ var _ = Describe("Dataplane NodeSet RabbitMQ Finalizer Management", func() {
 	}
 
 	// Helper to simulate deployment completion
-	SimulateDeploymentComplete := func(deploymentName types.NamespacedName, nodesetName string, ansibleLimit []string) {
+	SimulateDeploymentComplete := func(deploymentName types.NamespacedName, nodesetName string, _ []string) {
 		// First, complete the AnsibleEE jobs for each service
 		deployment := GetDataplaneDeployment(deploymentName)
 		nodeset := GetDataplaneNodeSet(types.NamespacedName{
@@ -273,7 +273,7 @@ var _ = Describe("Dataplane NodeSet RabbitMQ Finalizer Management", func() {
 		})
 
 		AfterEach(func() {
-			k8sClient.Delete(ctx, novaUser)
+			_ = k8sClient.Delete(ctx, novaUser)
 		})
 
 		It("Should add finalizer only after ALL nodes are updated in rolling deployment", func() {
@@ -444,7 +444,7 @@ var _ = Describe("Dataplane NodeSet RabbitMQ Finalizer Management", func() {
 		})
 
 		AfterEach(func() {
-			k8sClient.Delete(ctx, novaUser)
+			_ = k8sClient.Delete(ctx, novaUser)
 		})
 
 		It("Should add independent finalizers from each nodeset to shared user", func() {
@@ -618,8 +618,8 @@ var _ = Describe("Dataplane NodeSet RabbitMQ Finalizer Management", func() {
 		})
 
 		AfterEach(func() {
-			k8sClient.Delete(ctx, oldUser)
-			k8sClient.Delete(ctx, newUser)
+			_ = k8sClient.Delete(ctx, oldUser)
+			_ = k8sClient.Delete(ctx, newUser)
 		})
 
 		It("Should switch finalizer from old user to new user after rotation completes", func() {
@@ -792,9 +792,9 @@ var _ = Describe("Dataplane NodeSet RabbitMQ Finalizer Management", func() {
 		})
 
 		AfterEach(func() {
-			k8sClient.Delete(ctx, novaUser)
-			k8sClient.Delete(ctx, neutronUser)
-			k8sClient.Delete(ctx, ironicUser)
+			_ = k8sClient.Delete(ctx, novaUser)
+			_ = k8sClient.Delete(ctx, neutronUser)
+			_ = k8sClient.Delete(ctx, ironicUser)
 		})
 
 		It("Should manage service-specific finalizers independently across different clusters", func() {
@@ -945,7 +945,7 @@ var _ = Describe("Dataplane NodeSet RabbitMQ Finalizer Management", func() {
 		})
 
 		AfterEach(func() {
-			k8sClient.Delete(ctx, novaUser)
+			_ = k8sClient.Delete(ctx, novaUser)
 		})
 
 		It("Should use deployment completion time not creation time", func() {
@@ -1002,7 +1002,7 @@ var _ = Describe("Dataplane NodeSet RabbitMQ Finalizer Management", func() {
 			// Rotate secret (secret modified time = now, AFTER deployment creation)
 			UpdateNovaCellConfigSecret("cell1", "nova-user2", "rabbitmq-cell1")
 			newUser := CreateRabbitMQUser("nova-user2")
-			DeferCleanup(func() { k8sClient.Delete(ctx, newUser) })
+			DeferCleanup(func() { _ = k8sClient.Delete(ctx, newUser) })
 
 			// Wait for secret to propagate
 			time.Sleep(time.Second * 2)
@@ -1120,7 +1120,7 @@ var _ = Describe("Dataplane NodeSet RabbitMQ Finalizer Management", func() {
 			// Change secret (this triggers credential rotation)
 			UpdateNovaCellConfigSecret("cell1", "nova-user2", "rabbitmq-cell1")
 			newUser := CreateRabbitMQUser("nova-user2")
-			DeferCleanup(func() { k8sClient.Delete(ctx, newUser) })
+			DeferCleanup(func() { _ = k8sClient.Delete(ctx, newUser) })
 
 			// Wait for secret to propagate
 			time.Sleep(time.Second * 2)
@@ -1237,7 +1237,7 @@ var _ = Describe("Dataplane NodeSet RabbitMQ Finalizer Management", func() {
 			// Change secret to user2
 			UpdateNovaCellConfigSecret("cell1", "nova-user2", "rabbitmq-cell1")
 			user2 := CreateRabbitMQUser("nova-user2")
-			DeferCleanup(func() { k8sClient.Delete(ctx, user2) })
+			DeferCleanup(func() { _ = k8sClient.Delete(ctx, user2) })
 
 			// Wait for secret to propagate
 			time.Sleep(time.Second * 2)
@@ -1425,8 +1425,162 @@ var _ = Describe("Dataplane NodeSet RabbitMQ Finalizer Management", func() {
 			Eventually(func(g Gomega) {
 				nodeset := GetDataplaneNodeSet(dataplaneNodeSetName)
 				// Should have exactly 2 nodes defined (not 4 with hostName and ansibleHost)
-				g.Expect(len(nodeset.Spec.Nodes)).Should(Equal(2))
+				g.Expect(nodeset.Spec.Nodes).Should(HaveLen(2))
 			}, th.Timeout, th.Interval).Should(Succeed())
 		})
 	})
+
+	Context("When RabbitMQ users have cleanup-blocked finalizer", func() {
+		var novaUser *rabbitmqv1.RabbitMQUser
+
+		BeforeEach(func() {
+			// Create Nova cell config secret
+			CreateNovaCellConfigSecret("cell1", "nova-user1", "rabbitmq-cell1")
+
+			// Create RabbitMQ user with cleanup-blocked finalizer
+			novaUser = CreateRabbitMQUser("nova-user1")
+
+			// Manually add the cleanup-blocked finalizer to simulate migration scenario
+			Eventually(func(g Gomega) {
+				user := GetRabbitMQUser("nova-user1")
+				user.Finalizers = append(user.Finalizers, rabbitmqv1.RabbitMQUserCleanupBlockedFinalizer)
+				g.Expect(k8sClient.Update(ctx, user)).Should(Succeed())
+			}, timeout, interval).Should(Succeed())
+
+			// Setup infrastructure
+			netConfigName := types.NamespacedName{Namespace: namespace, Name: "dataplane-netconfig-cleanup"}
+			DeferCleanup(th.DeleteInstance, CreateNetConfig(netConfigName, DefaultNetConfigSpec()))
+
+			dnsMasqName := types.NamespacedName{Namespace: namespace, Name: "dnsmasq-cleanup"}
+			DeferCleanup(th.DeleteInstance, CreateDNSMasq(dnsMasqName, DefaultDNSMasqSpec()))
+			SimulateDNSMasqComplete(dnsMasqName)
+
+			CreateSSHSecret(types.NamespacedName{
+				Namespace: namespace,
+				Name:      "nova-migration-ssh-key",
+			})
+		})
+
+		AfterEach(func() {
+			_ = k8sClient.Delete(ctx, novaUser)
+		})
+
+		It("Should remove cleanup-blocked finalizer during normal reconciliation", func() {
+			// Verify cleanup-blocked finalizer is present initially
+			Eventually(func(g Gomega) {
+				user := GetRabbitMQUser("nova-user1")
+				g.Expect(user.Finalizers).Should(ContainElement(rabbitmqv1.RabbitMQUserCleanupBlockedFinalizer))
+			}, timeout, interval).Should(Succeed())
+
+			// Create nodeset with Nova service
+			nodeSetSpec := map[string]interface{}{
+				"preProvisioned": true,
+				"services":       []string{"nova"},
+				"nodes": map[string]dataplanev1.NodeSection{
+					"compute-0": {
+						HostName: "compute-0",
+						Ansible: dataplanev1.AnsibleOpts{
+							AnsibleHost: "192.168.122.100",
+						},
+					},
+				},
+				"nodeTemplate": map[string]interface{}{
+					"ansibleSSHPrivateKeySecret": "dataplane-ansible-ssh-private-key-secret",
+					"managementNetwork":          "ctlplane",
+					"ansible": map[string]interface{}{
+						"ansibleUser": "cloud-admin",
+					},
+				},
+			}
+			dataplaneNodeSetName := types.NamespacedName{
+				Name:      "edpm-nodeset-cleanup-test",
+				Namespace: namespace,
+			}
+			DeferCleanup(th.DeleteInstance, CreateDataplaneNodeSet(dataplaneNodeSetName, nodeSetSpec))
+
+			CreateSSHSecret(types.NamespacedName{
+				Namespace: namespace,
+				Name:      "nova-migration-ssh-key",
+			})
+
+			SimulateIPSetComplete(types.NamespacedName{Namespace: namespace, Name: "compute-0"})
+			SimulateDNSDataComplete(dataplaneNodeSetName)
+
+			// Deploy the nodeset
+			deploymentName := types.NamespacedName{
+				Name:      "cleanup-deployment",
+				Namespace: namespace,
+			}
+			deploymentSpec := map[string]interface{}{
+				"nodeSets": []string{dataplaneNodeSetName.Name},
+			}
+			DeferCleanup(th.DeleteInstance, CreateDataplaneDeployment(deploymentName, deploymentSpec))
+			SimulateDeploymentComplete(deploymentName, dataplaneNodeSetName.Name, []string{"compute-0"})
+
+			// Verify the cleanup-blocked finalizer is removed
+			Eventually(func(g Gomega) {
+				user := GetRabbitMQUser("nova-user1")
+				g.Expect(user.Finalizers).ShouldNot(ContainElement(rabbitmqv1.RabbitMQUserCleanupBlockedFinalizer),
+					"cleanup-blocked finalizer should be automatically removed during reconciliation")
+			}, timeout, interval).Should(Succeed())
+
+			// Verify our nodeset finalizer is still added
+			Eventually(func(g Gomega) {
+				user := GetRabbitMQUser("nova-user1")
+				hasNodesetFinalizer := false
+				for _, f := range user.Finalizers {
+					if len(f) > 11 && f[:11] == "nodeset.os/" {
+						hasNodesetFinalizer = true
+						break
+					}
+				}
+				g.Expect(hasNodesetFinalizer).Should(BeTrue(),
+					"nodeset finalizer should still be present")
+			}, timeout, interval).Should(Succeed())
+		})
+
+		It("Should remove cleanup-blocked finalizer during nodeset deletion", func() {
+			// Create a simple nodeset
+			nodeSetSpec := map[string]interface{}{
+				"preProvisioned": true,
+				"services":       []string{"nova"},
+				"nodes": map[string]dataplanev1.NodeSection{
+					"compute-0": {
+						HostName: "compute-0",
+					},
+				},
+				"nodeTemplate": map[string]interface{}{
+					"ansibleSSHPrivateKeySecret": "dataplane-ansible-ssh-private-key-secret",
+				},
+			}
+			dataplaneNodeSetName := types.NamespacedName{
+				Name:      "edpm-nodeset-deletion-cleanup",
+				Namespace: namespace,
+			}
+			CreateDataplaneNodeSet(dataplaneNodeSetName, nodeSetSpec)
+
+			// Verify cleanup-blocked finalizer is present
+			Eventually(func(g Gomega) {
+				user := GetRabbitMQUser("nova-user1")
+				g.Expect(user.Finalizers).Should(ContainElement(rabbitmqv1.RabbitMQUserCleanupBlockedFinalizer))
+			}, timeout, interval).Should(Succeed())
+
+			// Delete the nodeset
+			nodeset := GetDataplaneNodeSet(dataplaneNodeSetName)
+			Expect(k8sClient.Delete(ctx, nodeset)).Should(Succeed())
+
+			// Wait for nodeset to be deleted
+			Eventually(func(g Gomega) {
+				err := k8sClient.Get(ctx, dataplaneNodeSetName, nodeset)
+				g.Expect(err).Should(HaveOccurred())
+			}, timeout, interval).Should(Succeed())
+
+			// Verify cleanup-blocked finalizer is removed from the user
+			Eventually(func(g Gomega) {
+				user := GetRabbitMQUser("nova-user1")
+				g.Expect(user.Finalizers).ShouldNot(ContainElement(rabbitmqv1.RabbitMQUserCleanupBlockedFinalizer),
+					"cleanup-blocked finalizer should be removed during nodeset deletion")
+			}, timeout, interval).Should(Succeed())
+		})
+	})
 })