-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Upgrade to terraform 0.12 #394
Upgrade to terraform 0.12 #394
Conversation
coalesce(lookup(map, key, ""), default) -> lookup(map, key, default)
* always set a value for these tags * fix tag value * fix tag value * default element available * added default value * added a general default without this default - TF is throwing an error when running a destroy
https://github.com/terraform-aws-modules/terraform-aws-vpc/releases/tag/v2.6.0 was released this morning, so you might want to update the example. |
Also, possibly an unfair ask, but #338 adds some very useful functionality to v0.11 that would be super helpful in v0.12. |
CHANGELOG.md
Outdated
@@ -18,6 +18,8 @@ project adheres to [Semantic Versioning](http://semver.org/). | |||
|
|||
### Changed | |||
|
|||
- Finally, Terraform 0.12 support (by @alex-goncharov @nauxliu @timboven) | |||
- All the xx_count variables have been removed (by @nauxliu) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💙
@@ -1,4 +1,5 @@ | |||
data "aws_region" "current" {} | |||
data "aws_region" "current" { | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does terraform fmt
in 0.12 make this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
terraform 0.12upgrade
does
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
terraform fmt
for 0.12.2
appears to be satisfied with either syntax. terraform -0.12upgrade
has some unexpected opinions, like ending each file with a double line break, which terraform fmt
also appears neutral on.
Some travis config is broken I think as it says |
@max-rocket-internet Fixed, Ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me!
Thanks @nauxliu 🎉 It re-ordered the tags on all my resources but that was all. I'm using one Let's get some more people to test before merging though. |
Tested, works - like @max-rocket-internet I saw some reordering of tags, but nothing destructive. I was previously using the alex-goncharov fork. |
Fair warning: I'm a newbie when it comes to terraform. I'm getting some errors here in some recently-inherited code with this fork:
Those two lines are in the module declaration, and appear to be the dreaded |
@mathuin The upgrade guide has been added to the issue description. |
@mathuin @max-rocket-internet @markcornick |
Thanks @nauxliu for taking care of this! |
Thans @nauxliu, works for me. |
@nauxliu This is a little off-topic but is there any simple way I can use this in my existing code right now? I tried with |
@cawilliamson this code works for me:
Ensure you commented out the |
@nauxliu Everything appears to be working! |
This branch worked perfectly for met to spin up a new cluster. I didn't exercise all of the different options, but the basic use case is definitely working. |
I can confirm it's working here too! @nauxliu nice work! |
Used this branch to upgrade to Terraform 0.12, felt some uneasiness over these lines in the plan: module.k8s1.module.eks_cluster.local_file.config_map_aws_auth[0] must be replaced But everything worked out okay. |
I think this is OK. I have these in version control and the contents didn't actually change. |
OK I've done one more test and it looks good to me. I'll merge 🚀 |
Keep in mind this is a breaking change, not a big fan of forcing everybody to terraform 0.12 as not everybody can easily update to this version |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Description
Add Terraform 0.12 support, resolve #376
This is a fork of alex-goncharov's fork and resolved conflicts
Upgrade Guide
Remove
xx_count
variables from your codeReplace strings with commas with lists
Affected attributes:
example:
Update worker group tags
Update worker_groups_launch_template_mixed override instance types