Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

removed availability_zones from asg #247

Merged
merged 2 commits into from
Nov 19, 2019

Conversation

JoseD92
Copy link
Contributor

@JoseD92 JoseD92 commented Sep 6, 2019

This PR removes availability_zones from the asg module, as https://www.terraform.io/docs/providers/aws/r/autoscaling_group.html says availability_zones and vpc_zone_identifier should not be used together causing issue #242, it is also important to note that availability_zones is required to use EC2-Classic, so this update will make terraform-aws-foundation possibly not work on EC2-Classic

@Magicloud Magicloud requested a review from ketzacoatl September 9, 2019 15:36
Copy link
Contributor

@ketzacoatl ketzacoatl left a comment

Choose a reason for hiding this comment

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

The code change looks fine, but this PR should include some artifact to validate correctness. For example, you can run plan/apply/tests in one or more of the example projects/env. All PRs should include some validation (paste/screenshot/etc) that helps prove to a reviewer that the results expected were actually seen. It's even better when the person testing is different from the implementer, but the validation noted above is critical when the same person is implementing and testing.

@JoseD92
Copy link
Contributor Author

JoseD92 commented Nov 12, 2019

running examples with master branch will result, when applying, in:

...
aws_security_group.sg: Creation complete after 5s [id=sg-044dff8dea12d391c]
module.asg.aws_launch_configuration.cluster: Creating...
module.asg.aws_launch_configuration.cluster: Creation complete after 3s [id=terraform-20191112203922703500000001]

Error: Provider produced inconsistent final plan

When expanding the plan for module.asg.aws_autoscaling_group.cluster to
include new values learned so far during apply, provider "aws" produced an
invalid new value for .availability_zones: was known, but now unknown.

This is a bug in the provider, which should be reported in the provider's own
issue tracker.

Using the change in this PR allows examples to apply correctly taking with the asg's taking the AZ of the subnets used in creating them.

@Magicloud
Copy link
Contributor

Looks good to me. Did not know the PR is still open and I got the issue again.

@qrilka
Copy link
Contributor

qrilka commented Dec 5, 2019

This broke the tests:

qrilka@qdesktop ~/ws/fpco/terraform-aws-foundation/tests $ make plan

Error: Unsupported argument

  on ../modules/consul-leaders/main.tf line 29, in module "leader-asg":
  29:   azs                = ["${var.region}a", "${var.region}c"]

An argument named "azs" is not expected here.

make: *** [Makefile:25: plan] Ошибка 1

@ketzacoatl ketzacoatl deleted the 242-remove-availability_zones-from-asg branch January 9, 2020 12:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants