From bfbf13054aa09bc8e25dd3162e356b8801b35585 Mon Sep 17 00:00:00 2001 From: Zhiying Lin Date: Thu, 28 Sep 2023 17:04:31 +0800 Subject: [PATCH 1/2] test: add e2e tests to validate the revision history --- .../e2e/placement_selecting_resources_test.go | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/test/e2e/placement_selecting_resources_test.go b/test/e2e/placement_selecting_resources_test.go index 3f1c2c52a..1c50cf87d 100644 --- a/test/e2e/placement_selecting_resources_test.go +++ b/test/e2e/placement_selecting_resources_test.go @@ -17,6 +17,7 @@ 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" "go.goms.io/fleet/pkg/controllers/clusterresourceplacement" @@ -1033,3 +1034,106 @@ var _ = Describe("validating CRP when placing cluster scope resource (other than Eventually(finalizerRemovedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to remove controller finalizers from CRP %s", crpName) }) }) + +var _ = Describe("validating CRP revision history 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), + }, + }, + RevisionHistoryLimit: pointer.Int32(1), + }, + } + 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) + }) + + 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} + + 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)) + + 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("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) + }) +}) + From 646f59a09e0fdea4aeb2bd1de07459c20273e13d Mon Sep 17 00:00:00 2001 From: Zhiying Lin Date: Mon, 16 Oct 2023 17:27:34 +0800 Subject: [PATCH 2/2] address comments --- test/e2e/actuals_test.go | 21 ++++ .../e2e/placement_selecting_resources_test.go | 105 ++++++++++++++++-- 2 files changed, 115 insertions(+), 11 deletions(-) diff --git a/test/e2e/actuals_test.go b/test/e2e/actuals_test.go index 4ab297274..c8d17fb71 100644 --- 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/pkg/controllers/clusterresourceplacement" @@ -224,3 +225,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 index 1c50cf87d..9957d7121 100644 --- a/test/e2e/placement_selecting_resources_test.go +++ b/test/e2e/placement_selecting_resources_test.go @@ -17,7 +17,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" "go.goms.io/fleet/pkg/controllers/clusterresourceplacement" @@ -1035,7 +1034,7 @@ var _ = Describe("validating CRP when placing cluster scope resource (other than }) }) -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() { @@ -1107,16 +1106,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) + }) - 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 remove placed resources from all member clusters", checkIfRemovedWorkResourcesFromAllMemberClusters) - 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 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) + }) + + It("should update CRP status as expected", checkIfPlacedWorkResourcesOnAllMemberClusters) + + 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() { @@ -1136,4 +1220,3 @@ var _ = Describe("validating CRP revision history when updating resource selecto Eventually(finalizerRemovedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to remove controller finalizers from CRP %s", crpName) }) }) -