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.

Sign up for GitHub

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

Jump to bottom

resource/aws_elasticache_replication_group: Support Cluster Mode Enabled online shard reconfiguration #3932

Merged
merged 4 commits into from
Apr 3, 2018
Merged

resource/aws_elasticache_replication_group: Support Cluster Mode Enabled online shard reconfiguration #3932

merged 4 commits into from
Apr 3, 2018

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Mar 27, 2018 •
edited
Loading

Closes #2904

  • Properly read cluster_mode back into Terraform state
  • Improves cluster_mode update plan output by migrating state from TypeSet to TypeList
  • Allows configurable timeouts for create, delete, and update
 16 tests passed (all tests)
=== RUN   TestAccAWSElasticacheReplicationGroup_clusteringAndCacheNodesCausesError
--- PASS: TestAccAWSElasticacheReplicationGroup_clusteringAndCacheNodesCausesError (21.18s)
=== RUN   TestAccAWSElasticacheReplicationGroup_Uppercase
--- PASS: TestAccAWSElasticacheReplicationGroup_Uppercase (906.19s)
=== RUN   TestAccAWSElasticacheReplicationGroup_basic
--- PASS: TestAccAWSElasticacheReplicationGroup_basic (966.37s)
=== RUN   TestAccAWSElasticacheReplicationGroup_importBasic
--- PASS: TestAccAWSElasticacheReplicationGroup_importBasic (972.76s)
=== RUN   TestAccAWSElasticacheReplicationGroup_enableSnapshotting
--- PASS: TestAccAWSElasticacheReplicationGroup_enableSnapshotting (1007.92s)
=== RUN   TestAccAWSElasticacheReplicationGroup_updateParameterGroup
--- PASS: TestAccAWSElasticacheReplicationGroup_updateParameterGroup (1009.62s)
=== RUN   TestAccAWSElasticacheReplicationGroup_enableAtRestEncryption
--- PASS: TestAccAWSElasticacheReplicationGroup_enableAtRestEncryption (1128.85s)
=== RUN   TestAccAWSElasticacheReplicationGroup_enableAuthTokenTransitEncryption
--- PASS: TestAccAWSElasticacheReplicationGroup_enableAuthTokenTransitEncryption (1132.64s)
=== RUN   TestAccAWSElasticacheReplicationGroup_updateDescription
--- PASS: TestAccAWSElasticacheReplicationGroup_updateDescription (1140.29s)
=== RUN   TestAccAWSElasticacheReplicationGroup_vpc
--- PASS: TestAccAWSElasticacheReplicationGroup_vpc (1171.57s)
=== RUN   TestAccAWSElasticacheReplicationGroup_updateMaintenanceWindow
--- PASS: TestAccAWSElasticacheReplicationGroup_updateMaintenanceWindow (1247.89s)
=== RUN   TestAccAWSElasticacheReplicationGroup_redisClusterInVpc2
--- PASS: TestAccAWSElasticacheReplicationGroup_redisClusterInVpc2 (1444.62s)
=== RUN   TestAccAWSElasticacheReplicationGroup_ClusterMode_Basic
--- PASS: TestAccAWSElasticacheReplicationGroup_ClusterMode_Basic (1472.17s)
=== RUN   TestAccAWSElasticacheReplicationGroup_multiAzInVpc
--- PASS: TestAccAWSElasticacheReplicationGroup_multiAzInVpc (1665.92s)
=== RUN   TestAccAWSElasticacheReplicationGroup_updateNodeSize
--- PASS: TestAccAWSElasticacheReplicationGroup_updateNodeSize (1907.65s)
=== RUN   TestAccAWSElasticacheReplicationGroup_ClusterMode_NumNodeGroups
--- PASS: TestAccAWSElasticacheReplicationGroup_ClusterMode_NumNodeGroups (4394.47s)

@bflad
resource/aws_elasticache_replication_group: Support Cluster Mode Enab…
681377a 
…led online shard reconfiguration

* Properly read cluster_mode back into Terraform state
* Improves cluster_mode update plan output by migrating state from TypeSet to TypeList
* Allows configurable timeouts for create, delete, and update
@bflad bflad added enhancement Requests to existing resources that expand the functionality or scope. service/elasticache Issues and PRs that pertain to the elasticache service. labels Mar 27, 2018
@bflad bflad added this to the v1.13.0 milestone Mar 27, 2018
@ghost ghost added the size/L Managed by automation to categorize the size of a PR. label Mar 27, 2018
@bflad
resource/aws_elasticache_replication_group: Mark cluster_mode Compute…
a684e3d 
…d: true for now
@bflad
tests/resource/aws_elasticache_replication_group: Fix number_cache_cl…
673dbe7 
…usters check in TestAccAWSElasticacheReplicationGroup_ClusterMode_NumNodeGroups
@bflad bflad requested a review from a team March 27, 2018 20:41
@bflad bflad mentioned this pull request Mar 28, 2018
Closed
@bflad bflad modified the milestones: v1.13.0, v1.14.0 Mar 29, 2018
tombuildsstuff
tombuildsstuff approved these changes Apr 3, 2018
View reviewed changes
Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

A couple of minor points but this otherwise LGTM 👍🏼

aws/resource_aws_elasticache_replication_group.go Outdated
}
} else {
d.Set("cluster_mode", []interface{}{})
}
Copy link
Contributor

Choose a reason for hiding this comment

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

minor this logic could live within the flatten function tbh?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its a little awkward because in order to keep backwards compatibility we have to use the other attribute. I can move it though.

aws/resource_aws_elasticache_replication_group_test.go Outdated
replicas_per_node_group = 1
num_node_groups = 2
num_node_groups = %[2]d
replicas_per_node_group = %[3]d
Copy link
Contributor

Choose a reason for hiding this comment

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

There’s a bug in govet where it won’t raise an error for these if you’re using the numbered index, but will if you’re using them as the actual type eg “%d” (when specifying the wrong type). Up to you, but I’d suggest there’s value in listing these variables explicitly (or by assigning them to a Terraform variable above and referencing that) so that these errors are raised?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wish I could fix that bug. 😉 For now I'll revert back to non-indexed references.

@ghost ghost added the size/L Managed by automation to categorize the size of a PR. label Apr 3, 2018
@bflad bflad merged commit bb2478b into master Apr 3, 2018
@bflad bflad deleted the f-aws_elasticache_replication_group-online-reshard branch April 3, 2018 02:26
bflad added a commit that referenced this pull request Apr 3, 2018
@bflad
Update CHANGELOG for #3932
e97d77a
@bflad
Copy link
Contributor Author

bflad commented Apr 6, 2018

This has been released in version 1.14.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

mattclegg pushed a commit to mattclegg/terraform-provider-aws that referenced this pull request Oct 13, 2018
REMOVE forceful recreation of resource and rely on Cluster Mode Enabl…
ed76270 
…ed online shard reconfiguration hashicorp#3932
@mattclegg mattclegg mentioned this pull request Oct 19, 2018
Closed
@ghost
Copy link

ghost commented Apr 6, 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 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tombuildsstuff tombuildsstuff tombuildsstuff approved these changes

Assignees
No one assigned
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/elasticache Issues and PRs that pertain to the elasticache service. size/L Managed by automation to categorize the size of a PR.
Projects
None yet
Milestone
v1.14.0
Development

Successfully merging this pull request may close these issues.

Feature Request: Amazon ElastiCache Redis Cluster Online Resizing
2 participants
@bflad @tombuildsstuff