Skip to content

Commit

Permalink
chore: Uses use_replication_spec_per_shard in data sources (#2896)
Browse files Browse the repository at this point in the history
* enable TestAccClusterAdvancedClusterConfig_asymmetricShardedNewSchema

* overrideUsingLegacySchema

* numShardsMapFromOldAPI

* refactor ExtraAPIInfo build

* address feedback

* typo

* fill some computed null values

* Revert "fill some computed null values"

This reverts commit bab0bcf.

* fill null value for BackingProviderName

* clause when legacy cluster model can't be created

* Revert "fill null value for BackingProviderName"

This reverts commit 1fb37c1.

* skip id checks in replication_specs
  • Loading branch information
lantoli authored Dec 16, 2024
1 parent 064955c commit 61eda15
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 48 deletions.
31 changes: 16 additions & 15 deletions internal/service/advancedcluster/resource_advanced_cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/stretchr/testify/mock"

"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/conversion"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/config"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/service/advancedcluster"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/testutil/acc"
)
Expand Down Expand Up @@ -771,8 +772,6 @@ func TestAccClusterAdvancedClusterConfig_symmetricShardedNewSchemaToAsymmetricAd
}

func TestAccClusterAdvancedClusterConfig_asymmetricShardedNewSchema(t *testing.T) {
// TODO: enable when datasource attribute use_replication_spec_per_shard is used
acc.SkipIfAdvancedClusterV2Schema(t)
resource.ParallelTest(t, asymmetricShardedNewSchemaTestCase(t, true))
}

Expand Down Expand Up @@ -2141,19 +2140,21 @@ func checkShardedNewSchema(isAcc bool, diskSizeGB int, firstInstanceSize, lastIn

pluralChecks = acc.AddAttrChecksPrefixSchemaV2(isAcc, dataSourcePluralName, pluralChecks, clusterChecks, "results.0")

// expected id attribute only if cluster is symmetric
if isAsymmetricCluster {
pluralChecks = append(pluralChecks, checkAggr(isAcc, []string{}, map[string]string{
"replication_specs.0.id": "",
"replication_specs.1.id": "",
}))
pluralChecks = acc.AddAttrChecksSchemaV2(isAcc, dataSourcePluralName, pluralChecks, map[string]string{
"results.0.replication_specs.0.id": "",
"results.0.replication_specs.1.id": "",
})
} else {
pluralChecks = append(pluralChecks, checkAggr(isAcc, []string{"replication_specs.0.id", "replication_specs.1.id"}, map[string]string{}))
pluralChecks = acc.AddAttrSetChecksSchemaV2(isAcc, dataSourcePluralName, pluralChecks, "results.0.replication_specs.0.id", "results.0.replication_specs.1.id")
if !config.AdvancedClusterV2Schema() { // TODDO: id is not filled yet in
// expected id attribute only if cluster is symmetric
if isAsymmetricCluster {
pluralChecks = append(pluralChecks, checkAggr(isAcc, []string{}, map[string]string{
"replication_specs.0.id": "",
"replication_specs.1.id": "",
}))
pluralChecks = acc.AddAttrChecksSchemaV2(isAcc, dataSourcePluralName, pluralChecks, map[string]string{
"results.0.replication_specs.0.id": "",
"results.0.replication_specs.1.id": "",
})
} else {
pluralChecks = append(pluralChecks, checkAggr(isAcc, []string{"replication_specs.0.id", "replication_specs.1.id"}, map[string]string{}))
pluralChecks = acc.AddAttrSetChecksSchemaV2(isAcc, dataSourcePluralName, pluralChecks, "results.0.replication_specs.0.id", "results.0.replication_specs.1.id")
}
}

return checkAggr(isAcc,
Expand Down
3 changes: 1 addition & 2 deletions internal/service/advancedclustertpf/data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ func (d *ds) readCluster(ctx context.Context, diags *diag.Diagnostics, modelDS *
ProjectID: modelDS.ProjectID,
Name: modelDS.Name,
}
// TODO: pass !UseReplicationSpecPerShard to overrideUsingLegacySchema
modelOut, extraInfo := getBasicClusterModel(ctx, diags, d.Client, clusterResp, modelIn)
modelOut, extraInfo := getBasicClusterModel(ctx, diags, d.Client, clusterResp, modelIn, !useReplicationSpecPerShard)
if diags.HasError() {
return nil
}
Expand Down
3 changes: 1 addition & 2 deletions internal/service/advancedclustertpf/plural_data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,7 @@ func (d *pluralDS) readClusters(ctx context.Context, diags *diag.Diagnostics, pl
ProjectID: pluralModel.ProjectID,
Name: types.StringValue(clusterResp.GetName()),
}
// TODO: pass !UseReplicationSpecPerShard to overrideUsingLegacySchema
modelOut, extraInfo := getBasicClusterModel(ctx, diags, d.Client, clusterResp, modelIn)
modelOut, extraInfo := getBasicClusterModel(ctx, diags, d.Client, clusterResp, modelIn, !useReplicationSpecPerShard)
if diags.HasError() {
return nil
}
Expand Down
13 changes: 8 additions & 5 deletions internal/service/advancedclustertpf/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func (r *rs) Update(ctx context.Context, req resource.UpdateRequest, resp *resou
}
modelOut := &state
if clusterResp != nil {
modelOut, _ = getBasicClusterModel(ctx, diags, r.Client, clusterResp, &plan)
modelOut, _ = getBasicClusterModel(ctx, diags, r.Client, clusterResp, &plan, false)
if diags.HasError() {
return
}
Expand Down Expand Up @@ -239,7 +239,7 @@ func (r *rs) createCluster(ctx context.Context, plan *TFModel, diags *diag.Diagn
return nil
}
}
modelOut, _ := getBasicClusterModel(ctx, diags, r.Client, clusterResp, plan)
modelOut, _ := getBasicClusterModel(ctx, diags, r.Client, clusterResp, plan, false)
if diags.HasError() {
return nil
}
Expand All @@ -263,7 +263,7 @@ func (r *rs) readCluster(ctx context.Context, diags *diag.Diagnostics, modelIn *
diags.AddError("errorRead", fmt.Sprintf(errorRead, clusterName, err.Error()))
return nil
}
modelOut, _ := getBasicClusterModel(ctx, diags, r.Client, readResp, modelIn)
modelOut, _ := getBasicClusterModel(ctx, diags, r.Client, readResp, modelIn, false)
if diags.HasError() {
return nil
}
Expand Down Expand Up @@ -429,11 +429,14 @@ func (r *rs) applyTenantUpgrade(ctx context.Context, plan *TFModel, upgradeReque
return AwaitChanges(ctx, api, &plan.Timeouts, diags, projectID, clusterName, changeReasonUpdate)
}

func getBasicClusterModel(ctx context.Context, diags *diag.Diagnostics, client *config.MongoDBClient, clusterResp *admin.ClusterDescription20240805, modelIn *TFModel) (*TFModel, *ExtraAPIInfo) {
apiInfo := resolveAPIInfo(ctx, modelIn, diags, clusterResp, client)
func getBasicClusterModel(ctx context.Context, diags *diag.Diagnostics, client *config.MongoDBClient, clusterResp *admin.ClusterDescription20240805, modelIn *TFModel, forceLegacySchema bool) (*TFModel, *ExtraAPIInfo) {
apiInfo := resolveAPIInfo(ctx, diags, client, modelIn, clusterResp, forceLegacySchema)
if diags.HasError() {
return nil, nil
}
if forceLegacySchema && apiInfo.AsymmetricShardUnsupported { // can't create a model if legacy is forced but cluster does not support it
return nil, apiInfo
}
modelOut := NewTFModel(ctx, clusterResp, modelIn.Timeouts, diags, *apiInfo)
if diags.HasError() {
return nil, nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,23 +105,13 @@ func getAdvancedClusterContainerID(containers []admin.CloudProviderContainer, cl
return ""
}

func getReplicationSpecIDsFromOldAPI(ctx context.Context, projectID, clusterName string, api admin20240530.ClustersApi) (zoneNameSpecIDs map[string]string, asymmetricShardUnsupported bool, err error) {
var clusterOldAPI *admin20240530.AdvancedClusterDescription
clusterOldAPI, _, err = api.GetCluster(ctx, projectID, clusterName).Execute()
if err != nil {
if apiError, ok := admin20240530.AsError(err); ok {
if apiError.GetErrorCode() == "ASYMMETRIC_SHARD_UNSUPPORTED" {
return nil, true, nil // an error is expected in old API in case of an asymmetric shard. In that case, replication_specs.*.id attribute will not be populated.
}
}
return nil, false, fmt.Errorf("error reading advanced cluster with 2023-02-01 API (%s): %s", clusterName, err)
}
specs := clusterOldAPI.GetReplicationSpecs()
zoneNameSpecIDs = make(map[string]string, len(specs))
func replicationSpecIDsFromOldAPI(clusterRespOld *admin20240530.AdvancedClusterDescription) map[string]string {
specs := clusterRespOld.GetReplicationSpecs()
zoneNameSpecIDs := make(map[string]string, len(specs))
for _, spec := range specs {
zoneNameSpecIDs[spec.GetZoneName()] = spec.GetId()
}
return zoneNameSpecIDs, false, nil
return zoneNameSpecIDs
}

func convertHardwareSpecToOldSDK(hwspec *admin.HardwareSpec20240805) *admin20240530.HardwareSpec {
Expand Down
46 changes: 36 additions & 10 deletions internal/service/advancedclustertpf/resource_compatiblity.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package advancedclustertpf

import (
"context"
"fmt"
"reflect"

"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/conversion"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/config"
admin20240530 "go.mongodb.org/atlas-sdk/v20240530005/admin"
"go.mongodb.org/atlas-sdk/v20241113003/admin"
)

Expand Down Expand Up @@ -37,13 +39,22 @@ func findNumShardsUpdates(ctx context.Context, state, plan *TFModel, diags *diag
return planCounts
}

func resolveAPIInfo(ctx context.Context, plan *TFModel, diags *diag.Diagnostics, clusterLatest *admin.ClusterDescription20240805, client *config.MongoDBClient) *ExtraAPIInfo {
rootDiskSize := conversion.NilForUnknown(plan.DiskSizeGB, plan.DiskSizeGB.ValueFloat64Pointer())
projectID := plan.ProjectID.ValueString()
zoneNameSpecIDs, asymmetricShardUnsupported, err := getReplicationSpecIDsFromOldAPI(ctx, projectID, plan.Name.ValueString(), client.AtlasV220240530.ClustersApi)
func resolveAPIInfo(ctx context.Context, diags *diag.Diagnostics, client *config.MongoDBClient, plan *TFModel, clusterLatest *admin.ClusterDescription20240805, forceLegacySchema bool) *ExtraAPIInfo {
var (
api20240530 = client.AtlasV220240530.ClustersApi
rootDiskSize = conversion.NilForUnknown(plan.DiskSizeGB, plan.DiskSizeGB.ValueFloat64Pointer())
projectID = plan.ProjectID.ValueString()
clusterName = plan.Name.ValueString()
asymmetricShardUnsupported = false
)
clusterRespOld, _, err := api20240530.GetCluster(ctx, projectID, clusterName).Execute()
if err != nil {
diags.AddError("getReplicationSpecIDsFromOldAPI", err.Error())
return nil
if admin20240530.IsErrorCode(err, "ASYMMETRIC_SHARD_UNSUPPORTED") {
asymmetricShardUnsupported = true
} else {
diags.AddError("errorRead", fmt.Sprintf("error reading advanced cluster with 2024-05-30 API (%s): %s", clusterName, err))
return nil
}
}
if rootDiskSize == nil {
rootDiskSize = findRegionRootDiskSize(clusterLatest.ReplicationSpecs)
Expand All @@ -53,14 +64,20 @@ func resolveAPIInfo(ctx context.Context, plan *TFModel, diags *diag.Diagnostics,
diags.AddError("resolveContainerIDs failed", err.Error())
return nil
}
return &ExtraAPIInfo{
info := &ExtraAPIInfo{
ContainerIDs: containerIDs,
UsingLegacySchema: usingLegacySchema(ctx, plan.ReplicationSpecs, diags),
ZoneNameNumShards: numShardsMap(ctx, plan.ReplicationSpecs, diags),
RootDiskSize: rootDiskSize,
ZoneNameReplicationSpecIDs: zoneNameSpecIDs,
ZoneNameReplicationSpecIDs: replicationSpecIDsFromOldAPI(clusterRespOld),
AsymmetricShardUnsupported: asymmetricShardUnsupported,
}
if forceLegacySchema {
info.UsingLegacySchema = true
info.ZoneNameNumShards = numShardsMapFromOldAPI(clusterRespOld) // plan is empty in data source Read when forcing legacy, so we get num_shards from the old API
} else {
info.UsingLegacySchema = usingLegacySchema(ctx, plan.ReplicationSpecs, diags)
info.ZoneNameNumShards = numShardsMap(ctx, plan.ReplicationSpecs, diags)
}
return info
}

// instead of using `num_shards` explode the replication specs, and set disk_size_gb
Expand Down Expand Up @@ -182,6 +199,15 @@ func numShardsMap(ctx context.Context, input types.List, diags *diag.Diagnostics
return counts
}

func numShardsMapFromOldAPI(clusterRespOld *admin20240530.AdvancedClusterDescription) map[string]int64 {
ret := make(map[string]int64)
for i := range clusterRespOld.GetReplicationSpecs() {
spec := &clusterRespOld.GetReplicationSpecs()[i]
ret[spec.GetZoneName()] = int64(spec.GetNumShards())
}
return ret
}

func isNumShardsGreaterThanOne(counts []int64) bool {
for _, count := range counts {
if count > 1 {
Expand Down

0 comments on commit 61eda15

Please sign in to comment.