Remove the unused Deployment finalizer

Previous code added a finalizer on the Deployment running the controller
when pruneObjectBehavior was used. This commit removes that unnecessary
finalizer if present.

Relates:
https://issues.redhat.com/browse/ACM-2923

Signed-off-by: mprahl <mprahl@users.noreply.github.com>

controllers/configurationpolicy_controller.go

@@ -166,8 +166,18 @@ func (r *ConfigurationPolicyReconciler) PeriodicallyExecConfigPolicies(
 	const waiting = 10 * time.Minute
 
 	exiting := false
+	deploymentFinalizerRemoved := false
 
 	for !exiting {
+		if !deploymentFinalizerRemoved {
+			err := r.removeLegacyDeploymentFinalizer()
+			if err == nil {
+				deploymentFinalizerRemoved = true
+			} else {
+				log.Error(err, "Failed to remove the legacy Deployment finalizer. Will try again.")
+			}
+		}
+
 		start := time.Now()
 		policiesList := policyv1.ConfigurationPolicyList{}
 
@@ -2777,25 +2787,60 @@ func convertPolicyStatusToString(plc *policyv1.ConfigurationPolicy) (results str
 	return result
 }
 
-func (r *ConfigurationPolicyReconciler) isBeingUninstalled() (bool, error) {
+// getDeployment gets the Deployment object associated with this controller. If the controller is running outside of
+// a cluster, no Deployment object or error will be returned.
+func (r *ConfigurationPolicyReconciler) getDeployment() (*appsv1.Deployment, error) {
 	key, err := common.GetOperatorNamespacedName()
 	if err != nil {
 		// Running locally
 		if errors.Is(err, common.ErrNoNamespace) || errors.Is(err, common.ErrRunLocal) {
-			return false, nil
+			return nil, nil
 		}
 
-		return false, err
+		return nil, err
 	}
 
 	deployment := appsv1.Deployment{}
 	if err := r.Client.Get(context.TODO(), key, &deployment); err != nil {
+		return nil, err
+	}
+
+	return &deployment, nil
+}
+
+func (r *ConfigurationPolicyReconciler) isBeingUninstalled() (bool, error) {
+	deployment, err := r.getDeployment()
+	if deployment == nil || err != nil {
 		return false, err
 	}
 
 	return deployment.Annotations[common.UninstallingAnnotation] == "true", nil
 }
 
+// removeLegacyDeploymentFinalizer removes the policy.open-cluster-management.io/delete-related-objects on the
+// Deployment object. This finalizer is no longer needed on the Deployment object, so it is removed.
+func (r *ConfigurationPolicyReconciler) removeLegacyDeploymentFinalizer() error {
+	deployment, err := r.getDeployment()
+	if deployment == nil || err != nil {
+		return err
+	}
+
+	for i, finalizer := range deployment.Finalizers {
+		if finalizer == pruneObjectFinalizer {
+			newFinalizers := append(deployment.Finalizers[:i], deployment.Finalizers[i+1:]...)
+			deployment.SetFinalizers(newFinalizers)
+
+			log.Info("Removing the legacy finalizer on the controller Deployment")
+
+			err := r.Client.Update(context.TODO(), deployment)
+
+			return err
+		}
+	}
+
+	return nil
+}
+
 func recoverFlow() {
 	if r := recover(); r != nil {
 		// V(-2) is the error level

test/e2e/case29_trigger_uninstall_test.go

@@ -119,3 +119,59 @@ var _ = Describe("Clean up during uninstalls", Label("running-in-cluster"), Orde
 		}, defaultTimeoutSeconds, 1).Should(Succeed())
 	})
 })
+
+// This test only works when the controller is running in the cluster.
+var _ = Describe("Clean up the finalizer on the Deployment", Label("running-in-cluster"), Ordered, func() {
+	const (
+		deploymentName       string = "config-policy-controller"
+		deploymentNamespace  string = "open-cluster-management-agent-addon"
+		pruneObjectFinalizer string = "policy.open-cluster-management.io/delete-related-objects"
+	)
+
+	It("verifies that the Deployment finalizer is removed", func() {
+		deploymentRsrc := clientManaged.AppsV1().Deployments(deploymentNamespace)
+
+		By("Adding a finalizer to the Deployment")
+		Eventually(func(g Gomega) {
+			deployment, err := deploymentRsrc.Get(context.TODO(), deploymentName, metav1.GetOptions{})
+			g.Expect(err).To(BeNil())
+
+			deployment.SetFinalizers(append(deployment.Finalizers, pruneObjectFinalizer))
+			_, err = deploymentRsrc.Update(context.TODO(), deployment, metav1.UpdateOptions{})
+			g.Expect(err).To(BeNil())
+		}, defaultTimeoutSeconds, 1).Should(Succeed())
+
+		// Trigger a restart so that the finalizer removal logic is executed.
+		utils.Kubectl("-n", deploymentNamespace, "rollout", "restart", "deployment/"+deploymentName)
+
+		By("Waiting for the finalizer on the Deployment to be removed")
+		Eventually(func(g Gomega) {
+			deployment, err := deploymentRsrc.Get(context.TODO(), deploymentName, metav1.GetOptions{})
+			g.Expect(err).To(BeNil())
+
+			g.Expect(deployment.Finalizers).ToNot(ContainElement(pruneObjectFinalizer))
+		}, defaultTimeoutSeconds*2, 1).Should(Succeed())
+	})
+
+	AfterAll(func() {
+		deploymentRsrc := clientManaged.AppsV1().Deployments(deploymentNamespace)
+
+		// Use an eventually in case there are update conflicts and there needs to be a retry
+		Eventually(func(g Gomega) {
+			deployment, err := deploymentRsrc.Get(context.TODO(), deploymentName, metav1.GetOptions{})
+			g.Expect(err).To(BeNil())
+
+			for i, finalizer := range deployment.Finalizers {
+				if finalizer == pruneObjectFinalizer {
+					newFinalizers := append(deployment.Finalizers[:i], deployment.Finalizers[i+1:]...)
+					deployment.SetFinalizers(newFinalizers)
+
+					_, err := deploymentRsrc.Update(context.TODO(), deployment, metav1.UpdateOptions{})
+					g.Expect(err).To(BeNil())
+
+					break
+				}
+			}
+		}, defaultTimeoutSeconds, 1).Should(Succeed())
+	})
+})