Skip to content

CLOUDP-83733: more tests on clusters (create only) #185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
antonlisovenko merged 3 commits into from
Mar 29, 2021
Merged

CLOUDP-83733: more tests on clusters (create only) #185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
019251b
CLOUDP-83733: more tests on clusters
Mar 25, 2021
c45adda
extending project timeout
Mar 26, 2021
0c3b6ae
wip
Mar 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 36 additions & 27 deletions pkg/controller/atlascluster/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor Author

@antonlisovenko antonlisovenko Mar 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did some renamings of variables as it was a bit hard to follow which cluster is from Atlas and which - from the Operator later when we did merging magic

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{
Expand All @@ -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))
}
}

Expand All @@ -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) {
Copy link
Contributor Author

@antonlisovenko antonlisovenko Mar 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made public to use inside integration tests

if err = compat.JSONCopy(&result, atlasCluster); err != nil {
return
}

Expand All @@ -110,20 +110,29 @@ 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
}

if err = compat.JSONSliceMerge(&result.ReplicationSpecs, spec.ReplicationSpecs); err != nil {
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)
}
Expand Down
91 changes: 86 additions & 5 deletions pkg/controller/atlascluster/cluster_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,102 @@ 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 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)
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
// 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 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")

merged, err := MergedCluster(*atlasCluster, operatorCluster.Spec)
assert.NoError(t, err)

equal := clustersEqual(zap.S(), atlasClusterEnum, merged)
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",
}}

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)
})
}
func int64ptr(i int64) *int64 {
return &i
}
Loading