From 019251b1a846eda360dd74af781dbb751d73d070 Mon Sep 17 00:00:00 2001 From: antonlisovenko Date: Thu, 25 Mar 2021 17:57:36 +0000 Subject: [PATCH 1/3] CLOUDP-83733: more tests on clusters --- pkg/controller/atlascluster/cluster.go | 63 ++++++----- pkg/controller/atlascluster/cluster_test.go | 41 +++++++- test/int/cluster_test.go | 109 +++++++++++++++----- 3 files changed, 154 insertions(+), 59 deletions(-) diff --git a/pkg/controller/atlascluster/cluster.go b/pkg/controller/atlascluster/cluster.go index 5d0ec914e5..8ef83581d4 100644 --- a/pkg/controller/atlascluster/cluster.go +++ b/pkg/controller/atlascluster/cluster.go @@ -15,42 +15,42 @@ import ( "github.com/mongodb/mongodb-atlas-kubernetes/pkg/util/compat" ) -func (r *AtlasClusterReconciler) ensureClusterState(ctx *workflow.Context, project *mdbv1.AtlasProject, cluster *mdbv1.AtlasCluster) (c *mongodbatlas.Cluster, _ workflow.Result) { - c, resp, err := ctx.Client.Clusters.Get(context.Background(), project.Status.ID, cluster.Spec.Name) +func (r *AtlasClusterReconciler) ensureClusterState(ctx *workflow.Context, project *mdbv1.AtlasProject, cluster *mdbv1.AtlasCluster) (atlasCluster *mongodbatlas.Cluster, _ workflow.Result) { + atlasCluster, resp, err := ctx.Client.Clusters.Get(context.Background(), project.Status.ID, cluster.Spec.Name) if err != nil { if resp == nil { - return c, workflow.Terminate(workflow.Internal, err.Error()) + return atlasCluster, workflow.Terminate(workflow.Internal, err.Error()) } if resp.StatusCode != http.StatusNotFound { - return c, workflow.Terminate(workflow.ClusterNotCreatedInAtlas, err.Error()) + return atlasCluster, workflow.Terminate(workflow.ClusterNotCreatedInAtlas, err.Error()) } - c, err = cluster.Spec.Cluster() + atlasCluster, err = cluster.Spec.Cluster() if err != nil { - return c, workflow.Terminate(workflow.Internal, err.Error()) + return atlasCluster, workflow.Terminate(workflow.Internal, err.Error()) } ctx.Log.Infof("Cluster %s doesn't exist in Atlas - creating", cluster.Spec.Name) - c, _, err = ctx.Client.Clusters.Create(context.Background(), project.Status.ID, c) + atlasCluster, _, err = ctx.Client.Clusters.Create(context.Background(), project.Status.ID, atlasCluster) if err != nil { - return c, workflow.Terminate(workflow.ClusterNotCreatedInAtlas, err.Error()) + return atlasCluster, workflow.Terminate(workflow.ClusterNotCreatedInAtlas, err.Error()) } } - switch c.StateName { + switch atlasCluster.StateName { case "IDLE": - resultingCluster, err := mergedCluster(*c, cluster.Spec) + resultingCluster, err := MergedCluster(*atlasCluster, cluster.Spec) if err != nil { - return c, workflow.Terminate(workflow.Internal, err.Error()) + return atlasCluster, workflow.Terminate(workflow.Internal, err.Error()) } - if done := clustersEqual(ctx.Log, *c, resultingCluster); done { - return c, workflow.OK() + if done := ClustersEqual(ctx.Log, *atlasCluster, resultingCluster); done { + return atlasCluster, workflow.OK() } if cluster.Spec.Paused != nil { - if c.Paused == nil || *c.Paused != *cluster.Spec.Paused { + if atlasCluster.Paused == nil || *atlasCluster.Paused != *cluster.Spec.Paused { // paused is different from Atlas // we need to first send a special (un)pause request before reconciling everything else resultingCluster = mongodbatlas.Cluster{ @@ -64,23 +64,23 @@ func (r *AtlasClusterReconciler) ensureClusterState(ctx *workflow.Context, proje resultingCluster = cleanupCluster(resultingCluster) - c, _, err = ctx.Client.Clusters.Update(context.Background(), project.Status.ID, cluster.Spec.Name, &resultingCluster) + atlasCluster, _, err = ctx.Client.Clusters.Update(context.Background(), project.Status.ID, cluster.Spec.Name, &resultingCluster) if err != nil { - return c, workflow.Terminate(workflow.ClusterNotUpdatedInAtlas, err.Error()) + return atlasCluster, workflow.Terminate(workflow.ClusterNotUpdatedInAtlas, err.Error()) } - return c, workflow.InProgress(workflow.ClusterUpdating, "cluster is updating") + return atlasCluster, workflow.InProgress(workflow.ClusterUpdating, "cluster is updating") case "CREATING": - return c, workflow.InProgress(workflow.ClusterCreating, "cluster is provisioning") + return atlasCluster, workflow.InProgress(workflow.ClusterCreating, "cluster is provisioning") case "UPDATING", "REPAIRING": - return c, workflow.InProgress(workflow.ClusterUpdating, "cluster is updating") + return atlasCluster, workflow.InProgress(workflow.ClusterUpdating, "cluster is updating") // TODO: add "DELETING", "DELETED", handle 404 on delete default: - return c, workflow.Terminate(workflow.Internal, fmt.Sprintf("unknown cluster state %q", c.StateName)) + return atlasCluster, workflow.Terminate(workflow.Internal, fmt.Sprintf("unknown cluster state %q", atlasCluster.StateName)) } } @@ -99,9 +99,9 @@ func cleanupCluster(cluster mongodbatlas.Cluster) mongodbatlas.Cluster { return cluster } -// mergedCluster will return the result of merging AtlasClusterSpec with Atlas Cluster -func mergedCluster(cluster mongodbatlas.Cluster, spec mdbv1.AtlasClusterSpec) (result mongodbatlas.Cluster, err error) { - if err = compat.JSONCopy(&result, cluster); err != nil { +// MergedCluster will return the result of merging AtlasClusterSpec with Atlas Cluster +func MergedCluster(atlasCluster mongodbatlas.Cluster, spec mdbv1.AtlasClusterSpec) (result mongodbatlas.Cluster, err error) { + if err = compat.JSONCopy(&result, atlasCluster); err != nil { return } @@ -110,7 +110,7 @@ func mergedCluster(cluster mongodbatlas.Cluster, spec mdbv1.AtlasClusterSpec) (r } // TODO: might need to do this with other slices - if err = compat.JSONSliceMerge(&result.ReplicationSpecs, cluster.ReplicationSpecs); err != nil { + if err = compat.JSONSliceMerge(&result.ReplicationSpecs, atlasCluster.ReplicationSpecs); err != nil { return } @@ -118,12 +118,21 @@ func mergedCluster(cluster mongodbatlas.Cluster, spec mdbv1.AtlasClusterSpec) (r return } + // According to the docs for 'providerSettings.regionName' (https://docs.atlas.mongodb.com/reference/api/clusters-create-one/): + // "Don't specify this parameter when creating a multi-region cluster using the replicationSpec object or a Global + // Cluster with the replicationSpecs array." + // The problem is that Atlas API accepts the create/update request but then returns the 'ProviderSettings.RegionName' empty in GET request + // So we need to consider this while comparing (to avoid perpetual updates) + if len(result.ReplicationSpecs) > 0 && atlasCluster.ProviderSettings.RegionName == "" { + result.ProviderSettings.RegionName = "" + } + return } -// clustersEqual compares two Atlas Clusters -func clustersEqual(log *zap.SugaredLogger, clusterA mongodbatlas.Cluster, clusterB mongodbatlas.Cluster) bool { - d := cmp.Diff(clusterA, clusterB, cmpopts.EquateEmpty()) +// ClustersEqual compares two Atlas Clusters +func ClustersEqual(log *zap.SugaredLogger, clusterAtlas mongodbatlas.Cluster, clusterOperator mongodbatlas.Cluster) bool { + d := cmp.Diff(clusterAtlas, clusterOperator, cmpopts.EquateEmpty()) if d != "" { log.Debugf("Clusters are different: %s", d) } diff --git a/pkg/controller/atlascluster/cluster_test.go b/pkg/controller/atlascluster/cluster_test.go index c7de050d27..effc71fce9 100644 --- a/pkg/controller/atlascluster/cluster_test.go +++ b/pkg/controller/atlascluster/cluster_test.go @@ -25,21 +25,54 @@ func TestClusterMatchesSpec(t *testing.T) { ClusterType: mdbv1.TypeGeoSharded, } - merged, err := mergedCluster(atlasCluster, operatorCluster) + merged, err := MergedCluster(atlasCluster, operatorCluster) assert.NoError(t, err) - equal := clustersEqual(zap.S(), atlasCluster, merged) + equal := ClustersEqual(zap.S(), atlasCluster, merged) assert.True(t, equal) }) + t.Run("Clusters match (ProviderSettings.RegionName ignored)", func(t *testing.T) { + common := mdbv1.DefaultAWSCluster("test-ns", "project-name") + // Note, that in reality it seems that Atlas nullifies ProviderSettings.RegionName only if RegionsConfig are specified + // but it's ok not to overcomplicate + common.Spec.ReplicationSpecs = append(common.Spec.ReplicationSpecs, mdbv1.ReplicationSpec{ + NumShards: int64ptr(2), + }) + // Emulating Atlas behavior when it nullifies the ProviderSettings.RegionName + atlasCluster, err := common.DeepCopy().WithRegionName("").Spec.Cluster() + assert.NoError(t, err) + operatorCluster := common.DeepCopy() + + merged, err := MergedCluster(*atlasCluster, operatorCluster.Spec) + assert.NoError(t, err) + + equal := ClustersEqual(zap.S(), *atlasCluster, merged) + assert.True(t, equal) + }) + t.Run("Clusters don't match (ProviderSettings.RegionName was changed)", func(t *testing.T) { + atlasCluster, err := mdbv1.DefaultAWSCluster("test-ns", "project-name").WithRegionName("US_WEST_1").Spec.Cluster() + assert.NoError(t, err) + // RegionName has changed and no ReplicationSpecs are specified (meaning ProviderSettings.RegionName is mandatory) + operatorCluster := mdbv1.DefaultAWSCluster("test-ns", "project-name").WithRegionName("EU_EAST_1") + + merged, err := MergedCluster(*atlasCluster, operatorCluster.Spec) + assert.NoError(t, err) + + equal := ClustersEqual(zap.S(), *atlasCluster, merged) + assert.False(t, equal) + }) t.Run("Clusters don't match (enums)", func(t *testing.T) { atlasClusterEnum := mongodbatlas.Cluster{ClusterType: "GEOSHARDED"} operatorClusterEnum := mdbv1.AtlasClusterSpec{ClusterType: mdbv1.TypeReplicaSet} - merged, err := mergedCluster(atlasClusterEnum, operatorClusterEnum) + merged, err := MergedCluster(atlasClusterEnum, operatorClusterEnum) assert.NoError(t, err) - equal := clustersEqual(zap.S(), atlasClusterEnum, merged) + equal := ClustersEqual(zap.S(), atlasClusterEnum, merged) assert.False(t, equal) }) } +func int64ptr(i int64) *int64 { + return &i +} diff --git a/test/int/cluster_test.go b/test/int/cluster_test.go index 9c727f1a45..c7650cb284 100644 --- a/test/int/cluster_test.go +++ b/test/int/cluster_test.go @@ -9,16 +9,24 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "go.mongodb.org/atlas/mongodbatlas" + "go.uber.org/zap" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" mdbv1 "github.com/mongodb/mongodb-atlas-kubernetes/pkg/api/v1" "github.com/mongodb/mongodb-atlas-kubernetes/pkg/api/v1/status" + "github.com/mongodb/mongodb-atlas-kubernetes/pkg/controller/atlascluster" "github.com/mongodb/mongodb-atlas-kubernetes/pkg/controller/workflow" "github.com/mongodb/mongodb-atlas-kubernetes/pkg/util/kube" "github.com/mongodb/mongodb-atlas-kubernetes/pkg/util/testutil" ) +const ( + // Set this to true if you are debugging cluster creation. + // This may not help much if there was the update though... + ClusterDevMode = false +) + var _ = Describe("AtlasCluster", func() { const interval = time.Second * 1 @@ -47,6 +55,10 @@ var _ = Describe("AtlasCluster", func() { Expect(k8sClient.Create(context.Background(), &connectionSecret)).To(Succeed()) createdProject = mdbv1.DefaultProject(namespace.Name, connectionSecret.Name) + if ClusterDevMode { + // While developing tests we need to reuse the same project + createdProject.Spec.Name = "dev-test atlas-project" + } By("Creating the project " + createdProject.Name) Expect(k8sClient.Create(context.Background(), createdProject)).To(Succeed()) Eventually(testutil.WaitFor(k8sClient, createdProject, status.TrueCondition(status.ReadyType)), @@ -54,6 +66,9 @@ var _ = Describe("AtlasCluster", func() { }) AfterEach(func() { + if ClusterDevMode { + return + } if createdProject != nil && createdProject.Status.ID != "" { if createdCluster != nil { By("Removing Atlas Cluster " + createdCluster.Name) @@ -68,14 +83,18 @@ var _ = Describe("AtlasCluster", func() { removeControllersAndNamespace() }) - doCommonChecks := func() { + doCommonStatusChecks := func() { By("Checking observed Cluster state", func() { + atlasCluster, _, err := atlasClient.Clusters.Get(context.Background(), createdProject.Status.ID, createdCluster.Spec.Name) + Expect(err).ToNot(HaveOccurred()) + Expect(createdCluster.Status.ConnectionStrings).NotTo(BeNil()) - Expect(createdCluster.Status.ConnectionStrings.Standard).NotTo(BeNil()) - Expect(createdCluster.Status.ConnectionStrings.StandardSrv).NotTo(BeNil()) - Expect(createdCluster.Status.MongoDBVersion).NotTo(BeNil()) - Expect(createdCluster.Status.MongoURIUpdated).NotTo(BeNil()) + Expect(createdCluster.Status.ConnectionStrings.Standard).To(Equal(atlasCluster.ConnectionStrings.Standard)) + Expect(createdCluster.Status.ConnectionStrings.StandardSrv).To(Equal(atlasCluster.ConnectionStrings.StandardSrv)) + Expect(createdCluster.Status.MongoDBVersion).To(Equal(atlasCluster.MongoDBVersion)) + Expect(createdCluster.Status.MongoURIUpdated).To(Equal(atlasCluster.MongoURIUpdated)) Expect(createdCluster.Status.StateName).To(Equal("IDLE")) + Expect(createdCluster.Status.Conditions).To(HaveLen(2)) Expect(createdCluster.Status.Conditions).To(ConsistOf(testutil.MatchConditions( status.TrueCondition(status.ClusterReadyType), status.TrueCondition(status.ReadyType), @@ -90,14 +109,10 @@ var _ = Describe("AtlasCluster", func() { atlasCluster, _, err := atlasClient.Clusters.Get(context.Background(), createdProject.Status.ID, createdCluster.Spec.Name) Expect(err).ToNot(HaveOccurred()) - createdAtlasCluster, err := createdCluster.Spec.Cluster() + mergedCluster, err := atlascluster.MergedCluster(*atlasCluster, createdCluster.Spec) Expect(err).ToNot(HaveOccurred()) - Expect(atlasCluster.Name).To(Equal(createdAtlasCluster.Name)) - Expect(atlasCluster.Labels).To(ConsistOf(createdAtlasCluster.Labels)) - Expect(atlasCluster.ProviderSettings.InstanceSizeName).To(Equal(createdAtlasCluster.ProviderSettings.InstanceSizeName)) - Expect(atlasCluster.ProviderSettings.ProviderName).To(Equal(createdAtlasCluster.ProviderSettings.ProviderName)) - Expect(atlasCluster.ProviderSettings.RegionName).To(Equal(createdAtlasCluster.ProviderSettings.RegionName)) + Expect(atlascluster.ClustersEqual(zap.S(), *atlasCluster, mergedCluster)).To(BeTrue()) for _, check := range additionalChecks { check(atlasCluster) @@ -125,7 +140,7 @@ var _ = Describe("AtlasCluster", func() { Eventually(testutil.WaitFor(k8sClient, createdCluster, status.TrueCondition(status.ReadyType), validateClusterCreatingFunc()), 30*time.Minute, interval).Should(BeTrue()) - doCommonChecks() + doCommonStatusChecks() checkAtlasState() }) @@ -135,7 +150,7 @@ var _ = Describe("AtlasCluster", func() { }) createdCluster.Spec.ClusterType = "SHARDED" performUpdate(40 * time.Minute) - doCommonChecks() + doCommonStatusChecks() checkAtlasState() }) }) @@ -152,14 +167,14 @@ var _ = Describe("AtlasCluster", func() { Eventually(testutil.WaitFor(k8sClient, createdCluster, status.TrueCondition(status.ReadyType), validateClusterCreatingFunc()), 1800, interval).Should(BeTrue()) - doCommonChecks() + doCommonStatusChecks() checkAtlasState() }) By("Increasing InstanceSize", func() { createdCluster.Spec.ProviderSettings.InstanceSizeName = "M30" performUpdate(40 * time.Minute) - doCommonChecks() + doCommonStatusChecks() checkAtlasState() }) }) @@ -176,13 +191,12 @@ var _ = Describe("AtlasCluster", func() { Eventually(testutil.WaitFor(k8sClient, createdCluster, status.TrueCondition(status.ReadyType), validateClusterCreatingFunc()), 1800, interval).Should(BeTrue()) - doCommonChecks() + doCommonStatusChecks() checkAtlasState() }) By("Change cluster to GEOSHARDED", func() { createdCluster.Spec.ClusterType = "GEOSHARDED" - createdCluster.Spec.ProviderSettings.RegionName = "" createdCluster.Spec.ReplicationSpecs = []mdbv1.ReplicationSpec{ { NumShards: int64ptr(1), @@ -204,7 +218,46 @@ var _ = Describe("AtlasCluster", func() { }, } performUpdate(80 * time.Minute) - doCommonChecks() + doCommonStatusChecks() + checkAtlasState() + }) + }) + }) + + Describe("Create/Update the cluster (more complex scenario)", func() { + It("Should be created", func() { + createdCluster = mdbv1.DefaultAWSCluster(namespace.Name, createdProject.Name) + createdCluster.Spec.ClusterType = mdbv1.TypeReplicaSet + createdCluster.Spec.AutoScaling = &mdbv1.AutoScalingSpec{ + Compute: &mdbv1.ComputeSpec{ + Enabled: boolptr(true), + ScaleDownEnabled: boolptr(true), + }, + } + createdCluster.Spec.ProviderSettings.AutoScaling = &mdbv1.AutoScalingSpec{ + Compute: &mdbv1.ComputeSpec{ + MaxInstanceSize: "M20", + MinInstanceSize: "M10", + }, + } + createdCluster.Spec.ProviderSettings.InstanceSizeName = "M10" + createdCluster.Spec.Labels = []mdbv1.LabelSpec{{Key: "createdBy", Value: "Atlas Operator"}} + createdCluster.Spec.ReplicationSpecs = []mdbv1.ReplicationSpec{{ + NumShards: int64ptr(1), + ZoneName: "Zone 1", + // One interesting thing: if the regionsConfig is not empty - Atlas nullifies the 'providerSettings.regionName' field + RegionsConfig: map[string]mdbv1.RegionsConfig{ + "US_EAST_1": {AnalyticsNodes: int64ptr(0), ElectableNodes: int64ptr(1), Priority: int64ptr(6), ReadOnlyNodes: int64ptr(0)}, + "US_WEST_2": {AnalyticsNodes: int64ptr(0), ElectableNodes: int64ptr(2), Priority: int64ptr(7), ReadOnlyNodes: int64ptr(0)}, + }}} + + By(fmt.Sprintf("Creating the Cluster %s", kube.ObjectKeyFromObject(createdCluster)), func() { + Expect(k8sClient.Create(context.Background(), createdCluster)).To(Succeed()) + + Eventually(testutil.WaitFor(k8sClient, createdCluster, status.TrueCondition(status.ReadyType), validateClusterCreatingFunc()), + 1800, interval).Should(BeTrue()) + + doCommonStatusChecks() checkAtlasState() }) }) @@ -241,7 +294,7 @@ var _ = Describe("AtlasCluster", func() { Eventually(testutil.WaitFor(k8sClient, createdCluster, status.TrueCondition(status.ReadyType)), 20*time.Minute, interval).Should(BeTrue()) - doCommonChecks() + doCommonStatusChecks() checkAtlasState() }) }) @@ -255,21 +308,21 @@ var _ = Describe("AtlasCluster", func() { Eventually(testutil.WaitFor(k8sClient, createdCluster, status.TrueCondition(status.ReadyType), validateClusterCreatingFunc()), 1800, interval).Should(BeTrue()) - doCommonChecks() + doCommonStatusChecks() checkAtlasState() }) By("Updating the Cluster labels", func() { createdCluster.Spec.Labels = []mdbv1.LabelSpec{{Key: "int-test", Value: "true"}} performUpdate(20 * time.Minute) - doCommonChecks() + doCommonStatusChecks() checkAtlasState() }) By("Updating the Cluster backups settings", func() { createdCluster.Spec.ProviderBackupEnabled = boolptr(true) performUpdate(20 * time.Minute) - doCommonChecks() + doCommonStatusChecks() checkAtlasState(func(c *mongodbatlas.Cluster) { Expect(c.ProviderBackupEnabled).To(Equal(createdCluster.Spec.ProviderBackupEnabled)) }) @@ -278,7 +331,7 @@ var _ = Describe("AtlasCluster", func() { By("Decreasing the Cluster disk size", func() { createdCluster.Spec.DiskSizeGB = intptr(10) performUpdate(20 * time.Minute) - doCommonChecks() + doCommonStatusChecks() checkAtlasState(func(c *mongodbatlas.Cluster) { Expect(*c.DiskSizeGB).To(BeEquivalentTo(*createdCluster.Spec.DiskSizeGB)) @@ -290,7 +343,7 @@ var _ = Describe("AtlasCluster", func() { By("Pausing the cluster", func() { createdCluster.Spec.Paused = boolptr(true) performUpdate(20 * time.Minute) - doCommonChecks() + doCommonStatusChecks() checkAtlasState(func(c *mongodbatlas.Cluster) { Expect(c.Paused).To(Equal(createdCluster.Spec.Paused)) }) @@ -319,14 +372,14 @@ var _ = Describe("AtlasCluster", func() { By("Unpausing the cluster", func() { createdCluster.Spec.Paused = boolptr(false) performUpdate(20 * time.Minute) - doCommonChecks() + doCommonStatusChecks() checkAtlasState(func(c *mongodbatlas.Cluster) { Expect(c.Paused).To(Equal(createdCluster.Spec.Paused)) }) }) By("Checking that modifications were applied after unpausing", func() { - doCommonChecks() + doCommonStatusChecks() checkAtlasState(func(c *mongodbatlas.Cluster) { Expect(c.ProviderBackupEnabled).To(Equal(createdCluster.Spec.ProviderBackupEnabled)) }) @@ -358,7 +411,7 @@ var _ = Describe("AtlasCluster", func() { By("Fixing the Cluster", func() { createdCluster.Spec.ProviderSettings.AutoScaling = nil performUpdate(20 * time.Minute) - doCommonChecks() + doCommonStatusChecks() checkAtlasState() }) }) @@ -386,7 +439,7 @@ var _ = Describe("AtlasCluster", func() { By("Fixing the Cluster", func() { createdCluster.Spec.ProviderSettings.InstanceSizeName = oldSizeName performUpdate(20 * time.Minute) - doCommonChecks() + doCommonStatusChecks() checkAtlasState() }) }) From c45addab577d647c742cb7ae438a445d1fd391ff Mon Sep 17 00:00:00 2001 From: antonlisovenko Date: Fri, 26 Mar 2021 09:19:39 +0000 Subject: [PATCH 2/3] extending project timeout --- test/int/cluster_test.go | 2 +- test/int/dbuser_test.go | 2 +- test/int/project_test.go | 46 ++++++++++++++++++++++------------------ 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/test/int/cluster_test.go b/test/int/cluster_test.go index c7650cb284..60d4592cea 100644 --- a/test/int/cluster_test.go +++ b/test/int/cluster_test.go @@ -62,7 +62,7 @@ var _ = Describe("AtlasCluster", func() { By("Creating the project " + createdProject.Name) Expect(k8sClient.Create(context.Background(), createdProject)).To(Succeed()) Eventually(testutil.WaitFor(k8sClient, createdProject, status.TrueCondition(status.ReadyType)), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) }) AfterEach(func() { diff --git a/test/int/dbuser_test.go b/test/int/dbuser_test.go index c7bade7244..27884336f7 100644 --- a/test/int/dbuser_test.go +++ b/test/int/dbuser_test.go @@ -78,7 +78,7 @@ var _ = Describe("AtlasDatabaseUser", func() { Expect(k8sClient.Create(context.Background(), createdProject)).To(Succeed()) Eventually(testutil.WaitFor(k8sClient, createdProject, status.TrueCondition(status.ReadyType)), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) }) }) diff --git a/test/int/project_test.go b/test/int/project_test.go index 5d0d6de5e3..5cbb4fbd9e 100644 --- a/test/int/project_test.go +++ b/test/int/project_test.go @@ -22,6 +22,10 @@ import ( "github.com/mongodb/mongodb-atlas-kubernetes/pkg/util/testutil" ) +const ( + ProjectCreationTimeout = 30 +) + var _ = Describe("AtlasProject", func() { const interval = time.Second * 1 @@ -83,7 +87,7 @@ var _ = Describe("AtlasProject", func() { Expect(k8sClient.Create(context.Background(), expectedProject)).ToNot(HaveOccurred()) Eventually(testutil.WaitFor(k8sClient, createdProject, status.TrueCondition(status.ReadyType)), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) checkAtlasProjectIsReady() @@ -100,7 +104,7 @@ var _ = Describe("AtlasProject", func() { expectedCondition := status.FalseCondition(status.ProjectReadyType).WithReason(string(workflow.AtlasCredentialsNotProvided)) Eventually(testutil.WaitFor(k8sClient, createdProject, expectedCondition), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) expectedConditionsMatchers := testutil.MatchConditions( status.FalseCondition(status.ProjectReadyType), @@ -129,7 +133,7 @@ var _ = Describe("AtlasProject", func() { Expect(k8sClient.Create(context.Background(), expectedProject)).ToNot(HaveOccurred()) Eventually(testutil.WaitFor(k8sClient, createdProject, status.TrueCondition(status.ReadyType)), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) // Updating (the existing project is expected to be read from Atlas) By("Updating the project") @@ -138,7 +142,7 @@ var _ = Describe("AtlasProject", func() { Expect(k8sClient.Update(context.Background(), createdProject)).To(Succeed()) Eventually(testutil.WaitFor(k8sClient, createdProject, status.TrueCondition(status.ReadyType)), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) Expect(testutil.ReadAtlasResource(k8sClient, createdProject)).To(BeTrue()) Expect(createdProject.Status.Conditions).To(ContainElement(testutil.MatchCondition(status.TrueCondition(status.ProjectReadyType)))) @@ -168,10 +172,10 @@ var _ = Describe("AtlasProject", func() { Expect(k8sClient.Create(context.Background(), secondProject)).ToNot(HaveOccurred()) Eventually(testutil.WaitFor(k8sClient, createdProject, status.TrueCondition(status.ReadyType)), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) Eventually(testutil.WaitFor(k8sClient, secondProject, status.TrueCondition(status.ReadyType)), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) }) By("Breaking the Connection Secret", func() { connectionSecret = buildConnectionSecret("my-atlas-key") @@ -181,9 +185,9 @@ var _ = Describe("AtlasProject", func() { // Both projects are expected to get to Failed state right away expectedCondition := status.FalseCondition(status.ProjectReadyType).WithReason(string(workflow.ProjectNotCreatedInAtlas)) Eventually(testutil.WaitFor(k8sClient, createdProject, expectedCondition), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) Eventually(testutil.WaitFor(k8sClient, secondProject, expectedCondition), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) }) By("Fixing the Connection Secret", func() { connectionSecret = buildConnectionSecret("my-atlas-key") @@ -191,9 +195,9 @@ var _ = Describe("AtlasProject", func() { // Both projects are expected to recover Eventually(testutil.WaitFor(k8sClient, createdProject, status.TrueCondition(status.ReadyType)), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) Eventually(testutil.WaitFor(k8sClient, secondProject, status.TrueCondition(status.ReadyType)), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) }) }) }) @@ -205,7 +209,7 @@ var _ = Describe("AtlasProject", func() { Expect(k8sClient.Create(context.Background(), createdProject)).ToNot(HaveOccurred()) Eventually(testutil.WaitFor(k8sClient, createdProject, status.TrueCondition(status.ReadyType), validateNoErrorsIPAccessListDuringCreate), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) checkAtlasProjectIsReady() checkExpiredAccessLists([]project.IPAccessList{}) @@ -220,7 +224,7 @@ var _ = Describe("AtlasProject", func() { Expect(k8sClient.Create(context.Background(), createdProject)).ToNot(HaveOccurred()) Eventually(testutil.WaitFor(k8sClient, createdProject, status.TrueCondition(status.ReadyType), validateNoErrorsIPAccessListDuringCreate), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) checkAtlasProjectIsReady() checkExpiredAccessLists([]project.IPAccessList{}) @@ -236,7 +240,7 @@ var _ = Describe("AtlasProject", func() { Expect(k8sClient.Create(context.Background(), createdProject)).ToNot(HaveOccurred()) Eventually(testutil.WaitFor(k8sClient, createdProject, status.TrueCondition(status.ReadyType), validateNoErrorsIPAccessListDuringCreate), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) checkAtlasProjectIsReady() checkExpiredAccessLists([]project.IPAccessList{expiredList}) @@ -255,7 +259,7 @@ var _ = Describe("AtlasProject", func() { Expect(k8sClient.Create(context.Background(), createdProject)).ToNot(HaveOccurred()) Eventually(testutil.WaitFor(k8sClient, createdProject, status.FalseCondition(status.IPAccessListReadyType)), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) ipAccessFailedCondition := status.FalseCondition(status.IPAccessListReadyType). WithReason(string(workflow.ProjectIPNotCreatedInAtlas)). @@ -281,7 +285,7 @@ var _ = Describe("AtlasProject", func() { Expect(k8sClient.Create(context.Background(), createdProject)).To(Succeed()) Eventually(testutil.WaitFor(k8sClient, createdProject, status.TrueCondition(status.ReadyType), validateNoErrorsIPAccessListDuringCreate), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) }) By("Updating the IP Access List comment and delete date", func() { // Just a note: Atlas doesn't allow to make the "permanent" entity "temporary". But it works the other way @@ -290,7 +294,7 @@ var _ = Describe("AtlasProject", func() { Expect(k8sClient.Update(context.Background(), createdProject)).To(Succeed()) Eventually(testutil.WaitFor(k8sClient, createdProject, status.TrueCondition(status.ReadyType), validateNoErrorsIPAccessListDuringUpdate), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) checkAtlasProjectIsReady() checkExpiredAccessLists([]project.IPAccessList{}) @@ -308,7 +312,7 @@ var _ = Describe("AtlasProject", func() { Expect(k8sClient.Create(context.Background(), createdProject)).ToNot(HaveOccurred()) Eventually(testutil.WaitFor(k8sClient, createdProject, status.TrueCondition(status.ReadyType), validateNoErrorsIPAccessListDuringCreate), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) }) By("Updating the IP Access List IPAddress", func() { twoDaysLater := time.Now().Add(time.Hour * 48).Format("2006-01-02T15:04:05Z") @@ -319,7 +323,7 @@ var _ = Describe("AtlasProject", func() { Expect(k8sClient.Update(context.Background(), createdProject)).To(Succeed()) Eventually(testutil.WaitFor(k8sClient, createdProject, status.TrueCondition(status.ReadyType), validateNoErrorsIPAccessListDuringUpdate), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) checkAtlasProjectIsReady() checkExpiredAccessLists([]project.IPAccessList{}) @@ -339,7 +343,7 @@ var _ = Describe("AtlasProject", func() { Expect(k8sClient.Create(context.Background(), createdProject)).To(Succeed()) Eventually(testutil.WaitFor(k8sClient, createdProject, status.TrueCondition(status.ReadyType)), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) expectedConditionsMatchers := testutil.MatchConditions( status.TrueCondition(status.ProjectReadyType), @@ -356,7 +360,7 @@ var _ = Describe("AtlasProject", func() { Expect(k8sClient.Create(context.Background(), createdProject)).ToNot(HaveOccurred()) Eventually(testutil.WaitFor(k8sClient, createdProject, status.FalseCondition(status.ReadyType)), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) expectedConditionsMatchers := testutil.MatchConditions( status.FalseCondition(status.ProjectReadyType).WithReason(string(workflow.AtlasCredentialsNotProvided)), @@ -371,7 +375,7 @@ var _ = Describe("AtlasProject", func() { Expect(k8sClient.Create(context.Background(), &globalConnectionSecret)).To(Succeed()) Eventually(testutil.WaitFor(k8sClient, createdProject, status.TrueCondition(status.ReadyType)), - 20, interval).Should(BeTrue()) + ProjectCreationTimeout, interval).Should(BeTrue()) }) }) }) From 0c3b6ae4112865649704ae25ced006e5b2de0005 Mon Sep 17 00:00:00 2001 From: antonlisovenko Date: Fri, 26 Mar 2021 17:00:48 +0000 Subject: [PATCH 3/3] wip --- pkg/controller/atlascluster/cluster_test.go | 58 +++++++++++++++++++-- test/int/cluster_test.go | 46 +++++++++++++--- 2 files changed, 91 insertions(+), 13 deletions(-) diff --git a/pkg/controller/atlascluster/cluster_test.go b/pkg/controller/atlascluster/cluster_test.go index effc71fce9..d9e9c91947 100644 --- a/pkg/controller/atlascluster/cluster_test.go +++ b/pkg/controller/atlascluster/cluster_test.go @@ -31,6 +31,16 @@ func TestClusterMatchesSpec(t *testing.T) { equal := ClustersEqual(zap.S(), atlasCluster, merged) assert.True(t, equal) }) + t.Run("Clusters don't match (enums)", func(t *testing.T) { + atlasClusterEnum := mongodbatlas.Cluster{ClusterType: "GEOSHARDED"} + operatorClusterEnum := mdbv1.AtlasClusterSpec{ClusterType: mdbv1.TypeReplicaSet} + + merged, err := MergedCluster(atlasClusterEnum, operatorClusterEnum) + assert.NoError(t, err) + + equal := ClustersEqual(zap.S(), atlasClusterEnum, merged) + assert.False(t, equal) + }) t.Run("Clusters match (ProviderSettings.RegionName ignored)", func(t *testing.T) { common := mdbv1.DefaultAWSCluster("test-ns", "project-name") // Note, that in reality it seems that Atlas nullifies ProviderSettings.RegionName only if RegionsConfig are specified @@ -61,15 +71,53 @@ func TestClusterMatchesSpec(t *testing.T) { equal := ClustersEqual(zap.S(), *atlasCluster, merged) assert.False(t, equal) }) + t.Run("Clusters match when Atlas adds default ReplicationSpecs", func(t *testing.T) { + atlasCluster, err := mdbv1.DefaultAWSCluster("test-ns", "project-name").Spec.Cluster() + assert.NoError(t, err) + atlasCluster.ReplicationSpecs = []mongodbatlas.ReplicationSpec{{ + ID: "id", + NumShards: int64ptr(1), + ZoneName: "zone1", + RegionsConfig: map[string]mongodbatlas.RegionsConfig{ + "US_EAST": {AnalyticsNodes: int64ptr(0), ElectableNodes: int64ptr(3), Priority: int64ptr(7), ReadOnlyNodes: int64ptr(0)}}}, + } + operatorCluster := mdbv1.DefaultAWSCluster("test-ns", "project-name") - t.Run("Clusters don't match (enums)", func(t *testing.T) { - atlasClusterEnum := mongodbatlas.Cluster{ClusterType: "GEOSHARDED"} - operatorClusterEnum := mdbv1.AtlasClusterSpec{ClusterType: mdbv1.TypeReplicaSet} + merged, err := MergedCluster(*atlasCluster, operatorCluster.Spec) + assert.NoError(t, err) - merged, err := MergedCluster(atlasClusterEnum, operatorClusterEnum) + equal := ClustersEqual(zap.S(), *atlasCluster, merged) + assert.True(t, equal) + }) + t.Run("Clusters don't match when Atlas adds default ReplicationSpecs and Operator overrides something", func(t *testing.T) { + atlasCluster, err := mdbv1.DefaultAWSCluster("test-ns", "project-name").Spec.Cluster() assert.NoError(t, err) + atlasCluster.ReplicationSpecs = []mongodbatlas.ReplicationSpec{{ + ID: "id", + NumShards: int64ptr(1), + ZoneName: "zone1", + RegionsConfig: map[string]mongodbatlas.RegionsConfig{ + "US_EAST": {AnalyticsNodes: int64ptr(0), ElectableNodes: int64ptr(3), Priority: int64ptr(7), ReadOnlyNodes: int64ptr(0)}}}, + } + operatorCluster := mdbv1.DefaultAWSCluster("test-ns", "project-name") + operatorCluster.Spec.ReplicationSpecs = []mdbv1.ReplicationSpec{{ + NumShards: int64ptr(2), + ZoneName: "zone5", + }} - equal := ClustersEqual(zap.S(), atlasClusterEnum, merged) + merged, err := MergedCluster(*atlasCluster, operatorCluster.Spec) + assert.NoError(t, err) + + expectedReplicationSpecs := []mongodbatlas.ReplicationSpec{{ + ID: "id", + NumShards: int64ptr(2), + ZoneName: "zone5", + RegionsConfig: map[string]mongodbatlas.RegionsConfig{ + "US_EAST": {AnalyticsNodes: int64ptr(0), ElectableNodes: int64ptr(3), Priority: int64ptr(7), ReadOnlyNodes: int64ptr(0)}}}, + } + assert.Equal(t, expectedReplicationSpecs, merged.ReplicationSpecs) + + equal := ClustersEqual(zap.S(), *atlasCluster, merged) assert.False(t, equal) }) } diff --git a/test/int/cluster_test.go b/test/int/cluster_test.go index 60d4592cea..479e84bc32 100644 --- a/test/int/cluster_test.go +++ b/test/int/cluster_test.go @@ -131,17 +131,29 @@ var _ = Describe("AtlasCluster", func() { Describe("Create cluster & change ReplicationSpecs", func() { It("Should Succeed", func() { - expectedCluster := mdbv1.DefaultGCPCluster(namespace.Name, createdProject.Name) + createdCluster = mdbv1.DefaultGCPCluster(namespace.Name, createdProject.Name) - By(fmt.Sprintf("Creating the Cluster %s", kube.ObjectKeyFromObject(expectedCluster)), func() { - createdCluster.ObjectMeta = expectedCluster.ObjectMeta - Expect(k8sClient.Create(context.Background(), expectedCluster)).ToNot(HaveOccurred()) + // Atlas will add some defaults in case the Atlas Operator doesn't set them + replicationSpecsCheck := func(cluster *mongodbatlas.Cluster) { + Expect(cluster.ReplicationSpecs).To(HaveLen(1)) + Expect(cluster.ReplicationSpecs[0].ID).NotTo(BeNil()) + Expect(cluster.ReplicationSpecs[0].ZoneName).To(Equal("Zone 1")) + Expect(cluster.ReplicationSpecs[0].RegionsConfig).To(HaveLen(1)) + Expect(cluster.ReplicationSpecs[0].RegionsConfig[createdCluster.Spec.ProviderSettings.RegionName]).NotTo(BeNil()) + } + + By(fmt.Sprintf("Creating the Cluster %s", kube.ObjectKeyFromObject(createdCluster)), func() { + Expect(k8sClient.Create(context.Background(), createdCluster)).ToNot(HaveOccurred()) Eventually(testutil.WaitFor(k8sClient, createdCluster, status.TrueCondition(status.ReadyType), validateClusterCreatingFunc()), 30*time.Minute, interval).Should(BeTrue()) doCommonStatusChecks() - checkAtlasState() + + singleNumShard := func(cluster *mongodbatlas.Cluster) { + Expect(cluster.ReplicationSpecs[0].NumShards).To(Equal(int64ptr(1))) + } + checkAtlasState(replicationSpecsCheck, singleNumShard) }) By("Updating ReplicationSpecs", func() { @@ -149,9 +161,15 @@ var _ = Describe("AtlasCluster", func() { NumShards: int64ptr(2), }) createdCluster.Spec.ClusterType = "SHARDED" + performUpdate(40 * time.Minute) doCommonStatusChecks() - checkAtlasState() + + twoNumShard := func(cluster *mongodbatlas.Cluster) { + Expect(cluster.ReplicationSpecs[0].NumShards).To(Equal(int64ptr(2))) + } + // ReplicationSpecs has the same defaults but the number of shards has changed + checkAtlasState(replicationSpecsCheck, twoNumShard) }) }) }) @@ -251,14 +269,26 @@ var _ = Describe("AtlasCluster", func() { "US_WEST_2": {AnalyticsNodes: int64ptr(0), ElectableNodes: int64ptr(2), Priority: int64ptr(7), ReadOnlyNodes: int64ptr(0)}, }}} - By(fmt.Sprintf("Creating the Cluster %s", kube.ObjectKeyFromObject(createdCluster)), func() { + replicationSpecsCheckFunc := func(c *mongodbatlas.Cluster) { + cluster, err := createdCluster.Spec.Cluster() + Expect(err).NotTo(HaveOccurred()) + expectedReplicationSpecs := cluster.ReplicationSpecs + + // The ID field is added by Atlas - we don't have it in our specs + Expect(c.ReplicationSpecs[0].ID).NotTo(BeNil()) + c.ReplicationSpecs[0].ID = "" + // Apart from 'ID' all other fields are equal to the ones sent by the Operator + Expect(c.ReplicationSpecs).To(Equal(expectedReplicationSpecs)) + } + By("Creating the Cluster", func() { Expect(k8sClient.Create(context.Background(), createdCluster)).To(Succeed()) Eventually(testutil.WaitFor(k8sClient, createdCluster, status.TrueCondition(status.ReadyType), validateClusterCreatingFunc()), 1800, interval).Should(BeTrue()) doCommonStatusChecks() - checkAtlasState() + + checkAtlasState(replicationSpecsCheckFunc) }) }) })