Skip to content
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

chore: Uses use_replication_spec_per_shard in data sources #2896

Merged
merged 13 commits into from
Dec 16, 2024
Merged

chore: Uses use_replication_spec_per_shard in data sources #2896

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
5d80ddc
enable TestAccClusterAdvancedClusterConfig_asymmetricShardedNewSchema
lantoli Dec 16, 2024
4650006
overrideUsingLegacySchema
lantoli Dec 16, 2024
a72969d
numShardsMapFromOldAPI
lantoli Dec 16, 2024
e9e73d0
refactor ExtraAPIInfo build
lantoli Dec 16, 2024
d39cf2f
address feedback
lantoli Dec 16, 2024
c0f4a9e
typo
lantoli Dec 16, 2024
bab0bcf
fill some computed null values
lantoli Dec 16, 2024
97a74c3
Revert "fill some computed null values"
lantoli Dec 16, 2024
75b5c25
Merge branch 'master' into CLOUDP-290307_data_source_legacy_view
lantoli Dec 16, 2024
1fb37c1
fill null value for BackingProviderName
lantoli Dec 16, 2024
b0143be
clause when legacy cluster model can't be created
lantoli Dec 16, 2024
c7b599b
Revert "fill null value for BackingProviderName"
lantoli Dec 16, 2024
e8f848e
skip id checks in replication_specs
lantoli Dec 16, 2024
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
2 changes: 0 additions & 2 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 @@ -771,8 +771,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
3 changes: 1 addition & 2 deletions internal/service/advancedclustertpf/data_source.go
View file
Open in desktop
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
View file
Open in desktop
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
10 changes: 5 additions & 5 deletions internal/service/advancedclustertpf/resource.go
View file
Open in desktop
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,8 +429,8 @@ 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, overrideUsingLegacySchema bool) (*TFModel, *ExtraAPIInfo) {
apiInfo := resolveAPIInfo(ctx, diags, client, modelIn, clusterResp, overrideUsingLegacySchema)
if diags.HasError() {
return nil, nil
}
Expand Down
18 changes: 4 additions & 14 deletions internal/service/advancedclustertpf/resource_compatibility_reuse.go
View file
Open in desktop
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
44 changes: 37 additions & 7 deletions internal/service/advancedclustertpf/resource_compatiblity.go
View file
Open in desktop
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,21 @@ 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 {
func resolveAPIInfo(ctx context.Context, diags *diag.Diagnostics, client *config.MongoDBClient, plan *TFModel, clusterLatest *admin.ClusterDescription20240805, overrideUsingLegacySchema bool) *ExtraAPIInfo {
Copy link
Collaborator

Choose a reason for hiding this comment

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

[nit] forceLegacySchema less verbose?

Copy link
Member Author

Choose a reason for hiding this comment

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

changed in: d39cf2f

rootDiskSize := conversion.NilForUnknown(plan.DiskSizeGB, plan.DiskSizeGB.ValueFloat64Pointer())
projectID := plan.ProjectID.ValueString()
zoneNameSpecIDs, asymmetricShardUnsupported, err := getReplicationSpecIDsFromOldAPI(ctx, projectID, plan.Name.ValueString(), client.AtlasV220240530.ClustersApi)
clusterName := plan.Name.ValueString()
asymmetricShardUnsupported := false

api20240530 := client.AtlasV220240530.ClustersApi
clusterRespOld, _, err := api20240530.GetCluster(ctx, projectID, clusterName).Execute()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice refactor!

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-08-05 API (%s): %s", clusterName, err))
return nil
}
}
if rootDiskSize == nil {
rootDiskSize = findRegionRootDiskSize(clusterLatest.ReplicationSpecs)
Expand All @@ -53,12 +63,23 @@ func resolveAPIInfo(ctx context.Context, plan *TFModel, diags *diag.Diagnostics,
diags.AddError("resolveContainerIDs failed", err.Error())
return nil
}
var (
legacySchema bool
zoneNameNumShards map[string]int64
)
if overrideUsingLegacySchema {
legacySchema = true
zoneNameNumShards = numShardsMapFromOldAPI(clusterRespOld)
Copy link
Member Author

Choose a reason for hiding this comment

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

plan is empty in Read when overriding legacy, so we get it from the old API

Copy link
Collaborator

Choose a reason for hiding this comment

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

Add this comment to the code?

Copy link
Member Author

Choose a reason for hiding this comment

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

added in: d39cf2f

} else {
legacySchema = usingLegacySchema(ctx, plan.ReplicationSpecs, diags)
zoneNameNumShards = numShardsMap(ctx, plan.ReplicationSpecs, diags)
}
return &ExtraAPIInfo{
ContainerIDs: containerIDs,
UsingLegacySchema: usingLegacySchema(ctx, plan.ReplicationSpecs, diags),
ZoneNameNumShards: numShardsMap(ctx, plan.ReplicationSpecs, diags),
UsingLegacySchema: legacySchema,
ZoneNameNumShards: zoneNameNumShards,
RootDiskSize: rootDiskSize,
ZoneNameReplicationSpecIDs: zoneNameSpecIDs,
ZoneNameReplicationSpecIDs: replicationSpecIDsFromOldAPI(clusterRespOld),
AsymmetricShardUnsupported: asymmetricShardUnsupported,
}
}
Expand Down Expand Up @@ -182,6 +203,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
Loading