resource/aws_cloudwatch_log_stream: Prevent early state removal #11617
Conversation
References: * hashicorp#11611 The AWS logs service has eventual consistency considerations. The `aws_cloudwatch_log_stream` resource immediately tries to read a stream after creation. If the stream is not found, the logs service returns a 200 OK with an empty list of streams. Since no streams are present, the `aws_cloudwatch_log_stream` resource removes the created resource from state, leading to a "produced an unexpected new value for was present, but now absent" error. With the changes in this commit, the empty list of streams in the response for the newly created resource will result in a NotFoundError being returned and a retry of the read request. A subsequent retry should hopefully be successful, leading to the state being preserved. Output from acceptance testing: ``` make testacc TEST=./aws TESTARGS='-run=TestAccAWSCloudWatchLogStream_' ... --- PASS: TestAccAWSCloudWatchLogStream_disappears_LogGroup (16.77s) --- PASS: TestAccAWSCloudWatchLogStream_disappears (19.55s) --- PASS: TestAccAWSCloudWatchLogStream_basic (19.91s) ```
ghost
added
size/XS
| var ls *cloudwatchlogs.LogStream | ||
| var exists bool | ||
|
|
||
| err := resource.Retry(2*time.Minute, func() *resource.RetryError { |
There was a problem hiding this comment.
I'm not really sure what a good upper bound for the retry timeout would be. In automation today, we do a second Terraform apply after the first one fails and that has always failed with a ResourceAlreadyExistsException: The specified log stream already exists error. The second run probably completes in well under 2 minutes in most cases.
There was a problem hiding this comment.
Two minutes is pretty reasonable 👍 We can/will likely add a constant for this in the future.
| var err error | ||
| ls, exists, err = lookupCloudWatchLogStream(conn, d.Id(), group, nil) | ||
| if err != nil { | ||
| return resource.NonRetryableError(err) |
There was a problem hiding this comment.
Maybe it would be good to retry for other kinds of error, too - not sure. For the cases we've seen so far that we'd benefit from retrying on, the initial lookup returns a 200 OK so not retrying on failures would be backward compatible.
bflad
added
bug
There was a problem hiding this comment.
Thanks so much for this fix, @camlow325 🚀
Output from acceptance testing:
--- PASS: TestAccAWSCloudWatchLogStream_disappears_LogGroup (6.57s)
--- PASS: TestAccAWSCloudWatchLogStream_disappears (7.24s)
--- PASS: TestAccAWSCloudWatchLogStream_basic (7.52s)
|
This has been released in version 2.48.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks! |
|
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
Community Note
Closes #11611
Release note for CHANGELOG:
The AWS logs service has eventual consistency considerations. The
aws_cloudwatch_log_streamresource immediately tries to read a stream after creation. If the stream is not found, the logs service returns a 200 OK with an empty list of streams. Since no streams are present, theaws_cloudwatch_log_streamresource removes the created resource from state, leading to a "produced an unexpected new value for was present, but now absent" error.With the changes in this commit, the empty list of streams in the response for the newly created resource will result in a NotFoundError being returned and a retry of the read request. A subsequent retry should hopefully be successful, leading to the state being preserved.
Output from acceptance testing: