resource/aws_kinesis_stream: Retry deletion on LimitExceededException

[radeksimko]

This is to address the following test failure:

=== RUN   TestAccAWSKinesisStream_shardLevelMetrics
--- FAIL: TestAccAWSKinesisStream_shardLevelMetrics (149.15s)
    testing.go:573: Error destroying resource! WARNING: Dangling resources
        may exist. The full state and error is shown below.
        
        Error: Error applying: 1 error(s) occurred:
        
        * aws_kinesis_stream.test_stream (destroy): 1 error(s) occurred:
        
        * aws_kinesis_stream.test_stream: LimitExceededException: Rate exceeded for stream terraform-kinesis-test-6203015600745191945 under account *******.
            status code: 400, request id: e8d516ee-dc31-d34e-ba35-e408e9363a84
        
        State: aws_kinesis_stream.test_stream:
          ID = arn:aws:kinesis:us-west-2:*******:stream/terraform-kinesis-test-6203015600745191945
          provider = provider.aws
          arn = arn:aws:kinesis:us-west-2:*******:stream/terraform-kinesis-test-6203015600745191945
          encryption_type = NONE
          kms_key_id = 
          name = terraform-kinesis-test-6203015600745191945
          retention_period = 24
          shard_count = 2
          shard_level_metrics.# = 1
          shard_level_metrics.123700221 = IncomingBytes
          tags.% = 1
          tags.Name = tf-test

Snippet from debug log:

2018/01/23 08:21:58 [DEBUG] [aws-sdk-go] DEBUG: Response kinesis/DeleteStream Details:
---[ RESPONSE ]--------------------------------------
HTTP/1.1 400 Bad Request
Connection: close
Content-Length: 143
Content-Type: application/x-amz-json-1.1
Date: Tue, 23 Jan 2018 08:21:57 GMT
Server: Apache-Coyote/1.1
X-Amz-Id-2: CjcrmKKLBS6budOskFaF9os926/iICeccBe89jmhfDQocgvMubxeBLkvJSnNbMfg9yJMCxTYGMHDkYAiibQUT51f0YmExBsC
X-Amzn-Requestid: e8d516ee-dc31-d34e-ba35-e408e9363a84

{"__type":"LimitExceededException","message":"Rate exceeded for stream terraform-kinesis-test-6203015600745191945 under account *REDACTED*."}
-----------------------------------------------------

Test results

=== RUN   TestAccAWSKinesisStream_basic
--- PASS: TestAccAWSKinesisStream_basic (94.90s)
=== RUN   TestAccAWSKinesisStream_importBasic
--- PASS: TestAccAWSKinesisStream_importBasic (95.57s)
=== RUN   TestAccAWSKinesisStream_encryptionWithoutKmsKeyThrowsError
--- PASS: TestAccAWSKinesisStream_encryptionWithoutKmsKeyThrowsError (108.26s)
=== RUN   TestAccAWSKinesisStream_retentionPeriod
--- PASS: TestAccAWSKinesisStream_retentionPeriod (162.46s)
=== RUN   TestAccAWSKinesisStream_shardLevelMetrics
--- PASS: TestAccAWSKinesisStream_shardLevelMetrics (173.29s)
=== RUN   TestAccAWSKinesisStream_shardCount
--- PASS: TestAccAWSKinesisStream_shardCount (214.29s)
=== RUN   TestAccAWSKinesisStream_createMultipleConcurrentStreams
--- PASS: TestAccAWSKinesisStream_createMultipleConcurrentStreams (237.69s)
=== RUN   TestAccAWSKinesisStream_encryption
--- PASS: TestAccAWSKinesisStream_encryption (267.77s)

[bflad]

This LGTM to me as-is, however would this be similarly or better handled with modifying the existing client.kinesisconn.Handlers.Retry? https://github.com/terraform-providers/terraform-provider-aws/blob/master/aws/config.go#L455-L467

[radeksimko]

That's a very valid point - I'll rework that. I just checked the SDK's retry mechanism and it looks like it's very close to ours:
https://github.com/aws/aws-sdk-go/blob/4bd6f8bc55ec7eaf68efb9ec72983fbc0b79aba2/aws/client/default_retryer.go#L38

[radeksimko]

Actually I'm not so sure, I think the reason we keep retry logic for Create here is because there are different contexts in which LimitExceededException is raised. I assume breaching some hard AWS limits can be context we don't want to retry on.

It doesn't matter for Describe & List, because these won't ever run into such situation, but I'm not so sure about Create and Delete. 🤔

[bflad]

Would something like this maybe help?

	client.kinesisconn.Handlers.Retry.PushBack(func(r *request.Request) {
		err, ok := r.Error.(awserr.Error)
		if !ok || err == nil {
			return
		}
		if err.Code() == kinesis.ErrCodeLimitExceededException {
			if !strings.HasPrefix(r.Operation.Name, "Describe") && !strings.HasPrefix(r.Operation.Name, "List") && !strings.Contains(err.Message(), "Rate exceeded for stream") {
				return
			}
			r.Retryable = aws.Bool(true)
		}
	})

[radeksimko]

Done. 😉

[bflad]

This has been released in terraform-provider-aws version 1.9.0. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

[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!