More triggers for replacement, better handling of enabled=false (clou…

…dposse#32)

README.md

@@ -197,7 +197,7 @@ Available targets:
 | additional\_tag\_map | Additional tags for appending to tags\_as\_list\_of\_maps. Not added to `tags`. | `map(string)` | `{}` | no |
 | after\_cluster\_joining\_userdata | Additional `bash` commands to execute on each worker node after joining the EKS cluster (after executing the `bootstrap.sh` script). For more info, see https://kubedex.com/90-days-of-aws-eks-in-production | `string` | `""` | no |
 | ami\_image\_id | AMI to use. Ignored of `launch_template_id` is supplied. | `string` | `null` | no |
-| ami\_release\_version | AMI version to use, e.g. "1.16.13-20200821" (no "v"). Defaults to latest version for Kubernetes version. | `string` | `null` | no |
+| ami\_release\_version | EKS AMI version to use, e.g. "1.16.13-20200821" (no "v"). Defaults to latest version for Kubernetes version. | `string` | `null` | no |
 | ami\_type | Type of Amazon Machine Image (AMI) associated with the EKS Node Group.<br>Defaults to `AL2_x86_64`. Valid values: `AL2_x86_64`, `AL2_x86_64_GPU`, and `AL2_ARM_64`. | `string` | `"AL2_x86_64"` | no |
 | attributes | Additional attributes (e.g. `1`) | `list(string)` | `[]` | no |
 | before\_cluster\_joining\_userdata | Additional `bash` commands to execute on each worker node before joining the EKS cluster (before executing the `bootstrap.sh` script). For more info, see https://kubedex.com/90-days-of-aws-eks-in-production | `string` | `""` | no |
@@ -215,7 +215,7 @@ Available targets:
 | existing\_workers\_role\_policy\_arns | List of existing policy ARNs that will be attached to the workers default role on creation | `list(string)` | `[]` | no |
 | existing\_workers\_role\_policy\_arns\_count | Obsolete and ignored. Allowed for backward compatibility. | `number` | `0` | no |
 | id\_length\_limit | Limit `id` to this many characters.<br>Set to `0` for unlimited length.<br>Set to `null` for default, which is `0`.<br>Does not affect `id_full`. | `number` | `null` | no |
-| instance\_types | Set of instance types associated with the EKS Node Group. Defaults to ["t3.medium"]. Terraform will only perform drift detection if a configuration value is provided | `list(string)` | <pre>[<br>  "t3.medium"<br>]</pre> | no |
+| instance\_types | Single instance type to use for this node group, passed as a list. Defaults to ["t3.medium"].<br>It is a list because Launch Templates take a list, and it is a single type because EKS only supports a single type per node group. | `list(string)` | <pre>[<br>  "t3.medium"<br>]</pre> | no |
 | kubelet\_additional\_options | Additional flags to pass to kubelet.<br>DO NOT include `--node-labels` or `--node-taints`,<br>use `kubernetes_labels` and `kubernetes_taints` to specify those." | `string` | `""` | no |
 | kubernetes\_labels | Key-value mapping of Kubernetes labels. Only labels that are applied with the EKS API are managed by this argument.<br>Other Kubernetes labels applied to the EKS Node Group will not be managed. | `map(string)` | `{}` | no |
 | kubernetes\_taints | Key-value mapping of Kubernetes taints. | `map(string)` | `{}` | no |

ami.tf

@@ -11,7 +11,7 @@ locals {
   // 1. prefix of var.ami_release_version
   // 2. var.kubernetes_version
   // 3. data.eks_cluster.this.kubernetes_version
-  need_cluster_kubernetes_version = local.need_ami_id && length(compact([var.ami_release_version, var.kubernetes_version])) == 0
+  need_cluster_kubernetes_version = local.enabled ? local.need_ami_id && length(compact([var.ami_release_version, var.kubernetes_version])) == 0 : false
 
   ami_kubernetes_version = local.need_ami_id ? (local.need_cluster_kubernetes_version ? data.aws_eks_cluster.this[0].version :
     regex("^(\\d+\\.\\d+)", coalesce(var.ami_release_version, var.kubernetes_version))[0]

docs/terraform.md

@@ -23,7 +23,7 @@
 | additional\_tag\_map | Additional tags for appending to tags\_as\_list\_of\_maps. Not added to `tags`. | `map(string)` | `{}` | no |
 | after\_cluster\_joining\_userdata | Additional `bash` commands to execute on each worker node after joining the EKS cluster (after executing the `bootstrap.sh` script). For more info, see https://kubedex.com/90-days-of-aws-eks-in-production | `string` | `""` | no |
 | ami\_image\_id | AMI to use. Ignored of `launch_template_id` is supplied. | `string` | `null` | no |
-| ami\_release\_version | AMI version to use, e.g. "1.16.13-20200821" (no "v"). Defaults to latest version for Kubernetes version. | `string` | `null` | no |
+| ami\_release\_version | EKS AMI version to use, e.g. "1.16.13-20200821" (no "v"). Defaults to latest version for Kubernetes version. | `string` | `null` | no |
 | ami\_type | Type of Amazon Machine Image (AMI) associated with the EKS Node Group.<br>Defaults to `AL2_x86_64`. Valid values: `AL2_x86_64`, `AL2_x86_64_GPU`, and `AL2_ARM_64`. | `string` | `"AL2_x86_64"` | no |
 | attributes | Additional attributes (e.g. `1`) | `list(string)` | `[]` | no |
 | before\_cluster\_joining\_userdata | Additional `bash` commands to execute on each worker node before joining the EKS cluster (before executing the `bootstrap.sh` script). For more info, see https://kubedex.com/90-days-of-aws-eks-in-production | `string` | `""` | no |
@@ -41,7 +41,7 @@
 | existing\_workers\_role\_policy\_arns | List of existing policy ARNs that will be attached to the workers default role on creation | `list(string)` | `[]` | no |
 | existing\_workers\_role\_policy\_arns\_count | Obsolete and ignored. Allowed for backward compatibility. | `number` | `0` | no |
 | id\_length\_limit | Limit `id` to this many characters.<br>Set to `0` for unlimited length.<br>Set to `null` for default, which is `0`.<br>Does not affect `id_full`. | `number` | `null` | no |
-| instance\_types | Set of instance types associated with the EKS Node Group. Defaults to ["t3.medium"]. Terraform will only perform drift detection if a configuration value is provided | `list(string)` | <pre>[<br>  "t3.medium"<br>]</pre> | no |
+| instance\_types | Single instance type to use for this node group, passed as a list. Defaults to ["t3.medium"].<br>It is a list because Launch Templates take a list, and it is a single type because EKS only supports a single type per node group. | `list(string)` | <pre>[<br>  "t3.medium"<br>]</pre> | no |
 | kubelet\_additional\_options | Additional flags to pass to kubelet.<br>DO NOT include `--node-labels` or `--node-taints`,<br>use `kubernetes_labels` and `kubernetes_taints` to specify those." | `string` | `""` | no |
 | kubernetes\_labels | Key-value mapping of Kubernetes labels. Only labels that are applied with the EKS API are managed by this argument.<br>Other Kubernetes labels applied to the EKS Node Group will not be managed. | `map(string)` | `{}` | no |
 | kubernetes\_taints | Key-value mapping of Kubernetes taints. | `map(string)` | `{}` | no |

main.tf

@@ -6,6 +6,14 @@ locals {
   # This allows us to refer to resources that are only conditionally created and avoid creating
   # dependencies on them that would not be avoided by using expressions like `join("",expr)`.
   #
+  # We use this pattern with enabled for every boolean that begins with `need_` even though
+  # it is sometimes redundant, to ensure that ever `need_` is false and every dependent
+  # expression is not evaluated when enabled is false. Avoiding expression evaluations
+  # is also why, even for boolean expressions, we use
+  #   local.enabled ? expression : false
+  # rather than
+  #   local.enabled && expression
+  #
   # The expression
   #   length(compact([var.launch_template_version])) > 0
   # is a shorter way of accomplishing the same test as
@@ -21,12 +29,12 @@ locals {
   configured_ami_image_id = var.ami_image_id == null ? "" : var.ami_image_id
 
   # See https://aws.amazon.com/blogs/containers/introducing-launch-template-and-custom-ami-support-in-amazon-eks-managed-node-groups/
-  features_require_ami = local.need_bootstrap
-  need_ami_id          = local.features_require_ami && length(local.configured_ami_image_id) == 0
+  features_require_ami = local.enabled && local.need_bootstrap
+  need_ami_id          = local.enabled ? local.features_require_ami && length(local.configured_ami_image_id) == 0 : false
 
-  features_require_launch_template = length(var.resources_to_tag) > 0 || local.need_userdata || local.features_require_ami
-  generate_launch_template         = local.features_require_launch_template && length(local.configured_launch_template_name) == 0
-  use_launch_template              = local.features_require_launch_template || length(local.configured_launch_template_name) > 0
+  features_require_launch_template = local.enabled ? length(var.resources_to_tag) > 0 || local.need_userdata || local.features_require_ami : false
+  generate_launch_template         = local.enabled ? local.features_require_launch_template && length(local.configured_launch_template_name) == 0 : false
+  use_launch_template              = local.enabled ? local.features_require_launch_template || length(local.configured_launch_template_name) > 0 : false
 
   launch_template_id = local.use_launch_template ? (length(local.configured_launch_template_name) > 0 ? data.aws_launch_template.this[0].id : aws_launch_template.default[0].id) : ""
   launch_template_version = local.use_launch_template ? (
@@ -60,7 +68,7 @@ locals {
 
   aws_policy_prefix = format("arn:%s:iam::aws:policy", join("", data.aws_partition.current.*.partition))
 
-  get_cluster_data = local.enabled && (local.need_cluster_kubernetes_version || local.need_bootstrap)
+  get_cluster_data = local.enabled ? (local.need_cluster_kubernetes_version || local.need_bootstrap) : false
 }
 
 data "aws_eks_cluster" "this" {
@@ -220,16 +228,19 @@ resource "random_pet" "cbd" {
   length    = 1
 
   keepers = {
-    ami_type       = var.ami_type
-    disk_size      = local.use_launch_template ? null : var.disk_size
-    instance_types = join(",", local.use_launch_template ? [] : var.instance_types)
-    node_role_arn  = join("", aws_iam_role.default.*.arn)
+    ami_type            = var.ami_type
+    ami_release_version = var.ami_release_version
+    kubernetes_version  = var.kubernetes_version
+    disk_size           = local.use_launch_template ? null : var.disk_size
+    instance_types      = join(",", local.use_launch_template ? [] : var.instance_types)
+    node_role_arn       = join("", aws_iam_role.default.*.arn)
 
     ec2_ssh_key               = var.ec2_ssh_key == null ? "" : var.ec2_ssh_key
     source_security_group_ids = join(",", var.source_security_group_ids)
     subnet_ids                = join(",", var.subnet_ids)
 
-    launch_template_id = local.launch_template_id
+    launch_template_id  = local.launch_template_id
+    launch_template_ami = local.launch_template_ami
   }
 
   depends_on = [var.module_depends_on]

userdata.tf

@@ -40,12 +40,13 @@ locals {
     cluster_name               = local.get_cluster_data ? data.aws_eks_cluster.this[0].name : null
   }
 
-  need_bootstrap = length(compact([local.kubelet_taint_args, var.kubelet_additional_options,
+  need_bootstrap = local.enabled ? length(compact([local.kubelet_taint_args, var.kubelet_additional_options,
     local.userdata_vars.bootstrap_extra_args,
     local.userdata_vars.after_cluster_joining_userdata]
-  )) > 0
+  )) > 0 : false
 
-  need_userdata = (var.userdata_override == null) && (length(local.userdata_vars.before_cluster_joining_userdata) > 0) || local.need_bootstrap
+  # If var.userdata_override = "" then we explicitly set userdata to ""
+  need_userdata = local.enabled && var.userdata_override == null ? (length(local.userdata_vars.before_cluster_joining_userdata) > 0) || local.need_bootstrap : false
 
   userdata = local.need_userdata ? base64encode(templatefile("${path.module}/userdata.tpl", merge(local.userdata_vars, local.cluster_data))) : var.userdata_override
-}
+}

variables.tf

@@ -83,9 +83,11 @@ variable "disk_size" {
 
 variable "instance_types" {
   type        = list(string)
-  description = "Set of instance types associated with the EKS Node Group. Defaults to [\"t3.medium\"]. Terraform will only perform drift detection if a configuration value is provided"
   default     = ["t3.medium"]
-
+  description = <<-EOT
+    Single instance type to use for this node group, passed as a list. Defaults to ["t3.medium"].
+    It is a list because Launch Templates take a list, and it is a single type because EKS only supports a single type per node group.
+    EOT
   validation {
     condition = (
       length(var.instance_types) == 1
@@ -134,7 +136,7 @@ variable "ami_image_id" {
 
 variable "ami_release_version" {
   type        = string
-  description = "AMI version to use, e.g. \"1.16.13-20200821\" (no \"v\"). Defaults to latest version for Kubernetes version."
+  description = "EKS AMI version to use, e.g. \"1.16.13-20200821\" (no \"v\"). Defaults to latest version for Kubernetes version."
   default     = null
   validation {
     condition = (