fix: Add retry logic for transient disk creation errors#2389
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new global Linode API retry condition to automatically retry transient instance disk creation failures that occur while an instance is still provisioning (manifesting as either HTTP 400 “Linode busy.” or connection-level EOF errors).
Changes:
- Added
InstanceDiskCreateBusyRetry()retry condition scoped to thelinode/instances/{id}/disksendpoint. - Registered the new retry condition in
ApplyAllRetryConditions()so it applies across the provider. - Added unit tests covering “Linode busy.” and EOF retry behavior and endpoint scoping.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
linode/helper/retry.go |
Adds and registers a new retry condition for transient disk-creation busy/EOF failures. |
linode/helper/retry_test.go |
Introduces unit tests validating the new retry condition behavior and scoping. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Change test URLs from 'https://api.linode.com/v4/...' to '/v4/...' to match what resty.Request.URL contains in production. This allows url.ParseRequestURI to work correctly in the retry condition. Addresses Copilot reviewer feedback on PR linode#2389.
## Problem
Disk creation frequently fails in development environments with two
manifestations of the same underlying issue:
1. HTTP 400 with "Linode busy." error when attempting to create a disk
immediately after instance creation
2. EOF errors (e.g., "[002] unexpected EOF") when the API connection
is reset during the busy state
Both errors indicate the instance is still provisioning and not yet
ready to accept disk operations. These are transient infrastructure
state issues that should be automatically retried rather than failing
the Terraform operation.
## Root Cause
When a Linode instance is in a provisioning state, the API backend may:
- Return a clean 400 error with "Linode busy." message, or
- Experience connection resets/timeouts, manifesting as EOF errors
The same condition produces different error types depending on backend
load and timing. The provider previously did not retry either error type
for disk creation operations.
## Solution
Added `InstanceDiskCreateBusyRetry()` to the global retry conditions in
`linode/helper/retry.go`. This function:
- Retries HTTP 400 responses containing "Linode busy." on disk creation
- Retries EOF errors (including "[002] unexpected EOF") on disk creation
- Only applies to the disk creation endpoint pattern:
`linode/instances/{id}/disks`
- Uses response body checking for 400s to avoid retrying validation errors
- Verifies request URL for EOF errors to scope retries appropriately
The retry is applied globally via the existing `ApplyAllRetryConditions()`
mechanism, similar to how 500 errors are handled for other resources.
## Implementation Details
The retry condition function checks both error types:
1. For EOF errors: Verifies the error message contains "EOF" and the
request URL matches the disk creation endpoint pattern
2. For "Linode busy." errors: Checks for HTTP 400 status, verifies the
endpoint pattern, and searches the response body for the specific
error message
This dual approach handles the same transient state issue regardless of
how it manifests at the HTTP layer.
## Testing
- Added comprehensive unit tests in `linode/helper/retry_test.go`:
- Validates retry on "Linode busy." 400 responses
- Validates retry on EOF errors with various formats
- Ensures retries only occur on disk creation endpoints
- Verifies no retries on other endpoints or error types
- All existing unit tests pass
- Manual testing confirms retries work in development environments
## Related Patterns
This follows the existing pattern used for LKE cluster operations, which
also retry EOF errors during provisioning:
```go
// Sometimes the K8S API will raise an EOF error if polling immediately after
// a cluster is created. We should retry accordingly.
```
Fixes intermittent disk creation failures in development ecosystems where
instances are frequently created and modified in rapid succession.
Change test URLs from 'https://api.linode.com/v4/...' to '/v4/...' to match what resty.Request.URL contains in production. This allows url.ParseRequestURI to work correctly in the retry condition. Addresses Copilot reviewer feedback on PR linode#2389.
20a19a4 to
ab6b4fa
Compare
yec-akamai
left a comment
There was a problem hiding this comment.
Works on my end, just minor things:
Replace standard library log.Printf calls with terraform-plugin-log's
tflog functions in InstanceDiskCreateBusyRetry() to provide structured
logging consistent with Terraform provider best practices.
Changes:
- log.Printf("[DEBUG]") -> tflog.Debug() with structured fields
- log.Printf("[WARN]") -> tflog.Warn() with structured fields
- Use response.Request.Context() to access request context
This provides proper context propagation for logging while maintaining
compatibility with the retry condition callback signature.
7645306 to
8161e00
Compare
lgarber-akamai
left a comment
There was a problem hiding this comment.
Seems sensible to me, should be good after merge conflicts are resolved. Thanks for the contribution!
Resolved conflict in retry.go by adapting InstanceDiskCreateBusyRetry() to use http.Response (linodego v2) instead of resty.Response (linodego v1).
Improves retry handling for transient disk creation failures that occur when creating disks on newly-provisioned Linode instances. These errors manifest as [002] error codes from linodego, which indicate network or transport-level failures during response decoding. This is a refactor of the original implementation (aligned to linodego v1) to work with linodego v2's http.Response API instead of resty.Response. Changes: - Check for [002] error code prefix instead of just "EOF" string - Catches both "unexpected EOF" and "failed to decode response body" - Aligns with linodego's error categorization system - Extract request context for better log correlation with trace IDs - Uses response.Request.Context() when available - Falls back to context.Background() for safety - Add comprehensive structured logging with tflog.Debug - Logs all retry decisions with relevant context - Includes request paths and error details for debugging - Adapt to http.Response API (linodego v2 migration) - response.StatusCode field instead of method - io.ReadAll() for body reading with restoration - Direct access to response.Request.URL.Path - Update all unit tests to reflect [002] error code checking - Add test case for "failed to decode response body" variant - Add test case for plain "EOF" without [002] (should not retry) This change maintains backward compatibility while providing more robust error detection and significantly better observability for debugging transient provisioning issues.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Adds stricter validation to InstanceDiskCreateBusyRetry to prevent false positives and improve reliability: Implementation changes: - Restrict retries to POST requests only (disk creation) - Prevents retrying on GET/DELETE/etc operations - Narrow [002] error matching to EOF-related errors only - Must contain "EOF" or "failed to decode response body" - Excludes unrelated [002] errors like "connection reset" - Add nil body validation before attempting to read response body - Remove response body content from debug logs (keep decision flags) Test coverage enhancements: - Add test for non-POST method with "Linode busy." error - Add test for nil response body - Add test for [002] errors without EOF/decode keywords - Add test for [002] EOF errors with non-POST method - Update all existing tests to explicitly specify HTTP method These changes ensure the retry logic only triggers for legitimate disk creation failures during instance provisioning, avoiding unnecessary retries on unrelated operations or error conditions.
Enhances InstanceDiskCreateBusyRetry() to handle transient connection errors that occur when creating disks on newly provisioned instances. Key changes: - Check for [002] error code prefix (not just "EOF" string) to match both "[002] unexpected EOF" and "[002] failed to decode response body" - Add HTTP method validation to only retry POST requests - Add nil body guard to prevent panics when response body is missing - Narrow [002] matching to EOF-related errors only (EOF or decode failure) - Extract request context from response for trace correlation in logs - Replace log.Printf with structured tflog.Debug/Warn calls - Add conservative documentation reflecting observed error patterns The [002] error code is observed during instance provisioning when the connection is reset while reading the HTTP response body. This typically occurs when attempting to create a disk immediately after instance creation while the instance is still initializing. Testing: - All 16 unit tests pass (including 4 new test cases) - Validates [002] EOF errors, decode errors, method checks, and nil guards - Tested successfully in development environment (5/5 success rate) Related to addressing intermittent "Linode busy." errors during automated instance provisioning workflows.
…imizations Enhances the instance disk creation retry logic with better observability, stricter validation, and performance improvements while maintaining the existing retry behavior for [002] network errors and "Linode busy." responses. Changes: - Optimize err.Error() calls: Cache result once as errStr, reuse 6 times - Add comprehensive tflog.Debug() statements with standardized format: - "Detected [002] network error" for initial detection - "<condition>, retrying" or "<condition>, not retrying" for decisions - "Evaluated 400 response body for 'Linode busy.' message" for evaluation - Add POST method validation for [002] network errors (disk creation is POST-only) - Add path logging in all decision branches for better debugging - Fix resource leak: Use unconditional body close with dedicated readErr variable - Fix body restoration: Always restore response.Body before checking readErr to ensure downstream error handlers can read the body - Use regexp.MustCompile instead of regexp.Compile + log.Fatal (matches codebase pattern) - Standardize all log messages with "InstanceDiskCreateBusyRetry:" prefix - Restructure error handling with nested blocks for clearer control flow The enhanced logging provides request context correlation via response.Request.Context() for better trace ID tracking across retry attempts. All changes preserve the existing retry logic while improving debuggability in production environments. Validated with: - All 16 unit tests passing (//go:build unit) - Production testing showing proper [002] error detection and retry
0666bb3 to
28bdc61
Compare
lgarber-akamai
left a comment
There was a problem hiding this comment.
Seems pretty stable in my local testing. Thanks again for the contribution!
yec-akamai
left a comment
There was a problem hiding this comment.
Thank you for the work!
Problem
Disk creation can fail in development environments with two manifestations of the same underlying issue:
Both errors indicate the instance is still provisioning and not yet ready to accept disk operations. These are transient infrastructure state issues that should be automatically retried rather than failing the Terraform operation.
Root Cause
When a Linode instance is in a provisioning state, the API backend may:
• Return a clean 400 error with "Linode busy." message, or
• Experience connection resets/timeouts, manifesting as [002]-prefixed network errors
The same condition produces different error types depending on backend load and timing. The provider previously did not retry either error type for disk creation operations.
Solution
Added
InstanceDiskCreateBusyRetry()to the global retry conditions inlinode/helper/retry.go. This function:• Retries HTTP 400 responses containing "Linode busy." on disk creation
• Retries [002] network errors (including "[002] unexpected EOF" and "[002] failed to decode response body") on disk creation
• Only applies to the disk creation endpoint pattern:
linode/instances/{id}/disks• Uses response body checking for 400s to avoid retrying validation errors
• Verifies request URL and HTTP method for [002] errors to scope retries appropriately
Note: Only errors with the
[002]error code prefix are retried. Plain EOF errors without[002]are not retried as they may indicate different failure modes unrelated to instance provisioning state.The retry is applied globally via the existing
ApplyAllRetryConditions()mechanism, similar to how 500 errors are handled for other resources.Implementation Details
The retry condition function checks both error types:
This dual approach handles the same transient state issue regardless of how it manifests at the HTTP layer.
Testing
• Added comprehensive unit tests in
linode/helper/retry_test.go:◦ Validates retry on "Linode busy." 400 responses
◦ Validates retry on [002] network errors with various formats
◦ Ensures retries only occur on disk creation endpoints
◦ Verifies no retries on other endpoints or error types
• All existing unit tests pass
• Manual testing confirms retries work in development environments
Related Patterns
This follows the existing pattern used for LKE cluster operations, which also retry EOF errors during provisioning. The LKE code has a comment noting: "Sometimes the K8S API will raise an EOF error if polling immediately after a cluster is created. We should retry accordingly."
Fixes intermittent disk creation failures in development ecosystems where instances are frequently created and modified in rapid succession.