From ecec0cd4098553e9a06d3c898ddcffb4c43cfc16 Mon Sep 17 00:00:00 2001 From: Leo Antoli <430982+lantoli@users.noreply.github.com> Date: Thu, 24 Jul 2025 11:09:56 +0200 Subject: [PATCH 01/19] CreateFlexClusterNew --- internal/service/flexcluster/resource.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/internal/service/flexcluster/resource.go b/internal/service/flexcluster/resource.go index d61d59a7b0..0541c893c8 100644 --- a/internal/service/flexcluster/resource.go +++ b/internal/service/flexcluster/resource.go @@ -69,7 +69,7 @@ func (r *rs) Create(ctx context.Context, req resource.CreateRequest, resp *resou clusterName := tfModel.Name.ValueString() connV2 := r.Client.AtlasV2 - flexClusterResp, err := CreateFlexCluster(ctx, projectID, clusterName, flexClusterReq, connV2.FlexClustersApi) + flexClusterResp, err := CreateFlexClusterNew(ctx, projectID, clusterName, flexClusterReq, connV2.FlexClustersApi) if err != nil { resp.Diagnostics.AddError(fmt.Sprintf(ErrorCreateFlex, err.Error()), fmt.Sprintf("Name: %s, Project ID: %s", clusterName, projectID)) return @@ -203,7 +203,8 @@ func splitFlexClusterImportID(id string) (projectID, clusterName *string, err er return } -func CreateFlexCluster(ctx context.Context, projectID, clusterName string, flexClusterReq *admin.FlexClusterDescriptionCreate20241113, client admin.FlexClustersApi) (*admin.FlexClusterDescription20241113, error) { +// TODO: using CreateFlexClusterNew to avoid changes in adv_cluster and running their acc tests while doing this PR, rename to CreateFlexCluster and update adv_cluster call params before merging. +func CreateFlexClusterNew(ctx context.Context, projectID, clusterName string, flexClusterReq *admin.FlexClusterDescriptionCreate20241113, client admin.FlexClustersApi) (*admin.FlexClusterDescription20241113, error) { _, _, err := client.CreateFlexCluster(ctx, projectID, flexClusterReq).Execute() if err != nil { return nil, err @@ -221,6 +222,11 @@ func CreateFlexCluster(ctx context.Context, projectID, clusterName string, flexC return flexClusterResp, nil } +// TODO: keeping CreateFlexCluster to avoid changes in adv_cluster and running their acc tests while doing this PR, remove before merging. +func CreateFlexCluster(ctx context.Context, projectID, clusterName string, flexClusterReq *admin.FlexClusterDescriptionCreate20241113, client admin.FlexClustersApi) (*admin.FlexClusterDescription20241113, error) { + return CreateFlexClusterNew(ctx, projectID, clusterName, flexClusterReq, client) +} + func GetFlexCluster(ctx context.Context, projectID, clusterName string, client admin.FlexClustersApi) (*admin.FlexClusterDescription20241113, error) { flexCluster, _, err := client.GetFlexCluster(ctx, projectID, clusterName).Execute() if err != nil { From 09ea70c0f9a79622c5f422e7517592947ddc237f Mon Sep 17 00:00:00 2001 From: Leo Antoli <430982+lantoli@users.noreply.github.com> Date: Thu, 24 Jul 2025 11:40:35 +0200 Subject: [PATCH 02/19] changelog --- .changelog/3525.txt | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changelog/3525.txt diff --git a/.changelog/3525.txt b/.changelog/3525.txt new file mode 100644 index 0000000000..a4b2539fac --- /dev/null +++ b/.changelog/3525.txt @@ -0,0 +1,7 @@ +```release-note:enhancement +resource/mongodbatlas_flex_cluster: Adds `timeouts` attribute for create, update and delete operations +``` + +```release-note:enhancement +resource/mongodbatlas_flex_cluster: Adds `delete_on_create_timeout` attribute to indicate whether to delete the resource if its creation times out +``` From 4e577218144c983a708e8354c86c315659bc03a9 Mon Sep 17 00:00:00 2001 From: Leo Antoli <430982+lantoli@users.noreply.github.com> Date: Thu, 24 Jul 2025 12:17:54 +0200 Subject: [PATCH 03/19] schema, model & auto-generated doc --- docs/resources/flex_cluster.md | 12 ++++++ internal/service/flexcluster/data_source.go | 10 ++++- .../service/flexcluster/plural_data_source.go | 3 +- .../service/flexcluster/resource_schema.go | 38 ++++++++++++------- 4 files changed, 48 insertions(+), 15 deletions(-) diff --git a/docs/resources/flex_cluster.md b/docs/resources/flex_cluster.md index a70a883a9c..21bde67f46 100644 --- a/docs/resources/flex_cluster.md +++ b/docs/resources/flex_cluster.md @@ -46,8 +46,10 @@ output "mongodbatlas_flex_clusters_names" { ### Optional +- `delete_on_create_timeout` (Boolean) Indicates whether to delete the resource if creation times out. Default is `true`. When Terraform apply fails, it returns immediately without waiting for cleanup to complete. If you suspect a transient error, wait before retrying to allow resource deletion to finish. - `tags` (Map of String) Map that contains key-value pairs between 1 to 255 characters in length for tagging and categorizing the instance. - `termination_protection_enabled` (Boolean) Flag that indicates whether termination protection is enabled on the cluster. If set to `true`, MongoDB Cloud won't delete the cluster. If set to `false`, MongoDB Cloud will delete the cluster. +- `timeouts` (Attributes) (see [below for nested schema](#nestedatt--timeouts)) ### Read-Only @@ -74,6 +76,16 @@ Read-Only: - `provider_name` (String) Human-readable label that identifies the cloud service provider. + +### Nested Schema for `timeouts` + +Optional: + +- `create` (String) A string that can be [parsed as a duration](https://pkg.go.dev/time#ParseDuration) consisting of numbers and unit suffixes, such as "30s" or "2h45m". Valid time units are "s" (seconds), "m" (minutes), "h" (hours). +- `delete` (String) A string that can be [parsed as a duration](https://pkg.go.dev/time#ParseDuration) consisting of numbers and unit suffixes, such as "30s" or "2h45m". Valid time units are "s" (seconds), "m" (minutes), "h" (hours). Setting a timeout for a Delete operation is only applicable if changes are saved into state before the destroy operation occurs. +- `update` (String) A string that can be [parsed as a duration](https://pkg.go.dev/time#ParseDuration) consisting of numbers and unit suffixes, such as "30s" or "2h45m". Valid time units are "s" (seconds), "m" (minutes), "h" (hours). + + ### Nested Schema for `backup_settings` diff --git a/internal/service/flexcluster/data_source.go b/internal/service/flexcluster/data_source.go index b8d75e5e9f..07bd7665a4 100644 --- a/internal/service/flexcluster/data_source.go +++ b/internal/service/flexcluster/data_source.go @@ -4,6 +4,7 @@ import ( "context" "github.com/hashicorp/terraform-plugin-framework/datasource" + dsschema "github.com/hashicorp/terraform-plugin-framework/datasource/schema" "github.com/mongodb/terraform-provider-mongodbatlas/internal/common/conversion" "github.com/mongodb/terraform-provider-mongodbatlas/internal/config" ) @@ -25,10 +26,17 @@ type ds struct { func (d *ds) Schema(ctx context.Context, req datasource.SchemaRequest, resp *datasource.SchemaResponse) { resp.Schema = conversion.DataSourceSchemaFromResource(ResourceSchema(ctx), &conversion.DataSourceSchemaRequest{ - RequiredFields: []string{"project_id", "name"}, + RequiredFields: []string{"project_id", "name"}, + OverridenFields: dataSourceOverridenFields(), }) } +func dataSourceOverridenFields() map[string]dsschema.Attribute { + return map[string]dsschema.Attribute{ + "delete_on_create_timeout": nil, + } +} + func (d *ds) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) { var tfModel TFModel resp.Diagnostics.Append(req.Config.Get(ctx, &tfModel)...) diff --git a/internal/service/flexcluster/plural_data_source.go b/internal/service/flexcluster/plural_data_source.go index 74197fe335..2a4492746e 100644 --- a/internal/service/flexcluster/plural_data_source.go +++ b/internal/service/flexcluster/plural_data_source.go @@ -27,7 +27,8 @@ type pluralDS struct { func (d *pluralDS) Schema(ctx context.Context, req datasource.SchemaRequest, resp *datasource.SchemaResponse) { resp.Schema = conversion.PluralDataSourceSchemaFromResource(ResourceSchema(ctx), &conversion.PluralDataSourceSchemaRequest{ - RequiredFields: []string{"project_id"}, + RequiredFields: []string{"project_id"}, + OverridenFields: dataSourceOverridenFields(), }) } diff --git a/internal/service/flexcluster/resource_schema.go b/internal/service/flexcluster/resource_schema.go index 08f5a9f471..4fabb29f90 100644 --- a/internal/service/flexcluster/resource_schema.go +++ b/internal/service/flexcluster/resource_schema.go @@ -3,6 +3,7 @@ package flexcluster import ( "context" + "github.com/hashicorp/terraform-plugin-framework-timeouts/resource/timeouts" "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/mongodb/terraform-provider-mongodbatlas/internal/common/customplanmodifier" @@ -145,24 +146,35 @@ func ResourceSchema(ctx context.Context) schema.Schema { }, MarkdownDescription: "Method by which the cluster maintains the MongoDB versions.", }, + "delete_on_create_timeout": schema.BoolAttribute{ + Optional: true, + MarkdownDescription: "Indicates whether to delete the resource if creation times out. Default is `true`. When Terraform apply fails, it returns immediately without waiting for cleanup to complete. If you suspect a transient error, wait before retrying to allow resource deletion to finish.", + }, + "timeouts": timeouts.Attributes(ctx, timeouts.Opts{ + Create: true, + Update: true, + Delete: true, + }), }, } } type TFModel struct { - ProviderSettings types.Object `tfsdk:"provider_settings"` - ConnectionStrings types.Object `tfsdk:"connection_strings"` - Tags types.Map `tfsdk:"tags"` - CreateDate types.String `tfsdk:"create_date"` - ProjectId types.String `tfsdk:"project_id"` - Id types.String `tfsdk:"id"` - MongoDbversion types.String `tfsdk:"mongo_db_version"` - Name types.String `tfsdk:"name"` - ClusterType types.String `tfsdk:"cluster_type"` - StateName types.String `tfsdk:"state_name"` - VersionReleaseSystem types.String `tfsdk:"version_release_system"` - BackupSettings types.Object `tfsdk:"backup_settings"` - TerminationProtectionEnabled types.Bool `tfsdk:"termination_protection_enabled"` + Tags types.Map `tfsdk:"tags"` + MongoDbversion types.String `tfsdk:"mongo_db_version"` + ClusterType types.String `tfsdk:"cluster_type"` + CreateDate types.String `tfsdk:"create_date"` + ProjectId types.String `tfsdk:"project_id"` + Id types.String `tfsdk:"id"` + ProviderSettings types.Object `tfsdk:"provider_settings"` + Name types.String `tfsdk:"name"` + ConnectionStrings types.Object `tfsdk:"connection_strings"` + StateName types.String `tfsdk:"state_name"` + VersionReleaseSystem types.String `tfsdk:"version_release_system"` + BackupSettings types.Object `tfsdk:"backup_settings"` + Timeouts timeouts.Value `tfsdk:"timeouts"` + DeleteOnCreateTimeout types.Bool `tfsdk:"delete_on_create_timeout"` + TerminationProtectionEnabled types.Bool `tfsdk:"termination_protection_enabled"` } type TFBackupSettings struct { From ab2452135a350e9b448356621e024a70cfc518de Mon Sep 17 00:00:00 2001 From: Leo Antoli <430982+lantoli@users.noreply.github.com> Date: Thu, 24 Jul 2025 13:39:14 +0200 Subject: [PATCH 04/19] non-updatable for multiple types like string and bool --- .../customplanmodifier/non_updatable.go | 38 +++++++++++++------ .../service/flexcluster/resource_schema.go | 6 ++- 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/internal/common/customplanmodifier/non_updatable.go b/internal/common/customplanmodifier/non_updatable.go index 7f47282bb1..8697bdc3d4 100644 --- a/internal/common/customplanmodifier/non_updatable.go +++ b/internal/common/customplanmodifier/non_updatable.go @@ -4,33 +4,47 @@ import ( "context" "fmt" + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" planmodifier "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" ) +// NonUpdatableStringAttributePlanModifier creates a plan modifier that prevents updates to string attributes. func NonUpdatableStringAttributePlanModifier() planmodifier.String { - return &nonUpdatableStringAttributePlanModifier{} + return &nonUpdatableAttributePlanModifier{} } -type nonUpdatableStringAttributePlanModifier struct { +// NonUpdatableBoolAttributePlanModifier creates a plan modifier that prevents updates to boolean attributes. +func NonUpdatableBoolAttributePlanModifier() planmodifier.Bool { + return &nonUpdatableAttributePlanModifier{} } -func (d *nonUpdatableStringAttributePlanModifier) Description(ctx context.Context) string { +// Plan modifier that implements non-updatable behavior for multiple attribute types +type nonUpdatableAttributePlanModifier struct{} + +func (d *nonUpdatableAttributePlanModifier) Description(ctx context.Context) string { return d.MarkdownDescription(ctx) } -func (d *nonUpdatableStringAttributePlanModifier) MarkdownDescription(ctx context.Context) string { +func (d *nonUpdatableAttributePlanModifier) MarkdownDescription(ctx context.Context) string { return "Ensures that update operations fails when updating an attribute." } -func (d *nonUpdatableStringAttributePlanModifier) PlanModifyString(ctx context.Context, req planmodifier.StringRequest, resp *planmodifier.StringResponse) { - planAttributeValue := req.PlanValue - stateAttributeValue := req.StateValue +func (d *nonUpdatableAttributePlanModifier) PlanModifyString(ctx context.Context, req planmodifier.StringRequest, resp *planmodifier.StringResponse) { + validateNonUpdatable(req.PlanValue, req.StateValue, req.Path, &resp.Diagnostics) +} + +func (d *nonUpdatableAttributePlanModifier) PlanModifyBool(ctx context.Context, req planmodifier.BoolRequest, resp *planmodifier.BoolResponse) { + validateNonUpdatable(req.PlanValue, req.StateValue, req.Path, &resp.Diagnostics) +} - if !stateAttributeValue.IsNull() && stateAttributeValue.ValueString() != planAttributeValue.ValueString() { - resp.Diagnostics.AddError( - fmt.Sprintf("%s cannot be updated", req.Path), - fmt.Sprintf("%s cannot be updated", req.Path), +func validateNonUpdatable(planValue, stateValue attr.Value, attrPath path.Path, diagnostics *diag.Diagnostics, +) { + if !stateValue.IsNull() && !stateValue.Equal(planValue) { + diagnostics.AddError( + fmt.Sprintf("%s cannot be updated", attrPath), + fmt.Sprintf("%s cannot be updated", attrPath), ) - return } } diff --git a/internal/service/flexcluster/resource_schema.go b/internal/service/flexcluster/resource_schema.go index 4fabb29f90..4e0596de90 100644 --- a/internal/service/flexcluster/resource_schema.go +++ b/internal/service/flexcluster/resource_schema.go @@ -147,7 +147,11 @@ func ResourceSchema(ctx context.Context) schema.Schema { MarkdownDescription: "Method by which the cluster maintains the MongoDB versions.", }, "delete_on_create_timeout": schema.BoolAttribute{ - Optional: true, + Optional: true, + PlanModifiers: []planmodifier.Bool{ + customplanmodifier.NonUpdatableBoolAttributePlanModifier(), + }, + MarkdownDescription: "Indicates whether to delete the resource if creation times out. Default is `true`. When Terraform apply fails, it returns immediately without waiting for cleanup to complete. If you suspect a transient error, wait before retrying to allow resource deletion to finish.", }, "timeouts": timeouts.Attributes(ctx, timeouts.Opts{ From 69f0c40a2e4d4dfc95d36613c1dee39ba1e84f75 Mon Sep 17 00:00:00 2001 From: Leo Antoli <430982+lantoli@users.noreply.github.com> Date: Thu, 24 Jul 2025 16:09:53 +0200 Subject: [PATCH 05/19] rename to create only --- .../common/customplanmodifier/create_only.go | 51 +++++++++++++++++++ .../customplanmodifier/non_updatable.go | 50 ------------------ .../service/flexcluster/resource_schema.go | 10 ++-- 3 files changed, 56 insertions(+), 55 deletions(-) create mode 100644 internal/common/customplanmodifier/create_only.go delete mode 100644 internal/common/customplanmodifier/non_updatable.go diff --git a/internal/common/customplanmodifier/create_only.go b/internal/common/customplanmodifier/create_only.go new file mode 100644 index 0000000000..4080f9d3da --- /dev/null +++ b/internal/common/customplanmodifier/create_only.go @@ -0,0 +1,51 @@ +package customplanmodifier + +import ( + "context" + "fmt" + + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" + planmodifier "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" +) + +// CreateOnlyStringPlanModifier creates a plan modifier that prevents updates to string attributes. +func CreateOnlyStringPlanModifier() planmodifier.String { + return &createOnlyAttributePlanModifier{} +} + +// CreateOnlyBoolPlanModifier creates a plan modifier that prevents updates to boolean attributes. +func CreateOnlyBoolPlanModifier() planmodifier.Bool { + return &createOnlyAttributePlanModifier{} +} + +// Plan modifier that implements create-only behavior for multiple attribute types +type createOnlyAttributePlanModifier struct{} + +func (d *createOnlyAttributePlanModifier) Description(ctx context.Context) string { + return d.MarkdownDescription(ctx) +} + +func (d *createOnlyAttributePlanModifier) MarkdownDescription(ctx context.Context) string { + return "Ensures that update operations fail when attempting to modify a create-only attribute." +} + +func (d *createOnlyAttributePlanModifier) PlanModifyString(ctx context.Context, req planmodifier.StringRequest, resp *planmodifier.StringResponse) { + validateCreateOnly(req.PlanValue, req.StateValue, req.Path, &resp.Diagnostics) +} + +func (d *createOnlyAttributePlanModifier) PlanModifyBool(ctx context.Context, req planmodifier.BoolRequest, resp *planmodifier.BoolResponse) { + validateCreateOnly(req.PlanValue, req.StateValue, req.Path, &resp.Diagnostics) +} + +// validateCreateOnly checks if an attribute value has changed and adds an error if it has +func validateCreateOnly(planValue, stateValue attr.Value, attrPath path.Path, diagnostics *diag.Diagnostics, +) { + if !stateValue.IsNull() && !stateValue.Equal(planValue) { + diagnostics.AddError( + fmt.Sprintf("%s cannot be updated", attrPath), + fmt.Sprintf("%s cannot be updated", attrPath), + ) + } +} diff --git a/internal/common/customplanmodifier/non_updatable.go b/internal/common/customplanmodifier/non_updatable.go deleted file mode 100644 index 8697bdc3d4..0000000000 --- a/internal/common/customplanmodifier/non_updatable.go +++ /dev/null @@ -1,50 +0,0 @@ -package customplanmodifier - -import ( - "context" - "fmt" - - "github.com/hashicorp/terraform-plugin-framework/attr" - "github.com/hashicorp/terraform-plugin-framework/diag" - "github.com/hashicorp/terraform-plugin-framework/path" - planmodifier "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" -) - -// NonUpdatableStringAttributePlanModifier creates a plan modifier that prevents updates to string attributes. -func NonUpdatableStringAttributePlanModifier() planmodifier.String { - return &nonUpdatableAttributePlanModifier{} -} - -// NonUpdatableBoolAttributePlanModifier creates a plan modifier that prevents updates to boolean attributes. -func NonUpdatableBoolAttributePlanModifier() planmodifier.Bool { - return &nonUpdatableAttributePlanModifier{} -} - -// Plan modifier that implements non-updatable behavior for multiple attribute types -type nonUpdatableAttributePlanModifier struct{} - -func (d *nonUpdatableAttributePlanModifier) Description(ctx context.Context) string { - return d.MarkdownDescription(ctx) -} - -func (d *nonUpdatableAttributePlanModifier) MarkdownDescription(ctx context.Context) string { - return "Ensures that update operations fails when updating an attribute." -} - -func (d *nonUpdatableAttributePlanModifier) PlanModifyString(ctx context.Context, req planmodifier.StringRequest, resp *planmodifier.StringResponse) { - validateNonUpdatable(req.PlanValue, req.StateValue, req.Path, &resp.Diagnostics) -} - -func (d *nonUpdatableAttributePlanModifier) PlanModifyBool(ctx context.Context, req planmodifier.BoolRequest, resp *planmodifier.BoolResponse) { - validateNonUpdatable(req.PlanValue, req.StateValue, req.Path, &resp.Diagnostics) -} - -func validateNonUpdatable(planValue, stateValue attr.Value, attrPath path.Path, diagnostics *diag.Diagnostics, -) { - if !stateValue.IsNull() && !stateValue.Equal(planValue) { - diagnostics.AddError( - fmt.Sprintf("%s cannot be updated", attrPath), - fmt.Sprintf("%s cannot be updated", attrPath), - ) - } -} diff --git a/internal/service/flexcluster/resource_schema.go b/internal/service/flexcluster/resource_schema.go index 4e0596de90..45f1400801 100644 --- a/internal/service/flexcluster/resource_schema.go +++ b/internal/service/flexcluster/resource_schema.go @@ -22,14 +22,14 @@ func ResourceSchema(ctx context.Context) schema.Schema { "project_id": schema.StringAttribute{ Required: true, PlanModifiers: []planmodifier.String{ - customplanmodifier.NonUpdatableStringAttributePlanModifier(), + customplanmodifier.CreateOnlyStringPlanModifier(), }, MarkdownDescription: "Unique 24-hexadecimal character string that identifies the project.", }, "name": schema.StringAttribute{ Required: true, PlanModifiers: []planmodifier.String{ - customplanmodifier.NonUpdatableStringAttributePlanModifier(), + customplanmodifier.CreateOnlyStringPlanModifier(), }, MarkdownDescription: "Human-readable label that identifies the instance.", }, @@ -38,7 +38,7 @@ func ResourceSchema(ctx context.Context) schema.Schema { "backing_provider_name": schema.StringAttribute{ Required: true, PlanModifiers: []planmodifier.String{ - customplanmodifier.NonUpdatableStringAttributePlanModifier(), + customplanmodifier.CreateOnlyStringPlanModifier(), }, MarkdownDescription: "Cloud service provider on which MongoDB Cloud provisioned the flex cluster.", }, @@ -59,7 +59,7 @@ func ResourceSchema(ctx context.Context) schema.Schema { "region_name": schema.StringAttribute{ Required: true, PlanModifiers: []planmodifier.String{ - customplanmodifier.NonUpdatableStringAttributePlanModifier(), + customplanmodifier.CreateOnlyStringPlanModifier(), }, MarkdownDescription: "Human-readable label that identifies the geographic location of your MongoDB flex cluster. The region you choose can affect network latency for clients accessing your databases. For a complete list of region names, see [AWS](https://docs.atlas.mongodb.com/reference/amazon-aws/#std-label-amazon-aws), [GCP](https://docs.atlas.mongodb.com/reference/google-gcp/), and [Azure](https://docs.atlas.mongodb.com/reference/microsoft-azure/).", }, @@ -149,7 +149,7 @@ func ResourceSchema(ctx context.Context) schema.Schema { "delete_on_create_timeout": schema.BoolAttribute{ Optional: true, PlanModifiers: []planmodifier.Bool{ - customplanmodifier.NonUpdatableBoolAttributePlanModifier(), + customplanmodifier.CreateOnlyBoolPlanModifier(), }, MarkdownDescription: "Indicates whether to delete the resource if creation times out. Default is `true`. When Terraform apply fails, it returns immediately without waiting for cleanup to complete. If you suspect a transient error, wait before retrying to allow resource deletion to finish.", From 3bed1dbc655e77d7cf20385b35b085e5ef965886 Mon Sep 17 00:00:00 2001 From: Leo Antoli <430982+lantoli@users.noreply.github.com> Date: Thu, 24 Jul 2025 16:37:07 +0200 Subject: [PATCH 06/19] fill Timeouts in model --- internal/service/flexcluster/resource.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/service/flexcluster/resource.go b/internal/service/flexcluster/resource.go index 0541c893c8..33c26d6fa5 100644 --- a/internal/service/flexcluster/resource.go +++ b/internal/service/flexcluster/resource.go @@ -80,6 +80,7 @@ func (r *rs) Create(ctx context.Context, req resource.CreateRequest, resp *resou resp.Diagnostics.Append(diags...) return } + newFlexClusterModel.Timeouts = tfModel.Timeouts if conversion.UseNilForEmpty(tfModel.Tags, newFlexClusterModel.Tags) { newFlexClusterModel.Tags = types.MapNull(types.StringType) @@ -113,6 +114,7 @@ func (r *rs) Read(ctx context.Context, req resource.ReadRequest, resp *resource. resp.Diagnostics.Append(diags...) return } + newFlexClusterModel.Timeouts = flexClusterState.Timeouts if conversion.UseNilForEmpty(flexClusterState.Tags, newFlexClusterModel.Tags) { newFlexClusterModel.Tags = types.MapNull(types.StringType) @@ -150,6 +152,7 @@ func (r *rs) Update(ctx context.Context, req resource.UpdateRequest, resp *resou resp.Diagnostics.Append(diags...) return } + newFlexClusterModel.Timeouts = plan.Timeouts if conversion.UseNilForEmpty(plan.Tags, newFlexClusterModel.Tags) { newFlexClusterModel.Tags = types.MapNull(types.StringType) From 9e51912284fc8e30309eae5dff17476f9d5f89bd Mon Sep 17 00:00:00 2001 From: Oriol Arbusi Abadal Date: Tue, 29 Jul 2025 11:27:16 +0200 Subject: [PATCH 07/19] remove delete_on_create_timeout by default in DataSourceSchemaFromResource --- internal/common/conversion/schema_generation.go | 4 ++-- internal/service/flexcluster/data_source.go | 10 +--------- internal/service/flexcluster/plural_data_source.go | 3 +-- 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/internal/common/conversion/schema_generation.go b/internal/common/conversion/schema_generation.go index c5a3998bc6..133748c56f 100644 --- a/internal/common/conversion/schema_generation.go +++ b/internal/common/conversion/schema_generation.go @@ -91,10 +91,10 @@ var convertNestedMappings = map[string]reflect.Type{ } func convertAttrs(rsAttrs map[string]schema.Attribute, requiredFields []string) map[string]dsschema.Attribute { - const ignoreField = "timeouts" + ignoreFields := []string{"timeouts", "delete_on_create_timeout"} dsAttrs := make(map[string]dsschema.Attribute, len(rsAttrs)) for name, attr := range rsAttrs { - if name == ignoreField { + if slices.Contains(ignoreFields, name) { continue } dsAttrs[name] = convertElement(name, attr, requiredFields).(dsschema.Attribute) diff --git a/internal/service/flexcluster/data_source.go b/internal/service/flexcluster/data_source.go index 07bd7665a4..b8d75e5e9f 100644 --- a/internal/service/flexcluster/data_source.go +++ b/internal/service/flexcluster/data_source.go @@ -4,7 +4,6 @@ import ( "context" "github.com/hashicorp/terraform-plugin-framework/datasource" - dsschema "github.com/hashicorp/terraform-plugin-framework/datasource/schema" "github.com/mongodb/terraform-provider-mongodbatlas/internal/common/conversion" "github.com/mongodb/terraform-provider-mongodbatlas/internal/config" ) @@ -26,17 +25,10 @@ type ds struct { func (d *ds) Schema(ctx context.Context, req datasource.SchemaRequest, resp *datasource.SchemaResponse) { resp.Schema = conversion.DataSourceSchemaFromResource(ResourceSchema(ctx), &conversion.DataSourceSchemaRequest{ - RequiredFields: []string{"project_id", "name"}, - OverridenFields: dataSourceOverridenFields(), + RequiredFields: []string{"project_id", "name"}, }) } -func dataSourceOverridenFields() map[string]dsschema.Attribute { - return map[string]dsschema.Attribute{ - "delete_on_create_timeout": nil, - } -} - func (d *ds) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) { var tfModel TFModel resp.Diagnostics.Append(req.Config.Get(ctx, &tfModel)...) diff --git a/internal/service/flexcluster/plural_data_source.go b/internal/service/flexcluster/plural_data_source.go index 2a4492746e..74197fe335 100644 --- a/internal/service/flexcluster/plural_data_source.go +++ b/internal/service/flexcluster/plural_data_source.go @@ -27,8 +27,7 @@ type pluralDS struct { func (d *pluralDS) Schema(ctx context.Context, req datasource.SchemaRequest, resp *datasource.SchemaResponse) { resp.Schema = conversion.PluralDataSourceSchemaFromResource(ResourceSchema(ctx), &conversion.PluralDataSourceSchemaRequest{ - RequiredFields: []string{"project_id"}, - OverridenFields: dataSourceOverridenFields(), + RequiredFields: []string{"project_id"}, }) } From 342d64a23d3ceb20292cbf69c296fa4c0e3b13ea Mon Sep 17 00:00:00 2001 From: Oriol Arbusi Abadal Date: Wed, 30 Jul 2025 10:54:43 +0200 Subject: [PATCH 08/19] implement timeout and delete_on_create --- .../resource_advanced_cluster.go | 6 +- .../resource_advanced_cluster_test.go | 2 +- .../advancedclustertpf/common_admin_sdk.go | 2 +- .../service/advancedclustertpf/resource.go | 4 +- internal/service/flexcluster/data_source.go | 5 +- internal/service/flexcluster/model.go | 4 +- internal/service/flexcluster/model_test.go | 4 +- internal/service/flexcluster/resource.go | 104 +++++++++++++++--- .../service/flexcluster/resource_schema.go | 19 +++- internal/service/flexcluster/resource_test.go | 39 +++++++ .../service/flexcluster/state_transition.go | 8 +- .../flexcluster/state_transition_test.go | 2 +- 12 files changed, 167 insertions(+), 32 deletions(-) diff --git a/internal/service/advancedcluster/resource_advanced_cluster.go b/internal/service/advancedcluster/resource_advanced_cluster.go index fad01490cd..d1eff52531 100644 --- a/internal/service/advancedcluster/resource_advanced_cluster.go +++ b/internal/service/advancedcluster/resource_advanced_cluster.go @@ -462,7 +462,7 @@ func resourceCreate(ctx context.Context, d *schema.ResourceData, meta any) diag. if isFlex { flexClusterReq := advancedclustertpf.NewFlexCreateReq(clusterName, d.Get("termination_protection_enabled").(bool), conversion.ExpandTagsFromSetSchema(d), replicationSpecs) - flexClusterResp, err := flexcluster.CreateFlexCluster(ctx, projectID, clusterName, flexClusterReq, connV2.FlexClustersApi) + flexClusterResp, err := flexcluster.CreateFlexCluster(ctx, projectID, clusterName, flexClusterReq, connV2.FlexClustersApi, &timeout) if err != nil { return diag.FromErr(fmt.Errorf(flexcluster.ErrorCreateFlex, err)) } @@ -1328,7 +1328,7 @@ func resourceDelete(ctx context.Context, d *schema.ResourceData, meta any) diag. replicationSpecs := expandAdvancedReplicationSpecs(d.Get("replication_specs").([]any), nil) if advancedclustertpf.IsFlex(replicationSpecs) { - err := flexcluster.DeleteFlexCluster(ctx, projectID, clusterName, connV2.FlexClustersApi) + err := flexcluster.DeleteFlexCluster(ctx, projectID, clusterName, connV2.FlexClustersApi, nil) if err != nil { return diag.FromErr(fmt.Errorf(flexcluster.ErrorDeleteFlex, clusterName, err)) } @@ -1540,7 +1540,7 @@ func resourceUpdateFlexCluster(ctx context.Context, flexUpdateRequest *admin.Fle projectID := ids["project_id"] clusterName := ids["cluster_name"] - _, err := flexcluster.UpdateFlexCluster(ctx, projectID, clusterName, flexUpdateRequest, connV2.FlexClustersApi) + _, err := flexcluster.UpdateFlexCluster(ctx, projectID, clusterName, flexUpdateRequest, connV2.FlexClustersApi, nil) if err != nil { return diag.FromErr(fmt.Errorf(flexcluster.ErrorUpdateFlex, err)) } diff --git a/internal/service/advancedcluster/resource_advanced_cluster_test.go b/internal/service/advancedcluster/resource_advanced_cluster_test.go index 1fdc0eb434..0b6b660805 100644 --- a/internal/service/advancedcluster/resource_advanced_cluster_test.go +++ b/internal/service/advancedcluster/resource_advanced_cluster_test.go @@ -1457,7 +1457,7 @@ func TestAccAdvancedCluster_createTimeoutWithDeleteOnCreateFlex(t *testing.T) { err := flexcluster.WaitStateTransitionDelete(t.Context(), &admin.GetFlexClusterApiParams{ GroupId: projectID, Name: clusterName, - }, acc.ConnV2().FlexClustersApi) + }, acc.ConnV2().FlexClustersApi, nil) require.NoError(t, err) time.Sleep(1 * time.Minute) // decrease the chance of `CONTAINER_WAITING_FOR_FAST_RECORD_CLEAN_UP`: "A transient error occurred. Please try again in a minute or use a different name" } diff --git a/internal/service/advancedclustertpf/common_admin_sdk.go b/internal/service/advancedclustertpf/common_admin_sdk.go index 5c3763ffe6..7f4e4c8feb 100644 --- a/internal/service/advancedclustertpf/common_admin_sdk.go +++ b/internal/service/advancedclustertpf/common_admin_sdk.go @@ -172,7 +172,7 @@ func DeleteCluster(ctx context.Context, diags *diag.Diagnostics, client *config. addErrorDiag(diags, operationDelete, defaultAPIErrorDetails(waitParams.ClusterName, err)) return } - err := flexcluster.DeleteFlexCluster(ctx, waitParams.ProjectID, waitParams.ClusterName, client.AtlasV2.FlexClustersApi) + err := flexcluster.DeleteFlexCluster(ctx, waitParams.ProjectID, waitParams.ClusterName, client.AtlasV2.FlexClustersApi, nil) if err != nil { addErrorDiag(diags, operationDeleteFlex, defaultAPIErrorDetails(waitParams.ClusterName, err)) return diff --git a/internal/service/advancedclustertpf/resource.go b/internal/service/advancedclustertpf/resource.go index fd97e6dec5..89a1173cc9 100644 --- a/internal/service/advancedclustertpf/resource.go +++ b/internal/service/advancedclustertpf/resource.go @@ -148,7 +148,7 @@ func (r *rs) Create(ctx context.Context, req resource.CreateRequest, resp *resou } if isFlex { flexClusterReq := NewFlexCreateReq(latestReq.GetName(), latestReq.GetTerminationProtectionEnabled(), latestReq.Tags, latestReq.ReplicationSpecs) - flexClusterResp, err := flexcluster.CreateFlexCluster(ctx, plan.ProjectID.ValueString(), latestReq.GetName(), flexClusterReq, r.Client.AtlasV2.FlexClustersApi) + flexClusterResp, err := flexcluster.CreateFlexCluster(ctx, plan.ProjectID.ValueString(), latestReq.GetName(), flexClusterReq, r.Client.AtlasV2.FlexClustersApi, &waitParams.Timeout) if err != nil { diags.AddError(fmt.Sprintf(flexcluster.ErrorCreateFlex, clusterDetailStr), err.Error()) return @@ -620,7 +620,7 @@ func handleFlexUpdate(ctx context.Context, diags *diag.Diagnostics, client *conf clusterName := plan.Name.ValueString() flexCluster, err := flexcluster.UpdateFlexCluster(ctx, plan.ProjectID.ValueString(), clusterName, GetFlexClusterUpdateRequest(configReq.Tags, configReq.TerminationProtectionEnabled), - client.AtlasV2.FlexClustersApi) + client.AtlasV2.FlexClustersApi, nil) if err != nil { diags.AddError(fmt.Sprintf(flexcluster.ErrorUpdateFlex, clusterName), err.Error()) return nil diff --git a/internal/service/flexcluster/data_source.go b/internal/service/flexcluster/data_source.go index b8d75e5e9f..eb2b66f9e2 100644 --- a/internal/service/flexcluster/data_source.go +++ b/internal/service/flexcluster/data_source.go @@ -30,7 +30,7 @@ func (d *ds) Schema(ctx context.Context, req datasource.SchemaRequest, resp *dat } func (d *ds) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) { - var tfModel TFModel + var tfModel TFModelDS resp.Diagnostics.Append(req.Config.Get(ctx, &tfModel)...) if resp.Diagnostics.HasError() { return @@ -48,5 +48,6 @@ func (d *ds) Read(ctx context.Context, req datasource.ReadRequest, resp *datasou resp.Diagnostics.Append(diags...) return } - resp.Diagnostics.Append(resp.State.Set(ctx, newFlexClusterModel)...) + newFlexClusterModelDS := conversion.CopyModel[TFModelDS](newFlexClusterModel) + resp.Diagnostics.Append(resp.State.Set(ctx, newFlexClusterModelDS)...) } diff --git a/internal/service/flexcluster/model.go b/internal/service/flexcluster/model.go index 5273a8b190..144d65afa2 100644 --- a/internal/service/flexcluster/model.go +++ b/internal/service/flexcluster/model.go @@ -44,13 +44,13 @@ func NewTFModel(ctx context.Context, apiResp *admin.FlexClusterDescription202411 func NewTFModelDSP(ctx context.Context, projectID string, input []admin.FlexClusterDescription20241113) (*TFModelDSP, diag.Diagnostics) { diags := &diag.Diagnostics{} - tfModels := make([]TFModel, len(input)) + tfModels := make([]TFModelDS, len(input)) for i := range input { item := &input[i] tfModel, diagsLocal := NewTFModel(ctx, item) diags.Append(diagsLocal...) if tfModel != nil { - tfModels[i] = *tfModel + tfModels[i] = *conversion.CopyModel[TFModelDS](tfModel) } } if diags.HasError() { diff --git a/internal/service/flexcluster/model_test.go b/internal/service/flexcluster/model_test.go index cb7bad35b0..31d3765581 100644 --- a/internal/service/flexcluster/model_test.go +++ b/internal/service/flexcluster/model_test.go @@ -174,7 +174,7 @@ func TestNewTFModelDSP(t *testing.T) { "Complete TF state": { expectedTFModelDSP: &flexcluster.TFModelDSP{ ProjectId: types.StringValue(projectID), - Results: []flexcluster.TFModel{ + Results: []flexcluster.TFModelDS{ { ProjectId: types.StringValue(projectID), Id: types.StringValue(id), @@ -277,7 +277,7 @@ func TestNewTFModelDSP(t *testing.T) { "No Flex Clusters": { expectedTFModelDSP: &flexcluster.TFModelDSP{ ProjectId: types.StringValue(projectID), - Results: []flexcluster.TFModel{}, + Results: []flexcluster.TFModelDS{}, }, input: []admin.FlexClusterDescription20241113{}, }, diff --git a/internal/service/flexcluster/resource.go b/internal/service/flexcluster/resource.go index 33c26d6fa5..2b819abfa5 100644 --- a/internal/service/flexcluster/resource.go +++ b/internal/service/flexcluster/resource.go @@ -6,13 +6,18 @@ import ( "fmt" "net/http" "regexp" + "time" "go.mongodb.org/atlas-sdk/v20250312005/admin" + "github.com/hashicorp/terraform-plugin-framework-timeouts/resource/timeouts" + "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/mongodb/terraform-provider-mongodbatlas/internal/common/cleanup" + "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/common/dsschema" "github.com/mongodb/terraform-provider-mongodbatlas/internal/common/retrystrategy" @@ -30,6 +35,12 @@ const ( ErrorUpgradeFlex = "error upgrading to a flex cluster: %s" ErrorDeleteFlex = "error deleting a flex cluster (%s): %s" ErrorNonUpdatableAttributes = "flex cluster update is not supported except for tags and termination_protection_enabled fields" + + // Operation constants for timeout resolution + operationCreate = "create" + operationRead = "read" + operationUpdate = "update" + operationDelete = "delete" ) var _ resource.ResourceWithConfigure = &rs{} @@ -68,8 +79,25 @@ func (r *rs) Create(ctx context.Context, req resource.CreateRequest, resp *resou projectID := tfModel.ProjectId.ValueString() clusterName := tfModel.Name.ValueString() + // Resolve timeout for create operation + createTimeout := resolveTimeout(ctx, &tfModel.Timeouts, operationCreate, &resp.Diagnostics) + if resp.Diagnostics.HasError() { + return + } + connV2 := r.Client.AtlasV2 - flexClusterResp, err := CreateFlexClusterNew(ctx, projectID, clusterName, flexClusterReq, connV2.FlexClustersApi) + flexClusterResp, err := CreateFlexCluster(ctx, projectID, clusterName, flexClusterReq, connV2.FlexClustersApi, &createTimeout) + + // Handle timeout with cleanup logic + deleteOnCreateTimeout := resolveDeleteOnCreateTimeout(tfModel.DeleteOnCreateTimeout) + err = cleanup.HandleCreateTimeout(deleteOnCreateTimeout, err, func(ctxCleanup context.Context) error { + cleanResp, cleanErr := r.Client.AtlasV2.FlexClustersApi.DeleteFlexCluster(ctxCleanup, projectID, clusterName).Execute() + if validate.StatusNotFound(cleanResp) { + return nil + } + return cleanErr + }) + if err != nil { resp.Diagnostics.AddError(fmt.Sprintf(ErrorCreateFlex, err.Error()), fmt.Sprintf("Name: %s, Project ID: %s", clusterName, projectID)) return @@ -81,6 +109,7 @@ func (r *rs) Create(ctx context.Context, req resource.CreateRequest, resp *resou return } newFlexClusterModel.Timeouts = tfModel.Timeouts + newFlexClusterModel.DeleteOnCreateTimeout = tfModel.DeleteOnCreateTimeout if conversion.UseNilForEmpty(tfModel.Tags, newFlexClusterModel.Tags) { newFlexClusterModel.Tags = types.MapNull(types.StringType) @@ -115,6 +144,7 @@ func (r *rs) Read(ctx context.Context, req resource.ReadRequest, resp *resource. return } newFlexClusterModel.Timeouts = flexClusterState.Timeouts + newFlexClusterModel.DeleteOnCreateTimeout = flexClusterState.DeleteOnCreateTimeout if conversion.UseNilForEmpty(flexClusterState.Tags, newFlexClusterModel.Tags) { newFlexClusterModel.Tags = types.MapNull(types.StringType) @@ -139,9 +169,15 @@ func (r *rs) Update(ctx context.Context, req resource.UpdateRequest, resp *resou projectID := plan.ProjectId.ValueString() clusterName := plan.Name.ValueString() + // Resolve timeout for update operation + updateTimeout := resolveTimeout(ctx, &plan.Timeouts, operationUpdate, &resp.Diagnostics) + if resp.Diagnostics.HasError() { + return + } + connV2 := r.Client.AtlasV2 - flexClusterResp, err := UpdateFlexCluster(ctx, projectID, clusterName, flexClusterReq, connV2.FlexClustersApi) + flexClusterResp, err := UpdateFlexCluster(ctx, projectID, clusterName, flexClusterReq, connV2.FlexClustersApi, &updateTimeout) if err != nil { resp.Diagnostics.AddError(fmt.Sprintf(ErrorUpdateFlex, clusterName), err.Error()) return @@ -153,6 +189,7 @@ func (r *rs) Update(ctx context.Context, req resource.UpdateRequest, resp *resou return } newFlexClusterModel.Timeouts = plan.Timeouts + newFlexClusterModel.DeleteOnCreateTimeout = plan.DeleteOnCreateTimeout if conversion.UseNilForEmpty(plan.Tags, newFlexClusterModel.Tags) { newFlexClusterModel.Tags = types.MapNull(types.StringType) @@ -172,7 +209,14 @@ func (r *rs) Delete(ctx context.Context, req resource.DeleteRequest, resp *resou projectID := flexClusterState.ProjectId.ValueString() clusterName := flexClusterState.Name.ValueString() - err := DeleteFlexCluster(ctx, projectID, clusterName, connV2.FlexClustersApi) + + // Resolve timeout for delete operation + deleteTimeout := resolveTimeout(ctx, &flexClusterState.Timeouts, operationDelete, &resp.Diagnostics) + if resp.Diagnostics.HasError() { + return + } + + err := DeleteFlexCluster(ctx, projectID, clusterName, connV2.FlexClustersApi, &deleteTimeout) if err != nil { resp.Diagnostics.AddError(fmt.Sprintf(ErrorDeleteFlex, projectID, clusterName), err.Error()) @@ -206,8 +250,43 @@ func splitFlexClusterImportID(id string) (projectID, clusterName *string, err er return } -// TODO: using CreateFlexClusterNew to avoid changes in adv_cluster and running their acc tests while doing this PR, rename to CreateFlexCluster and update adv_cluster call params before merging. -func CreateFlexClusterNew(ctx context.Context, projectID, clusterName string, flexClusterReq *admin.FlexClusterDescriptionCreate20241113, client admin.FlexClustersApi) (*admin.FlexClusterDescription20241113, error) { +// resolveTimeout extracts the appropriate timeout duration from the model for the given operation +func resolveTimeout(ctx context.Context, t *timeouts.Value, operationName string, diags *diag.Diagnostics) time.Duration { + var ( + timeoutDuration time.Duration + localDiags diag.Diagnostics + ) + switch operationName { + case operationCreate: + timeoutDuration, localDiags = t.Create(ctx, constant.DefaultTimeout) + diags.Append(localDiags...) + case operationRead: + timeoutDuration, localDiags = t.Read(ctx, constant.DefaultTimeout) + diags.Append(localDiags...) + case operationUpdate: + timeoutDuration, localDiags = t.Update(ctx, constant.DefaultTimeout) + diags.Append(localDiags...) + case operationDelete: + timeoutDuration, localDiags = t.Delete(ctx, constant.DefaultTimeout) + diags.Append(localDiags...) + default: + timeoutDuration = constant.DefaultTimeout + } + return timeoutDuration +} + +// resolveDeleteOnCreateTimeout returns true if delete_on_create_timeout should be enabled. +// Default behavior is true when not explicitly set to false. +func resolveDeleteOnCreateTimeout(deleteOnCreateTimeout types.Bool) bool { + // If null or unknown, default to true + if deleteOnCreateTimeout.IsNull() || deleteOnCreateTimeout.IsUnknown() { + return true + } + // Otherwise use the explicit value + return deleteOnCreateTimeout.ValueBool() +} + +func CreateFlexCluster(ctx context.Context, projectID, clusterName string, flexClusterReq *admin.FlexClusterDescriptionCreate20241113, client admin.FlexClustersApi, timeout *time.Duration) (*admin.FlexClusterDescription20241113, error) { _, _, err := client.CreateFlexCluster(ctx, projectID, flexClusterReq).Execute() if err != nil { return nil, err @@ -218,18 +297,13 @@ func CreateFlexClusterNew(ctx context.Context, projectID, clusterName string, fl Name: clusterName, } - flexClusterResp, err := WaitStateTransition(ctx, flexClusterParams, client, []string{retrystrategy.RetryStrategyCreatingState, retrystrategy.RetryStrategyUpdatingState, retrystrategy.RetryStrategyRepairingState}, []string{retrystrategy.RetryStrategyIdleState}, false, nil) + flexClusterResp, err := WaitStateTransition(ctx, flexClusterParams, client, []string{retrystrategy.RetryStrategyCreatingState, retrystrategy.RetryStrategyUpdatingState, retrystrategy.RetryStrategyRepairingState}, []string{retrystrategy.RetryStrategyIdleState}, false, timeout) if err != nil { return nil, err } return flexClusterResp, nil } -// TODO: keeping CreateFlexCluster to avoid changes in adv_cluster and running their acc tests while doing this PR, remove before merging. -func CreateFlexCluster(ctx context.Context, projectID, clusterName string, flexClusterReq *admin.FlexClusterDescriptionCreate20241113, client admin.FlexClustersApi) (*admin.FlexClusterDescription20241113, error) { - return CreateFlexClusterNew(ctx, projectID, clusterName, flexClusterReq, client) -} - func GetFlexCluster(ctx context.Context, projectID, clusterName string, client admin.FlexClustersApi) (*admin.FlexClusterDescription20241113, error) { flexCluster, _, err := client.GetFlexCluster(ctx, projectID, clusterName).Execute() if err != nil { @@ -238,7 +312,7 @@ func GetFlexCluster(ctx context.Context, projectID, clusterName string, client a return flexCluster, nil } -func UpdateFlexCluster(ctx context.Context, projectID, clusterName string, flexClusterReq *admin.FlexClusterDescriptionUpdate20241113, client admin.FlexClustersApi) (*admin.FlexClusterDescription20241113, error) { +func UpdateFlexCluster(ctx context.Context, projectID, clusterName string, flexClusterReq *admin.FlexClusterDescriptionUpdate20241113, client admin.FlexClustersApi, timeout *time.Duration) (*admin.FlexClusterDescription20241113, error) { _, _, err := client.UpdateFlexCluster(ctx, projectID, clusterName, flexClusterReq).Execute() if err != nil { return nil, err @@ -249,14 +323,14 @@ func UpdateFlexCluster(ctx context.Context, projectID, clusterName string, flexC Name: clusterName, } - flexClusterResp, err := WaitStateTransition(ctx, flexClusterParams, client, []string{retrystrategy.RetryStrategyUpdatingState, retrystrategy.RetryStrategyUpdatingState, retrystrategy.RetryStrategyRepairingState}, []string{retrystrategy.RetryStrategyIdleState}, false, nil) + flexClusterResp, err := WaitStateTransition(ctx, flexClusterParams, client, []string{retrystrategy.RetryStrategyUpdatingState, retrystrategy.RetryStrategyUpdatingState, retrystrategy.RetryStrategyRepairingState}, []string{retrystrategy.RetryStrategyIdleState}, false, timeout) if err != nil { return nil, err } return flexClusterResp, nil } -func DeleteFlexCluster(ctx context.Context, projectID, clusterName string, client admin.FlexClustersApi) error { +func DeleteFlexCluster(ctx context.Context, projectID, clusterName string, client admin.FlexClustersApi, timeout *time.Duration) error { if _, err := client.DeleteFlexCluster(ctx, projectID, clusterName).Execute(); err != nil { return err } @@ -266,7 +340,7 @@ func DeleteFlexCluster(ctx context.Context, projectID, clusterName string, clien Name: clusterName, } - return WaitStateTransitionDelete(ctx, flexClusterParams, client) + return WaitStateTransitionDelete(ctx, flexClusterParams, client, timeout) } func ListFlexClusters(ctx context.Context, projectID string, client admin.FlexClustersApi) (*[]admin.FlexClusterDescription20241113, error) { diff --git a/internal/service/flexcluster/resource_schema.go b/internal/service/flexcluster/resource_schema.go index 45f1400801..499ad73cd3 100644 --- a/internal/service/flexcluster/resource_schema.go +++ b/internal/service/flexcluster/resource_schema.go @@ -181,6 +181,23 @@ type TFModel struct { TerminationProtectionEnabled types.Bool `tfsdk:"termination_protection_enabled"` } +// TFModelDS differs from TFModel: removes timeouts and delete_on_create_timeout. +type TFModelDS struct { + Tags types.Map `tfsdk:"tags"` + MongoDbversion types.String `tfsdk:"mongo_db_version"` + ClusterType types.String `tfsdk:"cluster_type"` + CreateDate types.String `tfsdk:"create_date"` + ProjectId types.String `tfsdk:"project_id"` + Id types.String `tfsdk:"id"` + ProviderSettings types.Object `tfsdk:"provider_settings"` + Name types.String `tfsdk:"name"` + ConnectionStrings types.Object `tfsdk:"connection_strings"` + StateName types.String `tfsdk:"state_name"` + VersionReleaseSystem types.String `tfsdk:"version_release_system"` + BackupSettings types.Object `tfsdk:"backup_settings"` + TerminationProtectionEnabled types.Bool `tfsdk:"termination_protection_enabled"` +} + type TFBackupSettings struct { Enabled types.Bool `tfsdk:"enabled"` } @@ -215,5 +232,5 @@ var ProviderSettingsType = types.ObjectType{AttrTypes: map[string]attr.Type{ type TFModelDSP struct { ProjectId types.String `tfsdk:"project_id"` - Results []TFModel `tfsdk:"results"` + Results []TFModelDS `tfsdk:"results"` } diff --git a/internal/service/flexcluster/resource_test.go b/internal/service/flexcluster/resource_test.go index 9ef1ed55ac..6958a6cb9e 100644 --- a/internal/service/flexcluster/resource_test.go +++ b/internal/service/flexcluster/resource_test.go @@ -26,6 +26,26 @@ func TestAccFlexClusterRS_failedUpdate(t *testing.T) { resource.Test(t, *tc) } +func TestAccFlexClusterRS_timeouts(t *testing.T) { + var ( + projectID = acc.ProjectIDExecution(t) + clusterName = acc.RandomName() + provider = "AWS" + region = "US_EAST_1" + ) + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acc.PreCheckBasic(t) }, + ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, + CheckDestroy: acc.CheckDestroyFlexCluster, // resource is deleted when creation times out + Steps: []resource.TestStep{ + { + Config: configWithTimeouts(projectID, clusterName, provider, region, true), + ExpectError: regexp.MustCompile("will run cleanup because delete_on_create_timeout is true"), + }, + }, + }) +} + func basicTestCase(t *testing.T) *resource.TestCase { t.Helper() var ( @@ -122,6 +142,25 @@ func configBasic(projectID, clusterName, provider, region string, terminationPro `, projectID, clusterName, provider, region, terminationProtectionEnabled, tagsConfig, acc.FlexDataSource) } +func configWithTimeouts(projectID, clusterName, provider, region string, deleteOnCreateTimeout bool) string { + return fmt.Sprintf(` + resource "mongodbatlas_flex_cluster" "test" { + project_id = %[1]q + name = %[2]q + provider_settings = { + backing_provider_name = %[3]q + region_name = %[4]q + } + delete_on_create_timeout = %[5]t + timeouts = { + create = "1s" + update = "1s" + delete = "5m" + } + } + `, projectID, clusterName, provider, region, deleteOnCreateTimeout) +} + func checksFlexCluster(projectID, clusterName string, terminationProtectionEnabled, tagsCheck bool) resource.TestCheckFunc { checks := []resource.TestCheckFunc{acc.CheckExistsFlexCluster()} attrMap := map[string]string{ diff --git a/internal/service/flexcluster/state_transition.go b/internal/service/flexcluster/state_transition.go index cc1cd93afb..bdbf1115f4 100644 --- a/internal/service/flexcluster/state_transition.go +++ b/internal/service/flexcluster/state_transition.go @@ -40,12 +40,16 @@ func WaitStateTransition(ctx context.Context, requestParams *admin.GetFlexCluste return nil, errors.New("did not obtain valid result when waiting for flex cluster state transition") } -func WaitStateTransitionDelete(ctx context.Context, requestParams *admin.GetFlexClusterApiParams, client admin.FlexClustersApi) error { +func WaitStateTransitionDelete(ctx context.Context, requestParams *admin.GetFlexClusterApiParams, client admin.FlexClustersApi, timeout *time.Duration) error { + deleteTimeout := constant.DefaultTimeout + if timeout != nil { + deleteTimeout = *timeout + } stateConf := &retry.StateChangeConf{ Pending: []string{retrystrategy.RetryStrategyDeletingState}, Target: []string{retrystrategy.RetryStrategyDeletedState}, Refresh: refreshFunc(ctx, requestParams, client, false), - Timeout: 3 * time.Hour, + Timeout: deleteTimeout, MinTimeout: 3 * time.Second, Delay: 0, } diff --git a/internal/service/flexcluster/state_transition_test.go b/internal/service/flexcluster/state_transition_test.go index 7e9bc6c85f..e0ec13e16c 100644 --- a/internal/service/flexcluster/state_transition_test.go +++ b/internal/service/flexcluster/state_transition_test.go @@ -147,7 +147,7 @@ func TestFlexClusterStateTransitionForDelete(t *testing.T) { modelResp, httpResp, err := resp.get() m.EXPECT().GetFlexClusterExecute(mock.Anything).Return(modelResp, httpResp, err).Once() } - err := flexcluster.WaitStateTransitionDelete(t.Context(), requestParams, m) + err := flexcluster.WaitStateTransitionDelete(t.Context(), requestParams, m, nil) assert.Equal(t, tc.expectedError, err != nil) }) } From 3cfc917f11e898a99a15d0dd564863368f9e3567 Mon Sep 17 00:00:00 2001 From: Oriol Arbusi Abadal Date: Wed, 30 Jul 2025 14:13:20 +0200 Subject: [PATCH 09/19] resolve timeout refactor --- internal/common/cleanup/handle_timeout.go | 30 ++++++++++++++ .../service/advancedclustertpf/resource.go | 25 +----------- internal/service/flexcluster/resource.go | 40 ++----------------- 3 files changed, 34 insertions(+), 61 deletions(-) diff --git a/internal/common/cleanup/handle_timeout.go b/internal/common/cleanup/handle_timeout.go index 5640303fb4..9004e5085a 100644 --- a/internal/common/cleanup/handle_timeout.go +++ b/internal/common/cleanup/handle_timeout.go @@ -6,8 +6,10 @@ import ( "strings" "time" + "github.com/hashicorp/terraform-plugin-framework-timeouts/resource/timeouts" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" + "github.com/mongodb/terraform-provider-mongodbatlas/internal/common/constant" ) const ( @@ -69,3 +71,31 @@ func ReplaceContextDeadlineExceededDiags(diags *diag.Diagnostics, duration time. } } } + +const ( + OperationCreate = "create" + OperationUpdate = "update" + OperationDelete = "delete" +) + +// ResolveTimeout extracts the appropriate timeout duration from the model for the given operation +func ResolveTimeout(ctx context.Context, t *timeouts.Value, operationName string, diags *diag.Diagnostics) time.Duration { + var ( + timeoutDuration time.Duration + localDiags diag.Diagnostics + ) + switch operationName { + case OperationCreate: + timeoutDuration, localDiags = t.Create(ctx, constant.DefaultTimeout) + diags.Append(localDiags...) + case OperationUpdate: + timeoutDuration, localDiags = t.Update(ctx, constant.DefaultTimeout) + diags.Append(localDiags...) + case OperationDelete: + timeoutDuration, localDiags = t.Delete(ctx, constant.DefaultTimeout) + diags.Append(localDiags...) + default: + timeoutDuration = constant.DefaultTimeout + } + return timeoutDuration +} diff --git a/internal/service/advancedclustertpf/resource.go b/internal/service/advancedclustertpf/resource.go index 89a1173cc9..13d775d58b 100644 --- a/internal/service/advancedclustertpf/resource.go +++ b/internal/service/advancedclustertpf/resource.go @@ -3,11 +3,9 @@ package advancedclustertpf import ( "context" "fmt" - "time" "go.mongodb.org/atlas-sdk/v20250312005/admin" - "github.com/hashicorp/terraform-plugin-framework-timeouts/resource/timeouts" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/types" @@ -494,7 +492,7 @@ func updateModelAdvancedConfig(ctx context.Context, diags *diag.Diagnostics, cli func resolveClusterWaitParams(ctx context.Context, model *TFModel, diags *diag.Diagnostics, operation string) *ClusterWaitParams { projectID := model.ProjectID.ValueString() clusterName := model.Name.ValueString() - operationTimeout := resolveTimeout(ctx, &model.Timeouts, operation, diags) + operationTimeout := cleanup.ResolveTimeout(ctx, &model.Timeouts, operation, diags) if diags.HasError() { return nil } @@ -506,27 +504,6 @@ func resolveClusterWaitParams(ctx context.Context, model *TFModel, diags *diag.D } } -func resolveTimeout(ctx context.Context, t *timeouts.Value, operationName string, diags *diag.Diagnostics) time.Duration { - var ( - timeoutDuration time.Duration - localDiags diag.Diagnostics - ) - switch operationName { - case operationCreate: - timeoutDuration, localDiags = t.Create(ctx, constant.DefaultTimeout) - diags.Append(localDiags...) - case operationUpdate: - timeoutDuration, localDiags = t.Update(ctx, constant.DefaultTimeout) - diags.Append(localDiags...) - case operationDelete: - timeoutDuration, localDiags = t.Delete(ctx, constant.DefaultTimeout) - diags.Append(localDiags...) - default: - timeoutDuration = constant.DefaultTimeout - } - return timeoutDuration -} - type clusterDiff struct { clusterPatchOnlyReq *admin.ClusterDescription20240805 upgradeTenantReq *admin.LegacyAtlasTenantClusterUpgradeRequest diff --git a/internal/service/flexcluster/resource.go b/internal/service/flexcluster/resource.go index 2b819abfa5..057e81cbfb 100644 --- a/internal/service/flexcluster/resource.go +++ b/internal/service/flexcluster/resource.go @@ -10,14 +10,11 @@ import ( "go.mongodb.org/atlas-sdk/v20250312005/admin" - "github.com/hashicorp/terraform-plugin-framework-timeouts/resource/timeouts" - "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/mongodb/terraform-provider-mongodbatlas/internal/common/cleanup" - "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/common/dsschema" "github.com/mongodb/terraform-provider-mongodbatlas/internal/common/retrystrategy" @@ -35,12 +32,6 @@ const ( ErrorUpgradeFlex = "error upgrading to a flex cluster: %s" ErrorDeleteFlex = "error deleting a flex cluster (%s): %s" ErrorNonUpdatableAttributes = "flex cluster update is not supported except for tags and termination_protection_enabled fields" - - // Operation constants for timeout resolution - operationCreate = "create" - operationRead = "read" - operationUpdate = "update" - operationDelete = "delete" ) var _ resource.ResourceWithConfigure = &rs{} @@ -80,7 +71,7 @@ func (r *rs) Create(ctx context.Context, req resource.CreateRequest, resp *resou clusterName := tfModel.Name.ValueString() // Resolve timeout for create operation - createTimeout := resolveTimeout(ctx, &tfModel.Timeouts, operationCreate, &resp.Diagnostics) + createTimeout := cleanup.ResolveTimeout(ctx, &tfModel.Timeouts, cleanup.OperationCreate, &resp.Diagnostics) if resp.Diagnostics.HasError() { return } @@ -170,7 +161,7 @@ func (r *rs) Update(ctx context.Context, req resource.UpdateRequest, resp *resou clusterName := plan.Name.ValueString() // Resolve timeout for update operation - updateTimeout := resolveTimeout(ctx, &plan.Timeouts, operationUpdate, &resp.Diagnostics) + updateTimeout := cleanup.ResolveTimeout(ctx, &plan.Timeouts, cleanup.OperationUpdate, &resp.Diagnostics) if resp.Diagnostics.HasError() { return } @@ -211,7 +202,7 @@ func (r *rs) Delete(ctx context.Context, req resource.DeleteRequest, resp *resou clusterName := flexClusterState.Name.ValueString() // Resolve timeout for delete operation - deleteTimeout := resolveTimeout(ctx, &flexClusterState.Timeouts, operationDelete, &resp.Diagnostics) + deleteTimeout := cleanup.ResolveTimeout(ctx, &flexClusterState.Timeouts, cleanup.OperationDelete, &resp.Diagnostics) if resp.Diagnostics.HasError() { return } @@ -250,31 +241,6 @@ func splitFlexClusterImportID(id string) (projectID, clusterName *string, err er return } -// resolveTimeout extracts the appropriate timeout duration from the model for the given operation -func resolveTimeout(ctx context.Context, t *timeouts.Value, operationName string, diags *diag.Diagnostics) time.Duration { - var ( - timeoutDuration time.Duration - localDiags diag.Diagnostics - ) - switch operationName { - case operationCreate: - timeoutDuration, localDiags = t.Create(ctx, constant.DefaultTimeout) - diags.Append(localDiags...) - case operationRead: - timeoutDuration, localDiags = t.Read(ctx, constant.DefaultTimeout) - diags.Append(localDiags...) - case operationUpdate: - timeoutDuration, localDiags = t.Update(ctx, constant.DefaultTimeout) - diags.Append(localDiags...) - case operationDelete: - timeoutDuration, localDiags = t.Delete(ctx, constant.DefaultTimeout) - diags.Append(localDiags...) - default: - timeoutDuration = constant.DefaultTimeout - } - return timeoutDuration -} - // resolveDeleteOnCreateTimeout returns true if delete_on_create_timeout should be enabled. // Default behavior is true when not explicitly set to false. func resolveDeleteOnCreateTimeout(deleteOnCreateTimeout types.Bool) bool { From bb7c2570438c8711eb7dc8f97868849d2d113709 Mon Sep 17 00:00:00 2001 From: Oriol Arbusi Abadal Date: Wed, 30 Jul 2025 14:14:41 +0200 Subject: [PATCH 10/19] remove unnecessary update and delete timeouts in test --- internal/service/flexcluster/resource_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/service/flexcluster/resource_test.go b/internal/service/flexcluster/resource_test.go index 6958a6cb9e..0e86a00016 100644 --- a/internal/service/flexcluster/resource_test.go +++ b/internal/service/flexcluster/resource_test.go @@ -154,8 +154,6 @@ func configWithTimeouts(projectID, clusterName, provider, region string, deleteO delete_on_create_timeout = %[5]t timeouts = { create = "1s" - update = "1s" - delete = "5m" } } `, projectID, clusterName, provider, region, deleteOnCreateTimeout) From 56b6dc8da2d15cb659e646082e0d4fc8c3ea3a1d Mon Sep 17 00:00:00 2001 From: Oriol Arbusi Abadal Date: Wed, 30 Jul 2025 14:25:39 +0200 Subject: [PATCH 11/19] pass timeout for update and delete --- .../service/advancedcluster/resource_advanced_cluster.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/service/advancedcluster/resource_advanced_cluster.go b/internal/service/advancedcluster/resource_advanced_cluster.go index d1eff52531..edf78d0510 100644 --- a/internal/service/advancedcluster/resource_advanced_cluster.go +++ b/internal/service/advancedcluster/resource_advanced_cluster.go @@ -1326,9 +1326,10 @@ func resourceDelete(ctx context.Context, d *schema.ResourceData, meta any) diag. } replicationSpecs := expandAdvancedReplicationSpecs(d.Get("replication_specs").([]any), nil) + timeout := d.Timeout(schema.TimeoutDelete) if advancedclustertpf.IsFlex(replicationSpecs) { - err := flexcluster.DeleteFlexCluster(ctx, projectID, clusterName, connV2.FlexClustersApi, nil) + err := flexcluster.DeleteFlexCluster(ctx, projectID, clusterName, connV2.FlexClustersApi, &timeout) if err != nil { return diag.FromErr(fmt.Errorf(flexcluster.ErrorDeleteFlex, clusterName, err)) } @@ -1539,8 +1540,9 @@ func resourceUpdateFlexCluster(ctx context.Context, flexUpdateRequest *admin.Fle ids := conversion.DecodeStateID(d.Id()) projectID := ids["project_id"] clusterName := ids["cluster_name"] + timeout := d.Timeout(schema.TimeoutUpdate) - _, err := flexcluster.UpdateFlexCluster(ctx, projectID, clusterName, flexUpdateRequest, connV2.FlexClustersApi, nil) + _, err := flexcluster.UpdateFlexCluster(ctx, projectID, clusterName, flexUpdateRequest, connV2.FlexClustersApi, &timeout) if err != nil { return diag.FromErr(fmt.Errorf(flexcluster.ErrorUpdateFlex, err)) } From b0d39a851e34732d8c6b42e7485bb3224994dbe5 Mon Sep 17 00:00:00 2001 From: Oriol Arbusi Abadal Date: Wed, 30 Jul 2025 14:27:11 +0200 Subject: [PATCH 12/19] explicitly check that cluster does not exist due to delete_on_create_timeout --- internal/service/flexcluster/resource_test.go | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/internal/service/flexcluster/resource_test.go b/internal/service/flexcluster/resource_test.go index 0e86a00016..6a8c35779b 100644 --- a/internal/service/flexcluster/resource_test.go +++ b/internal/service/flexcluster/resource_test.go @@ -1,11 +1,13 @@ package flexcluster_test import ( + "context" "fmt" "regexp" "testing" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/mongodb/terraform-provider-mongodbatlas/internal/testutil/acc" ) @@ -36,11 +38,15 @@ func TestAccFlexClusterRS_timeouts(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acc.PreCheckBasic(t) }, ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, - CheckDestroy: acc.CheckDestroyFlexCluster, // resource is deleted when creation times out + CheckDestroy: acc.CheckDestroyFlexCluster, // verify cleanup was effective Steps: []resource.TestStep{ { Config: configWithTimeouts(projectID, clusterName, provider, region, true), ExpectError: regexp.MustCompile("will run cleanup because delete_on_create_timeout is true"), + Check: resource.ComposeAggregateTestCheckFunc( + // Explicitly verify the cluster was deleted due to timeout logic, not just test cleanup + checkClusterNotExists(projectID, clusterName), + ), }, }, }) @@ -187,3 +193,19 @@ func checksFlexCluster(projectID, clusterName string, terminationProtectionEnabl checks = acc.AddAttrChecks(dataSourcePluralName, checks, pluralMap) return acc.CheckRSAndDS(resourceName, &dataSourceName, &dataSourcePluralName, attrSet, attrMap, checks...) } + +// checkClusterNotExists verifies that the flex cluster does not exist, +// confirming that delete_on_create_timeout logic successfully cleaned up the resource +func checkClusterNotExists(projectID, clusterName string) resource.TestCheckFunc { + return func(s *terraform.State) error { + _, resp, err := acc.ConnV2().FlexClustersApi.GetFlexCluster(context.Background(), projectID, clusterName).Execute() + if err != nil { + // 404 is expected - cluster should not exist + if resp != nil && resp.StatusCode == 404 { + return nil + } + return fmt.Errorf("unexpected error checking cluster existence: %v", err) + } + return fmt.Errorf("cluster %s still exists in project %s, delete_on_create_timeout logic may not have worked", clusterName, projectID) + } +} From 55f67aa9146420be990cf68d92cac92fdb8132d9 Mon Sep 17 00:00:00 2001 From: Oriol Arbusi Abadal Date: Wed, 30 Jul 2025 15:25:48 +0200 Subject: [PATCH 13/19] timeout as value instead of pointer --- .../resource_advanced_cluster.go | 2 +- .../resource_advanced_cluster_test.go | 3 ++- .../service/advancedclustertpf/model_flex.go | 2 +- internal/service/flexcluster/resource.go | 6 +++--- .../service/flexcluster/state_transition.go | 17 ++++------------- .../flexcluster/state_transition_test.go | 5 +++-- 6 files changed, 14 insertions(+), 21 deletions(-) diff --git a/internal/service/advancedcluster/resource_advanced_cluster.go b/internal/service/advancedcluster/resource_advanced_cluster.go index edf78d0510..022d00be85 100644 --- a/internal/service/advancedcluster/resource_advanced_cluster.go +++ b/internal/service/advancedcluster/resource_advanced_cluster.go @@ -1434,7 +1434,7 @@ func waitStateTransitionFlexUpgrade(ctx context.Context, client admin.FlexCluste GroupId: projectID, Name: name, } - flexClusterResp, err := flexcluster.WaitStateTransition(ctx, flexClusterParams, client, []string{retrystrategy.RetryStrategyUpdatingState}, []string{retrystrategy.RetryStrategyIdleState}, true, &timeout) + flexClusterResp, err := flexcluster.WaitStateTransition(ctx, flexClusterParams, client, []string{retrystrategy.RetryStrategyUpdatingState}, []string{retrystrategy.RetryStrategyIdleState}, true, timeout) if err != nil { return nil, err } diff --git a/internal/service/advancedcluster/resource_advanced_cluster_test.go b/internal/service/advancedcluster/resource_advanced_cluster_test.go index 0b6b660805..0594c7a8bf 100644 --- a/internal/service/advancedcluster/resource_advanced_cluster_test.go +++ b/internal/service/advancedcluster/resource_advanced_cluster_test.go @@ -20,6 +20,7 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + "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/config" "github.com/mongodb/terraform-provider-mongodbatlas/internal/service/advancedcluster" @@ -1457,7 +1458,7 @@ func TestAccAdvancedCluster_createTimeoutWithDeleteOnCreateFlex(t *testing.T) { err := flexcluster.WaitStateTransitionDelete(t.Context(), &admin.GetFlexClusterApiParams{ GroupId: projectID, Name: clusterName, - }, acc.ConnV2().FlexClustersApi, nil) + }, acc.ConnV2().FlexClustersApi, constant.DefaultTimeout) require.NoError(t, err) time.Sleep(1 * time.Minute) // decrease the chance of `CONTAINER_WAITING_FOR_FAST_RECORD_CLEAN_UP`: "A transient error occurred. Please try again in a minute or use a different name" } diff --git a/internal/service/advancedclustertpf/model_flex.go b/internal/service/advancedclustertpf/model_flex.go index 592231c13f..c8b93aac60 100644 --- a/internal/service/advancedclustertpf/model_flex.go +++ b/internal/service/advancedclustertpf/model_flex.go @@ -152,7 +152,7 @@ func FlexUpgrade(ctx context.Context, diags *diag.Diagnostics, client *config.Mo Name: waitParams.ClusterName, } - flexClusterResp, err := flexcluster.WaitStateTransition(ctx, flexClusterParams, client.AtlasV2.FlexClustersApi, []string{retrystrategy.RetryStrategyUpdatingState}, []string{retrystrategy.RetryStrategyIdleState}, true, &waitParams.Timeout) + flexClusterResp, err := flexcluster.WaitStateTransition(ctx, flexClusterParams, client.AtlasV2.FlexClustersApi, []string{retrystrategy.RetryStrategyUpdatingState}, []string{retrystrategy.RetryStrategyIdleState}, true, waitParams.Timeout) if err != nil { diags.AddError(fmt.Sprintf(flexcluster.ErrorUpgradeFlex, req.Name), err.Error()) return nil diff --git a/internal/service/flexcluster/resource.go b/internal/service/flexcluster/resource.go index 057e81cbfb..df0c8426ce 100644 --- a/internal/service/flexcluster/resource.go +++ b/internal/service/flexcluster/resource.go @@ -263,7 +263,7 @@ func CreateFlexCluster(ctx context.Context, projectID, clusterName string, flexC Name: clusterName, } - flexClusterResp, err := WaitStateTransition(ctx, flexClusterParams, client, []string{retrystrategy.RetryStrategyCreatingState, retrystrategy.RetryStrategyUpdatingState, retrystrategy.RetryStrategyRepairingState}, []string{retrystrategy.RetryStrategyIdleState}, false, timeout) + flexClusterResp, err := WaitStateTransition(ctx, flexClusterParams, client, []string{retrystrategy.RetryStrategyCreatingState, retrystrategy.RetryStrategyUpdatingState, retrystrategy.RetryStrategyRepairingState}, []string{retrystrategy.RetryStrategyIdleState}, false, *timeout) if err != nil { return nil, err } @@ -289,7 +289,7 @@ func UpdateFlexCluster(ctx context.Context, projectID, clusterName string, flexC Name: clusterName, } - flexClusterResp, err := WaitStateTransition(ctx, flexClusterParams, client, []string{retrystrategy.RetryStrategyUpdatingState, retrystrategy.RetryStrategyUpdatingState, retrystrategy.RetryStrategyRepairingState}, []string{retrystrategy.RetryStrategyIdleState}, false, timeout) + flexClusterResp, err := WaitStateTransition(ctx, flexClusterParams, client, []string{retrystrategy.RetryStrategyUpdatingState, retrystrategy.RetryStrategyUpdatingState, retrystrategy.RetryStrategyRepairingState}, []string{retrystrategy.RetryStrategyIdleState}, false, *timeout) if err != nil { return nil, err } @@ -306,7 +306,7 @@ func DeleteFlexCluster(ctx context.Context, projectID, clusterName string, clien Name: clusterName, } - return WaitStateTransitionDelete(ctx, flexClusterParams, client, timeout) + return WaitStateTransitionDelete(ctx, flexClusterParams, client, *timeout) } func ListFlexClusters(ctx context.Context, projectID string, client admin.FlexClustersApi) (*[]admin.FlexClusterDescription20241113, error) { diff --git a/internal/service/flexcluster/state_transition.go b/internal/service/flexcluster/state_transition.go index bdbf1115f4..b60811970b 100644 --- a/internal/service/flexcluster/state_transition.go +++ b/internal/service/flexcluster/state_transition.go @@ -9,21 +9,16 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" - "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/common/retrystrategy" "github.com/mongodb/terraform-provider-mongodbatlas/internal/common/validate" ) -func WaitStateTransition(ctx context.Context, requestParams *admin.GetFlexClusterApiParams, client admin.FlexClustersApi, pendingStates, desiredStates []string, isUpgradeFromM0 bool, timeout *time.Duration) (*admin.FlexClusterDescription20241113, error) { - if timeout == nil { - timeout = conversion.Pointer(constant.DefaultTimeout) - } +func WaitStateTransition(ctx context.Context, requestParams *admin.GetFlexClusterApiParams, client admin.FlexClustersApi, pendingStates, desiredStates []string, isUpgradeFromM0 bool, timeout time.Duration) (*admin.FlexClusterDescription20241113, error) { stateConf := &retry.StateChangeConf{ Pending: pendingStates, Target: desiredStates, Refresh: refreshFunc(ctx, requestParams, client, isUpgradeFromM0), - Timeout: *timeout, + Timeout: timeout, MinTimeout: 3 * time.Second, Delay: 0, } @@ -40,16 +35,12 @@ func WaitStateTransition(ctx context.Context, requestParams *admin.GetFlexCluste return nil, errors.New("did not obtain valid result when waiting for flex cluster state transition") } -func WaitStateTransitionDelete(ctx context.Context, requestParams *admin.GetFlexClusterApiParams, client admin.FlexClustersApi, timeout *time.Duration) error { - deleteTimeout := constant.DefaultTimeout - if timeout != nil { - deleteTimeout = *timeout - } +func WaitStateTransitionDelete(ctx context.Context, requestParams *admin.GetFlexClusterApiParams, client admin.FlexClustersApi, timeout time.Duration) error { stateConf := &retry.StateChangeConf{ Pending: []string{retrystrategy.RetryStrategyDeletingState}, Target: []string{retrystrategy.RetryStrategyDeletedState}, Refresh: refreshFunc(ctx, requestParams, client, false), - Timeout: deleteTimeout, + Timeout: timeout, MinTimeout: 3 * time.Second, Delay: 0, } diff --git a/internal/service/flexcluster/state_transition_test.go b/internal/service/flexcluster/state_transition_test.go index e0ec13e16c..914a20cf1b 100644 --- a/internal/service/flexcluster/state_transition_test.go +++ b/internal/service/flexcluster/state_transition_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "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/flexcluster" ) @@ -102,7 +103,7 @@ func TestFlexClusterStateTransition(t *testing.T) { modelResp, httpResp, err := resp.get() m.EXPECT().GetFlexClusterExecute(mock.Anything).Return(modelResp, httpResp, err).Once() } - resp, err := flexcluster.WaitStateTransition(t.Context(), requestParams, m, tc.pendingStates, tc.desiredStates, tc.isUpgradeFromM0, nil) + resp, err := flexcluster.WaitStateTransition(t.Context(), requestParams, m, tc.pendingStates, tc.desiredStates, tc.isUpgradeFromM0, constant.DefaultTimeout) assert.Equal(t, tc.expectedError, err != nil) if resp != nil { assert.Equal(t, *tc.expectedState, *resp.StateName) @@ -147,7 +148,7 @@ func TestFlexClusterStateTransitionForDelete(t *testing.T) { modelResp, httpResp, err := resp.get() m.EXPECT().GetFlexClusterExecute(mock.Anything).Return(modelResp, httpResp, err).Once() } - err := flexcluster.WaitStateTransitionDelete(t.Context(), requestParams, m, nil) + err := flexcluster.WaitStateTransitionDelete(t.Context(), requestParams, m, constant.DefaultTimeout) assert.Equal(t, tc.expectedError, err != nil) }) } From 8d2d1f01bae27c11bd0e1275a90c92ba6c4bd80a Mon Sep 17 00:00:00 2001 From: Oriol Arbusi Abadal Date: Wed, 30 Jul 2025 16:15:17 +0200 Subject: [PATCH 14/19] remove redunadnt check --- internal/service/flexcluster/resource_test.go | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/internal/service/flexcluster/resource_test.go b/internal/service/flexcluster/resource_test.go index 6a8c35779b..1640d8eda5 100644 --- a/internal/service/flexcluster/resource_test.go +++ b/internal/service/flexcluster/resource_test.go @@ -1,13 +1,11 @@ package flexcluster_test import ( - "context" "fmt" "regexp" "testing" "github.com/hashicorp/terraform-plugin-testing/helper/resource" - "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/mongodb/terraform-provider-mongodbatlas/internal/testutil/acc" ) @@ -43,10 +41,6 @@ func TestAccFlexClusterRS_timeouts(t *testing.T) { { Config: configWithTimeouts(projectID, clusterName, provider, region, true), ExpectError: regexp.MustCompile("will run cleanup because delete_on_create_timeout is true"), - Check: resource.ComposeAggregateTestCheckFunc( - // Explicitly verify the cluster was deleted due to timeout logic, not just test cleanup - checkClusterNotExists(projectID, clusterName), - ), }, }, }) @@ -193,19 +187,3 @@ func checksFlexCluster(projectID, clusterName string, terminationProtectionEnabl checks = acc.AddAttrChecks(dataSourcePluralName, checks, pluralMap) return acc.CheckRSAndDS(resourceName, &dataSourceName, &dataSourcePluralName, attrSet, attrMap, checks...) } - -// checkClusterNotExists verifies that the flex cluster does not exist, -// confirming that delete_on_create_timeout logic successfully cleaned up the resource -func checkClusterNotExists(projectID, clusterName string) resource.TestCheckFunc { - return func(s *terraform.State) error { - _, resp, err := acc.ConnV2().FlexClustersApi.GetFlexCluster(context.Background(), projectID, clusterName).Execute() - if err != nil { - // 404 is expected - cluster should not exist - if resp != nil && resp.StatusCode == 404 { - return nil - } - return fmt.Errorf("unexpected error checking cluster existence: %v", err) - } - return fmt.Errorf("cluster %s still exists in project %s, delete_on_create_timeout logic may not have worked", clusterName, projectID) - } -} From 37b192a9123768784055a8d9120701dfab6723d7 Mon Sep 17 00:00:00 2001 From: Oriol Arbusi Abadal Date: Wed, 30 Jul 2025 16:22:57 +0200 Subject: [PATCH 15/19] final fixes for timeout --- .../advancedcluster/resource_advanced_cluster.go | 4 ++-- .../service/advancedclustertpf/common_admin_sdk.go | 2 +- internal/service/advancedclustertpf/resource.go | 6 +++--- internal/service/flexcluster/resource.go | 12 ++++++------ 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/internal/service/advancedcluster/resource_advanced_cluster.go b/internal/service/advancedcluster/resource_advanced_cluster.go index 022d00be85..4b2c928ed3 100644 --- a/internal/service/advancedcluster/resource_advanced_cluster.go +++ b/internal/service/advancedcluster/resource_advanced_cluster.go @@ -1329,7 +1329,7 @@ func resourceDelete(ctx context.Context, d *schema.ResourceData, meta any) diag. timeout := d.Timeout(schema.TimeoutDelete) if advancedclustertpf.IsFlex(replicationSpecs) { - err := flexcluster.DeleteFlexCluster(ctx, projectID, clusterName, connV2.FlexClustersApi, &timeout) + err := flexcluster.DeleteFlexCluster(ctx, projectID, clusterName, connV2.FlexClustersApi, timeout) if err != nil { return diag.FromErr(fmt.Errorf(flexcluster.ErrorDeleteFlex, clusterName, err)) } @@ -1542,7 +1542,7 @@ func resourceUpdateFlexCluster(ctx context.Context, flexUpdateRequest *admin.Fle clusterName := ids["cluster_name"] timeout := d.Timeout(schema.TimeoutUpdate) - _, err := flexcluster.UpdateFlexCluster(ctx, projectID, clusterName, flexUpdateRequest, connV2.FlexClustersApi, &timeout) + _, err := flexcluster.UpdateFlexCluster(ctx, projectID, clusterName, flexUpdateRequest, connV2.FlexClustersApi, timeout) if err != nil { return diag.FromErr(fmt.Errorf(flexcluster.ErrorUpdateFlex, err)) } diff --git a/internal/service/advancedclustertpf/common_admin_sdk.go b/internal/service/advancedclustertpf/common_admin_sdk.go index 7f4e4c8feb..a139d9f52f 100644 --- a/internal/service/advancedclustertpf/common_admin_sdk.go +++ b/internal/service/advancedclustertpf/common_admin_sdk.go @@ -172,7 +172,7 @@ func DeleteCluster(ctx context.Context, diags *diag.Diagnostics, client *config. addErrorDiag(diags, operationDelete, defaultAPIErrorDetails(waitParams.ClusterName, err)) return } - err := flexcluster.DeleteFlexCluster(ctx, waitParams.ProjectID, waitParams.ClusterName, client.AtlasV2.FlexClustersApi, nil) + err := flexcluster.DeleteFlexCluster(ctx, waitParams.ProjectID, waitParams.ClusterName, client.AtlasV2.FlexClustersApi, waitParams.Timeout) if err != nil { addErrorDiag(diags, operationDeleteFlex, defaultAPIErrorDetails(waitParams.ClusterName, err)) return diff --git a/internal/service/advancedclustertpf/resource.go b/internal/service/advancedclustertpf/resource.go index 13d775d58b..440d0a5cca 100644 --- a/internal/service/advancedclustertpf/resource.go +++ b/internal/service/advancedclustertpf/resource.go @@ -269,7 +269,7 @@ func (r *rs) Update(ctx context.Context, req resource.UpdateRequest, resp *resou } return case diff.isUpdateOfFlex: - if flexOut := handleFlexUpdate(ctx, diags, r.Client, &plan); flexOut != nil { + if flexOut := handleFlexUpdate(ctx, diags, r.Client, waitParams, &plan); flexOut != nil { diags.Append(resp.State.Set(ctx, flexOut)...) } return @@ -589,7 +589,7 @@ func handleFlexUpgrade(ctx context.Context, diags *diag.Diagnostics, client *con return NewTFModelFlexResource(ctx, diags, flexCluster, GetPriorityOfFlexReplicationSpecs(configReq.ReplicationSpecs), plan) } -func handleFlexUpdate(ctx context.Context, diags *diag.Diagnostics, client *config.MongoDBClient, plan *TFModel) *TFModel { +func handleFlexUpdate(ctx context.Context, diags *diag.Diagnostics, client *config.MongoDBClient, waitParams *ClusterWaitParams, plan *TFModel) *TFModel { configReq := normalizeFromTFModel(ctx, plan, diags, false) if diags.HasError() { return nil @@ -597,7 +597,7 @@ func handleFlexUpdate(ctx context.Context, diags *diag.Diagnostics, client *conf clusterName := plan.Name.ValueString() flexCluster, err := flexcluster.UpdateFlexCluster(ctx, plan.ProjectID.ValueString(), clusterName, GetFlexClusterUpdateRequest(configReq.Tags, configReq.TerminationProtectionEnabled), - client.AtlasV2.FlexClustersApi, nil) + client.AtlasV2.FlexClustersApi, waitParams.Timeout) if err != nil { diags.AddError(fmt.Sprintf(flexcluster.ErrorUpdateFlex, clusterName), err.Error()) return nil diff --git a/internal/service/flexcluster/resource.go b/internal/service/flexcluster/resource.go index df0c8426ce..71967cc498 100644 --- a/internal/service/flexcluster/resource.go +++ b/internal/service/flexcluster/resource.go @@ -168,7 +168,7 @@ func (r *rs) Update(ctx context.Context, req resource.UpdateRequest, resp *resou connV2 := r.Client.AtlasV2 - flexClusterResp, err := UpdateFlexCluster(ctx, projectID, clusterName, flexClusterReq, connV2.FlexClustersApi, &updateTimeout) + flexClusterResp, err := UpdateFlexCluster(ctx, projectID, clusterName, flexClusterReq, connV2.FlexClustersApi, updateTimeout) if err != nil { resp.Diagnostics.AddError(fmt.Sprintf(ErrorUpdateFlex, clusterName), err.Error()) return @@ -207,7 +207,7 @@ func (r *rs) Delete(ctx context.Context, req resource.DeleteRequest, resp *resou return } - err := DeleteFlexCluster(ctx, projectID, clusterName, connV2.FlexClustersApi, &deleteTimeout) + err := DeleteFlexCluster(ctx, projectID, clusterName, connV2.FlexClustersApi, deleteTimeout) if err != nil { resp.Diagnostics.AddError(fmt.Sprintf(ErrorDeleteFlex, projectID, clusterName), err.Error()) @@ -278,7 +278,7 @@ func GetFlexCluster(ctx context.Context, projectID, clusterName string, client a return flexCluster, nil } -func UpdateFlexCluster(ctx context.Context, projectID, clusterName string, flexClusterReq *admin.FlexClusterDescriptionUpdate20241113, client admin.FlexClustersApi, timeout *time.Duration) (*admin.FlexClusterDescription20241113, error) { +func UpdateFlexCluster(ctx context.Context, projectID, clusterName string, flexClusterReq *admin.FlexClusterDescriptionUpdate20241113, client admin.FlexClustersApi, timeout time.Duration) (*admin.FlexClusterDescription20241113, error) { _, _, err := client.UpdateFlexCluster(ctx, projectID, clusterName, flexClusterReq).Execute() if err != nil { return nil, err @@ -289,14 +289,14 @@ func UpdateFlexCluster(ctx context.Context, projectID, clusterName string, flexC Name: clusterName, } - flexClusterResp, err := WaitStateTransition(ctx, flexClusterParams, client, []string{retrystrategy.RetryStrategyUpdatingState, retrystrategy.RetryStrategyUpdatingState, retrystrategy.RetryStrategyRepairingState}, []string{retrystrategy.RetryStrategyIdleState}, false, *timeout) + flexClusterResp, err := WaitStateTransition(ctx, flexClusterParams, client, []string{retrystrategy.RetryStrategyUpdatingState, retrystrategy.RetryStrategyUpdatingState, retrystrategy.RetryStrategyRepairingState}, []string{retrystrategy.RetryStrategyIdleState}, false, timeout) if err != nil { return nil, err } return flexClusterResp, nil } -func DeleteFlexCluster(ctx context.Context, projectID, clusterName string, client admin.FlexClustersApi, timeout *time.Duration) error { +func DeleteFlexCluster(ctx context.Context, projectID, clusterName string, client admin.FlexClustersApi, timeout time.Duration) error { if _, err := client.DeleteFlexCluster(ctx, projectID, clusterName).Execute(); err != nil { return err } @@ -306,7 +306,7 @@ func DeleteFlexCluster(ctx context.Context, projectID, clusterName string, clien Name: clusterName, } - return WaitStateTransitionDelete(ctx, flexClusterParams, client, *timeout) + return WaitStateTransitionDelete(ctx, flexClusterParams, client, timeout) } func ListFlexClusters(ctx context.Context, projectID string, client admin.FlexClustersApi) (*[]admin.FlexClusterDescription20241113, error) { From 13b41ce9afd820a16c1203e709a202d941367787 Mon Sep 17 00:00:00 2001 From: Oriol Arbusi Abadal Date: Thu, 31 Jul 2025 11:38:12 +0200 Subject: [PATCH 16/19] improve testing --- .../resource_advanced_cluster_test.go | 112 +++++++++++------- internal/service/flexcluster/resource_test.go | 103 ++++++++++------ .../resource_organization_test.go | 13 +- internal/testutil/acc/common_config.go | 53 +++++++++ 4 files changed, 187 insertions(+), 94 deletions(-) create mode 100644 internal/testutil/acc/common_config.go diff --git a/internal/service/advancedcluster/resource_advanced_cluster_test.go b/internal/service/advancedcluster/resource_advanced_cluster_test.go index 0594c7a8bf..0553219e5f 100644 --- a/internal/service/advancedcluster/resource_advanced_cluster_test.go +++ b/internal/service/advancedcluster/resource_advanced_cluster_test.go @@ -18,14 +18,11 @@ import ( "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" - "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/config" "github.com/mongodb/terraform-provider-mongodbatlas/internal/service/advancedcluster" "github.com/mongodb/terraform-provider-mongodbatlas/internal/service/advancedclustertpf" - "github.com/mongodb/terraform-provider-mongodbatlas/internal/service/flexcluster" "github.com/mongodb/terraform-provider-mongodbatlas/internal/testutil/acc" "github.com/mongodb/terraform-provider-mongodbatlas/internal/testutil/unit" ) @@ -134,7 +131,7 @@ func testAccAdvancedClusterFlexUpgrade(t *testing.T, instanceSize string, includ Check: checkTenant(true, projectID, clusterName), }, { - Config: configFlexCluster(t, projectID, clusterName, "AWS", "US_EAST_1", defaultZoneName, false), + Config: configFlexCluster(t, projectID, clusterName, "AWS", "US_EAST_1", defaultZoneName, "", false, nil), Check: checkFlexClusterConfig(projectID, clusterName, "AWS", "US_EAST_1", false), }, } @@ -1433,39 +1430,6 @@ func TestAccAdvancedCluster_createTimeoutWithDeleteOnCreateReplicaset(t *testing resource.ParallelTest(t, *createCleanupTest(t, configCall, waitOnClusterDeleteDone, true)) } -func TestAccAdvancedCluster_createTimeoutWithDeleteOnCreateFlex(t *testing.T) { - var ( - projectID, clusterName = acc.ProjectIDExecutionWithCluster(t, 1) - configCall = func(t *testing.T, timeoutSection string) string { - t.Helper() - return acc.ConvertAdvancedClusterToPreviewProviderV2(t, true, fmt.Sprintf(` - resource "mongodbatlas_advanced_cluster" "test" { - project_id = %[1]q - name = %[2]q - cluster_type = "REPLICASET" - replication_specs { - region_configs { - provider_name = "FLEX" - backing_provider_name = "AWS" - region_name = "US_EAST_1" - priority = 7 - } - } - %[3]s - }`, projectID, clusterName, timeoutSection)) - } - waitOnClusterDeleteDone = func() { - err := flexcluster.WaitStateTransitionDelete(t.Context(), &admin.GetFlexClusterApiParams{ - GroupId: projectID, - Name: clusterName, - }, acc.ConnV2().FlexClustersApi, constant.DefaultTimeout) - require.NoError(t, err) - time.Sleep(1 * time.Minute) // decrease the chance of `CONTAINER_WAITING_FOR_FAST_RECORD_CLEAN_UP`: "A transient error occurred. Please try again in a minute or use a different name" - } - ) - resource.ParallelTest(t, *createCleanupTest(t, configCall, waitOnClusterDeleteDone, false)) -} - func createCleanupTest(t *testing.T, configCall func(t *testing.T, timeoutSection string) string, waitOnClusterDeleteDone func(), isUpdateSupported bool) *resource.TestCase { t.Helper() var ( @@ -3234,7 +3198,7 @@ func configFCVPinning(t *testing.T, orgID, projectName, clusterName string, pinn `, orgID, projectName, clusterName, mongoDBMajorVersion, pinnedFCVAttr)) + dataSourcesTFNewSchema } -func configFlexCluster(t *testing.T, projectID, clusterName, providerName, region, zoneName string, withTags bool) string { +func configFlexCluster(t *testing.T, projectID, clusterName, providerName, region, zoneName, timeoutConfig string, withTags bool, deleteOnCreateTimeout *bool) string { t.Helper() zoneNameLine := "" if zoneName != "" { @@ -3248,6 +3212,12 @@ func configFlexCluster(t *testing.T, projectID, clusterName, providerName, regio value = "testValue" }` } + deleteOnCreateTimeoutConfig := "" + if deleteOnCreateTimeout != nil { + deleteOnCreateTimeoutConfig = fmt.Sprintf(` + delete_on_create_timeout = %[1]t + `, *deleteOnCreateTimeout) + } return acc.ConvertAdvancedClusterToPreviewProviderV2(t, true, fmt.Sprintf(` resource "mongodbatlas_advanced_cluster" "test" { project_id = %[1]q @@ -3263,16 +3233,19 @@ func configFlexCluster(t *testing.T, projectID, clusterName, providerName, regio %[5]s } %[6]s + %[7]s termination_protection_enabled = false + %[8]s } - `, projectID, clusterName, providerName, region, zoneNameLine, tags)+dataSourcesTFOldSchema+ + `, projectID, clusterName, providerName, region, zoneNameLine, tags, timeoutConfig, deleteOnCreateTimeoutConfig)+dataSourcesTFOldSchema+ strings.ReplaceAll(acc.FlexDataSource, "mongodbatlas_flex_cluster.", "mongodbatlas_advanced_cluster.")) } func TestAccClusterFlexCluster_basic(t *testing.T) { var ( - projectID = acc.ProjectIDExecution(t) - clusterName = acc.RandomClusterName() + projectID = acc.ProjectIDExecution(t) + clusterName = acc.RandomClusterName() + emptyTimeoutConfig = "" ) resource.Test(t, resource.TestCase{ PreCheck: func() { acc.PreCheckBasic(t) }, @@ -3280,22 +3253,73 @@ func TestAccClusterFlexCluster_basic(t *testing.T) { CheckDestroy: acc.CheckDestroyFlexCluster, Steps: []resource.TestStep{ { - Config: configFlexCluster(t, projectID, clusterName, "AWS", "US_EAST_1", "", false), + Config: configFlexCluster(t, projectID, clusterName, "AWS", "US_EAST_1", "", emptyTimeoutConfig, false, nil), Check: checkFlexClusterConfig(projectID, clusterName, "AWS", "US_EAST_1", false), }, { - Config: configFlexCluster(t, projectID, clusterName, "AWS", "US_EAST_1", "", true), + Config: configFlexCluster(t, projectID, clusterName, "AWS", "US_EAST_1", "", emptyTimeoutConfig, true, nil), Check: checkFlexClusterConfig(projectID, clusterName, "AWS", "US_EAST_1", true), }, acc.TestStepImportCluster(resourceName), { - Config: configFlexCluster(t, projectID, clusterName, "AWS", "US_EAST_2", "", true), + Config: configFlexCluster(t, projectID, clusterName, "AWS", "US_EAST_2", "", emptyTimeoutConfig, true, nil), ExpectError: regexp.MustCompile("flex cluster update is not supported except for tags and termination_protection_enabled fields"), }, }, }) } +func TestAccAdvancedCluster_createTimeoutWithDeleteOnCreateFlex(t *testing.T) { + var ( + projectID = acc.ProjectIDExecution(t) + clusterName = acc.RandomName() + createTimeout = "1s" + deleteOnCreateTimeout = true + ) + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acc.PreCheckBasic(t) }, + ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, + CheckDestroy: acc.CheckDestroyFlexCluster, + Steps: []resource.TestStep{ + { + Config: configFlexCluster(t, projectID, clusterName, "AWS", "US_EAST_1", "", acc.TimeoutConfig(&createTimeout, nil, nil, false), false, &deleteOnCreateTimeout), + ExpectError: regexp.MustCompile("context deadline exceeded"), // with the current implementation, this is the error that is returned + }, + }, + }) +} + +func TestAccAdvancedCluster_updateDeleteTimeoutFlex(t *testing.T) { + var ( + projectID = acc.ProjectIDExecution(t) + clusterName = acc.RandomName() + updateTimeout = "1s" + deleteTimeout = "1s" + ) + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acc.PreCheckBasic(t) }, + ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, + CheckDestroy: acc.CheckDestroyFlexCluster, + Steps: []resource.TestStep{ + { + Config: configFlexCluster(t, projectID, clusterName, "AWS", "US_EAST_1", "", acc.TimeoutConfig(nil, &updateTimeout, &deleteTimeout, false), false, nil), + }, + { + Config: configFlexCluster(t, projectID, clusterName, "AWS", "US_EAST_1", "", acc.TimeoutConfig(nil, &updateTimeout, &deleteTimeout, false), true, nil), + ExpectError: regexp.MustCompile("timeout while waiting for state to become 'IDLE'"), + }, + { + Config: acc.ConfigEmpty(), // triggers delete and because delete timeout is 1s, it times out + ExpectError: regexp.MustCompile("timeout while waiting for state to become 'DELETED'"), + }, + { + // deletion of the flex cluster has been triggered, but has timed out in previous step, so this is needed in order to avoid "Error running post-test destroy, there may be dangling resource [...] Cluster already requested to be deleted" + Config: acc.ConfigRemove(resourceName), + }, + }, + }) +} + func checkFlexClusterConfig(projectID, clusterName, providerName, region string, tagsCheck bool) resource.TestCheckFunc { checks := []resource.TestCheckFunc{acc.CheckExistsFlexCluster()} attrMapAdvCluster := map[string]string{ diff --git a/internal/service/flexcluster/resource_test.go b/internal/service/flexcluster/resource_test.go index 1640d8eda5..5fd0ef27d3 100644 --- a/internal/service/flexcluster/resource_test.go +++ b/internal/service/flexcluster/resource_test.go @@ -26,12 +26,14 @@ func TestAccFlexClusterRS_failedUpdate(t *testing.T) { resource.Test(t, *tc) } -func TestAccFlexClusterRS_timeouts(t *testing.T) { +func TestAccFlexClusterRS_createTimeout(t *testing.T) { var ( - projectID = acc.ProjectIDExecution(t) - clusterName = acc.RandomName() - provider = "AWS" - region = "US_EAST_1" + projectID = acc.ProjectIDExecution(t) + clusterName = acc.RandomName() + provider = "AWS" + region = "US_EAST_1" + createTimeout = "1s" + deleteOnCreateTimeout = true ) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acc.PreCheckBasic(t) }, @@ -39,20 +41,53 @@ func TestAccFlexClusterRS_timeouts(t *testing.T) { CheckDestroy: acc.CheckDestroyFlexCluster, // verify cleanup was effective Steps: []resource.TestStep{ { - Config: configWithTimeouts(projectID, clusterName, provider, region, true), + Config: configBasic(projectID, clusterName, provider, region, acc.TimeoutConfig(&createTimeout, nil, nil, true), true, false, &deleteOnCreateTimeout), ExpectError: regexp.MustCompile("will run cleanup because delete_on_create_timeout is true"), }, }, }) } +func TestAccFlexClusterRS_updateDeleteTimeout(t *testing.T) { + var ( + projectID = acc.ProjectIDExecution(t) + clusterName = acc.RandomName() + provider = "AWS" + region = "US_EAST_1" + updateTimeout = "1s" + deleteTimeout = "1s" + ) + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acc.PreCheckBasic(t) }, + ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, + Steps: []resource.TestStep{ + { + Config: configBasic(projectID, clusterName, provider, region, acc.TimeoutConfig(nil, &updateTimeout, &deleteTimeout, true), false, false, nil), + }, + { + Config: configBasic(projectID, clusterName, provider, region, acc.TimeoutConfig(nil, &updateTimeout, &deleteTimeout, true), false, true, nil), + ExpectError: regexp.MustCompile("timeout while waiting for state to become 'IDLE'"), + }, + { + Config: acc.ConfigEmpty(), // triggers delete and because delete timeout is 1s, it times out + ExpectError: regexp.MustCompile("timeout while waiting for state to become 'DELETED'"), + }, + { + // deletion of the flex cluster has been triggered, but has timed out in previous step, so this is needed in order to avoid "Error running post-test destroy, there may be dangling resource [...] Cluster already requested to be deleted" + Config: acc.ConfigRemove(resourceName), + }, + }, + }) +} + func basicTestCase(t *testing.T) *resource.TestCase { t.Helper() var ( - projectID = acc.ProjectIDExecution(t) - clusterName = acc.RandomName() - provider = "AWS" - region = "US_EAST_1" + projectID = acc.ProjectIDExecution(t) + clusterName = acc.RandomName() + provider = "AWS" + region = "US_EAST_1" + emptyTimeoutConfig = "" ) return &resource.TestCase{ PreCheck: func() { acc.PreCheckBasic(t) }, @@ -60,15 +95,15 @@ func basicTestCase(t *testing.T) *resource.TestCase { CheckDestroy: acc.CheckDestroyFlexCluster, Steps: []resource.TestStep{ { - Config: configBasic(projectID, clusterName, provider, region, true, false), + Config: configBasic(projectID, clusterName, provider, region, emptyTimeoutConfig, true, false, nil), Check: checksFlexCluster(projectID, clusterName, true, false), }, { - Config: configBasic(projectID, clusterName, provider, region, false, true), + Config: configBasic(projectID, clusterName, provider, region, emptyTimeoutConfig, false, true, nil), Check: checksFlexCluster(projectID, clusterName, false, true), }, { - Config: configBasic(projectID, clusterName, provider, region, true, true), + Config: configBasic(projectID, clusterName, provider, region, emptyTimeoutConfig, true, true, nil), ResourceName: resourceName, ImportStateIdFunc: acc.ImportStateIDFuncProjectIDClusterName(resourceName, "project_id", "name"), ImportState: true, @@ -89,6 +124,7 @@ func failedUpdateTestCase(t *testing.T) *resource.TestCase { providerUpdated = "GCP" region = "US_EAST_1" regionUpdated = "US_EAST_2" + emptyTimeoutConfig = "" ) return &resource.TestCase{ PreCheck: func() { acc.PreCheckBasic(t) }, @@ -96,30 +132,30 @@ func failedUpdateTestCase(t *testing.T) *resource.TestCase { CheckDestroy: acc.CheckDestroyFlexCluster, Steps: []resource.TestStep{ { - Config: configBasic(projectID, clusterName, provider, region, false, false), + Config: configBasic(projectID, clusterName, provider, region, emptyTimeoutConfig, false, false, nil), Check: checksFlexCluster(projectID, clusterName, false, false), }, { - Config: configBasic(projectID, clusterNameUpdated, provider, region, false, false), + Config: configBasic(projectID, clusterNameUpdated, provider, region, emptyTimeoutConfig, false, false, nil), ExpectError: regexp.MustCompile("name cannot be updated"), }, { - Config: configBasic(projectIDUpdated, clusterName, provider, region, false, false), + Config: configBasic(projectIDUpdated, clusterName, provider, region, emptyTimeoutConfig, false, false, nil), ExpectError: regexp.MustCompile("project_id cannot be updated"), }, { - Config: configBasic(projectID, clusterName, providerUpdated, region, false, false), + Config: configBasic(projectID, clusterName, providerUpdated, region, emptyTimeoutConfig, false, false, nil), ExpectError: regexp.MustCompile("provider_settings.backing_provider_name cannot be updated"), }, { - Config: configBasic(projectID, clusterName, provider, regionUpdated, false, false), + Config: configBasic(projectID, clusterName, provider, regionUpdated, emptyTimeoutConfig, false, false, nil), ExpectError: regexp.MustCompile("provider_settings.region_name cannot be updated"), }, }, } } -func configBasic(projectID, clusterName, provider, region string, terminationProtectionEnabled, tags bool) string { +func configBasic(projectID, clusterName, provider, region, timeoutConfig string, terminationProtectionEnabled, tags bool, deleteOnCreateTimeout *bool) string { tagsConfig := "" if tags { tagsConfig = ` @@ -127,6 +163,12 @@ func configBasic(projectID, clusterName, provider, region string, terminationPro testKey = "testValue" }` } + deleteOnCreateTimeoutConfig := "" + if deleteOnCreateTimeout != nil { + deleteOnCreateTimeoutConfig = fmt.Sprintf(` + delete_on_create_timeout = %[1]t + `, *deleteOnCreateTimeout) + } return fmt.Sprintf(` resource "mongodbatlas_flex_cluster" "test" { project_id = %[1]q @@ -137,26 +179,11 @@ func configBasic(projectID, clusterName, provider, region string, terminationPro } termination_protection_enabled = %[5]t %[6]s + %[7]s + %[8]s } - %[7]s - `, projectID, clusterName, provider, region, terminationProtectionEnabled, tagsConfig, acc.FlexDataSource) -} - -func configWithTimeouts(projectID, clusterName, provider, region string, deleteOnCreateTimeout bool) string { - return fmt.Sprintf(` - resource "mongodbatlas_flex_cluster" "test" { - project_id = %[1]q - name = %[2]q - provider_settings = { - backing_provider_name = %[3]q - region_name = %[4]q - } - delete_on_create_timeout = %[5]t - timeouts = { - create = "1s" - } - } - `, projectID, clusterName, provider, region, deleteOnCreateTimeout) + %[9]s + `, projectID, clusterName, provider, region, terminationProtectionEnabled, deleteOnCreateTimeoutConfig, tagsConfig, timeoutConfig, acc.FlexDataSource) } func checksFlexCluster(projectID, clusterName string, terminationProtectionEnabled, tagsCheck bool) resource.TestCheckFunc { diff --git a/internal/service/organization/resource_organization_test.go b/internal/service/organization/resource_organization_test.go index 5fd1f6e7a7..b41e9510e6 100644 --- a/internal/service/organization/resource_organization_test.go +++ b/internal/service/organization/resource_organization_test.go @@ -262,7 +262,7 @@ func TestAccConfigRSOrganization_import(t *testing.T) { { // Use removed block so the organization is not deleted. // Even if something goes wrong, the organization wouldn't be deleted if it has some projects, it would return ORG_NOT_EMPTY error. - Config: configImportRemove(), + Config: acc.ConfigRemove(resourceName), }, }, }) @@ -308,17 +308,6 @@ func configImportSet(orgID, orgName string) string { `, orgID, orgName) } -func configImportRemove() string { - return ` - removed { - from = mongodbatlas_organization.test - lifecycle { - destroy = false - } - } - ` -} - func checkExists(resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[resourceName] diff --git a/internal/testutil/acc/common_config.go b/internal/testutil/acc/common_config.go new file mode 100644 index 0000000000..0b51817f9c --- /dev/null +++ b/internal/testutil/acc/common_config.go @@ -0,0 +1,53 @@ +package acc + +import "fmt" + +func TimeoutConfig(createTimeout, updateTimeout, deleteTimeout *string, isTPF bool) string { + createTimeoutConfig := "" + updateTimeoutConfig := "" + deleteTimeoutConfig := "" + + if createTimeout != nil { + createTimeoutConfig = fmt.Sprintf(` + create = %q + `, *createTimeout) + } + if updateTimeout != nil { + updateTimeoutConfig = fmt.Sprintf(` + update = %q + `, *updateTimeout) + } + if deleteTimeout != nil { + deleteTimeoutConfig = fmt.Sprintf(` + delete = %q + `, *deleteTimeout) + } + timeoutConfig := "timeouts" + if isTPF { + timeoutConfig = "timeouts =" + } + return fmt.Sprintf(` + %[1]s { + %[2]s + %[3]s + %[4]s + } + `, timeoutConfig, createTimeoutConfig, updateTimeoutConfig, deleteTimeoutConfig) +} + +func ConfigRemove(resourceName string) string { + return fmt.Sprintf(` + removed { + from = %s + lifecycle { + destroy = false + } + } + `, resourceName) +} + +func ConfigEmpty() string { + return ` + # empty config to trigger delete + ` +} From 4ff82b3f48385b0fa297af74b1c8b11d11ab4b27 Mon Sep 17 00:00:00 2001 From: Oriol Arbusi Abadal Date: Thu, 31 Jul 2025 13:22:28 +0200 Subject: [PATCH 17/19] remove config on import step --- internal/service/flexcluster/resource_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/service/flexcluster/resource_test.go b/internal/service/flexcluster/resource_test.go index 5fd0ef27d3..d8a868e471 100644 --- a/internal/service/flexcluster/resource_test.go +++ b/internal/service/flexcluster/resource_test.go @@ -103,7 +103,6 @@ func basicTestCase(t *testing.T) *resource.TestCase { Check: checksFlexCluster(projectID, clusterName, false, true), }, { - Config: configBasic(projectID, clusterName, provider, region, emptyTimeoutConfig, true, true, nil), ResourceName: resourceName, ImportStateIdFunc: acc.ImportStateIDFuncProjectIDClusterName(resourceName, "project_id", "name"), ImportState: true, From d1d777cba675942ceaeeb8c97aed450e77fb4193 Mon Sep 17 00:00:00 2001 From: Oriol Arbusi Abadal Date: Thu, 31 Jul 2025 15:02:55 +0200 Subject: [PATCH 18/19] remove redundant checkDestroy --- internal/service/flexcluster/resource_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/service/flexcluster/resource_test.go b/internal/service/flexcluster/resource_test.go index d8a868e471..f831112684 100644 --- a/internal/service/flexcluster/resource_test.go +++ b/internal/service/flexcluster/resource_test.go @@ -38,7 +38,6 @@ func TestAccFlexClusterRS_createTimeout(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acc.PreCheckBasic(t) }, ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, - CheckDestroy: acc.CheckDestroyFlexCluster, // verify cleanup was effective Steps: []resource.TestStep{ { Config: configBasic(projectID, clusterName, provider, region, acc.TimeoutConfig(&createTimeout, nil, nil, true), true, false, &deleteOnCreateTimeout), From 66c72efffea3ae6fcd55571f65b4cb53cbc16e1e Mon Sep 17 00:00:00 2001 From: Oriol Arbusi Abadal Date: Thu, 31 Jul 2025 15:03:40 +0200 Subject: [PATCH 19/19] remove redundant checkDestroy in adv_cluster test --- .../service/advancedcluster/resource_advanced_cluster_test.go | 1 - internal/service/flexcluster/resource_test.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/service/advancedcluster/resource_advanced_cluster_test.go b/internal/service/advancedcluster/resource_advanced_cluster_test.go index 0553219e5f..e92d6ca9ca 100644 --- a/internal/service/advancedcluster/resource_advanced_cluster_test.go +++ b/internal/service/advancedcluster/resource_advanced_cluster_test.go @@ -3279,7 +3279,6 @@ func TestAccAdvancedCluster_createTimeoutWithDeleteOnCreateFlex(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acc.PreCheckBasic(t) }, ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, - CheckDestroy: acc.CheckDestroyFlexCluster, Steps: []resource.TestStep{ { Config: configFlexCluster(t, projectID, clusterName, "AWS", "US_EAST_1", "", acc.TimeoutConfig(&createTimeout, nil, nil, false), false, &deleteOnCreateTimeout), diff --git a/internal/service/flexcluster/resource_test.go b/internal/service/flexcluster/resource_test.go index f831112684..c112e90455 100644 --- a/internal/service/flexcluster/resource_test.go +++ b/internal/service/flexcluster/resource_test.go @@ -26,7 +26,7 @@ func TestAccFlexClusterRS_failedUpdate(t *testing.T) { resource.Test(t, *tc) } -func TestAccFlexClusterRS_createTimeout(t *testing.T) { +func TestAccFlexClusterRS_createTimeoutWithDeleteOnCreateFlex(t *testing.T) { var ( projectID = acc.ProjectIDExecution(t) clusterName = acc.RandomName()