diff --git a/lambdas/functions/control-plane/src/lambda.ts b/lambdas/functions/control-plane/src/lambda.ts index 4594a1289e..68d2a08771 100644 --- a/lambdas/functions/control-plane/src/lambda.ts +++ b/lambdas/functions/control-plane/src/lambda.ts @@ -88,7 +88,7 @@ export async function scaleDownHandler(event: unknown, context: Context): Promis logger.logEventIfEnabled(event); try { - await scaleDown(); + await scaleDown(() => context.getRemainingTimeInMillis()); } catch (e) { logger.error(`${(e as Error).message}`, { error: e as Error }); } diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-down-contract.test.ts b/lambdas/functions/control-plane/src/scale-runners/scale-down-contract.test.ts index a4d52acc40..c3d185fe3f 100644 --- a/lambdas/functions/control-plane/src/scale-runners/scale-down-contract.test.ts +++ b/lambdas/functions/control-plane/src/scale-runners/scale-down-contract.test.ts @@ -7,6 +7,16 @@ import { controlPlaneProviderRegistry } from '../control-plane-providers'; import { scaleDown } from './scale-down'; import type { ScaleDownComputeProvider } from './types'; +vi.mock('../github/auth', () => ({ + createGithubAppAuth: vi.fn().mockResolvedValue({ token: 'app-token', appIndex: 0 }), + createGithubInstallationAuth: vi.fn().mockResolvedValue({ token: 'installation-token' }), + getStoredInstallationId: vi.fn().mockResolvedValue(123), + createOctokitClient: vi.fn().mockResolvedValue({ + actions: { listSelfHostedRunnersForOrg: vi.fn() }, + paginate: vi.fn().mockResolvedValue([]), + }), +})); + const mockedResolveCapability = vi.spyOn(controlPlaneProviderRegistry, 'capability'); const cleanEnv = process.env; diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-down.test.ts b/lambdas/functions/control-plane/src/scale-runners/scale-down.test.ts index 3583247f8d..562cb0207d 100644 --- a/lambdas/functions/control-plane/src/scale-runners/scale-down.test.ts +++ b/lambdas/functions/control-plane/src/scale-runners/scale-down.test.ts @@ -209,21 +209,21 @@ describe('Scale down runners', () => { mockOctokit.paginate.mockResolvedValue([]); mockOctokit.actions.deleteSelfHostedRunnerFromRepo.mockImplementation((repo) => { - if (repo.runner_id.includes('busy')) { + if (String(repo.runner_id).includes('busy')) { throw Error(); } return { status: 204 }; }); mockOctokit.actions.deleteSelfHostedRunnerFromOrg.mockImplementation((repo) => { - if (repo.runner_id.includes('busy')) { + if (String(repo.runner_id).includes('busy')) { throw Error(); } return { status: 204 }; }); mockOctokit.actions.getSelfHostedRunnerForRepo.mockImplementation((repo) => { - if (repo.runner_id.includes('busy')) { + if (String(repo.runner_id).includes('busy')) { return { data: { busy: true }, }; @@ -233,7 +233,7 @@ describe('Scale down runners', () => { }; }); mockOctokit.actions.getSelfHostedRunnerForOrg.mockImplementation((repo) => { - if (repo.runner_id.includes('busy')) { + if (String(repo.runner_id).includes('busy')) { return { data: { busy: true }, }; @@ -262,6 +262,132 @@ describe('Scale down runners', () => { mockCreateClient.mockResolvedValue(mockOctokit as unknown as Octokit); }); + it.each(['Org', 'Repo'] as const)('preserves legacy %s providers without GitHub identity fields', async (type) => { + const runner = createRunnerTestData('legacy-provider', type, 60, true, false, true); + delete runner.githubRunnerName; + delete runner.githubRunnerId; + mockGitHubRunners([runner]); + mockListRunners.mockResolvedValueOnce([]).mockResolvedValueOnce([runner]).mockResolvedValue([]); + await scaleDown(); + expect(mockOctokit.paginate).toHaveBeenCalledWith( + expect.anything(), + expect.not.objectContaining({ name: expect.anything() }), + ); + expect(mockTerminateRunners).toHaveBeenCalledWith(runner.id); + }); + + it('retains paged EC2 records without a trusted identity across sweeps', async () => { + const runner = createRunnerTestData('untrusted-prefix', 'Org', 60, false, false, false); + delete runner.githubRunnerName; + delete runner.githubRunnerId; + mockedResolveCapability.mockReturnValue(() => ({ + ...mockComputeProvider, + listPage: vi.fn().mockResolvedValue({ runners: [runner] }), + })); + await scaleDown(); + runner.orphan = true; + await scaleDown(); + expect(mockOctokit.paginate).not.toHaveBeenCalled(); + expect(mockMarkOrphan).not.toHaveBeenCalled(); + expect(mockTerminateRunners).not.toHaveBeenCalled(); + }); + + describe('incremental stateless EC2 inventory', () => { + it('cleans a page before a later listing failure and starts fresh against the reduced inventory', async () => { + const first = createRunnerTestData('first-page', 'Org', 60, true, false, true); + const second = createRunnerTestData('second-page', 'Org', 60, true, false, true); + first.githubRunnerName = first.id; + second.githubRunnerName = second.id; + mockGitHubRunners([first, second]); + const listPage = vi + .fn() + .mockResolvedValueOnce({ runners: [first], nextToken: 'page-2' }) + .mockImplementationOnce(() => { + expect(mockTerminateRunners).toHaveBeenCalledWith(first.id); + throw new Error('EC2 listing unavailable'); + }); + mockedResolveCapability.mockReturnValue(() => ({ ...mockComputeProvider, listPage })); + await expect(scaleDown()).rejects.toThrow('EC2 listing unavailable'); + // Terminated instances disappear from the next invocation's inventory. + listPage.mockResolvedValue({ runners: [second] }); + await scaleDown(); + expect(listPage).toHaveBeenLastCalledWith(ENVIRONMENT, undefined); + expect(mockTerminateRunners).toHaveBeenCalledWith(second.id); + expect(mockTerminateRunners).toHaveBeenCalledTimes(2); + expect(mockListRunners).not.toHaveBeenCalled(); + expect(mockOctokit.paginate).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ name: first.id })); + }); + + it('shares the idle allowance across pages in one invocation', async () => { + process.env.SCALE_DOWN_CONFIG = JSON.stringify([{ idleCount: 1, cron: '* * * * * *', timeZone: 'UTC' }]); + const first = createRunnerTestData('reserved', 'Org', 60, true, false, false); + const second = createRunnerTestData('excess', 'Org', 60, true, false, true); + mockGitHubRunners([first, second]); + const listPage = vi + .fn() + .mockResolvedValueOnce({ runners: [first], nextToken: 'second' }) + .mockResolvedValue({ runners: [second] }); + mockedResolveCapability.mockReturnValue(() => ({ ...mockComputeProvider, listPage })); + await scaleDown(); + expect(mockTerminateRunners).toHaveBeenCalledWith(second.id); + expect(mockTerminateRunners).not.toHaveBeenCalledWith(first.id); + expect(listPage).toHaveBeenLastCalledWith(ENVIRONMENT, 'second'); + }); + + it('stops before the deadline after retaining completed cleanup', async () => { + let remaining = 60000; + const first = createRunnerTestData('first', 'Org', 60, true, false, true); + const second = createRunnerTestData('second', 'Org', 60, true, false, true); + mockGitHubRunners([first, second]); + mockTerminateRunners.mockImplementationOnce(async () => { + remaining = 0; + }); + const listPage = vi.fn().mockResolvedValue({ runners: [first, second], nextToken: 'next' }); + mockedResolveCapability.mockReturnValue(() => ({ ...mockComputeProvider, listPage })); + await scaleDown(() => remaining); + expect(mockTerminateRunners).toHaveBeenCalledTimes(1); + expect(listPage).toHaveBeenCalledTimes(1); + }); + + it.each(['Org', 'Repo'] as const)( + 'uses a known %s registration ID without listing GitHub runners', + async (type) => { + const runner = createRunnerTestData('known-id', type, 60, true, false, true); + runner.githubRunnerId = '42'; + runner.githubRunnerName = undefined; + const get = + type === 'Org' + ? mockOctokit.actions.getSelfHostedRunnerForOrg + : mockOctokit.actions.getSelfHostedRunnerForRepo; + get.mockResolvedValue({ data: { id: 42, name: runner.id, busy: false, status: 'online' } }); + mockedResolveCapability.mockReturnValue(() => ({ + ...mockComputeProvider, + listPage: vi.fn().mockResolvedValue({ runners: [runner] }), + })); + await scaleDown(); + expect(get).toHaveBeenCalledWith(expect.objectContaining({ runner_id: 42 })); + expect(mockOctokit.paginate).not.toHaveBeenCalled(); + expect(mockTerminateRunners).toHaveBeenCalledWith(runner.id); + }, + ); + + it('continues after one runner lookup fails on a page', async () => { + const first = createRunnerTestData('failed-lookup', 'Org', 60, true, false, false); + const second = createRunnerTestData('working-lookup', 'Org', 60, true, false, true); + first.githubRunnerName = first.id; + second.githubRunnerName = second.id; + mockGitHubRunners([second]); + mockOctokit.paginate.mockRejectedValueOnce(new Error('GitHub unavailable')); + mockedResolveCapability.mockReturnValue(() => ({ + ...mockComputeProvider, + listPage: vi.fn().mockResolvedValue({ runners: [first, second] }), + })); + await scaleDown(); + expect(mockTerminateRunners).toHaveBeenCalledWith(second.id); + expect(mockTerminateRunners).not.toHaveBeenCalledWith(first.id); + }); + }); + const endpoints = ['https://api.github.com', 'https://github.enterprise.something', 'https://companyname.ghe.com']; describe.each(endpoints)('for %s', (endpoint) => { @@ -409,6 +535,78 @@ describe('Scale down runners', () => { checkNonTerminated(runners); }); + it('preserves a runner registered after orphan marking when its registration ID tag is missing', async () => { + const runner = createRunnerTestData('late-registration', type, MINIMUM_BOOT_TIME + 1, true, true, false); + mockProviderRunners([runner]); + mockGitHubRunners([runner]); + + await scaleDown(); + + expect(mockTerminateRunners).not.toHaveBeenCalled(); + expect(mockUnmarkOrphan).toHaveBeenCalledWith(runner.id); + }); + + it('preserves an untagged orphan when GitHub listing fails', async () => { + const runner = createRunnerTestData('unverified', type, MINIMUM_BOOT_TIME + 1, false, true, false); + mockProviderRunners([runner]); + mockOctokit.paginate.mockRejectedValue(new Error('GitHub unavailable')); + + await scaleDown(); + + expect(mockTerminateRunners).not.toHaveBeenCalled(); + expect(mockUnmarkOrphan).not.toHaveBeenCalled(); + }); + + it('continues processing orphans after a tagged runner lookup fails', async () => { + const unverified = createRunnerTestData( + 'unverified', + type, + MINIMUM_BOOT_TIME + 1, + false, + true, + false, + undefined, + 123, + ); + const orphan = createRunnerTestData('orphan-next', type, MINIMUM_BOOT_TIME + 1, false, true, true); + mockProviderRunners([unverified, orphan]); + mockGitHubRunners([]); + mockOctokit.actions.getSelfHostedRunnerForOrg.mockRejectedValue(new Error('GitHub unavailable')); + mockOctokit.actions.getSelfHostedRunnerForRepo.mockRejectedValue(new Error('GitHub unavailable')); + + await scaleDown(); + + expect(mockTerminateRunners).not.toHaveBeenCalledWith(unverified.id); + expect(mockTerminateRunners).toHaveBeenCalledWith(orphan.id); + }); + + it('continues processing orphans after a provider termination fails', async () => { + const first = createRunnerTestData('first', type, MINIMUM_BOOT_TIME + 1, false, true, false, undefined, 123); + const next = createRunnerTestData('next', type, MINIMUM_BOOT_TIME + 1, false, true, true); + mockProviderRunners([first, next]); + mockGitHubRunners([]); + const missing = new RequestError('Not found', 404, { + request: { method: 'GET', url: 'https://api.github.com/test', headers: {} }, + }); + mockOctokit.actions.getSelfHostedRunnerForOrg.mockRejectedValue(missing); + mockOctokit.actions.getSelfHostedRunnerForRepo.mockRejectedValue(missing); + mockTerminateRunners.mockRejectedValueOnce(new Error('EC2 unavailable')).mockResolvedValue(undefined); + + await scaleDown(); + + expect(mockTerminateRunners).toHaveBeenCalledWith(next.id); + }); + + it('preserves orphans whose ownership cannot be verified', async () => { + const runner = createRunnerTestData('unknown-owner', type, MINIMUM_BOOT_TIME + 1, false, true, false); + runner.owner = ''; + mockProviderRunners([runner]); + + await scaleDown(); + + expect(mockTerminateRunners).not.toHaveBeenCalled(); + }); + it('Should test if orphaned runner, untag if online and busy, else terminate (JIT)', async () => { const orphanRunner = createRunnerTestData( 'orphan-jit', @@ -831,6 +1029,7 @@ function createRunnerTestData( ): RunnerTestItem { return { id: `i-${name}-${type.toLowerCase()}`, + githubRunnerName: `i-${name}-${type.toLowerCase()}`, launchTime: moment(new Date()).subtract(minutesLaunchedAgo, 'minutes').toDate(), type, owner: diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-down.ts b/lambdas/functions/control-plane/src/scale-runners/scale-down.ts index 1e3e838aed..033360a2f0 100644 --- a/lambdas/functions/control-plane/src/scale-runners/scale-down.ts +++ b/lambdas/functions/control-plane/src/scale-runners/scale-down.ts @@ -102,34 +102,49 @@ async function getGitHubRunnerBusyState(client: Octokit, runner: RunnerInfo, run return state.busy; } -async function listGitHubRunners(runner: RunnerInfo): Promise { - const key = runner.owner; - const cachedRunners = githubCache.runners.get(key); - if (cachedRunners) { - logger.debug(`[listGithubRunners] Cache hit for ${key}`); - return cachedRunners; - } +function hasGitHubRunnerId(runner: RunnerInfo): boolean { + return /^[1-9]\d*$/.test(runner.githubRunnerId ?? '') && Number.isSafeInteger(Number(runner.githubRunnerId)); +} - logger.debug(`[listGithubRunners] Cache miss for ${key}`); +async function listGitHubRunners(runner: RunnerInfo, computeProvider: ScaleDownComputeProvider): Promise { const client = await getOrCreateOctokit(runner); - let runners; - if (runner.type === 'Org') { - runners = await client.paginate(client.actions.listSelfHostedRunnersForOrg, { - org: runner.owner, - per_page: 100, - }); - } else { - const [owner, repo] = runner.owner.split('/'); - runners = await client.paginate(client.actions.listSelfHostedRunnersForRepo, { - owner, - repo, - per_page: 100, - }); + if (hasGitHubRunnerId(runner)) { + const state = await getGitHubSelfHostedRunnerState(client, runner, Number(runner.githubRunnerId)); + return state ? [state] : []; + } + if (runner.githubRunnerName !== undefined) { + const owner = + runner.type === 'Org' + ? { org: runner.owner } + : { + owner: runner.owner.split('/')[0], + repo: runner.owner.split('/')[1], + }; + const method = + runner.type === 'Org' ? client.actions.listSelfHostedRunnersForOrg : client.actions.listSelfHostedRunnersForRepo; + return client.paginate(method, { ...owner, name: runner.githubRunnerName, per_page: 100 }); + } + if (!computeProvider.listPage) { + // Preserve the legacy provider contract. Cache only a complete successful + // listing; a failed/partial lookup must never establish absence. + const key = `${runner.type}:${runner.owner}`; + const cached = githubCache.runners.get(key); + if (cached) return cached; + const runners = + runner.type === 'Org' + ? await client.paginate(client.actions.listSelfHostedRunnersForOrg, { org: runner.owner, per_page: 100 }) + : await client.paginate(client.actions.listSelfHostedRunnersForRepo, { + owner: runner.owner.split('/')[0], + repo: runner.owner.split('/')[1], + per_page: 100, + }); + githubCache.runners.set(key, runners); + return runners; } - githubCache.runners.set(key, runners); - logger.debug(`[listGithubRunners] Cache set for ${key}`); - logger.debug(`[listGithubRunners] Runners: ${JSON.stringify(runners)}`); - return runners; + // Without either identity we cannot establish absence with a bounded query. + // Do not let a legacy/incomplete record force an organization-wide inventory + // before other instances can be cleaned up. + throw new Error(`Runner '${runner.id}' has no GitHub ID or complete runner name; skipping unverifiable cleanup`); } function runnerMinimumTimeExceeded(runner: RunnerInfo): boolean { @@ -283,8 +298,10 @@ async function evaluateAndRemoveRunners( runners: RunnerInfo[], scaleDownConfigs: ScalingDownConfigList, computeProvider: ScaleDownComputeProvider, + sweep = { idleRemaining: getIdleRunnerCount(scaleDownConfigs) }, + remainingTime = () => Infinity, ): Promise { - let idleCounter = getIdleRunnerCount(scaleDownConfigs); + let idleCounter = sweep.idleRemaining; const evictionStrategy = getEvictionStrategy(scaleDownConfigs); const ownerTags = new Set(runners.map((runner) => runner.owner)); @@ -295,35 +312,41 @@ async function evaluateAndRemoveRunners( logger.debug(`Found: '${ownerRunners.length}' active GitHub runners with owner tag: '${ownerTag}'`); logger.debug(`Active GitHub runners with owner tag: '${ownerTag}': ${JSON.stringify(ownerRunners)}`); for (const runner of ownerRunners) { - if (runner.bypassRemoval) { - logger.debug(`Runner '${runner.id}' has bypass-removal tag set, skipping evaluation.`); - continue; - } - const ghRunners = await listGitHubRunners(runner); - const ghRunnersFiltered = ghRunners.filter((ghRunner: { name: string }) => ghRunner.name.endsWith(runner.id)); - logger.debug(`Found: '${ghRunnersFiltered.length}' GitHub runners for runner: '${runner.id}'`); - logger.debug(`GitHub runners for runner: '${runner.id}': ${JSON.stringify(ghRunnersFiltered)}`); - if (ghRunnersFiltered.length) { - if (runnerMinimumTimeExceeded(runner)) { - if (idleCounter > 0) { - idleCounter--; - // A runner kept idle is not evaluated for removal, so its idle marker cannot be - // refreshed by busy readings. Clear it so a later evaluation starts a fresh window. - await clearIdleDetection(runner, computeProvider); - logger.info(`Runner '${runner.id}' will be kept idle.`); - } else { - logger.info(`Terminating all non busy runners.`); - await removeRunner( - runner, - ghRunnersFiltered.map((runner: { id: number }) => runner.id), - computeProvider, - ); + if (remainingTime() < 10000) return; + try { + if (runner.bypassRemoval) { + logger.debug(`Runner '${runner.id}' has bypass-removal tag set, skipping evaluation.`); + continue; + } + const ghRunners = await listGitHubRunners(runner, computeProvider); + const ghRunnersFiltered = ghRunners.filter((ghRunner: { name: string }) => ghRunner.name.endsWith(runner.id)); + logger.debug(`Found: '${ghRunnersFiltered.length}' GitHub runners for runner: '${runner.id}'`); + logger.debug(`GitHub runners for runner: '${runner.id}': ${JSON.stringify(ghRunnersFiltered)}`); + if (ghRunnersFiltered.length) { + if (runnerMinimumTimeExceeded(runner)) { + if (idleCounter > 0) { + idleCounter--; + sweep.idleRemaining = idleCounter; + // A runner kept idle is not evaluated for removal, so its idle marker cannot be + // refreshed by busy readings. Clear it so a later evaluation starts a fresh window. + await clearIdleDetection(runner, computeProvider); + logger.info(`Runner '${runner.id}' will be kept idle.`); + } else { + logger.info(`Terminating all non busy runners.`); + await removeRunner( + runner, + ghRunnersFiltered.map((runner: { id: number }) => runner.id), + computeProvider, + ); + } } + } else if (computeProvider.bootTimeExceeded(runner)) { + await markOrphan(runner.id, computeProvider); + } else { + logger.debug(`Runner ${runner.id} has not yet booted.`); } - } else if (computeProvider.bootTimeExceeded(runner)) { - await markOrphan(runner.id, computeProvider); - } else { - logger.debug(`Runner ${runner.id} has not yet booted.`); + } catch (error) { + logger.warn(`Failed to evaluate runner '${runner.id}'; continuing cleanup.`, { error }); } } } @@ -367,31 +390,46 @@ async function lastChanceCheckOrphanRunner(runner: RunnerInfo): Promise return isOrphan; } -async function terminateOrphan(environment: string, computeProvider: ScaleDownComputeProvider): Promise { +async function terminateOrphan( + environment: string, + computeProvider: ScaleDownComputeProvider, + page?: RunnerInfo[], + remainingTime = () => Infinity, +): Promise { + let orphanRunners: RunnerInfo[]; try { - const orphanRunners = await computeProvider.list(environment, true); + orphanRunners = page ?? (await computeProvider.list(environment, true)); + } catch (error) { + logger.warn('Failed to list orphan runners.', { error }); + return; + } - for (const runner of orphanRunners) { - if (runner.bypassRemoval) { - logger.info(`Orphan runner '${runner.id}' has bypass-removal tag set, skipping termination.`); - continue; - } - if (runner.githubRunnerId) { - const isOrphan = await lastChanceCheckOrphanRunner(runner); - if (isOrphan) { - await computeProvider.terminate(runner.id); - } else { - await unMarkOrphan(runner.id, computeProvider); - } + for (const runner of orphanRunners) { + if (remainingTime() < 10000) return; + if (runner.bypassRemoval) { + logger.info(`Orphan runner '${runner.id}' has bypass-removal tag set, skipping termination.`); + continue; + } + if (!runner.owner || !runner.type) { + logger.warn(`Cannot verify orphan runner '${runner.id}' without its owner and type, skipping termination.`); + continue; + } + try { + // A runner can register after it was marked orphan, even if writing its + // registration ID back to the compute provider failed. Check GitHub again. + const isOrphan = hasGitHubRunnerId(runner) + ? await lastChanceCheckOrphanRunner(runner) + : !(await listGitHubRunners(runner, computeProvider)).some((registered) => registered.name.endsWith(runner.id)); + if (isOrphan) { + await computeProvider.terminate(runner.id); } else { - logger.info(`Terminating orphan runner '${runner.id}'`); - await computeProvider.terminate(runner.id).catch((e) => { - logger.error(`Failed to terminate orphan runner '${runner.id}'`, { error: e }); - }); + await unMarkOrphan(runner.id, computeProvider); } + } catch (error) { + // Leave this runner for a later invocation without blocking other owners + // or runners when one GitHub request or provider termination fails. + logger.warn(`Failed to process orphan runner '${runner.id}'.`, { error }); } - } catch (e) { - logger.warn(`Failure during orphan termination processing.`, { error: e }); } } @@ -417,7 +455,7 @@ function filterRunners(runners: RunnerInfo[]): RunnerInfo[] { return runners.filter((runner) => runner.owner && runner.type && !runner.orphan); } -export async function scaleDown(): Promise { +export async function scaleDown(remainingTime = () => Infinity): Promise { githubCache.reset(); const environment = process.env.ENVIRONMENT; const scaleDownConfigs = JSON.parse(process.env.SCALE_DOWN_CONFIG) as ScalingDownConfigList; @@ -427,6 +465,30 @@ export async function scaleDown(): Promise { type: computeProviderType, }; + if (computeProvider.listPage) { + const sweep = { idleRemaining: getIdleRunnerCount(scaleDownConfigs) }; + let nextToken: string | undefined; + do { + if (remainingTime() < 10000) return; + const page = await computeProvider.listPage(environment, nextToken); + await terminateOrphan( + environment, + computeProvider, + page.runners.filter((runner) => runner.orphan), + remainingTime, + ); + await evaluateAndRemoveRunners( + filterRunners(page.runners), + scaleDownConfigs, + computeProvider, + sweep, + remainingTime, + ); + nextToken = page.nextToken; + } while (nextToken); + return; + } + // first runners marked to be orphan. await terminateOrphan(environment, computeProvider); diff --git a/lambdas/functions/control-plane/src/test/compute-provider-contracts/scale-down.ts b/lambdas/functions/control-plane/src/test/compute-provider-contracts/scale-down.ts index 58fd4b4426..6e7864227c 100644 --- a/lambdas/functions/control-plane/src/test/compute-provider-contracts/scale-down.ts +++ b/lambdas/functions/control-plane/src/test/compute-provider-contracts/scale-down.ts @@ -47,9 +47,11 @@ export function defineScaleDownContractTests({ expect(provider.list).toHaveBeenNthCalledWith(2, 'test-environment'); }); - it('terminates an orphan that has no GitHub runner identity', async () => { + it('terminates an orphan after GitHub confirms no matching registration', async () => { vi.mocked(provider.list) - .mockResolvedValueOnce([{ id: 'orphan-runner', owner: 'owner', type: 'Org', orphan: true }]) + .mockResolvedValueOnce([ + { id: 'orphan-runner', githubRunnerName: 'orphan-runner', owner: 'owner', type: 'Org', orphan: true }, + ]) .mockResolvedValue([]); await scaleDown(); diff --git a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/scale-down.ts b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/scale-down.ts index f2f4eb303a..c6361d4569 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/scale-down.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/scale-down.ts @@ -13,6 +13,9 @@ export function createEc2ScaleDownCapability( ): Omit { return { list: (environment, orphan) => ec2Operations.list({ environment, orphan }), + ...(ec2Operations.listPage + ? { listPage: (environment: string, nextToken?: string) => ec2Operations.listPage!(environment, nextToken) } + : {}), bootTimeExceeded, markOrphan: (id) => ec2Operations.tag(id, [{ Key: 'ghr:orphan', Value: 'true' }]), unmarkOrphan: (id) => ec2Operations.untag(id, [{ Key: 'ghr:orphan', Value: 'true' }]), diff --git a/lambdas/libs/compute-providers/aws/ec2/src/runners.test.ts b/lambdas/libs/compute-providers/aws/ec2/src/runners.test.ts index 9e5c116ef4..6e3b55c991 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/runners.test.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/runners.test.ts @@ -36,6 +36,9 @@ const ORG_NAME = 'SomeAwesomeCoder'; const REPO_NAME = `${ORG_NAME}/some-amazing-library`; const ENVIRONMENT = 'unit-test-environment'; const RUNNER_NAME_PREFIX = ''; +beforeEach(() => { + process.env.RUNNER_NAME_PREFIX = RUNNER_NAME_PREFIX; +}); const RUNNER_TYPES: RunnerType[] = ['Repo', 'Org']; mockEC2Client.on(DescribeInstancesCommand).resolves({}); @@ -95,6 +98,7 @@ describe('list instances', () => { launchTime: new Date('2020-10-10T14:48:00.000+09:00'), type: 'Org', owner: 'CoderToCat', + githubRunnerName: 'i-1234', orphan: false, bypassRemoval: false, }); @@ -109,6 +113,7 @@ describe('list instances', () => { launchTime: new Date('2020-10-10T14:48:00.000+09:00'), type: 'Org', owner: 'CoderToCat', + githubRunnerName: 'i-1234', orphan: false, githubRunnerId: '9876543210', bypassRemoval: false, @@ -130,6 +135,7 @@ describe('list instances', () => { launchTime: instances.Reservations![0].Instances![0].LaunchTime!, type: 'Org', owner: 'CoderToCat', + githubRunnerName: 'i-1234', orphan: true, bypassRemoval: false, }); @@ -1804,3 +1810,33 @@ describe('create runner with useDedicatedHost', () => { expect(runInstancesInput).not.toHaveProperty('WeightedCapacity'); }); }); + +describe('resumable EC2 cleanup pages', () => { + it('returns one bounded page and its continuation without loading the next page', async () => { + mockEC2Client.reset(); + mockEC2Client.on(DescribeInstancesCommand).resolves({ ...mockRunningInstances, NextToken: 'next-page' }); + const page = await ec2Operations.listPage!('test-environment', 'current-page'); + expect(mockEC2Client).toHaveReceivedCommandTimes(DescribeInstancesCommand, 1); + expect(mockEC2Client).toHaveReceivedCommandWith(DescribeInstancesCommand, { + MaxResults: 10, + NextToken: 'current-page', + Filters: expect.arrayContaining([{ Name: 'tag:ghr:environment', Values: ['test-environment'] }]), + }); + expect(page.nextToken).toBe('next-page'); + expect(page.runners[0].githubRunnerName).toBe('i-1234'); + }); +}); + +describe('trusted EC2 runner identity', () => { + it.each([undefined, 'configured_'])( + 'does not trust an instance tag inconsistent with configured prefix %s', + async (prefix) => { + if (prefix === undefined) delete process.env.RUNNER_NAME_PREFIX; + else process.env.RUNNER_NAME_PREFIX = prefix; + mockEC2Client.reset(); + mockEC2Client.on(DescribeInstancesCommand).resolves(mockRunningInstances); + const page = await ec2Operations.listPage!('test-environment'); + expect(page.runners[0].githubRunnerName).toBeUndefined(); + }, + ); +}); diff --git a/lambdas/libs/compute-providers/aws/ec2/src/runners.ts b/lambdas/libs/compute-providers/aws/ec2/src/runners.ts index 6269790adb..108474b3d2 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/runners.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/runners.ts @@ -39,6 +39,8 @@ export interface Ec2RunnerRequestContext { export interface Ec2RunnerResourceOperations { list(filters?: Ec2ListRunnerFilters): Promise; + listPage?(environment: string, nextToken?: string): Promise<{ runners: RunnerInfo[]; nextToken?: string }>; + create(runnerParameters: RunnerInputParameters): Promise; terminate(instanceId: string): Promise; tag(instanceId: string, tags: Tag[]): Promise; @@ -64,6 +66,17 @@ async function runWithRequestSignal( export function createEc2RunnerClient(ec2Client: EC2Client): Ec2RunnerClient { return { forRequest: ({ signal }) => ({ + listPage: async (environment, nextToken) => { + const page = await ec2Client.send( + new DescribeInstancesCommand({ + Filters: constructFilters({ environment })[0], + NextToken: nextToken, + MaxResults: 10, + }), + { abortSignal: signal }, + ); + return { runners: getRunnerInfo(page), nextToken: page.NextToken }; + }, list: (filters) => runWithRequestSignal(signal, () => listEc2Runners(ec2Client, filters, signal)), create: (runnerParameters) => runWithRequestSignal(signal, () => createEc2Runner(ec2Client, runnerParameters, signal)), @@ -147,6 +160,13 @@ function getRunnerInfo(runningInstances: DescribeInstancesResult) { for (const i of r.Instances) { runners.push({ id: i.InstanceId as string, + // Caller-supplied EC2 tags can override the prefix used by JIT + // registration. Only trust a tag matching the Lambda configuration. + githubRunnerName: + process.env.RUNNER_NAME_PREFIX !== undefined && + i.Tags?.find((tag) => tag.Key === 'ghr:runner_name_prefix')?.Value === process.env.RUNNER_NAME_PREFIX + ? `${process.env.RUNNER_NAME_PREFIX}${i.InstanceId}` + : undefined, launchTime: i.LaunchTime, owner: i.Tags?.find((e) => e.Key === 'ghr:Owner')?.Value as string, type: i.Tags?.find((e) => e.Key === 'ghr:Type')?.Value as RunnerInfo['type'], diff --git a/lambdas/libs/compute-providers/core/index.ts b/lambdas/libs/compute-providers/core/index.ts index 2457719a42..a26b3bc580 100644 --- a/lambdas/libs/compute-providers/core/index.ts +++ b/lambdas/libs/compute-providers/core/index.ts @@ -89,6 +89,8 @@ export interface RunnerInfo { org?: string; orphan?: boolean; githubRunnerId?: string; + /** Complete registration name from a trusted provider configuration/source. */ + githubRunnerName?: string; bypassRemoval?: boolean; /** * When scale-down first observed this runner reporting idle, as an ISO-8601 string. @@ -106,6 +108,9 @@ export interface ListRunnerFilters { export interface ScaleDownComputeProvider extends ComputeProvider { list(environment: string, orphan?: boolean): Promise; + /** Bounded inventory page; tokens are local to an invocation. Supply a GitHub ID or trusted complete name for cleanup. */ + listPage?(environment: string, nextToken?: string): Promise<{ runners: RunnerInfo[]; nextToken?: string }>; + bootTimeExceeded(runner: RunnerInfo): boolean; markOrphan(id: string): Promise; unmarkOrphan(id: string): Promise; diff --git a/lambdas/libs/compute-providers/registry.test.ts b/lambdas/libs/compute-providers/registry.test.ts index 77f8030257..f2430be4c7 100644 --- a/lambdas/libs/compute-providers/registry.test.ts +++ b/lambdas/libs/compute-providers/registry.test.ts @@ -26,7 +26,7 @@ it('exposes every configured provider through both capability registries', () => getCurrentRunners: expect.any(Function), createRunners: expect.any(Function), }); - expect(controlPlaneRegistry.capability(type, 'scaleDown')()).toEqual({ + expect(controlPlaneRegistry.capability(type, 'scaleDown')()).toMatchObject({ list: expect.any(Function), bootTimeExceeded: expect.any(Function), markOrphan: expect.any(Function), diff --git a/lambdas/libs/compute-providers/templates/provider/README.md b/lambdas/libs/compute-providers/templates/provider/README.md index 68a6d7c5ea..570d3dbaad 100644 --- a/lambdas/libs/compute-providers/templates/provider/README.md +++ b/lambdas/libs/compute-providers/templates/provider/README.md @@ -27,3 +27,7 @@ Implement every capability before registering the provider: Provider-specific tests should remain beside the provider implementation. The generic orchestration contracts remain owned by the control-plane package. + +Scale-down providers may optionally implement `listPage` to process inventory incrementally. Each paged record needs a GitHub runner ID or a complete registration name from a trusted source; missing identities are retained, never treated as absent. Existing list-only providers remain compatible and use a complete, invocation-cached GitHub owner listing when identity fields are absent. A failed listing is not cached as an empty result. + +For EC2, scale-down receives `RUNNER_NAME_PREFIX` from the same Terraform runner configuration as scale-up. The instance prefix tag must match that configuration before it can supply a missing-ID lookup name. Instances with missing or mismatched prefix tags and no GitHub ID are retained; correct their tags or restore the registration ID before expecting automatic cleanup. diff --git a/modules/orchestration-providers/webhook/scale-down-state-diagram.md b/modules/orchestration-providers/webhook/scale-down-state-diagram.md index 2e94d9ba79..2876d56fc9 100644 --- a/modules/orchestration-providers/webhook/scale-down-state-diagram.md +++ b/modules/orchestration-providers/webhook/scale-down-state-diagram.md @@ -2,139 +2,42 @@ -The scale-down Lambda function runs on a scheduled basis (every 5 minutes by default) to manage GitHub Actions runner instances. It performs a two-phase cleanup process: first terminating confirmed orphaned instances, then evaluating active runners to maintain the desired idle capacity while removing unnecessary instances. +Scale-down is stateless. Each invocation lists current EC2 inventory in bounded pages (ten instances per request) and processes orphaned and active runners from each page before requesting the next. It does not collect the entire fleet before cleanup. Successfully terminated instances disappear from subsequent inventories; the next scheduled invocation starts a fresh scan of what remains. Individual runner failures do not block other runners, and a later page failure retains all earlier cleanup. A deadline guard stops new work with ten seconds remaining. -```mermaid -stateDiagram-v2 - [*] --> ScheduledExecution : Cron Trigger every 5 min - - ScheduledExecution --> Phase1_OrphanTermination : Start Phase 1 - - state Phase1_OrphanTermination { - [*] --> ListOrphanInstances : Query EC2 for ghr orphan true - - ListOrphanInstances --> CheckOrphanType : For each orphan - - state CheckOrphanType <> - CheckOrphanType --> HasRunnerIdTag : Has ghr github runner id - CheckOrphanType --> TerminateOrphan : No runner ID tag - - HasRunnerIdTag --> LastChanceCheck : Query GitHub API - - state LastChanceCheck <> - LastChanceCheck --> ConfirmedOrphan : Offline and busy - LastChanceCheck --> FalsePositive : Exists and not problematic - - ConfirmedOrphan --> TerminateOrphan - FalsePositive --> RemoveOrphanTag - - TerminateOrphan --> NextOrphan : Continue processing - RemoveOrphanTag --> NextOrphan - - NextOrphan --> CheckOrphanType : More orphans? - NextOrphan --> Phase2_ActiveRunners : All processed - } - - Phase1_OrphanTermination --> Phase2_ActiveRunners : Phase 1 Complete - - state Phase2_ActiveRunners { - [*] --> ListActiveRunners : Query non-orphan EC2 instances - - ListActiveRunners --> GroupByOwner : Sort by owner and repo - - GroupByOwner --> ProcessOwnerGroup : For each owner - - state ProcessOwnerGroup { - [*] --> SortByStrategy : Apply eviction strategy - SortByStrategy --> ProcessRunner : Oldest first or newest first - - ProcessRunner --> QueryGitHub : Get GitHub runners for owner - - QueryGitHub --> MatchRunner : Find runner by instance ID suffix - - state MatchRunner <> - MatchRunner --> FoundInGitHub : Runner exists in GitHub - MatchRunner --> NotFoundInGitHub : Runner not in GitHub - - state FoundInGitHub { - [*] --> CheckMinimumTime : Has minimum runtime passed? - - state CheckMinimumTime <> - CheckMinimumTime --> TooYoung : Runtime less than minimum - CheckMinimumTime --> CheckIdleQuota : Runtime greater than or equal to minimum +GitHub runner IDs and exact runner names avoid organization-wide inventories. A runner without either identity is left alone rather than blocking cleanup or being assumed absent. The EC2 provider supplies the full name from its `ghr:runner_name_prefix` tag and instance ID. - TooYoung --> NextRunner +The eviction strategy orders runners within each page. The idle allowance is shared across pages within one invocation and recalculated on each invocation. Strict global oldest/newest ordering requires a full inventory; incremental cleanup instead preserves the configured allowance while processing each available page. Pagination can shift as instances disappear, so subsequent scheduled scans reconcile remaining items. Busy, retained, and failed runners may be checked again; no cursor or completed-item list is persisted. - state CheckIdleQuota <> - CheckIdleQuota --> KeepIdle : Idle quota available - CheckIdleQuota --> CheckBusyState : Quota full - - KeepIdle --> NextRunner - - state CheckBusyState <> - CheckBusyState --> KeepBusy : Runner busy - CheckBusyState --> TerminateIdle : Runner idle - - KeepBusy --> NextRunner - TerminateIdle --> DeregisterFromGitHub - DeregisterFromGitHub --> TerminateInstance - TerminateInstance --> NextRunner - } - - state NotFoundInGitHub { - [*] --> CheckBootTime : Has boot time exceeded? - - state CheckBootTime <> - CheckBootTime --> StillBooting : Boot time less than threshold - CheckBootTime --> MarkOrphan : Boot time greater than or equal to threshold - - StillBooting --> NextRunner - MarkOrphan --> TagAsOrphan : Set ghr orphan true - TagAsOrphan --> NextRunner - } - - NextRunner --> ProcessRunner : More runners in group? - NextRunner --> NextOwnerGroup : Group complete - } - - NextOwnerGroup --> ProcessOwnerGroup : More owner groups? - NextOwnerGroup --> ExecutionComplete : All groups processed - } - - Phase2_ActiveRunners --> ExecutionComplete : Phase 2 Complete - - ExecutionComplete --> [*] : Wait for next cron trigger - - note right of LastChanceCheck - Uses ghr github runner id tag - for precise GitHub API lookup - end note - - note right of MatchRunner - Matches GitHub runner name - ending with EC2 instance ID - end note - - note right of CheckMinimumTime - Minimum running time in minutes - (Linux: 5min, Windows: 15min, OSX: 20min) - end note - - note right of CheckBootTime - Runner boot time in minutes - Default configuration value +```mermaid +stateDiagram-v2 + [*] --> FetchPage : Fresh scheduled scan + FetchPage --> SelectRunner : One bounded EC2 page + FetchPage --> [*] : Listing failure retains earlier cleanup + SelectRunner --> CheckDeadline : Next runner + CheckDeadline --> [*] : Near deadline + CheckDeadline --> VerifyRunner : Time remains + VerifyRunner --> Cleanup : Verified orphan or eligible idle runner + VerifyRunner --> SelectRunner : Busy, retained, incomplete identity, or lookup error + Cleanup --> SelectRunner : Success or isolated failure + SelectRunner --> FetchPage : Page complete with more pages + SelectRunner --> [*] : Inventory exhausted + + note right of VerifyRunner + GitHub lookup by runner ID or exact name. + Retention, busy checks, boot grace, and + bypass-removal protection still apply. end note ``` - ## Key Decision Points | State | Condition | Action | |-------|-----------|--------| | **Orphan w/ Runner ID** | GitHub: offline + busy | Terminate (confirmed orphan) | | **Orphan w/ Runner ID** | GitHub: exists + healthy | Remove orphan tag (false positive) | -| **Orphan w/o Runner ID** | Always | Terminate (no way to verify) | +| **Orphan w/o Runner ID** | Exact-name lookup confirms absence | Terminate | +| **Orphan w/o identity** | Cannot verify registration | Preserve for a later sweep | | **Active Runner Found** | Runtime < minimum | Keep (too young) | | **Active Runner Found** | Idle quota available | Keep as idle | | **Active Runner Found** | Quota full + idle | Terminate + deregister | diff --git a/modules/orchestration-providers/webhook/scale-runners/scale-down.tf b/modules/orchestration-providers/webhook/scale-runners/scale-down.tf index efe57570f6..f2410c8ca8 100644 --- a/modules/orchestration-providers/webhook/scale-runners/scale-down.tf +++ b/modules/orchestration-providers/webhook/scale-runners/scale-down.tf @@ -24,6 +24,7 @@ resource "aws_lambda_function" "scale_down" { SCALE_DOWN_IDLE_CONFIRMATION_SECONDS = var.config.scale_down.idle_confirmation_seconds NODE_TLS_REJECT_UNAUTHORIZED = var.config.github.enterprise_server.url != null && !var.config.github.enterprise_server.ssl_verify ? 0 : 1 POWERTOOLS_LOGGER_LOG_EVENT = var.config.observability.logs.level == "debug" ? "true" : "false" + RUNNER_NAME_PREFIX = var.config.runner.name_prefix SCALE_DOWN_CONFIG = jsonencode(var.config.scale_down.idle_config) POWERTOOLS_SERVICE_NAME = "${var.config.prefix}-scale-down" POWERTOOLS_METRICS_NAMESPACE = var.config.observability.metrics.namespace diff --git a/modules/orchestration-providers/webhook/scale-runners/tests/scale-runners.tftest.hcl b/modules/orchestration-providers/webhook/scale-runners/tests/scale-runners.tftest.hcl index 22b695aac2..5224724d71 100644 --- a/modules/orchestration-providers/webhook/scale-runners/tests/scale-runners.tftest.hcl +++ b/modules/orchestration-providers/webhook/scale-runners/tests/scale-runners.tftest.hcl @@ -211,6 +211,11 @@ variables { } run "assembles_provider_neutral_scaling_control_plane" { + assert { + condition = aws_lambda_function.scale_down.environment[0].variables["RUNNER_NAME_PREFIX"] == var.config.runner.name_prefix + error_message = "Scale-down must receive the same trusted runner prefix as scale-up." + } + command = plan assert { diff --git a/modules/runners/scale-down-state-diagram.md b/modules/runners/scale-down-state-diagram.md index 64e32bc141..c60c259472 100644 --- a/modules/runners/scale-down-state-diagram.md +++ b/modules/runners/scale-down-state-diagram.md @@ -2,139 +2,42 @@ -The scale-down Lambda function runs on a scheduled basis (every 5 minutes by default) to manage GitHub Actions runner instances. It performs a two-phase cleanup process: first terminating confirmed orphaned instances, then evaluating active runners to maintain the desired idle capacity while removing unnecessary instances. +Scale-down is stateless. Each invocation lists current EC2 inventory in bounded pages (ten instances per request) and processes orphaned and active runners from each page before requesting the next. It does not collect the entire fleet before cleanup. Successfully terminated instances disappear from subsequent inventories; the next scheduled invocation starts a fresh scan of what remains. Individual runner failures do not block other runners, and a later page failure retains all earlier cleanup. A deadline guard stops new work with ten seconds remaining. -```mermaid -stateDiagram-v2 - [*] --> ScheduledExecution : Cron Trigger every 5 min - - ScheduledExecution --> Phase1_OrphanTermination : Start Phase 1 - - state Phase1_OrphanTermination { - [*] --> ListOrphanInstances : Query EC2 for ghr orphan true - - ListOrphanInstances --> CheckOrphanType : For each orphan - - state CheckOrphanType <> - CheckOrphanType --> HasRunnerIdTag : Has ghr github runner id - CheckOrphanType --> TerminateOrphan : No runner ID tag - - HasRunnerIdTag --> LastChanceCheck : Query GitHub API - - state LastChanceCheck <> - LastChanceCheck --> ConfirmedOrphan : Offline and busy - LastChanceCheck --> FalsePositive : Exists and not problematic - - ConfirmedOrphan --> TerminateOrphan - FalsePositive --> RemoveOrphanTag - - TerminateOrphan --> NextOrphan : Continue processing - RemoveOrphanTag --> NextOrphan - - NextOrphan --> CheckOrphanType : More orphans? - NextOrphan --> Phase2_ActiveRunners : All processed - } - - Phase1_OrphanTermination --> Phase2_ActiveRunners : Phase 1 Complete - - state Phase2_ActiveRunners { - [*] --> ListActiveRunners : Query non-orphan EC2 instances - - ListActiveRunners --> GroupByOwner : Sort by owner and repo - - GroupByOwner --> ProcessOwnerGroup : For each owner - - state ProcessOwnerGroup { - [*] --> SortByStrategy : Apply eviction strategy - SortByStrategy --> ProcessRunner : Oldest first or newest first - - ProcessRunner --> QueryGitHub : Get GitHub runners for owner - - QueryGitHub --> MatchRunner : Find runner by instance ID suffix - - state MatchRunner <> - MatchRunner --> FoundInGitHub : Runner exists in GitHub - MatchRunner --> NotFoundInGitHub : Runner not in GitHub - - state FoundInGitHub { - [*] --> CheckMinimumTime : Has minimum runtime passed? - - state CheckMinimumTime <> - CheckMinimumTime --> TooYoung : Runtime less than minimum - CheckMinimumTime --> CheckIdleQuota : Runtime greater than or equal to minimum +GitHub runner IDs and exact runner names avoid organization-wide inventories. A runner without either identity is left alone rather than blocking cleanup or being assumed absent. The EC2 provider supplies the full name from its `ghr:runner_name_prefix` tag and instance ID. - TooYoung --> NextRunner +The eviction strategy orders runners within each page. The idle allowance is shared across pages within one invocation and recalculated on each invocation. Strict global oldest/newest ordering requires a full inventory; incremental cleanup instead preserves the configured allowance while processing each available page. Pagination can shift as instances disappear, so subsequent scheduled scans reconcile remaining items. Busy, retained, and failed runners may be checked again; no cursor or completed-item list is persisted. - state CheckIdleQuota <> - CheckIdleQuota --> KeepIdle : Idle quota available - CheckIdleQuota --> CheckBusyState : Quota full - - KeepIdle --> NextRunner - - state CheckBusyState <> - CheckBusyState --> KeepBusy : Runner busy - CheckBusyState --> TerminateIdle : Runner idle - - KeepBusy --> NextRunner - TerminateIdle --> DeregisterFromGitHub - DeregisterFromGitHub --> TerminateInstance - TerminateInstance --> NextRunner - } - - state NotFoundInGitHub { - [*] --> CheckBootTime : Has boot time exceeded? - - state CheckBootTime <> - CheckBootTime --> StillBooting : Boot time less than threshold - CheckBootTime --> MarkOrphan : Boot time greater than or equal to threshold - - StillBooting --> NextRunner - MarkOrphan --> TagAsOrphan : Set ghr orphan true - TagAsOrphan --> NextRunner - } - - NextRunner --> ProcessRunner : More runners in group? - NextRunner --> NextOwnerGroup : Group complete - } - - NextOwnerGroup --> ProcessOwnerGroup : More owner groups? - NextOwnerGroup --> ExecutionComplete : All groups processed - } - - Phase2_ActiveRunners --> ExecutionComplete : Phase 2 Complete - - ExecutionComplete --> [*] : Wait for next cron trigger - - note right of LastChanceCheck - Uses ghr github runner id tag - for precise GitHub API lookup - end note - - note right of MatchRunner - Matches GitHub runner name - ending with EC2 instance ID - end note - - note right of CheckMinimumTime - Minimum running time in minutes - (Linux: 5min, Windows: 15min, OSX: 20min) - end note - - note right of CheckBootTime - Runner boot time in minutes - Default configuration value +```mermaid +stateDiagram-v2 + [*] --> FetchPage : Fresh scheduled scan + FetchPage --> SelectRunner : One bounded EC2 page + FetchPage --> [*] : Listing failure retains earlier cleanup + SelectRunner --> CheckDeadline : Next runner + CheckDeadline --> [*] : Near deadline + CheckDeadline --> VerifyRunner : Time remains + VerifyRunner --> Cleanup : Verified orphan or eligible idle runner + VerifyRunner --> SelectRunner : Busy, retained, incomplete identity, or lookup error + Cleanup --> SelectRunner : Success or isolated failure + SelectRunner --> FetchPage : Page complete with more pages + SelectRunner --> [*] : Inventory exhausted + + note right of VerifyRunner + GitHub lookup by runner ID or exact name. + Retention, busy checks, boot grace, and + bypass-removal protection still apply. end note ``` - ## Key Decision Points | State | Condition | Action | |-------|-----------|--------| | **Orphan w/ Runner ID** | GitHub: offline + busy | Terminate (confirmed orphan) | | **Orphan w/ Runner ID** | GitHub: exists + healthy | Remove orphan tag (false positive) | -| **Orphan w/o Runner ID** | Always | Terminate (no way to verify) | +| **Orphan w/o Runner ID** | Exact-name lookup confirms absence | Terminate | +| **Orphan w/o identity** | Cannot verify registration | Preserve for a later sweep | | **Active Runner Found** | Runtime < minimum | Keep (too young) | | **Active Runner Found** | Idle quota available | Keep as idle | | **Active Runner Found** | Quota full + idle | Terminate + deregister | diff --git a/modules/runners/scale-down.tf b/modules/runners/scale-down.tf index ff7c91dff8..8b179b46bc 100644 --- a/modules/runners/scale-down.tf +++ b/modules/runners/scale-down.tf @@ -39,6 +39,7 @@ resource "aws_lambda_function" "scale_down" { PARAMETER_GITHUB_APPS_MANIFEST_NAME = var.github_app_parameters.additional_apps_manifest != null ? var.github_app_parameters.additional_apps_manifest.name : "" POWERTOOLS_LOGGER_LOG_EVENT = var.log_level == "debug" ? "true" : "false" RUNNER_BOOT_TIME_IN_MINUTES = var.runner_boot_time_in_minutes + RUNNER_NAME_PREFIX = var.runner_name_prefix SCALE_DOWN_CONFIG = jsonencode(var.idle_config) SCALE_DOWN_IDLE_CONFIRMATION_SECONDS = var.scale_down_idle_confirmation_seconds POWERTOOLS_SERVICE_NAME = "${var.prefix}-scale-down" diff --git a/modules/runners/tests/pool.tftest.hcl b/modules/runners/tests/pool.tftest.hcl index 2471aa5b16..e47fa30d70 100644 --- a/modules/runners/tests/pool.tftest.hcl +++ b/modules/runners/tests/pool.tftest.hcl @@ -69,6 +69,11 @@ variables { } run "plan_with_pool_enabled" { + assert { + condition = aws_lambda_function.scale_down.environment[0].variables["RUNNER_NAME_PREFIX"] == var.runner_name_prefix + error_message = "Scale-down must receive the configured runner name prefix." + } + command = plan assert {