Remove the unused Deployment finalizer

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())
+ })
+})