diff --git a/docs/data-sources/sfs_resource_pool.md b/docs/data-sources/sfs_resource_pool.md index 119848999..97c2b992b 100644 --- a/docs/data-sources/sfs_resource_pool.md +++ b/docs/data-sources/sfs_resource_pool.md @@ -33,6 +33,7 @@ data "stackit_sfs_resource_pool" "resourcepool" { ### Optional - `region` (String) The resource region. Read-only attribute that reflects the provider region. +- `timeouts` (Attributes) (see [below for nested schema](#nestedatt--timeouts)) ### Read-Only @@ -48,6 +49,14 @@ data "stackit_sfs_resource_pool" "resourcepool" { - `snapshot_policy` (Attributes) Name of the snapshot policy. (see [below for nested schema](#nestedatt--snapshot_policy)) - `snapshots_are_visible` (Boolean) If set to true, snapshots are visible and accessible to users. (default: false) + +### Nested Schema for `timeouts` + +Optional: + +- `read` (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 `snapshot_policy` diff --git a/docs/resources/sfs_resource_pool.md b/docs/resources/sfs_resource_pool.md index 60368822f..ca0cdffcb 100644 --- a/docs/resources/sfs_resource_pool.md +++ b/docs/resources/sfs_resource_pool.md @@ -51,6 +51,7 @@ resource "stackit_sfs_resource_pool" "resourcepool" { - `region` (String) The resource region. If not defined, the provider region is used. - `snapshot_policy` (Attributes) Name of the snapshot policy. (see [below for nested schema](#nestedatt--snapshot_policy)) - `snapshots_are_visible` (Boolean) If set to true, snapshots are visible and accessible to users. (default: false) +- `timeouts` (Attributes) (see [below for nested schema](#nestedatt--timeouts)) ### Read-Only @@ -68,6 +69,17 @@ Read-Only: - `name` (String) Name of the snapshot policy. + + +### 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. +- `read` (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). Read operations occur during any refresh or planning operation when refresh is enabled. +- `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). + ## Import Import is supported using the following syntax: diff --git a/stackit/internal/services/sfs/resourcepool/datasource.go b/stackit/internal/services/sfs/resourcepool/datasource.go index 65c2c89af..787d4175a 100644 --- a/stackit/internal/services/sfs/resourcepool/datasource.go +++ b/stackit/internal/services/sfs/resourcepool/datasource.go @@ -7,6 +7,7 @@ import ( "net/http" "time" + "github.com/hashicorp/terraform-plugin-framework-timeouts/datasource/timeouts" "github.com/hashicorp/terraform-plugin-framework-validators/listvalidator" "github.com/hashicorp/terraform-plugin-framework/datasource" "github.com/hashicorp/terraform-plugin-framework/datasource/schema" @@ -45,6 +46,7 @@ type dataSourceModel struct { SnapshotsAreVisible types.Bool `tfsdk:"snapshots_are_visible"` SnapshotPolicy *SnapshotPolicyModel `tfsdk:"snapshot_policy"` Labels types.Map `tfsdk:"labels"` + Timeouts timeouts.Value `tfsdk:"timeouts"` } type resourcePoolDataSource struct { @@ -90,6 +92,15 @@ func (r *resourcePoolDataSource) Read(ctx context.Context, req datasource.ReadRe if resp.Diagnostics.HasError() { return } + + readTimeout, diags := model.Timeouts.Read(ctx, core.DefaultOperationTimeout) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } + ctx, cancel := context.WithTimeout(ctx, readTimeout) + defer cancel() + projectId := model.ProjectId.ValueString() resourcePoolId := model.ResourcePoolId.ValueString() region := r.providerData.GetRegionWithOverride(model.Region) @@ -130,7 +141,7 @@ func (r *resourcePoolDataSource) Read(ctx context.Context, req datasource.ReadRe } // Schema implements datasource.DataSource. -func (r *resourcePoolDataSource) Schema(_ context.Context, _ datasource.SchemaRequest, resp *datasource.SchemaResponse) { +func (r *resourcePoolDataSource) Schema(ctx context.Context, _ datasource.SchemaRequest, resp *datasource.SchemaResponse) { description := "Resource-pool datasource schema. Must have a `region` specified in the provider configuration." resp.Schema = schema.Schema{ MarkdownDescription: features.AddBetaDescription(description, core.Datasource), @@ -217,6 +228,7 @@ func (r *resourcePoolDataSource) Schema(_ context.Context, _ datasource.SchemaRe ElementType: types.StringType, Computed: true, }, + "timeouts": timeouts.Attributes(ctx), }, } } diff --git a/stackit/internal/services/sfs/resourcepool/resource.go b/stackit/internal/services/sfs/resourcepool/resource.go index 096510ceb..e2bf1f886 100644 --- a/stackit/internal/services/sfs/resourcepool/resource.go +++ b/stackit/internal/services/sfs/resourcepool/resource.go @@ -8,6 +8,7 @@ import ( "net/http" "strings" + "github.com/hashicorp/terraform-plugin-framework-timeouts/resource/timeouts" "github.com/hashicorp/terraform-plugin-framework-validators/listvalidator" "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/diag" @@ -43,18 +44,19 @@ var ( ) type Model struct { - Id types.String `tfsdk:"id"` // needed by TF - ProjectId types.String `tfsdk:"project_id"` - ResourcePoolId types.String `tfsdk:"resource_pool_id"` - AvailabilityZone types.String `tfsdk:"availability_zone"` - IpAcl types.List `tfsdk:"ip_acl"` - Name types.String `tfsdk:"name"` - Labels types.Map `tfsdk:"labels"` - PerformanceClass types.String `tfsdk:"performance_class"` - SizeGigabytes types.Int32 `tfsdk:"size_gigabytes"` - SnapshotPolicy types.Object `tfsdk:"snapshot_policy"` - Region types.String `tfsdk:"region"` - SnapshotsAreVisible types.Bool `tfsdk:"snapshots_are_visible"` + Id types.String `tfsdk:"id"` // needed by TF + ProjectId types.String `tfsdk:"project_id"` + ResourcePoolId types.String `tfsdk:"resource_pool_id"` + AvailabilityZone types.String `tfsdk:"availability_zone"` + IpAcl types.List `tfsdk:"ip_acl"` + Name types.String `tfsdk:"name"` + Labels types.Map `tfsdk:"labels"` + PerformanceClass types.String `tfsdk:"performance_class"` + SizeGigabytes types.Int32 `tfsdk:"size_gigabytes"` + SnapshotPolicy types.Object `tfsdk:"snapshot_policy"` + Region types.String `tfsdk:"region"` + SnapshotsAreVisible types.Bool `tfsdk:"snapshots_are_visible"` + Timeouts timeouts.Value `tfsdk:"timeouts"` } type SnapshotPolicyModel struct { @@ -134,7 +136,7 @@ func (r *resourcePoolResource) Configure(ctx context.Context, req resource.Confi } // Schema defines the schema for the resource. -func (r *resourcePoolResource) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) { +func (r *resourcePoolResource) Schema(ctx context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) { description := "Resource-pool resource schema. Must have a `region` specified in the provider configuration." resp.Schema = schema.Schema{ MarkdownDescription: features.AddBetaDescription(description, core.Resource), @@ -245,6 +247,7 @@ func (r *resourcePoolResource) Schema(_ context.Context, _ resource.SchemaReques }, }, }, + "timeouts": timeouts.AttributesAll(ctx), }, } } @@ -259,6 +262,17 @@ func (r *resourcePoolResource) Create(ctx context.Context, req resource.CreateRe return } + // The wait handler only enforces its own timeout when the context carries no deadline, + // so the context deadline set here is what actually bounds the polling. + waiterTimeout := wait.CreateResourcePoolWaitHandler(ctx, r.client.DefaultAPI, "", "", "").GetTimeout() //nolint:tfctxinit,tfwriteid // false positive - only called to read the default wait handler timeout + createTimeout, diags := model.Timeouts.Create(ctx, waiterTimeout+core.DefaultTimeoutMargin) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } + ctx, cancel := context.WithTimeout(ctx, createTimeout) + defer cancel() + projectId := model.ProjectId.ValueString() region := model.Region.ValueString() ctx = tflog.SetField(ctx, "project_id", projectId) @@ -301,7 +315,8 @@ func (r *resourcePoolResource) Create(ctx context.Context, req resource.CreateRe response, err := wait.CreateResourcePoolWaitHandler(ctx, r.client.DefaultAPI, projectId, region, *resourcePool.ResourcePool.Id). WaitWithContext(ctx) if err != nil { - core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating resource pool", fmt.Sprintf("resource pool creation waiting: %v", err)) + core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating resource pool", + fmt.Sprintf("resource pool creation waiting: %v%s", err, utils.TimeoutHint(ctx, "create", createTimeout))) return } ctx = tflog.SetField(ctx, "resource_pool_id", response.ResourcePool.Id) @@ -343,6 +358,15 @@ func (r *resourcePoolResource) Read(ctx context.Context, req resource.ReadReques if resp.Diagnostics.HasError() { return } + + readTimeout, diags := model.Timeouts.Read(ctx, core.DefaultOperationTimeout) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } + ctx, cancel := context.WithTimeout(ctx, readTimeout) + defer cancel() + projectId := model.ProjectId.ValueString() resourcePoolId := model.ResourcePoolId.ValueString() if resourcePoolId == "" { @@ -395,6 +419,16 @@ func (r *resourcePoolResource) Update(ctx context.Context, req resource.UpdateRe if resp.Diagnostics.HasError() { return } + + waiterTimeout := wait.UpdateResourcePoolWaitHandler(ctx, r.client.DefaultAPI, "", "", "").GetTimeout() //nolint:tfctxinit // false positive - only called to read the default wait handler timeout + updateTimeout, diags := model.Timeouts.Update(ctx, waiterTimeout+core.DefaultTimeoutMargin) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } + ctx, cancel := context.WithTimeout(ctx, updateTimeout) + defer cancel() + projectId := model.ProjectId.ValueString() resourcePoolId := model.ResourcePoolId.ValueString() region := model.Region.ValueString() @@ -445,7 +479,8 @@ func (r *resourcePoolResource) Update(ctx context.Context, req resource.UpdateRe getResponse, err := wait.UpdateResourcePoolWaitHandler(ctx, r.client.DefaultAPI, projectId, region, resourcePoolId).WaitWithContext(ctx) if err != nil { - core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating resource pool", fmt.Sprintf("resource pool get: %v", err)) + core.LogAndAddError(ctx, &resp.Diagnostics, "Error updating resource pool", + fmt.Sprintf("resource pool update waiting: %v%s", err, utils.TimeoutHint(ctx, "update", updateTimeout))) return } err = mapFields(ctx, region, getResponse.ResourcePool, &model) @@ -471,6 +506,15 @@ func (r *resourcePoolResource) Delete(ctx context.Context, req resource.DeleteRe return } + waiterTimeout := wait.DeleteResourcePoolWaitHandler(ctx, r.client.DefaultAPI, "", "", "").GetTimeout() //nolint:tfctxinit // false positive - only called to read the default wait handler timeout + deleteTimeout, diags := model.Timeouts.Delete(ctx, waiterTimeout+core.DefaultTimeoutMargin) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } + ctx, cancel := context.WithTimeout(ctx, deleteTimeout) + defer cancel() + projectId := model.ProjectId.ValueString() resourcePoolId := model.ResourcePoolId.ValueString() region := model.Region.ValueString() @@ -498,7 +542,8 @@ func (r *resourcePoolResource) Delete(ctx context.Context, req resource.DeleteRe // only delete, if no error occurred _, err = wait.DeleteResourcePoolWaitHandler(ctx, r.client.DefaultAPI, projectId, region, resourcePoolId).WaitWithContext(ctx) if err != nil { - core.LogAndAddError(ctx, &resp.Diagnostics, "Error deleting resource pool", fmt.Sprintf("resource pool deletion waiting: %v", err)) + core.LogAndAddError(ctx, &resp.Diagnostics, "Error deleting resource pool", + fmt.Sprintf("resource pool deletion waiting: %v%s", err, utils.TimeoutHint(ctx, "delete", deleteTimeout))) return } diff --git a/stackit/internal/services/sfs/sfs_acc_test.go b/stackit/internal/services/sfs/sfs_acc_test.go index c513a584f..1b376fefd 100644 --- a/stackit/internal/services/sfs/sfs_acc_test.go +++ b/stackit/internal/services/sfs/sfs_acc_test.go @@ -550,6 +550,10 @@ func TestAccResourcePoolResourceMax(t *testing.T) { data "stackit_sfs_resource_pool" "resource_pool_ds" { project_id = stackit_sfs_resource_pool.resourcepool.project_id resource_pool_id = stackit_sfs_resource_pool.resourcepool.resource_pool_id + + timeouts = { + read = "20m" + } } `, testutil.NewConfigBuilder().EnableBetaResources(true).BuildProviderConfig(), resourceResourcePoolMaxConfig, diff --git a/stackit/internal/services/sfs/sfs_test.go b/stackit/internal/services/sfs/sfs_test.go index 5567f9c54..149663c16 100644 --- a/stackit/internal/services/sfs/sfs_test.go +++ b/stackit/internal/services/sfs/sfs_test.go @@ -5,6 +5,7 @@ import ( "net/http" "regexp" "testing" + "time" "github.com/google/uuid" "github.com/hashicorp/terraform-plugin-testing/helper/resource" @@ -162,3 +163,56 @@ resource "stackit_sfs_share" "example" { }, }) } + +// TestSfsResourcePoolCreateTimeout asserts that the configured `timeouts.create` bounds the create wait. Only the +// create timeout is covered: read, update and delete would each need a successful create beforehand, which makes +// those tests slow and flaky. +func TestSfsResourcePoolCreateTimeout(t *testing.T) { + projectId := uuid.NewString() + s := testutil.NewMockServer(t) + defer s.Server.Close() + tfConfig := fmt.Sprintf(` +provider "stackit" { + default_region = "eu01" + sfs_custom_endpoint = "%s" + service_account_token = "mock-server-needs-no-auth" + enable_beta_resources = true +} +resource "stackit_sfs_resource_pool" "resourcepool" { + project_id = "%s" + name = "sfs-instance" + availability_zone = "eu01-m" + performance_class = "Standard" + size_gigabytes = 512 + ip_acl = ["192.168.2.0/24"] + + timeouts = { + create = "10ms" + read = "10ms" + update = "10ms" + delete = "10ms" + } +} +`, s.Server.URL, projectId) + + resource.UnitTest(t, resource.TestCase{ + ProtoV6ProviderFactories: testutil.TestAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + PreConfig: func() { + s.Reset(testutil.MockResponse{ + Description: "answers later than the configured timeout allows", + Handler: func(_ http.ResponseWriter, r *http.Request) { + select { + case <-r.Context().Done(): + case <-time.After(20 * time.Millisecond): + } + }, + }) + }, + Config: tfConfig, + ExpectError: regexp.MustCompile("deadline exceeded"), + }, + }, + }) +} diff --git a/stackit/internal/services/sfs/testdata/resource-pool-max.tf b/stackit/internal/services/sfs/testdata/resource-pool-max.tf index 149ebc251..531768f2b 100644 --- a/stackit/internal/services/sfs/testdata/resource-pool-max.tf +++ b/stackit/internal/services/sfs/testdata/resource-pool-max.tf @@ -25,4 +25,11 @@ resource "stackit_sfs_resource_pool" "resourcepool" { snapshot_policy = { id = var.snapshot_policy_id } + + timeouts = { + create = "20m" + read = "20m" + update = "20m" + delete = "20m" + } } diff --git a/stackit/internal/utils/timeouts.go b/stackit/internal/utils/timeouts.go new file mode 100644 index 000000000..9d4cc87e8 --- /dev/null +++ b/stackit/internal/utils/timeouts.go @@ -0,0 +1,19 @@ +package utils + +import ( + "context" + "errors" + "fmt" + "time" +) + +// TimeoutHint names the configured timeout when the deadline of ctx is what ended a wait, e.g. +// TimeoutHint(ctx, "create", createTimeout). Wait handlers report a timeout, a terminal error state and a failing poll +// through the same error, so the hint is empty unless the deadline was exceeded; otherwise it would point at the wrong +// cause. The hint starts with a newline so it can be appended to a diagnostic detail. +func TimeoutHint(ctx context.Context, operation string, timeout time.Duration) string { + if !errors.Is(ctx.Err(), context.DeadlineExceeded) { + return "" + } + return fmt.Sprintf("\nThe wait gave up after the configured `timeouts.%s` of %s; raise it if the operation regularly needs longer.", operation, timeout) +} diff --git a/stackit/internal/utils/timeouts_test.go b/stackit/internal/utils/timeouts_test.go new file mode 100644 index 000000000..96eb4bdf5 --- /dev/null +++ b/stackit/internal/utils/timeouts_test.go @@ -0,0 +1,51 @@ +package utils + +import ( + "context" + "testing" + "time" +) + +func TestTimeoutHint(t *testing.T) { + deadlineExceeded, cancelDeadlineExceeded := context.WithDeadline(context.Background(), time.Now().Add(-time.Minute)) + defer cancelDeadlineExceeded() + deadlineAhead, cancelDeadlineAhead := context.WithTimeout(context.Background(), time.Hour) + defer cancelDeadlineAhead() + canceled, cancel := context.WithCancel(context.Background()) + cancel() + + tests := []struct { + description string + ctx context.Context + expected string + }{ + { + "deadline exceeded", + deadlineExceeded, + "\nThe wait gave up after the configured `timeouts.create` of 20m0s; raise it if the operation regularly needs longer.", + }, + { + "wait failed before the deadline", + deadlineAhead, + "", + }, + { + "canceled", + canceled, + "", + }, + { + "no deadline", + context.Background(), + "", + }, + } + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + output := TimeoutHint(tt.ctx, "create", 20*time.Minute) + if output != tt.expected { + t.Fatalf("TimeoutHint() = %q, want %q", output, tt.expected) + } + }) + } +}