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.

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

Add retry mechanism and timeout when reading ecr_repository #3859

Closed
wants to merge 1 commit into from

Conversation

mildred
Copy link
Contributor

@mildred mildred commented Mar 21, 2018

When creating an ECR repository, creation can be successful but it may
not be available right away through DescribeRepositories API request. We
add a retry mechanism until the repository appears instead of marking
the repository aas not existing.

Fixes #3849

@ghost ghost added the size/M Managed by automation to categorize the size of a PR. label Mar 21, 2018
When creating an ECR repository, creation can be successful but it may
not be available right away through DescribeRepositories API request. We
add a retry mechanism until the repository appears instead of marking
the repository aas not existing.

Fixes hashicorp#3849
@ghost ghost added the size/M Managed by automation to categorize the size of a PR. label Mar 21, 2018
`aws_ecr_repository` provides the following [Timeouts](/docs/configuration/resources.html#timeouts)
configuration options:

- `read` - (Default `10 minutes`) How long to wait for a repository to be listed.
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably specifically mention on creation. We also generally prefer to default to 1 minute unless there is some known documentation about eventual consistency taking a certain amount of time.

if err != nil {
if d.IsNewResource() && isAWSErr(err, ecr.ErrCodeRepositoryNotFoundException, "") {
return resource.RetryableError(err)
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nitpick: the else and nesting here are unnecessary

@bflad bflad added bug Addresses a defect in current functionality. service/ecr Issues and PRs that pertain to the ecr service. labels Mar 21, 2018
@mildred
Copy link
Contributor Author

mildred commented Mar 26, 2018

@bflad unfortunately, I didn't really think when opening this pull request but we use this commit internally, and I don't want to change the default timeout until we change our use of this resource. I'll close this PR and there is a new PR opened with the changes you requested in #3910

There is no specific AWS documentation telling that an ECR read could take time. They only told us that read requests were more rate limited than write requests.

@mildred mildred closed this Mar 26, 2018
@ghost
Copy link

ghost commented Apr 7, 2020

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 ghost locked and limited conversation to collaborators Apr 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/ecr Issues and PRs that pertain to the ecr service. size/M Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ECR creation sometimes fails with Resource '...' does not have attribute 'repository_url' for variable '...'
2 participants