Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,18 @@ Cron expressions are parsed by [cron-parser](https://github.com/harrisiirak/cron

For time zones please check [TZ database name column](https://en.wikipedia.org/wiki/List_of_tz_database_time_zones) for the supported values.

### Idle confirmation window <!-- omit in toc -->

Before terminating a runner, the scale-down lambda asks GitHub whether the runner is busy. That busy flag can be stale: it can read `false` for a runner that was assigned a job a few seconds earlier, and in rare cases for a runner that has been executing a job for several minutes. When that happens the lambda terminates an instance mid-job and the job fails with "The runner has received a shutdown signal".

Set `scale_down_idle_confirmation_seconds` to require not-busy readings that span at least the given window before a runner is terminated. On the first not-busy reading the lambda tags the instance with `ghr:idle_detected_at` and defers termination. It terminates only when a later evaluation still reads not-busy and the window has elapsed. Any busy reading in between removes the tag and restarts the window. Use at least one scale-down schedule interval, for example `300` for the default five minute schedule, so that two consecutive evaluations must agree. The trade-off is that a genuinely idle runner lives one extra interval before it is removed.

```hcl
scale_down_idle_confirmation_seconds = 300
```

The default of `0` keeps the previous single-reading behaviour. The `multi_runner_config` equivalent is `runner_config.scale_down_idle_confirmation_seconds`.

## Ephemeral runners

You can configure runners to be ephemeral, in which case runners will be used only for one job. The feature should be used in conjunction with listening for the workflow job event. Please consider the following:
Expand Down
1 change: 1 addition & 0 deletions lambdas/functions/control-plane/src/modules.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ declare namespace NodeJS {
LOG_LEVEL: 'silly' | 'trace' | 'debug' | 'info' | 'warn' | 'error' | 'fatal';
LOG_TYPE: 'json' | 'pretty' | 'hidden';
MINIMUM_RUNNING_TIME_IN_MINUTES: string;
SCALE_DOWN_IDLE_CONFIRMATION_SECONDS?: string;
PARAMETER_GITHUB_APP_CLIENT_ID_NAME: string;
PARAMETER_GITHUB_APP_CLIENT_SECRET_NAME: string;
PARAMETER_GITHUB_APP_ID_NAME: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ const computeProviders = providerTypes.map((type) => ({
bootTimeExceeded: vi.fn(),
markOrphan: vi.fn(),
unmarkOrphan: vi.fn(),
markIdle: vi.fn(),
unmarkIdle: vi.fn(),
terminate: vi.fn(),
} satisfies ScaleDownComputeProvider,
}));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ const mockComputeProvider = {
bootTimeExceeded: vi.fn(),
markOrphan: vi.fn(),
unmarkOrphan: vi.fn(),
markIdle: vi.fn(),
unmarkIdle: vi.fn(),
terminate: vi.fn(),
} satisfies Omit<ScaleDownComputeProvider, 'type'>;

Expand All @@ -49,6 +51,8 @@ const mockListRunners = vi.mocked(mockComputeProvider.list);
const mockBootTimeExceeded = vi.mocked(mockComputeProvider.bootTimeExceeded);
const mockMarkOrphan = vi.mocked(mockComputeProvider.markOrphan);
const mockUnmarkOrphan = vi.mocked(mockComputeProvider.unmarkOrphan);
const mockMarkIdle = vi.mocked(mockComputeProvider.markIdle);
const mockUnmarkIdle = vi.mocked(mockComputeProvider.unmarkIdle);
const mockTerminateRunners = vi.mocked(mockComputeProvider.terminate);

const cleanEnv = process.env;
Expand Down Expand Up @@ -691,6 +695,94 @@ describe('Scale down runners', () => {
});
});
});

describe('Scale down with the idle confirmation window', () => {
const CONFIRMATION_SECONDS = 300;

beforeEach(() => {
process.env.SCALE_DOWN_IDLE_CONFIRMATION_SECONDS = CONFIRMATION_SECONDS.toString();
});

it('starts the window instead of terminating on the first not-busy reading', async () => {
const runners = [createRunnerTestData('idle-1', 'Org', MINIMUM_TIME_RUNNING_IN_MINUTES + 1, true, false, false)];
mockGitHubRunners(runners);
mockProviderRunners(runners);

await scaleDown();

expect(mockMarkIdle).toHaveBeenCalledWith(runners[0].id, expect.any(String));
expect(mockTerminateRunners).not.toHaveBeenCalled();
});

it('defers termination while the window has not elapsed', async () => {
const runners = [createRunnerTestData('idle-1', 'Org', MINIMUM_TIME_RUNNING_IN_MINUTES + 1, true, false, false)];
runners[0].idleDetectedAt = new Date(Date.now() - (CONFIRMATION_SECONDS - 240) * 1000).toISOString();
mockGitHubRunners(runners);
mockProviderRunners(runners);

await scaleDown();

expect(mockMarkIdle).not.toHaveBeenCalled();
expect(mockTerminateRunners).not.toHaveBeenCalled();
});

it('terminates once not-busy readings span the window', async () => {
const runners = [createRunnerTestData('idle-1', 'Org', MINIMUM_TIME_RUNNING_IN_MINUTES + 1, true, false, true)];
runners[0].idleDetectedAt = new Date(Date.now() - (CONFIRMATION_SECONDS + 60) * 1000).toISOString();
mockGitHubRunners(runners);
mockProviderRunners(runners);

await scaleDown();

expect(mockTerminateRunners).toHaveBeenCalledWith(runners[0].id);
});

it('clears the window when a runner reads busy again', async () => {
// A busy reading is the signal that the earlier not-busy reading was stale; the window
// must restart from scratch rather than keep counting from the first observation.
const runners = [createRunnerTestData('idle-1', 'Org', MINIMUM_TIME_RUNNING_IN_MINUTES + 1, true, false, false)];
runners[0].idleDetectedAt = new Date(Date.now() - (CONFIRMATION_SECONDS + 60) * 1000).toISOString();
mockGitHubRunners(runners);
mockProviderRunners(runners);
mockOctokit.actions.getSelfHostedRunnerForOrg.mockImplementation(() => ({ data: { busy: true } }));
mockOctokit.actions.getSelfHostedRunnerForRepo.mockImplementation(() => ({ data: { busy: true } }));

await scaleDown();

expect(mockUnmarkIdle).toHaveBeenCalledWith(runners[0].id);
expect(mockMarkIdle).not.toHaveBeenCalled();
expect(mockTerminateRunners).not.toHaveBeenCalled();
});

it('clears the window for runners kept idle by the idle config', async () => {
// A kept-idle runner is never evaluated for removal, so a marker left on it would go stale
// and allow immediate termination once the idle count drops. The marker must be cleared.
process.env.SCALE_DOWN_CONFIG = JSON.stringify([
{ idleCount: 1, cron: '* * * * * *', timeZone: 'Europe/Amsterdam' },
]);
const runners = [createRunnerTestData('idle-1', 'Org', MINIMUM_TIME_RUNNING_IN_MINUTES + 1, true, false, false)];
runners[0].idleDetectedAt = new Date(Date.now() - (CONFIRMATION_SECONDS + 60) * 1000).toISOString();
mockGitHubRunners(runners);
mockProviderRunners(runners);

await scaleDown();

expect(mockUnmarkIdle).toHaveBeenCalledWith(runners[0].id);
expect(mockTerminateRunners).not.toHaveBeenCalled();
});

it('terminates on a single reading when the window is disabled (0)', async () => {
process.env.SCALE_DOWN_IDLE_CONFIRMATION_SECONDS = '0';
const runners = [createRunnerTestData('idle-1', 'Org', MINIMUM_TIME_RUNNING_IN_MINUTES + 1, true, false, true)];
mockGitHubRunners(runners);
mockProviderRunners(runners);

await scaleDown();

expect(mockMarkIdle).not.toHaveBeenCalled();
expect(mockTerminateRunners).toHaveBeenCalledWith(runners[0].id);
});
});
});

function mockProviderRunners(runners: RunnerTestItem[]) {
Expand Down
59 changes: 59 additions & 0 deletions lambdas/functions/control-plane/src/scale-runners/scale-down.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,58 @@ async function deleteGitHubRunner(
}
}

function idleConfirmationSeconds(): number {
const raw = process.env.SCALE_DOWN_IDLE_CONFIRMATION_SECONDS;
const parsed = raw === undefined || raw === '' ? 0 : Number(raw);
return Number.isFinite(parsed) && parsed > 0 ? parsed : 0;
}

// GitHub's busy flag can be stale: it reads false for runners that are actively executing
// a job, both shortly after job assignment (observed 25-60s lag) and deep into a running
// job (observed 12+ minutes). See #5085. A single busy=false reading is therefore not
// sufficient evidence that a runner is idle. When SCALE_DOWN_IDLE_CONFIRMATION_SECONDS > 0,
// require busy=false readings spanning at least that window before terminating; any
// busy=true reading in between resets the window (see clearIdleDetection).
async function idleConfirmed(runner: RunnerInfo, computeProvider: ScaleDownComputeProvider): Promise<boolean> {
const confirmationSeconds = idleConfirmationSeconds();
if (confirmationSeconds === 0) {
return true;
}
const idleDetectedAt = runner.idleDetectedAt;
const idleForSeconds = idleDetectedAt ? (Date.now() - Date.parse(idleDetectedAt)) / 1000 : NaN;
if (Number.isNaN(idleForSeconds)) {
// No marker yet, or an unparsable one: (re)start the confirmation window.
await computeProvider.markIdle(runner.id, new Date().toISOString());
logger.info(
`Runner '${runner.id}' reads idle; deferring termination for at least ` +
`${confirmationSeconds}s to confirm the busy state is not stale.`,
);
return false;
}
if (idleForSeconds < confirmationSeconds) {
logger.info(
`Runner '${runner.id}' reads idle since '${idleDetectedAt}' ` +
`(${Math.round(idleForSeconds)}s < ${confirmationSeconds}s); deferring termination.`,
);
return false;
}
logger.info(
`Runner '${runner.id}' confirmed idle since '${idleDetectedAt}' ` +
`(${Math.round(idleForSeconds)}s >= ${confirmationSeconds}s).`,
);
return true;
}

async function clearIdleDetection(runner: RunnerInfo, computeProvider: ScaleDownComputeProvider): Promise<void> {
if (idleConfirmationSeconds() === 0) {
return;
}
if (runner.idleDetectedAt) {
await computeProvider.unmarkIdle(runner.id);
logger.info(`Runner '${runner.id}' is busy again; idle-detection window reset.`);
}
}

async function removeRunner(
runner: RunnerInfo,
ghRunnerIds: number[],
Expand All @@ -192,6 +244,9 @@ async function removeRunner(
);

if (states.every((busy) => busy === false)) {
if (!(await idleConfirmed(runner, computeProvider))) {
return;
}
const results = await Promise.all(
ghRunnerIds.map((ghRunnerId) => deleteGitHubRunner(githubInstallationClient, runner, ghRunnerId)),
);
Expand All @@ -213,6 +268,7 @@ async function removeRunner(
);
}
} else {
await clearIdleDetection(runner, computeProvider);
logger.info(`Runner '${runner.id}' cannot be de-registered, because it is still busy.`);
}
} catch (e) {
Expand Down Expand Up @@ -251,6 +307,9 @@ async function evaluateAndRemoveRunners(
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.`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ export function defineScaleDownContractTests<TType extends string>({
vi.mocked(provider.bootTimeExceeded).mockReturnValue(false);
vi.mocked(provider.markOrphan).mockResolvedValue();
vi.mocked(provider.unmarkOrphan).mockResolvedValue();
vi.mocked(provider.markIdle).mockResolvedValue();
vi.mocked(provider.unmarkIdle).mockResolvedValue();
vi.mocked(provider.terminate).mockResolvedValue();
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';

import type { RunnerInfo, RunnerType } from '../../../../core';
import { createEc2ScaleDownCapability } from './scale-down';
import { IDLE_DETECTED_TAG, createEc2ScaleDownCapability } from './scale-down';
import type { Ec2RunnerResourceOperations } from '../runners';

const mockListRunners = vi.fn<Ec2RunnerResourceOperations['list']>();
Expand Down Expand Up @@ -61,6 +61,19 @@ describe('Scale down runners', () => {
expect(mockUntagRunner).toHaveBeenCalledWith(runner.id, [{ Key: 'ghr:orphan', Value: 'true' }]);
});

it('Should persist and clear the idle-detection marker as an instance tag.', async () => {
mockTagRunner.mockResolvedValue();
mockUntagRunner.mockResolvedValue();
const detectedAt = '2026-08-05T10:05:00.000Z';

await capability.markIdle(runner.id, detectedAt);
await capability.unmarkIdle(runner.id);

expect(mockTagRunner).toHaveBeenCalledWith(runner.id, [{ Key: IDLE_DETECTED_TAG, Value: detectedAt }]);
expect(mockUntagRunner).toHaveBeenCalledWith(runner.id, [{ Key: IDLE_DETECTED_TAG }]);
expect(mockTerminateRunner).not.toHaveBeenCalled();
});

it(`Should respect booting runner.`, async () => {
const scaleDownRunner: RunnerInfo = {
...runner,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
import type { ScaleDownComputeProvider } from '../../../../core';
import { bootTimeExceeded, type Ec2RunnerResourceOperations } from '../runners';

/**
* Idle-confirmation window (see ScaleDownComputeProvider.markIdle). EC2 persists the
* observation as an instance tag, so it survives between scale-down invocations without
* any extra state store — the same mechanism `ghr:orphan` uses.
*/
export const IDLE_DETECTED_TAG = 'ghr:idle_detected_at';

export function createEc2ScaleDownCapability(
ec2Operations: Ec2RunnerResourceOperations,
): Omit<ScaleDownComputeProvider, 'type'> {
Expand All @@ -9,6 +16,8 @@ export function createEc2ScaleDownCapability(
bootTimeExceeded,
markOrphan: (id) => ec2Operations.tag(id, [{ Key: 'ghr:orphan', Value: 'true' }]),
unmarkOrphan: (id) => ec2Operations.untag(id, [{ Key: 'ghr:orphan', Value: 'true' }]),
markIdle: (id, at) => ec2Operations.tag(id, [{ Key: IDLE_DETECTED_TAG, Value: at }]),
unmarkIdle: (id) => ec2Operations.untag(id, [{ Key: IDLE_DETECTED_TAG }]),
terminate: (id) => ec2Operations.terminate(id),
};
}
1 change: 1 addition & 0 deletions lambdas/libs/compute-providers/aws/ec2/src/runners.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ function getRunnerInfo(runningInstances: DescribeInstancesResult) {
orphan: i.Tags?.find((e) => e.Key === 'ghr:orphan')?.Value === 'true',
githubRunnerId: i.Tags?.find((e) => e.Key === 'ghr:github_runner_id')?.Value as string,
bypassRemoval: i.Tags?.find((e) => e.Key === 'ghr:bypass-removal')?.Value === 'true',
idleDetectedAt: i.Tags?.find((e) => e.Key === 'ghr:idle_detected_at')?.Value,
});
}
}
Expand Down
13 changes: 13 additions & 0 deletions lambdas/libs/compute-providers/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ export interface RunnerInfo {
orphan?: boolean;
githubRunnerId?: string;
bypassRemoval?: boolean;
/**
* When scale-down first observed this runner reporting idle, as an ISO-8601 string.
* Set and cleared via `markIdle` / `unmarkIdle`; absent when no marker is recorded.
*/
idleDetectedAt?: string;
}

export interface ListRunnerFilters {
Expand All @@ -105,6 +110,14 @@ export interface ScaleDownComputeProvider extends ComputeProvider {
markOrphan(id: string): Promise<void>;
unmarkOrphan(id: string): Promise<void>;
terminate(id: string): Promise<void>;
/**
* Record that the runner was observed idle at `at` (ISO-8601), so a later cycle can tell
* how long it has read idle. Surfaces back on `RunnerInfo.idleDetectedAt`. Only called
* when the idle-confirmation window (SCALE_DOWN_IDLE_CONFIRMATION_SECONDS) is enabled.
*/
markIdle(id: string, at: string): Promise<void>;
/** Clear the idle marker — the runner was seen busy again, so the window restarts. */
unmarkIdle(id: string): Promise<void>;
}

export interface RunnerStatus {
Expand Down
2 changes: 2 additions & 0 deletions lambdas/libs/compute-providers/registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ it('exposes every configured provider through both capability registries', () =>
bootTimeExceeded: expect.any(Function),
markOrphan: expect.any(Function),
unmarkOrphan: expect.any(Function),
markIdle: expect.any(Function),
unmarkIdle: expect.any(Function),
terminate: expect.any(Function),
});
expect(webhookProviderRegistry.capability(type, 'dynamicLabels').getViolations).toEqual(expect.any(Function));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ export function createTemplateScaleDownCapability(): Omit<ScaleDownComputeProvid
},
markOrphan: async (id) => notImplemented(`scaleDown.markOrphan(${id})`),
unmarkOrphan: async (id) => notImplemented(`scaleDown.unmarkOrphan(${id})`),
// Persist and clear the idle-detection marker used by the scale-down idle-confirmation
// window (SCALE_DOWN_IDLE_CONFIRMATION_SECONDS). Only called when the window is enabled.
markIdle: async (id, at) => notImplemented(`scaleDown.markIdle(${id}, ${at})`),
unmarkIdle: async (id) => notImplemented(`scaleDown.unmarkIdle(${id})`),
terminate: async (id) => notImplemented(`scaleDown.terminate(${id})`),
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ it('exposes every compute provider capability from its compute-provider entry po
bootTimeExceeded: expect.any(Function),
markOrphan: expect.any(Function),
unmarkOrphan: expect.any(Function),
markIdle: expect.any(Function),
unmarkIdle: expect.any(Function),
terminate: expect.any(Function),
});
expect(webhookPlugin.type).toBe(webhookProvider.type);
Expand Down
1 change: 1 addition & 0 deletions main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ module "runners" {
scale_down_schedule_expression = var.scale_down_schedule_expression
minimum_running_time_in_minutes = var.minimum_running_time_in_minutes
runner_boot_time_in_minutes = var.runner_boot_time_in_minutes
scale_down_idle_confirmation_seconds = var.scale_down_idle_confirmation_seconds
runner_disable_default_labels = var.runner_disable_default_labels
runner_labels = local.runner_labels
runner_as_root = var.runner_as_root
Expand Down
4 changes: 4 additions & 0 deletions modules/multi-runner/config.experimental.resolved.tf
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ locals {
v.orchestration_provider.webhook.lambda.scale.down.idle_config,
local.normalized_config.orchestration_provider.webhook.lambda.scale.down.idle_config,
)
idle_confirmation_seconds = coalesce(
v.orchestration_provider.webhook.lambda.scale.down.idle_confirmation_seconds,
local.normalized_config.orchestration_provider.webhook.lambda.scale.down.idle_confirmation_seconds,
)
tags = merge(local.normalized_config.orchestration_provider.webhook.lambda.scale.down.tags, v.orchestration_provider.webhook.lambda.scale.down.tags)
})
})
Expand Down
2 changes: 2 additions & 0 deletions modules/multi-runner/config.experimental.translation.tf
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ locals {
schedule_expression = "cron(*/5 * * * ? *)"
minimum_running_time_in_minutes = null
idle_config = []
idle_confirmation_seconds = 0
tags = {}
}
}
Expand Down Expand Up @@ -390,6 +391,7 @@ locals {
schedule_expression = v.runner_config.scale_down_schedule_expression
minimum_running_time_in_minutes = v.runner_config.minimum_running_time_in_minutes
idle_config = v.runner_config.idle_config
idle_confirmation_seconds = v.runner_config.scale_down_idle_confirmation_seconds
tags = {}
}
}
Expand Down
Loading
Loading