Skip to content

Commit 9e8a3a7

Browse files
authored
CLOUDP-130484: Refactor the Advanced Deployment Handler (#615)
1 parent 14801f8 commit 9e8a3a7

File tree

6 files changed

+116
-119
lines changed

6 files changed

+116
-119
lines changed

config/crd/bases/atlas.mongodb.com_atlasdeployments.yaml

Lines changed: 0 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -61,67 +61,10 @@ spec:
6161
type: object
6262
clusterType:
6363
type: string
64-
connectionStrings:
65-
description: ConnectionStrings configuration for applications
66-
use to connect to this deployment.
67-
properties:
68-
awsPrivateLink:
69-
additionalProperties:
70-
type: string
71-
type: object
72-
awsPrivateLinkSrv:
73-
additionalProperties:
74-
type: string
75-
type: object
76-
private:
77-
type: string
78-
privateEndpoint:
79-
items:
80-
description: PrivateEndpointSpec connection strings. Each
81-
object describes the connection strings you can use to
82-
connect to this deployment through a private endpoint.
83-
Atlas returns this parameter only if you deployed a private
84-
endpoint to all regions to which you deployed this deployment's
85-
nodes.
86-
properties:
87-
connectionString:
88-
type: string
89-
endpoints:
90-
items:
91-
description: EndpointSpec through which you connect
92-
to Atlas.
93-
properties:
94-
endpointId:
95-
type: string
96-
providerName:
97-
type: string
98-
region:
99-
type: string
100-
type: object
101-
type: array
102-
srvConnectionString:
103-
type: string
104-
type:
105-
type: string
106-
type: object
107-
type: array
108-
privateSrv:
109-
type: string
110-
standard:
111-
type: string
112-
standardSrv:
113-
type: string
114-
type: object
115-
createDate:
116-
type: string
11764
diskSizeGB:
11865
type: integer
11966
encryptionAtRestProvider:
12067
type: string
121-
groupId:
122-
type: string
123-
id:
124-
type: string
12568
labels:
12669
items:
12770
description: LabelSpec contains key-value pairs that tag and
@@ -154,8 +97,6 @@ spec:
15497
replicationSpecs:
15598
items:
15699
properties:
157-
id:
158-
type: string
159100
numShards:
160101
type: integer
161102
regionConfigs:
@@ -255,8 +196,6 @@ spec:
255196
type: array
256197
rootCertType:
257198
type: string
258-
stateName:
259-
type: string
260199
versionReleaseSystem:
261200
type: string
262201
type: object

pkg/api/v1/atlasdeployment_types.go

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -146,11 +146,8 @@ type AdvancedDeploymentSpec struct {
146146
BackupEnabled *bool `json:"backupEnabled,omitempty"`
147147
BiConnector *BiConnectorSpec `json:"biConnector,omitempty"`
148148
ClusterType string `json:"clusterType,omitempty"`
149-
ConnectionStrings *ConnectionStrings `json:"connectionStrings,omitempty"`
150149
DiskSizeGB *int `json:"diskSizeGB,omitempty"`
151150
EncryptionAtRestProvider string `json:"encryptionAtRestProvider,omitempty"`
152-
GroupID string `json:"groupId,omitempty"`
153-
ID string `json:"id,omitempty"`
154151
Labels []common.LabelSpec `json:"labels,omitempty"`
155152
MongoDBMajorVersion string `json:"mongoDBMajorVersion,omitempty"`
156153
MongoDBVersion string `json:"mongoDBVersion,omitempty"`
@@ -161,13 +158,18 @@ type AdvancedDeploymentSpec struct {
161158
Name string `json:"name,omitempty"`
162159
Paused *bool `json:"paused,omitempty"`
163160
PitEnabled *bool `json:"pitEnabled,omitempty"`
164-
StateName string `json:"stateName,omitempty"`
165161
ReplicationSpecs []*AdvancedReplicationSpec `json:"replicationSpecs,omitempty"`
166-
CreateDate string `json:"createDate,omitempty"`
167162
RootCertType string `json:"rootCertType,omitempty"`
168163
VersionReleaseSystem string `json:"versionReleaseSystem,omitempty"`
169164
}
170165

166+
// ToAtlas converts the AdvancedDeploymentSpec to native Atlas client ToAtlas format.
167+
func (s *AdvancedDeploymentSpec) ToAtlas() (*mongodbatlas.AdvancedCluster, error) {
168+
result := &mongodbatlas.AdvancedCluster{}
169+
err := compat.JSONCopy(result, s)
170+
return result, err
171+
}
172+
171173
// ServerlessSpec defines the desired state of Atlas Serverless Instance
172174
type ServerlessSpec struct {
173175
// Name of the serverless deployment as it appears in Atlas.
@@ -179,13 +181,6 @@ type ServerlessSpec struct {
179181
ProviderSettings *ProviderSettingsSpec `json:"providerSettings"`
180182
}
181183

182-
// AdvancedDeployment converts the AdvancedDeploymentSpec to native Atlas client AdvancedDeployment format.
183-
func (s *AdvancedDeploymentSpec) AdvancedDeployment() (*mongodbatlas.AdvancedCluster, error) {
184-
result := &mongodbatlas.AdvancedCluster{}
185-
err := compat.JSONCopy(result, s)
186-
return result, err
187-
}
188-
189184
// BiConnector specifies BI Connector for Atlas configuration on this deployment.
190185
type BiConnector struct {
191186
Enabled *bool `json:"enabled,omitempty"`
@@ -223,7 +218,6 @@ type EndpointSpec struct {
223218

224219
type AdvancedReplicationSpec struct {
225220
NumShards int `json:"numShards,omitempty"`
226-
ID string `json:"id,omitempty"`
227221
ZoneName string `json:"zoneName,omitempty"`
228222
RegionConfigs []*AdvancedRegionConfig `json:"regionConfigs,omitempty"`
229223
}
@@ -546,10 +540,10 @@ func newServerlessInstance(namespace, name, nameInAtlas, backingProviderName, re
546540
}
547541

548542
func NewAwsAdvancedDeployment(namespace, name, nameInAtlas string) *AtlasDeployment {
549-
return newAwsAdvancedDeployment(namespace, name, nameInAtlas, "M5", "AWS", "US_EAST_1")
543+
return newAwsAdvancedDeployment(namespace, name, nameInAtlas, "M10", "AWS", "US_EAST_1", 3)
550544
}
551545

552-
func newAwsAdvancedDeployment(namespace, name, nameInAtlas, instanceSize, backingProviderName, regionName string) *AtlasDeployment {
546+
func newAwsAdvancedDeployment(namespace, name, nameInAtlas, instanceSize, providerName, regionName string, nodeCount int) *AtlasDeployment {
553547
priority := 7
554548
return &AtlasDeployment{
555549
ObjectMeta: metav1.ObjectMeta{
@@ -567,10 +561,10 @@ func newAwsAdvancedDeployment(namespace, name, nameInAtlas, instanceSize, backin
567561
Priority: &priority,
568562
ElectableSpecs: &Specs{
569563
InstanceSize: instanceSize,
564+
NodeCount: &nodeCount,
570565
},
571-
BackingProviderName: backingProviderName,
572-
ProviderName: "TENANT",
573-
RegionName: regionName,
566+
ProviderName: providerName,
567+
RegionName: regionName,
574568
},
575569
},
576570
}},

pkg/api/v1/zz_generated.deepcopy.go

Lines changed: 0 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/controller/atlasdeployment/advanced_deployment.go

Lines changed: 42 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ func (r *AtlasDeploymentReconciler) ensureAdvancedDeploymentState(ctx *workflow.
2929
return advancedDeployment, workflow.Terminate(workflow.DeploymentNotCreatedInAtlas, err.Error())
3030
}
3131

32-
advancedDeployment, err = advancedDeploymentSpec.AdvancedDeployment()
32+
advancedDeployment, err = advancedDeploymentSpec.ToAtlas()
3333
if err != nil {
3434
return advancedDeployment, workflow.Terminate(workflow.Internal, err.Error())
3535
}
@@ -58,73 +58,87 @@ func (r *AtlasDeploymentReconciler) ensureAdvancedDeploymentState(ctx *workflow.
5858
}
5959
}
6060

61-
func advancedDeploymentIdle(ctx *workflow.Context, project *mdbv1.AtlasProject, deployment *mdbv1.AtlasDeployment, advancedDeployment *mongodbatlas.AdvancedCluster) (*mongodbatlas.AdvancedCluster, workflow.Result) {
62-
resultingDeployment, err := MergedAdvancedDeployment(*advancedDeployment, deployment.Spec)
61+
func advancedDeploymentIdle(ctx *workflow.Context, project *mdbv1.AtlasProject, deployment *mdbv1.AtlasDeployment, atlasDeploymentAsAtlas *mongodbatlas.AdvancedCluster) (*mongodbatlas.AdvancedCluster, workflow.Result) {
62+
specDeployment, atlasDeployment, err := MergedAdvancedDeployment(*atlasDeploymentAsAtlas, *deployment.Spec.AdvancedDeploymentSpec)
6363
if err != nil {
64-
return advancedDeployment, workflow.Terminate(workflow.Internal, err.Error())
64+
return atlasDeploymentAsAtlas, workflow.Terminate(workflow.Internal, err.Error())
6565
}
6666

67-
if done := AdvancedDeploymentsEqual(ctx.Log, *advancedDeployment, resultingDeployment); done {
68-
return advancedDeployment, workflow.OK()
67+
if areEqual := AdvancedDeploymentsEqual(ctx.Log, specDeployment, atlasDeployment); areEqual {
68+
return atlasDeploymentAsAtlas, workflow.OK()
6969
}
7070

71-
if deployment.Spec.AdvancedDeploymentSpec.Paused != nil {
72-
if advancedDeployment.Paused == nil || *advancedDeployment.Paused != *deployment.Spec.AdvancedDeploymentSpec.Paused {
71+
if specDeployment.Paused != nil {
72+
if atlasDeployment.Paused == nil || *atlasDeployment.Paused != *specDeployment.Paused {
7373
// paused is different from Atlas
7474
// we need to first send a special (un)pause request before reconciling everything else
75-
resultingDeployment = mongodbatlas.AdvancedCluster{
75+
specDeployment = mdbv1.AdvancedDeploymentSpec{
7676
Paused: deployment.Spec.AdvancedDeploymentSpec.Paused,
7777
}
7878
} else {
7979
// otherwise, don't send the paused field
80-
resultingDeployment.Paused = nil
80+
specDeployment.Paused = nil
8181
}
8282
}
8383

84-
resultingDeployment = cleanupAdvancedDeployment(resultingDeployment)
84+
deploymentAsAtlas, err := cleanupTheSpec(specDeployment).ToAtlas()
85+
if err != nil {
86+
return atlasDeploymentAsAtlas, workflow.Terminate(workflow.Internal, err.Error())
87+
}
8588

86-
advancedDeployment, _, err = ctx.Client.AdvancedClusters.Update(context.Background(), project.Status.ID, deployment.Spec.AdvancedDeploymentSpec.Name, &resultingDeployment)
89+
atlasDeploymentAsAtlas, _, err = ctx.Client.AdvancedClusters.Update(context.Background(), project.Status.ID, deployment.Spec.AdvancedDeploymentSpec.Name, deploymentAsAtlas)
8790
if err != nil {
88-
return advancedDeployment, workflow.Terminate(workflow.DeploymentNotUpdatedInAtlas, err.Error())
91+
return atlasDeploymentAsAtlas, workflow.Terminate(workflow.DeploymentNotUpdatedInAtlas, err.Error())
8992
}
9093

9194
return nil, workflow.InProgress(workflow.DeploymentUpdating, "deployment is updating")
9295
}
9396

94-
func cleanupAdvancedDeployment(deployment mongodbatlas.AdvancedCluster) mongodbatlas.AdvancedCluster {
95-
deployment.ID = ""
97+
func cleanupTheSpec(deployment mdbv1.AdvancedDeploymentSpec) *mdbv1.AdvancedDeploymentSpec {
9698
deployment.MongoDBVersion = ""
97-
deployment.StateName = ""
98-
deployment.ConnectionStrings = nil
99-
return deployment
99+
return &deployment
100100
}
101101

102102
// MergedAdvancedDeployment will return the result of merging AtlasDeploymentSpec with Atlas Advanced Deployment
103-
func MergedAdvancedDeployment(advancedDeployment mongodbatlas.AdvancedCluster, spec mdbv1.AtlasDeploymentSpec) (mongodbatlas.AdvancedCluster, error) {
104-
result := mongodbatlas.AdvancedCluster{}
105-
if err := compat.JSONCopy(&result, advancedDeployment); err != nil {
106-
return result, err
103+
func MergedAdvancedDeployment(atlasDeploymentAsAtlas mongodbatlas.AdvancedCluster, specDeployment mdbv1.AdvancedDeploymentSpec) (mergedDeployment mdbv1.AdvancedDeploymentSpec, atlasDeployment mdbv1.AdvancedDeploymentSpec, err error) {
104+
atlasDeployment, err = AdvancedDeploymentFromAtlas(atlasDeploymentAsAtlas)
105+
if err != nil {
106+
return
107107
}
108108

109-
if err := compat.JSONCopy(&result, spec.AdvancedDeploymentSpec); err != nil {
110-
return result, err
109+
mergedDeployment = mdbv1.AdvancedDeploymentSpec{}
110+
if err = compat.JSONCopy(&mergedDeployment, atlasDeployment); err != nil {
111+
return
112+
}
113+
114+
if err = compat.JSONCopy(&mergedDeployment, specDeployment); err != nil {
115+
return
111116
}
112117

113-
for i, replicationSpec := range advancedDeployment.ReplicationSpecs {
118+
for i, replicationSpec := range atlasDeployment.ReplicationSpecs {
114119
for k, v := range replicationSpec.RegionConfigs {
115120
// the response does not return backing provider names in some situations.
116121
// if this is the case, we want to strip these fields so they do not cause a bad comparison.
117122
if v.BackingProviderName == "" {
118-
result.ReplicationSpecs[i].RegionConfigs[k].BackingProviderName = ""
123+
mergedDeployment.ReplicationSpecs[i].RegionConfigs[k].BackingProviderName = ""
119124
}
120125
}
121126
}
127+
return
128+
}
129+
130+
func AdvancedDeploymentFromAtlas(advancedDeployment mongodbatlas.AdvancedCluster) (mdbv1.AdvancedDeploymentSpec, error) {
131+
result := mdbv1.AdvancedDeploymentSpec{}
132+
if err := compat.JSONCopy(&result, advancedDeployment); err != nil {
133+
return result, err
134+
}
135+
122136
return result, nil
123137
}
124138

125139
// AdvancedDeploymentsEqual compares two Atlas Advanced Deployments
126-
func AdvancedDeploymentsEqual(log *zap.SugaredLogger, deploymentAtlas mongodbatlas.AdvancedCluster, deploymentOperator mongodbatlas.AdvancedCluster) bool {
127-
d := cmp.Diff(deploymentAtlas, deploymentOperator, cmpopts.EquateEmpty())
140+
func AdvancedDeploymentsEqual(log *zap.SugaredLogger, deploymentAtlas mdbv1.AdvancedDeploymentSpec, deploymentOperator mdbv1.AdvancedDeploymentSpec) bool {
141+
d := cmp.Diff(deploymentOperator, deploymentAtlas, cmpopts.EquateEmpty())
128142
if d != "" {
129143
log.Debugf("Deployments are different: %s", d)
130144
}

pkg/controller/atlasdeployment/advanced_deployment_test.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@ import (
1111

1212
func TestMergedAdvancedDeployment(t *testing.T) {
1313
defaultAtlas := v1.DefaultAwsAdvancedDeployment("default", "my-project")
14+
defaultAtlas.Spec.AdvancedDeploymentSpec.ReplicationSpecs[0].RegionConfigs[0].ProviderName = "TENANT"
1415
defaultAtlas.Spec.AdvancedDeploymentSpec.ReplicationSpecs[0].RegionConfigs[0].BackingProviderName = "AWS"
16+
defaultAtlas.Spec.AdvancedDeploymentSpec.ReplicationSpecs[0].RegionConfigs[0].ElectableSpecs = &v1.Specs{
17+
InstanceSize: "M5",
18+
}
1519

1620
t.Run("Test merging clusters removes backing provider name if empty", func(t *testing.T) {
1721
advancedCluster := mongodbatlas.AdvancedCluster{
@@ -30,7 +34,7 @@ func TestMergedAdvancedDeployment(t *testing.T) {
3034
},
3135
}
3236

33-
merged, err := MergedAdvancedDeployment(advancedCluster, defaultAtlas.Spec)
37+
merged, _, err := MergedAdvancedDeployment(advancedCluster, *defaultAtlas.Spec.AdvancedDeploymentSpec)
3438
assert.NoError(t, err)
3539
assert.Empty(t, merged.ReplicationSpecs[0].RegionConfigs[0].BackingProviderName)
3640
})
@@ -52,7 +56,7 @@ func TestMergedAdvancedDeployment(t *testing.T) {
5256
},
5357
}
5458

55-
merged, err := MergedAdvancedDeployment(advancedCluster, defaultAtlas.Spec)
59+
merged, _, err := MergedAdvancedDeployment(advancedCluster, *defaultAtlas.Spec.AdvancedDeploymentSpec)
5660
assert.NoError(t, err)
5761
assert.Equal(t, "AWS", merged.ReplicationSpecs[0].RegionConfigs[0].BackingProviderName)
5862
})

0 commit comments

Comments
 (0)