From 150d227c99d517366b9304663a6fdc55b0bb8475 Mon Sep 17 00:00:00 2001 From: Connor Bell Date: Thu, 30 Sep 2021 14:51:19 +0100 Subject: [PATCH] fix: `instance_types` from a Set to a List, so instance order preference is preserved (#1154) * Preserve ordering of chosen instance types * Switch from for_each to count due to for_each not supporting lists for iterating * Missed a each.value * Format launch template list in the order we specify * typo * Missed a change from set to list * Remove unneeded change --- README.md | 21 +++------------------ modules/runners/README.md | 29 +++-------------------------- modules/runners/main.tf | 8 ++++---- modules/runners/variables.tf | 2 +- variables.tf | 2 +- 5 files changed, 12 insertions(+), 50 deletions(-) diff --git a/README.md b/README.md index ead428f14d..2edffdbec7 100644 --- a/README.md +++ b/README.md @@ -343,23 +343,6 @@ No requirements. | aws | n/a | | random | n/a | -## Modules - -| Name | Source | Version | -|------|--------|---------| -| runner_binaries | ./modules/runner-binaries-syncer | | -| runners | ./modules/runners | | -| ssm | ./modules/ssm | | -| webhook | ./modules/webhook | | - -## Resources - -| Name | -|------| -| [aws_resourcegroups_group](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/resourcegroups_group) | -| [aws_sqs_queue](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/sqs_queue) | -| [random_string](https://registry.terraform.io/providers/hashicorp/random/latest/docs/resources/string) | - ## Inputs | Name | Description | Type | Default | Required | @@ -380,7 +363,7 @@ No requirements. | idle\_config | List of time period that can be defined as cron expression to keep a minimum amount of runners active instead of scaling down to 0. By defining this list you can ensure that in time periods that match the cron expression within 5 seconds a runner is kept idle. | 
list(object({
    cron      = string
    timeZone  = string
    idleCount = number
  }))
| `[]` | no | | instance\_profile\_path | The path that will be added to the instance\_profile, if not set the environment name will be used. | `string` | `null` | no | | instance\_type | [DEPRECATED] See instance\_types. | `string` | `"m5.large"` | no | -| instance\_types | List of instance types for the action runner. | `set(string)` | `null` | no | +| instance\_types | List of instance types for the action runner. | `list(string)` | `null` | no | | key\_name | Key pair name | `string` | `null` | no | | kms\_key\_arn | Optional CMK Key ARN to be used for Parameter Store. This key must be in the current account. | `string` | `null` | no | | lambda\_s3\_bucket | S3 bucket from which to specify lambda functions. This is an alternative to providing local files directly. | `any` | `null` | no | @@ -398,6 +381,7 @@ No requirements. | runner\_binaries\_syncer\_lambda\_timeout | Time out of the binaries sync lambda in seconds. | `number` | `300` | no | | runner\_binaries\_syncer\_lambda\_zip | File location of the binaries sync lambda zip file. | `string` | `null` | no | | runner\_boot\_time\_in\_minutes | The minimum time for an EC2 runner to boot and register as a runner. | `number` | `5` | no | +| runner\_egress\_rules | List of egress rules for the GitHub runner instances. | 
list(object({
    cidr_blocks      = list(string)
    ipv6_cidr_blocks = list(string)
    prefix_list_ids  = list(string)
    from_port        = number
    protocol         = string
    security_groups  = list(string)
    self             = bool
    to_port          = number
    description      = string
  }))
| 
[
  {
    "cidr_blocks": [
      "0.0.0.0/0"
    ],
    "description": null,
    "from_port": 0,
    "ipv6_cidr_blocks": [
      "::/0"
    ],
    "prefix_list_ids": null,
    "protocol": "-1",
    "security_groups": null,
    "self": null,
    "to_port": 0
  }
]
| no | | runner\_extra\_labels | Extra labels for the runners (GitHub). Separate each label by a comma | `string` | `""` | no | | runner\_group\_name | Name of the runner group. | `string` | `"Default"` | no | | runner\_iam\_role\_managed\_policy\_arns | Attach AWS or customer-managed IAM policies (by ARN) to the runner IAM role | `list(string)` | `[]` | no | @@ -431,6 +415,7 @@ No requirements. | runners | n/a | | ssm\_parameters | n/a | | webhook | n/a | + ## Contribution diff --git a/modules/runners/README.md b/modules/runners/README.md index 280b5a1300..752ca37093 100644 --- a/modules/runners/README.md +++ b/modules/runners/README.md @@ -58,31 +58,6 @@ No requirements. |------|---------| | aws | n/a | -## Modules - -No Modules. - -## Resources - -| Name | -|------| -| [aws_ami](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/ami) | -| [aws_caller_identity](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/caller_identity) | -| [aws_cloudwatch_event_rule](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudwatch_event_rule) | -| [aws_cloudwatch_event_target](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudwatch_event_target) | -| [aws_cloudwatch_log_group](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudwatch_log_group) | -| [aws_iam_instance_profile](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_instance_profile) | -| [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](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy) | -| [aws_iam_role_policy_attachment](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | -| [aws_lambda_event_source_mapping](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lambda_event_source_mapping) | -| [aws_lambda_function](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lambda_function) | -| [aws_lambda_permission](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lambda_permission) | -| [aws_launch_template](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/launch_template) | -| [aws_security_group](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group) | -| [aws_ssm_parameter](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ssm_parameter) | - ## Inputs | Name | Description | Type | Default | Required | @@ -93,6 +68,7 @@ No Modules. | block\_device\_mappings | The EC2 instance block device configuration. Takes the following keys: `device_name`, `delete_on_termination`, `volume_type`, `volume_size`, `encrypted`, `iops` | `map(string)` | `{}` | no | | cloudwatch\_config | (optional) Replaces the module default cloudwatch log config. See https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch-Agent-Configuration-File-Details.html for details. | `string` | `null` | no | | create\_service\_linked\_role\_spot | (optional) create the service linked role for spot instances that is required by the scale-up lambda. | `bool` | `false` | no | +| egress\_rules | List of egress rules for the GitHub runner instances. | 
list(object({
    cidr_blocks      = list(string)
    ipv6_cidr_blocks = list(string)
    prefix_list_ids  = list(string)
    from_port        = number
    protocol         = string
    security_groups  = list(string)
    self             = bool
    to_port          = number
    description      = string
  }))
| 
[
  {
    "cidr_blocks": [
      "0.0.0.0/0"
    ],
    "description": null,
    "from_port": 0,
    "ipv6_cidr_blocks": [
      "::/0"
    ],
    "prefix_list_ids": null,
    "protocol": "-1",
    "security_groups": null,
    "self": null,
    "to_port": 0
  }
]
| no | | enable\_cloudwatch\_agent | Enabling the cloudwatch agent on the ec2 runner instances, the runner contains default config. Configuration can be overridden via `cloudwatch_config`. | `bool` | `true` | no | | enable\_organization\_runners | n/a | `bool` | n/a | yes | | enable\_ssm\_on\_runners | Enable to allow access to the runner instances for debugging purposes via SSM. Note that this adds additional permissions to the runner instances. | `bool` | n/a | yes | @@ -102,7 +78,7 @@ No Modules. | idle\_config | List of time period that can be defined as cron expression to keep a minimum amount of runners active instead of scaling down to 0. By defining this list you can ensure that in time periods that match the cron expression within 5 seconds a runner is kept idle. | 
list(object({
    cron      = string
    timeZone  = string
    idleCount = number
  }))
| `[]` | no | | instance\_profile\_path | The path that will be added to the instance\_profile, if not set the environment name will be used. | `string` | `null` | no | | instance\_type | [DEPRECATED] See instance\_types. | `string` | `"m5.large"` | no | -| instance\_types | List of instance types for the action runner. | `set(string)` | `null` | no | +| instance\_types | List of instance types for the action runner. | `list(string)` | `null` | no | | key\_name | Key pair name | `string` | `null` | no | | kms\_key\_arn | Optional CMK Key ARN to be used for Parameter Store. | `string` | `null` | no | | lambda\_s3\_bucket | S3 bucket from which to specify lambda functions. This is an alternative to providing local files directly. | `any` | `null` | no | @@ -150,6 +126,7 @@ No Modules. | role\_runner | n/a | | role\_scale\_down | n/a | | role\_scale\_up | n/a | + ## Philips Forest diff --git a/modules/runners/main.tf b/modules/runners/main.tf index dc8694b773..5c510c3d3c 100644 --- a/modules/runners/main.tf +++ b/modules/runners/main.tf @@ -18,7 +18,7 @@ locals { userdata_arm_patch = "${path.module}/templates/arm-runner-patch.tpl" userdata_install_config_runner = "${path.module}/templates/install-config-runner.sh" - instance_types = var.instance_types == null ? [var.instance_type] : var.instance_types + instance_types = distinct(var.instance_types == null ? [var.instance_type] : var.instance_types) kms_key_arn = var.kms_key_arn != null ? var.kms_key_arn : "" } @@ -38,9 +38,9 @@ data "aws_ami" "runner" { } resource "aws_launch_template" "runner" { - for_each = local.instance_types + count = length(local.instance_types) - name = "${var.environment}-action-runner-${each.value}" + name = "${var.environment}-action-runner-${local.instance_types[count.index]}" dynamic "block_device_mappings" { for_each = [var.block_device_mappings] @@ -72,7 +72,7 @@ resource "aws_launch_template" "runner" { } image_id = data.aws_ami.runner.id - instance_type = each.value + instance_type = local.instance_types[count.index] key_name = var.key_name vpc_security_group_ids = compact(concat( diff --git a/modules/runners/variables.tf b/modules/runners/variables.tf index bc3f6be0c0..eed34f89e1 100644 --- a/modules/runners/variables.tf +++ b/modules/runners/variables.tf @@ -65,7 +65,7 @@ variable "instance_type" { variable "instance_types" { description = "List of instance types for the action runner." - type = set(string) + type = list(string) default = null } diff --git a/variables.tf b/variables.tf index fca140b685..6b9c9fb1a8 100644 --- a/variables.tf +++ b/variables.tf @@ -351,7 +351,7 @@ variable "volume_size" { variable "instance_types" { description = "List of instance types for the action runner." - type = set(string) + type = list(string) default = null }