Skip to content

fix: Avoid error when removing read only spec in region config that does not define electable specs #3162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Mar 14, 2025
Merged

fix: Avoid error when removing read only spec in region config that does not define electable specs #3162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6826179
add additional step for ensuring empty plans are encountered when blo…
AgustinBettati Mar 12, 2025
7d9e609
adjust test step check making it different from one for migration tests
AgustinBettati Mar 12, 2025
61232c9
add logic for copying values from state for read only spec if electab…
AgustinBettati Mar 12, 2025
096a23a
refactor removeBlocksFromConfig into regular acceptance test
AgustinBettati Mar 12, 2025
d8c94ef
adding changelog
AgustinBettati Mar 13, 2025
86f5706
fix replicationSpecsAutoScaling avoiding misalignment of instance_siz…
AgustinBettati Mar 13, 2025
9709558
fix removeBlocksFromConfig by using node_count instead of instance_si…
AgustinBettati Mar 13, 2025
ff33457
fix for handling change of instance size in separate region config
AgustinBettati Mar 14, 2025
f8ba8ca
Revert "fix removeBlocksFromConfig by using node_count instead of ins…
AgustinBettati Mar 14, 2025
7870b3c
favour electable specs of same region config over one defined in gene…
AgustinBettati Mar 14, 2025
d857fff
adjust check to consider change in read only spec of different region…
AgustinBettati Mar 14, 2025
152afb5
adding additional docs clarification comments
AgustinBettati Mar 14, 2025
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
View file
Open in desktop
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): Avoids error when removing `read_only_specs` in `region_configs` that does not define `electable_specs`
```
68 changes: 44 additions & 24 deletions internal/service/advancedcluster/resource_advanced_cluster_test.go
View file
Open in desktop
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 (also aligns instance_size/disk_size_gb to values in state)
acc.TestStepCheckEmptyPlan(configReplicationSpecsAutoScaling(t, true, projectID, clusterName, nil, "M10", 10, 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": instanceSize,
"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
312 changes: 0 additions & 312 deletions ...rvice/advancedcluster/testdata/TestAccMockableAdvancedCluster_removeBlocksFromConfig.yaml
View file
Open in desktop

This file was deleted.

92 changes: 0 additions & 92 deletions ...removeBlocksFromConfig/01_01_POST__api_atlas_v2_groups_{groupId}_clusters_2024-10-23.json
View file
Open in desktop

This file was deleted.

95 changes: 0 additions & 95 deletions ...mConfig/02_01_PATCH__api_atlas_v2_groups_{groupId}_clusters_{clusterName}_2024-10-23.json
View file
Open in desktop

This file was deleted.

Empty file removed ...Config/03_01_DELETE__api_atlas_v2_groups_{groupId}_clusters_{clusterName}_2023-02-01.json
View file
Open in desktop
Empty file.
Loading
Loading