Skip to content
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.

Sign up for GitHub

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

Jump to bottom

docs/contributing: Initial Retries and Waiters documentation #17508

Merged
merged 6 commits into from
Feb 10, 2021
Merged

docs/contributing: Initial Retries and Waiters documentation #17508

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a560575
docs/contributing: Initial Retries and Waiters documentation
bflad Feb 8, 2021
6d0f4d3
docs/contributing: Fix typo
bflad Feb 8, 2021
ebaa745
docs/contributing: Remove Retries and Waiters placeholders in various…
bflad Feb 8, 2021
3fa5bdb
docs/contributing: Fix tfawserr function reference
bflad Feb 8, 2021
0db37c4
Apply suggestions from code review
bflad Feb 10, 2021
8c906e2
Update docs/contributing/retries-and-waiters.md
bflad Feb 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 15 additions & 18 deletions docs/contributing/retries-and-waiters.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ The AWS Go SDK middleware provides hooks for injecting custom logic in the servi

```go
client.kafkaconn.Handlers.Retry.PushBack(func(r *request.Request) {
if tfaws.ErrorMessageContains(r.Error, kafka.ErrCodeTooManyRequestsException, "Too Many Requests") {
if tfawserr.ErrMessageContains(r.Error, kafka.ErrCodeTooManyRequestsException, "Too Many Requests") {
r.Retryable = aws.Bool(true)
}
})
Expand All @@ -102,12 +102,11 @@ Given a properly ordered Terraform configuration, eventual consistency can unexp
This type of logic should not be implemented to overcome improperly ordered Terraform configurations as it is not guaranteed to work in larger environments.
bflad marked this conversation as resolved.
Show resolved Hide resolved

```go
// aws/internal/service/{SERVICE}/waiter/consts.go (created if does not exist)
package {SERVICE}
// aws/internal/service/example/waiter/waiter.go (created if does not exist)

const (
// Maximum amount of time to wait for Example eventual consistency
ExampleTimeout = 2 * time.Minute
// Maximum amount of time to wait for Thing operation eventual consistency
ThingOperationTimeout = 2 * time.Minute
)
```

Expand All @@ -116,11 +115,11 @@ const (

import (
// ... other imports ...
"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/{SERVICE}/waiter"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/example/waiter"
)

// ... Create, Read, Update, or Delete function ...
err := resource.Retry(waiter.ExampleTimeout, func() *resource.RetryError {
err := resource.Retry(waiter.ThingOperationTimeout, func() *resource.RetryError {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All this example code is beautiful. 😍

_, err := conn./* ... AWS Go SDK operation with eventual consistency errors ... */

// Retryable conditions which can be checked.
Expand Down Expand Up @@ -172,7 +171,7 @@ Each AWS service API (and sometimes even operations within the same API) varies

import (
// ... other imports ...
// By convention, cross-service waiter imports are named {SERVICE}waiter
// By convention, cross-service waiter imports are aliased as {SERVICE}waiter
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nooice

iamwaiter "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/iam/waiter"
)

Expand Down Expand Up @@ -209,13 +208,11 @@ When the eventual consistency lies with the existence or configuration state of
When encountering this type of situation, every operation after the initial operation must account for the condition, which can be tricky depending on the resource logic itself. Most resources should be written where the `Create` function will return the `Read` function to fill in all computed attributes and ensure the configuration was applied correctly. In these cases, the `Read` function will need additional logic to overcome the temporary condition on resource creation. For eventually consistent resources, these "not found" type errors can still occur in the `Read` function even after implementing [Resource Lifecycle Waiters](#resource-lifecycle-waiters) in the Create function for the resource itself.
bflad marked this conversation as resolved.
Show resolved Hide resolved

```go
// aws/internal/service/{SERVICE}/waiter/consts.go (created if does not exist)

package {SERVICE}
// aws/internal/service/example/waiter/waiter.go (created if does not exist)

const (
// Maximum amount of time to wait for Example eventual consistency on creation
ExampleCreationTimeout = 2 * time.Minute
// Maximum amount of time to wait for Thing eventual consistency on creation
ThingCreationTimeout = 2 * time.Minute
)
```

Expand All @@ -233,7 +230,7 @@ function ExampleThingRead(d *schema.ResourceData, meta interface{}) error {
input := &example.OperationInput{/* ... */}

var output *example.OperationOutput
err := resource.Retry(waiter.ExampleCreationTimeout, func() *resource.RetryError {
err := resource.Retry(waiter.ThingCreationTimeout, func() *resource.RetryError {
var err error
output, err = conn.Operation(input)

Expand Down Expand Up @@ -288,7 +285,7 @@ In rare cases, it may be easier to duplicate all `Read` function logic into the
An emergent solution for handling eventual consistency with attribute values on updates is to introduce a custom `resource.StateChangeConf` and `resource.RefreshStateFunc` handlers. For example:

```go
// aws/internal/service/{SERVICE}/waiter/status.go (created if does not exist)
// aws/internal/service/example/waiter/status.go (created if does not exist)

// ThingAttribute fetches the Thing and its Attribute
func ThingAttribute(conn *example.Example, id string) resource.StateRefreshFunc {
Expand All @@ -313,7 +310,7 @@ func ThingAttribute(conn *example.Example, id string) resource.StateRefreshFunc
```

```go
// aws/internal/service/{SERVICE}/waiter/waiter.go (created if does not exist)
// aws/internal/service/example/waiter/waiter.go (created if does not exist)

const (
ThingAttributePropagationTimeout = 2 * time.Minute
Expand Down Expand Up @@ -378,7 +375,7 @@ If it is necessary to customize the timeouts and polling with these, it is gener
Most of the codebase uses `resource.StateChangeConf` and `resource.RefreshStateFunc` handlers for tracking either component level status fields or explicit tracking identifiers. These should be placed in the `aws/internal/service/{SERVICE}/waiter` package and split into separate functions. For example:

```go
// aws/internal/service/{SERVICE}/waiter/status.go (created if does not exist)
// aws/internal/service/example/waiter/status.go (created if does not exist)

// ThingStatus fetches the Thing and its Status
func ThingStatus(conn *example.Example, id string) resource.StateRefreshFunc {
Expand All @@ -403,7 +400,7 @@ func ThingStatus(conn *example.Example, id string) resource.StateRefreshFunc {
```

```go
// aws/internal/service/{SERVICE}/waiter/waiter.go (created if does not exist)
// aws/internal/service/example/waiter/waiter.go (created if does not exist)

const (
ThingCreationTimeout = 2 * time.Minute
Expand Down