Use uninstall annotation on policies

With the move to being event-driven by default, finalizers were not
being removed during the uninstall process because those policies were
not triggered to be reconciled. Now the `trigger-uninstall` process will
add an annotation to policies with finalizers, which the main controller
now watches for.

The value of the uninstall annotation is now a timestamp, as opposed to
just being set to "true" - this helps provide visibility, and lets the
`trigger-uninstall` process to update the annotation multiple times to
ensure a reconcile is triggered.

Refs:
 - https://issues.redhat.com/browse/ACM-14707

Signed-off-by: Justin Kulikauskas <[email protected]>

Author: JustinKuli

controllers/configurationpolicy_controller.go

@@ -121,12 +121,15 @@ func (r *ConfigurationPolicyReconciler) SetupWithManager(
 						return true
 					}
 
-					oldAnnotations := e.ObjectOld.GetAnnotations()
-					newAnnotations := e.ObjectNew.GetAnnotations()
+					oldAnnos := e.ObjectOld.GetAnnotations()
+					newAnnos := e.ObjectNew.GetAnnotations()
 
 					// These are the options that change evaluation behavior that aren't in the spec.
-					if oldAnnotations[disableTemplatesAnnotation] != newAnnotations[disableTemplatesAnnotation] ||
-						oldAnnotations[IVAnnotation] != newAnnotations[IVAnnotation] {
+					specialAnnoChanged := oldAnnos[IVAnnotation] != newAnnos[IVAnnotation] ||
+						oldAnnos[disableTemplatesAnnotation] != newAnnos[disableTemplatesAnnotation] ||
+						oldAnnos[common.UninstallingAnnotation] != newAnnos[common.UninstallingAnnotation]
+
+					if specialAnnoChanged {
 						return true
 					}
 
@@ -3449,5 +3452,12 @@ func IsBeingUninstalled(client client.Client) (bool, error) {
 		return false, err
 	}
 
-	return deployment.Annotations[common.UninstallingAnnotation] == "true", nil
+	value, found := deployment.Annotations[common.UninstallingAnnotation]
+	if !found {
+		return false, nil
+	}
+
+	notUninstalling := value == "false" || value == ""
+
+	return !notUninstalling, nil
 }

pkg/triggeruninstall/triggeruninstall.go

@@ -45,7 +45,7 @@ func TriggerUninstall(
 		}
 
 		annotations := deployment.GetAnnotations()
-		annotations[common.UninstallingAnnotation] = "true"
+		annotations[common.UninstallingAnnotation] = time.Now().Format(time.RFC3339)
 		deployment.SetAnnotations(annotations)
 
 		_, err = deploymentRsrc.Update(ctx, deployment, metav1.UpdateOptions{})
@@ -77,9 +77,9 @@ func TriggerUninstall(
 		default:
 		}
 
-		configPolicies, err := dynamicClient.Resource(configPolicyGVR).Namespace(policyNamespace).List(
-			ctx, metav1.ListOptions{},
-		)
+		policyClient := dynamicClient.Resource(configPolicyGVR).Namespace(policyNamespace)
+
+		configPolicies, err := policyClient.List(ctx, metav1.ListOptions{})
 		if err != nil {
 			if k8serrors.IsServerTimeout(err) || k8serrors.IsTimeout(err) {
 				klog.Infof("Retrying listing the ConfigurationPolicy objects due to error: %s", err)
@@ -96,16 +96,26 @@ func TriggerUninstall(
 			if len(configPolicy.GetFinalizers()) != 0 {
 				cleanedUp = false
 
-				break
+				annos := configPolicy.GetAnnotations()
+				annos[common.UninstallingAnnotation] = time.Now().Format(time.RFC3339)
+
+				updatedPolicy := configPolicy.DeepCopy()
+
+				updatedPolicy.SetAnnotations(annos)
+
+				if _, err := policyClient.Update(ctx, updatedPolicy, metav1.UpdateOptions{}); err != nil {
+					klog.Errorf("Error updating policy %v/%v with an uninstalling annotation: %s",
+						configPolicy.GetNamespace(), configPolicy.GetName(), err)
+				}
 			}
 		}
 
 		if cleanedUp {
 			break
 		}
 
-		klog.Info("The uninstall preparation is not complete. Sleeping two seconds before checking again.")
-		time.Sleep(2 * time.Second)
+		klog.Info("The uninstall preparation is not complete. Sleeping five seconds before checking again.")
+		time.Sleep(5 * time.Second)
 	}
 
 	return nil

test/e2e/case29_trigger_uninstall_test.go

@@ -53,6 +53,9 @@ var _ = Describe("Clean up during uninstalls", Label("running-in-cluster"), Orde
 			g.Expect(policy2.GetFinalizers()).To(ContainElement(pruneObjectFinalizer))
 		}, defaultTimeoutSeconds, 1).Should(Succeed())
 
+		By("Waiting 25 seconds to ensure there are no leftover reconciles")
+		time.Sleep(25 * time.Second)
+
 		By("Triggering an uninstall")
 		config, err := LoadConfig("", kubeconfigManaged, "")
 		Expect(err).ToNot(HaveOccurred())
@@ -73,9 +76,9 @@ var _ = Describe("Clean up during uninstalls", Label("running-in-cluster"), Orde
 			context.TODO(), deploymentName, metav1.GetOptions{},
 		)
 		Expect(err).ToNot(HaveOccurred())
-		Expect(deployment.GetAnnotations()).To(HaveKeyWithValue(common.UninstallingAnnotation, "true"))
+		Expect(deployment.GetAnnotations()).To(HaveKey(common.UninstallingAnnotation))
 
-		By("Verifying that the ConfiguratioPolicy finalizers have been removed")
+		By("Verifying that the ConfigurationPolicy finalizers have been removed")
 		policy := utils.GetWithTimeout(
 			clientManagedDynamic, gvrConfigPolicy, policyName, testNamespace, true, defaultTimeoutSeconds,
 		)