Skip to content

Commit 6d767ca

Browse files
committed
fix: Remove fifo queues
1 parent d1e407f commit 6d767ca

File tree

13 files changed

+23
-60
lines changed

13 files changed

+23
-60
lines changed

examples/arm64/main.tf

-3
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,6 @@ module "runners" {
8080
delay_webhook_event = 5
8181
runners_maximum_count = 1
8282

83-
# set up a fifo queue to remain order
84-
enable_fifo_build_queue = true
85-
8683
# override scaling down
8784
scale_down_schedule_expression = "cron(* * * * ? *)"
8885
}

examples/multi-runner/main.tf

-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ module "runners" {
4747
# labelMatchers = [["self-hosted", "linux", "x64", "amazon"]]
4848
# exactMatch = false
4949
# }
50-
# fifo = true
5150
# delay_webhook_event = 0
5251
# runner_config = {
5352
# runner_os = "linux"

lambdas/functions/control-plane/src/local.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ const sqsEvent = {
2828
messageAttributes: {},
2929
md5OfBody: '4aef3bd70526e152e86426a0938cbec6',
3030
eventSource: 'aws:sqs',
31-
eventSourceARN: 'arn:aws:sqs:us-west-2:916370655143:cicddev-queued-builds.fifo',
31+
eventSourceARN: 'arn:aws:sqs:us-west-2:916370655143:cicddev-queued-builds',
3232
awsRegion: 'us-west-2',
3333
},
3434
],

lambdas/functions/webhook/jest.config.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ const config: Config = {
66
...defaultConfig,
77
coverageThreshold: {
88
global: {
9-
statements: 99.13,
9+
statements: 99.19,
1010
branches: 96.87,
1111
functions: 100,
12-
lines: 99.09,
12+
lines: 99.16,
1313
},
1414
},
1515
};

lambdas/functions/webhook/src/sqs/index.test.ts

+3-26
Original file line numberDiff line numberDiff line change
@@ -27,49 +27,26 @@ describe('Test sending message to SQS.', () => {
2727
repositoryName: 'test',
2828
repositoryOwner: 'owner',
2929
queueId: queueUrl,
30-
queueFifo: false,
3130
repoOwnerType: 'Organization',
3231
};
3332

3433
afterEach(() => {
3534
jest.clearAllMocks();
3635
});
3736

38-
it('no fifo queue', async () => {
37+
it('should queue a message', async () => {
3938
// Arrange
40-
const no_fifo_message: ActionRequestMessage = {
41-
...message,
42-
queueFifo: false,
43-
};
4439
const sqsMessage: SendMessageCommandInput = {
4540
QueueUrl: queueUrl,
46-
MessageBody: JSON.stringify(no_fifo_message),
41+
MessageBody: JSON.stringify(message),
4742
};
4843
// Act
49-
const result = await sendActionRequest(no_fifo_message);
44+
const result = await sendActionRequest(message);
5045

5146
// Assert
5247
expect(mockSQS.sendMessage).toBeCalledWith(sqsMessage);
5348
expect(result).resolves;
5449
});
55-
56-
it('use a fifo queue', async () => {
57-
// Arrange
58-
const fifo_message: ActionRequestMessage = {
59-
...message,
60-
queueFifo: true,
61-
};
62-
const sqsMessage: SendMessageCommandInput = {
63-
QueueUrl: queueUrl,
64-
MessageBody: JSON.stringify(fifo_message),
65-
};
66-
// Act
67-
const result = await sendActionRequest(fifo_message);
68-
69-
// Assert
70-
expect(mockSQS.sendMessage).toBeCalledWith({ ...sqsMessage, MessageGroupId: String(message.id) });
71-
expect(result).resolves;
72-
});
7350
});
7451

7552
describe('Test sending message to SQS.', () => {

lambdas/functions/webhook/src/sqs/index.ts

-5
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ export interface ActionRequestMessage {
1212
repositoryOwner: string;
1313
installationId: number;
1414
queueId: string;
15-
queueFifo: boolean;
1615
repoOwnerType: string;
1716
}
1817

@@ -27,7 +26,6 @@ export interface RunnerMatcherConfig {
2726
matcherConfig: MatcherConfig;
2827
id: string;
2928
arn: string;
30-
fifo: boolean;
3129
}
3230

3331
export interface GithubWorkflowEvent {
@@ -43,9 +41,6 @@ export const sendActionRequest = async (message: ActionRequestMessage): Promise<
4341
};
4442

4543
logger.debug(`sending message to SQS: ${JSON.stringify(sqsMessage)}`);
46-
if (message.queueFifo) {
47-
sqsMessage.MessageGroupId = String(message.id);
48-
}
4944

5045
await sqs.sendMessage(sqsMessage);
5146
};

lambdas/functions/webhook/src/webhook/index.test.ts

-3
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,6 @@ describe('handler', () => {
449449
eventType: 'workflow_job',
450450
installationId: 0,
451451
queueId: 'ubuntu-queue-id',
452-
queueFifo: false,
453452
repoOwnerType: 'Organization',
454453
});
455454
});
@@ -492,7 +491,6 @@ describe('handler', () => {
492491
eventType: 'workflow_job',
493492
installationId: 0,
494493
queueId: 'ubuntu-queue-id',
495-
queueFifo: false,
496494
repoOwnerType: 'Organization',
497495
});
498496
});
@@ -532,7 +530,6 @@ describe('handler', () => {
532530
eventType: 'workflow_job',
533531
installationId: 0,
534532
queueId: 'ubuntu-queue-id',
535-
queueFifo: false,
536533
repoOwnerType: 'Organization',
537534
});
538535
});

lambdas/functions/webhook/src/webhook/index.ts

-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ async function handleWorkflowJob(
5252
eventType: githubEvent,
5353
installationId: installationId,
5454
queueId: queue.id,
55-
queueFifo: queue.fifo,
5655
repoOwnerType: body.repository.owner.type,
5756
});
5857
logger.info(`Successfully queued job for ${body.repository.full_name} to the queue ${queue.id}`);

main.tf

+2-3
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ resource "aws_sqs_queue_policy" "webhook_events_workflow_job_queue_policy" {
5858
}
5959

6060
resource "aws_sqs_queue" "queued_builds" {
61-
name = "${var.prefix}-queued-builds${var.enable_fifo_build_queue ? ".fifo" : ""}"
61+
name = "${var.prefix}-queued-builds"
6262
delay_seconds = var.delay_webhook_event
6363
visibility_timeout_seconds = var.runners_scale_up_lambda_timeout
6464
message_retention_seconds = var.job_queue_retention_in_seconds
@@ -103,7 +103,7 @@ resource "aws_sqs_queue_policy" "build_queue_dlq_policy" {
103103

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

108108
sqs_managed_sse_enabled = var.queue_encryption.sqs_managed_sse_enabled
109109
kms_master_key_id = var.queue_encryption.kms_master_key_id
@@ -135,7 +135,6 @@ module "webhook" {
135135
(aws_sqs_queue.queued_builds.id) = {
136136
id : aws_sqs_queue.queued_builds.id
137137
arn : aws_sqs_queue.queued_builds.arn
138-
fifo : var.enable_fifo_build_queue
139138
matcherConfig : {
140139
labelMatchers : [local.runner_labels]
141140
exactMatch : var.enable_runner_workflow_job_labels_check_all

modules/multi-runner/queues.tf

+7-10
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,12 @@ data "aws_iam_policy_document" "deny_unsecure_transport" {
2727
}
2828

2929
resource "aws_sqs_queue" "queued_builds" {
30-
for_each = var.multi_runner_config
31-
name = "${var.prefix}-${each.key}-queued-builds${each.value.fifo ? ".fifo" : ""}"
32-
delay_seconds = each.value.runner_config.delay_webhook_event
33-
visibility_timeout_seconds = var.runners_scale_up_lambda_timeout
34-
message_retention_seconds = each.value.runner_config.job_queue_retention_in_seconds
35-
fifo_queue = each.value.fifo
36-
receive_wait_time_seconds = 0
37-
content_based_deduplication = each.value.fifo
30+
for_each = var.multi_runner_config
31+
name = "${var.prefix}-${each.key}-queued-builds"
32+
delay_seconds = each.value.runner_config.delay_webhook_event
33+
visibility_timeout_seconds = var.runners_scale_up_lambda_timeout
34+
message_retention_seconds = each.value.runner_config.job_queue_retention_in_seconds
35+
receive_wait_time_seconds = 0
3836
redrive_policy = each.value.redrive_build_queue.enabled ? jsonencode({
3937
deadLetterTargetArn = aws_sqs_queue.queued_builds_dlq[each.key].arn,
4038
maxReceiveCount = each.value.redrive_build_queue.maxReceiveCount
@@ -55,12 +53,11 @@ resource "aws_sqs_queue_policy" "build_queue_policy" {
5553

5654
resource "aws_sqs_queue" "queued_builds_dlq" {
5755
for_each = { for config, values in var.multi_runner_config : config => values if values.redrive_build_queue.enabled }
58-
name = "${var.prefix}-${each.key}-queued-builds_dead_letter${each.value.fifo ? ".fifo" : ""}"
56+
name = "${var.prefix}-${each.key}-queued-builds_dead_letter"
5957

6058
sqs_managed_sse_enabled = var.queue_encryption.sqs_managed_sse_enabled
6159
kms_master_key_id = var.queue_encryption.kms_master_key_id
6260
kms_data_key_reuse_period_seconds = var.queue_encryption.kms_data_key_reuse_period_seconds
63-
fifo_queue = each.value.fifo
6461
tags = var.tags
6562
}
6663

modules/multi-runner/variables.tf

-1
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,6 @@ variable "multi_runner_config" {
125125
exactMatch = optional(bool, false)
126126
priority = optional(number, 999)
127127
})
128-
fifo = optional(bool, false)
129128
redrive_build_queue = optional(object({
130129
enabled = bool
131130
maxReceiveCount = number

modules/webhook/variables.tf

+2-3
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,8 @@ variable "tags" {
2525
variable "runner_matcher_config" {
2626
description = "SQS queue to publish accepted build events based on the runner type. When exact match is disabled the webhook accepts the event if one of the workflow job labels is part of the matcher. The priority defines the order the matchers are applied."
2727
type = map(object({
28-
arn = string
29-
id = string
30-
fifo = bool
28+
arn = string
29+
id = string
3130
matcherConfig = object({
3231
labelMatchers = list(list(string))
3332
exactMatch = bool

variables.tf

+6-1
Original file line numberDiff line numberDiff line change
@@ -630,9 +630,14 @@ variable "lambda_principals" {
630630
}
631631

632632
variable "enable_fifo_build_queue" {
633-
description = "Enable a FIFO queue to keep the order of events received by the webhook. Recommended for repo level runners."
633+
description = "(FEATURE REMOVED) Enable a FIFO queue to keep the order of events received by the webhook. Recommended for repo level runners."
634634
type = bool
635635
default = false
636+
637+
validation {
638+
condition = var.enable_fifo_build_queue == false
639+
error_message = "The feature for FIFO build queue is not supported anymore."
640+
}
636641
}
637642

638643
variable "redrive_build_queue" {

0 commit comments

Comments
 (0)