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

[WIP, NeedHelp] resource/aws_elasticache_cluster: Change availability_zones from Set to List #2685

Closed
wants to merge 1 commit into from
Closed

[WIP, NeedHelp] resource/aws_elasticache_cluster: Change availability_zones from Set to List #2685

wants to merge 1 commit into from

Conversation

atsushi-ishibashi
Copy link
Contributor

@atsushi-ishibashi atsushi-ishibashi commented Dec 16, 2017 •
edited
Loading

Fix: #2658

TF_ACC=1 go test ./aws -v -run=TestAccAWSElasticacheCluster_multiAZInVpc -timeout 120m
=== RUN   TestAccAWSElasticacheCluster_multiAZInVpc
--- PASS: TestAccAWSElasticacheCluster_multiAZInVpc (1039.93s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1039.959s

・TODO

  • Sort list to avoid update
  • State migration because this will cause breaking-change.

@jen20 jen20 added the bug Addresses a defect in current functionality. label Dec 17, 2017
@apparentlymart apparentlymart mentioned this pull request Dec 20, 2017
Closed
cep21
cep21 reviewed Dec 20, 2017
View reviewed changes
aws/resource_aws_elasticache_cluster_test.go
port = 11211
subnet_group_name = "${aws_elasticache_subnet_group.bar.name}"
security_group_ids = ["${aws_security_group.bar.id}"]
parameter_group_name = "default.memcached1.4"
az_mode = "cross-az"
availability_zones = [
"us-west-2a",
"us-west-2b"
"us-west-2b",
Copy link
Contributor

Choose a reason for hiding this comment

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

The order does not matter. Does Terraform have a type where it sorts the list first before comparing for changes (for example, if this list changes to "2a 2b 2a" from "2a 2a 2b", that's not actually something that needs to be updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cep21 Sorting list is good idea to avoid not necessary update. Thank you!

@atsushi-ishibashi
Copy link
Contributor Author

atsushi-ishibashi commented Dec 30, 2017 •
edited
Loading

Are there anyone who have an idea to sort list before set, like StateFunc?

@atsushi-ishibashi atsushi-ishibashi changed the title r/aws_elasticache_cluster: Change type Set to List [WIP, NeedHelp]r/aws_elasticache_cluster: Change type Set to List Jan 3, 2018
@radeksimko radeksimko added the service/elasticache Issues and PRs that pertain to the elasticache service. label Jan 16, 2018
@radeksimko radeksimko changed the title [WIP, NeedHelp]r/aws_elasticache_cluster: Change type Set to List [WIP, NeedHelp] resource/aws_elasticache_cluster: Change type Set to List Jan 16, 2018
@radeksimko radeksimko changed the title [WIP, NeedHelp] resource/aws_elasticache_cluster: Change type Set to List [WIP, NeedHelp] resource/aws_elasticache_cluster: Change availability_zones from Set to List Jan 16, 2018
@radeksimko radeksimko added the breaking-change Introduces a breaking change in current functionality; usually deferred to the next major release. label Feb 8, 2018
@bflad
Copy link
Contributor

bflad commented Apr 25, 2018

@atsushi-ishibashi thanks for your work here and apologies you got stuck in the same spot here as #1476! 😄 In a similar manner to #1476, I'm going to close this PR in preference of a migration path similar to #4117. For other folks, there is a much longer explanation in that PR. I will make a note to personally get an updated PR in for this resource in v1.17.0 or shortly after.

@bflad bflad closed this Apr 25, 2018
@bflad
Copy link
Contributor

bflad commented Jun 13, 2018

The replacement PR here, #4741, was merged and will release with version 1.23.0 of the AWS provider. It deprecates availability_zones (TypeSet) in favor of a new attribute preferred_availability_zones (TypeList).

@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.
Reviewers

@cep21 cep21 cep21 left review comments

Assignees
No one assigned
Labels
breaking-change Introduces a breaking change in current functionality; usually deferred to the next major release. bug Addresses a defect in current functionality. service/elasticache Issues and PRs that pertain to the elasticache service.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

aws_elasticache_cluster does not allow duplicate availability_zones values
5 participants
@atsushi-ishibashi @bflad @cep21 @radeksimko @jen20