Add Retry to instanceProfileAddRole to handle IAM's eventual consistency #6079
Conversation
ghost
added
size/XS
| err := resource.Retry(30*time.Second, func() *resource.RetryError { | ||
| var err error | ||
| _, err = iamconn.AddRoleToInstanceProfile(request) | ||
| // Todo: handle retryable and non-retryable errors |
There was a problem hiding this comment.
Hi @joakiboxxx! Thanks for submitting this! Can you please update this to the below (including the comment):
// IAM unfortunately does not provide a better error code or message for eventual consistency
// InvalidParameterValue: Value (XXX) for parameter iamInstanceProfile.name is invalid. Invalid IAM Instance Profile name
if isAWSErr(err, "InvalidParameterValue", "Invalid IAM Instance Profile name") {
return resource.RetryableError(err)
}
if err != nil {
return resource.NonRetryableError(err)
}
return nilThere was a problem hiding this comment.
Hi Brian!
I applied your patch and tested it, and the change no longer solves my problem ;) With a debug inside the Retry callback, I get this from err:
[DEBUG] Error adding role NoSuchEntity: The role with name joakiboxxx_role cannot be found.
Wouldn't you agree that NoSuchEntity is indeed a retryable error? If so, may I propose the following:
if isAWSErr(err, "InvalidParameterValue", "Invalid IAM Instance Profile name") || isAWSErr(err, "NoSuchEntity", "The role with name") {
return resource.RetryableError(err)
}There was a problem hiding this comment.
Sorry if I missed that before! 😅 Indeed the NoSuchEntity error is much more expected and should fix the issue.
bflad
added
the
waiting-response
joakiboxxx
force-pushed
the
instanceProfileAddRole-Retry
branch
from
9862e9f
to
c720894
Compare
bflad
removed
the
waiting-response
There was a problem hiding this comment.
LGTM, thanks @joakiboxxx!! 🚀
--- PASS: TestAccAWSIAMInstanceProfile_missingRoleThrowsError (1.61s)
--- PASS: TestAccAWSIAMInstanceProfile_basic (8.35s)
--- PASS: TestAccAWSIAMInstanceProfile_withRoleNotRoles (8.81s)
--- PASS: TestAccAWSIAMInstanceProfile_namePrefix (8.98s)
--- PASS: TestAccAWSIAMInstanceProfile_importBasic (9.58s)
|
Thanks, @bflad! Happy to help :) |
|
This has been released in version 1.40.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
) * Roles are attached to profiles with eventual consistency via retries hashicorp/terraform-provider-aws#6079 * required 1.40 provider
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
ghost
locked and limited conversation to collaborators
Fixes #2012