Fix allow several instance types (#54)

patrickjahns · Nuru · web-flow · commit 61ac9304b3f4 · 2021-02-28T20:01:35.000-08:00
* feat: allow to specify more than a single instance type

* Remove instance list from launch template

Co-authored-by: Nuru &lt;Nuru@users.noreply.github.com&gt;
diff --git a/.github/mergify.yml b/.github/mergify.yml
@@ -56,3 +56,10 @@ pull_request_rules:
       changes_requested: true
       approved: true
       message: "This Pull Request has been updated, so we're dismissing all reviews."
+
+- name: "close Pull Requests without files changed"
+  conditions:
+    - "#files=0"
+  actions:
+    close:
+      message: "This pull request has been automatically closed by Mergify because there are no longer any changes."
diff --git a/.github/workflows/auto-format.yml b/.github/workflows/auto-format.yml
@@ -6,7 +6,7 @@ on:
 jobs:
   auto-format:
     runs-on: ubuntu-latest
-    container: cloudposse/build-harness:slim-latest
+    container: cloudposse/build-harness:latest
     steps:
     # Checkout the pull request branch
     #  "An action in a workflow run can’t trigger a new workflow run. For example, if an action pushes code using
@@ -29,6 +29,8 @@ jobs:
     - name: Auto Format
       if: github.event.pull_request.state == 'open'
       shell: bash
+      env:
+        GITHUB_TOKEN: "${{ secrets.PUBLIC_REPO_ACCESS_TOKEN }}"
       run: make BUILD_HARNESS_PATH=/build-harness PACKAGES_PREFER_HOST=true -f /build-harness/templates/Makefile.build-harness pr/auto-format/host
 
     # Commit changes (if any) to the PR branch
diff --git a/.github/workflows/auto-release.yml b/.github/workflows/auto-release.yml
@@ -3,17 +3,23 @@ name: auto-release
 on:
   push:
     branches:
-    - master
+      - master
 
 jobs:
   publish:
     runs-on: ubuntu-latest
     steps:
-    # Drafts your next Release notes as Pull Requests are merged into "master"
-    - uses: release-drafter/release-drafter@v5
-      with:
-        publish: true
-        prerelease: false
-        config-name: auto-release.yml
-      env:
-        GITHUB_TOKEN: ${{ secrets.PUBLIC_REPO_ACCESS_TOKEN }}
+      # Get PR from merged commit to master
+      - uses: actions-ecosystem/action-get-merged-pull-request@v1
+        id: get-merged-pull-request
+        with:
+          github_token: ${{ secrets.PUBLIC_REPO_ACCESS_TOKEN }}
+      # Drafts your next Release notes as Pull Requests are merged into "master"
+      - uses: release-drafter/release-drafter@v5
+        if: "!contains(steps.get-merged-pull-request.outputs.labels, 'no-release')"
+        with:
+          publish: true
+          prerelease: false
+          config-name: auto-release.yml
+        env:
+          GITHUB_TOKEN: ${{ secrets.PUBLIC_REPO_ACCESS_TOKEN }}
diff --git a/README.md b/README.md
@@ -222,6 +222,31 @@ Available targets:
 | aws | >= 3.0 |
 | random | >= 2.0 |
 
+## Modules
+
+| Name | Source | Version |
+|------|--------|---------|
+| label | cloudposse/label/null | 0.24.1 |
+| this | cloudposse/label/null | 0.24.1 |
+
+## Resources
+
+| Name |
+|------|
+| [aws_ami](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/ami) |
+| [aws_eks_cluster](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/eks_cluster) |
+| [aws_eks_node_group](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/eks_node_group) |
+| [aws_iam_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) |
+| [aws_iam_policy_document](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) |
+| [aws_iam_role](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role) |
+| [aws_iam_role_policy_attachment](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) |
+| [aws_launch_template](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/launch_template) |
+| [aws_launch_template](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/launch_template) |
+| [aws_partition](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/partition) |
+| [aws_security_group](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group) |
+| [aws_security_group_rule](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule) |
+| [random_pet](https://registry.terraform.io/providers/hashicorp/random/latest/docs/resources/pet) |
+
 ## Inputs
 
 | Name | Description | Type | Default | Required |
@@ -234,14 +259,14 @@ Available targets:
 | 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 |
 | bootstrap\_additional\_options | Additional options to bootstrap.sh. DO NOT include `--kubelet-additional-args`, use `kubelet_additional_args` var instead. | `string` | `""` | no |
-| capacity\_type | Type of capacity associated with the EKS Node Group. Valid values: ON\_DEMAND, SPOT. <br>Terraform will only perform drift detection if a configuration value is provided. | `string` | `"ON_DEMAND"` | no |
+| capacity\_type | Type of capacity associated with the EKS Node Group. Valid values: "ON\_DEMAND", "SPOT", or `null`.<br>Terraform will only perform drift detection if a configuration value is provided. | `string` | `null` | no |
 | cluster\_autoscaler\_enabled | Set true to label the node group so that the [Kubernetes Cluster Autoscaler](https://github.com/kubernetes/autoscaler/blob/master/cluster-autoscaler/cloudprovider/aws/README.md#auto-discovery-setup) will discover and autoscale it | `bool` | `null` | no |
 | cluster\_name | The name of the EKS cluster | `string` | n/a | yes |
 | context | Single object for setting entire context at once.<br>See description of individual variables for details.<br>Leave string and numeric variables as `null` to use default value.<br>Individual variable settings (non-null) override settings in context object,<br>except for attributes, tags, and additional\_tag\_map, which are merged. | `any` | <pre>{<br>  "additional_tag_map": {},<br>  "attributes": [],<br>  "delimiter": null,<br>  "enabled": true,<br>  "environment": null,<br>  "id_length_limit": null,<br>  "label_key_case": null,<br>  "label_order": [],<br>  "label_value_case": null,<br>  "name": null,<br>  "namespace": null,<br>  "regex_replace_chars": null,<br>  "stage": null,<br>  "tags": {}<br>}</pre> | no |
 | create\_before\_destroy | Set true in order to create the new node group before destroying the old one.<br>If false, the old node group will be destroyed first, causing downtime.<br>Changing this setting will always cause node group to be replaced. | `bool` | `false` | no |
 | delimiter | Delimiter to be used between `namespace`, `environment`, `stage`, `name` and `attributes`.<br>Defaults to `-` (hyphen). Set to `""` to use no delimiter at all. | `string` | `null` | no |
 | desired\_size | Initial desired number of worker nodes (external changes ignored) | `number` | n/a | yes |
-| disk\_size | Disk size in GiB for worker nodes. Defaults to 20. Ignored it `launch_template_id` is supplied.<br>Terraform will only perform drift detection if a configuration value is provided. | `number` | `20` | no |
+| disk\_size | Disk size in GiB for worker nodes. Defaults to 20. Ignored when `launch_template_id` is supplied.<br>Terraform will only perform drift detection if a configuration value is provided. | `number` | `20` | no |
 | disk\_type | If provided, will be used as volume type of created ebs disk on EC2 instances | `string` | `null` | no |
 | ec2\_ssh\_key | SSH key pair name to use to access the worker nodes | `string` | `null` | no |
 | enable\_cluster\_autoscaler | (Deprecated, use `cluster_autoscaler_enabled`) Set true to allow Kubernetes Cluster Auto Scaler to scale the node group | `bool` | `null` | no |
@@ -250,7 +275,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 (minimum 6).<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 | 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 |
+| instance\_types | Instance types to use for this node group (up to 20). Defaults to ["t3.medium"].<br>Ignored when `launch_template_id` is supplied. | `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 |
@@ -288,7 +313,6 @@ Available targets:
 | eks\_node\_group\_role\_arn | ARN of the worker nodes IAM role |
 | eks\_node\_group\_role\_name | Name of the worker nodes IAM role |
 | eks\_node\_group\_status | Status of the EKS Node Group |
-
 <!-- markdownlint-restore -->
 
 
diff --git a/docs/terraform.md b/docs/terraform.md
@@ -16,6 +16,31 @@
 | aws | >= 3.0 |
 | random | >= 2.0 |
 
+## Modules
+
+| Name | Source | Version |
+|------|--------|---------|
+| label | cloudposse/label/null | 0.24.1 |
+| this | cloudposse/label/null | 0.24.1 |
+
+## Resources
+
+| Name |
+|------|
+| [aws_ami](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/ami) |
+| [aws_eks_cluster](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/eks_cluster) |
+| [aws_eks_node_group](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/eks_node_group) |
+| [aws_iam_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) |
+| [aws_iam_policy_document](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) |
+| [aws_iam_role](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role) |
+| [aws_iam_role_policy_attachment](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) |
+| [aws_launch_template](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/launch_template) |
+| [aws_launch_template](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/launch_template) |
+| [aws_partition](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/partition) |
+| [aws_security_group](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group) |
+| [aws_security_group_rule](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule) |
+| [random_pet](https://registry.terraform.io/providers/hashicorp/random/latest/docs/resources/pet) |
+
 ## Inputs
 
 | Name | Description | Type | Default | Required |
@@ -28,14 +53,14 @@
 | 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 |
 | bootstrap\_additional\_options | Additional options to bootstrap.sh. DO NOT include `--kubelet-additional-args`, use `kubelet_additional_args` var instead. | `string` | `""` | no |
-| capacity\_type | Type of capacity associated with the EKS Node Group. Valid values: ON\_DEMAND, SPOT. <br>Terraform will only perform drift detection if a configuration value is provided. | `string` | `"ON_DEMAND"` | no |
+| capacity\_type | Type of capacity associated with the EKS Node Group. Valid values: "ON\_DEMAND", "SPOT", or `null`.<br>Terraform will only perform drift detection if a configuration value is provided. | `string` | `null` | no |
 | cluster\_autoscaler\_enabled | Set true to label the node group so that the [Kubernetes Cluster Autoscaler](https://github.com/kubernetes/autoscaler/blob/master/cluster-autoscaler/cloudprovider/aws/README.md#auto-discovery-setup) will discover and autoscale it | `bool` | `null` | no |
 | cluster\_name | The name of the EKS cluster | `string` | n/a | yes |
 | context | Single object for setting entire context at once.<br>See description of individual variables for details.<br>Leave string and numeric variables as `null` to use default value.<br>Individual variable settings (non-null) override settings in context object,<br>except for attributes, tags, and additional\_tag\_map, which are merged. | `any` | <pre>{<br>  "additional_tag_map": {},<br>  "attributes": [],<br>  "delimiter": null,<br>  "enabled": true,<br>  "environment": null,<br>  "id_length_limit": null,<br>  "label_key_case": null,<br>  "label_order": [],<br>  "label_value_case": null,<br>  "name": null,<br>  "namespace": null,<br>  "regex_replace_chars": null,<br>  "stage": null,<br>  "tags": {}<br>}</pre> | no |
 | create\_before\_destroy | Set true in order to create the new node group before destroying the old one.<br>If false, the old node group will be destroyed first, causing downtime.<br>Changing this setting will always cause node group to be replaced. | `bool` | `false` | no |
 | delimiter | Delimiter to be used between `namespace`, `environment`, `stage`, `name` and `attributes`.<br>Defaults to `-` (hyphen). Set to `""` to use no delimiter at all. | `string` | `null` | no |
 | desired\_size | Initial desired number of worker nodes (external changes ignored) | `number` | n/a | yes |
-| disk\_size | Disk size in GiB for worker nodes. Defaults to 20. Ignored it `launch_template_id` is supplied.<br>Terraform will only perform drift detection if a configuration value is provided. | `number` | `20` | no |
+| disk\_size | Disk size in GiB for worker nodes. Defaults to 20. Ignored when `launch_template_id` is supplied.<br>Terraform will only perform drift detection if a configuration value is provided. | `number` | `20` | no |
 | disk\_type | If provided, will be used as volume type of created ebs disk on EC2 instances | `string` | `null` | no |
 | ec2\_ssh\_key | SSH key pair name to use to access the worker nodes | `string` | `null` | no |
 | enable\_cluster\_autoscaler | (Deprecated, use `cluster_autoscaler_enabled`) Set true to allow Kubernetes Cluster Auto Scaler to scale the node group | `bool` | `null` | no |
@@ -44,7 +69,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 (minimum 6).<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 | 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 |
+| instance\_types | Instance types to use for this node group (up to 20). Defaults to ["t3.medium"].<br>Ignored when `launch_template_id` is supplied. | `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 |
@@ -82,5 +107,4 @@
 | eks\_node\_group\_role\_arn | ARN of the worker nodes IAM role |
 | eks\_node\_group\_role\_name | Name of the worker nodes IAM role |
 | eks\_node\_group\_status | Status of the EKS Node Group |
-
 <!-- markdownlint-restore -->
diff --git a/launch-template.tf b/launch-template.tf
@@ -71,9 +71,10 @@ resource "aws_launch_template" "default" {
   name_prefix            = module.label.id
   update_default_version = true
 
-  instance_type = var.instance_types[0]
-  image_id      = local.launch_template_ami == "" ? null : local.launch_template_ami
-  key_name      = local.have_ssh_key ? var.ec2_ssh_key : null
+  # Never include instance type in launch template because it is limited to just one
+  # https://docs.aws.amazon.com/eks/latest/APIReference/API_CreateNodegroup.html#API_CreateNodegroup_RequestSyntax
+  image_id = local.launch_template_ami == "" ? null : local.launch_template_ami
+  key_name = local.have_ssh_key ? var.ec2_ssh_key : null
 
   dynamic "tag_specifications" {
     for_each = var.resources_to_tag
diff --git a/main.tf b/main.tf
@@ -57,11 +57,16 @@ data "aws_eks_cluster" "this" {
 locals {
   ng_needs_remote_access = local.have_ssh_key && ! local.use_launch_template
   ng = {
-    cluster_name    = var.cluster_name
-    node_role_arn   = join("", aws_iam_role.default.*.arn)
-    subnet_ids      = var.subnet_ids
-    disk_size       = local.use_launch_template ? null : var.disk_size
-    instance_types  = local.use_launch_template ? null : var.instance_types
+    cluster_name  = var.cluster_name
+    node_role_arn = join("", aws_iam_role.default.*.arn)
+    # Keep sorted so that change in order does not trigger replacement via random_pet
+    subnet_ids = sort(var.subnet_ids)
+    disk_size  = local.use_launch_template ? null : var.disk_size
+    # Always supply instance types via the node group, not the launch template,
+    # because node group supports up to 20 types but launch template does not.
+    # See https://docs.aws.amazon.com/eks/latest/APIReference/API_CreateNodegroup.html#API_CreateNodegroup_RequestSyntax
+    # Keep sorted so that change in order does not trigger replacement via random_pet
+    instance_types  = sort(var.instance_types)
     ami_type        = local.launch_template_ami == "" ? var.ami_type : null
     capacity_type   = var.capacity_type
     labels          = var.kubernetes_labels == null ? {} : var.kubernetes_labels
@@ -77,9 +82,10 @@ locals {
     }
 
     # Configure remote access via Launch Template if we are using one
-    need_remote_access        = local.ng_needs_remote_access
-    ec2_ssh_key               = local.have_ssh_key ? var.ec2_ssh_key : "none"
-    source_security_group_ids = local.ng_needs_remote_access ? var.source_security_group_ids : []
+    need_remote_access = local.ng_needs_remote_access
+    ec2_ssh_key        = local.have_ssh_key ? var.ec2_ssh_key : "none"
+    # Keep sorted so that change in order does not trigger replacement via random_pet
+    source_security_group_ids = local.ng_needs_remote_access ? sort(var.source_security_group_ids) : []
   }
 }
 
@@ -93,7 +99,7 @@ resource "random_pet" "cbd" {
     node_role_arn   = local.ng.node_role_arn
     subnet_ids      = join(",", local.ng.subnet_ids)
     disk_size       = local.ng.disk_size
-    instance_types  = local.ng.instance_types == null ? "" : local.ng.instance_types[0]
+    instance_types  = join(",", local.ng.instance_types)
     ami_type        = local.ng.ami_type
     release_version = local.ng.release_version
     version         = local.ng.version
diff --git a/variables.tf b/variables.tf
