diff --git a/docs/rate-limits-and-tuning.md b/docs/rate-limits-and-tuning.md index 88c74bf6d5..263a0fdc1e 100644 --- a/docs/rate-limits-and-tuning.md +++ b/docs/rate-limits-and-tuning.md @@ -74,6 +74,14 @@ Rate limits are **disabled by default** on GitHub Enterprise Server and must be | `POST /actions/runners/generate-jitconfig` | `actions_runner_registration` | 1 per instance created | | `GET /actions/runners` (listSelfHostedRunners) | `core` | Scale-down, pool | +### Incremental scale-down API cost + +EC2 scale-down processes up to 100 instances per inventory page and looks up GitHub registrations individually by ID or exact name. This lets cleanup begin before the whole fleet is listed, but increases GitHub requests compared with the former owner-wide cached listing. A name lookup may itself paginate. Eligible removals also recheck busy state and delete the registration; authentication and orphan rechecks add requests. + +For a scan of N eligible instances every T minutes, identity lookups alone can approach `N × 60 / T` requests per hour. For example, 1,000 instances scanned every five minutes can require about 12,000 identity requests per hour, before removals or other scaling traffic. Retained instances may be checked again each run. Monitor App quota and throttling, choose a schedule that leaves room for scale-up/pool traffic, and configure additional Apps when appropriate. + +The EC2 inventory page size is 100: approximately ten `DescribeInstances` requests for 1,000 returned instances, excluding retries. These calls share EC2 API limits with other scaling operations. Page processing remains incremental, with a deadline check before new pages and runners; no scan progress is persisted. + ## AWS Rate Limits ### SSM Parameter Store 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..2047ccfdc7 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 @@ -10,6 +10,22 @@ import { githubCache } from './cache'; import { newestFirstStrategy, oldestFirstStrategy, scaleDown } from './scale-down'; import type { RunnerInfo, RunnerType, ScaleDownComputeProvider } from './types'; +const scaleLogs = vi.hoisted(() => ({ info: vi.fn(), error: vi.fn() })); +vi.mock('@aws-github-runner/aws-powertools-util', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + createChildLogger: (name: string) => { + const child = actual.createChildLogger(name); + if (name === 'scale-down') { + vi.spyOn(child, 'info').mockImplementation(scaleLogs.info); + vi.spyOn(child, 'error').mockImplementation(scaleLogs.error); + } + return child; + }, + }; +}); + vi.mock('../github/auth', () => ({ createGithubAppAuth: vi.fn(), createGithubInstallationAuth: vi.fn(), @@ -209,21 +225,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 +249,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 +278,207 @@ 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(); + }); + + it('retains a custom-name registration after an exact-name miss, including an existing orphan', async () => { + const runner = createRunnerTestData('custom-name', 'Org', 60, true, true, false); + delete runner.githubRunnerId; + mockOctokit.paginate.mockResolvedValue([]); + mockedResolveCapability.mockReturnValue(() => ({ + ...mockComputeProvider, + listPage: vi.fn().mockResolvedValue({ runners: [runner] }), + })); + await scaleDown(); + runner.orphan = true; + await scaleDown(); + expect(mockMarkOrphan).not.toHaveBeenCalled(); + expect(mockTerminateRunners).not.toHaveBeenCalled(); + expect(scaleLogs.error).toHaveBeenCalledWith( + expect.stringContaining('not found by expected name'), + expect.objectContaining({ code: 'UNVERIFIABLE_RUNNER', runnerId: runner.id }), + ); + }); + + it('rejects an unidentifiable paged runner before creating a GitHub client', async () => { + const runner = createRunnerTestData('no-identity', 'Org', 60, false, false, false); + delete runner.githubRunnerName; + delete runner.githubRunnerId; + mockedResolveCapability.mockReturnValue(() => ({ + ...mockComputeProvider, + listPage: vi.fn().mockResolvedValue({ runners: [runner] }), + })); + await scaleDown(); + expect(mockedAppAuth).not.toHaveBeenCalled(); + expect(mockCreateClient).not.toHaveBeenCalled(); + expect(scaleLogs.error).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ code: 'UNVERIFIABLE_RUNNER' }), + ); + }); + + it('logs paged scan totals and orphan termination', async () => { + const runner = createRunnerTestData('verified-orphan', 'Org', 60, false, false, true); + runner.orphan = true; + runner.githubRunnerId = '42'; + mockOctokit.actions.getSelfHostedRunnerForOrg.mockRejectedValue( + new RequestError('not found', 404, { + request: { method: 'GET', url: 'https://api.github.com/test', headers: {} }, + }), + ); + mockedResolveCapability.mockReturnValue(() => ({ + ...mockComputeProvider, + listPage: vi.fn().mockResolvedValue({ runners: [runner] }), + })); + await scaleDown(); + expect(scaleLogs.info).toHaveBeenCalledWith(`Terminating orphan runner '${runner.id}'.`); + expect(scaleLogs.info).toHaveBeenCalledWith( + 'Scale-down inventory scan finished.', + expect.objectContaining({ + pages: 1, + scannedRunners: 1, + terminatedRunners: 1, + completed: true, + stoppedForDeadline: false, + }), + ); + }); + + 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'); + expect(scaleLogs.info).toHaveBeenCalledWith( + 'Scale-down inventory scan finished.', + expect.objectContaining({ completed: false, pages: 1, terminatedRunners: 1 }), + ); + // 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(scaleLogs.info).toHaveBeenCalledWith( + expect.stringContaining('Stopping scale-down'), + expect.objectContaining({ remainingTimeMs: 0 }), + ); + expect(scaleLogs.info).toHaveBeenCalledWith( + 'Scale-down inventory scan finished.', + expect.objectContaining({ completed: false, stoppedForDeadline: true }), + ); + 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 +626,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 +1120,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..c2e391549a 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,62 @@ 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}`); +class UnverifiableRunnerError extends Error {} + +async function listGitHubRunners(runner: RunnerInfo, computeProvider: ScaleDownComputeProvider): Promise { + if (computeProvider.listPage && !hasGitHubRunnerId(runner) && runner.githubRunnerName === undefined) { + throw new UnverifiableRunnerError(`Runner '${runner.id}' has no trusted GitHub identity; retaining instance`); + } 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; + const matches = await client.paginate(method, { ...owner, name: runner.githubRunnerName, per_page: 100 }); + if (matches.length > 0) return matches; + // Custom start scripts may register another name. A filtered miss alone + // cannot prove that this instance has no live (possibly busy) registration. + if (computeProvider.listPage) { + throw new UnverifiableRunnerError(`Runner '${runner.id}' was not found by expected name; retaining instance`); + } + } + 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 UnverifiableRunnerError( + `Runner '${runner.id}' has no GitHub ID or complete runner name; retaining instance`, + ); } function runnerMinimumTimeExceeded(runner: RunnerInfo): boolean { @@ -283,8 +311,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 +325,43 @@ 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) { + if (error instanceof UnverifiableRunnerError) { + logger.error(error.message, { code: 'UNVERIFIABLE_RUNNER', runnerId: runner.id, owner: runner.owner }); + } else logger.warn(`Failed to evaluate runner '${runner.id}'; continuing cleanup.`, { error }); } } } @@ -367,31 +405,52 @@ 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.error(`Cannot verify orphan runner '${runner.id}' without its owner and type, skipping termination.`, { + code: 'UNVERIFIABLE_RUNNER', + runnerId: runner.id, + }); + 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) { + logger.info(`Terminating orphan runner '${runner.id}'.`); + 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. + if (error instanceof UnverifiableRunnerError) { + logger.error(error.message, { code: 'UNVERIFIABLE_RUNNER', runnerId: runner.id, owner: runner.owner }); + } else logger.warn(`Failed to process orphan runner '${runner.id}'.`, { error }); } - } catch (e) { - logger.warn(`Failure during orphan termination processing.`, { error: e }); } } @@ -417,8 +476,20 @@ 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 readRemainingTime = remainingTime; + let stoppedForDeadline = false; + remainingTime = () => { + const remaining = readRemainingTime(); + if (remaining < 10000 && !stoppedForDeadline) { + stoppedForDeadline = true; + logger.info('Stopping scale-down before the Lambda deadline; remaining work will be rediscovered next run.', { + remainingTimeMs: remaining, + }); + } + return remaining; + }; const environment = process.env.ENVIRONMENT; const scaleDownConfigs = JSON.parse(process.env.SCALE_DOWN_CONFIG) as ScalingDownConfigList; const computeProviderType = resolveComputeProviderType(process.env.COMPUTE_PROVIDER_TYPE); @@ -427,8 +498,59 @@ export async function scaleDown(): Promise { type: computeProviderType, }; + if (computeProvider.listPage) { + const sweep = { idleRemaining: getIdleRunnerCount(scaleDownConfigs) }; + let pages = 0; + let scannedRunners = 0; + let terminatedRunners = 0; + let completed = false; + const terminate = computeProvider.terminate; + computeProvider.terminate = async (id) => { + await terminate(id); + terminatedRunners++; + }; + let nextToken: string | undefined; + try { + do { + if (remainingTime() < 10000) return; + const page = await computeProvider.listPage(environment, nextToken); + pages++; + scannedRunners += page.runners.length; + logger.info( + `Found: '${page.runners.length}' ${computeProvider.type.toUpperCase()} runners in inventory page.`, + { page: pages, activeRunners: filterRunners(page.runners).length }, + ); + 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); + completed = !stoppedForDeadline; + } finally { + logger.info('Scale-down inventory scan finished.', { + provider: computeProvider.type, + pages, + scannedRunners, + terminatedRunners, + completed, + stoppedForDeadline, + }); + } + return; + } + // first runners marked to be orphan. - await terminateOrphan(environment, computeProvider); + await terminateOrphan(environment, computeProvider, undefined, remainingTime); // next scale down idle runners with respect to config and mark potential orphans const providerRunners = await listRunners(environment, computeProvider); @@ -444,7 +566,7 @@ export async function scaleDown(): Promise { } const runners = filterRunners(providerRunners); - await evaluateAndRemoveRunners(runners, scaleDownConfigs, computeProvider); + await evaluateAndRemoveRunners(runners, scaleDownConfigs, computeProvider, undefined, remainingTime); const activeProviderRunnersCountAfter = (await listRunners(environment, computeProvider)).length; logger.info( 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 1abea33b70..87ac1d344e 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, }); @@ -1811,3 +1817,42 @@ 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: 100, + 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(); + }, + ); +}); + +it('does not issue a cleanup page request after request cancellation', async () => { + mockEC2Client.reset(); + const controller = new AbortController(); + controller.abort(); + const client = createEc2RunnerClient(new EC2Client({})).forRequest({ signal: controller.signal }); + await expect(client.listPage!('test-environment')).rejects.toThrow(); + expect(mockEC2Client).not.toHaveReceivedCommand(DescribeInstancesCommand); +}); diff --git a/lambdas/libs/compute-providers/aws/ec2/src/runners.ts b/lambdas/libs/compute-providers/aws/ec2/src/runners.ts index b03eb9fec3..d19481104c 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,18 @@ async function runWithRequestSignal( export function createEc2RunnerClient(ec2Client: EC2Client): Ec2RunnerClient { return { forRequest: ({ signal }) => ({ + listPage: (environment, nextToken) => + runWithRequestSignal(signal, async () => { + const page = await ec2Client.send( + new DescribeInstancesCommand({ + Filters: constructFilters({ environment })[0], + NextToken: nextToken, + MaxResults: 100, + }), + { 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 +161,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 0ad066c6ea..2eba6651ac 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..d3bf6fc361 100644 --- a/modules/orchestration-providers/webhook/scale-down-state-diagram.md +++ b/modules/orchestration-providers/webhook/scale-down-state-diagram.md @@ -2,144 +2,47 @@ -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 (up to 100 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 - - TooYoung --> NextRunner +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 only when its `ghr:runner_name_prefix` tag matches the configured prefix, then appends the instance ID. An exact-name miss without a registration ID is retained because a custom script may have registered a different name. - state CheckIdleQuota <> - CheckIdleQuota --> KeepIdle : Idle quota available - CheckIdleQuota --> CheckBusyState : Quota full +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. - 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 misses | Retain and report unverifiable identity | +| **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 | | **Active Runner Found** | Quota full + busy | Keep running | -| **Active Runner Missing** | Boot time exceeded | Mark as orphan | +| **Active Runner Missing** | Absence verified by ID (or complete legacy listing), boot time exceeded | Mark as orphan | | **Active Runner Missing** | Still booting | Wait | ## Configuration Parameters @@ -148,3 +51,9 @@ stateDiagram-v2 - **Minimum Runtime**: Linux 5min, Windows 15min, OSX 20min - **Boot Timeout**: Configurable via `orchestration_provider.webhook.runner.boot_time_in_minutes`; stable-v1 inputs are translated from `runner_boot_time_in_minutes`. - **Idle Config**: Per-environment configuration for desired idle runners + +## Operational visibility and naming + +Paged cleanup emits an info log for each inventory page and a final `Scale-down inventory scan finished.` summary with `pages`, `scannedRunners`, `terminatedRunners`, `completed`, and `stoppedForDeadline`. These are observations from the scan, not an atomic before/after fleet census. Update dashboards that depend on the old fleet-count log messages to use these fields. Deadline exits and orphan termination also emit info logs; a listing failure still emits the partial summary. + +Missing-ID lookups require the configured prefix followed by the instance ID. Custom start scripts should retain the GitHub runner-ID tag. An exact-name miss does not prove absence: paged cleanup preserves the instance and reports `UNVERIFIABLE_RUNNER` at error level. Missing identity and mismatched prefix tags are reported the same way, before attempting authentication. Alert on this code and repair the identity/tag or investigate the instance manually; later scans do not silently turn uncertainty into permission to terminate. Existing list-only providers retain their complete-list fallback. 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..71cb2d69b2 100644 --- a/modules/runners/scale-down-state-diagram.md +++ b/modules/runners/scale-down-state-diagram.md @@ -2,144 +2,47 @@ -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 (up to 100 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 - - TooYoung --> NextRunner +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 only when its `ghr:runner_name_prefix` tag matches the configured prefix, then appends the instance ID. An exact-name miss without a registration ID is retained because a custom script may have registered a different name. - state CheckIdleQuota <> - CheckIdleQuota --> KeepIdle : Idle quota available - CheckIdleQuota --> CheckBusyState : Quota full +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. - 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 misses | Retain and report unverifiable identity | +| **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 | | **Active Runner Found** | Quota full + busy | Keep running | -| **Active Runner Missing** | Boot time exceeded | Mark as orphan | +| **Active Runner Missing** | Absence verified by ID (or complete legacy listing), boot time exceeded | Mark as orphan | | **Active Runner Missing** | Still booting | Wait | ## Configuration Parameters @@ -148,3 +51,9 @@ stateDiagram-v2 - **Minimum Runtime**: Linux 5min, Windows 15min, OSX 20min - **Boot Timeout**: Configurable via `runner_boot_time_in_minutes` - **Idle Config**: Per-environment configuration for desired idle runners + +## Operational visibility and naming + +Paged cleanup emits an info log for each inventory page and a final `Scale-down inventory scan finished.` summary with `pages`, `scannedRunners`, `terminatedRunners`, `completed`, and `stoppedForDeadline`. These are observations from the scan, not an atomic before/after fleet census. Update dashboards that depend on the old fleet-count log messages to use these fields. Deadline exits and orphan termination also emit info logs; a listing failure still emits the partial summary. + +Missing-ID lookups require the configured prefix followed by the instance ID. Custom start scripts should retain the GitHub runner-ID tag. An exact-name miss does not prove absence: paged cleanup preserves the instance and reports `UNVERIFIABLE_RUNNER` at error level. Missing identity and mismatched prefix tags are reported the same way, before attempting authentication. Alert on this code and repair the identity/tag or investigate the instance manually; later scans do not silently turn uncertainty into permission to terminate. Existing list-only providers retain their complete-list fallback. 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 874e19f2d6..223cadb138 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 {