Skip to content

Commit 1389eb8

Browse files
author
Anton
authored
CLOUDP-83733: more tests on clusters (create only) (#185)
1 parent 705481e commit 1389eb8

File tree

5 files changed

+266
-89
lines changed

5 files changed

+266
-89
lines changed

‎pkg/controller/atlascluster/cluster.go‎

Lines changed: 36 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -15,42 +15,42 @@ import (
1515
"github.com/mongodb/mongodb-atlas-kubernetes/pkg/util/compat"
1616
)
1717

18-
func (r *AtlasClusterReconciler) ensureClusterState(ctx *workflow.Context, project *mdbv1.AtlasProject, cluster *mdbv1.AtlasCluster) (c *mongodbatlas.Cluster, _ workflow.Result) {
19-
c, resp, err := ctx.Client.Clusters.Get(context.Background(), project.Status.ID, cluster.Spec.Name)
18+
func (r *AtlasClusterReconciler) ensureClusterState(ctx *workflow.Context, project *mdbv1.AtlasProject, cluster *mdbv1.AtlasCluster) (atlasCluster *mongodbatlas.Cluster, _ workflow.Result) {
19+
atlasCluster, resp, err := ctx.Client.Clusters.Get(context.Background(), project.Status.ID, cluster.Spec.Name)
2020
if err != nil {
2121
if resp == nil {
22-
return c, workflow.Terminate(workflow.Internal, err.Error())
22+
return atlasCluster, workflow.Terminate(workflow.Internal, err.Error())
2323
}
2424

2525
if resp.StatusCode != http.StatusNotFound {
26-
return c, workflow.Terminate(workflow.ClusterNotCreatedInAtlas, err.Error())
26+
return atlasCluster, workflow.Terminate(workflow.ClusterNotCreatedInAtlas, err.Error())
2727
}
2828

29-
c, err = cluster.Spec.Cluster()
29+
atlasCluster, err = cluster.Spec.Cluster()
3030
if err != nil {
31-
return c, workflow.Terminate(workflow.Internal, err.Error())
31+
return atlasCluster, workflow.Terminate(workflow.Internal, err.Error())
3232
}
3333

3434
ctx.Log.Infof("Cluster %s doesn't exist in Atlas - creating", cluster.Spec.Name)
35-
c, _, err = ctx.Client.Clusters.Create(context.Background(), project.Status.ID, c)
35+
atlasCluster, _, err = ctx.Client.Clusters.Create(context.Background(), project.Status.ID, atlasCluster)
3636
if err != nil {
37-
return c, workflow.Terminate(workflow.ClusterNotCreatedInAtlas, err.Error())
37+
return atlasCluster, workflow.Terminate(workflow.ClusterNotCreatedInAtlas, err.Error())
3838
}
3939
}
4040

41-
switch c.StateName {
41+
switch atlasCluster.StateName {
4242
case "IDLE":
43-
resultingCluster, err := mergedCluster(*c, cluster.Spec)
43+
resultingCluster, err := MergedCluster(*atlasCluster, cluster.Spec)
4444
if err != nil {
45-
return c, workflow.Terminate(workflow.Internal, err.Error())
45+
return atlasCluster, workflow.Terminate(workflow.Internal, err.Error())
4646
}
4747

48-
if done := clustersEqual(ctx.Log, *c, resultingCluster); done {
49-
return c, workflow.OK()
48+
if done := ClustersEqual(ctx.Log, *atlasCluster, resultingCluster); done {
49+
return atlasCluster, workflow.OK()
5050
}
5151

5252
if cluster.Spec.Paused != nil {
53-
if c.Paused == nil || *c.Paused != *cluster.Spec.Paused {
53+
if atlasCluster.Paused == nil || *atlasCluster.Paused != *cluster.Spec.Paused {
5454
// paused is different from Atlas
5555
// we need to first send a special (un)pause request before reconciling everything else
5656
resultingCluster = mongodbatlas.Cluster{
@@ -64,23 +64,23 @@ func (r *AtlasClusterReconciler) ensureClusterState(ctx *workflow.Context, proje
6464

6565
resultingCluster = cleanupCluster(resultingCluster)
6666

67-
c, _, err = ctx.Client.Clusters.Update(context.Background(), project.Status.ID, cluster.Spec.Name, &resultingCluster)
67+
atlasCluster, _, err = ctx.Client.Clusters.Update(context.Background(), project.Status.ID, cluster.Spec.Name, &resultingCluster)
6868
if err != nil {
69-
return c, workflow.Terminate(workflow.ClusterNotUpdatedInAtlas, err.Error())
69+
return atlasCluster, workflow.Terminate(workflow.ClusterNotUpdatedInAtlas, err.Error())
7070
}
7171

72-
return c, workflow.InProgress(workflow.ClusterUpdating, "cluster is updating")
72+
return atlasCluster, workflow.InProgress(workflow.ClusterUpdating, "cluster is updating")
7373

7474
case "CREATING":
75-
return c, workflow.InProgress(workflow.ClusterCreating, "cluster is provisioning")
75+
return atlasCluster, workflow.InProgress(workflow.ClusterCreating, "cluster is provisioning")
7676

7777
case "UPDATING", "REPAIRING":
78-
return c, workflow.InProgress(workflow.ClusterUpdating, "cluster is updating")
78+
return atlasCluster, workflow.InProgress(workflow.ClusterUpdating, "cluster is updating")
7979

8080
// TODO: add "DELETING", "DELETED", handle 404 on delete
8181

8282
default:
83-
return c, workflow.Terminate(workflow.Internal, fmt.Sprintf("unknown cluster state %q", c.StateName))
83+
return atlasCluster, workflow.Terminate(workflow.Internal, fmt.Sprintf("unknown cluster state %q", atlasCluster.StateName))
8484
}
8585
}
8686

@@ -99,9 +99,9 @@ func cleanupCluster(cluster mongodbatlas.Cluster) mongodbatlas.Cluster {
9999
return cluster
100100
}
101101

102-
// mergedCluster will return the result of merging AtlasClusterSpec with Atlas Cluster
103-
func mergedCluster(cluster mongodbatlas.Cluster, spec mdbv1.AtlasClusterSpec) (result mongodbatlas.Cluster, err error) {
104-
if err = compat.JSONCopy(&result, cluster); err != nil {
102+
// MergedCluster will return the result of merging AtlasClusterSpec with Atlas Cluster
103+
func MergedCluster(atlasCluster mongodbatlas.Cluster, spec mdbv1.AtlasClusterSpec) (result mongodbatlas.Cluster, err error) {
104+
if err = compat.JSONCopy(&result, atlasCluster); err != nil {
105105
return
106106
}
107107

@@ -110,20 +110,29 @@ func mergedCluster(cluster mongodbatlas.Cluster, spec mdbv1.AtlasClusterSpec) (r
110110
}
111111

112112
// TODO: might need to do this with other slices
113-
if err = compat.JSONSliceMerge(&result.ReplicationSpecs, cluster.ReplicationSpecs); err != nil {
113+
if err = compat.JSONSliceMerge(&result.ReplicationSpecs, atlasCluster.ReplicationSpecs); err != nil {
114114
return
115115
}
116116

117117
if err = compat.JSONSliceMerge(&result.ReplicationSpecs, spec.ReplicationSpecs); err != nil {
118118
return
119119
}
120120

121+
// According to the docs for 'providerSettings.regionName' (https://docs.atlas.mongodb.com/reference/api/clusters-create-one/):
122+
// "Don't specify this parameter when creating a multi-region cluster using the replicationSpec object or a Global
123+
// Cluster with the replicationSpecs array."
124+
// The problem is that Atlas API accepts the create/update request but then returns the 'ProviderSettings.RegionName' empty in GET request
125+
// So we need to consider this while comparing (to avoid perpetual updates)
126+
if len(result.ReplicationSpecs) > 0 && atlasCluster.ProviderSettings.RegionName == "" {
127+
result.ProviderSettings.RegionName = ""
128+
}
129+
121130
return
122131
}
123132

124-
// clustersEqual compares two Atlas Clusters
125-
func clustersEqual(log *zap.SugaredLogger, clusterA mongodbatlas.Cluster, clusterB mongodbatlas.Cluster) bool {
126-
d := cmp.Diff(clusterA, clusterB, cmpopts.EquateEmpty())
133+
// ClustersEqual compares two Atlas Clusters
134+
func ClustersEqual(log *zap.SugaredLogger, clusterAtlas mongodbatlas.Cluster, clusterOperator mongodbatlas.Cluster) bool {
135+
d := cmp.Diff(clusterAtlas, clusterOperator, cmpopts.EquateEmpty())
127136
if d != "" {
128137
log.Debugf("Clusters are different: %s", d)
129138
}

‎pkg/controller/atlascluster/cluster_test.go‎

Lines changed: 86 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,21 +25,102 @@ func TestClusterMatchesSpec(t *testing.T) {
2525
ClusterType: mdbv1.TypeGeoSharded,
2626
}
2727

28-
merged, err := mergedCluster(atlasCluster, operatorCluster)
28+
merged, err := MergedCluster(atlasCluster, operatorCluster)
2929
assert.NoError(t, err)
3030

31-
equal := clustersEqual(zap.S(), atlasCluster, merged)
31+
equal := ClustersEqual(zap.S(), atlasCluster, merged)
3232
assert.True(t, equal)
3333
})
34-
3534
t.Run("Clusters don't match (enums)", func(t *testing.T) {
3635
atlasClusterEnum := mongodbatlas.Cluster{ClusterType: "GEOSHARDED"}
3736
operatorClusterEnum := mdbv1.AtlasClusterSpec{ClusterType: mdbv1.TypeReplicaSet}
3837

39-
merged, err := mergedCluster(atlasClusterEnum, operatorClusterEnum)
38+
merged, err := MergedCluster(atlasClusterEnum, operatorClusterEnum)
39+
assert.NoError(t, err)
40+
41+
equal := ClustersEqual(zap.S(), atlasClusterEnum, merged)
42+
assert.False(t, equal)
43+
})
44+
t.Run("Clusters match (ProviderSettings.RegionName ignored)", func(t *testing.T) {
45+
common := mdbv1.DefaultAWSCluster("test-ns", "project-name")
46+
// Note, that in reality it seems that Atlas nullifies ProviderSettings.RegionName only if RegionsConfig are specified
47+
// but it's ok not to overcomplicate
48+
common.Spec.ReplicationSpecs = append(common.Spec.ReplicationSpecs, mdbv1.ReplicationSpec{
49+
NumShards: int64ptr(2),
50+
})
51+
// Emulating Atlas behavior when it nullifies the ProviderSettings.RegionName
52+
atlasCluster, err := common.DeepCopy().WithRegionName("").Spec.Cluster()
53+
assert.NoError(t, err)
54+
operatorCluster := common.DeepCopy()
55+
56+
merged, err := MergedCluster(*atlasCluster, operatorCluster.Spec)
57+
assert.NoError(t, err)
58+
59+
equal := ClustersEqual(zap.S(), *atlasCluster, merged)
60+
assert.True(t, equal)
61+
})
62+
t.Run("Clusters don't match (ProviderSettings.RegionName was changed)", func(t *testing.T) {
63+
atlasCluster, err := mdbv1.DefaultAWSCluster("test-ns", "project-name").WithRegionName("US_WEST_1").Spec.Cluster()
64+
assert.NoError(t, err)
65+
// RegionName has changed and no ReplicationSpecs are specified (meaning ProviderSettings.RegionName is mandatory)
66+
operatorCluster := mdbv1.DefaultAWSCluster("test-ns", "project-name").WithRegionName("EU_EAST_1")
67+
68+
merged, err := MergedCluster(*atlasCluster, operatorCluster.Spec)
69+
assert.NoError(t, err)
70+
71+
equal := ClustersEqual(zap.S(), *atlasCluster, merged)
72+
assert.False(t, equal)
73+
})
74+
t.Run("Clusters match when Atlas adds default ReplicationSpecs", func(t *testing.T) {
75+
atlasCluster, err := mdbv1.DefaultAWSCluster("test-ns", "project-name").Spec.Cluster()
76+
assert.NoError(t, err)
77+
atlasCluster.ReplicationSpecs = []mongodbatlas.ReplicationSpec{{
78+
ID: "id",
79+
NumShards: int64ptr(1),
80+
ZoneName: "zone1",
81+
RegionsConfig: map[string]mongodbatlas.RegionsConfig{
82+
"US_EAST": {AnalyticsNodes: int64ptr(0), ElectableNodes: int64ptr(3), Priority: int64ptr(7), ReadOnlyNodes: int64ptr(0)}}},
83+
}
84+
operatorCluster := mdbv1.DefaultAWSCluster("test-ns", "project-name")
85+
86+
merged, err := MergedCluster(*atlasCluster, operatorCluster.Spec)
4087
assert.NoError(t, err)
4188

42-
equal := clustersEqual(zap.S(), atlasClusterEnum, merged)
89+
equal := ClustersEqual(zap.S(), *atlasCluster, merged)
90+
assert.True(t, equal)
91+
})
92+
t.Run("Clusters don't match when Atlas adds default ReplicationSpecs and Operator overrides something", func(t *testing.T) {
93+
atlasCluster, err := mdbv1.DefaultAWSCluster("test-ns", "project-name").Spec.Cluster()
94+
assert.NoError(t, err)
95+
atlasCluster.ReplicationSpecs = []mongodbatlas.ReplicationSpec{{
96+
ID: "id",
97+
NumShards: int64ptr(1),
98+
ZoneName: "zone1",
99+
RegionsConfig: map[string]mongodbatlas.RegionsConfig{
100+
"US_EAST": {AnalyticsNodes: int64ptr(0), ElectableNodes: int64ptr(3), Priority: int64ptr(7), ReadOnlyNodes: int64ptr(0)}}},
101+
}
102+
operatorCluster := mdbv1.DefaultAWSCluster("test-ns", "project-name")
103+
operatorCluster.Spec.ReplicationSpecs = []mdbv1.ReplicationSpec{{
104+
NumShards: int64ptr(2),
105+
ZoneName: "zone5",
106+
}}
107+
108+
merged, err := MergedCluster(*atlasCluster, operatorCluster.Spec)
109+
assert.NoError(t, err)
110+
111+
expectedReplicationSpecs := []mongodbatlas.ReplicationSpec{{
112+
ID: "id",
113+
NumShards: int64ptr(2),
114+
ZoneName: "zone5",
115+
RegionsConfig: map[string]mongodbatlas.RegionsConfig{
116+
"US_EAST": {AnalyticsNodes: int64ptr(0), ElectableNodes: int64ptr(3), Priority: int64ptr(7), ReadOnlyNodes: int64ptr(0)}}},
117+
}
118+
assert.Equal(t, expectedReplicationSpecs, merged.ReplicationSpecs)
119+
120+
equal := ClustersEqual(zap.S(), *atlasCluster, merged)
43121
assert.False(t, equal)
44122
})
45123
}
124+
func int64ptr(i int64) *int64 {
125+
return &i
126+
}

0 commit comments

Comments
 (0)