try to fix apply strategy e2e

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

Author: britaniar

test/e2e/placement_apply_strategy_test.go

@@ -23,6 +23,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
@@ -37,7 +38,7 @@ const (
 	e2eTestFieldManager = "e2e-test-field-manager"
 )
 
-var _ = FDescribe("validating CRP when resources exists", Ordered, func() {
+var _ = Describe("validating CRP when resources exists", Ordered, func() {
 	crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess())
 	annotationKey := "annotation-key"
 	annotationValue := "annotation-value"
@@ -77,17 +78,27 @@ var _ = FDescribe("validating CRP when resources exists", Ordered, func() {
 		})
 
 		AfterAll(func() {
-			By("removing the owner reference of the namespace")
-			// Remove the extra owner reference from the namespace to avoid deletion delays.
-			// With the new change to use foreground deletion on applied Work resources, the Work will be blocked
-			// from deleting until the applied work is fully deleted. If the namespace has
-			// multiple owner references, it won’t be deleted immediately, which can cause resource overlap
-			// and flakiness in subsequent tests.
-			removeOtherOwner(allMemberClusters[0])
+			//By("removing the owner reference of the namespace")
+			//// Remove the extra owner reference from the namespace to avoid deletion delays.
+			//// With the new change to use foreground deletion on applied Work resources, the Work will be blocked
+			//// from deleting until the applied work is fully deleted. If the namespace has
+			//// multiple owner references, it won’t be deleted immediately, which can cause resource overlap
+			//// and flakiness in subsequent tests.
+			//removeOtherOwner(allMemberClusters[0])
 
 			By("deleting created work resources on member cluster")
 			cleanWorkResourcesOnCluster(allMemberClusters[0])
 
+			// Check if work is deleted. Needed to ensure that the Work resource is cleaned up before the next CRP is created.
+			// This is because the Work resource is created with a finalizer that blocks deletion until the all applied work
+			// and applied work itself is successfully deleted. If the Work resource is not deleted, it can cause resource overlap
+			// and flakiness in subsequent tests.
+			By("Check if work is deleted")
+			Eventually(func() bool {
+				work := &placementv1beta1.Work{}
+				return errors.IsNotFound(hubClient.Get(ctx, types.NamespacedName{Name: "crp-1-work", Namespace: "fleet-member-kind-cluster-1"}, work))
+			}, 6*eventuallyDuration, eventuallyInterval).Should(BeTrue(), "Work resource should be deleted from hub")
+
 			By(fmt.Sprintf("deleting placement %s", crpName))
 			cleanupCRP(crpName)
 
@@ -128,7 +139,7 @@ var _ = FDescribe("validating CRP when resources exists", Ordered, func() {
 		It("namespace should be kept on member cluster", func() {
 			Consistently(func() error {
 				ns := &corev1.Namespace{}
-				return allMemberClusters[0].KubeClient.Get(ctx, types.NamespacedName{Name: workNamespaceName}, ns)
+				return hubClient.Get(ctx, types.NamespacedName{Name: workNamespaceName}, ns)
 			}, consistentlyDuration, consistentlyInterval).Should(Succeed(), "Namespace which is not owned by the CRP should not be deleted")
 		})
 	})
@@ -148,6 +159,16 @@ var _ = FDescribe("validating CRP when resources exists", Ordered, func() {
 		})
 
 		AfterAll(func() {
+			// Check if work is deleted. Needed to ensure that the Work resource is cleaned up before the next CRP is created.
+			// This is because the Work resource is created with a finalizer that blocks deletion until the all applied work
+			// and applied work itself is successfully deleted. If the Work resource is not deleted, it can cause resource overlap
+			// and flakiness in subsequent tests.
+			By("Check if work is deleted")
+			Eventually(func() bool {
+				work := &placementv1beta1.Work{}
+				return errors.IsNotFound(hubClient.Get(ctx, types.NamespacedName{Name: "crp-1-work", Namespace: "fleet-member-kind-cluster-1"}, work))
+			}, 6*eventuallyDuration, eventuallyInterval).Should(BeTrue(), "Work resource should be deleted from hub")
+
 			By(fmt.Sprintf("deleting placement %s", crpName))
 			cleanupCRP(crpName)
 		})
@@ -203,6 +224,7 @@ var _ = FDescribe("validating CRP when resources exists", Ordered, func() {
 		AfterAll(func() {
 			By(fmt.Sprintf("deleting placement %s", crpName))
 			cleanupCRP(crpName)
+
 		})
 
 		It("should update CRP status as expected", func() {
@@ -259,19 +281,29 @@ var _ = FDescribe("validating CRP when resources exists", Ordered, func() {
 		})
 
 		AfterAll(func() {
-			By("removing the owner reference of the namespace")
-			// Remove the extra owner reference from the namespace to avoid deletion delays.
-			// With the new change to use foreground deletion on applied Work resources, the Work will be blocked
-			// from deleting until the applied work is fully deleted. If the namespace has
-			// multiple owner references, it won’t be deleted immediately, which can cause resource overlap
+			//By("removing the owner reference of the namespace")
+			//// Remove the extra owner reference from the namespace to avoid deletion delays.
+			//// With the new change to use foreground deletion on applied Work resources, the Work will be blocked
+			//// from deleting until the applied work is fully deleted. If the namespace has
+			//// multiple owner references, it won’t be deleted immediately, which can cause resource overlap
+			//// and flakiness in subsequent tests.
+			//removeOtherOwner(allMemberClusters[0])
+
+			By("deleting created work resources on member cluster")
+			cleanWorkResourcesOnCluster(allMemberClusters[0])
+
+			// Check if work is deleted. Needed to ensure that the Work resource is cleaned up before the next CRP is created.
+			// This is because the Work resource is created with a finalizer that blocks deletion until the all applied work
+			// and applied work itself is successfully deleted. If the Work resource is not deleted, it can cause resource overlap
 			// and flakiness in subsequent tests.
-			removeOtherOwner(allMemberClusters[0])
+			By("Check if work is deleted")
+			Eventually(func() bool {
+				work := &placementv1beta1.Work{}
+				return errors.IsNotFound(hubClient.Get(ctx, types.NamespacedName{Name: "crp-1-work", Namespace: "fleet-member-kind-cluster-1"}, work))
+			}, 6*eventuallyDuration, eventuallyInterval).Should(BeTrue(), "Work resource should be deleted from hub")
 
 			By(fmt.Sprintf("deleting placement %s", crpName))
 			cleanupCRP(crpName)
-
-			By("deleting created work resources on member cluster")
-			cleanWorkResourcesOnCluster(allMemberClusters[0])
 		})
 
 		It("should update CRP status as expected", func() {
@@ -402,19 +434,29 @@ var _ = FDescribe("validating CRP when resources exists", Ordered, func() {
 		})
 
 		AfterAll(func() {
-			By("removing the owner reference of the namespace")
-			// Remove the extra owner reference from the namespace to avoid deletion delays.
-			// With the new change to use foreground deletion on applied Work resources, the Work will be blocked
-			// from deleting until the applied work is fully deleted. If the namespace has
-			// multiple owner references, it won’t be deleted immediately, which can cause resource overlap
+			//By("removing the owner reference of the namespace")
+			//// Remove the extra owner reference from the namespace to avoid deletion delays.
+			//// With the new change to use foreground deletion on applied Work resources, the Work will be blocked
+			//// from deleting until the applied work is fully deleted. If the namespace has
+			//// multiple owner references, it won’t be deleted immediately, which can cause resource overlap
+			//// and flakiness in subsequent tests.
+			//removeOtherOwner(allMemberClusters[0])
+
+			By("deleting created work resources on member cluster")
+			cleanWorkResourcesOnCluster(allMemberClusters[0])
+
+			// Check if work is deleted. Needed to ensure that the Work resource is cleaned up before the next CRP is created.
+			// This is because the Work resource is created with a finalizer that blocks deletion until the all applied work
+			// and applied work itself is successfully deleted. If the Work resource is not deleted, it can cause resource overlap
 			// and flakiness in subsequent tests.
-			removeOtherOwner(allMemberClusters[0])
+			By("Check if work is deleted")
+			Eventually(func() bool {
+				work := &placementv1beta1.Work{}
+				return errors.IsNotFound(hubClient.Get(ctx, types.NamespacedName{Name: "crp-1-work", Namespace: "fleet-member-kind-cluster-1"}, work))
+			}, 7*eventuallyDuration, eventuallyInterval).Should(BeTrue(), "Work resource should be deleted from hub")
 
 			By(fmt.Sprintf("deleting placement %s", crpName))
 			cleanupCRP(crpName)
-
-			By("deleting created work resources on member cluster")
-			cleanWorkResourcesOnCluster(allMemberClusters[0])
 		})
 
 		It("should update CRP status as expected", func() {
@@ -638,14 +680,6 @@ var _ = FDescribe("validating CRP when resources exists", Ordered, func() {
 		})
 
 		AfterAll(func() {
-			By("removing the owner reference of the namespace")
-			// Remove the extra owner reference from the namespace to avoid deletion delays.
-			// With the new change to use foreground deletion on applied Work resources, the Work will be blocked
-			// from deleting until the applied work is fully deleted. If the namespace has
-			// multiple owner references, it won’t be deleted immediately, which can cause resource overlap
-			// and flakiness in subsequent tests.
-			removeOtherOwner(allMemberClusters[0])
-
 			ensureCRPAndRelatedResourcesDeleted(crpName, allMemberClusters)
 
 			ensureCRPAndRelatedResourcesDeleted(conflictedCRPName, allMemberClusters)

test/e2e/utils_test.go

@@ -972,6 +972,8 @@ func removeOtherOwner(cluster *framework.Cluster) {
 		for _, ref := range ns.OwnerReferences {
 			if ref.Name != otherOwner.Name {
 				newOwnerRefs = append(newOwnerRefs, ref)
+			} else if ref.Name == otherOwner.Name {
+				By(fmt.Sprintf("Removing owner reference %s from namespace %s", otherOwner.Name, ns.Name))
 			}
 		}