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

support more nodes than there are AZs defined #108

Merged
merged 4 commits into from
Feb 14, 2021

Conversation

gusse
Copy link
Contributor

@gusse gusse commented Feb 10, 2021

what

why

  • availability_zones parameter shouldn't be required.
  • You might want to define the AZs you use and have more nodes than AZs, e.g. use two AZs and 4 nodes

references

@gusse gusse requested review from a team as code owners February 10, 2021 13:56
@@ -86,7 +86,7 @@ resource "aws_elasticache_replication_group" "default" {
number_cache_clusters = var.cluster_mode_enabled ? null : var.cluster_size
port = var.port
parameter_group_name = join("", aws_elasticache_parameter_group.default.*.name)
availability_zones = var.cluster_mode_enabled ? null : slice(var.availability_zones, 0, var.cluster_size)
availability_zones = length(var.availability_zones) == 0 ? null : [for n in range(0, var.cluster_size) : element(var.availability_zones, n)]

Choose a reason for hiding this comment

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

Just curious. Is there a difference between slice(var.availability_zones, 0, var.cluster_size) and [for n in range(0, var.cluster_size) : element(var.availability_zones, n)] ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

element() has this "wrap-around" behavior built in, so if a list has 3 indexes and you ask for the fourth, it will return the first one for you. That enables you to define - for example - var.cluster_size = 3 and var.availability_zones = ["us-east-1a", "us-east-1b"], which would fail with slice() as there are less indexes (2) than you are asking for (3). And element() function returns just one element at a time, thus the for loop and range() to populate the list.

https://www.terraform.io/docs/language/functions/element.html
https://www.terraform.io/docs/language/functions/slice.html

@SweetOps
Copy link
Contributor

/test all

@SweetOps
Copy link
Contributor

/rebuild readme

@SweetOps
Copy link
Contributor

/rebuild-readme

@gusse gusse requested a review from a team as a code owner February 11, 2021 11:25
@gusse gusse requested review from jhosteny and woz5999 and removed request for a team February 11, 2021 11:25
@SweetOps
Copy link
Contributor

/test readme

@SweetOps
Copy link
Contributor

/test all

@SweetOps SweetOps changed the title Fix for #63 support more nodes than there are AZs defined Feb 11, 2021
@SweetOps SweetOps added the patch A minor, backward compatible change label Feb 14, 2021
@SweetOps
Copy link
Contributor

@Mergifyio update

@mergify
Copy link
Contributor

mergify bot commented Feb 14, 2021

Command update: success

Branch has been successfully updated

@SweetOps
Copy link
Contributor

/test all

@SweetOps SweetOps merged commit 250f4d2 into cloudposse:master Feb 14, 2021
@ppapishe
Copy link

ppapishe commented Jul 9, 2021

@SweetOps @gusse , this still fails for me. How we do check if this test worked?

@minota87
Copy link

minota87 commented Feb 28, 2022

@hawkesn ^^

brian-weis-msr pushed a commit to Measurabl/terraform-aws-elasticache-redis that referenced this pull request Apr 2, 2024
* allow empty list of availability_zones

availability_zones parameter shouldn't be required

* enable more nodes to be created than AZs

* Updated README.md

Co-authored-by: actions-bot <58130806+actions-bot@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch A minor, backward compatible change
Projects
None yet
6 participants