Skip to content

Commit b30760b

Browse files
refactor(runner-providers): centralize shared runner contracts (github-aws-runners#5248)
## Description - Define the provider-neutral `RunnerType`, `RunnerInfo`, `ListRunnerFilters`, and `CreateRunnerResult` contracts in runner-provider core. - Use one `RunnerInfo` shape across control-plane and EC2, with `id`, `githubRunnerId`, and required `owner` and `type` fields. - Keep EC2-specific inputs and the EC2 status-filter extension in `runners.d.ts`. - Consolidate control-plane type re-exports in `scale-runners/types.ts` and remove the redundant scale-up and scale-down provider type shims. - Update the existing tests in place without adding or removing test cases. ### Compatibility note This PR intentionally normalizes the exported `listEC2Runners` result fields from `instanceId`/`runnerId` to `id`/`githubRunnerId` and removes the legacy core type aliases. This differs from the compatibility approach initially described in github-aws-runners#5247 and is included explicitly for review. ## Test Plan - `cd lambdas && NX_DAEMON=false yarn test` (all 8 projects passed) - Control-plane suite: 14 test files / 326 tests passed - Runner-provider suite: 10 test files / 270 tests passed - Focused scale-down contract, EC2 listing, and pool tests passed after the final type tightening - ESLint and Prettier passed for control-plane and runner-providers - Exact test-title inventory: 276 before and after, with no additions or removals - `git diff --check` Local TypeScript build validation remains blocked by the installed `@aws-sdk/client-ec2` declarations missing exports used throughout the existing EC2 sources; the same environment issue affects untouched code. ## Related Issues Closes github-aws-runners#5247 Stacked on github-aws-runners#5246
1 parent e45602b commit b30760b

26 files changed

Lines changed: 167 additions & 237 deletions

.github/workflows/codeql.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ on:
44
push:
55
branches: [ "main", "develop", "v1" ]
66
pull_request:
7-
branches: [ "main", "develop", "v1" ]
87
paths-ignore:
98
- '**/*.md'
109
schedule:

.github/workflows/lambda.yml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ name: Build lambdas
22

33
on:
44
pull_request:
5-
branches:
6-
- main
75
paths:
86
- 'lambdas/**'
97
- '.github/workflows/lambda.yml'

.github/workflows/ovs.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
name: OSV-Scanner
22
on:
33
pull_request:
4-
branches: [main]
54
merge_group:
65
branches: [main]
76

lambdas/functions/control-plane/src/pool/pool.test.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -64,25 +64,25 @@ const MINIMUM_TIME_RUNNING = 15;
6464

6565
const ec2InstancesRegistered = [
6666
{
67-
instanceId: 'i-1-idle',
67+
id: 'i-1-idle',
6868
launchTime: new Date(),
6969
type: 'Org',
7070
owner: ORG,
7171
},
7272
{
73-
instanceId: 'i-2-busy',
73+
id: 'i-2-busy',
7474
launchTime: new Date(),
7575
type: 'Org',
7676
owner: ORG,
7777
},
7878
{
79-
instanceId: 'i-3-offline',
79+
id: 'i-3-offline',
8080
launchTime: new Date(),
8181
type: 'Org',
8282
owner: ORG,
8383
},
8484
{
85-
instanceId: 'i-4-idle-older-than-minimum-time-running',
85+
id: 'i-4-idle-older-than-minimum-time-running',
8686
launchTime: moment(new Date())
8787
.subtract(MINIMUM_TIME_RUNNING + 3, 'minutes')
8888
.toDate(),
@@ -258,15 +258,15 @@ describe('Test simple pool.', () => {
258258
mockListRunners.mockImplementation(async () => [
259259
...ec2InstancesRegistered,
260260
{
261-
instanceId: 'i-4-still-booting',
261+
id: 'i-4-still-booting',
262262
launchTime: moment(new Date())
263263
.subtract(MINIMUM_TIME_RUNNING - 3, 'minutes')
264264
.toDate(),
265265
type: 'Org',
266266
owner: ORG,
267267
},
268268
{
269-
instanceId: 'i-5-orphan',
269+
id: 'i-5-orphan',
270270
launchTime: moment(new Date())
271271
.subtract(MINIMUM_TIME_RUNNING + 3, 'minutes')
272272
.toDate(),
@@ -289,15 +289,15 @@ describe('Test simple pool.', () => {
289289
mockListRunners.mockImplementation(async () => [
290290
...ec2InstancesRegistered,
291291
{
292-
instanceId: 'i-4-still-booting',
292+
id: 'i-4-still-booting',
293293
launchTime: moment(new Date())
294294
.subtract(MINIMUM_TIME_RUNNING - 3, 'minutes')
295295
.toDate(),
296296
type: 'Org',
297297
owner: ORG,
298298
},
299299
{
300-
instanceId: 'i-5-orphan',
300+
id: 'i-5-orphan',
301301
launchTime: moment(new Date())
302302
.subtract(MINIMUM_TIME_RUNNING + 3, 'minutes')
303303
.toDate(),
@@ -386,13 +386,13 @@ describe('Test simple pool.', () => {
386386
mockListRunners.mockImplementation(async () => [
387387
...ec2InstancesRegistered,
388388
{
389-
instanceId: 'i-5-idle',
389+
id: 'i-5-idle',
390390
launchTime: new Date(),
391391
type: 'Org',
392392
owner: ORG,
393393
},
394394
{
395-
instanceId: 'i-6-idle',
395+
id: 'i-6-idle',
396396
launchTime: new Date(),
397397
type: 'Org',
398398
owner: ORG,

lambdas/functions/control-plane/src/scale-runners/scale-down-contract.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { providerTypes } from '../test/runner-provider-contracts/provider-types'
55
import { defineScaleDownContractTests } from '../test/runner-provider-contracts/scale-down';
66
import { controlPlaneProviderRegistry } from '../control-plane-providers';
77
import { scaleDown } from './scale-down';
8-
import type { ScaleDownRunnerProvider } from './scale-down-provider';
8+
import type { ScaleDownRunnerProvider } from './types';
99

1010
const mockedResolveCapability = vi.spyOn(controlPlaneProviderRegistry, 'capability');
1111

lambdas/functions/control-plane/src/scale-runners/scale-down-provider.ts

Lines changed: 0 additions & 5 deletions
This file was deleted.

lambdas/functions/control-plane/src/scale-runners/scale-down.test.ts

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { controlPlaneProviderRegistry } from '../control-plane-providers';
77
import * as ghAuth from '../github/auth';
88
import { githubCache } from './cache';
99
import { newestFirstStrategy, oldestFirstStrategy, scaleDown } from './scale-down';
10-
import type { RunnerInfo, ScaleDownRunnerProvider } from './scale-down-provider';
10+
import type { RunnerInfo, RunnerType, ScaleDownRunnerProvider } from './types';
1111

1212
vi.mock('../github/auth', () => ({
1313
createGithubAppAuth: vi.fn(),
@@ -72,25 +72,25 @@ describe('When runners are sorted', () => {
7272
id: '1',
7373
launchTime: moment(new Date()).subtract(1, 'minute').toDate(),
7474
owner: 'owner',
75-
type: 'type',
75+
type: 'Org',
7676
},
7777
{
7878
id: '3',
7979
launchTime: moment(new Date()).subtract(3, 'minute').toDate(),
8080
owner: 'owner',
81-
type: 'type',
81+
type: 'Org',
8282
},
8383
{
8484
id: '2',
8585
launchTime: moment(new Date()).subtract(2, 'minute').toDate(),
8686
owner: 'owner',
87-
type: 'type',
87+
type: 'Org',
8888
},
8989
{
9090
id: '0',
9191
launchTime: moment(new Date()).subtract(0, 'minute').toDate(),
9292
owner: 'owner',
93-
type: 'type',
93+
type: 'Org',
9494
},
9595
];
9696

@@ -117,13 +117,13 @@ describe('When runners are sorted', () => {
117117
id: '4',
118118
launchTime: same,
119119
owner: 'owner',
120-
type: 'type',
120+
type: 'Org',
121121
});
122122
runnersTest.push({
123123
id: '5',
124124
launchTime: same,
125125
owner: 'owner',
126-
type: 'type',
126+
type: 'Org',
127127
});
128128
runnersTest.sort(oldestFirstStrategy);
129129
expect(runnersTest[3].launchTime).not.toEqual(same);
@@ -142,19 +142,19 @@ describe('When runners are sorted', () => {
142142
id: '0',
143143
launchTime: undefined,
144144
owner: 'owner',
145-
type: 'type',
145+
type: 'Org',
146146
},
147147
{
148148
id: '1',
149149
launchTime: moment(new Date()).subtract(3, 'minute').toDate(),
150150
owner: 'owner',
151-
type: 'type',
151+
type: 'Org',
152152
},
153153
{
154154
id: '0',
155155
launchTime: undefined,
156156
owner: 'owner',
157-
type: 'type',
157+
type: 'Org',
158158
},
159159
];
160160
runnersTest.sort(oldestFirstStrategy);
@@ -266,7 +266,6 @@ describe('Scale down runners', () => {
266266
}
267267
});
268268

269-
type RunnerType = 'Repo' | 'Org';
270269
const runnerTypes: RunnerType[] = ['Org', 'Repo'];
271270
describe.each(runnerTypes)('For %s runners.', (type) => {
272271
it(`Should terminate runner without idle config ${type} runners.`, async () => {
@@ -729,7 +728,7 @@ function mockGitHubRunners(runners: RunnerTestItem[]) {
729728

730729
function createRunnerTestData(
731730
name: string,
732-
type: 'Org' | 'Repo',
731+
type: RunnerType,
733732
minutesLaunchedAgo: number,
734733
registered: boolean,
735734
orphan: boolean,

lambdas/functions/control-plane/src/scale-runners/scale-down.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { GhRunners, githubCache } from './cache';
1111
import { ScalingDownConfigList, getEvictionStrategy, getIdleRunnerCount } from './scale-down-config';
1212
import { metricGitHubAppRateLimit } from '../github/rate-limit';
1313
import { getGitHubEnterpriseApiUrl } from './github-runner';
14-
import type { RunnerInfo, RunnerList, ScaleDownRunnerProvider } from './scale-down-provider';
14+
import type { RunnerInfo, ScaleDownRunnerProvider } from './types';
1515

1616
const logger = createChildLogger('scale-down');
1717

@@ -93,7 +93,7 @@ async function getGitHubRunnerBusyState(client: Octokit, runner: RunnerInfo, run
9393
}
9494

9595
async function listGitHubRunners(runner: RunnerInfo): Promise<GhRunners> {
96-
const key = runner.owner as string;
96+
const key = runner.owner;
9797
const cachedRunners = githubCache.runners.get(key);
9898
if (cachedRunners) {
9999
logger.debug(`[listGithubRunners] Cache hit for ${key}`);
@@ -278,11 +278,10 @@ async function unMarkOrphan(id: string, runnerProvider: ScaleDownRunnerProvider)
278278
}
279279
}
280280

281-
async function lastChanceCheckOrphanRunner(runner: RunnerList): Promise<boolean> {
282-
const registeredRunner = runner as RunnerInfo;
283-
const client = await getOrCreateOctokit(registeredRunner);
281+
async function lastChanceCheckOrphanRunner(runner: RunnerInfo): Promise<boolean> {
282+
const client = await getOrCreateOctokit(runner);
284283
const runnerId = parseInt(runner.githubRunnerId || '0');
285-
const state = await getGitHubSelfHostedRunnerState(client, registeredRunner, runnerId);
284+
const state = await getGitHubSelfHostedRunnerState(client, runner, runnerId);
286285
let isOrphan = false;
287286

288287
if (state === null) {
@@ -343,10 +342,10 @@ async function listRunners(environment: string, runnerProvider: ScaleDownRunnerP
343342
return await runnerProvider.list(environment);
344343
}
345344

346-
function filterRunners(runners: RunnerList[]): RunnerInfo[] {
345+
function filterRunners(runners: RunnerInfo[]): RunnerInfo[] {
347346
// Managed runners are launched with owner and type tags together. Exclude incomplete records because both
348347
// values are required to select the GitHub owner and runner API used during scale-down.
349-
return runners.filter((runner) => runner.owner && runner.type && !runner.orphan) as RunnerInfo[];
348+
return runners.filter((runner) => runner.owner && runner.type && !runner.orphan);
350349
}
351350

352351
export async function scaleDown(): Promise<void> {

lambdas/functions/control-plane/src/scale-runners/scale-up-contract.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@ import * as ghAuth from '../github/auth';
77
import { controlPlaneProviderRegistry } from '../control-plane-providers';
88
import * as githubRunner from './github-runner';
99
import { scaleUp } from './scale-up';
10-
import type { ScaleUpRunnerProvider } from './scale-up-provider';
11-
import type { ActionRequestMessageSQS } from './types';
10+
import type { ActionRequestMessageSQS, ScaleUpRunnerProvider } from './types';
1211

1312
vi.mock('../github/auth', () => ({
1413
createGithubAppAuth: vi.fn(),

lambdas/functions/control-plane/src/scale-runners/scale-up-provider.ts

Lines changed: 0 additions & 7 deletions
This file was deleted.

0 commit comments

Comments
 (0)