Skip to content

Commit

Permalink
fix: Correction enable_enable_fifo_build_queue (#2857)
Browse files Browse the repository at this point in the history
fix: Variable enable_enable_fifo_build_queue -> enable_enable_fifo_build_queue

Co-authored-by: GuptaNavdeep1983 <navdeep.gupta@philips.com>
  • Loading branch information
npalm and GuptaNavdeep1983 authored Jan 6, 2023
1 parent 84411cb commit 455e272
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 11 deletions.
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ You can configure runners to be ephemeral, runners will be used only for one job
- The scale down lambda is still active, and should only remove orphan instances. But there is no strict check in place. So ensure you configure the `minimum_running_time_in_minutes` to a value that is high enough to got your runner booted and connected to avoid it got terminated before executing a job.
- The messages sent from the webhook lambda to scale-up lambda are by default delayed delayed by SQS, to give available runners to option to start the job before the decision is made to scale more runners. For ephemeral runners there is no need to wait. Set `delay_webhook_event` to `0`.
- All events on the queue will lead to a new runner crated by the lambda. By setting `enable_job_queued_check` to `true` you can enforce only create a runner if the event has a correlated queued job. Setting this can avoid creating useless runners, for example whn jobs got cancelled before a runner is created. We suggest to use this in combination with a pool.
- To ensure runners are created in the same order GitHub sends the events we use by default a FIFO queue, this is mainly relevant for repo level runners. For ephemeral runners you can set `enable_enable_fifo_build_queue` to `false`.
- To ensure runners are created in the same order GitHub sends the events we use by default a FIFO queue, this is mainly relevant for repo level runners. For ephemeral runners you can set `enable_fifo_build_queue` to `false`.
- Error related to scaling should be retried via SQS. You can configure `job_queue_retention_in_seconds` `redrive_build_queue` to tune the behavior. We have no mechanism to avoid events will never processed, which means potential no runner could be created and the job in GitHub can time out in 6 hours.

The example for [ephemeral runners](./examples/ephemeral) is based on the [default example](./examples/default). Have look on the diff to see the major configuration differences.
Expand Down Expand Up @@ -455,8 +455,9 @@ We welcome any improvement to the standard module to make the default as secure
| <a name="input_delay_webhook_event"></a> [delay\_webhook\_event](#input\_delay\_webhook\_event) | The number of seconds the event accepted by the webhook is invisible on the queue before the scale up lambda will receive the event. | `number` | `30` | no |
| <a name="input_disable_runner_autoupdate"></a> [disable\_runner\_autoupdate](#input\_disable\_runner\_autoupdate) | Disable the auto update of the github runner agent. Be-aware there is a grace period of 30 days, see also the [GitHub article](https://github.blog/changelog/2022-02-01-github-actions-self-hosted-runners-can-now-disable-automatic-updates/) | `bool` | `false` | no |
| <a name="input_enable_cloudwatch_agent"></a> [enable\_cloudwatch\_agent](#input\_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 |
| <a name="input_enable_enable_fifo_build_queue"></a> [enable\_enable\_fifo\_build\_queue](#input\_enable\_enable\_fifo\_build\_queue) | Enable a FIFO queue to remain the order of events received by the webhook. Suggest to set to true for repo level runners. | `bool` | `false` | no |
| <a name="input_enable_enable_fifo_build_queue"></a> [enable\_enable\_fifo\_build\_queue](#input\_enable\_enable\_fifo\_build\_queue) | DEPCRECATED: Replaced by `enable_fifo_build_queue` / `fifo_build_queue`. | `string` | `null` | no |
| <a name="input_enable_ephemeral_runners"></a> [enable\_ephemeral\_runners](#input\_enable\_ephemeral\_runners) | Enable ephemeral runners, runners will only be used once. | `bool` | `false` | no |
| <a name="input_enable_fifo_build_queue"></a> [enable\_fifo\_build\_queue](#input\_enable\_fifo\_build\_queue) | Enable a FIFO queue to remain the order of events received by the webhook. Suggest to set to true for repo level runners. | `bool` | `false` | no |
| <a name="input_enable_job_queued_check"></a> [enable\_job\_queued\_check](#input\_enable\_job\_queued\_check) | Only scale if the job event received by the scale up lambda is is in the state queued. By default enabled for non ephemeral runners and disabled for ephemeral. Set this variable to overwrite the default behavior. | `bool` | `null` | no |
| <a name="input_enable_managed_runner_security_group"></a> [enable\_managed\_runner\_security\_group](#input\_enable\_managed\_runner\_security\_group) | Enabling the default managed security group creation. Unmanaged security groups can be specified via `runner_additional_security_group_ids`. | `bool` | `true` | no |
| <a name="input_enable_organization_runners"></a> [enable\_organization\_runners](#input\_enable\_organization\_runners) | Register runners to organization, instead of repo level | `bool` | `false` | no |
Expand Down
2 changes: 1 addition & 1 deletion examples/arm64/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ module "runners" {
runners_maximum_count = 1

# set up a fifo queue to remain order
enable_enable_fifo_build_queue = true
enable_fifo_build_queue = true

# override scaling down
scale_down_schedule_expression = "cron(* * * * ? *)"
Expand Down
2 changes: 1 addition & 1 deletion examples/default/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ module "runners" {
runners_maximum_count = 1

# set up a fifo queue to remain order
enable_enable_fifo_build_queue = true
enable_fifo_build_queue = true

# override scaling down
scale_down_schedule_expression = "cron(* * * * ? *)"
Expand Down
12 changes: 6 additions & 6 deletions main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ resource "aws_sqs_queue_policy" "webhook_events_workflow_job_queue_policy" {
}

resource "aws_sqs_queue" "queued_builds" {
name = "${var.prefix}-queued-builds${var.enable_enable_fifo_build_queue ? ".fifo" : ""}"
name = "${var.prefix}-queued-builds${var.enable_fifo_build_queue ? ".fifo" : ""}"
delay_seconds = var.delay_webhook_event
visibility_timeout_seconds = var.runners_scale_up_lambda_timeout
message_retention_seconds = var.job_queue_retention_in_seconds
fifo_queue = var.enable_enable_fifo_build_queue
fifo_queue = var.enable_fifo_build_queue
receive_wait_time_seconds = 0
content_based_deduplication = var.enable_enable_fifo_build_queue
content_based_deduplication = var.enable_fifo_build_queue
redrive_policy = var.redrive_build_queue.enabled ? jsonencode({
deadLetterTargetArn = aws_sqs_queue.queued_builds_dlq[0].arn,
maxReceiveCount = var.redrive_build_queue.maxReceiveCount
Expand Down Expand Up @@ -104,12 +104,12 @@ resource "aws_sqs_queue_policy" "build_queue_dlq_policy" {

resource "aws_sqs_queue" "queued_builds_dlq" {
count = var.redrive_build_queue.enabled ? 1 : 0
name = "${var.prefix}-queued-builds_dead_letter${var.enable_enable_fifo_build_queue ? ".fifo" : ""}"
name = "${var.prefix}-queued-builds_dead_letter${var.enable_fifo_build_queue ? ".fifo" : ""}"

sqs_managed_sse_enabled = var.queue_encryption.sqs_managed_sse_enabled
kms_master_key_id = var.queue_encryption.kms_master_key_id
kms_data_key_reuse_period_seconds = var.queue_encryption.kms_data_key_reuse_period_seconds
fifo_queue = var.enable_enable_fifo_build_queue
fifo_queue = var.enable_fifo_build_queue
tags = var.tags
}

Expand All @@ -133,7 +133,7 @@ module "webhook" {
(aws_sqs_queue.queued_builds.id) = {
id : aws_sqs_queue.queued_builds.id
arn : aws_sqs_queue.queued_builds.arn
fifo : var.enable_enable_fifo_build_queue
fifo : var.enable_fifo_build_queue
matcherConfig : {
labelMatchers : [split(",", local.runner_labels)]
exactMatch : var.enable_runner_workflow_job_labels_check_all
Expand Down
11 changes: 11 additions & 0 deletions variables.deprecated.tf
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,14 @@ variable "fifo_build_queue" {
error_message = "DEPCRECATED, replaced by `enable_fifo_build_queue`."
}
}

variable "enable_enable_fifo_build_queue" {
description = "DEPCRECATED: Replaced by `enable_fifo_build_queue` / `fifo_build_queue`."
type = string
default = null

validation {
condition = anytrue([var.enable_enable_fifo_build_queue == null])
error_message = "DEPCRECATED, replaced by `enable_fifo_build_queue`."
}
}
2 changes: 1 addition & 1 deletion variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ variable "lambda_principals" {
default = []
}

variable "enable_enable_fifo_build_queue" {
variable "enable_fifo_build_queue" {
description = "Enable a FIFO queue to remain the order of events received by the webhook. Suggest to set to true for repo level runners."
type = bool
default = false
Expand Down

0 comments on commit 455e272

Please sign in to comment.