Terraform-aws-eks: add Terraform 0.12 support

[mattmessinger]

I have issues

I'm submitting a...

• [ ] bug report
• [x] feature request
• [ ] support request
• [ ] kudos, thank you, warm fuzzy

What is the current behavior?

Currently you cannot use this module with Terraform >= 0.12.0

There are several errors relating to:
"Error: Unsupported block type"
"Error: Missing resource instance key"

Environment details

• Affected module version:
• OS: NA
• Terraform version: v0.12.0-rc1

[nauxliu]

The v0.12 has been released

[stefansedich]

Is anyone already working on this? would be keep to assist on any efforts if required.

[a7i]

@stefansedich: I have and currently running in multiple environments: dev, stage, prod.

https://github.com/a7i/terraform-aws-eks/tree/feature/tf12upgrade

I would be more than happy to create a PR but I believe we need to discuss how to support backwards compatibility (pre v0.12). If we decide to upgrade, perhaps we can bump the Terraform Module version to >= 12.x.x, let me know your thoughts.

module "eks" {
  source  = "terraform-aws-modules/eks/aws"
  version = "~> 12.0.0"
}

[stefansedich]

Ah great @a7i I will check it out, how did you handle the dependencies like the VPC module? it looked to me yesterday as they still have some changes to make too on their end.

[a7i]

@stefansedich Have also done that one. Though, I need to update the examples in the eks module.

https://github.com/a7i/terraform-aws-vpc/tree/terraform-012

[alex-goncharov]

@a7i there is PR for that too https://github.com/terraform-aws-modules/terraform-aws-vpc/pull/265

[eytanhanig]

The official 0.12 upgrade guide states that "If you are using semantic versioning, such as in a Terraform registry, the updates made by the upgrade tool should be considered a breaking change and published as a new major version" and "for any module using a undocumented workarounds for v0.11 limitations it is unlikely to be possible to both update it for Terraform v0.12 and retain v0.11 compatibility at the same time."

It seems like breaking changes are to be expected so long as we bump the module's major version.

[alex-goncharov]

Another attempt: https://github.com/alex-goncharov/terraform-aws-eks/tree/terraform-0.12

Uses strict typing, examples upgraded and tested with VPC module PR. Also couple of live EKS use it already.

[tjorri]

@alex-goncharov, fyi, you may have caught it already, but the change in Terraform 0.12 with coalesce("","") resulting in a "no non-null, non-empty-string arguments" error (as opposed to be valid in prior versions), especially workers_launch_template_mixed.tf and workers.tf have now issues with targetgroup_arns and enabled_metrics attributes for aws_autoscaling_group as the default values are empty.

[alex-goncharov]

Thanks @tjorri, removed bunch of unnecessary coalesce calls in last commit.

[Vrtak-CZ]

@alex-goncharov I'm just testing your 0.12 branch and experienced this error

module.kube_first.aws_autoscaling_group.workers_launch_template_mixed[0]: Creating...

Error: kube-first-kube-wg-classic-spot20190527114031133700000004: invalid tag attributes: value missing

  on .terraform/modules/kube_first/workers_launch_template_mixed.tf line 3, in resource "aws_autoscaling_group" "workers_launch_template_mixed":
   3: resource "aws_autoscaling_group" "workers_launch_template_mixed" {

I have autoscaling enabled on this worker group

autoscaling_enabled = true
protect_from_scale_in = true

looks like the problem is here https://github.com/alex-goncharov/terraform-aws-eks/blob/0438e4334199d0c6dcc17e5b6a6e58312435db4b/workers.tf#L102-L106

btw thx for your work!

[stefansedich]

Just curious are we going to use this time to fix some of the things that might not be nescessary anymore? like the _count properties for one? I know for sure that I could clean up the way I had to originaly handle tags due to the 0.11.x limitations but it would be a breaking change.

[alex-goncharov]

Our approach is to do minimal upgrade work possible so that we can use 0.12 everywhere and then proceed with other fixes with regular cadence.

[alex-goncharov]

@Vrtak-CZ that does not look like a full error output, could you please check if there is more. Also can you show me your plan, the module call part.

[dirgapeter]

@Vrtak-CZ Adding true as value worked for me:

    {
      "key"                 = "k8s.io/cluster-autoscaler/${aws_eks_cluster.this.name}"
      "value"               = "true"
      "propagate_at_launch" = false
    },

[neutronth]

confirm the issue of @Vrtak-CZ and fixed as @dirgapeter proposes

refers to the https://github.com/terraform-providers/terraform-provider-aws/blob/master/aws/autoscaling_tags.go

func autoscalingTagSchema() *schema.Schema {
    return &schema.Schema{
        Type:     schema.TypeSet,
        Optional: true,
        Elem: &schema.Resource{
            Schema: map[string]*schema.Schema{
                ...
                                ...
                "value": {
                    Type:     schema.TypeString,
                    Required: true,
                },
                                ...
                                ...

I'm not sure that terraform >= 0.12 may check "value" = "" as invalidated or another part filter out this key with empty string value.

[max-rocket-internet]

Are we able to remove some Terraform language hacks with 0.12 also?

For example, all the xx_count variables, having actual lists in the workers group maps for subnets instead if strings with commas etc.

[skang0601]

I really want 0.12 support in this module to get rid of all the ugly hacks but this project may want to hold off (I have a selfish reason) or maintain support for 0.11.

I'm a user of Terragrunt and 0.12 is currently not supported with that tool: https://github.com/gruntwork-io/terragrunt/issues/466.

The impression I got from Hashicorp was that they were expecting most users to stick with 0.11 till the end of the year https://www.reddit.com/r/Terraform/comments/aos3o0/where_is_terraform_012/eg6eol8?utm_source=share&utm_medium=web2x

[eytanhanig]

Why don't we follow the example of the terraform-aws-vpc module, which has upgraded the master branch to Terraform 0.12 and created a separate branch, terraform011, for 0.11 PRs:

Terraform versions

Terraform 0.12. Pin module version to ~> v2.0. Submit pull-requests to master branch.

Terraform 0.11. Pin module version to ~> v1.0. Submit pull-requests to terraform011 branch.

[stefansedich]

@max-rocket-internet agreed I think this effort should push towards a 2.x release and a full port/cleanup performed instead of just chasing an easy 0.12 migration, easier to bite the breaking changes with the first 2.x release.

Edit: well maybe 5.x in the case of this module, forget latest was at 4.x

[nauxliu]

@alex-goncharov autoscaling_enabled is not working on your branch, I have opened a PR, please take a look.

[timboven]

@alex-goncharov running a destroy is also giving lots of issues related to coalesce:

Error: Error in function call

on .terraform/modules/tenant-cluster/local.tf line 11, in locals:
11: cluster_iam_role_name = coalesce(
12:
13:
14:
|----------------
| aws_iam_role.cluster is empty tuple
| var.cluster_iam_role_name is ""

Call to function "coalesce" failed: no non-null, non-empty-string arguments.

Error: Error in function call

on .terraform/modules/tenant-cluster/local.tf line 15, in locals:
15: cluster_iam_role_arn = coalesce(
16:
17:
18:
|----------------
| aws_iam_role.cluster is empty tuple
| data.aws_iam_role.custom_cluster_iam_role is empty tuple

Call to function "coalesce" failed: no non-null, non-empty-string arguments.

Error: Error in function call

on .terraform/modules/tenant-cluster/outputs.tf line 120, in output "worker_iam_role_name":
120: coalescelist(
121:
122:
123:
124:
125:
|----------------
| aws_iam_role.workers is empty tuple
| data.aws_iam_instance_profile.custom_worker_group_iam_instance_profile is empty tuple
| data.aws_iam_instance_profile.custom_worker_group_launch_template_iam_instance_profile is empty tuple
| data.aws_iam_instance_profile.custom_worker_group_launch_template_mixed_iam_instance_profile is empty tuple

Call to function "coalescelist" failed: no non-null arguments.

Error: Error in function call

on .terraform/modules/tenant-cluster/outputs.tf line 133, in output "worker_iam_role_arn":
133: coalescelist(
134:
135:
136:
137:
138:
|----------------
| aws_iam_role.workers is empty tuple
| data.aws_iam_instance_profile.custom_worker_group_iam_instance_profile is empty tuple
| data.aws_iam_instance_profile.custom_worker_group_launch_template_iam_instance_profile is empty tuple
| data.aws_iam_instance_profile.custom_worker_group_launch_template_mixed_iam_instance_profile is empty tuple

Call to function "coalescelist" failed: no non-null arguments.

[timboven]

@alex-goncharov I have opened alex-goncharov/terraform-aws-eks/pull/2 to fix these things

[willejs]

The only thing I ask is that anyone who attempts this makes sure it's backwards compatible, and the refactoring does not cause breaking changes causing destructive terraform diffs. If removing the 'hacks' re-orders lists and causes destructive diffs we should avoid this and leave the hacks in on the first pass. I am happy to help test this, or even help with the refactoring. :-)

[max-rocket-internet]

The only thing I ask is that anyone who attempts this makes sure it's backwards compatible, and the refactoring does not cause breaking changes causing destructive terraform diffs

Should be possible 👍

Why don't we follow the example of the terraform-aws-vpc module, which has upgraded the master branch to Terraform 0.12

Sounds good 👍

and created a separate branch, terraform011

Can do that, for sure. But it may not even necessary to support 0.11 still. The module seems pretty stable over the 1-3 months. We would only need to start a terraform011 branch if there was some feature or bugfix that was urgent for any remaining 0.11 users.

[chenrui333]

@mattmessinger I think you can update the terraform version to v0.12.1 instead of v0.12.0-rc1

Cheers!

[aholbreich]

I could not understand out ouf your discussion.
what is the workround to get it work with terafrom 0.12? Can anyone tell?

terraform plan

Error: Unsupported block type

  on .terraform/modules/eks/terraform-aws-modules-terraform-aws-eks-d6fa9f4/aws_auth.tf line 40, in data "template_file" "launch_template_mixed_worker_role_arns":
  40:   vars {

Blocks of type "vars" are not expected here. Did you mean to define argument
"vars"? If so, use the equals sign to assign it a value.

[toughrogrammer]

I could not understand out ouf your discussion.
what is the workround to get it work with terafrom 0.12? Can anyone tell?

terraform plan

Error: Unsupported block type

  on .terraform/modules/eks/terraform-aws-modules-terraform-aws-eks-d6fa9f4/aws_auth.tf line 40, in data "template_file" "launch_template_mixed_worker_role_arns":
  40:   vars {

Blocks of type "vars" are not expected here. Did you mean to define argument
"vars"? If so, use the equals sign to assign it a value.

I forgave to use terraform 0.12 cause that problem.

[alex-goncharov]

Thanks @timboven and @nauxliu, PRs are merged.

[nauxliu]

@alex-goncharov I have using your branch to manage two EKS cluster for a week and it works with no problem. I think it's time to open a PR to merge back to this repo.

[stefansedich]

@alex-goncharov FYI I have done a test migration for our stack over to using your fork and everything appears to be working correctly with what I can see no destructive changes when looking at the plan, just a bunch of "1" => "true" in tags and ASG values.

[max-rocket-internet]

I think it's time to open a PR to merge back to this repo.

Agreed!! Any progress @alex-goncharov?

[eytanhanig]

I'm seeing some weird behavior when I try to tear down a cluster with source = "github.com/alex-goncharov/terraform-aws-eks.git?ref=terraform-0.12":

module.eks.aws_iam_role.cluster[0]: Destruction complete after 0s
module.eks.aws_security_group.cluster[0]: Destruction complete after 2s

Error: Error in function call

  on .terraform/modules/eks/local.tf line 6, in locals:
   6:   cluster_security_group_id = coalesce(
   7:
   8:
   9:
    |----------------
    | aws_security_group.cluster is empty tuple
    | var.cluster_security_group_id is ""

Call to function "coalesce" failed: no non-null, non-empty-string arguments.


Error: Error in function call

  on .terraform/modules/eks/local.tf line 22, in locals:
  22:   worker_security_group_id = coalesce(
  23:
  24:
  25:
    |----------------
    | aws_security_group.workers is empty tuple
    | var.worker_security_group_id is ""

Call to function "coalesce" failed: no non-null, non-empty-string arguments.

This is when using Terraform Enterprise as the backend and remote executor (TF 0.12.2). Has anyone had success creating a fresh cluster from the forked module and then deleting it?

[maposemo]

I used terraform version 0.12.2 but this issue has not been solved.

Error: Unsupported block type

on .terraform/modules/my-cluster/terraform-aws-modules-terraform-aws-eks-d6fa9f4/aws_auth.tf line 40, in data "template_file" "launch_template_mixed_worker_role_arns":
40: vars {

Blocks of type "vars" are not expected here. Did you mean to define argument
"vars"? If so, use the equals sign to assign it a value.

[timboven]

@eytanhanig I have been creating multiple fresh clusters the last couple of days (and destroyed them as well with TF) -- I didn't get those issues anymore but in my tests I'm passing in a security group for the workers - didn't test without passing in a security group (and I assume your error is in that scenario?)

[timboven]

@maposemo I only got your kind of error when I used a older TF version (so not 0.12.x) by accident.
This is a changed syntax in TF 0.12.0 so it should be valid (and actually works in my different test environments as well). Could you validate that you are actually using 0.12.2 in that test and not an older version by accident?

[timboven]

@eytanhanig I have been creating multiple fresh clusters the last couple of days (and destroyed them as well with TF) -- I didn't get those issues anymore but in my tests I'm passing in a security group for the workers - didn't test without passing in a security group (and I assume your error is in that scenario?)

and quickly looking at the code & error -- it also seemed like you didn't let the module create a security group either? (as then I would expect aws_security_group.cluster & aws_security_group.workers to be non-empty ?)

[maposemo]

@timboven
I had these things in my project folder(Ubuntu 16.04).
1. rm -rf .terraform
2. terraform init
Downloading terraform-aws-modules/eks/aws 4.0.2 for my-cluster...
- my-cluster in .terraform/modules/my-cluster/terraform-aws-modules-terraform-aws-eks-d6fa9f4
...
* provider.aws: version = "~> 2.15"
* provider.local: version = "~> 1.2"
* provider.null: version = "~> 2.1"
* provider.template: version = "~> 2.1"

Terraform has been successfully initialized!
3. terraform --version
Terraform v0.12.2
+ provider.aws v2.15.0
+ provider.local v1.2.2
+ provider.null v2.1.2
+ provider.template v2.1.2
4. terraform plan
Error: Unsupported block type

on .terraform/modules/my-cluster/terraform-aws-modules-terraform-aws-eks-d6fa9f4/aws_auth.tf line 40, in data "template_file" "launch_template_mixed_worker_role_arns":
40: vars {

Blocks of type "vars" are not expected here. Did you mean to define argument
"vars"? If so, use the equals sign to assign it a value.
...

[maposemo]

@timboven
This is my-cluster code.

module "my-cluster" {
source = "terraform-aws-modules/eks/aws"
cluster_name = "my-cluster"
subnets = ["subnet-abcde012", "subnet-bcde012a", "subnet-fghi345a"]
vpc_id = "vpc-1234556abcdef"

worker_groups = [
{
instance_type = "m4.large"
asg_max_size = 5
}
]

tags = {
environment = "test"
}
}

[max-rocket-internet]

Everyone, please test this PR so we can merge it 😃
https://github.com/terraform-aws-modules/terraform-aws-eks/pull/394

[eytanhanig]

@eytanhanig I have been creating multiple fresh clusters the last couple of days (and destroyed them as well with TF) -- I didn't get those issues anymore but in my tests I'm passing in a security group for the workers - didn't test without passing in a security group (and I assume your error is in that scenario?)

and quickly looking at the code & error -- it also seemed like you didn't let the module create a security group either? (as then I would expect aws_security_group.cluster & aws_security_group.workers to be non-empty ?)

I did not override the security group creation, and the two aforementioned SGs were created.

[alex-goncharov]

Thanks @max-rocket-internet! I've been travelling last 2 weeks and could not spare a minute.

[max-rocket-internet]

Thanks to all for the help in getting us to TF version 0.12 🎉🎉🎉 Especially @nauxliu and @alex-goncharov 💙