feat(sfs): make resource pool wait timeouts configurable - #1740
Open
devpie wants to merge 1 commit into
Open
Conversation
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. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
devpie
force-pushed
the
fix/sfs-resource-pool-timeouts
branch
from
September 4, 2026 13:12
1c1872e to
09296ec
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
relates to #1737
stackit_sfs_resource_poolcannot create a pool that STACKIT needs more than 10 minutes toprovision.
CreateResourcePoolWaitHandlerand its update/delete counterparts setSetTimeout(10 * time.Minute), and the resource passed a context without a deadline, so thatdefault was the only limit and no configuration could reach it.
This adds a
timeoutsattribute (create/read/update/delete) following the pattern ofdremio/instance.Why a context deadline and not
SetTimeoutcore/wait.WaitWithContextapplies the handler's own timeout only when the incoming contextcarries no deadline:
(core v0.26.0, wait.go)
Setting a context deadline per CRUD method therefore replaces the hardcoded value, and no
SetTimeoutcall is needed.TestWaitHandlerTimeoutIsBoundedByContextpins that behaviour, so anSDK bump that enforces the handler timeout unconditionally fails the build instead of silently
capping the configured value again. Defaults stay at the wait handler value plus
core.DefaultTimeoutMargin, so unconfigured resources keep their current behaviour.Two things beyond the plain attribute
The configured timeouts are written to state together with the IDs, before the create wait
starts.
SetAndLogStateFieldswrites three attributes and the framework fills the rest withnull, so
timeoutswould be null in the entry a failed wait leaves behind. Refresh and destroy ofthat entry would then fall back to the defaults — on exactly the recovery path those values are
needed for.
TestSfsResourcePoolKeepsConfiguredTimeoutsOnErrorcovers it.The wait error is more specific. It now says that Terraform marks the resource tainted and
replaces it on the next run, points at
terraform untaintand the import ID, and mentionstimeouts.createonly when this context's deadline is what ended the wait. The wait handlerreports terminal error states and failing polls through the same error; telling the user to raise
a timeout in those cases would be wrong — the repo's own
TestSfsResourcePoolSavesIDsOnErrordrives the failing-poll case.
On point 2 of the issue
The issue also asks to keep the resource in state when the wait times out. That is already
implemented:
utils.SetAndLogStateFieldswrites the IDs before the wait (CONTRIBUTING.md statesthis as project doctrine, and
TestSfsResourcePoolSavesIDsOnErrorcovers it), the frameworkinitialises the create response state to a null object so no unknowns leak, and Terraform keeps the
object and marks it tainted rather than discarding it. What follows is a replace, not a lost entry.
No provider change was needed for that, only the corrected wording of the error.
No
ImportStateVerifyIgnoreis needed for that: terraform-plugin-testing deletestimeoutsandtimeouts.*fromboth sides of the comparison unconditionally, after the ignore loop
(testing_new_import_state.go:398-411, v1.16.0).
The DNS acceptance tests rely on the same behaviour.
Not changed
sfs/sharehas the same hardcoded 10 minutes in all three of its wait handlers. Out of scope forthis issue — happy to follow up if wanted.
Checklist
make fmtexamples/directory) — deliberately not: no example inexamples/has ever carried atimeoutsblock (git log -S timeouts -- examples/is empty), and feat(dns) add timeouts to dns resources and datasources #1345, which addedtimeoutsto the DNS resources, put the demonstrable configuration intestdata/resource-max.tfinstead. Hardcoding durations on a registry page would also pin numbers that go stale when the SDK waiter default moves.make generate-docs(will be checked by CI)testdata/resource-pool-max.tfnow setstimeouts, mirroringdns/testdata/resource-max.tf, soTestAccResourcePoolResourceMaxcovers the attribute across create, import-verify and updatemake test(will be checked by CI)make lint(will be checked by CI)🤖 Generated with Claude Code