address comments

Azure · Oct 16, 2023 · f0cab39 · f0cab39
1 parent 066224c
commit f0cab39
Show file tree

Hide file tree

Showing 2 changed files with 115 additions and 10 deletions.
diff --git a/test/e2e/actuals_test.go b/test/e2e/actuals_test.go
@@ -14,6 +14,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1"
 	"go.goms.io/fleet/test/e2e/framework"
@@ -216,3 +217,23 @@ func crpRemovedActual() func() error {
 		return nil
 	}
 }
+
+func validateCRPSnapshotRevisions(crpName string, wantPolicySnapshotRevision, wantResourceSnapshotRevision int) error {
+	matchingLabels := client.MatchingLabels{placementv1beta1.CRPTrackingLabel: crpName}
+
+	snapshotList := &placementv1beta1.ClusterSchedulingPolicySnapshotList{}
+	if err := hubClient.List(ctx, snapshotList, matchingLabels); err != nil {
+		return err
+	}
+	if len(snapshotList.Items) != wantPolicySnapshotRevision {
+		return fmt.Errorf("clusterSchedulingPolicySnapshotList got %v, want 1", len(snapshotList.Items))
+	}
+	resourceSnapshotList := &placementv1beta1.ClusterResourceSnapshotList{}
+	if err := hubClient.List(ctx, resourceSnapshotList, matchingLabels); err != nil {
+		return err
+	}
+	if len(resourceSnapshotList.Items) != wantResourceSnapshotRevision {
+		return fmt.Errorf("clusterResourceSnapshotList got %v, want 2", len(snapshotList.Items))
+	}
+	return nil
+}
diff --git a/test/e2e/placement_selecting_resources_test.go b/test/e2e/placement_selecting_resources_test.go
@@ -15,7 +15,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/pointer"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1"
 )
@@ -594,7 +593,7 @@ var _ = Describe("validating CRP when deleting resources in a matching namespace
 	})
 })
 
-var _ = Describe("validating CRP revision history when updating resource selector", Ordered, func() {
+var _ = Describe("validating CRP revision history allowing single revision when updating resource selector", Ordered, func() {
 	crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess())
 
 	BeforeAll(func() {
@@ -666,16 +665,101 @@ var _ = Describe("validating CRP revision history when updating resource selecto
 
 	It("should update CRP status as expected", checkIfPlacedWorkResourcesOnAllMemberClusters)
 
-	It("should have one policy snapshot revision and one resource snapshot revision only", func() {
-		matchingLabels := client.MatchingLabels{placementv1beta1.CRPTrackingLabel: crpName}
+	It("should have one policy snapshot revision and one resource snapshot revision", func() {
+		Expect(validateCRPSnapshotRevisions(crpName, 1, 1)).Should(Succeed(), "Failed to validate the revision history")
+	})
+
+	It("can delete the CRP", func() {
+		// Delete the CRP.
+		crp := &placementv1beta1.ClusterResourcePlacement{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: crpName,
+			},
+		}
+		Expect(hubClient.Delete(ctx, crp)).To(Succeed(), "Failed to delete CRP %s", crpName)
+	})
+
+	It("should remove placed resources from all member clusters", checkIfRemovedWorkResourcesFromAllMemberClusters)
+
+	It("should remove controller finalizers from CRP", func() {
+		finalizerRemovedActual := allFinalizersExceptForCustomDeletionBlockerRemovedFromCRPActual()
+		Eventually(finalizerRemovedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to remove controller finalizers from CRP %s", crpName)
+	})
+})
+
+var _ = Describe("validating CRP revision history allowing multiple revisions when updating resource selector", Ordered, func() {
+	crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess())
+
+	BeforeAll(func() {
+		By("creating work resources")
+		createWorkResources()
+
+		// Create the CRP.
+		crp := &placementv1beta1.ClusterResourcePlacement{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: crpName,
+				// Add a custom finalizer; this would allow us to better observe
+				// the behavior of the controllers.
+				Finalizers: []string{customDeletionBlockerFinalizer},
+			},
+			Spec: placementv1beta1.ClusterResourcePlacementSpec{
+				ResourceSelectors: []placementv1beta1.ClusterResourceSelector{
+					{
+						Group:   "",
+						Kind:    "Namespace",
+						Version: "v1",
+						Name:    "invalid-namespace",
+					},
+				},
+				Strategy: placementv1beta1.RolloutStrategy{
+					RollingUpdate: &placementv1beta1.RollingUpdateConfig{
+						UnavailablePeriodSeconds: pointer.Int(5),
+					},
+				},
+			},
+		}
+		By(fmt.Sprintf("creating placement %s", crpName))
+		Expect(hubClient.Create(ctx, crp)).To(Succeed(), "Failed to create CRP %s", crpName)
+	})
+
+	AfterAll(func() {
+		By(fmt.Sprintf("deleting placement %s", crpName))
+		cleanupCRP(crpName)
+
+		By("deleting created work resources")
+		cleanupWorkResources()
+	})
+
+	It("should update CRP status as expected", func() {
+		crpStatusUpdatedActual := func() error {
+			return validateCRPStatus(types.NamespacedName{Name: crpName}, nil)
+		}
+		Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName)
+	})
+
+	It("adding resource selectors", func() {
+		updateFunc := func() error {
+			crp := &placementv1beta1.ClusterResourcePlacement{}
+			if err := hubClient.Get(ctx, types.NamespacedName{Name: crpName}, crp); err != nil {
+				return err
+			}
+
+			crp.Spec.ResourceSelectors = append(crp.Spec.ResourceSelectors, placementv1beta1.ClusterResourceSelector{
+				Group:   "",
+				Kind:    "Namespace",
+				Version: "v1",
+				Name:    fmt.Sprintf(workNamespaceNameTemplate, GinkgoParallelProcess()),
+			})
+			// may hit 409
+			return hubClient.Update(ctx, crp)
+		}
+		Eventually(updateFunc(), eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update the crp %s", crpName)
+	})
 
-		snapshotList := &placementv1beta1.ClusterSchedulingPolicySnapshotList{}
-		Expect(hubClient.List(ctx, snapshotList, matchingLabels)).Should(Succeed(), "Failed to list the policy revisions")
-		Expect(len(snapshotList.Items)).Should(Equal(1), "clusterSchedulingPolicySnapshotList got %v, want 1", len(snapshotList.Items))
+	It("should update CRP status as expected", checkIfPlacedWorkResourcesOnAllMemberClusters)
 
-		resourceSnapshotList := &placementv1beta1.ClusterResourceSnapshotList{}
-		Expect(hubClient.List(ctx, resourceSnapshotList, matchingLabels)).Should(Succeed(), "Failed to list the resource revisions")
-		Expect(len(snapshotList.Items)).Should(Equal(1), "clusterResourceSnapshotList got %v, want 1", len(snapshotList.Items))
+	It("should have one policy snapshot revision and two resource snapshot revisions", func() {
+		Expect(validateCRPSnapshotRevisions(crpName, 1, 2)).Should(Succeed(), "Failed to validate the revision history")
 	})
 
 	It("can delete the CRP", func() {