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

Allow configuring number of node groups and replicas per node directly for "Redis (cluster mode disabled)" #16829

Merged
merged 3 commits into from
Jan 6, 2021

Conversation

gdavison
Copy link
Contributor

Typically, when creating a "Redis (cluster mode disabled)" aws_elasticache_replication_group, the number of nodes is specified using number_cache_clusters, and when creating a "Redis (cluster mode enabled)" aws_elasticache_replication_group, the configuration of node groups is specified using cluster_mode.num_node_groups and cluster_mode.replicas_per_node_group. Whether cluster mode is enabled is determined by the parameter group assigned to the replication group.

However, the API returns both parameters regardless of the cluster mode setting in the parameter group used by the replication group.

The value of number_cache_clusters will always be equal to cluster_mode.num_node_groups * (1 + cluster_mode.replicas_per_node_group). With cluster mode disabled, cluster_mode.num_node_groups is 1.

Currently, the resource allows setting

cluster_mode {
  num_node_groups         = 1
  replicas_per_node_group = 1
}

when cluster mode is not enabled, but does not return a value for the cluster_mode block.

This PR adds the returned value for the cluster_mode block and adds a cluster_enabled output parameter.

Release note for CHANGELOG:

* resource/aws_elasticache_replication_group: Allows configuring `replicas_per_node_group` for "Redis (cluster mode disabled)"

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSElasticacheReplicationGroup_'

--- PASS: TestAccAWSElasticacheReplicationGroup_clusteringAndCacheNodesCausesError (151.68s)
--- PASS: TestAccAWSElasticacheReplicationGroup_ClusterMode_NonClusteredParameterGroup (868.50s)
--- PASS: TestAccAWSElasticacheReplicationGroup_updateParameterGroup (1128.57s)
--- PASS: TestAccAWSElasticacheReplicationGroup_ClusterMode_Basic (1180.88s)
--- PASS: TestAccAWSElasticacheReplicationGroup_updateMaintenanceWindow (1189.76s)
--- PASS: TestAccAWSElasticacheReplicationGroup_enableSnapshotting (1488.39s)
--- PASS: TestAccAWSElasticacheReplicationGroup_vpc (1498.01s)
--- PASS: TestAccAWSElasticacheReplicationGroup_basic (1498.58s)
--- PASS: TestAccAWSElasticacheReplicationGroup_enableAuthTokenTransitEncryption (1684.19s)
--- PASS: TestAccAWSElasticacheReplicationGroup_multiAzInVpc (1763.90s)
--- PASS: TestAccAWSElasticacheReplicationGroup_useCmkKmsKeyId (1811.39s)
--- PASS: TestAccAWSElasticacheReplicationGroup_enableAtRestEncryption (1822.63s)
--- PASS: TestAccAWSElasticacheReplicationGroup_tags (2021.41s)
--- PASS: TestAccAWSElasticacheReplicationGroup_redisClusterInVpc2 (2076.63s)
--- PASS: TestAccAWSElasticacheReplicationGroup_updateNodeSize (2114.40s)
--- PASS: TestAccAWSElasticacheReplicationGroup_updateDescription (2324.69s)
--- PASS: TestAccAWSElasticacheReplicationGroup_Uppercase (2177.23s)
--- PASS: TestAccAWSElasticacheReplicationGroup_NumberCacheClusters (2993.37s)
--- PASS: TestAccAWSElasticacheReplicationGroup_NumberCacheClusters_Failover_AutoFailoverDisabled (3094.51s)
--- PASS: TestAccAWSElasticacheReplicationGroup_NumberCacheClusters_Failover_AutoFailoverEnabled (3261.51s)
--- PASS: TestAccAWSElasticacheReplicationGroup_ClusterMode_NumNodeGroups (3890.04s)

@gdavison gdavison requested a review from a team as a code owner December 17, 2020 20:47
@ghost ghost added size/M Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. service/elasticache Issues and PRs that pertain to the elasticache service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Dec 17, 2020
@YakDriver YakDriver self-assigned this Jan 6, 2021
@YakDriver
Copy link
Member

Holy smokes. Nothing like nice, quick, easy acceptance tests. I'll get back to you when they run. I've looked over the code and it looks good.

Copy link
Member

@YakDriver YakDriver left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

--- PASS: TestAccAWSElasticacheReplicationGroup_clusteringAndCacheNodesCausesError (5.55s)
--- PASS: TestAccAWSElasticacheReplicationGroup_ClusterMode_Basic (1161.24s)
--- PASS: TestAccAWSElasticacheReplicationGroup_ClusterMode_NonClusteredParameterGroup (1456.18s)
--- PASS: TestAccAWSElasticacheReplicationGroup_enableAtRestEncryption (631.29s)
--- PASS: TestAccAWSElasticacheReplicationGroup_enableAuthTokenTransitEncryption (1400.50s)
--- PASS: TestAccAWSElasticacheReplicationGroup_enableSnapshotting (1396.52s)
--- PASS: TestAccAWSElasticacheReplicationGroup_multiAzInVpc (1013.35s)
--- PASS: TestAccAWSElasticacheReplicationGroup_NumberCacheClusters (1398.35s)
--- PASS: TestAccAWSElasticacheReplicationGroup_redisClusterInVpc2 (1460.76s)
--- PASS: TestAccAWSElasticacheReplicationGroup_tags (1396.92s)
--- PASS: TestAccAWSElasticacheReplicationGroup_updateMaintenanceWindow (845.59s)
--- PASS: TestAccAWSElasticacheReplicationGroup_Uppercase (1400.40s)
--- PASS: TestAccAWSElasticacheReplicationGroup_vpc (672.53s)

@gdavison gdavison merged commit 1291ad7 into master Jan 6, 2021
@gdavison gdavison deleted the b-elasticache-cluster-mode branch January 6, 2021 22:39
@github-actions github-actions bot added this to the v3.23.0 milestone Jan 6, 2021
gdavison added a commit that referenced this pull request Jan 6, 2021
@ghost
Copy link

ghost commented Jan 8, 2021

This has been released in version 3.23.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 Feb 6, 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 Feb 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. service/elasticache Issues and PRs that pertain to the elasticache service. size/M 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.

2 participants