Skip to content

Commit

Permalink
fix: instance_types from a Set to a List, so instance order prefere…
Browse files Browse the repository at this point in the history
…nce 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
  • Loading branch information
Makeshift authored Sep 30, 2021
1 parent c8b8139 commit 150d227
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 50 deletions.
21 changes: 3 additions & 18 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand All @@ -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. | <pre>list(object({<br> cron = string<br> timeZone = string<br> idleCount = number<br> }))</pre> | `[]` | 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 |
Expand All @@ -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. | <pre>list(object({<br> cidr_blocks = list(string)<br> ipv6_cidr_blocks = list(string)<br> prefix_list_ids = list(string)<br> from_port = number<br> protocol = string<br> security_groups = list(string)<br> self = bool<br> to_port = number<br> description = string<br> }))</pre> | <pre>[<br> {<br> "cidr_blocks": [<br> "0.0.0.0/0"<br> ],<br> "description": null,<br> "from_port": 0,<br> "ipv6_cidr_blocks": [<br> "::/0"<br> ],<br> "prefix_list_ids": null,<br> "protocol": "-1",<br> "security_groups": null,<br> "self": null,<br> "to_port": 0<br> }<br>]</pre> | 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 |
Expand Down Expand Up @@ -431,6 +415,7 @@ No requirements.
| runners | n/a |
| ssm\_parameters | n/a |
| webhook | n/a |

<!-- END OF PRE-COMMIT-TERRAFORM DOCS HOOK -->

## Contribution
Expand Down
29 changes: 3 additions & 26 deletions modules/runners/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand All @@ -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. | <pre>list(object({<br> cidr_blocks = list(string)<br> ipv6_cidr_blocks = list(string)<br> prefix_list_ids = list(string)<br> from_port = number<br> protocol = string<br> security_groups = list(string)<br> self = bool<br> to_port = number<br> description = string<br> }))</pre> | <pre>[<br> {<br> "cidr_blocks": [<br> "0.0.0.0/0"<br> ],<br> "description": null,<br> "from_port": 0,<br> "ipv6_cidr_blocks": [<br> "::/0"<br> ],<br> "prefix_list_ids": null,<br> "protocol": "-1",<br> "security_groups": null,<br> "self": null,<br> "to_port": 0<br> }<br>]</pre> | 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 |
Expand All @@ -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. | <pre>list(object({<br> cron = string<br> timeZone = string<br> idleCount = number<br> }))</pre> | `[]` | 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 |
Expand Down Expand Up @@ -150,6 +126,7 @@ No Modules.
| role\_runner | n/a |
| role\_scale\_down | n/a |
| role\_scale\_up | n/a |

<!-- END OF PRE-COMMIT-TERRAFORM DOCS HOOK -->

## Philips Forest
Expand Down
8 changes: 4 additions & 4 deletions modules/runners/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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 : ""
}
Expand All @@ -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]
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion modules/runners/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down

0 comments on commit 150d227

Please sign in to comment.