From b5050fa23099332cf053c00b28dc57e3ae432c4f Mon Sep 17 00:00:00 2001 From: Agustin Bettati Date: Mon, 8 Jul 2024 14:32:59 +0200 Subject: [PATCH 1/9] wip --- .../advancedcluster/model_advanced_cluster.go | 5 ++++- .../resource_advanced_cluster_test.go | 12 ++++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/internal/service/advancedcluster/model_advanced_cluster.go b/internal/service/advancedcluster/model_advanced_cluster.go index c11f57450f..c68ffb7f67 100644 --- a/internal/service/advancedcluster/model_advanced_cluster.go +++ b/internal/service/advancedcluster/model_advanced_cluster.go @@ -942,8 +942,11 @@ func expandRegionConfigSpec(tfList []any, providerName string, rootDiskSizeGB *f apiObject.NodeCount = conversion.Pointer(v.(int)) } - // TODO: CLOUDP-258270 once disk_size_gb schema attribute is added at this level, we will check if defined in config and prioritize over value defined at root level (deprecated and to be removed) apiObject.DiskSizeGB = rootDiskSizeGB + // disk size gb defined in inner level will take precedence over root level. + if v, ok := tfMap["disk_size_gb"]; ok { + apiObject.DiskSizeGB = conversion.Pointer(v.(float64)) + } return apiObject } diff --git a/internal/service/advancedcluster/resource_advanced_cluster_test.go b/internal/service/advancedcluster/resource_advanced_cluster_test.go index c376a4b5d5..969ef305ab 100644 --- a/internal/service/advancedcluster/resource_advanced_cluster_test.go +++ b/internal/service/advancedcluster/resource_advanced_cluster_test.go @@ -705,6 +705,7 @@ func configSingleProvider(projectID, name string) string { name = %[2]q cluster_type = "REPLICASET" retain_backups_enabled = "true" + disk_size_gb = 60 replication_specs { region_configs { @@ -734,8 +735,9 @@ func checkSingleProvider(projectID, name string) resource.TestCheckFunc { return checkAggr( []string{"replication_specs.#", "replication_specs.0.region_configs.#"}, map[string]string{ - "project_id": projectID, - "name": name}, + "project_id": projectID, + "disk_size_gb": "60", + "name": name}, resource.TestCheckResourceAttr(resourceName, "retain_backups_enabled", "true"), resource.TestCheckResourceAttrWith(resourceName, "replication_specs.0.region_configs.0.electable_specs.0.disk_iops", acc.IntGreatThan(0)), resource.TestCheckResourceAttrWith(dataSourceName, "replication_specs.0.region_configs.0.electable_specs.0.disk_iops", acc.IntGreatThan(0))) @@ -1210,6 +1212,7 @@ func configShardedNewSchema(orgID, projectName, name, instanceSizeSpec1, instanc instance_size = %[4]q disk_iops = %[6]d node_count = 3 + disk_size_gb = 60 } analytics_specs { instance_size = %[4]q @@ -1227,6 +1230,7 @@ func configShardedNewSchema(orgID, projectName, name, instanceSizeSpec1, instanc instance_size = %[5]q disk_iops = %[7]d node_count = 3 + disk_size_gb = 60 } analytics_specs { instance_size = %[5]q @@ -1255,6 +1259,10 @@ func checkShardedNewSchema(instanceSizeSpec1, instanceSizeSpec2, diskIopsSpec1, "replication_specs.0.id": "", "replication_specs.0.region_configs.0.electable_specs.0.instance_size": instanceSizeSpec1, "replication_specs.1.region_configs.0.electable_specs.0.instance_size": instanceSizeSpec2, + "replication_specs.0.region_configs.0.electable_specs.0.disk_size_gb": "60", + "replication_specs.1.region_configs.0.electable_specs.0.disk_size_gb": "60", + "replication_specs.0.region_configs.0.analytics_specs.0.disk_size_gb": "60", + "replication_specs.0.region_configs.0.read_only_specs.0.disk_size_gb": "60", "replication_specs.0.region_configs.0.electable_specs.0.disk_iops": diskIopsSpec1, "replication_specs.1.region_configs.0.electable_specs.0.disk_iops": diskIopsSpec2, }) From 0872a727cf239a7d46bc6c4bff227f56d7c5137a Mon Sep 17 00:00:00 2001 From: Agustin Bettati Date: Tue, 9 Jul 2024 22:36:03 +0200 Subject: [PATCH 2/9] define function for setting root disk_size_gb when calling new api in read --- .../advancedcluster/model_advanced_cluster.go | 14 ++++++++++++++ .../advancedcluster/resource_advanced_cluster.go | 5 ++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/internal/service/advancedcluster/model_advanced_cluster.go b/internal/service/advancedcluster/model_advanced_cluster.go index c68ffb7f67..7e136bb8fb 100644 --- a/internal/service/advancedcluster/model_advanced_cluster.go +++ b/internal/service/advancedcluster/model_advanced_cluster.go @@ -278,6 +278,20 @@ func IsSharedTier(instanceSize string) bool { return instanceSize == "M0" || instanceSize == "M2" || instanceSize == "M5" } +// getDiskSizeGBFromReplicationSpec obtains the diskSizeGB value by looking into the electable spec of the first replication spec. +// Independent storage size scaling is not supported (CLOUDP-201331), meaning all electable/analytics/read only configs in all replication specs are the same. +func getDiskSizeGBFromReplicationSpec(cluster *admin.ClusterDescription20240710) float64 { + specs := cluster.GetReplicationSpecs() + if len(specs) < 1 { + return 0 + } + configs := specs[0].GetRegionConfigs() + if len(configs) < 1 { + return 0 + } + return configs[0].ElectableSpecs.GetDiskSizeGB() +} + func UpgradeRefreshFunc(ctx context.Context, name, projectID string, client admin20231115.ClustersApi) retry.StateRefreshFunc { return func() (any, string, error) { cluster, resp, err := client.GetCluster(ctx, projectID, name).Execute() diff --git a/internal/service/advancedcluster/resource_advanced_cluster.go b/internal/service/advancedcluster/resource_advanced_cluster.go index 6bfe607f14..e2b9d38238 100644 --- a/internal/service/advancedcluster/resource_advanced_cluster.go +++ b/internal/service/advancedcluster/resource_advanced_cluster.go @@ -538,7 +538,10 @@ func resourceRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Di return diag.FromErr(fmt.Errorf(errorRead, clusterName, err)) } - // TODO: CLOUDP-258270 set disk size gb at root level + // root disk_size_gb defined for backwards compatibility avoiding breaking changes + if err := d.Set("disk_size_gb", getDiskSizeGBFromReplicationSpec(cluster)); err != nil { + return diag.FromErr(fmt.Errorf(ErrorClusterAdvancedSetting, "disk_size_gb", clusterName, err)) + } replicationSpecs, err = flattenAdvancedReplicationSpecs(ctx, cluster.GetReplicationSpecs(), d.Get("replication_specs").([]any), d, connV2) if err != nil { From f780530d3f71a068b8322b05fe24074bd24b6435 Mon Sep 17 00:00:00 2001 From: Agustin Bettati Date: Wed, 10 Jul 2024 10:53:30 +0200 Subject: [PATCH 3/9] add test with old shard schema and disk size gb at inner level --- .../advancedcluster/model_advanced_cluster.go | 2 +- .../resource_advanced_cluster_test.go | 122 +++++++++++++++--- 2 files changed, 103 insertions(+), 21 deletions(-) diff --git a/internal/service/advancedcluster/model_advanced_cluster.go b/internal/service/advancedcluster/model_advanced_cluster.go index 7e136bb8fb..23a4fe458f 100644 --- a/internal/service/advancedcluster/model_advanced_cluster.go +++ b/internal/service/advancedcluster/model_advanced_cluster.go @@ -958,7 +958,7 @@ func expandRegionConfigSpec(tfList []any, providerName string, rootDiskSizeGB *f apiObject.DiskSizeGB = rootDiskSizeGB // disk size gb defined in inner level will take precedence over root level. - if v, ok := tfMap["disk_size_gb"]; ok { + if v, ok := tfMap["disk_size_gb"]; ok && v.(float64) != 0 { apiObject.DiskSizeGB = conversion.Pointer(v.(float64)) } diff --git a/internal/service/advancedcluster/resource_advanced_cluster_test.go b/internal/service/advancedcluster/resource_advanced_cluster_test.go index 969ef305ab..677e1c2f4a 100644 --- a/internal/service/advancedcluster/resource_advanced_cluster_test.go +++ b/internal/service/advancedcluster/resource_advanced_cluster_test.go @@ -405,8 +405,8 @@ func TestAccClusterAdvancedClusterConfig_replicationSpecsAndShardUpdating(t *tes CheckDestroy: acc.CheckDestroyCluster, Steps: []resource.TestStep{ { - Config: configMultiZoneWithShards(orgID, projectName, clusterName, 1, 1, false), - Check: checkMultiZoneWithShards(clusterName, 1, 1), + Config: configMultiZoneWithShardsOldSchema(orgID, projectName, clusterName, 1, 1, false), + Check: checkMultiZoneWithShardsOldSchema(clusterName, 1, 1), }, // TODO: CLOUDP-259828 updating from single sharded to using old schema should throw an error here }, @@ -454,7 +454,7 @@ func TestAccClusterAdvancedClusterConfig_selfManagedSharding(t *testing.T) { CheckDestroy: acc.CheckDestroyCluster, Steps: []resource.TestStep{ { - Config: configMultiZoneWithShards(orgID, projectName, clusterName, 1, 1, true), + Config: configMultiZoneWithShardsOldSchema(orgID, projectName, clusterName, 1, 1, true), Check: resource.ComposeAggregateTestCheckFunc( checkExists(resourceName), resource.TestCheckResourceAttr(resourceName, "global_cluster_self_managed_sharding", "true"), @@ -462,7 +462,7 @@ func TestAccClusterAdvancedClusterConfig_selfManagedSharding(t *testing.T) { ), }, { - Config: configMultiZoneWithShards(orgID, projectName, clusterName, 1, 1, false), + Config: configMultiZoneWithShardsOldSchema(orgID, projectName, clusterName, 1, 1, false), ExpectError: regexp.MustCompile("CANNOT_MODIFY_GLOBAL_CLUSTER_MANAGEMENT_SETTING"), }, }, @@ -502,12 +502,33 @@ func TestAccClusterAdvancedClusterConfig_symmetricGeoShardedOldSchema(t *testing CheckDestroy: acc.CheckDestroyCluster, Steps: []resource.TestStep{ { - Config: configMultiZoneWithShards(orgID, projectName, clusterName, 2, 2, false), - Check: checkMultiZoneWithShards(clusterName, 2, 2), + Config: configMultiZoneWithShardsOldSchema(orgID, projectName, clusterName, 2, 2, false), + Check: checkMultiZoneWithShardsOldSchema(clusterName, 2, 2), }, { - Config: configMultiZoneWithShards(orgID, projectName, clusterName, 3, 3, false), - Check: checkMultiZoneWithShards(clusterName, 3, 3), + Config: configMultiZoneWithShardsOldSchema(orgID, projectName, clusterName, 3, 3, false), + Check: checkMultiZoneWithShardsOldSchema(clusterName, 3, 3), + }, + }, + }) +} + +func TestAccClusterAdvancedClusterConfig_symmetricShardedOldSchemaDiskSizeGBAtElectableLevel(t *testing.T) { + acc.SkipTestForCI(t) // TODO: CLOUDP-260154 for ensuring this use case is supported + var ( + orgID = os.Getenv("MONGODB_ATLAS_ORG_ID") + projectName = acc.RandomProjectName() + clusterName = acc.RandomClusterName() + ) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acc.PreCheckBasic(t) }, + ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, + CheckDestroy: acc.CheckDestroyCluster, + Steps: []resource.TestStep{ + { + Config: configShardedOldSchemaDiskSizeGBElectableLevel(orgID, projectName, clusterName), + Check: checkShardedOldSchemaDiskSizeGBElectableLevel(), }, }, }) @@ -737,7 +758,9 @@ func checkSingleProvider(projectID, name string) resource.TestCheckFunc { map[string]string{ "project_id": projectID, "disk_size_gb": "60", - "name": name}, + "replication_specs.0.region_configs.0.electable_specs.0.disk_size_gb": "60", + "replication_specs.0.region_configs.0.analytics_specs.0.disk_size_gb": "60", + "name": name}, resource.TestCheckResourceAttr(resourceName, "retain_backups_enabled", "true"), resource.TestCheckResourceAttrWith(resourceName, "replication_specs.0.region_configs.0.electable_specs.0.disk_iops", acc.IntGreatThan(0)), resource.TestCheckResourceAttrWith(dataSourceName, "replication_specs.0.region_configs.0.electable_specs.0.disk_iops", acc.IntGreatThan(0))) @@ -1132,7 +1155,7 @@ func configReplicationSpecsAnalyticsAutoScaling(projectID, clusterName string, p `, projectID, clusterName, p.Compute.GetEnabled(), p.DiskGB.GetEnabled(), p.Compute.GetMaxInstanceSize()) } -func configMultiZoneWithShards(orgID, projectName, name string, numShardsFirstZone, numShardsSecondZone int, selfManagedSharding bool) string { +func configMultiZoneWithShardsOldSchema(orgID, projectName, name string, numShardsFirstZone, numShardsSecondZone int, selfManagedSharding bool) string { return fmt.Sprintf(` resource "mongodbatlas_project" "cluster_project" { org_id = %[1]q @@ -1146,6 +1169,7 @@ func configMultiZoneWithShards(orgID, projectName, name string, numShardsFirstZo mongo_db_major_version = "7.0" cluster_type = "GEOSHARDED" global_cluster_self_managed_sharding = %[6]t + disk_size_gb = 60 replication_specs { zone_name = "zone n1" @@ -1193,6 +1217,71 @@ func configMultiZoneWithShards(orgID, projectName, name string, numShardsFirstZo `, orgID, projectName, name, numShardsFirstZone, numShardsSecondZone, selfManagedSharding) } +func checkMultiZoneWithShardsOldSchema(name string, numShardsFirstZone, numShardsSecondZone int) resource.TestCheckFunc { + return checkAggr( + []string{"project_id"}, + map[string]string{ + "name": name, + "disk_size_gb": "60", + "replication_specs.0.region_configs.0.electable_specs.0.disk_size_gb": "60", + "replication_specs.0.region_configs.0.analytics_specs.0.disk_size_gb": "60", + "replication_specs.0.num_shards": strconv.Itoa(numShardsFirstZone), + "replication_specs.1.num_shards": strconv.Itoa(numShardsSecondZone), + }) +} + +func configShardedOldSchemaDiskSizeGBElectableLevel(orgID, projectName, name string) string { + return fmt.Sprintf(` + resource "mongodbatlas_project" "cluster_project" { + org_id = %[1]q + name = %[2]q + } + + resource "mongodbatlas_advanced_cluster" "test" { + project_id = mongodbatlas_project.cluster_project.id + name = %[3]q + backup_enabled = false + mongo_db_major_version = "7.0" + cluster_type = "SHARDED" + + replication_specs { + num_shards = 2 + + region_configs { + electable_specs { + instance_size = "M10" + node_count = 3 + disk_size_gb = 60 + } + analytics_specs { + instance_size = "M10" + node_count = 0 + disk_size_gb = 60 + } + provider_name = "AWS" + priority = 7 + region_name = "US_EAST_1" + } + } + } + + data "mongodbatlas_advanced_cluster" "test" { + project_id = mongodbatlas_advanced_cluster.test.project_id + name = mongodbatlas_advanced_cluster.test.name + } + `, orgID, projectName, name) +} + +func checkShardedOldSchemaDiskSizeGBElectableLevel() resource.TestCheckFunc { + return checkAggr( + []string{}, + map[string]string{ + "replication_specs.0.num_shards": "2", + "replication_specs.0.region_configs.0.electable_specs.0.disk_size_gb": "60", + "replication_specs.0.region_configs.0.analytics_specs.0.disk_size_gb": "60", + }) +} + func configShardedNewSchema(orgID, projectName, name, instanceSizeSpec1, instanceSizeSpec2 string, diskIopsSpec1, diskIopsSpec2 int) string { return fmt.Sprintf(` resource "mongodbatlas_project" "cluster_project" { @@ -1217,6 +1306,7 @@ func configShardedNewSchema(orgID, projectName, name, instanceSizeSpec1, instanc analytics_specs { instance_size = %[4]q node_count = 1 + disk_size_gb = 60 } provider_name = "AWS" priority = 7 @@ -1235,6 +1325,7 @@ func configShardedNewSchema(orgID, projectName, name, instanceSizeSpec1, instanc analytics_specs { instance_size = %[5]q node_count = 1 + disk_size_gb = 60 } provider_name = "AWS" priority = 7 @@ -1255,6 +1346,7 @@ func checkShardedNewSchema(instanceSizeSpec1, instanceSizeSpec2, diskIopsSpec1, return checkAggr( []string{"replication_specs.0.external_id", "replication_specs.0.zone_id", "replication_specs.1.external_id", "replication_specs.1.zone_id"}, map[string]string{ + "disk_size_gb": "60", "replication_specs.#": "2", "replication_specs.0.id": "", "replication_specs.0.region_configs.0.electable_specs.0.instance_size": instanceSizeSpec1, @@ -1267,13 +1359,3 @@ func checkShardedNewSchema(instanceSizeSpec1, instanceSizeSpec2, diskIopsSpec1, "replication_specs.1.region_configs.0.electable_specs.0.disk_iops": diskIopsSpec2, }) } - -func checkMultiZoneWithShards(name string, numShardsFirstZone, numShardsSecondZone int) resource.TestCheckFunc { - return checkAggr( - []string{"project_id"}, - map[string]string{ - "name": name, - "replication_specs.0.num_shards": strconv.Itoa(numShardsFirstZone), - "replication_specs.1.num_shards": strconv.Itoa(numShardsSecondZone), - }) -} From fe3ed6f958ac49557c0ce5c9883de002bb631d13 Mon Sep 17 00:00:00 2001 From: Agustin Bettati Date: Wed, 10 Jul 2024 11:54:51 +0200 Subject: [PATCH 4/9] adjust check which was searching for readOnly spec --- .../service/advancedcluster/resource_advanced_cluster_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/advancedcluster/resource_advanced_cluster_test.go b/internal/service/advancedcluster/resource_advanced_cluster_test.go index 677e1c2f4a..4e528bd851 100644 --- a/internal/service/advancedcluster/resource_advanced_cluster_test.go +++ b/internal/service/advancedcluster/resource_advanced_cluster_test.go @@ -1354,7 +1354,7 @@ func checkShardedNewSchema(instanceSizeSpec1, instanceSizeSpec2, diskIopsSpec1, "replication_specs.0.region_configs.0.electable_specs.0.disk_size_gb": "60", "replication_specs.1.region_configs.0.electable_specs.0.disk_size_gb": "60", "replication_specs.0.region_configs.0.analytics_specs.0.disk_size_gb": "60", - "replication_specs.0.region_configs.0.read_only_specs.0.disk_size_gb": "60", + "replication_specs.1.region_configs.0.analytics_specs.0.disk_size_gb": "60", "replication_specs.0.region_configs.0.electable_specs.0.disk_iops": diskIopsSpec1, "replication_specs.1.region_configs.0.electable_specs.0.disk_iops": diskIopsSpec2, }) From 494901ccf2fd3a22497470b42a193a06c47528a7 Mon Sep 17 00:00:00 2001 From: Agustin Bettati Date: Wed, 10 Jul 2024 12:03:33 +0200 Subject: [PATCH 5/9] define root size gb at root level of data source when using new API --- .../service/advancedcluster/data_source_advanced_cluster.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/service/advancedcluster/data_source_advanced_cluster.go b/internal/service/advancedcluster/data_source_advanced_cluster.go index 091ce6bcc2..89cef15dc3 100644 --- a/internal/service/advancedcluster/data_source_advanced_cluster.go +++ b/internal/service/advancedcluster/data_source_advanced_cluster.go @@ -303,6 +303,11 @@ func dataSourceRead(ctx context.Context, d *schema.ResourceData, meta any) diag. clusterID = clusterDescLatest.GetId() + // root disk_size_gb defined for backwards compatibility avoiding breaking changes + if err := d.Set("disk_size_gb", getDiskSizeGBFromReplicationSpec(clusterDescLatest)); err != nil { + return diag.FromErr(fmt.Errorf(ErrorClusterAdvancedSetting, "disk_size_gb", clusterName, err)) + } + replicationSpecs, err = flattenAdvancedReplicationSpecsDS(ctx, clusterDescLatest.GetReplicationSpecs(), d, connV2) if err != nil { return diag.FromErr(fmt.Errorf(ErrorClusterAdvancedSetting, "replication_specs", clusterName, err)) From 603b2177a453b017022879af47cb7d992245527b Mon Sep 17 00:00:00 2001 From: Agustin Bettati Date: Wed, 10 Jul 2024 15:18:20 +0200 Subject: [PATCH 6/9] add unit testing and adjust acceptance test --- .../data_source_advanced_cluster.go | 2 +- .../advancedcluster/model_advanced_cluster.go | 6 +-- .../model_advanced_cluster_test.go | 42 +++++++++++++++++++ .../resource_advanced_cluster.go | 2 +- ...esource_advanced_cluster_migration_test.go | 2 +- .../resource_advanced_cluster_test.go | 29 +++++++++---- 6 files changed, 68 insertions(+), 15 deletions(-) diff --git a/internal/service/advancedcluster/data_source_advanced_cluster.go b/internal/service/advancedcluster/data_source_advanced_cluster.go index 89cef15dc3..856d4b6917 100644 --- a/internal/service/advancedcluster/data_source_advanced_cluster.go +++ b/internal/service/advancedcluster/data_source_advanced_cluster.go @@ -304,7 +304,7 @@ func dataSourceRead(ctx context.Context, d *schema.ResourceData, meta any) diag. clusterID = clusterDescLatest.GetId() // root disk_size_gb defined for backwards compatibility avoiding breaking changes - if err := d.Set("disk_size_gb", getDiskSizeGBFromReplicationSpec(clusterDescLatest)); err != nil { + if err := d.Set("disk_size_gb", GetDiskSizeGBFromReplicationSpec(clusterDescLatest)); err != nil { return diag.FromErr(fmt.Errorf(ErrorClusterAdvancedSetting, "disk_size_gb", clusterName, err)) } diff --git a/internal/service/advancedcluster/model_advanced_cluster.go b/internal/service/advancedcluster/model_advanced_cluster.go index 23a4fe458f..3c9a806a21 100644 --- a/internal/service/advancedcluster/model_advanced_cluster.go +++ b/internal/service/advancedcluster/model_advanced_cluster.go @@ -278,9 +278,9 @@ func IsSharedTier(instanceSize string) bool { return instanceSize == "M0" || instanceSize == "M2" || instanceSize == "M5" } -// getDiskSizeGBFromReplicationSpec obtains the diskSizeGB value by looking into the electable spec of the first replication spec. -// Independent storage size scaling is not supported (CLOUDP-201331), meaning all electable/analytics/read only configs in all replication specs are the same. -func getDiskSizeGBFromReplicationSpec(cluster *admin.ClusterDescription20240710) float64 { +// GetDiskSizeGBFromReplicationSpec obtains the diskSizeGB value by looking into the electable spec of the first replication spec. +// Independent storage size scaling is not supported (CLOUDP-201331), meaning all electable/analytics/readOnly configs in all replication specs are the same. +func GetDiskSizeGBFromReplicationSpec(cluster *admin.ClusterDescription20240710) float64 { specs := cluster.GetReplicationSpecs() if len(specs) < 1 { return 0 diff --git a/internal/service/advancedcluster/model_advanced_cluster_test.go b/internal/service/advancedcluster/model_advanced_cluster_test.go index a635f6f2c9..a968d2a7ef 100644 --- a/internal/service/advancedcluster/model_advanced_cluster_test.go +++ b/internal/service/advancedcluster/model_advanced_cluster_test.go @@ -150,6 +150,48 @@ func TestFlattenReplicationSpecs(t *testing.T) { } } +func TestGetDiskSizeGBFromReplicationSpec(t *testing.T) { + diskSizeGBValue := 40.0 + hardwareSpec := admin.HardwareSpec20240710{ + DiskSizeGB: admin.PtrFloat64(diskSizeGBValue), + } + regionConfig := []admin.CloudRegionConfig20240710{{ + ElectableSpecs: &hardwareSpec, + }} + replicationSpecsWithValue := []admin.ReplicationSpec20240710{{RegionConfigs: ®ionConfig}} + + emptyRegionConfig := []admin.CloudRegionConfig20240710{{}} + replicationSpecsEmptyElectable := []admin.ReplicationSpec20240710{{RegionConfigs: &emptyRegionConfig}} + + testCases := map[string]struct { + clusterDescription admin.ClusterDescription20240710 + expectedDiskSizeResult float64 + }{ + "cluster description with disk size gb value at electable spec": { + clusterDescription: admin.ClusterDescription20240710{ + ReplicationSpecs: &replicationSpecsWithValue, + }, + expectedDiskSizeResult: diskSizeGBValue, + }, + "cluster description with no electable spec": { + clusterDescription: admin.ClusterDescription20240710{ + ReplicationSpecs: &replicationSpecsEmptyElectable, + }, + expectedDiskSizeResult: 0, + }, + "cluster description with no replication spec": { + clusterDescription: admin.ClusterDescription20240710{}, + expectedDiskSizeResult: 0, + }, + } + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + result := advancedcluster.GetDiskSizeGBFromReplicationSpec(&tc.clusterDescription) + assert.InEpsilon(t, tc.expectedDiskSizeResult, result, 0.001) // using InEpsilon as type is float64 + }) + } +} + type Result struct { response any error error diff --git a/internal/service/advancedcluster/resource_advanced_cluster.go b/internal/service/advancedcluster/resource_advanced_cluster.go index e2b9d38238..cb7dbb409f 100644 --- a/internal/service/advancedcluster/resource_advanced_cluster.go +++ b/internal/service/advancedcluster/resource_advanced_cluster.go @@ -539,7 +539,7 @@ func resourceRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Di } // root disk_size_gb defined for backwards compatibility avoiding breaking changes - if err := d.Set("disk_size_gb", getDiskSizeGBFromReplicationSpec(cluster)); err != nil { + if err := d.Set("disk_size_gb", GetDiskSizeGBFromReplicationSpec(cluster)); err != nil { return diag.FromErr(fmt.Errorf(ErrorClusterAdvancedSetting, "disk_size_gb", clusterName, err)) } diff --git a/internal/service/advancedcluster/resource_advanced_cluster_migration_test.go b/internal/service/advancedcluster/resource_advanced_cluster_migration_test.go index 747ba13601..51ce30613f 100644 --- a/internal/service/advancedcluster/resource_advanced_cluster_migration_test.go +++ b/internal/service/advancedcluster/resource_advanced_cluster_migration_test.go @@ -25,7 +25,7 @@ func TestMigAdvancedCluster_singleAWSProvider(t *testing.T) { { ExternalProviders: mig.ExternalProviders(), Config: config, - Check: checkSingleProvider(projectID, clusterName), + Check: checkSingleProvider(projectID, clusterName, false), }, mig.TestStepCheckEmptyPlan(config), }, diff --git a/internal/service/advancedcluster/resource_advanced_cluster_test.go b/internal/service/advancedcluster/resource_advanced_cluster_test.go index 4e528bd851..4b1bee37f9 100644 --- a/internal/service/advancedcluster/resource_advanced_cluster_test.go +++ b/internal/service/advancedcluster/resource_advanced_cluster_test.go @@ -64,7 +64,7 @@ func TestAccClusterAdvancedCluster_singleProvider(t *testing.T) { Steps: []resource.TestStep{ { Config: configSingleProvider(projectID, clusterName), - Check: checkSingleProvider(projectID, clusterName), + Check: checkSingleProvider(projectID, clusterName, true), }, { ResourceName: resourceName, @@ -752,18 +752,28 @@ func configSingleProvider(projectID, name string) string { `, projectID, name) } -func checkSingleProvider(projectID, name string) resource.TestCheckFunc { +func checkSingleProvider(projectID, name string, checkDiskSizeGBInnerLevel bool) resource.TestCheckFunc { + additionalChecks := []resource.TestCheckFunc{ + resource.TestCheckResourceAttr(resourceName, "retain_backups_enabled", "true"), + resource.TestCheckResourceAttrWith(resourceName, "replication_specs.0.region_configs.0.electable_specs.0.disk_iops", acc.IntGreatThan(0)), + resource.TestCheckResourceAttrWith(dataSourceName, "replication_specs.0.region_configs.0.electable_specs.0.disk_iops", acc.IntGreatThan(0)), + } + if checkDiskSizeGBInnerLevel { + additionalChecks = append(additionalChecks, + checkAggr([]string{}, map[string]string{ + "replication_specs.0.region_configs.0.electable_specs.0.disk_size_gb": "60", + "replication_specs.0.region_configs.0.analytics_specs.0.disk_size_gb": "60", + }), + ) + } return checkAggr( []string{"replication_specs.#", "replication_specs.0.region_configs.#"}, map[string]string{ "project_id": projectID, "disk_size_gb": "60", - "replication_specs.0.region_configs.0.electable_specs.0.disk_size_gb": "60", - "replication_specs.0.region_configs.0.analytics_specs.0.disk_size_gb": "60", - "name": name}, - resource.TestCheckResourceAttr(resourceName, "retain_backups_enabled", "true"), - resource.TestCheckResourceAttrWith(resourceName, "replication_specs.0.region_configs.0.electable_specs.0.disk_iops", acc.IntGreatThan(0)), - resource.TestCheckResourceAttrWith(dataSourceName, "replication_specs.0.region_configs.0.electable_specs.0.disk_iops", acc.IntGreatThan(0))) + "name": name}, + additionalChecks..., + ) } func configIncorrectTypeGobalClusterSelfManagedSharding(projectID, name string) string { @@ -1276,7 +1286,8 @@ func checkShardedOldSchemaDiskSizeGBElectableLevel() resource.TestCheckFunc { return checkAggr( []string{}, map[string]string{ - "replication_specs.0.num_shards": "2", + "replication_specs.0.num_shards": "2", + "disk_size_gb": "60", "replication_specs.0.region_configs.0.electable_specs.0.disk_size_gb": "60", "replication_specs.0.region_configs.0.analytics_specs.0.disk_size_gb": "60", }) From 0fadcd820cfd56e98d5d617a7103320007ce87ae Mon Sep 17 00:00:00 2001 From: Agustin Bettati Date: Wed, 10 Jul 2024 15:29:01 +0200 Subject: [PATCH 7/9] fix float comparison --- .../service/advancedcluster/model_advanced_cluster_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/service/advancedcluster/model_advanced_cluster_test.go b/internal/service/advancedcluster/model_advanced_cluster_test.go index a968d2a7ef..07a07d7253 100644 --- a/internal/service/advancedcluster/model_advanced_cluster_test.go +++ b/internal/service/advancedcluster/model_advanced_cluster_test.go @@ -3,6 +3,7 @@ package advancedcluster_test import ( "context" "errors" + "fmt" "net/http" "testing" @@ -187,7 +188,7 @@ func TestGetDiskSizeGBFromReplicationSpec(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { result := advancedcluster.GetDiskSizeGBFromReplicationSpec(&tc.clusterDescription) - assert.InEpsilon(t, tc.expectedDiskSizeResult, result, 0.001) // using InEpsilon as type is float64 + assert.Equal(t, fmt.Sprintf("%.f", tc.expectedDiskSizeResult), fmt.Sprintf("%.f", result)) // formatting to string to avoid float comparison }) } } From 66ee5734920078dc686af6c4896daafb33b5d3dc Mon Sep 17 00:00:00 2001 From: Agustin Bettati Date: Thu, 11 Jul 2024 10:13:29 +0200 Subject: [PATCH 8/9] fix merge conflicts --- .../advancedcluster/model_advanced_cluster.go | 2 +- .../model_advanced_cluster_test.go | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/service/advancedcluster/model_advanced_cluster.go b/internal/service/advancedcluster/model_advanced_cluster.go index 4dbab2e251..69ba3d95f5 100644 --- a/internal/service/advancedcluster/model_advanced_cluster.go +++ b/internal/service/advancedcluster/model_advanced_cluster.go @@ -280,7 +280,7 @@ func IsSharedTier(instanceSize string) bool { // GetDiskSizeGBFromReplicationSpec obtains the diskSizeGB value by looking into the electable spec of the first replication spec. // Independent storage size scaling is not supported (CLOUDP-201331), meaning all electable/analytics/readOnly configs in all replication specs are the same. -func GetDiskSizeGBFromReplicationSpec(cluster *admin.ClusterDescription20240710) float64 { +func GetDiskSizeGBFromReplicationSpec(cluster *admin.ClusterDescription20250101) float64 { specs := cluster.GetReplicationSpecs() if len(specs) < 1 { return 0 diff --git a/internal/service/advancedcluster/model_advanced_cluster_test.go b/internal/service/advancedcluster/model_advanced_cluster_test.go index afb50df591..1c117ad922 100644 --- a/internal/service/advancedcluster/model_advanced_cluster_test.go +++ b/internal/service/advancedcluster/model_advanced_cluster_test.go @@ -153,35 +153,35 @@ func TestFlattenReplicationSpecs(t *testing.T) { func TestGetDiskSizeGBFromReplicationSpec(t *testing.T) { diskSizeGBValue := 40.0 - hardwareSpec := admin.HardwareSpec20240710{ + hardwareSpec := admin.HardwareSpec20250101{ DiskSizeGB: admin.PtrFloat64(diskSizeGBValue), } - regionConfig := []admin.CloudRegionConfig20240710{{ + regionConfig := []admin.CloudRegionConfig20250101{{ ElectableSpecs: &hardwareSpec, }} - replicationSpecsWithValue := []admin.ReplicationSpec20240710{{RegionConfigs: ®ionConfig}} + replicationSpecsWithValue := []admin.ReplicationSpec20250101{{RegionConfigs: ®ionConfig}} - emptyRegionConfig := []admin.CloudRegionConfig20240710{{}} - replicationSpecsEmptyElectable := []admin.ReplicationSpec20240710{{RegionConfigs: &emptyRegionConfig}} + emptyRegionConfig := []admin.CloudRegionConfig20250101{{}} + replicationSpecsEmptyElectable := []admin.ReplicationSpec20250101{{RegionConfigs: &emptyRegionConfig}} testCases := map[string]struct { - clusterDescription admin.ClusterDescription20240710 + clusterDescription admin.ClusterDescription20250101 expectedDiskSizeResult float64 }{ "cluster description with disk size gb value at electable spec": { - clusterDescription: admin.ClusterDescription20240710{ + clusterDescription: admin.ClusterDescription20250101{ ReplicationSpecs: &replicationSpecsWithValue, }, expectedDiskSizeResult: diskSizeGBValue, }, "cluster description with no electable spec": { - clusterDescription: admin.ClusterDescription20240710{ + clusterDescription: admin.ClusterDescription20250101{ ReplicationSpecs: &replicationSpecsEmptyElectable, }, expectedDiskSizeResult: 0, }, "cluster description with no replication spec": { - clusterDescription: admin.ClusterDescription20240710{}, + clusterDescription: admin.ClusterDescription20250101{}, expectedDiskSizeResult: 0, }, } From 8ae8973f3637e67c348f4bc6545fe511d80af012 Mon Sep 17 00:00:00 2001 From: Agustin Bettati Date: Thu, 11 Jul 2024 17:39:42 +0200 Subject: [PATCH 9/9] include structs defined in test case --- .../model_advanced_cluster_test.go | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/internal/service/advancedcluster/model_advanced_cluster_test.go b/internal/service/advancedcluster/model_advanced_cluster_test.go index 1c117ad922..ebb0668da0 100644 --- a/internal/service/advancedcluster/model_advanced_cluster_test.go +++ b/internal/service/advancedcluster/model_advanced_cluster_test.go @@ -153,16 +153,6 @@ func TestFlattenReplicationSpecs(t *testing.T) { func TestGetDiskSizeGBFromReplicationSpec(t *testing.T) { diskSizeGBValue := 40.0 - hardwareSpec := admin.HardwareSpec20250101{ - DiskSizeGB: admin.PtrFloat64(diskSizeGBValue), - } - regionConfig := []admin.CloudRegionConfig20250101{{ - ElectableSpecs: &hardwareSpec, - }} - replicationSpecsWithValue := []admin.ReplicationSpec20250101{{RegionConfigs: ®ionConfig}} - - emptyRegionConfig := []admin.CloudRegionConfig20250101{{}} - replicationSpecsEmptyElectable := []admin.ReplicationSpec20250101{{RegionConfigs: &emptyRegionConfig}} testCases := map[string]struct { clusterDescription admin.ClusterDescription20250101 @@ -170,13 +160,21 @@ func TestGetDiskSizeGBFromReplicationSpec(t *testing.T) { }{ "cluster description with disk size gb value at electable spec": { clusterDescription: admin.ClusterDescription20250101{ - ReplicationSpecs: &replicationSpecsWithValue, + ReplicationSpecs: &[]admin.ReplicationSpec20250101{{ + RegionConfigs: &[]admin.CloudRegionConfig20250101{{ + ElectableSpecs: &admin.HardwareSpec20250101{ + DiskSizeGB: admin.PtrFloat64(diskSizeGBValue), + }, + }}, + }}, }, expectedDiskSizeResult: diskSizeGBValue, }, "cluster description with no electable spec": { clusterDescription: admin.ClusterDescription20250101{ - ReplicationSpecs: &replicationSpecsEmptyElectable, + ReplicationSpecs: &[]admin.ReplicationSpec20250101{ + {RegionConfigs: &[]admin.CloudRegionConfig20250101{{}}}, + }, }, expectedDiskSizeResult: 0, },