resource/aws_appautoscaling_policy: Prevent state removal at creation

[camlow325]

Community Note

• Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
• Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Closes #10549

Release note for CHANGELOG:

* resource/aws_appautoscaling_policy: Prevent state removal of resource immediately after creation due to eventual consistency ([#10549](https://github.com/terraform-providers/terraform-provider-aws/issues/10549))

The AWS application-autoscaling service has eventual consistency considerations. The aws_appautoscaling_policy resource immediately tries to read a scaling policy after creation. If the policy is not found, the application-autoscaling service returns a 200 OK with an
empty list of scaling policies. Since no scaling policy is present, the aws_appautoscaling_policy 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 scaling policies 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=TestAccAWSAppautoScalingPolicy_'
...
--- PASS: TestAccAWSAppautoScalingPolicy_multiplePoliciesSameName (34.27s)
--- PASS: TestAccAWSAppautoScalingPolicy_dynamoDb (36.59s)
--- PASS: TestAccAWSAppautoScalingPolicy_multiplePoliciesSameResource (37.73s)
--- PASS: TestAccAWSAppautoScalingPolicy_disappears (68.94s)
--- PASS: TestAccAWSAppautoScalingPolicy_basic (72.25s)
--- PASS: TestAccAWSAppautoScalingPolicy_scaleOutAndIn (74.31s)
--- PASS: TestAccAWSAppautoScalingPolicy_spotFleetRequest (77.85s)
--- PASS: TestAccAWSAppautoScalingPolicy_ResourceId_ForceNew (88.97s)

[camlow325]

This is my first contribution to the project and I'm not familiar with this code base. If there's a sane test that would make sense to add to this PR, please let me know. Where the problem has been very timing specific, I think a test would need to mock one or more responses from AWS with an empty result in order to trigger the retry code. It wasn't immediately obvious to me if there's a pattern for doing that in these tests that would be good to follow. Thanks much for reviewing this!

[bflad]

This looks good to me, thanks @camlow325 for this fix. 🚀 Specific testing this type of eventual consistency issue is not easily achievable since it depends on behavior inherent in the AWS API, so we have to rely on existing acceptance tests to try and ensure no additional regressions are introduced.

Output from acceptance testing:

--- PASS: TestValidateAppautoscalingPolicyImportInput (0.00s)
--- PASS: TestAccAWSAppautoScalingPolicy_multiplePoliciesSameName (18.83s)
--- PASS: TestAccAWSAppautoScalingPolicy_dynamoDb (19.77s)
--- PASS: TestAccAWSAppautoScalingPolicy_multiplePoliciesSameResource (20.60s)
--- PASS: TestAccAWSAppautoScalingPolicy_basic (55.30s)
--- PASS: TestAccAWSAppautoScalingPolicy_spotFleetRequest (60.57s)
--- PASS: TestAccAWSAppautoScalingPolicy_disappears (65.29s)
--- PASS: TestAccAWSAppautoScalingPolicy_scaleOutAndIn (66.84s)
--- PASS: TestAccAWSAppautoScalingPolicy_ResourceId_ForceNew (72.57s)

[ghost]

This has been released in version 2.44.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!

[ghost]

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!