From 02220d07f81caf0bd51ff5ffaddaf9c508071852 Mon Sep 17 00:00:00 2001 From: Anatoli Tsikhamirau Date: Sat, 12 Sep 2026 20:46:06 +0200 Subject: [PATCH] fix(job-retry): respect ENABLE_JOB_QUEUED_CHECK in retry path The retry Lambda always called the GitHub API to check whether a job was still queued before requeuing it, even when ENABLE_JOB_QUEUED_CHECK is disabled for the main scale-up path. This made it impossible to fully disable the job-status check, since the retry path kept issuing it on every retry attempt and consuming rate-limit budget. Documented the retry lambda now respecting this flag too, in docs/configuration.md and the enable_job_queued_check variable descriptions. --- docs/configuration.md | 2 +- .../src/scale-runners/job-retry.test.ts | 66 +++++++++++++++++++ .../src/scale-runners/job-retry.ts | 6 +- modules/runners/variables.tf | 2 +- variables.tf | 2 +- 5 files changed, 73 insertions(+), 5 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 4948229c27..d74f6a5432 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -164,7 +164,7 @@ You can configure runners to be ephemeral, in which case runners will be used on - 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 get your runner booted and connected to avoid it being terminated before executing a job. - The messages sent from the webhook lambda to the scale-up lambda are by default delayed by SQS, to give available runners a chance 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 in the queue will lead to a new runner created by the lambda. By setting `enable_job_queued_check` to `true` you can enforce a rule of only creating a runner if the event has a correlated queued job. Setting this can avoid creating useless runners. For example, a job getting cancelled before a runner was created or if the job was already picked up by another runner. We suggest using this in combination with a pool. +- All events in the queue will lead to a new runner created by the lambda. By setting `enable_job_queued_check` to `true` you can enforce a rule of only creating a runner if the event has a correlated queued job. Setting this can avoid creating useless runners. For example, a job getting cancelled before a runner was created or if the job was already picked up by another runner. We suggest using this in combination with a pool. The retry lambda respects this same setting, so disabling the check applies consistently everywhere it's evaluated. - Errors related to scaling should be retried via SQS. You can configure `job_queue_retention_in_seconds` and `redrive_build_queue` to tune the behavior. We have no mechanism to avoid events never being processed, which means potentially no runner gets created and the job in GitHub times out in 6 hours. The example for [ephemeral runners](examples/ephemeral.md) is based on the [default example](examples/default.md). Have look at the diff to see the major configuration differences. diff --git a/lambdas/functions/control-plane/src/scale-runners/job-retry.test.ts b/lambdas/functions/control-plane/src/scale-runners/job-retry.test.ts index c4ff1e5d76..4237a796f9 100644 --- a/lambdas/functions/control-plane/src/scale-runners/job-retry.test.ts +++ b/lambdas/functions/control-plane/src/scale-runners/job-retry.test.ts @@ -270,6 +270,72 @@ describe(`Test job retry check`, () => { // assert expect(publishMessage).not.toHaveBeenCalled(); }); + + it(`should publish a message for retry without calling the GitHub API when ENABLE_JOB_QUEUED_CHECK is false, even if the job is no longer queued.`, async () => { + // setup + mockOctokit.actions.getJobForWorkflowRun.mockImplementation(() => ({ + data: { + status: 'completed', + }, + })); + + const message: ActionRequestMessageRetry = { + eventType: 'workflow_job', + id: 0, + installationId: 0, + repositoryName: 'test', + repositoryOwner: 'github-aws-runners', + repoOwnerType: 'Organization', + retryCounter: 0, + }; + process.env.ENABLE_ORGANIZATION_RUNNERS = 'true'; + process.env.RUNNER_NAME_PREFIX = 'test'; + process.env.ENABLE_JOB_QUEUED_CHECK = 'false'; + process.env.JOB_QUEUE_SCALE_UP_URL = + 'https://sqs.eu-west-1.amazonaws.com/123456789/webhook_events_workflow_job_queue'; + + // act + await checkAndRetryJob(message); + + // assert + expect(mockOctokit.actions.getJobForWorkflowRun).not.toHaveBeenCalled(); + expect(publishMessage).toHaveBeenCalledWith( + JSON.stringify({ + ...message, + }), + 'https://sqs.eu-west-1.amazonaws.com/123456789/webhook_events_workflow_job_queue', + ); + }); + + it(`should still check job status by default (ENABLE_JOB_QUEUED_CHECK unset) and skip retry when job is no longer queued.`, async () => { + // setup + mockOctokit.actions.getJobForWorkflowRun.mockImplementation(() => ({ + data: { + status: 'completed', + }, + })); + + const message: ActionRequestMessageRetry = { + eventType: 'workflow_job', + id: 0, + installationId: 0, + repositoryName: 'test', + repositoryOwner: 'github-aws-runners', + repoOwnerType: 'Organization', + retryCounter: 0, + }; + process.env.ENABLE_ORGANIZATION_RUNNERS = 'true'; + process.env.RUNNER_NAME_PREFIX = 'test'; + process.env.JOB_QUEUE_SCALE_UP_URL = + 'https://sqs.eu-west-1.amazonaws.com/123456789/webhook_events_workflow_job_queue'; + + // act + await checkAndRetryJob(message); + + // assert + expect(mockOctokit.actions.getJobForWorkflowRun).toHaveBeenCalled(); + expect(publishMessage).not.toHaveBeenCalled(); + }); }); describe('Test job retry handler (batch processing)', () => { diff --git a/lambdas/functions/control-plane/src/scale-runners/job-retry.ts b/lambdas/functions/control-plane/src/scale-runners/job-retry.ts index 8f7d6e2289..aeb7612218 100644 --- a/lambdas/functions/control-plane/src/scale-runners/job-retry.ts +++ b/lambdas/functions/control-plane/src/scale-runners/job-retry.ts @@ -44,6 +44,7 @@ export async function checkAndRetryJob(payload: ActionRequestMessageRetry): Prom const runnerNamePrefix = process.env.RUNNER_NAME_PREFIX ?? ''; const jobQueueUrl = process.env.JOB_QUEUE_SCALE_UP_URL ?? ''; const enableMetrics = yn(process.env.ENABLE_METRIC_JOB_RETRY, { default: false }); + const enableJobQueuedCheck = yn(process.env.ENABLE_JOB_QUEUED_CHECK, { default: true }); const environment = process.env.ENVIRONMENT; addPersistentContextToChildLogger({ @@ -63,8 +64,9 @@ export async function checkAndRetryJob(payload: ActionRequestMessageRetry): Prom const { ghesApiUrl } = getGitHubEnterpriseApiUrl(); const ghClient = await getOctokit(ghesApiUrl, enableOrgLevel, payload); - // check job is still queued - if (await isJobQueued(ghClient, payload)) { + // check job is still queued, unless the check is disabled (same flag the scale-up path uses) + const jobQueued = enableJobQueuedCheck ? await isJobQueued(ghClient, payload) : true; + if (jobQueued) { await publishMessage(JSON.stringify(payload), jobQueueUrl); createMetric(enableMetrics, environment, payload); logger.info(`Job is still queued, message published to build queue and will be handled by scale-up.`, { payload }); diff --git a/modules/runners/variables.tf b/modules/runners/variables.tf index 1d2f9b35a9..cd3f0781fa 100644 --- a/modules/runners/variables.tf +++ b/modules/runners/variables.tf @@ -595,7 +595,7 @@ variable "enable_ephemeral_runners" { } variable "enable_job_queued_check" { - description = "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." + description = "Only scale if the job event received by the scale up lambda (and the job retry lambda) is in the state queued. By default enabled for non ephemeral runners and disabled for ephemeral. Set this variable to overwrite the default behavior." type = bool default = null } diff --git a/variables.tf b/variables.tf index f384921c09..d98471e8d7 100644 --- a/variables.tf +++ b/variables.tf @@ -784,7 +784,7 @@ variable "aws_dynamic_labels_policy" { } variable "enable_job_queued_check" { - description = "Only scale if the job event received by the scale up lambda is in the queued state. By default enabled for non ephemeral runners and disabled for ephemeral. Set this variable to overwrite the default behavior." + description = "Only scale if the job event received by the scale up lambda (and the job retry lambda) is in the queued state. By default enabled for non ephemeral runners and disabled for ephemeral. Set this variable to overwrite the default behavior." type = bool default = null }