From 6a14546ca2627ac8d270f7874dfc3d11871d6bfd Mon Sep 17 00:00:00 2001 From: Shintaro Haraguchi Date: Mon, 28 Sep 2026 16:47:15 +0900 Subject: [PATCH 1/2] fix(pool): count JIT-registered offline runners that are still booting With JIT config the runner is registered in GitHub before the agent on the instance connects, so it can be reported as offline while it is still booting. The pool counted a runner as booting only when it was not registered yet, so such a runner was excluded without checking its boot time, and a frequently running pool could launch extra instances. Count a registered runner that is offline and not busy while it is within its boot time. Offline runners that are busy, have no busy value, or whose boot time has expired are still excluded. Related #3799 --- .../aws/ec2/src/control-plane/pool.test.ts | 44 ++++++++++++++++--- .../aws/ec2/src/control-plane/pool.ts | 11 ++--- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.test.ts b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.test.ts index 3f8df82173..e725de26a3 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.test.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.test.ts @@ -1,5 +1,5 @@ import type { Octokit } from '@octokit/rest'; -import type { CreateGitHubRunnerConfig, CreateStartRunnerConfig, RunnerInfo } from '../../../../core'; +import type { CreateGitHubRunnerConfig, CreateStartRunnerConfig, RunnerInfo, RunnerStatus } from '../../../../core'; import { bootTimeExceeded, type Ec2RunnerResourceOperations } from '../runners'; import { createEc2PoolCapability } from './pool'; import { createRunners, type Ec2ProviderConfig, loadEc2ProviderConfig } from './runner-creation'; @@ -41,17 +41,47 @@ describe('createEc2PoolCapability.countAvailableRunners', () => { expect(mockBootTimeExceeded).not.toHaveBeenCalled(); }); - it('does not count registered busy or offline runners', () => { + it('does not count registered busy runners', () => { + const runners: RunnerInfo[] = [{ id: 'i-busy', owner: 'owner', type: 'Org' }]; + const runnerStatus = new Map([['i-busy', { busy: true, status: 'online' }]]); + + expect(capability.countAvailableRunners(runners, runnerStatus)).toBe(0); + expect(mockBootTimeExceeded).not.toHaveBeenCalled(); + }); + + it('counts registered offline runners that are still booting', () => { + // With JIT config the runner is registered in GitHub before the agent on the instance + // connects, so it can be reported as offline while it is still booting. + const runners: RunnerInfo[] = [{ id: 'i-jit-booting', owner: 'owner', type: 'Org' }]; + const runnerStatus = new Map([['i-jit-booting', { busy: false, status: 'offline' }]]); + mockBootTimeExceeded.mockReturnValue(false); + + expect(capability.countAvailableRunners(runners, runnerStatus)).toBe(1); + expect(mockBootTimeExceeded).toHaveBeenCalledWith(runners[0]); + }); + + it('does not count registered offline runners whose boot time expired', () => { + const runners: RunnerInfo[] = [{ id: 'i-offline', owner: 'owner', type: 'Org' }]; + const runnerStatus = new Map([['i-offline', { busy: false, status: 'offline' }]]); + mockBootTimeExceeded.mockReturnValue(true); + + expect(capability.countAvailableRunners(runners, runnerStatus)).toBe(0); + expect(mockBootTimeExceeded).toHaveBeenCalledWith(runners[0]); + }); + + it('does not count registered offline runners that are busy or have no busy value', () => { const runners: RunnerInfo[] = [ - { id: 'i-busy', owner: 'owner', type: 'Org' }, - { id: 'i-offline', owner: 'owner', type: 'Org' }, + { id: 'i-offline-busy', owner: 'owner', type: 'Org' }, + { id: 'i-offline-no-busy', owner: 'owner', type: 'Org' }, ]; - const runnerStatus = new Map([ - ['i-busy', { busy: true, status: 'online' }], - ['i-offline', { busy: false, status: 'offline' }], + const runnerStatus = new Map([ + ['i-offline-busy', { busy: true, status: 'offline' }], + ['i-offline-no-busy', { status: 'offline' } as RunnerStatus], ]); + mockBootTimeExceeded.mockReturnValue(false); expect(capability.countAvailableRunners(runners, runnerStatus)).toBe(0); + expect(capability.countAvailableRunners(runners, runnerStatus, true)).toBe(0); expect(mockBootTimeExceeded).not.toHaveBeenCalled(); }); diff --git a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.ts b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.ts index 43ec0aacf9..5027b07d06 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.ts @@ -13,13 +13,14 @@ function countAvailableEc2PoolRunners( // Runner should be considered idle if it is still booting, or is idle in GitHub let numberOfRunnersInPool = 0; for (const ec2Instance of ec2runners) { - if ( - (runnerStatus.get(ec2Instance.id)?.busy === false || includeBusyRunners) && - runnerStatus.get(ec2Instance.id)?.status === 'online' - ) { + const status = runnerStatus.get(ec2Instance.id); + // A runner can be registered before its agent connects (for example with JIT config) and is + // then reported as offline. Treat it like an unregistered runner until its boot time expires. + const registeredOffline = status?.status === 'offline' && status.busy === false; + if ((status?.busy === false || includeBusyRunners) && status?.status === 'online') { numberOfRunnersInPool++; logger.debug(`Runner ${ec2Instance.id} is idle in GitHub and counted as part of the pool`); - } else if (runnerStatus.get(ec2Instance.id) != null) { + } else if (status != null && !registeredOffline) { logger.debug(`Runner ${ec2Instance.id} is not idle in GitHub and NOT counted as part of the pool`); } else if (!bootTimeExceeded(ec2Instance)) { numberOfRunnersInPool++; From edc8323abeee2952d5ac55e18c1508a92f4c981c Mon Sep 17 00:00:00 2001 From: Shintaro Haraguchi Date: Mon, 28 Sep 2026 17:38:44 +0900 Subject: [PATCH 2/2] refactor(pool): give registered offline runners their own branch Move the check for registered offline runners that are not busy into its own branch instead of excluding them from the "not idle" branch with a negated flag. Behaviour is unchanged. Drop the test case for a runner status without a busy value, since RunnerStatus.busy is required and the case could only be built with a type cast. --- .../aws/ec2/src/control-plane/pool.test.ts | 14 ++++---------- .../aws/ec2/src/control-plane/pool.ts | 16 ++++++++++++---- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.test.ts b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.test.ts index e725de26a3..ae09fec2ab 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.test.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.test.ts @@ -1,5 +1,5 @@ import type { Octokit } from '@octokit/rest'; -import type { CreateGitHubRunnerConfig, CreateStartRunnerConfig, RunnerInfo, RunnerStatus } from '../../../../core'; +import type { CreateGitHubRunnerConfig, CreateStartRunnerConfig, RunnerInfo } from '../../../../core'; import { bootTimeExceeded, type Ec2RunnerResourceOperations } from '../runners'; import { createEc2PoolCapability } from './pool'; import { createRunners, type Ec2ProviderConfig, loadEc2ProviderConfig } from './runner-creation'; @@ -69,15 +69,9 @@ describe('createEc2PoolCapability.countAvailableRunners', () => { expect(mockBootTimeExceeded).toHaveBeenCalledWith(runners[0]); }); - it('does not count registered offline runners that are busy or have no busy value', () => { - const runners: RunnerInfo[] = [ - { id: 'i-offline-busy', owner: 'owner', type: 'Org' }, - { id: 'i-offline-no-busy', owner: 'owner', type: 'Org' }, - ]; - const runnerStatus = new Map([ - ['i-offline-busy', { busy: true, status: 'offline' }], - ['i-offline-no-busy', { status: 'offline' } as RunnerStatus], - ]); + it('does not count registered offline runners that are busy', () => { + const runners: RunnerInfo[] = [{ id: 'i-offline-busy', owner: 'owner', type: 'Org' }]; + const runnerStatus = new Map([['i-offline-busy', { busy: true, status: 'offline' }]]); mockBootTimeExceeded.mockReturnValue(false); expect(capability.countAvailableRunners(runners, runnerStatus)).toBe(0); diff --git a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.ts b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.ts index 5027b07d06..3d10d352e7 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/pool.ts @@ -14,13 +14,21 @@ function countAvailableEc2PoolRunners( let numberOfRunnersInPool = 0; for (const ec2Instance of ec2runners) { const status = runnerStatus.get(ec2Instance.id); - // A runner can be registered before its agent connects (for example with JIT config) and is - // then reported as offline. Treat it like an unregistered runner until its boot time expires. - const registeredOffline = status?.status === 'offline' && status.busy === false; if ((status?.busy === false || includeBusyRunners) && status?.status === 'online') { numberOfRunnersInPool++; logger.debug(`Runner ${ec2Instance.id} is idle in GitHub and counted as part of the pool`); - } else if (status != null && !registeredOffline) { + } else if (status?.status === 'offline' && status.busy === false) { + // A runner can be registered before its agent connects (for example with JIT config) and is + // then reported as offline. Count it as booting until its boot time expires. + if (!bootTimeExceeded(ec2Instance)) { + numberOfRunnersInPool++; + logger.info(`Runner ${ec2Instance.id} is registered offline, still booting and counted as part of the pool`); + } else { + logger.debug( + `Runner ${ec2Instance.id} is registered offline past its boot time and NOT counted as part of the pool`, + ); + } + } else if (status != null) { logger.debug(`Runner ${ec2Instance.id} is not idle in GitHub and NOT counted as part of the pool`); } else if (!bootTimeExceeded(ec2Instance)) { numberOfRunnersInPool++;