update to workapplier to use foreground deletion, add and fix tests

Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com>

Author: britaniar

pkg/controllers/workapplier/controller.go

@@ -417,7 +417,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		Kind:               fleetv1beta1.AppliedWorkKind,
 		Name:               appliedWork.GetName(),
 		UID:                appliedWork.GetUID(),
-		BlockOwnerDeletion: ptr.To(false),
+		BlockOwnerDeletion: ptr.To(true),
 	}
 
 	// Set the default values for the Work object to avoid additional validation logic in the
@@ -487,7 +487,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 
 // garbageCollectAppliedWork deletes the appliedWork and all the manifests associated with it from the cluster.
 func (r *Reconciler) garbageCollectAppliedWork(ctx context.Context, work *fleetv1beta1.Work) (ctrl.Result, error) {
-	deletePolicy := metav1.DeletePropagationBackground
+	deletePolicy := metav1.DeletePropagationForeground
 	if !controllerutil.ContainsFinalizer(work, fleetv1beta1.WorkFinalizer) {
 		return ctrl.Result{}, nil
 	}
@@ -496,18 +496,22 @@ func (r *Reconciler) garbageCollectAppliedWork(ctx context.Context, work *fleetv
 	appliedWork := fleetv1beta1.AppliedWork{
 		ObjectMeta: metav1.ObjectMeta{Name: work.Name},
 	}
-	err := r.spokeClient.Delete(ctx, &appliedWork, &client.DeleteOptions{PropagationPolicy: &deletePolicy})
-	switch {
-	case apierrors.IsNotFound(err):
-		klog.V(2).InfoS("The appliedWork is already deleted", "appliedWork", work.Name)
-	case err != nil:
-		klog.ErrorS(err, "Failed to delete the appliedWork", "appliedWork", work.Name)
+	if err := r.spokeClient.Delete(ctx, &appliedWork, &client.DeleteOptions{PropagationPolicy: &deletePolicy}); err != nil {
+		if apierrors.IsNotFound(err) {
+			klog.V(2).InfoS("The appliedWork is already deleted", "appliedWork", work.Name)
+			return r.removeWorkFinalizer(ctx, work)
+		}
+		klog.V(2).ErrorS(err, "Failed to delete the appliedWork", "appliedWork", work.Name)
 		return ctrl.Result{}, controller.NewAPIServerError(false, err)
-	default:
-		klog.InfoS("Successfully deleted the appliedWork", "appliedWork", work.Name)
 	}
-	controllerutil.RemoveFinalizer(work, fleetv1beta1.WorkFinalizer)
 
+	klog.V(2).InfoS("Deletion for appliedWork is in progress", "appliedWork", work.Name)
+	return ctrl.Result{Requeue: true}, nil
+}
+
+// removeWorkFinalizer removes the finalizer from the work and updates it in the hub.
+func (r *Reconciler) removeWorkFinalizer(ctx context.Context, work *fleetv1beta1.Work) (ctrl.Result, error) {
+	controllerutil.RemoveFinalizer(work, fleetv1beta1.WorkFinalizer)
 	if err := r.hubClient.Update(ctx, work, &client.UpdateOptions{}); err != nil {
 		klog.ErrorS(err, "Failed to remove the finalizer from the work", "work", klog.KObj(work))
 		return ctrl.Result{}, controller.NewAPIServerError(false, err)
@@ -557,6 +561,11 @@ func (r *Reconciler) ensureAppliedWork(ctx context.Context, work *fleetv1beta1.W
 			return nil, controller.NewAPIServerError(false, err)
 		}
 	}
+
+	if err := r.spokeClient.Get(ctx, types.NamespacedName{Name: workRef.Name}, appliedWork); err != nil {
+		klog.ErrorS(err, "Failed to get the appliedWork", "appliedWork", appliedWork.Name)
+		return nil, controller.NewAPIServerError(false, err)
+	}
 	klog.InfoS("Recreated the appliedWork resource", "appliedWork", workRef.Name)
 	return appliedWork, nil
 }

pkg/controllers/workapplier/controller_integration_migrated_helper_test.go

@@ -24,6 +24,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -137,3 +138,92 @@ func waitForWorkToBeHandled(workName, workNS string) *fleetv1beta1.Work {
 	}, eventuallyDuration, eventuallyInterval).Should(BeTrue())
 	return &resultWork
 }
+
+// verifyConfigMapExists verifies that the configmap exists in member cluster
+func verifyConfigMapExists(cm *corev1.ConfigMap, resourceNamespace string) {
+	Consistently(func() bool {
+		var configMap corev1.ConfigMap
+		err := memberClient.Get(context.Background(), types.NamespacedName{Name: cm.Name, Namespace: resourceNamespace}, &configMap)
+		return !apierrors.IsNotFound(err)
+	}, consistentlyDuration, consistentlyInterval).Should(BeTrue(), fmt.Sprintf("ConfigMap %s should not be deleted", cm.Name))
+}
+
+// verifyConfigmapIsRemoved verifies that the configmap is removed from member cluster
+func verifyConfigmapIsRemoved(cm *corev1.ConfigMap, ns string) {
+	Eventually(func() bool {
+		var configMap corev1.ConfigMap
+		err := memberClient.Get(context.Background(), types.NamespacedName{Name: cm.Name, Namespace: ns}, &configMap)
+		return apierrors.IsNotFound(err)
+	}, eventuallyDuration*5, eventuallyInterval).Should(BeTrue(), fmt.Sprintf("ConfigMap %s should be deleted", cm.Name))
+}
+
+// verifyWorkIsDeleted verifies that the work is deleted from the hub cluster
+func verifyWorkIsDeleted(work *fleetv1beta1.Work) {
+	Eventually(func() bool {
+		var currentWork fleetv1beta1.Work
+		return apierrors.IsNotFound(hubClient.Get(context.Background(), types.NamespacedName{Name: work.Name, Namespace: memberReservedNSName}, &currentWork))
+	}, eventuallyDuration, eventuallyInterval).Should(BeTrue(), "Work should be deleted")
+}
+
+// verifyAppliedWorkIsDeleted verifies that the applied work is deleted from the hub cluster
+func verifyAppliedWorkIsDeleted(name string) {
+	Eventually(func() bool {
+		var currentAppliedWork fleetv1beta1.AppliedWork
+		return apierrors.IsNotFound(hubClient.Get(context.Background(), types.NamespacedName{Name: name}, &currentAppliedWork))
+	}, eventuallyDuration, eventuallyInterval).Should(BeTrue(), "AppliedWork should be deleted")
+}
+
+// verifyNamespaceIsDeleted verifies that the namespace is deleted from the hub cluster
+func verifyNamespaceIsDeleted(nsName string) {
+	Eventually(func() bool {
+		var ns corev1.Namespace
+		return apierrors.IsNotFound(hubClient.Get(context.Background(), types.NamespacedName{Name: nsName}, &ns))
+	}, eventuallyDuration, eventuallyInterval).Should(BeTrue(), "Namespace should be deleted")
+}
+
+// removeFinalizersFromResource removes the finalizers from the configmap to delete
+func removeFinalizersFromResource(cm *corev1.ConfigMap) {
+	var configMap corev1.ConfigMap
+	Expect(memberClient.Get(context.Background(), types.NamespacedName{Name: cm.Name, Namespace: cm.Namespace}, &configMap)).Should(Succeed(), "Failed to get configmap")
+	controllerutil.RemoveFinalizer(&configMap, "example.com/finalizer")
+	Expect(memberClient.Update(context.Background(), &configMap)).Should(Succeed(), "Failed to remove finalizers from configmap")
+}
+
+func addFinalizerToConfigMap(cm *corev1.ConfigMap) {
+	// Add finalizer to the configmap
+	var configMap corev1.ConfigMap
+	Expect(memberClient.Get(context.Background(), types.NamespacedName{Name: cm.Name, Namespace: cm.Namespace}, &configMap)).Should(Succeed())
+	controllerutil.AddFinalizer(&configMap, "example.com/finalizer")
+	Expect(memberClient.Update(context.Background(), &configMap)).Should(Succeed())
+	By("Added finalizer to configmap")
+}
+
+func cleanupResources(cm, cm2 *corev1.ConfigMap, ns *corev1.Namespace, work *fleetv1beta1.Work) {
+	// Remove finalizers from the configmap
+	removeFinalizersFromResource(cm)
+	removeFinalizersFromResource(cm2)
+
+	// Delete the configmap
+	Expect(memberClient.Delete(context.Background(), cm)).Should(Succeed())
+	Expect(memberClient.Delete(context.Background(), cm2)).Should(Succeed())
+
+	// Verify the configmap is deleted
+	verifyConfigmapIsRemoved(cm, cm.Namespace)
+	verifyConfigmapIsRemoved(cm2, cm2.Namespace)
+
+	// Delete the namespace
+	Expect(memberClient.Delete(context.Background(), ns)).Should(Succeed())
+	verifyNamespaceIsDeleted(ns.Name)
+
+	// Remove finalizers from the applied work. Needed as there
+	var currentAppliedWork fleetv1beta1.AppliedWork
+	Expect(memberClient.Get(context.Background(), types.NamespacedName{Name: work.Name}, &currentAppliedWork)).Should(Succeed(), "Failed to get applied work")
+	controllerutil.RemoveFinalizer(&currentAppliedWork, metav1.FinalizerDeleteDependents)
+	Expect(memberClient.Update(context.Background(), &currentAppliedWork)).Should(Succeed(), "Failed to remove finalizers from applied work")
+
+	// verify the applied work is deleted
+	verifyAppliedWorkIsDeleted(work.Name)
+
+	// verify the work is deleted
+	verifyWorkIsDeleted(work)
+}