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

Sweeper: exclude replication group member clusters from aws_elasticache_cluster sweeper #17650

Merged
merged 4 commits into from
Feb 17, 2021

Conversation

gdavison
Copy link
Contributor

Prevents retry when trying to delete the last member cluster from a replication group.

Excludes replication group member clusters from sweeper.

Closes #17647

@gdavison gdavison requested a review from a team as a code owner February 17, 2021 01:05
@ghost ghost added size/S Managed by automation to categorize the size of a PR. service/elasticache Issues and PRs that pertain to the elasticache service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Feb 17, 2021
Copy link
Contributor

@anGie44 anGie44 left a comment

Choose a reason for hiding this comment

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

Looks great! just one small q more for my own curiosity

Comment on lines 57 to 60
if tfawserr.ErrMessageContains(err, elasticache.ErrCodeInvalidCacheClusterStateFault, "only member of a replication group") {
log.Printf("[WARN] ElastiCache Cache Cluster (%s) is member of a Replication Group. Ignoring. %s", id, err)
continue
}
Copy link
Contributor

Choose a reason for hiding this comment

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

qq: if we're filtering these specific clusters out in the DescribeCacheClustersInput , is it still possible to hit this type of error? doesn't hurt to catch the error tho 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, it's redundant since they're filtered out from the results

Copy link
Contributor

Choose a reason for hiding this comment

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

gotchu, sounds good! tho i'm always cautious with AWS APIs 😅

@gdavison gdavison merged commit 6c23c96 into main Feb 17, 2021
@gdavison gdavison deleted the elasticache-cluster-sweeper branch February 17, 2021 19:52
@github-actions github-actions bot added this to the v3.29.0 milestone Feb 17, 2021
github-actions bot pushed a commit that referenced this pull request Feb 17, 2021
heitorlessa added a commit to heitorlessa/terraform-provider-aws that referenced this pull request Feb 18, 2021
…r-aws into main

* 'main' of https://github.com/hashicorp/terraform-provider-aws: (599 commits)
  Adds CHANGELOG
  Updates acceptance tests and documentation
  Update CHANGELOG.md for hashicorp#17650
  Removes redundant error check
  Update CHANGELOG.md for hashicorp#17645
  resource/aws_lb_listener_certificate: Prevent resource ID parsing error with IAM Server Certificate names containing underscores (hashicorp#17645)
  resource/aws_instance: Apply attribute waiter logic to iam_instance_profile attribute (hashicorp#17414)
  Update CHANGELOG.md for hashicorp#17534
  Adds multi-error handling to ElastiCache Cluster sweeper
  Excludes Clusters in Replication Groups from sweeper
  Adds check for deleting only member of replication group and does not retry
  Update CHANGELOG.md for hashicorp#17204
  Update docs for cloudfront_distribution resource
  fix(aws_ses_reciept_rule): include period in name validation regex
  Add CHANGELOG entry.
  r/aws_dx_gateway_association: Fix 'TestAccAwsDxGatewayAssociation_multiVpnGatewaysSingleAccount'.
  r/aws_ec2_transit_gateway: Increase deletion IncorrectState retry to 5 minutes.
  Update CHANGELOG.md after release
  fix if evaluation
  r/aws_dx_gateway_association: Modernize acceptance tests.
  ...
heitorlessa added a commit to heitorlessa/terraform-provider-aws that referenced this pull request Feb 18, 2021
* main: (599 commits)
  Adds CHANGELOG
  Updates acceptance tests and documentation
  Update CHANGELOG.md for hashicorp#17650
  Removes redundant error check
  Update CHANGELOG.md for hashicorp#17645
  resource/aws_lb_listener_certificate: Prevent resource ID parsing error with IAM Server Certificate names containing underscores (hashicorp#17645)
  resource/aws_instance: Apply attribute waiter logic to iam_instance_profile attribute (hashicorp#17414)
  Update CHANGELOG.md for hashicorp#17534
  Adds multi-error handling to ElastiCache Cluster sweeper
  Excludes Clusters in Replication Groups from sweeper
  Adds check for deleting only member of replication group and does not retry
  Update CHANGELOG.md for hashicorp#17204
  Update docs for cloudfront_distribution resource
  fix(aws_ses_reciept_rule): include period in name validation regex
  Add CHANGELOG entry.
  r/aws_dx_gateway_association: Fix 'TestAccAwsDxGatewayAssociation_multiVpnGatewaysSingleAccount'.
  r/aws_ec2_transit_gateway: Increase deletion IncorrectState retry to 5 minutes.
  Update CHANGELOG.md after release
  fix if evaluation
  r/aws_dx_gateway_association: Modernize acceptance tests.
  ...
@ghost
Copy link

ghost commented Feb 19, 2021

This has been released in version 3.29.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
Copy link

ghost commented Mar 20, 2021

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 as resolved and limited conversation to collaborators Mar 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
service/elasticache Issues and PRs that pertain to the elasticache service. size/S Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sweeper: aws_elasticache_cluster does not correctly handle Cluster that is last member of Replication Group
2 participants