Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
3 changes: 3 additions & 0 deletions .changelog/3162.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/mongodbatlas_advanced_cluster (preview provider 2.0.0): Avoid error when removing `read_only_specs` in `region_configs` that does not define `electable_specs`
Copy link
Member Author

Choose a reason for hiding this comment

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

using mongodbatlas_advanced_cluster (preview provider 2.0.0) in the changelog

```
Original file line number Diff line number Diff line change
Expand Up @@ -213,14 +213,23 @@ func replicaSetAWSProviderTestCase(t *testing.T, isAcc bool) resource.TestCase {
}),
Check: checkReplicaSetAWSProvider(isAcc, projectID, clusterName, 60, 3, true, true),
},
// empty plan when analytics block is removed
acc.TestStepCheckEmptyPlan(configAWSProvider(t, isAcc, ReplicaSetAWSConfig{
ProjectID: projectID,
ClusterName: clusterName,
ClusterType: "REPLICASET",
DiskSizeGB: 60,
NodeCountElectable: 3,
WithAnalyticsSpecs: false,
})),
{
Config: configAWSProvider(t, isAcc, ReplicaSetAWSConfig{
ProjectID: projectID,
ClusterName: clusterName,
ClusterType: "REPLICASET",
DiskSizeGB: 50,
NodeCountElectable: 5,
WithAnalyticsSpecs: false, // removed as part of other updates, computed value is expected to be the same
WithAnalyticsSpecs: false, // other update made after removed analytics block, computed value is expected to be the same
}),
Check: checkReplicaSetAWSProvider(isAcc, projectID, clusterName, 50, 5, true, true),
},
Expand Down Expand Up @@ -557,8 +566,10 @@ func TestAccClusterAdvancedClusterConfig_replicationSpecsAutoScaling(t *testing.
acc.TestCheckResourceAttrPreviewProviderV2(true, resourceName, "replication_specs.0.region_configs.0.electable_specs.0.disk_size_gb", "10"), // modified disk size gb in config is ignored
),
},
// empty plan when auto_scaling block is removed
acc.TestStepCheckEmptyPlan(configReplicationSpecsAutoScaling(t, true, projectID, clusterName, nil, "M20", 20, 1)),
{
Config: configReplicationSpecsAutoScaling(t, true, projectID, clusterName, nil, "M10", 10, 2), // auto_scaling block removed together with other changes, preserves previous state
Config: configReplicationSpecsAutoScaling(t, true, projectID, clusterName, nil, "M10", 10, 2), // other change after autoscaling block removed, preserves previous state
Check: resource.ComposeAggregateTestCheckFunc(
acc.CheckExistsCluster(resourceName),
acc.TestCheckResourceAttrPreviewProviderV2(true, resourceName, "name", clusterName),
Expand Down Expand Up @@ -611,8 +622,10 @@ func TestAccClusterAdvancedClusterConfig_replicationSpecsAnalyticsAutoScaling(t
acc.TestCheckResourceAttrPreviewProviderV2(true, resourceName, "replication_specs.0.region_configs.0.analytics_auto_scaling.0.compute_enabled", "true"),
),
},
// empty plan when analytics_auto_scaling block is removed
acc.TestStepCheckEmptyPlan(configReplicationSpecsAnalyticsAutoScaling(t, true, projectID, clusterNameUpdated, nil, 1)),
{
Config: configReplicationSpecsAnalyticsAutoScaling(t, true, projectID, clusterNameUpdated, nil, 2), // analytics_auto_scaling block removed together with other changes, preserves previous state
Config: configReplicationSpecsAnalyticsAutoScaling(t, true, projectID, clusterNameUpdated, nil, 2), // other changes after analytics_auto_scaling block removed, preserves previous state
Check: resource.ComposeAggregateTestCheckFunc(
acc.CheckExistsCluster(resourceName),
acc.TestCheckResourceAttrPreviewProviderV2(true, resourceName, "name", clusterNameUpdated),
Expand Down Expand Up @@ -1375,23 +1388,25 @@ func TestAccMockableAdvancedCluster_shardedAddAnalyticsAndAutoScaling(t *testing
})
}

func TestAccMockableAdvancedCluster_removeBlocksFromConfig(t *testing.T) {
func TestAccAdvancedCluster_removeBlocksFromConfig(t *testing.T) {
Copy link
Member Author

Choose a reason for hiding this comment

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

refactored removeBlocksFromConfig from a Mockable to regular test.
Main motivation was that execution of this test is long (+40min) and getting a successful local execution for updating golden files is a challenge. @lantoli I know you recently added this test, let me know if you see a concern with this.

if !config.PreviewProviderV2AdvancedCluster() { // SDKv2 don't set "computed" specs in the state
t.Skip("This test is not applicable for SDKv2")
}
var (
projectID, clusterName = acc.ProjectIDExecutionWithCluster(t, 15)
)
unit.CaptureOrMockTestCaseAndRun(t, mockConfig, &resource.TestCase{
resource.ParallelTest(t, resource.TestCase{
ProtoV6ProviderFactories: acc.TestAccProviderV6Factories,
Steps: []resource.TestStep{
{
Config: configBlocks(t, projectID, clusterName, true),
Check: checkBlocks(true),
Config: configBlocks(t, projectID, clusterName, "M10", true),
Check: checkBlocks("M10"),
},
// removing blocks generates an empty plan
acc.TestStepCheckEmptyPlan(configBlocks(t, projectID, clusterName, "M10", false)),
{
Config: configBlocks(t, projectID, clusterName, false),
Check: checkBlocks(false),
Config: configBlocks(t, projectID, clusterName, "M20", false), // applying a change after removing blocks preserves previous state
Check: checkBlocks("M20"),
},
acc.TestStepImportCluster(resourceName),
},
Expand Down Expand Up @@ -1491,10 +1506,10 @@ func configSharded(t *testing.T, projectID, clusterName string, withUpdate bool)
`, projectID, clusterName, autoScaling, analyticsSpecs, analyticsSpecsForSpec2)) + dataSourcesTFNewSchema
}

func configBlocks(t *testing.T, projectID, clusterName string, firstStep bool) string {
func configBlocks(t *testing.T, projectID, clusterName, instanceSize string, defineBlocks bool) string {
t.Helper()
var extraConfig0, extraConfig1 string
autoStr := `
autoScalingBlocks := `
auto_scaling {
disk_gb_enabled = true
compute_enabled = true
Expand All @@ -1510,15 +1525,15 @@ func configBlocks(t *testing.T, projectID, clusterName string, firstStep bool) s
compute_scale_down_enabled = true
}
`
instanceSize1 := "M20"
if firstStep {
instanceSize1 = "M10"
if defineBlocks {
// read only + autoscaling blocks
extraConfig0 = `
read_only_specs {
instance_size = "M10"
node_count = 2
}
` + autoStr
` + autoScalingBlocks
// read only + analytics + autoscaling blocks
extraConfig1 = `
read_only_specs {
instance_size = "M10"
Expand All @@ -1528,7 +1543,7 @@ func configBlocks(t *testing.T, projectID, clusterName string, firstStep bool) s
instance_size = "M10"
node_count = 4
}
` + autoStr
` + autoScalingBlocks
}
return acc.ConvertAdvancedClusterToPreviewProviderV2(t, true, fmt.Sprintf(`
resource "mongodbatlas_advanced_cluster" "test" {
Expand Down Expand Up @@ -1562,29 +1577,34 @@ func configBlocks(t *testing.T, projectID, clusterName string, firstStep bool) s
}
%[5]s
}
region_configs { // region with no electable specs
provider_name = "AWS"
priority = 0
region_name = "US_EAST_1"
%[4]s
}
}
}
`, projectID, clusterName, instanceSize1, extraConfig0, extraConfig1))
`, projectID, clusterName, instanceSize, extraConfig0, extraConfig1))
}

func checkBlocks(firstStep bool) resource.TestCheckFunc {
func checkBlocks(instanceSize string) resource.TestCheckFunc {
checksMap := map[string]string{
"replication_specs.0.region_configs.0.electable_specs.0.instance_size": "M10",
"replication_specs.0.region_configs.0.electable_specs.0.node_count": "5",
"replication_specs.0.region_configs.0.read_only_specs.0.instance_size": "M10",
"replication_specs.0.region_configs.0.read_only_specs.0.node_count": "2",
"replication_specs.0.region_configs.0.analytics_specs.0.node_count": "0",

"replication_specs.1.region_configs.0.electable_specs.0.instance_size": "M10",
"replication_specs.1.region_configs.0.electable_specs.0.instance_size": instanceSize,
"replication_specs.1.region_configs.0.electable_specs.0.node_count": "3",
"replication_specs.1.region_configs.0.read_only_specs.0.instance_size": "M10",
"replication_specs.1.region_configs.0.read_only_specs.0.instance_size": instanceSize,
"replication_specs.1.region_configs.0.read_only_specs.0.node_count": "1",
"replication_specs.1.region_configs.0.analytics_specs.0.instance_size": "M10",
"replication_specs.1.region_configs.0.analytics_specs.0.node_count": "4",
}
if !firstStep {
checksMap["replication_specs.1.region_configs.0.electable_specs.0.instance_size"] = "M20"
checksMap["replication_specs.1.region_configs.0.read_only_specs.0.instance_size"] = "M20"

"replication_specs.1.region_configs.1.read_only_specs.0.instance_size": "M10",
"replication_specs.1.region_configs.1.read_only_specs.0.node_count": "2",
}
for repSpecsIdx := range 2 {
for _, block := range []string{"auto_scaling", "analytics_auto_scaling"} {
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

14 changes: 9 additions & 5 deletions internal/service/advancedclustertpf/plan_modifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,20 @@ func AdjustRegionConfigsChildren(ctx context.Context, diags *diag.Diagnostics, s
stateReadOnlySpecs := TFModelObject[TFSpecsModel](ctx, stateRegionConfigsTF[j].ReadOnlySpecs)
planReadOnlySpecs := TFModelObject[TFSpecsModel](ctx, planRegionConfigsTF[j].ReadOnlySpecs)
planElectableSpecs := TFModelObject[TFSpecsModel](ctx, planRegionConfigsTF[j].ElectableSpecs)
if stateReadOnlySpecs != nil && planElectableSpecs != nil { // read_only_specs is present in state and electable_specs in the plan
if stateReadOnlySpecs != nil { // read_only_specs is present in state
newPlanReadOnlySpecs := planReadOnlySpecs
if newPlanReadOnlySpecs == nil {
newPlanReadOnlySpecs = new(TFSpecsModel) // start with null attributes if not present plan
}
baseReadOnlySpecs := stateReadOnlySpecs
if planElectableSpecs != nil { // if electable_specs is in plan we rely on those values
baseReadOnlySpecs = planElectableSpecs
}
copyAttrIfDestNotKnown(&baseReadOnlySpecs.DiskSizeGb, &newPlanReadOnlySpecs.DiskSizeGb)
copyAttrIfDestNotKnown(&baseReadOnlySpecs.EbsVolumeType, &newPlanReadOnlySpecs.EbsVolumeType)
copyAttrIfDestNotKnown(&baseReadOnlySpecs.InstanceSize, &newPlanReadOnlySpecs.InstanceSize)
copyAttrIfDestNotKnown(&baseReadOnlySpecs.DiskIops, &newPlanReadOnlySpecs.DiskIops)
// unknown node_count is got from state, all other unknowns are got from electable_specs plan
copyAttrIfDestNotKnown(&planElectableSpecs.DiskSizeGb, &newPlanReadOnlySpecs.DiskSizeGb)
copyAttrIfDestNotKnown(&planElectableSpecs.EbsVolumeType, &newPlanReadOnlySpecs.EbsVolumeType)
copyAttrIfDestNotKnown(&planElectableSpecs.InstanceSize, &newPlanReadOnlySpecs.InstanceSize)
copyAttrIfDestNotKnown(&planElectableSpecs.DiskIops, &newPlanReadOnlySpecs.DiskIops)
copyAttrIfDestNotKnown(&stateReadOnlySpecs.NodeCount, &newPlanReadOnlySpecs.NodeCount)
objType, diagsLocal := types.ObjectValueFrom(ctx, SpecsObjType.AttrTypes, newPlanReadOnlySpecs)
diags.Append(diagsLocal...)
Expand Down
Loading
Loading