From fdd44c8f385fa8af83ff737827dcb3404fe3db59 Mon Sep 17 00:00:00 2001 From: Max Williams Date: Tue, 25 Sep 2018 12:30:56 +0200 Subject: [PATCH] Cosmetic fixes (#131) * changing syntax when referring to map keys without lookup function * Replacing map function with actual maps for easier reading * replacing map function in example * replacing map function in workers.tf and readme/main * update changelog --- CHANGELOG.md | 1 + README.md | 2 +- data.tf | 6 +-- examples/eks_test_fixture/main.tf | 71 +++++++++++++++++-------------- main.tf | 2 +- workers.tf | 48 +++++++++++---------- 6 files changed, 70 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cbb1dd..4ce9a5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ project adheres to [Semantic Versioning](http://semver.org/). - A subtle but thoughtful change. (Boomshakalaka, @self 🏀) - fix default worker subnets not working (by @erks) - fix default worker autoscaling_enabled not working (by @erks) +- Cosmetic syntax changes to improve readability. (by @max-rocket-internet) - add `protect_from_scale_in` to solve issue #134 (by @kinghajj) ## [[v1.6.0](https://github.com/terraform-aws-modules/terraform-aws-eks/compare/v1.5.0...v1.6.0)] - 2018-09-04] diff --git a/README.md b/README.md index 63f0227..05666de 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,7 @@ module "eks" { source = "terraform-aws-modules/eks/aws" cluster_name = "test-eks-cluster" subnets = ["subnet-abcde012", "subnet-bcde012a"] - tags = "${map("Environment", "test")}" + tags = {Environment = "test"} vpc_id = "vpc-abcde012" } ``` diff --git a/data.tf b/data.tf index b66c19e..29da467 100644 --- a/data.tf +++ b/data.tf @@ -77,8 +77,8 @@ data "template_file" "userdata" { cluster_name = "${aws_eks_cluster.this.name}" endpoint = "${aws_eks_cluster.this.endpoint}" cluster_auth_base64 = "${aws_eks_cluster.this.certificate_authority.0.data}" - pre_userdata = "${lookup(var.worker_groups[count.index], "pre_userdata",lookup(local.workers_group_defaults, "pre_userdata"))}" - additional_userdata = "${lookup(var.worker_groups[count.index], "additional_userdata",lookup(local.workers_group_defaults, "additional_userdata"))}" - kubelet_extra_args = "${lookup(var.worker_groups[count.index], "kubelet_extra_args",lookup(local.workers_group_defaults, "kubelet_extra_args"))}" + pre_userdata = "${lookup(var.worker_groups[count.index], "pre_userdata", local.workers_group_defaults["pre_userdata"])}" + additional_userdata = "${lookup(var.worker_groups[count.index], "additional_userdata", local.workers_group_defaults["additional_userdata"])}" + kubelet_extra_args = "${lookup(var.worker_groups[count.index], "kubelet_extra_args", local.workers_group_defaults["kubelet_extra_args"])}" } } diff --git a/examples/eks_test_fixture/main.tf b/examples/eks_test_fixture/main.tf index a1d959b..22babda 100644 --- a/examples/eks_test_fixture/main.tf +++ b/examples/eks_test_fixture/main.tf @@ -18,39 +18,46 @@ locals { # the commented out worker group list below shows an example of how to define # multiple worker groups of differing configurations - # worker_groups = "${list( - # map("asg_desired_capacity", "2", - # "asg_max_size", "10", - # "asg_min_size", "2", - # "instance_type", "m4.xlarge", - # "name", "worker_group_a", - # "subnets", "${join(",", module.vpc.private_subnets)}", - # ), - # map("asg_desired_capacity", "1", - # "asg_max_size", "5", - # "asg_min_size", "1", - # "instance_type", "m4.2xlarge", - # "name", "worker_group_b", - # "subnets", "${join(",", module.vpc.private_subnets)}", - # ), - # )}" + # worker_groups = [ + # { + # asg_desired_capacity = 2 + # asg_max_size = 10 + # asg_min_size = 2 + # instance_type = "m4.xlarge" + # name = "worker_group_a" + # additional_userdata = "echo foo bar" + # subnets = "${join(",", module.vpc.private_subnets)}" + # }, + # { + # asg_desired_capacity = 1 + # asg_max_size = 5 + # asg_min_size = 1 + # instance_type = "m4.2xlarge" + # name = "worker_group_b" + # additional_userdata = "echo foo bar" + # subnets = "${join(",", module.vpc.private_subnets)}" + # }, + # ] - worker_groups = "${list( - map("instance_type","t2.small", - "additional_userdata","echo foo bar", - "subnets", "${join(",", module.vpc.private_subnets)}", - ), - map("instance_type","t2.small", - "additional_userdata","echo foo bar", - "subnets", "${join(",", module.vpc.private_subnets)}", - "additional_security_group_ids", "${aws_security_group.worker_group_mgmt_one.id},${aws_security_group.worker_group_mgmt_two.id}" - ) - )}" - tags = "${map("Environment", "test", - "GithubRepo", "terraform-aws-eks", - "GithubOrg", "terraform-aws-modules", - "Workspace", "${terraform.workspace}", - )}" + worker_groups = [ + { + instance_type = "t2.small" + additional_userdata = "echo foo bar" + subnets = "${join(",", module.vpc.private_subnets)}" + }, + { + instance_type = "t2.small" + additional_userdata = "echo foo bar" + subnets = "${join(",", module.vpc.private_subnets)}" + additional_security_group_ids = "${aws_security_group.worker_group_mgmt_one.id},${aws_security_group.worker_group_mgmt_two.id}" + }, + ] + tags = { + Environment = "test" + GithubRepo = "terraform-aws-eks" + GithubOrg = "terraform-aws-modules" + Workspace = "${terraform.workspace}" + } } resource "random_string" "suffix" { diff --git a/main.tf b/main.tf index 872eec3..c4d5887 100644 --- a/main.tf +++ b/main.tf @@ -27,7 +27,7 @@ * source = "terraform-aws-modules/eks/aws" * cluster_name = "test-eks-cluster" * subnets = ["subnet-abcde012", "subnet-bcde012a"] -* tags = "${map("Environment", "test")}" +* tags = {Environment = "test"} * vpc_id = "vpc-abcde012" * } * ``` diff --git a/workers.tf b/workers.tf index a0b4756..fb3c4aa 100644 --- a/workers.tf +++ b/workers.tf @@ -1,18 +1,20 @@ resource "aws_autoscaling_group" "workers" { + + name_prefix = "${aws_eks_cluster.this.name}-${lookup(var.worker_groups[count.index], "name", count.index)}" - desired_capacity = "${lookup(var.worker_groups[count.index], "asg_desired_capacity", lookup(local.workers_group_defaults, "asg_desired_capacity"))}" - max_size = "${lookup(var.worker_groups[count.index], "asg_max_size",lookup(local.workers_group_defaults, "asg_max_size"))}" - min_size = "${lookup(var.worker_groups[count.index], "asg_min_size",lookup(local.workers_group_defaults, "asg_min_size"))}" + desired_capacity = "${lookup(var.worker_groups[count.index], "asg_desired_capacity", local.workers_group_defaults["asg_desired_capacity"])}" + max_size = "${lookup(var.worker_groups[count.index], "asg_max_size", local.workers_group_defaults["asg_max_size"])}" + min_size = "${lookup(var.worker_groups[count.index], "asg_min_size", local.workers_group_defaults["asg_min_size"])}" launch_configuration = "${element(aws_launch_configuration.workers.*.id, count.index)}" - vpc_zone_identifier = ["${split(",", coalesce(lookup(var.worker_groups[count.index], "subnets", ""), lookup(local.workers_group_defaults, "subnets")))}"] + vpc_zone_identifier = ["${split(",", coalesce(lookup(var.worker_groups[count.index], "subnets", ""), local.workers_group_defaults["subnets"]))}"] + protect_from_scale_in = "${lookup(var.worker_groups[count.index], "protect_from_scale_in", local.workers_group_defaults["protect_from_scale_in"])}" count = "${var.worker_group_count}" - protect_from_scale_in = "${lookup(var.worker_groups[count.index], "protect_from_scale_in", lookup(local.workers_group_defaults, "protect_from_scale_in"))}" tags = ["${concat( list( map("key", "Name", "value", "${aws_eks_cluster.this.name}-${lookup(var.worker_groups[count.index], "name", count.index)}-eks_asg", "propagate_at_launch", true), map("key", "kubernetes.io/cluster/${aws_eks_cluster.this.name}", "value", "owned", "propagate_at_launch", true), - map("key", "k8s.io/cluster-autoscaler/${lookup(var.worker_groups[count.index], "autoscaling_enabled", lookup(local.workers_group_defaults, "autoscaling_enabled")) == 1 ? "enabled" : "disabled" }", "value", "true", "propagate_at_launch", false), + map("key", "k8s.io/cluster-autoscaler/${lookup(var.worker_groups[count.index], "autoscaling_enabled", local.workers_group_defaults["autoscaling_enabled"]) == 1 ? "enabled" : "disabled" }", "value", "true", "propagate_at_launch", false), ), local.asg_tags) }"] @@ -24,16 +26,16 @@ resource "aws_autoscaling_group" "workers" { resource "aws_launch_configuration" "workers" { name_prefix = "${aws_eks_cluster.this.name}-${lookup(var.worker_groups[count.index], "name", count.index)}" - associate_public_ip_address = "${lookup(var.worker_groups[count.index], "public_ip", lookup(local.workers_group_defaults, "public_ip"))}" - security_groups = ["${local.worker_security_group_id}", "${var.worker_additional_security_group_ids}", "${compact(split(",",lookup(var.worker_groups[count.index],"additional_security_group_ids",lookup(local.workers_group_defaults, "additional_security_group_ids"))))}"] - iam_instance_profile = "${element(aws_iam_instance_profile.workers.*.id, count.index)}" - image_id = "${lookup(var.worker_groups[count.index], "ami_id", lookup(local.workers_group_defaults, "ami_id"))}" - instance_type = "${lookup(var.worker_groups[count.index], "instance_type", lookup(local.workers_group_defaults, "instance_type"))}" - key_name = "${lookup(var.worker_groups[count.index], "key_name", lookup(local.workers_group_defaults, "key_name"))}" + associate_public_ip_address = "${lookup(var.worker_groups[count.index], "public_ip", local.workers_group_defaults["public_ip"])}" + security_groups = ["${local.worker_security_group_id}", "${var.worker_additional_security_group_ids}", "${compact(split(",",lookup(var.worker_groups[count.index],"additional_security_group_ids", local.workers_group_defaults["additional_security_group_ids"])))}"] + iam_instance_profile = "${aws_iam_instance_profile.workers.id}" + image_id = "${lookup(var.worker_groups[count.index], "ami_id", local.workers_group_defaults["ami_id"])}" + instance_type = "${lookup(var.worker_groups[count.index], "instance_type", local.workers_group_defaults["instance_type"])}" + key_name = "${lookup(var.worker_groups[count.index], "key_name", local.workers_group_defaults["key_name"])}" user_data_base64 = "${base64encode(element(data.template_file.userdata.*.rendered, count.index))}" - ebs_optimized = "${lookup(var.worker_groups[count.index], "ebs_optimized", lookup(local.ebs_optimized, lookup(var.worker_groups[count.index], "instance_type", lookup(local.workers_group_defaults, "instance_type")), false))}" - enable_monitoring = "${lookup(var.worker_groups[count.index], "enable_monitoring", lookup(local.workers_group_defaults, "enable_monitoring"))}" - spot_price = "${lookup(var.worker_groups[count.index], "spot_price", lookup(local.workers_group_defaults, "spot_price"))}" + ebs_optimized = "${lookup(var.worker_groups[count.index], "ebs_optimized", lookup(local.ebs_optimized, lookup(var.worker_groups[count.index], "instance_type", local.workers_group_defaults["instance_type"]), false))}" + enable_monitoring = "${lookup(var.worker_groups[count.index], "enable_monitoring", local.workers_group_defaults["enable_monitoring"])}" + spot_price = "${lookup(var.worker_groups[count.index], "spot_price", local.workers_group_defaults["spot_price"])}" count = "${var.worker_group_count}" lifecycle { @@ -41,9 +43,9 @@ resource "aws_launch_configuration" "workers" { } root_block_device { - volume_size = "${lookup(var.worker_groups[count.index], "root_volume_size", lookup(local.workers_group_defaults, "root_volume_size"))}" - volume_type = "${lookup(var.worker_groups[count.index], "root_volume_type", lookup(local.workers_group_defaults, "root_volume_type"))}" - iops = "${lookup(var.worker_groups[count.index], "root_iops", lookup(local.workers_group_defaults, "root_iops"))}" + volume_size = "${lookup(var.worker_groups[count.index], "root_volume_size", local.workers_group_defaults["root_volume_size"])}" + volume_type = "${lookup(var.worker_groups[count.index], "root_volume_type", local.workers_group_defaults["root_volume_type"])}" + iops = "${lookup(var.worker_groups[count.index], "root_iops", local.workers_group_defaults["root_iops"])}" delete_on_termination = true } } @@ -119,11 +121,11 @@ resource "aws_iam_role_policy_attachment" "workers_AmazonEC2ContainerRegistryRea resource "null_resource" "tags_as_list_of_maps" { count = "${length(keys(var.tags))}" - triggers = "${map( - "key", "${element(keys(var.tags), count.index)}", - "value", "${element(values(var.tags), count.index)}", - "propagate_at_launch", "true" - )}" + triggers = { + key = "${element(keys(var.tags), count.index)}" + value = "${element(values(var.tags), count.index)}" + propagate_at_launch = "true" + } } resource "aws_iam_role_policy_attachment" "workers_autoscaling" {