Skip to content

Commit

Permalink
fix: Adjusts auto scaling configurations during `mongodbatlas_advance…
Browse files Browse the repository at this point in the history 
…d_cluster` updates (#2476)

* add logic to adjust autoScaling during updates

* add acceptance test

* minor

* adjust documentation of resource, include comment in sync function, add unit testing

* migration test followup adjustments for previous PR

* reduce instance sizes

---------

Co-authored-by: Agustin Bettati <[email protected]>
  • Loading branch information
@maastha @AgustinBettati
maastha and AgustinBettati authored Aug 9, 2024
1 parent bc76774 commit 75d092c
Show file tree
Hide file tree
Showing 6 changed files with 229 additions and 20 deletions.
4 changes: 2 additions & 2 deletions docs/resources/advanced_cluster.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -540,8 +540,8 @@ If you are upgrading a replica set to a sharded cluster, you cannot increase the
### region_configs

* `analytics_specs` - (Optional) Hardware specifications for [analytics nodes](https://docs.atlas.mongodb.com/reference/faq/deployment/#std-label-analytics-nodes-overview) needed in the region. Analytics nodes handle analytic data such as reporting queries from BI Connector for Atlas. Analytics nodes are read-only and can never become the [primary](https://docs.atlas.mongodb.com/reference/glossary/#std-term-primary). If you don't specify this parameter, no analytics nodes deploy to this region. See [below](#specs)
* `auto_scaling` - (Optional) Configuration for the Collection of settings that configures auto-scaling information for the cluster. The values for the `auto_scaling` parameter must be the same for every item in the `replication_specs` array. See [below](#auto_scaling)
* `analytics_auto_scaling` - (Optional) Configuration for the Collection of settings that configures analytics-auto-scaling information for the cluster. The values for the `analytics_auto_scaling` parameter must be the same for every item in the `replication_specs` array. See [below](#analytics_auto_scaling)
* `auto_scaling` - (Optional) Configuration for the Collection of settings that configures auto-scaling information for the cluster. The values for the `auto_scaling` parameter must be the same for all `region_configs` in all `replication_specs`. See [below](#auto_scaling)
* `analytics_auto_scaling` - (Optional) Configuration for the Collection of settings that configures analytics-auto-scaling information for the cluster. The values for the `analytics_auto_scaling` parameter must be the same for all `region_configs` in all `replication_specs`. See [below](#analytics_auto_scaling)
* `backing_provider_name` - (Optional) Cloud service provider on which you provision the host for a multi-tenant cluster. Use this only when a `provider_name` is `TENANT` and `instance_size` of a specs is `M2` or `M5`.
* `electable_specs` - (Optional) Hardware specifications for electable nodes in the region. Electable nodes can become the [primary](https://docs.atlas.mongodb.com/reference/glossary/#std-term-primary) and can enable local reads. If you do not specify this option, no electable nodes are deployed to the region. See [below](#specs)
* `priority` - (Optional) Election priority of the region. For regions with only read-only nodes, set this value to 0.
Expand Down
2 changes: 1 addition & 1 deletion internal/service/advancedcluster/resource_advanced_cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,7 @@ func updateRequest(ctx context.Context, d *schema.ResourceData, projectID, clust
}
updatedReplicationSpecs = specsWithIDs
}

SyncAutoScalingConfigs(updatedReplicationSpecs)
cluster.ReplicationSpecs = updatedReplicationSpecs
}

Expand Down
15 changes: 9 additions & 6 deletions internal/service/advancedcluster/resource_advanced_cluster_migration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import (
"github.com/mongodb/terraform-provider-mongodbatlas/internal/testutil/mig"
)

// last version that did not support new sharding schema or attributes
const versionBeforeISSRelease = "1.17.6"

func TestMigAdvancedCluster_replicaSetAWSProvider(t *testing.T) {
// once 1.18.0 is released we can adjust this to always check new attributes - CLOUDP-266096
testCase := replicaSetAWSProviderTestCase(t, false)
Expand Down Expand Up @@ -41,7 +44,7 @@ func TestMigAdvancedCluster_asymmetricShardedNewSchema(t *testing.T) {
mig.CreateAndRunTest(t, &testCase)
}

func TestMigAdvancedCluster_replicaSetAWSProviderUpdateAfterVerisonUpgrade(t *testing.T) {
func TestMigAdvancedCluster_replicaSetAWSProviderUpdate(t *testing.T) {
var (
projectID = acc.ProjectIDExecution(t)
clusterName = acc.RandomClusterName()
Expand All @@ -52,7 +55,7 @@ func TestMigAdvancedCluster_replicaSetAWSProviderUpdateAfterVerisonUpgrade(t *te
CheckDestroy: acc.CheckDestroyCluster,
Steps: []resource.TestStep{
{
ExternalProviders: acc.ExternalProviders("1.17.5"), // last version that did not support new sharding schema or attributes
ExternalProviders: acc.ExternalProviders(versionBeforeISSRelease),
Config: configReplicaSetAWSProvider(projectID, clusterName, 60, 3),
Check: checkReplicaSetAWSProvider(projectID, clusterName, 60, 3, false, false),
},
Expand All @@ -65,7 +68,7 @@ func TestMigAdvancedCluster_replicaSetAWSProviderUpdateAfterVerisonUpgrade(t *te
})
}

func TestMigAdvancedCluster_geoShardedOldSchemaUpdateAfterVerisonUpgrade(t *testing.T) {
func TestMigAdvancedCluster_geoShardedOldSchemaUpdate(t *testing.T) {
var (
orgID = os.Getenv("MONGODB_ATLAS_ORG_ID")
projectName = acc.RandomProjectName() // No ProjectIDExecution to avoid cross-region limits because multi-region
Expand All @@ -77,7 +80,7 @@ func TestMigAdvancedCluster_geoShardedOldSchemaUpdateAfterVerisonUpgrade(t *test
CheckDestroy: acc.CheckDestroyCluster,
Steps: []resource.TestStep{
{
ExternalProviders: acc.ExternalProviders("1.17.5"), // last version that did not support new sharding schema or attributes
ExternalProviders: acc.ExternalProviders(versionBeforeISSRelease),
Config: configGeoShardedOldSchema(orgID, projectName, clusterName, 2, 2, false),
Check: checkGeoShardedOldSchema(clusterName, 2, 2, false, false),
},
Expand All @@ -102,7 +105,7 @@ func TestMigAdvancedCluster_shardedMigrationFromOldToNewSchema(t *testing.T) {
CheckDestroy: acc.CheckDestroyCluster,
Steps: []resource.TestStep{
{
ExternalProviders: acc.ExternalProviders("1.17.5"), // last version that did not support new sharding schema or attributes
ExternalProviders: acc.ExternalProviders(versionBeforeISSRelease),
Config: configShardedTransitionOldToNewSchema(orgID, projectName, clusterName, false),
Check: checkShardedTransitionOldToNewSchema(false),
},
Expand All @@ -127,7 +130,7 @@ func TestMigAdvancedCluster_geoShardedMigrationFromOldToNewSchema(t *testing.T)
CheckDestroy: acc.CheckDestroyCluster,
Steps: []resource.TestStep{
{
ExternalProviders: acc.ExternalProviders("1.17.5"), // last version that did not support new sharding schema or attributes
ExternalProviders: acc.ExternalProviders(versionBeforeISSRelease),
Config: configGeoShardedTransitionOldToNewSchema(orgID, projectName, clusterName, false),
Check: checkGeoShardedTransitionOldToNewSchema(false),
},
Expand Down
22 changes: 11 additions & 11 deletions internal/service/advancedcluster/resource_advanced_cluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,12 @@ func singleShardedMultiCloudTestCase(t *testing.T, verifyExternalID bool) resour
CheckDestroy: acc.CheckDestroyCluster,
Steps: []resource.TestStep{
{
Config: configShardedOldSchemaMultiCloud(orgID, projectName, clusterName, 1, "M30"),
Check: checkShardedMultiCloud(clusterName, 1, "M30", verifyExternalID),
Config: configShardedOldSchemaMultiCloud(orgID, projectName, clusterName, 1, "M10"),
Check: checkShardedOldSchemaMultiCloud(clusterName, 1, "M10", verifyExternalID),
},
{
Config: configShardedOldSchemaMultiCloud(orgID, projectName, clusterNameUpdated, 1, "M30"),
Check: checkShardedMultiCloud(clusterNameUpdated, 1, "M30", verifyExternalID),
Config: configShardedOldSchemaMultiCloud(orgID, projectName, clusterNameUpdated, 1, "M10"),
Check: checkShardedOldSchemaMultiCloud(clusterNameUpdated, 1, "M10", verifyExternalID),
},
{
ResourceName: resourceName,
Expand Down Expand Up @@ -523,12 +523,12 @@ func TestAccClusterAdvancedClusterConfig_symmetricShardedOldSchema(t *testing.T)
CheckDestroy: acc.CheckDestroyCluster,
Steps: []resource.TestStep{
{
Config: configShardedOldSchemaMultiCloud(orgID, projectName, clusterName, 2, "M30"),
Check: checkShardedMultiCloud(clusterName, 2, "M30", false),
Config: configShardedOldSchemaMultiCloud(orgID, projectName, clusterName, 2, "M10"),
Check: checkShardedOldSchemaMultiCloud(clusterName, 2, "M10", false),
},
{
Config: configShardedOldSchemaMultiCloud(orgID, projectName, clusterName, 2, "M40"),
Check: checkShardedMultiCloud(clusterName, 2, "M40", false),
Config: configShardedOldSchemaMultiCloud(orgID, projectName, clusterName, 2, "M20"),
Check: checkShardedOldSchemaMultiCloud(clusterName, 2, "M20", false),
},
},
})
Expand Down Expand Up @@ -1049,7 +1049,7 @@ func configShardedOldSchemaMultiCloud(orgID, projectName, name string, numShards
num_shards = %[4]d
region_configs {
electable_specs {
instance_size = "M30"
instance_size = "M10"
node_count = 3
}
analytics_specs {
Expand All @@ -1062,7 +1062,7 @@ func configShardedOldSchemaMultiCloud(orgID, projectName, name string, numShards
}
region_configs {
electable_specs {
instance_size = "M30"
instance_size = "M10"
node_count = 2
}
provider_name = "AZURE"
Expand All @@ -1079,7 +1079,7 @@ func configShardedOldSchemaMultiCloud(orgID, projectName, name string, numShards
`, orgID, projectName, name, numShards, analyticsSize)
}

func checkShardedMultiCloud(name string, numShards int, analyticsSize string, verifyExternalID bool) resource.TestCheckFunc {
func checkShardedOldSchemaMultiCloud(name string, numShards int, analyticsSize string, verifyExternalID bool) resource.TestCheckFunc {
additionalChecks := []resource.TestCheckFunc{
resource.TestCheckResourceAttrWith(resourceName, "replication_specs.0.region_configs.0.electable_specs.0.disk_iops", acc.IntGreatThan(0)),
resource.TestCheckResourceAttrWith(resourceName, "replication_specs.0.region_configs.0.analytics_specs.0.disk_iops", acc.IntGreatThan(0)),
Expand Down
39 changes: 39 additions & 0 deletions internal/service/advancedcluster/resource_update_logic.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,42 @@ func groupIDsByZone(specs []admin.ReplicationSpec20250101) map[string][]string {
}
return result
}

// Having the following considerations:
// - Existing replication specs can have the autoscaling values present in the state with default values even if not defined in the config (case when cluster is imported)
// - API expects autoScaling and analyticsAutoScaling aligned cross all region configs in the PATCH request
// This function is needed to avoid errors if a new replication spec is added, ensuring the PATCH request will have the auto scaling aligned with other replication specs when not present in config.
func SyncAutoScalingConfigs(replicationSpecs *[]admin.ReplicationSpec20250101) {
if replicationSpecs == nil || len(*replicationSpecs) == 0 {
return
}

var defaultAnalyticsAutoScaling, defaultAutoScaling *admin.AdvancedAutoScalingSettings

for _, spec := range *replicationSpecs {
for i := range *spec.RegionConfigs {
regionConfig := &(*spec.RegionConfigs)[i]
if regionConfig.AutoScaling != nil && defaultAutoScaling == nil {
defaultAutoScaling = regionConfig.AutoScaling
}
if regionConfig.AnalyticsAutoScaling != nil && defaultAnalyticsAutoScaling == nil {
defaultAnalyticsAutoScaling = regionConfig.AnalyticsAutoScaling
}
}
}
applyDefaultAutoScaling(replicationSpecs, defaultAutoScaling, defaultAnalyticsAutoScaling)
}

func applyDefaultAutoScaling(replicationSpecs *[]admin.ReplicationSpec20250101, defaultAutoScaling, defaultAnalyticsAutoScaling *admin.AdvancedAutoScalingSettings) {
for _, spec := range *replicationSpecs {
for i := range *spec.RegionConfigs {
regionConfig := &(*spec.RegionConfigs)[i]
if regionConfig.AutoScaling == nil && defaultAutoScaling != nil {
regionConfig.AutoScaling = defaultAutoScaling
}
if regionConfig.AnalyticsAutoScaling == nil && defaultAnalyticsAutoScaling != nil {
regionConfig.AnalyticsAutoScaling = defaultAnalyticsAutoScaling
}
}
}
}
167 changes: 167 additions & 0 deletions internal/service/advancedcluster/resource_update_logic_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,3 +123,170 @@ func TestAddIDsToReplicationSpecs(t *testing.T) {
})
}
}

func TestSyncAutoScalingConfigs(t *testing.T) {
testCases := map[string]struct {
ReplicationSpecs []admin.ReplicationSpec20250101
ExpectedReplicationSpecs []admin.ReplicationSpec20250101
}{
"apply same autoscaling options for new replication spec which does not have autoscaling defined": {
ReplicationSpecs: []admin.ReplicationSpec20250101{
{
Id: admin.PtrString("id-1"),
RegionConfigs: &[]admin.CloudRegionConfig20250101{
{
AutoScaling: &admin.AdvancedAutoScalingSettings{
Compute: &admin.AdvancedComputeAutoScaling{
Enabled: admin.PtrBool(false),
ScaleDownEnabled: admin.PtrBool(false),
},
},
AnalyticsAutoScaling: &admin.AdvancedAutoScalingSettings{
Compute: &admin.AdvancedComputeAutoScaling{
Enabled: admin.PtrBool(false),
ScaleDownEnabled: admin.PtrBool(false),
},
},
},
},
},
{
Id: admin.PtrString("id-2"),
RegionConfigs: &[]admin.CloudRegionConfig20250101{
{
AutoScaling: nil,
AnalyticsAutoScaling: nil,
},
},
},
},
ExpectedReplicationSpecs: []admin.ReplicationSpec20250101{
{
Id: admin.PtrString("id-1"),
RegionConfigs: &[]admin.CloudRegionConfig20250101{
{
AutoScaling: &admin.AdvancedAutoScalingSettings{
Compute: &admin.AdvancedComputeAutoScaling{
Enabled: admin.PtrBool(false),
ScaleDownEnabled: admin.PtrBool(false),
},
},
AnalyticsAutoScaling: &admin.AdvancedAutoScalingSettings{
Compute: &admin.AdvancedComputeAutoScaling{
Enabled: admin.PtrBool(false),
ScaleDownEnabled: admin.PtrBool(false),
},
},
},
},
},
{
Id: admin.PtrString("id-2"),
RegionConfigs: &[]admin.CloudRegionConfig20250101{
{
AutoScaling: &admin.AdvancedAutoScalingSettings{
Compute: &admin.AdvancedComputeAutoScaling{
Enabled: admin.PtrBool(false),
ScaleDownEnabled: admin.PtrBool(false),
},
},
AnalyticsAutoScaling: &admin.AdvancedAutoScalingSettings{
Compute: &admin.AdvancedComputeAutoScaling{
Enabled: admin.PtrBool(false),
ScaleDownEnabled: admin.PtrBool(false),
},
},
},
},
},
},
},
// for this case the API will respond with an error and guide the user to align autoscaling options cross all nodes
"when different autoscaling options are defined values will not be changed": {
ReplicationSpecs: []admin.ReplicationSpec20250101{
{
Id: admin.PtrString("id-1"),
RegionConfigs: &[]admin.CloudRegionConfig20250101{
{
AutoScaling: &admin.AdvancedAutoScalingSettings{
Compute: &admin.AdvancedComputeAutoScaling{
Enabled: admin.PtrBool(false),
ScaleDownEnabled: admin.PtrBool(false),
},
},
AnalyticsAutoScaling: &admin.AdvancedAutoScalingSettings{
Compute: &admin.AdvancedComputeAutoScaling{
Enabled: admin.PtrBool(true),
ScaleDownEnabled: admin.PtrBool(true),
},
},
},
},
},
{
Id: admin.PtrString("id-2"),
RegionConfigs: &[]admin.CloudRegionConfig20250101{
{
AutoScaling: &admin.AdvancedAutoScalingSettings{
Compute: &admin.AdvancedComputeAutoScaling{
Enabled: admin.PtrBool(true),
},
},
AnalyticsAutoScaling: &admin.AdvancedAutoScalingSettings{
Compute: &admin.AdvancedComputeAutoScaling{
Enabled: admin.PtrBool(false),
},
},
},
},
},
},
ExpectedReplicationSpecs: []admin.ReplicationSpec20250101{
{
Id: admin.PtrString("id-1"),
RegionConfigs: &[]admin.CloudRegionConfig20250101{
{
AutoScaling: &admin.AdvancedAutoScalingSettings{
Compute: &admin.AdvancedComputeAutoScaling{
Enabled: admin.PtrBool(false),
ScaleDownEnabled: admin.PtrBool(false),
},
},
AnalyticsAutoScaling: &admin.AdvancedAutoScalingSettings{
Compute: &admin.AdvancedComputeAutoScaling{
Enabled: admin.PtrBool(true),
ScaleDownEnabled: admin.PtrBool(true),
},
},
},
},
},
{
Id: admin.PtrString("id-2"),
RegionConfigs: &[]admin.CloudRegionConfig20250101{
{
AutoScaling: &admin.AdvancedAutoScalingSettings{
Compute: &admin.AdvancedComputeAutoScaling{
Enabled: admin.PtrBool(true),
},
},
AnalyticsAutoScaling: &admin.AdvancedAutoScalingSettings{
Compute: &admin.AdvancedComputeAutoScaling{
Enabled: admin.PtrBool(false),
},
},
},
},
},
},
},
}

for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
specs := &tc.ReplicationSpecs
advancedcluster.SyncAutoScalingConfigs(specs)
assert.Equal(t, tc.ExpectedReplicationSpecs, *specs)
})
}
}

0 comments on commit 75d092c

Please sign in to comment.