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

resource/aws_rds_cluster: Retry deletion on InvalidDBClusterStateFault #3028

Merged
merged 1 commit into from
Jan 26, 2018

Conversation

radeksimko
Copy link
Member

This is to address the following intermittent test failure:

=== RUN   TestAccAWSRDSCluster_takeFinalSnapshot
--- FAIL: TestAccAWSRDSCluster_takeFinalSnapshot (49.13s)
    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_rds_cluster.default (destroy): 1 error(s) occurred:
        
        * aws_rds_cluster.default: RDS Cluster cannot be deleted: Cannot create a snapshot because the database instance tf-aurora-cluster-437515503953962644 is not currently in the available state.

I don't know if the 1 minute timeout is sufficient, we can always bump it if necessary. This is just a starting point.

Test results

=== RUN   TestAccAWSRDSCluster_missingUserNameCausesError
--- PASS: TestAccAWSRDSCluster_missingUserNameCausesError (3.31s)
=== RUN   TestAccAWSRDSCluster_iamAuth
--- PASS: TestAccAWSRDSCluster_iamAuth (109.82s)
=== RUN   TestAccAWSRDSCluster_basic
--- PASS: TestAccAWSRDSCluster_basic (110.56s)
=== RUN   TestAccAWSRDSCluster_encrypted
--- PASS: TestAccAWSRDSCluster_encrypted (110.86s)
=== RUN   TestAccAWSRDSCluster_importBasic
--- PASS: TestAccAWSRDSCluster_importBasic (111.11s)
=== RUN   TestAccAWSRDSCluster_updateIamRoles
--- PASS: TestAccAWSRDSCluster_updateIamRoles (120.71s)
=== RUN   TestAccAWSRDSCluster_backupsUpdate
--- PASS: TestAccAWSRDSCluster_backupsUpdate (121.64s)
=== RUN   TestAccAWSRDSCluster_generatedName
--- PASS: TestAccAWSRDSCluster_generatedName (127.02s)
=== RUN   TestAccAWSRDSCluster_updateTags
--- PASS: TestAccAWSRDSCluster_updateTags (127.76s)
=== RUN   TestAccAWSRDSCluster_namePrefix
--- PASS: TestAccAWSRDSCluster_namePrefix (129.11s)
=== RUN   TestAccAWSRDSCluster_kmsKey
--- PASS: TestAccAWSRDSCluster_kmsKey (141.01s)
=== RUN   TestAccAWSRDSCluster_takeFinalSnapshot
--- PASS: TestAccAWSRDSCluster_takeFinalSnapshot (191.50s)
=== RUN   TestAccAWSRDSClusterInstance_disappears
--- PASS: TestAccAWSRDSClusterInstance_disappears (543.06s)
=== RUN   TestAccAWSRDSClusterInstance_generatedName
--- PASS: TestAccAWSRDSClusterInstance_generatedName (644.77s)
=== RUN   TestAccAWSRDSClusterInstance_namePrefix
--- PASS: TestAccAWSRDSClusterInstance_namePrefix (665.52s)
=== RUN   TestAccAWSRDSClusterInstance_withInstanceEnhancedMonitor
--- PASS: TestAccAWSRDSClusterInstance_withInstanceEnhancedMonitor (712.32s)
=== RUN   TestAccAWSRDSClusterInstance_kmsKey
--- PASS: TestAccAWSRDSClusterInstance_kmsKey (730.22s)
=== RUN   TestAccAWSRDSClusterInstance_importBasic
--- PASS: TestAccAWSRDSClusterInstance_importBasic (782.37s)
=== RUN   TestAccAWSRDSClusterInstance_withInstancePerformanceInsights
--- PASS: TestAccAWSRDSClusterInstance_withInstancePerformanceInsights (832.13s)
=== RUN   TestAccAWSRDSClusterInstance_basic
--- PASS: TestAccAWSRDSClusterInstance_basic (1399.69s)

@radeksimko radeksimko added bug Addresses a defect in current functionality. service/rds Issues and PRs that pertain to the rds service. labels Jan 17, 2018
@bflad bflad self-requested a review January 17, 2018 17:55
@bflad bflad added this to the v1.7.1 milestone Jan 17, 2018
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

I think this is a decent approach for now. Two things come to mind though (not blockers):

  • We're not checking something like rds.ErrCodeDBClusterNotFoundFault here to skip further processing (although maybe rds.ErrCodeInvalidDBClusterStateFault already does?)
  • If we do happen to keep increasing the time is that there might be an off chance the cluster is already deleting (either outside TF or RDS API weirdness) in which we'd want to not keep retrying.
make testacc TEST=./aws TESTARGS='-run=TestAccAWSRDSCluster_takeFinalSnapshot'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSRDSCluster_takeFinalSnapshot -timeout 120m
=== RUN   TestAccAWSRDSCluster_takeFinalSnapshot
--- PASS: TestAccAWSRDSCluster_takeFinalSnapshot (150.43s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	150.482s

@radeksimko
Copy link
Member Author

If we do happen to keep increasing the time is that there might be an off chance the cluster is already deleting (either outside TF or RDS API weirdness) in which we'd want to not keep retrying.

That is a very good point. I will actually rework this to check & log the current state and see why this is really happening.

@radeksimko radeksimko modified the milestones: v1.7.1, v1.8.0 Jan 19, 2018
@radeksimko radeksimko merged commit 3c4bcce into master Jan 26, 2018
@radeksimko radeksimko deleted the b-rds-cluster-retry-deletion branch January 26, 2018 14:52
@bflad
Copy link
Contributor

bflad commented Jan 29, 2018

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

@ghost
Copy link

ghost commented Apr 8, 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 8, 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/rds Issues and PRs that pertain to the rds service.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants