From 73efd1f26b68049ff58b5b9ae2f93027a08f717a Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Tue, 5 Mar 2024 15:20:06 -0500 Subject: [PATCH 1/7] elasticache/replication_group: Fix available state before modifying tags --- .../service/elasticache/replication_group.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/internal/service/elasticache/replication_group.go b/internal/service/elasticache/replication_group.go index 92710c3b2494..c2f172b19e64 100644 --- a/internal/service/elasticache/replication_group.go +++ b/internal/service/elasticache/replication_group.go @@ -862,7 +862,13 @@ func resourceReplicationGroupUpdate(ctx context.Context, d *schema.ResourceData, } if requestUpdate { - _, err := conn.ModifyReplicationGroupWithContext(ctx, input) + // tagging may cause this resource to not yet be available, so wait for it to be available + _, err := WaitReplicationGroupAvailable(ctx, conn, d.Id(), d.Timeout(schema.TimeoutUpdate)) + if err != nil { + return sdkdiag.AppendErrorf(diags, "waiting for ElastiCache Replication Group (%s) to update: %s", d.Id(), err) + } + + _, err = conn.ModifyReplicationGroupWithContext(ctx, input) if err != nil { return sdkdiag.AppendErrorf(diags, "updating ElastiCache Replication Group (%s): %s", d.Id(), err) } @@ -881,7 +887,13 @@ func resourceReplicationGroupUpdate(ctx context.Context, d *schema.ResourceData, AuthToken: aws.String(d.Get("auth_token").(string)), } - _, err := conn.ModifyReplicationGroupWithContext(ctx, params) + // tagging may cause this resource to not yet be available, so wait for it to be available + _, err := WaitReplicationGroupAvailable(ctx, conn, d.Id(), d.Timeout(schema.TimeoutUpdate)) + if err != nil { + return sdkdiag.AppendErrorf(diags, "waiting for ElastiCache Replication Group (%s) to update: %s", d.Id(), err) + } + + _, err = conn.ModifyReplicationGroupWithContext(ctx, params) if err != nil { return sdkdiag.AppendErrorf(diags, "changing auth_token for ElastiCache Replication Group (%s): %s", d.Id(), err) } From 8bff4a24456766ec8d4a42fc3deffea4745c5c2d Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Mon, 11 Mar 2024 21:21:55 -0400 Subject: [PATCH 2/7] elasticache: Allow for tagging retries --- internal/service/elasticache/tags_gen.go | 38 ++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/internal/service/elasticache/tags_gen.go b/internal/service/elasticache/tags_gen.go index 51cf3d9882e0..251e46c6eeec 100644 --- a/internal/service/elasticache/tags_gen.go +++ b/internal/service/elasticache/tags_gen.go @@ -4,6 +4,7 @@ package elasticache import ( "context" "fmt" + "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/elasticache" @@ -12,6 +13,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/logging" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/types/option" "github.com/hashicorp/terraform-provider-aws/names" ) @@ -24,7 +26,17 @@ func listTags(ctx context.Context, conn elasticacheiface.ElastiCacheAPI, identif ResourceName: aws.String(identifier), } - output, err := conn.ListTagsForResourceWithContext(ctx, input) + output, err := tfresource.RetryGWhenMessageContains(ctx, 15*time.Minute, + func() (*elasticache.TagListMessage, error) { + return conn.ListTagsForResourceWithContext(ctx, input) + }, + []string{ + elasticache.ErrCodeInvalidReplicationGroupStateFault, + }, + []string{ + "not in available state", + }, + ) if err != nil { return tftags.New(ctx, nil), err @@ -123,7 +135,17 @@ func updateTags(ctx context.Context, conn elasticacheiface.ElastiCacheAPI, ident TagKeys: aws.StringSlice(removedTags.Keys()), } - _, err := conn.RemoveTagsFromResourceWithContext(ctx, input) + _, err := tfresource.RetryWhenMessageContains(ctx, 15*time.Minute, + func() (any, error) { + return conn.RemoveTagsFromResourceWithContext(ctx, input) + }, + []string{ + elasticache.ErrCodeInvalidReplicationGroupStateFault, + }, + []string{ + "not in available state", + }, + ) if err != nil { return fmt.Errorf("untagging resource (%s): %w", identifier, err) @@ -138,7 +160,17 @@ func updateTags(ctx context.Context, conn elasticacheiface.ElastiCacheAPI, ident Tags: Tags(updatedTags), } - _, err := conn.AddTagsToResourceWithContext(ctx, input) + _, err := tfresource.RetryWhenMessageContains(ctx, 15*time.Minute, + func() (any, error) { + return conn.AddTagsToResourceWithContext(ctx, input) + }, + []string{ + elasticache.ErrCodeInvalidReplicationGroupStateFault, + }, + []string{ + "not in available state", + }, + ) if err != nil { return fmt.Errorf("tagging resource (%s): %w", identifier, err) From 2f921efd391d2c83fee37aac56234a5b1e36db67 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Mon, 11 Mar 2024 21:22:41 -0400 Subject: [PATCH 3/7] elasticache: Generate tags retrier --- internal/service/elasticache/generate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/elasticache/generate.go b/internal/service/elasticache/generate.go index f7f3bdcb3c09..0436db543248 100644 --- a/internal/service/elasticache/generate.go +++ b/internal/service/elasticache/generate.go @@ -1,7 +1,7 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: MPL-2.0 -//go:generate go run ../../generate/tags/main.go -ListTags -ListTagsInIDElem=ResourceName -ListTagsOutTagsElem=TagList -ServiceTagsSlice -TagOp=AddTagsToResource -TagInIDElem=ResourceName -UntagOp=RemoveTagsFromResource -UpdateTags -CreateTags +//go:generate go run ../../generate/tags/main.go -ListTags -ListTagsInIDElem=ResourceName -ListTagsOutTagsElem=TagList -ServiceTagsSlice -TagOp=AddTagsToResource -TagInIDElem=ResourceName -UntagOp=RemoveTagsFromResource -UpdateTags -CreateTags -RetryTagsListTagsType=TagListMessage -RetryTagsErrorCodes=elasticache.ErrCodeInvalidReplicationGroupStateFault "-RetryTagsErrorMessages=not in available state" -RetryTagsTimeout=15m //go:generate go run ../../generate/tags/main.go -AWSSDKVersion=2 -TagsFunc=TagsV2 -KeyValueTagsFunc=keyValueTagsV2 -GetTagsInFunc=getTagsInV2 -SetTagsOutFunc=setTagsOutV2 -SkipAWSServiceImp -KVTValues -ServiceTagsSlice -- tagsv2_gen.go //go:generate go run ../../generate/servicepackage/main.go // ONLY generate directives and package declaration! Do not add anything else to this file. From f0e033b90a123d47013dd5a0e47a27b9036a6278 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Mon, 11 Mar 2024 21:23:09 -0400 Subject: [PATCH 4/7] tfresource: New retry options --- internal/tfresource/retry.go | 61 ++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/internal/tfresource/retry.go b/internal/tfresource/retry.go index 3396888dfaa5..d04f0eb418dd 100644 --- a/internal/tfresource/retry.go +++ b/internal/tfresource/retry.go @@ -57,6 +57,41 @@ func RetryWhen(ctx context.Context, timeout time.Duration, f func() (interface{} return output, nil } +// RetryGWhen is the generic version of RetryWhen which obviates the need for a type +// assertion after the call. It retries the function `f` when the error it returns +// satisfies `retryable`. `f` is retried until `timeout` expires. +func RetryGWhen[T any](ctx context.Context, timeout time.Duration, f func() (*T, error), retryable Retryable) (*T, error) { + var output *T + + err := Retry(ctx, timeout, func() *retry.RetryError { + var err error + var again bool + + output, err = f() + again, err = retryable(err) + + if again { + return retry.RetryableError(err) + } + + if err != nil { + return retry.NonRetryableError(err) + } + + return nil + }) + + if TimedOut(err) { + output, err = f() + } + + if err != nil { + return nil, err + } + + return output, nil +} + // RetryWhenAWSErrCodeEquals retries the specified function when it returns one of the specified AWS error codes. func RetryWhenAWSErrCodeEquals(ctx context.Context, timeout time.Duration, f func() (interface{}, error), codes ...string) (interface{}, error) { // nosemgrep:ci.aws-in-func-name return RetryWhen(ctx, timeout, f, func(err error) (bool, error) { @@ -90,6 +125,32 @@ func RetryWhenAWSErrMessageContains(ctx context.Context, timeout time.Duration, }) } +// RetryWhenMessageContains retries the specified function when it returns an error containing any of the specified messages. +func RetryWhenMessageContains(ctx context.Context, timeout time.Duration, f func() (interface{}, error), codes []string, messages []string) (interface{}, error) { + return RetryWhen(ctx, timeout, f, func(err error) (bool, error) { + for i, message := range messages { + if tfawserr.ErrMessageContains(err, codes[i], message) || tfawserr_sdkv2.ErrMessageContains(err, codes[i], message) { + return true, err + } + } + + return false, err + }) +} + +// RetryGWhenMessageContains retries the specified function when it returns an error containing any of the specified messages. +func RetryGWhenMessageContains[T any](ctx context.Context, timeout time.Duration, f func() (*T, error), codes []string, messages []string) (*T, error) { + return RetryGWhen(ctx, timeout, f, func(err error) (bool, error) { + for i, message := range messages { + if tfawserr.ErrMessageContains(err, codes[i], message) || tfawserr_sdkv2.ErrMessageContains(err, codes[i], message) { + return true, err + } + } + + return false, err + }) +} + func RetryWhenIsA[T error](ctx context.Context, timeout time.Duration, f func() (interface{}, error)) (interface{}, error) { return RetryWhen(ctx, timeout, f, func(err error) (bool, error) { if errs.IsA[T](err) { From bfa21e0ac10f090276da4f58f7ac461683afb643 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Mon, 11 Mar 2024 21:23:46 -0400 Subject: [PATCH 5/7] generators/tags: Retry tagging --- .../tags/templates/v1/get_tag_body.tmpl | 18 +++++++ .../tags/templates/v1/list_tags_body.tmpl | 53 ++++++++++++++++++ .../tags/templates/v1/update_tags_body.tmpl | 54 +++++++++++++++++++ 3 files changed, 125 insertions(+) diff --git a/internal/generate/tags/templates/v1/get_tag_body.tmpl b/internal/generate/tags/templates/v1/get_tag_body.tmpl index f7dc90fe26a6..04d5f9a44adb 100644 --- a/internal/generate/tags/templates/v1/get_tag_body.tmpl +++ b/internal/generate/tags/templates/v1/get_tag_body.tmpl @@ -23,7 +23,25 @@ func {{ .GetTagFunc }}(ctx context.Context, conn {{ .ClientType }}, identifier{{ }, } + {{ if .RetryTagsListTagsType }} + output, err := tfresource.RetryGWhenMessageContains(ctx, {{ .RetryTagsTimeout }}, + func() (*{{ .TagPackage }}.{{ .RetryTagsListTagsType }}, error) { + return conn.{{ .ListTagsOp }}WithContext(ctx, input) + }, + []string{ + {{- range .RetryTagsErrorCodes }} + {{ . }}, + {{- end }} + }, + []string{ + {{- range .RetryTagsErrorMessages }} + "{{ . }}", + {{- end }} + }, + ) + {{ else }} output, err := conn.{{ .ListTagsOp }}WithContext(ctx, input) + {{- end }} if err != nil { return nil, err diff --git a/internal/generate/tags/templates/v1/list_tags_body.tmpl b/internal/generate/tags/templates/v1/list_tags_body.tmpl index 30b63823db9d..292b1095446f 100644 --- a/internal/generate/tags/templates/v1/list_tags_body.tmpl +++ b/internal/generate/tags/templates/v1/list_tags_body.tmpl @@ -28,6 +28,39 @@ func {{ .ListTagsFunc }}(ctx context.Context, conn {{ .ClientType }}, identifier var output []*{{ .TagPackage }}.{{ or .TagType2 .TagType }} {{- end }} + {{ if .RetryTagsListTagsType }} + _, err := tfresource.RetryWhenMessageContains(ctx, {{ .RetryTagsTimeout }}, + func() (string, error) { + return "", conn.{{ .ListTagsOp }}PagesWithContext(ctx, input, func(page *{{ .TagPackage }}.{{ .ListTagsOp }}Output, lastPage bool) bool { + if page == nil { + return !lastPage + } + + {{ if .ServiceTagsMap }} + maps.Copy(output, page.{{ .ListTagsOutTagsElem }}) + {{- else }} + for _, v := range page.{{ .ListTagsOutTagsElem }} { + if v != nil { + output = append(output, v) + } + } + {{- end }} + + return !lastPage + }) + }, + []string{ + {{- range .RetryTagsErrorCodes }} + {{ . }}, + {{- end }} + }, + []string{ + {{- range .RetryTagsErrorMessages }} + "{{ . }}", + {{- end }} + }, + ) + {{ else }} err := conn.{{ .ListTagsOp }}PagesWithContext(ctx, input, func(page *{{ .TagPackage }}.{{ .ListTagsOp }}Output, lastPage bool) bool { if page == nil { return !lastPage @@ -45,9 +78,29 @@ func {{ .ListTagsFunc }}(ctx context.Context, conn {{ .ClientType }}, identifier return !lastPage }) + {{- end }} {{ else }} + {{- if .RetryTagsListTagsType }} + + output, err := tfresource.RetryGWhenMessageContains(ctx, {{ .RetryTagsTimeout }}, + func() (*{{ .TagPackage }}.{{ .RetryTagsListTagsType }}, error) { + return conn.{{ .ListTagsOp }}WithContext(ctx, input) + }, + []string{ + {{- range .RetryTagsErrorCodes }} + {{ . }}, + {{- end }} + }, + []string{ + {{- range .RetryTagsErrorMessages }} + "{{ . }}", + {{- end }} + }, + ) + {{- else }} output, err := conn.{{ .ListTagsOp }}WithContext(ctx, input) + {{- end }} {{- end }} {{ if and ( .ParentNotFoundErrCode ) ( .ParentNotFoundErrMsg ) }} diff --git a/internal/generate/tags/templates/v1/update_tags_body.tmpl b/internal/generate/tags/templates/v1/update_tags_body.tmpl index e94863b6b63d..27d0d5bc01af 100644 --- a/internal/generate/tags/templates/v1/update_tags_body.tmpl +++ b/internal/generate/tags/templates/v1/update_tags_body.tmpl @@ -60,7 +60,25 @@ func {{ .UpdateTagsFunc }}(ctx context.Context, conn {{ .ClientType }}, identifi {{- end }} } + {{ if .RetryTagsListTagsType }} + _, err := tfresource.RetryWhenMessageContains(ctx, {{ .RetryTagsTimeout }}, + func() (any, error) { + return conn.{{ .TagOp }}WithContext(ctx, input) + }, + []string{ + {{- range .RetryTagsErrorCodes }} + {{ . }}, + {{- end }} + }, + []string{ + {{- range .RetryTagsErrorMessages }} + "{{ . }}", + {{- end }} + }, + ) + {{ else }} _, err := conn.{{ .TagOp }}WithContext(ctx, input) + {{- end }} if err != nil { return fmt.Errorf("tagging resource (%s): %w", identifier, err) @@ -100,7 +118,25 @@ func {{ .UpdateTagsFunc }}(ctx context.Context, conn {{ .ClientType }}, identifi {{- end }} } + {{ if .RetryTagsListTagsType }} + _, err := tfresource.RetryWhenMessageContains(ctx, {{ .RetryTagsTimeout }}, + func() (any, error) { + return conn.{{ .UntagOp }}WithContext(ctx, input) + }, + []string{ + {{- range .RetryTagsErrorCodes }} + {{ . }}, + {{- end }} + }, + []string{ + {{- range .RetryTagsErrorMessages }} + "{{ . }}", + {{- end }} + }, + ) + {{ else }} _, err := conn.{{ .UntagOp }}WithContext(ctx, input) + {{- end }} if err != nil { return fmt.Errorf("untagging resource (%s): %w", identifier, err) @@ -138,7 +174,25 @@ func {{ .UpdateTagsFunc }}(ctx context.Context, conn {{ .ClientType }}, identifi {{- end }} } + {{ if .RetryTagsListTagsType }} + _, err := tfresource.RetryWhenMessageContains(ctx, {{ .RetryTagsTimeout }}, + func() (any, error) { + return conn.{{ .TagOp }}WithContext(ctx, input) + }, + []string{ + {{- range .RetryTagsErrorCodes }} + {{ . }}, + {{- end }} + }, + []string{ + {{- range .RetryTagsErrorMessages }} + "{{ . }}", + {{- end }} + }, + ) + {{ else }} _, err := conn.{{ .TagOp }}WithContext(ctx, input) + {{- end }} if err != nil { return fmt.Errorf("tagging resource (%s): %w", identifier, err) From 31adc262ed05029d87efea1eba11c05a9f8f52a2 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Mon, 11 Mar 2024 21:24:12 -0400 Subject: [PATCH 6/7] generator/tags: Add retry capability --- internal/generate/tags/main.go | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/internal/generate/tags/main.go b/internal/generate/tags/main.go index 5d576a78094a..7edaa4ace8f8 100644 --- a/internal/generate/tags/main.go +++ b/internal/generate/tags/main.go @@ -54,6 +54,10 @@ var ( listTagsOp = flag.String("ListTagsOp", "ListTagsForResource", "listTagsOp") listTagsOpPaginated = flag.Bool("ListTagsOpPaginated", false, "whether ListTagsOp is paginated") listTagsOutTagsElem = flag.String("ListTagsOutTagsElem", "Tags", "listTagsOutTagsElem") + retryTagsListTagsType = flag.String("RetryTagsListTagsType", "", "type of the first ListTagsOp return value such as TagListMessage") + retryTagsErrorCodes = flag.String("RetryTagsErrorCodes", "", "comma-separated list of error codes to retry, must be used with RetryTagsListTagsType and same length as RetryTagsErrorMessages") + retryTagsErrorMessages = flag.String("RetryTagsErrorMessages", "", "comma-separated list of error messages to retry, must be used with RetryTagsListTagsType and same length as RetryTagsErrorCodes") + retryTagsTimeout = flag.Duration("RetryTagsTimeout", 1*time.Minute, "Timeout for retrying tag operations") setTagsOutFunc = flag.String("SetTagsOutFunc", "setTagsOut", "setTagsOutFunc") tagInCustomVal = flag.String("TagInCustomVal", "", "tagInCustomVal") tagInIDElem = flag.String("TagInIDElem", "ResourceArn", "tagInIDElem") @@ -171,7 +175,11 @@ type TemplateData struct { ListTagsOutTagsElem string ParentNotFoundErrCode string ParentNotFoundErrMsg string - RetryCreateOnNotFound string + RetryCreateOnNotFound string // is this used? + RetryTagsListTagsType string + RetryTagsErrorCodes []string + RetryTagsErrorMessages []string + RetryTagsTimeout string ServiceTagsMap bool SetTagsOutFunc string TagInCustomVal string @@ -295,6 +303,15 @@ func main() { } } + var cleanRetryErrorCodes []string + for _, c := range strings.Split(*retryTagsErrorCodes, ",") { + if strings.HasPrefix(c, fmt.Sprintf("%s.", servicePackage)) || strings.HasPrefix(c, "types.") { + cleanRetryErrorCodes = append(cleanRetryErrorCodes, c) + } else { + cleanRetryErrorCodes = append(cleanRetryErrorCodes, fmt.Sprintf(`"%s"`, c)) + } + } + templateData := TemplateData{ AWSService: awsPkg, AWSServiceIfacePackage: awsIntfPkg, @@ -312,8 +329,8 @@ func main() { SkipServiceImp: *skipServiceImp, SkipTypesImp: *skipTypesImp, TfLogPkg: *updateTags, - TfResourcePkg: (*getTag || *waitForPropagation), - TimePkg: *waitForPropagation, + TfResourcePkg: *getTag || *waitForPropagation || *retryTagsListTagsType != "", + TimePkg: *waitForPropagation || *retryTagsListTagsType != "", CreateTagsFunc: createTagsFunc, GetTagFunc: *getTagFunc, @@ -329,6 +346,10 @@ func main() { ListTagsOutTagsElem: *listTagsOutTagsElem, ParentNotFoundErrCode: *parentNotFoundErrCode, ParentNotFoundErrMsg: *parentNotFoundErrMsg, + RetryTagsListTagsType: *retryTagsListTagsType, + RetryTagsErrorCodes: cleanRetryErrorCodes, + RetryTagsErrorMessages: strings.Split(*retryTagsErrorMessages, ","), + RetryTagsTimeout: formatDuration(*retryTagsTimeout), ServiceTagsMap: *serviceTagsMap, SetTagsOutFunc: *setTagsOutFunc, TagInCustomVal: *tagInCustomVal, From d134b55b5ef1a9dfc5fdcde820bc937d5384e1b8 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Mon, 11 Mar 2024 21:34:21 -0400 Subject: [PATCH 7/7] Add changelog --- .changelog/36310.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/36310.txt diff --git a/.changelog/36310.txt b/.changelog/36310.txt new file mode 100644 index 000000000000..1868b7c31f2c --- /dev/null +++ b/.changelog/36310.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_elasticache_replication_group: Fix bugs causing errors like `InvalidReplicationGroupState: Cluster not in available state to perform tagging operations.` +``` \ No newline at end of file