-
Notifications
You must be signed in to change notification settings - Fork 205
feat: Long-running operation improvements for mongodbatlas_encryption_at_rest_private_endpoint
resource
#3561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
cddcf57
5d6e1f9
df219b9
695283b
4344479
5858275
37c287e
72aca36
d77b328
6fe7345
90bb920
26b8dc7
8c61292
06aad46
4aa5b41
622b1a1
dcade6f
bf15a87
7547e48
c246282
d73d690
bb81f45
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
```release-note:enhancement | ||
resource/mongodbatlas_encryption_at_rest_private_endpoint: Adds `timeouts` attribute for create, update and delete operations | ||
``` | ||
|
||
```release-note:enhancement | ||
resource/mongodbatlas_encryption_at_rest_private_endpoint: Adds `delete_on_create_timeout` attribute to indicate whether to delete the resource if its creation times out | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -99,13 +99,26 @@ resource "mongodbatlas_encryption_at_rest_private_endpoint" "endpoint" { | |
- `project_id` (String) Unique 24-hexadecimal digit string that identifies your project. | ||
- `region_name` (String) Cloud provider region in which the Encryption At Rest private endpoint is located. | ||
|
||
### Optional | ||
|
||
- `delete_on_create_timeout` (Boolean) Indicates whether to delete the created resource if a timeout is reached when waiting for completion. When set to `true` and timeout occurs, it triggers the cleanup and returns immediately without waiting for deletion to complete. When set to `false`, the timeout will not trigger resource deletion. If you suspect a transient error when the value is `true`, wait before retrying to allow resource deletion to finish. Default is `true`. | ||
- `timeouts` (Attributes) (see [below for nested schema](#nestedatt--timeouts)) | ||
|
||
### Read-Only | ||
|
||
- `error_message` (String) Error message for failures associated with the Encryption At Rest private endpoint. | ||
- `id` (String) Unique 24-hexadecimal digit string that identifies the Private Endpoint Service. | ||
- `private_endpoint_connection_name` (String) Connection name of the Azure Private Endpoint. | ||
- `status` (String) State of the Encryption At Rest private endpoint. | ||
|
||
<a id="nestedatt--timeouts"></a> | ||
### Nested Schema for `timeouts` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @oarbusi I think we should leave a short why the update is not there, thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The update does not have timeout because the update operation is not supported in this resource. Considering that there's a note in the resource doc stating that update is not supported, I think it's not necessary to mention why there's no timeout for the update |
||
|
||
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. | ||
|
||
## Import | ||
Encryption At Rest Private Endpoint resource can be imported using the project ID, cloud provider, and private endpoint ID. The format must be `{project_id}-{cloud_provider}-{private_endpoint_id}` e.g. | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ import ( | |
"github.com/hashicorp/terraform-plugin-framework/path" | ||
"github.com/hashicorp/terraform-plugin-framework/resource" | ||
|
||
"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/cleanup" | ||
"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" | ||
|
@@ -63,13 +64,28 @@ func (r *encryptionAtRestPrivateEndpointRS) Create(ctx context.Context, req reso | |
return | ||
} | ||
|
||
finalResp, err := waitStateTransition(ctx, projectID, cloudProvider, createResp.GetId(), connV2.EncryptionAtRestUsingCustomerKeyManagementApi) | ||
createTimeout := cleanup.ResolveTimeout(ctx, &earPrivateEndpointPlan.Timeouts, cleanup.OperationCreate, &resp.Diagnostics) | ||
if resp.Diagnostics.HasError() { | ||
return | ||
} | ||
|
||
finalResp, err := waitStateTransition(ctx, projectID, cloudProvider, createResp.GetId(), connV2.EncryptionAtRestUsingCustomerKeyManagementApi, createTimeout) | ||
err = cleanup.HandleCreateTimeout(cleanup.ResolveDeleteOnCreateTimeout(earPrivateEndpointPlan.DeleteOnCreateTimeout), err, func(ctxCleanup context.Context) error { | ||
cleanResp, cleanErr := connV2.EncryptionAtRestUsingCustomerKeyManagementApi.RequestEncryptionAtRestPrivateEndpointDeletion(ctxCleanup, projectID, cloudProvider, createResp.GetId()).Execute() | ||
if validate.StatusNotFound(cleanResp) { | ||
return nil | ||
} | ||
Comment on lines
+75
to
+77
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. q: I see in some resources (e.g. networkpeering) the sanity check of seeing if delete return 404 is not being made. For consistency, would it make sense to move this check into There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In that case would consider that privatelinkendpointservice, privatelinkendpoint, onlinearchive, networkpeering, and cloudbackupsnapshot do not have this check in their cleanup function. Not certain if we want to keep this as a general practice. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is unrelated to long-running operations, but we could create a CLOUDP to investigate and unify behavior on 404 across the resources you mentioned? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i suppose we could keep the code simple here, e.g.:
and in HandleCreateTimeout don't raise error if cleanup funcs return 404 error, if we expect this will happen with most of the resources There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Correct, under this assumption I was proposing the same adjustment to HandleCreateTimeout |
||
return cleanErr | ||
}) | ||
|
||
if err != nil { | ||
resp.Diagnostics.AddError("error when waiting for status transition in creation", err.Error()) | ||
return | ||
} | ||
|
||
privateEndpointModel := NewTFEarPrivateEndpoint(*finalResp, projectID) | ||
privateEndpointModel.Timeouts = earPrivateEndpointPlan.Timeouts | ||
privateEndpointModel.DeleteOnCreateTimeout = earPrivateEndpointPlan.DeleteOnCreateTimeout | ||
resp.Diagnostics.Append(resp.State.Set(ctx, privateEndpointModel)...) | ||
|
||
diags := CheckErrorMessageAndStatus(finalResp) | ||
|
@@ -98,7 +114,10 @@ func (r *encryptionAtRestPrivateEndpointRS) Read(ctx context.Context, req resour | |
return | ||
} | ||
|
||
resp.Diagnostics.Append(resp.State.Set(ctx, NewTFEarPrivateEndpoint(*endpointModel, projectID))...) | ||
privateEndpointModel := NewTFEarPrivateEndpoint(*endpointModel, projectID) | ||
privateEndpointModel.Timeouts = earPrivateEndpointState.Timeouts | ||
privateEndpointModel.DeleteOnCreateTimeout = earPrivateEndpointState.DeleteOnCreateTimeout | ||
resp.Diagnostics.Append(resp.State.Set(ctx, privateEndpointModel)...) | ||
|
||
diags := CheckErrorMessageAndStatus(endpointModel) | ||
resp.Diagnostics.Append(diags...) | ||
|
@@ -124,7 +143,12 @@ func (r *encryptionAtRestPrivateEndpointRS) Delete(ctx context.Context, req reso | |
return | ||
} | ||
|
||
model, err := WaitDeleteStateTransition(ctx, projectID, cloudProvider, endpointID, connV2.EncryptionAtRestUsingCustomerKeyManagementApi) | ||
deleteTimeout := cleanup.ResolveTimeout(ctx, &earPrivateEndpointState.Timeouts, cleanup.OperationDelete, &resp.Diagnostics) | ||
if resp.Diagnostics.HasError() { | ||
return | ||
} | ||
|
||
model, err := WaitDeleteStateTransition(ctx, projectID, cloudProvider, endpointID, connV2.EncryptionAtRestUsingCustomerKeyManagementApi, deleteTimeout) | ||
if err != nil { | ||
resp.Diagnostics.AddError("error when waiting for status transition in delete", err.Error()) | ||
return | ||
|
Uh oh!
There was an error while loading. Please reload this page.