From 6644872da8da56910b882a24d8fa1a4d42979ea5 Mon Sep 17 00:00:00 2001 From: Jan Schreier Date: Fri, 4 Sep 2026 12:52:32 +0200 Subject: [PATCH 1/4] feat(sfs): make resource pool wait timeouts configurable CreateResourcePoolWaitHandler and its update/delete counterparts default to 10 minutes. The resource passed a context without a deadline, so that default was the only limit and no configuration could reach it. A pool that STACKIT needs longer than 10 minutes to provision could not be created at all. The SDK wait handler applies its own timeout only when the incoming context carries no deadline (core/wait.WaitWithContext). Setting a context deadline in each CRUD method therefore replaces the hardcoded value, which is what the new `timeouts` attribute does. Defaults stay at the wait handler value plus core.DefaultTimeoutMargin, so unconfigured resources keep their behavior. The configured timeouts are written to state together with the IDs before the create wait starts. Without that, a failed wait leaves an entry whose refresh and destroy fall back to the default timeouts - on exactly the recovery path those values are needed for. The error raised when the create wait handler gives up now says that Terraform marks the resource tainted and replaces it on the next run, names `untaint` and the import ID, and mentions `timeouts.create` only when this context's deadline is what ended the wait. The handler reports terminal error states and failing polls through the same error, which are not timeouts. TestWaitHandlerTimeoutIsBoundedByContext pins the SDK behavior the attribute depends on, so an SDK bump that enforces the handler timeout unconditionally fails the build instead of silently capping the configured value again. --- docs/resources/sfs_resource_pool.md | 12 + .../services/sfs/resourcepool/resource.go | 88 +++++-- stackit/internal/services/sfs/sfs_test.go | 234 ++++++++++++++++++ .../sfs/testdata/resource-pool-max.tf | 7 + 4 files changed, 327 insertions(+), 14 deletions(-) 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/resource.go b/stackit/internal/services/sfs/resourcepool/resource.go index 096510ceb..15995dd73 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) @@ -297,11 +311,31 @@ func (r *resourcePoolResource) Create(ctx context.Context, req resource.CreateRe if resp.Diagnostics.HasError() { return } + // The configured timeouts belong in that partial state as well. Without them a failed wait leaves an entry whose + // read and delete fall back to the defaults instead of the values the operator configured. + resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("timeouts"), model.Timeouts)...) + if resp.Diagnostics.HasError() { + return + } 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)) + // The wait handler reports a timeout, a terminal error state and a failing poll through the same error, so + // only mention the create timeout when this context's deadline is what ended the wait. + timeoutHint := "" + if errors.Is(ctx.Err(), context.DeadlineExceeded) { + timeoutHint = fmt.Sprintf(" The wait gave up after the configured `timeouts.create` of %s; raise it if the creation regularly needs longer.", createTimeout) + } + core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating resource pool", fmt.Sprintf( + "resource pool creation waiting: %v\n"+ + "The resource pool was created, and Terraform marks this resource as tainted, so the next apply replaces it. "+ + "Run `terraform untaint` on it first if the next run should refresh the existing pool instead. "+ + "If the state entry is lost, import the resource pool with the ID %q.%s", + err, + utils.BuildInternalTerraformId(projectId, region, *resourcePool.ResourcePool.Id).ValueString(), + timeoutHint, + )) return } ctx = tflog.SetField(ctx, "resource_pool_id", response.ResourcePool.Id) @@ -343,6 +377,14 @@ 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 +437,15 @@ 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() @@ -471,6 +522,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() diff --git a/stackit/internal/services/sfs/sfs_test.go b/stackit/internal/services/sfs/sfs_test.go index 5567f9c54..8b5a0a635 100644 --- a/stackit/internal/services/sfs/sfs_test.go +++ b/stackit/internal/services/sfs/sfs_test.go @@ -1,13 +1,19 @@ package sfs import ( + "context" + "encoding/json" "fmt" "net/http" + "net/http/httptest" "regexp" + "sync" "testing" + "time" "github.com/google/uuid" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + corewait "github.com/stackitcloud/stackit-sdk-go/core/wait" sfs "github.com/stackitcloud/stackit-sdk-go/services/sfs/v1api" "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/testutil" @@ -162,3 +168,231 @@ resource "stackit_sfs_share" "example" { }, }) } + +// TestSfsResourcePoolCreateTimeoutIsConfigurable asserts that the configured `timeouts.create` value is what ends +// the create wait. The wait handler applies its own hardcoded 10 minutes otherwise, which no configuration reaches. +// +// Three signals are needed, because the provider reports every wait failure through the same message: the error must +// name the configured value (only the deadline branch does that), the pool must have been polled more than once, and +// the polling window must be close to the configured value rather than to zero or to the mock's poll budget. +func TestSfsResourcePoolCreateTimeoutIsConfigurable(t *testing.T) { + projectId := uuid.NewString() + resourcePoolId := uuid.NewString() + const ( + region = "eu01" + // Longer than the wait handler's 5s throttle, so several polls happen before the deadline ends the wait. + createTimeout = 12 * time.Second + // Safety valve. Without a context deadline in Create the wait would run for the handler's own 10 minutes and + // blow the package test timeout before any assertion could report the regression. + pollBudget = 30 * time.Second + poolState = "creating" + ) + + var ( + mu sync.Mutex + createdAt time.Time + lastPollAt time.Time + polls int + deleted bool + ) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + mu.Lock() + defer mu.Unlock() + switch req.Method { + case http.MethodPost: + createdAt = time.Now() + writeJSON(t, w, sfs.CreateResourcePoolResponse{ + ResourcePool: &sfs.ResourcePool{Id: new(resourcePoolId)}, + }) + case http.MethodDelete: + deleted = true + w.WriteHeader(http.StatusAccepted) + default: + // Report the pool as gone once the test cleanup has deleted it, so the delete wait can finish. + if deleted { + w.WriteHeader(http.StatusNotFound) + return + } + polls++ + lastPollAt = time.Now() + if time.Since(createdAt) > pollBudget { + w.WriteHeader(http.StatusInternalServerError) + return + } + // The pool never becomes ready, so only a timeout can end the wait. + writeJSON(t, w, sfs.GetResourcePoolResponse{ + ResourcePool: &sfs.ResourcePool{Id: new(resourcePoolId), State: new(poolState)}, + }) + } + })) + defer server.Close() + + tfConfig := fmt.Sprintf(` +provider "stackit" { + default_region = "%s" + 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 = "%s" + } +} +`, region, server.URL, projectId, createTimeout) + + resource.UnitTest(t, resource.TestCase{ + ProtoV6ProviderFactories: testutil.TestAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: tfConfig, + // Only the deadline branch of the error names the configured value. + ExpectError: regexp.MustCompile(regexp.QuoteMeta(createTimeout.String())), + }, + }, + }) + + mu.Lock() + defer mu.Unlock() + if polls < 2 { + t.Errorf("the create wait polled %d times, expected it to keep polling until the configured timeout of %s", + polls, createTimeout) + } + waited := lastPollAt.Sub(createdAt) + if waited < createTimeout/2 { + t.Errorf("the create wait ran for %s, expected roughly the configured %s: it failed for some other reason "+ + "before the timeout was reached", waited, createTimeout) + } + if waited > pollBudget-5*time.Second { + t.Errorf("the create wait ran for %s, expected it to give up after the configured %s: the configured value "+ + "is ignored and the wait ran into the mock's poll budget", waited, createTimeout) + } +} + +func writeJSON(t *testing.T, w http.ResponseWriter, body any) { + t.Helper() + w.Header().Set("content-type", "application/json") + if err := json.NewEncoder(w).Encode(body); err != nil { + t.Errorf("Error writing response body: %v", err) + } +} + +// TestSfsResourcePoolKeepsConfiguredTimeoutsOnError asserts that the configured timeouts are part of the partial +// state the resource writes before it starts waiting. They are needed there: after a failed create Terraform marks +// the resource tainted and the next run refreshes it and destroys it, and both operations read their timeout from +// that state entry. Were the attribute missing, those steps would silently fall back to the default timeouts. +func TestSfsResourcePoolKeepsConfiguredTimeoutsOnError(t *testing.T) { + projectId := uuid.NewString() + resourcePoolId := uuid.NewString() + const ( + region = "eu01" + deleteTimeout = "42m" + ) + + s := testutil.NewMockServer(t) + defer s.Server.Close() + tfConfig := fmt.Sprintf(` +provider "stackit" { + default_region = "%s" + 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 = { + delete = "%s" + } +} +`, region, s.Server.URL, projectId, deleteTimeout) + + resource.UnitTest(t, resource.TestCase{ + ProtoV6ProviderFactories: testutil.TestAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + PreConfig: func() { + s.Reset( + testutil.MockResponse{ + Description: "create resource pool", + ToJsonBody: sfs.CreateResourcePoolResponse{ + ResourcePool: &sfs.ResourcePool{Id: new(resourcePoolId)}, + }, + }, + testutil.MockResponse{ + Description: "failing waiter", + StatusCode: http.StatusInternalServerError, + }, + ) + }, + Config: tfConfig, + ExpectError: regexp.MustCompile("Error creating resource pool"), + }, + { + PreConfig: func() { + pool := testutil.MockResponse{ + Description: "read resource pool", + ToJsonBody: sfs.GetResourcePoolResponse{ + ResourcePool: &sfs.ResourcePool{Id: new(resourcePoolId)}, + }, + } + // The step refreshes and then plans, and both read the resource. + s.Reset( + pool, + pool, + testutil.MockResponse{Description: "delete", StatusCode: http.StatusAccepted}, + testutil.MockResponse{Description: "delete waiter", StatusCode: http.StatusNotFound}, + ) + }, + RefreshState: true, + // The failed create left the resource tainted, so the follow-up plan is a replacement. + ExpectNonEmptyPlan: true, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("stackit_sfs_resource_pool.resourcepool", "resource_pool_id", resourcePoolId), + resource.TestCheckResourceAttr("stackit_sfs_resource_pool.resourcepool", "timeouts.delete", deleteTimeout), + ), + }, + }, + }) +} + +// TestWaitHandlerTimeoutIsBoundedByContext pins the SDK behavior that makes the `timeouts` +// attribute effective at all: the wait handler applies its own timeout only when the passed +// context carries no deadline. Should a future SDK version enforce the handler timeout +// unconditionally, a `timeouts.create` above the handler default would silently be capped +// again, which is the bug the attribute was added for. +func TestWaitHandlerTimeoutIsBoundedByContext(t *testing.T) { + const ( + handlerTimeout = 50 * time.Millisecond + contextTimeout = time.Second + ) + + // The check never finishes, so only one of the two timeouts can end the wait. + handler := corewait.New(func() (bool, *struct{}, error) { return false, nil, nil }) + handler.SetTimeout(handlerTimeout).SetThrottle(10 * time.Millisecond) + + ctx, cancel := context.WithTimeout(context.Background(), contextTimeout) + defer cancel() + + start := time.Now() + if _, err := handler.WaitWithContext(ctx); err == nil { + t.Fatal("expected the wait to time out") + } + if elapsed := time.Since(start); elapsed < contextTimeout { + t.Errorf("wait gave up after %s, expected it to run until the context deadline at %s: "+ + "the SDK wait handler enforces its own timeout despite the context deadline, so the "+ + "`timeouts` attribute can no longer raise it", elapsed, contextTimeout) + } +} 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" + } } From 47412c908dfe4e1069749b7364c040f7cb1b9189 Mon Sep 17 00:00:00 2001 From: Jan Schreier Date: Thu, 10 Sep 2026 15:34:52 +0200 Subject: [PATCH 2/4] refactor(sfs): address review feedback on the timeouts attribute - shorten the create wait error to the timeout hint, and emit the same hint in update and delete so the three read consistently - drop the write of the timeouts attribute into the partial state - add the timeouts attribute to the resource pool data source as well - replace the create timeout test with the shorter form used for dns, on the existing MockServer - keep a blank line before the timeout blocks in Read and Update --- docs/data-sources/sfs_resource_pool.md | 9 + .../services/sfs/resourcepool/datasource.go | 13 +- .../services/sfs/resourcepool/resource.go | 42 ++-- stackit/internal/services/sfs/sfs_test.go | 218 ++---------------- 4 files changed, 59 insertions(+), 223 deletions(-) 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/stackit/internal/services/sfs/resourcepool/datasource.go b/stackit/internal/services/sfs/resourcepool/datasource.go index 65c2c89af..2fdb36f2f 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,14 @@ 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 +140,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 +227,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 15995dd73..4c7ae3c76 100644 --- a/stackit/internal/services/sfs/resourcepool/resource.go +++ b/stackit/internal/services/sfs/resourcepool/resource.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "strings" + "time" "github.com/hashicorp/terraform-plugin-framework-timeouts/resource/timeouts" "github.com/hashicorp/terraform-plugin-framework-validators/listvalidator" @@ -252,6 +253,16 @@ func (r *resourcePoolResource) Schema(ctx context.Context, _ resource.SchemaRequ } } +// timeoutHint names the configured timeout when this context's deadline is what ended a wait. The wait handler +// reports a timeout, a terminal error state and a failing poll through the same error, so on the other two the +// hint would point at the wrong cause. +func timeoutHint(ctx context.Context, operation string, timeout time.Duration) string { + if !errors.Is(ctx.Err(), context.DeadlineExceeded) { + return "" + } + return fmt.Sprintf(" The wait gave up after the configured `timeouts.%s` of %s; raise it if the operation regularly needs longer.", operation, timeout) +} + // Create creates the resource and sets the initial Terraform state. func (r *resourcePoolResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { // nolint:gocritic // function signature required by Terraform // Retrieve values from plan @@ -311,31 +322,12 @@ func (r *resourcePoolResource) Create(ctx context.Context, req resource.CreateRe if resp.Diagnostics.HasError() { return } - // The configured timeouts belong in that partial state as well. Without them a failed wait leaves an entry whose - // read and delete fall back to the defaults instead of the values the operator configured. - resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("timeouts"), model.Timeouts)...) - if resp.Diagnostics.HasError() { - return - } response, err := wait.CreateResourcePoolWaitHandler(ctx, r.client.DefaultAPI, projectId, region, *resourcePool.ResourcePool.Id). WaitWithContext(ctx) if err != nil { - // The wait handler reports a timeout, a terminal error state and a failing poll through the same error, so - // only mention the create timeout when this context's deadline is what ended the wait. - timeoutHint := "" - if errors.Is(ctx.Err(), context.DeadlineExceeded) { - timeoutHint = fmt.Sprintf(" The wait gave up after the configured `timeouts.create` of %s; raise it if the creation regularly needs longer.", createTimeout) - } - core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating resource pool", fmt.Sprintf( - "resource pool creation waiting: %v\n"+ - "The resource pool was created, and Terraform marks this resource as tainted, so the next apply replaces it. "+ - "Run `terraform untaint` on it first if the next run should refresh the existing pool instead. "+ - "If the state entry is lost, import the resource pool with the ID %q.%s", - err, - utils.BuildInternalTerraformId(projectId, region, *resourcePool.ResourcePool.Id).ValueString(), - timeoutHint, - )) + core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating resource pool", + fmt.Sprintf("resource pool creation waiting: %v%s", err, timeoutHint(ctx, "create", createTimeout))) return } ctx = tflog.SetField(ctx, "resource_pool_id", response.ResourcePool.Id) @@ -377,6 +369,7 @@ 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() { @@ -437,6 +430,7 @@ 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...) @@ -496,7 +490,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 creating resource pool", + fmt.Sprintf("resource pool get: %v%s", err, timeoutHint(ctx, "update", updateTimeout))) return } err = mapFields(ctx, region, getResponse.ResourcePool, &model) @@ -558,7 +553,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, timeoutHint(ctx, "delete", deleteTimeout))) return } diff --git a/stackit/internal/services/sfs/sfs_test.go b/stackit/internal/services/sfs/sfs_test.go index 8b5a0a635..149663c16 100644 --- a/stackit/internal/services/sfs/sfs_test.go +++ b/stackit/internal/services/sfs/sfs_test.go @@ -1,19 +1,14 @@ package sfs import ( - "context" - "encoding/json" "fmt" "net/http" - "net/http/httptest" "regexp" - "sync" "testing" "time" "github.com/google/uuid" "github.com/hashicorp/terraform-plugin-testing/helper/resource" - corewait "github.com/stackitcloud/stackit-sdk-go/core/wait" sfs "github.com/stackitcloud/stackit-sdk-go/services/sfs/v1api" "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/testutil" @@ -169,138 +164,16 @@ resource "stackit_sfs_share" "example" { }) } -// TestSfsResourcePoolCreateTimeoutIsConfigurable asserts that the configured `timeouts.create` value is what ends -// the create wait. The wait handler applies its own hardcoded 10 minutes otherwise, which no configuration reaches. -// -// Three signals are needed, because the provider reports every wait failure through the same message: the error must -// name the configured value (only the deadline branch does that), the pool must have been polled more than once, and -// the polling window must be close to the configured value rather than to zero or to the mock's poll budget. -func TestSfsResourcePoolCreateTimeoutIsConfigurable(t *testing.T) { +// 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() - resourcePoolId := uuid.NewString() - const ( - region = "eu01" - // Longer than the wait handler's 5s throttle, so several polls happen before the deadline ends the wait. - createTimeout = 12 * time.Second - // Safety valve. Without a context deadline in Create the wait would run for the handler's own 10 minutes and - // blow the package test timeout before any assertion could report the regression. - pollBudget = 30 * time.Second - poolState = "creating" - ) - - var ( - mu sync.Mutex - createdAt time.Time - lastPollAt time.Time - polls int - deleted bool - ) - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - mu.Lock() - defer mu.Unlock() - switch req.Method { - case http.MethodPost: - createdAt = time.Now() - writeJSON(t, w, sfs.CreateResourcePoolResponse{ - ResourcePool: &sfs.ResourcePool{Id: new(resourcePoolId)}, - }) - case http.MethodDelete: - deleted = true - w.WriteHeader(http.StatusAccepted) - default: - // Report the pool as gone once the test cleanup has deleted it, so the delete wait can finish. - if deleted { - w.WriteHeader(http.StatusNotFound) - return - } - polls++ - lastPollAt = time.Now() - if time.Since(createdAt) > pollBudget { - w.WriteHeader(http.StatusInternalServerError) - return - } - // The pool never becomes ready, so only a timeout can end the wait. - writeJSON(t, w, sfs.GetResourcePoolResponse{ - ResourcePool: &sfs.ResourcePool{Id: new(resourcePoolId), State: new(poolState)}, - }) - } - })) - defer server.Close() - - tfConfig := fmt.Sprintf(` -provider "stackit" { - default_region = "%s" - 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 = "%s" - } -} -`, region, server.URL, projectId, createTimeout) - - resource.UnitTest(t, resource.TestCase{ - ProtoV6ProviderFactories: testutil.TestAccProtoV6ProviderFactories, - Steps: []resource.TestStep{ - { - Config: tfConfig, - // Only the deadline branch of the error names the configured value. - ExpectError: regexp.MustCompile(regexp.QuoteMeta(createTimeout.String())), - }, - }, - }) - - mu.Lock() - defer mu.Unlock() - if polls < 2 { - t.Errorf("the create wait polled %d times, expected it to keep polling until the configured timeout of %s", - polls, createTimeout) - } - waited := lastPollAt.Sub(createdAt) - if waited < createTimeout/2 { - t.Errorf("the create wait ran for %s, expected roughly the configured %s: it failed for some other reason "+ - "before the timeout was reached", waited, createTimeout) - } - if waited > pollBudget-5*time.Second { - t.Errorf("the create wait ran for %s, expected it to give up after the configured %s: the configured value "+ - "is ignored and the wait ran into the mock's poll budget", waited, createTimeout) - } -} - -func writeJSON(t *testing.T, w http.ResponseWriter, body any) { - t.Helper() - w.Header().Set("content-type", "application/json") - if err := json.NewEncoder(w).Encode(body); err != nil { - t.Errorf("Error writing response body: %v", err) - } -} - -// TestSfsResourcePoolKeepsConfiguredTimeoutsOnError asserts that the configured timeouts are part of the partial -// state the resource writes before it starts waiting. They are needed there: after a failed create Terraform marks -// the resource tainted and the next run refreshes it and destroys it, and both operations read their timeout from -// that state entry. Were the attribute missing, those steps would silently fall back to the default timeouts. -func TestSfsResourcePoolKeepsConfiguredTimeoutsOnError(t *testing.T) { - projectId := uuid.NewString() - resourcePoolId := uuid.NewString() - const ( - region = "eu01" - deleteTimeout = "42m" - ) - s := testutil.NewMockServer(t) defer s.Server.Close() tfConfig := fmt.Sprintf(` provider "stackit" { - default_region = "%s" + default_region = "eu01" sfs_custom_endpoint = "%s" service_account_token = "mock-server-needs-no-auth" enable_beta_resources = true @@ -314,85 +187,32 @@ resource "stackit_sfs_resource_pool" "resourcepool" { ip_acl = ["192.168.2.0/24"] timeouts = { - delete = "%s" + create = "10ms" + read = "10ms" + update = "10ms" + delete = "10ms" } } -`, region, s.Server.URL, projectId, deleteTimeout) +`, s.Server.URL, projectId) resource.UnitTest(t, resource.TestCase{ ProtoV6ProviderFactories: testutil.TestAccProtoV6ProviderFactories, Steps: []resource.TestStep{ { PreConfig: func() { - s.Reset( - testutil.MockResponse{ - Description: "create resource pool", - ToJsonBody: sfs.CreateResourcePoolResponse{ - ResourcePool: &sfs.ResourcePool{Id: new(resourcePoolId)}, - }, - }, - testutil.MockResponse{ - Description: "failing waiter", - StatusCode: http.StatusInternalServerError, + 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("Error creating resource pool"), - }, - { - PreConfig: func() { - pool := testutil.MockResponse{ - Description: "read resource pool", - ToJsonBody: sfs.GetResourcePoolResponse{ - ResourcePool: &sfs.ResourcePool{Id: new(resourcePoolId)}, - }, - } - // The step refreshes and then plans, and both read the resource. - s.Reset( - pool, - pool, - testutil.MockResponse{Description: "delete", StatusCode: http.StatusAccepted}, - testutil.MockResponse{Description: "delete waiter", StatusCode: http.StatusNotFound}, - ) - }, - RefreshState: true, - // The failed create left the resource tainted, so the follow-up plan is a replacement. - ExpectNonEmptyPlan: true, - Check: resource.ComposeAggregateTestCheckFunc( - resource.TestCheckResourceAttr("stackit_sfs_resource_pool.resourcepool", "resource_pool_id", resourcePoolId), - resource.TestCheckResourceAttr("stackit_sfs_resource_pool.resourcepool", "timeouts.delete", deleteTimeout), - ), + ExpectError: regexp.MustCompile("deadline exceeded"), }, }, }) } - -// TestWaitHandlerTimeoutIsBoundedByContext pins the SDK behavior that makes the `timeouts` -// attribute effective at all: the wait handler applies its own timeout only when the passed -// context carries no deadline. Should a future SDK version enforce the handler timeout -// unconditionally, a `timeouts.create` above the handler default would silently be capped -// again, which is the bug the attribute was added for. -func TestWaitHandlerTimeoutIsBoundedByContext(t *testing.T) { - const ( - handlerTimeout = 50 * time.Millisecond - contextTimeout = time.Second - ) - - // The check never finishes, so only one of the two timeouts can end the wait. - handler := corewait.New(func() (bool, *struct{}, error) { return false, nil, nil }) - handler.SetTimeout(handlerTimeout).SetThrottle(10 * time.Millisecond) - - ctx, cancel := context.WithTimeout(context.Background(), contextTimeout) - defer cancel() - - start := time.Now() - if _, err := handler.WaitWithContext(ctx); err == nil { - t.Fatal("expected the wait to time out") - } - if elapsed := time.Since(start); elapsed < contextTimeout { - t.Errorf("wait gave up after %s, expected it to run until the context deadline at %s: "+ - "the SDK wait handler enforces its own timeout despite the context deadline, so the "+ - "`timeouts` attribute can no longer raise it", elapsed, contextTimeout) - } -} From 7f8825e336829d1f77b7a7566ff58dae67a9b9a6 Mon Sep 17 00:00:00 2001 From: Jan Schreier Date: Thu, 10 Sep 2026 15:51:50 +0200 Subject: [PATCH 3/4] fix(sfs): correct the update wait diagnostic and follow through on the review The update wait branch kept the summary "Error creating resource pool" while the appended hint named `timeouts.update`, so the diagnostic contradicted itself. Summary and detail now say "updating", matching what #1741 changes the same line to. The hint starts on its own line again; concatenating it directly onto the wrapped error ran the two sentences together. Two follow-ups in the spirit of the review rather than its letter: the data source read timeout gets the same blank line that was asked for in the resource, and the acceptance-test data source declares a timeouts block, as the dns testdata does for its data source. --- stackit/internal/services/sfs/resourcepool/datasource.go | 1 + stackit/internal/services/sfs/resourcepool/resource.go | 6 +++--- stackit/internal/services/sfs/sfs_acc_test.go | 4 ++++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/stackit/internal/services/sfs/resourcepool/datasource.go b/stackit/internal/services/sfs/resourcepool/datasource.go index 2fdb36f2f..787d4175a 100644 --- a/stackit/internal/services/sfs/resourcepool/datasource.go +++ b/stackit/internal/services/sfs/resourcepool/datasource.go @@ -92,6 +92,7 @@ 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() { diff --git a/stackit/internal/services/sfs/resourcepool/resource.go b/stackit/internal/services/sfs/resourcepool/resource.go index 4c7ae3c76..7f1ac66f9 100644 --- a/stackit/internal/services/sfs/resourcepool/resource.go +++ b/stackit/internal/services/sfs/resourcepool/resource.go @@ -260,7 +260,7 @@ func timeoutHint(ctx context.Context, operation string, timeout time.Duration) s if !errors.Is(ctx.Err(), context.DeadlineExceeded) { return "" } - return fmt.Sprintf(" The wait gave up after the configured `timeouts.%s` of %s; raise it if the operation regularly needs longer.", operation, timeout) + return fmt.Sprintf("\nThe wait gave up after the configured `timeouts.%s` of %s; raise it if the operation regularly needs longer.", operation, timeout) } // Create creates the resource and sets the initial Terraform state. @@ -490,8 +490,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%s", err, timeoutHint(ctx, "update", updateTimeout))) + core.LogAndAddError(ctx, &resp.Diagnostics, "Error updating resource pool", + fmt.Sprintf("resource pool update waiting: %v%s", err, timeoutHint(ctx, "update", updateTimeout))) return } err = mapFields(ctx, region, getResponse.ResourcePool, &model) 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, From 06224cef9779602ce85048e57458152492209ef4 Mon Sep 17 00:00:00 2001 From: Jan Schreier Date: Fri, 11 Sep 2026 17:02:10 +0200 Subject: [PATCH 4/4] refactor(sfs): move the timeout hint to the utils package The hint that names the configured timeout after a wait ran out is not specific to SFS, so it now lives in stackit/internal/utils as TimeoutHint where other resources and data sources can use it. The move adds a unit test that pins the exact text, including the leading newline, and checks that a cancellation or a wait that failed before the deadline produce no hint. --- .../services/sfs/resourcepool/resource.go | 17 ++----- stackit/internal/utils/timeouts.go | 19 +++++++ stackit/internal/utils/timeouts_test.go | 51 +++++++++++++++++++ 3 files changed, 73 insertions(+), 14 deletions(-) create mode 100644 stackit/internal/utils/timeouts.go create mode 100644 stackit/internal/utils/timeouts_test.go diff --git a/stackit/internal/services/sfs/resourcepool/resource.go b/stackit/internal/services/sfs/resourcepool/resource.go index 7f1ac66f9..e2bf1f886 100644 --- a/stackit/internal/services/sfs/resourcepool/resource.go +++ b/stackit/internal/services/sfs/resourcepool/resource.go @@ -7,7 +7,6 @@ import ( "fmt" "net/http" "strings" - "time" "github.com/hashicorp/terraform-plugin-framework-timeouts/resource/timeouts" "github.com/hashicorp/terraform-plugin-framework-validators/listvalidator" @@ -253,16 +252,6 @@ func (r *resourcePoolResource) Schema(ctx context.Context, _ resource.SchemaRequ } } -// timeoutHint names the configured timeout when this context's deadline is what ended a wait. The wait handler -// reports a timeout, a terminal error state and a failing poll through the same error, so on the other two the -// hint would point at the wrong cause. -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) -} - // Create creates the resource and sets the initial Terraform state. func (r *resourcePoolResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { // nolint:gocritic // function signature required by Terraform // Retrieve values from plan @@ -327,7 +316,7 @@ func (r *resourcePoolResource) Create(ctx context.Context, req resource.CreateRe WaitWithContext(ctx) if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating resource pool", - fmt.Sprintf("resource pool creation waiting: %v%s", err, timeoutHint(ctx, "create", createTimeout))) + 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) @@ -491,7 +480,7 @@ 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 updating resource pool", - fmt.Sprintf("resource pool update waiting: %v%s", err, timeoutHint(ctx, "update", updateTimeout))) + fmt.Sprintf("resource pool update waiting: %v%s", err, utils.TimeoutHint(ctx, "update", updateTimeout))) return } err = mapFields(ctx, region, getResponse.ResourcePool, &model) @@ -554,7 +543,7 @@ func (r *resourcePoolResource) Delete(ctx context.Context, req resource.DeleteRe _, 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%s", err, timeoutHint(ctx, "delete", deleteTimeout))) + fmt.Sprintf("resource pool deletion waiting: %v%s", err, utils.TimeoutHint(ctx, "delete", deleteTimeout))) return } 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) + } + }) + } +}