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

aws_elasticache_cluster does not allow duplicate availability_zones values #2658

Closed
cep21 opened this issue Dec 13, 2017 · 8 comments · Fixed by #4741
Closed

aws_elasticache_cluster does not allow duplicate availability_zones values #2658

cep21 opened this issue Dec 13, 2017 · 8 comments · Fixed by #4741
Labels
bug Addresses a defect in current functionality. service/elasticache Issues and PRs that pertain to the elasticache service.
Milestone

Comments

@cep21
Copy link
Contributor

cep21 commented Dec 13, 2017

Terraform Version

< terraform -v
Terraform v0.11.1

  • provider.aws v1.5.0
  • provider.template v1.0.0

Affected Resource(s)

Please list the resources as a list, for example:

  • aws_elasticache_cluster

Terraform Configuration Files

resource "aws_elasticache_cluster" "my_cache" {
  cluster_id           = "..."
  engine               = "memcached"
  node_type            = "cache.r4.2xlarge"
  port                 = 11211
  num_cache_nodes      = "6"
  parameter_group_name = "default.memcached1.4"
  security_group_ids   = ["..."]
  subnet_group_name    = "..."
  availability_zones   = ["us-west-2a", "us-west-2b", "us-west-2c", "us-west-2a", "us-west-2b", "us-west-2c"]
}

Expected Behavior

Create a cluster in 6 AZ.

Actual Behavior

InvalidParameterCombination: Must specify the same number of preferred availability zones as requested number of nodes

Steps to Reproduce

Create a cluster with the same AZ name multiple times inside availability_zones

Important Factoids

I think something is deduping the availability_zones list, when it shouldn't be deduped. The plan step shows

  availability_zones.#:          "" => "3"
  availability_zones.2050015877: "" => "us-west-2c"
  availability_zones.221770259:  "" => "us-west-2b"
  availability_zones.2487133097: "" => "us-west-2a"

Even though the list size is 6.

Passing in just 3, unique availability_zones values will complain that the passed in list size isn't equal to num_cache_nodes value (6 in this case).

Or the logic should be smart enough to take a list that is only 3 AZ, but have some knowledge of how to distribute them.

@apparentlymart
Copy link
Contributor

Hi @cep21!

It looks like that attribute is defined as being a set rather than a list, which means it's an unordered collection without duplicates. So indeed you are right that it is being deduped within Terraform's type system, due to the data type specified here.

It looks like #2685 is already proposing the solution to this, if we think that preserving order and allowing duplicates is the better behavior here. I am not personally familiar enough with ElastiCache to know what's best here, but I did spend some time reading the relevant documentation and I think I see what happened here:

The documentation for PreferredAvailabilityZones mentions that the order is "not important", which is usually our trigger for considering something to be a set since that then avoids Terraform detecting unnecessary changes when the list is reordered in configuration.

However, later on the same documentation discusses the idea of repeating the same availability zone multiple times to put all the nodes in the same availability zone, suggesting that duplicates are in fact expected. It doesn't discuss the possibility of having a mixed list with several different zones but some duplicated, but I'd agree that this seems like a logical extension of the recommendation about a homogeneous list of zones.

The Terraform team is about to begin a break for the holidays, so hopefully we can take a closer look at PR #2685 in the new year.

@cep21
Copy link
Contributor Author

cep21 commented Dec 20, 2017

@apparentlymart

What the endpoint is trying to do, is you specify 5 servers, then you specify which AZ each those 5 servers go into. The order doens't matter, because if you list us-west-2a first or last, you still get a server in us-west-2a. But it's not a set, it has to allow duplicates because you may want 2 servers in us-west-2a, and only one in us-west-2b. That list would look like [us-west-2a, us-west-2a, us-west-2b] or it could look like [us-west-2a, us-west-2b, us-west-2a].

The correct behavior for terraform would be to sort the list first, then compare for changes.

@radeksimko radeksimko added the service/elasticache Issues and PRs that pertain to the elasticache service. label Jan 28, 2018
@cep21
Copy link
Contributor Author

cep21 commented Apr 25, 2018

@bflad I saw your comment on #2685 Does your change fix this too?

@bflad
Copy link
Contributor

bflad commented Apr 25, 2018

It will -- I do need to actually submit the PR sometime shortly that will introduce the new attribute (e.g. ordered_availability_zones) that will properly implement this as a list and deprecate the old attribute.

@bflad
Copy link
Contributor

bflad commented Jun 4, 2018

Pull request submitted: #4741

@bflad bflad modified the milestones: v1.22.0, v1.23.0 Jun 6, 2018
@bflad
Copy link
Contributor

bflad commented Jun 13, 2018

The aws_elasticache_cluster resource in version 1.23.0 of the AWS provider will have the current availability_zones argument (TypeSet) deprecated in favor of a new preferred_availability_zones argument (TypeList).

@bflad
Copy link
Contributor

bflad commented Jun 14, 2018

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

@ghost
Copy link

ghost commented Apr 5, 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 5, 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/elasticache Issues and PRs that pertain to the elasticache service.
Projects
None yet
4 participants