Skip to content

Commit 27766c4

Browse files
authored
Merge pull request #33531 from hashicorp/f-sdk-go-v2-retriers
S3 bucket resources retry delete on `conflicting conditional operation` errors
2 parents 715698e + fdab6be commit 27766c4

File tree

5 files changed

+106
-32
lines changed

5 files changed

+106
-32
lines changed

.changelog/33531.txt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
```release-note:bug
2+
resource/aws_s3_bucket_policy: Retry resource Delete on `OperationAborted: A conflicting conditional operation is currently in progress against this resource` errors
3+
```
4+
5+
```release-note:bug
6+
resource/aws_s3_bucket_versioning: Retry resource Delete on `OperationAborted: A conflicting conditional operation is currently in progress against this resource` errors
7+
```
8+
9+
```release-note:bug
10+
resource/aws_s3_bucket_accelerate_configuration: Retry resource Delete on `OperationAborted: A conflicting conditional operation is currently in progress against this resource` errors
11+
```

internal/conns/apiretry.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright (c) HashiCorp, Inc.
2+
// SPDX-License-Identifier: MPL-2.0
3+
4+
package conns
5+
6+
import (
7+
"github.com/aws/aws-sdk-go-v2/aws"
8+
"github.com/aws/aws-sdk-go-v2/aws/retry"
9+
)
10+
11+
// AddIsErrorRetryables returns a Retryer which runs the specified retryables on any error.
12+
func AddIsErrorRetryables(r aws.RetryerV2, retryables ...retry.IsErrorRetryable) aws.RetryerV2 {
13+
return &withIsErrorRetryables{
14+
RetryerV2: r,
15+
retryables: retryables,
16+
}
17+
}
18+
19+
type withIsErrorRetryables struct {
20+
aws.RetryerV2
21+
retryables retry.IsErrorRetryables
22+
}
23+
24+
func (r *withIsErrorRetryables) IsErrorRetryable(err error) bool {
25+
if v := r.retryables.IsErrorRetryable(err); v != aws.UnknownTernary {
26+
return v.Bool()
27+
}
28+
return r.RetryerV2.IsErrorRetryable(err)
29+
}

internal/conns/apiretry_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// Copyright (c) HashiCorp, Inc.
2+
// SPDX-License-Identifier: MPL-2.0
3+
4+
package conns
5+
6+
import (
7+
"errors"
8+
"testing"
9+
10+
"github.com/aws/aws-sdk-go-v2/aws"
11+
"github.com/aws/aws-sdk-go-v2/aws/retry"
12+
"github.com/hashicorp/terraform-provider-aws/internal/errs"
13+
)
14+
15+
func TestAddIsErrorRetryables(t *testing.T) {
16+
t.Parallel()
17+
18+
f := func(err error) aws.Ternary {
19+
if errs.Contains(err, "testing") {
20+
return aws.TrueTernary
21+
}
22+
return aws.UnknownTernary
23+
}
24+
testCases := []struct {
25+
name string
26+
err error
27+
expected bool
28+
}{
29+
{
30+
name: "no error",
31+
},
32+
{
33+
name: "non-retryable",
34+
err: errors.New(`this is not retryable`),
35+
},
36+
{
37+
name: "retryable",
38+
err: errors.New(`this is testing`),
39+
expected: true,
40+
},
41+
}
42+
43+
for _, testCase := range testCases {
44+
testCase := testCase
45+
t.Run(testCase.name, func(t *testing.T) {
46+
t.Parallel()
47+
48+
got := AddIsErrorRetryables(retry.NewStandard(), retry.IsErrorRetryableFunc(f)).IsErrorRetryable(testCase.err)
49+
if got, want := got, testCase.expected; got != want {
50+
t.Errorf("IsErrorRetryable(%q) = %v, want %v", testCase.err, got, want)
51+
}
52+
})
53+
}
54+
}

internal/service/s3/bucket_object_data_source_test.go

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -363,23 +363,6 @@ func TestAccS3BucketObjectDataSource_multipleSlashes(t *testing.T) {
363363
})
364364
}
365365

366-
func TestAccS3BucketObjectDataSource_singleSlashAsKey(t *testing.T) {
367-
ctx := acctest.Context(t)
368-
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
369-
370-
resource.ParallelTest(t, resource.TestCase{
371-
PreCheck: func() { acctest.PreCheck(ctx, t) },
372-
ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID),
373-
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
374-
PreventPostDestroyRefresh: true,
375-
Steps: []resource.TestStep{
376-
{
377-
Config: testAccBucketObjectDataSourceConfig_singleSlashAsKey(rName),
378-
},
379-
},
380-
})
381-
}
382-
383366
func testAccBucketObjectDataSourceConfig_basic(randInt int) string {
384367
return fmt.Sprintf(`
385368
resource "aws_s3_bucket" "object_bucket" {
@@ -685,16 +668,3 @@ data "aws_s3_object" "obj3" {
685668

686669
return resources, both
687670
}
688-
689-
func testAccBucketObjectDataSourceConfig_singleSlashAsKey(rName string) string {
690-
return fmt.Sprintf(`
691-
resource "aws_s3_bucket" "test" {
692-
bucket = %[1]q
693-
}
694-
695-
data "aws_s3_object" "test" {
696-
bucket = aws_s3_bucket.test.bucket
697-
key = "/"
698-
}
699-
`, rName)
700-
}

internal/service/s3/service_package.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,16 @@ import (
77
"context"
88

99
aws_sdkv2 "github.com/aws/aws-sdk-go-v2/aws"
10+
retry_sdkv2 "github.com/aws/aws-sdk-go-v2/aws/retry"
1011
s3_sdkv2 "github.com/aws/aws-sdk-go-v2/service/s3"
1112
aws_sdkv1 "github.com/aws/aws-sdk-go/aws"
1213
endpoints_sdkv1 "github.com/aws/aws-sdk-go/aws/endpoints"
1314
request_sdkv1 "github.com/aws/aws-sdk-go/aws/request"
1415
session_sdkv1 "github.com/aws/aws-sdk-go/aws/session"
1516
s3_sdkv1 "github.com/aws/aws-sdk-go/service/s3"
16-
"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
17+
tfawserr_sdkv1 "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
18+
tfawserr_sdkv2 "github.com/hashicorp/aws-sdk-go-base/v2/tfawserr"
19+
"github.com/hashicorp/terraform-provider-aws/internal/conns"
1720
)
1821

1922
// NewConn returns a new AWS SDK for Go v1 client for this service package's AWS API.
@@ -34,7 +37,7 @@ func (p *servicePackage) NewConn(ctx context.Context, m map[string]any) (*s3_sdk
3437
// CustomizeConn customizes a new AWS SDK for Go v1 client for this service package's AWS API.
3538
func (p *servicePackage) CustomizeConn(ctx context.Context, conn *s3_sdkv1.S3) (*s3_sdkv1.S3, error) {
3639
conn.Handlers.Retry.PushBack(func(r *request_sdkv1.Request) {
37-
if tfawserr.ErrMessageContains(r.Error, errCodeOperationAborted, "A conflicting conditional operation is currently in progress against this resource. Please try again.") {
40+
if tfawserr_sdkv1.ErrMessageContains(r.Error, errCodeOperationAborted, "A conflicting conditional operation is currently in progress against this resource. Please try again.") {
3841
r.Retryable = aws_sdkv1.Bool(true)
3942
}
4043
})
@@ -55,5 +58,12 @@ func (p *servicePackage) NewClient(ctx context.Context, config map[string]any) (
5558
o.Region = "aws-global"
5659
}
5760
o.UsePathStyle = config["s3_use_path_style"].(bool)
61+
62+
o.Retryer = conns.AddIsErrorRetryables(cfg.Retryer().(aws_sdkv2.RetryerV2), retry_sdkv2.IsErrorRetryableFunc(func(err error) aws_sdkv2.Ternary {
63+
if tfawserr_sdkv2.ErrMessageContains(err, errCodeOperationAborted, "A conflicting conditional operation is currently in progress against this resource. Please try again.") {
64+
return aws_sdkv2.TrueTernary
65+
}
66+
return aws_sdkv2.UnknownTernary // Delegate to configured Retryer.
67+
}))
5868
}), nil
5969
}

0 commit comments

Comments
 (0)