Skip to content

Commit

Permalink
refactor: Reuses advanced_cluster logic in SDKv2 & TPF initial sing…
Browse files Browse the repository at this point in the history
…le file `common.go` (#2947)

* refactor: Moves TestAccMockableAdvancedCluster_basicTenant to SDKv2

* test(unit): use IsTfLogDebug and support logging config in both REPLAY & CAPTURE

* refactor: move TPF resource_tests to sdkv2

* refactor: move testdata for TestAccMockableAdvancedCluster_replicasetAdvConfigUpdate.yaml

* refactor: move testdata for TestAccMockableAdvancedCluster_shardedBasic.yaml

* refactor: move testdata for TestAccMockableAdvancedCluster_symmetricShardedOldSchemaDiskSizeGBAtElectableLevel.yaml

* chore: log also query string in request matching and add QueryVars to mockConfig

* chore: Support testmact in makefile and use it in github action

* feat: add timeouts attribute support in advanced cluster schema conversion

* refactor: consolidate data source definitions in advanced cluster tests

* feat: add support for HTTP mock capture in testmact-capture target

* refactor: move testdata for TestAccMockableAdvancedCluster_tenantUpgrade.yaml

* refactor: move testdata for TestAccMockableAdvancedCluster_symmetricShardedOldSchema.yaml

* test: ensure timeouts.create attribute is only checked on resource

* fix: enforce explicit setting of ACCTEST_PACKAGES for testmact targets

* chore: regenerate test files

* chore: Add ORG_ID env var when using replay

* fix: update minimum_enabled_tls_protocol to TLS1.2 and refactor timeout check in advanced cluster tests

* test: Add more checks to new acceptance tests

* fix: update advanced cluster test data sources to include new schema

* chore: regen replicasetAdvConfigUpdate test

* fix: update acceptance tests configuration to use correct package path and add regex for test selection

* test: Refactor tests to use consistent TF config for data sources

* fix: update advanced cluster test to correct timeout attribute and refactor update for TestAccMockableAdvancedCluster_replicasetAdvConfigUpdate

* refactor: remove test cases functions for new tests

* chore: use new schema for testmact if it is not set

* chore: regenerate mock data

* refactor: Moves AddIDsToReplicationSpecs to TPF and reuse FormatMongoDBMajorVersion

* refactor: Replace getAdvancedClusterContainerID with advancedclustertpf.GetAdvancedClusterContainerID for improved code reuse

* refactor: Reuse advanced configuration handling in createCluster function

* refactor: Update acceptance tests runner to include common.go in advanced cluster filters

* refactor: rename test function and make update compatible with SDKv2

* refactor: remove redundant VERSION assignments in GNUmakefile [ci skip]

* refactor: remove redundant VERSION assignment in GNUmakefile

* apply PR suggestion
  • Loading branch information
EspenAlbert authored Jan 10, 2025
1 parent c13ae3b commit 19c703f
Show file tree
Hide file tree
Showing 9 changed files with 187 additions and 218 deletions.
1 change: 1 addition & 0 deletions .github/workflows/acceptance-tests-runner.yml
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ jobs:
filters: |
advanced_cluster:
- 'internal/service/advancedcluster/!(*_test).go' # matches any adv_cluster file change except test files
- 'internal/service/advancedclustertpf/common.go'
advanced_cluster_tpf:
- 'internal/service/advancedclustertpf/*.go'
- 'internal/service/advancedcluster/*_test.go'
Expand Down
4 changes: 0 additions & 4 deletions GNUmakefile
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ test: fmtcheck

.PHONY: testmact
testmact:
@$(eval VERSION=macct)
@$(eval ACCTEST_REGEX_RUN?=^TestAccMockable)
@$(eval export HTTP_MOCKER_REPLAY?=true)
@$(eval export HTTP_MOCKER_CAPTURE?=false)
Expand All @@ -57,7 +56,6 @@ testmact:

.PHONY: testmact-capture
testmact-capture:
@$(eval VERSION=macct)
@$(eval export ACCTEST_REGEX_RUN?=^TestAccMockable)
@$(eval export HTTP_MOCKER_REPLAY?=false)
@$(eval export HTTP_MOCKER_CAPTURE?=true)
Expand All @@ -69,13 +67,11 @@ testmact-capture:

.PHONY: testacc
testacc: fmtcheck
@$(eval VERSION=acc)
@$(eval ACCTEST_REGEX_RUN?=^TestAcc)
TF_ACC=1 go test $(ACCTEST_PACKAGES) -run '$(ACCTEST_REGEX_RUN)' -v -parallel $(PARALLEL_GO_TEST) $(TESTARGS) -timeout $(ACCTEST_TIMEOUT) -ldflags="$(LINKER_FLAGS)"

.PHONY: testaccgov
testaccgov: fmtcheck
@$(eval VERSION=acc)
TF_ACC=1 go test ./... -run 'TestAccProjectRSGovProject_CreateWithProjectOwner' -v -parallel 1 "$(TESTARGS) -timeout $(ACCTEST_TIMEOUT) -ldflags=$(LINKER_FLAGS) "

.PHONY: fmt
Expand Down
25 changes: 3 additions & 22 deletions internal/service/advancedcluster/model_advanced_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/constant"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/conversion"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/service/advancedclustertpf"
)

const minVersionForChangeStreamOptions = 6.0
Expand Down Expand Up @@ -404,10 +405,7 @@ func ResourceClusterListAdvancedRefreshFunc(ctx context.Context, projectID strin
}

func FormatMongoDBMajorVersion(val any) string {
if strings.Contains(val.(string), ".") {
return val.(string)
}
return fmt.Sprintf("%.1f", cast.ToFloat32(val))
return advancedclustertpf.FormatMongoDBMajorVersion(val.(string))
}

func flattenLabels(l []admin.ComponentLabel) []map[string]string {
Expand Down Expand Up @@ -711,7 +709,7 @@ func flattenAdvancedReplicationSpecRegionConfigs(ctx context.Context, apiObjects
if err != nil {
return nil, nil, err
}
if result := getAdvancedClusterContainerID(containers.GetResults(), &apiObject); result != "" {
if result := advancedclustertpf.GetAdvancedClusterContainerID(containers.GetResults(), &apiObject); result != "" {
// Will print as "providerName:regionName" = "containerId" in terraform show
containerIDs[fmt.Sprintf("%s:%s", apiObject.GetProviderName(), apiObject.GetRegionName())] = result
}
Expand Down Expand Up @@ -845,23 +843,6 @@ func flattenAdvancedReplicationSpecAutoScaling(apiObject *admin.AdvancedAutoScal
return tfList
}

func getAdvancedClusterContainerID(containers []admin.CloudProviderContainer, cluster *admin.CloudRegionConfig20240805) string {
if len(containers) == 0 {
return ""
}
for i := range containers {
if cluster.GetProviderName() == constant.GCP {
return containers[i].GetId()
}
if containers[i].GetProviderName() == cluster.GetProviderName() &&
containers[i].GetRegion() == cluster.GetRegionName() || // For Azure
containers[i].GetRegionName() == cluster.GetRegionName() { // For AWS
return containers[i].GetId()
}
}
return ""
}

func expandProcessArgs(d *schema.ResourceData, p map[string]any, mongodbMajorVersion *string) (admin20240530.ClusterDescriptionProcessArgs, admin.ClusterDescriptionProcessArgs20240805) {
res20240530 := admin20240530.ClusterDescriptionProcessArgs{}
res := admin.ClusterDescriptionProcessArgs20240805{}
Expand Down
20 changes: 2 additions & 18 deletions internal/service/advancedcluster/resource_update_logic.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/conversion"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/service/advancedclustertpf"
"go.mongodb.org/atlas-sdk/v20241113004/admin"
)

Expand All @@ -31,27 +32,10 @@ func populateIDValuesUsingNewAPI(ctx context.Context, projectID, clusterName str
}

zoneToReplicationSpecsIDs := groupIDsByZone(cluster.GetReplicationSpecs())
result := AddIDsToReplicationSpecs(*replicationSpecs, zoneToReplicationSpecsIDs)
result := advancedclustertpf.AddIDsToReplicationSpecs(*replicationSpecs, zoneToReplicationSpecsIDs)
return &result, nil
}

func AddIDsToReplicationSpecs(replicationSpecs []admin.ReplicationSpec20240805, zoneToReplicationSpecsIDs map[string][]string) []admin.ReplicationSpec20240805 {
for zoneName, availableIDs := range zoneToReplicationSpecsIDs {
var indexOfIDToUse = 0
for i := range replicationSpecs {
if indexOfIDToUse >= len(availableIDs) {
break // all available ids for this zone have been used
}
if replicationSpecs[i].GetZoneName() == zoneName {
newID := availableIDs[indexOfIDToUse]
indexOfIDToUse++
replicationSpecs[i].Id = &newID
}
}
}
return replicationSpecs
}

func groupIDsByZone(specs []admin.ReplicationSpec20240805) map[string][]string {
result := make(map[string][]string)
for _, spec := range specs {
Expand Down
116 changes: 0 additions & 116 deletions internal/service/advancedcluster/resource_update_logic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,122 +8,6 @@ import (
"go.mongodb.org/atlas-sdk/v20241113004/admin"
)

func TestAddIDsToReplicationSpecs(t *testing.T) {
testCases := map[string]struct {
ReplicationSpecs []admin.ReplicationSpec20240805
ZoneToReplicationSpecsIDs map[string][]string
ExpectedReplicationSpecs []admin.ReplicationSpec20240805
}{
"two zones with same amount of available ids and replication specs to populate": {
ReplicationSpecs: []admin.ReplicationSpec20240805{
{
ZoneName: admin.PtrString("Zone 1"),
},
{
ZoneName: admin.PtrString("Zone 2"),
},
{
ZoneName: admin.PtrString("Zone 1"),
},
{
ZoneName: admin.PtrString("Zone 2"),
},
},
ZoneToReplicationSpecsIDs: map[string][]string{
"Zone 1": {"zone1-id1", "zone1-id2"},
"Zone 2": {"zone2-id1", "zone2-id2"},
},
ExpectedReplicationSpecs: []admin.ReplicationSpec20240805{
{
ZoneName: admin.PtrString("Zone 1"),
Id: admin.PtrString("zone1-id1"),
},
{
ZoneName: admin.PtrString("Zone 2"),
Id: admin.PtrString("zone2-id1"),
},
{
ZoneName: admin.PtrString("Zone 1"),
Id: admin.PtrString("zone1-id2"),
},
{
ZoneName: admin.PtrString("Zone 2"),
Id: admin.PtrString("zone2-id2"),
},
},
},
"less available ids than replication specs to populate": {
ReplicationSpecs: []admin.ReplicationSpec20240805{
{
ZoneName: admin.PtrString("Zone 1"),
},
{
ZoneName: admin.PtrString("Zone 1"),
},
{
ZoneName: admin.PtrString("Zone 1"),
},
{
ZoneName: admin.PtrString("Zone 2"),
},
},
ZoneToReplicationSpecsIDs: map[string][]string{
"Zone 1": {"zone1-id1"},
"Zone 2": {"zone2-id1"},
},
ExpectedReplicationSpecs: []admin.ReplicationSpec20240805{
{
ZoneName: admin.PtrString("Zone 1"),
Id: admin.PtrString("zone1-id1"),
},
{
ZoneName: admin.PtrString("Zone 1"),
Id: nil,
},
{
ZoneName: admin.PtrString("Zone 1"),
Id: nil,
},
{
ZoneName: admin.PtrString("Zone 2"),
Id: admin.PtrString("zone2-id1"),
},
},
},
"more available ids than replication specs to populate": {
ReplicationSpecs: []admin.ReplicationSpec20240805{
{
ZoneName: admin.PtrString("Zone 1"),
},
{
ZoneName: admin.PtrString("Zone 2"),
},
},
ZoneToReplicationSpecsIDs: map[string][]string{
"Zone 1": {"zone1-id1", "zone1-id2"},
"Zone 2": {"zone2-id1", "zone2-id2"},
},
ExpectedReplicationSpecs: []admin.ReplicationSpec20240805{
{
ZoneName: admin.PtrString("Zone 1"),
Id: admin.PtrString("zone1-id1"),
},
{
ZoneName: admin.PtrString("Zone 2"),
Id: admin.PtrString("zone2-id1"),
},
},
},
}

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

func TestSyncAutoScalingConfigs(t *testing.T) {
testCases := map[string]struct {
ReplicationSpecs []admin.ReplicationSpec20240805
Expand Down
46 changes: 46 additions & 0 deletions internal/service/advancedclustertpf/common.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package advancedclustertpf

import (
"fmt"
"strings"

"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/constant"
"github.com/spf13/cast"
"go.mongodb.org/atlas-sdk/v20241113004/admin"
)

func FormatMongoDBMajorVersion(version string) string {
if strings.Contains(version, ".") {
return version
}
return fmt.Sprintf("%.1f", cast.ToFloat32(version))
}

func AddIDsToReplicationSpecs(replicationSpecs []admin.ReplicationSpec20240805, zoneToReplicationSpecsIDs map[string][]string) []admin.ReplicationSpec20240805 {
for zoneName, availableIDs := range zoneToReplicationSpecsIDs {
indexOfIDToUse := 0
for i := range replicationSpecs {
if indexOfIDToUse >= len(availableIDs) {
break // all available ids for this zone have been used
}
if replicationSpecs[i].GetZoneName() == zoneName {
newID := availableIDs[indexOfIDToUse]
indexOfIDToUse++
replicationSpecs[i].Id = &newID
}
}
}
return replicationSpecs
}

func GetAdvancedClusterContainerID(containers []admin.CloudProviderContainer, cluster *admin.CloudRegionConfig20240805) string {
for i, container := range containers {
gpc := cluster.GetProviderName() == constant.GCP
azure := container.GetProviderName() == cluster.GetProviderName() && container.GetRegion() == cluster.GetRegionName()
aws := container.GetRegionName() == cluster.GetRegionName()
if gpc || azure || aws {
return containers[i].GetId()
}
}
return ""
}
Loading

0 comments on commit 19c703f

Please sign in to comment.