Skip to content

Commit 783929f

Browse files
edersonbrilhanteedersonbrilhante
committed
refactor(runner-providers): centralize shared runner contracts
1 parent a3038d3 commit 783929f

23 files changed

Lines changed: 167 additions & 233 deletions

‎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.

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

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,12 @@ import * as ghAuth from '../github/auth';
1010
import { createStartRunnerConfig } from './github-runner';
1111
import { publishRetryMessage } from './job-retry';
1212
import * as scaleUpModule from './scale-up';
13-
import type { CreateScaleUpRunnersInput, CreateScaleUpRunnersResult, ScaleUpRunnerProvider } from './scale-up-provider';
14-
import type { ActionRequestMessageSQS } from './types';
13+
import type {
14+
ActionRequestMessageSQS,
15+
CreateRunnerResult,
16+
CreateScaleUpRunnersInput,
17+
ScaleUpRunnerProvider,
18+
} from './types';
1519
import { getParameter } from '@aws-github-runner/aws-ssm-util';
1620
import { beforeEach, describe, expect, it, vi } from 'vitest';
1721
import type { Octokit } from '@octokit/rest';
@@ -45,7 +49,7 @@ interface TestRunnerLookupInput {
4549
runnerOwner: string;
4650
}
4751

48-
const createRunner = vi.fn<(input: TestRunnerCreationInput) => Promise<CreateScaleUpRunnersResult>>();
52+
const createRunner = vi.fn<(input: TestRunnerCreationInput) => Promise<CreateRunnerResult>>();
4953
const listRunners = vi.fn<(input: TestRunnerLookupInput) => Promise<unknown[]>>();
5054
const mockCreateRunner = vi.mocked(createRunner);
5155
const mockListRunners = vi.mocked(listRunners);
@@ -96,10 +100,10 @@ vi.mock('./job-retry', () => ({
96100
checkAndRetryJob: vi.fn(),
97101
}));
98102

99-
export type RunnerType = 'ephemeral' | 'non-ephemeral';
103+
export type RunnerLifecycle = 'ephemeral' | 'non-ephemeral';
100104

101105
// for ephemeral and non-ephemeral runners
102-
const RUNNER_TYPES: RunnerType[] = ['ephemeral', 'non-ephemeral'];
106+
const RUNNER_TYPES: RunnerLifecycle[] = ['ephemeral', 'non-ephemeral'];
103107

104108
const mockedAppAuth = vi.mocked(ghAuth.createGithubAppAuth);
105109
const mockedInstallationAuth = vi.mocked(ghAuth.createGithubInstallationAuth);
@@ -143,9 +147,7 @@ function setDefaults() {
143147
process.env.ENVIRONMENT = EXPECTED_RUNNER_PARAMS.environment;
144148
}
145149

146-
async function createTestProviderRunners(
147-
input: CreateScaleUpRunnersInput<unknown>,
148-
): Promise<CreateScaleUpRunnersResult> {
150+
async function createTestProviderRunners(input: CreateScaleUpRunnersInput<unknown>): Promise<CreateRunnerResult> {
149151
const result = await mockCreateRunner({
150152
environment: process.env.ENVIRONMENT,
151153
runnerType: input.githubRunnerConfig.runnerType,
@@ -208,7 +210,7 @@ beforeEach(() => {
208210
});
209211
mockListRunners.mockImplementation(async () => [
210212
{
211-
instanceId: 'i-1234',
213+
id: 'i-1234',
212214
launchTime: new Date(),
213215
type: 'Org',
214216
owner: TEST_DATA_SINGLE.repositoryOwner,
@@ -285,7 +287,7 @@ describe('scaleUp with GHES', () => {
285287
// Simulate race condition where pool lambda created more runners than max
286288
mockListRunners.mockImplementation(async () =>
287289
Array.from({ length: 10 }, (_, i) => ({
288-
instanceId: `i-${i}`,
290+
id: `i-${i}`,
289291
launchTime: new Date(),
290292
type: 'Org',
291293
owner: TEST_DATA_SINGLE.repositoryOwner,
@@ -600,7 +602,7 @@ describe('scaleUp with GHES', () => {
600602

601603
it.each(RUNNER_TYPES)(
602604
'calls create start runner config of 40' + ' instances (ssm rate limit condition) to test time delay ',
603-
async (type: RunnerType) => {
605+
async (type: RunnerLifecycle) => {
604606
process.env.ENABLE_EPHEMERAL_RUNNERS = type === 'ephemeral' ? 'true' : 'false';
605607
process.env.RUNNERS_MAXIMUM_COUNT = '40';
606608
mockCreateRunner.mockImplementation(async () => {
@@ -909,7 +911,7 @@ describe('scaleUp with GHES', () => {
909911
process.env.RUNNERS_MAXIMUM_COUNT = '1'; // Set to 1 so with 1 existing, no new ones can be created
910912
mockListRunners.mockImplementation(async () => [
911913
{
912-
instanceId: 'i-existing',
914+
id: 'i-existing',
913915
launchTime: new Date(),
914916
type: 'Org',
915917
owner: TEST_DATA_SINGLE.repositoryOwner,
@@ -1390,7 +1392,7 @@ describe('scaleUp with public GH', () => {
13901392
process.env.RUNNERS_MAXIMUM_COUNT = '1'; // Set to 1 so with 1 existing, no new ones can be created
13911393
mockListRunners.mockImplementation(async () => [
13921394
{
1393-
instanceId: 'i-existing',
1395+
id: 'i-existing',
13941396
launchTime: new Date(),
13951397
type: 'Org',
13961398
owner: TEST_DATA_SINGLE.repositoryOwner,
@@ -1672,7 +1674,7 @@ describe('scaleUp with Github Data Residency', () => {
16721674
});
16731675
it.each(RUNNER_TYPES)(
16741676
'calls create start runner config of 40' + ' instances (ssm rate limit condition) to test time delay ',
1675-
async (type: RunnerType) => {
1677+
async (type: RunnerLifecycle) => {
16761678
process.env.ENABLE_EPHEMERAL_RUNNERS = type === 'ephemeral' ? 'true' : 'false';
16771679
process.env.RUNNERS_MAXIMUM_COUNT = '40';
16781680
mockCreateRunner.mockImplementation(async () => {
@@ -1864,7 +1866,7 @@ describe('scaleUp with Github Data Residency', () => {
18641866
process.env.RUNNERS_MAXIMUM_COUNT = '2';
18651867
mockListRunners.mockImplementation(async () => [
18661868
{
1867-
instanceId: 'i-existing',
1869+
id: 'i-existing',
18681870
launchTime: new Date(),
18691871
type: 'Org',
18701872
owner: TEST_DATA_SINGLE.repositoryOwner,

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@ import {
1414
validateSsmParameterStoreTags,
1515
} from './github-runner';
1616
import { publishRetryMessage } from './job-retry';
17-
import type { CreateScaleUpRunnersResult } from './scale-up-provider';
1817
import type {
1918
ActionRequestMessage,
2019
ActionRequestMessageRetry,
2120
ActionRequestMessageSQS,
2221
CreateGitHubRunnerConfig,
22+
CreateRunnerResult,
2323
} from './types';
2424

2525
const logger = createChildLogger('scale-up');
@@ -313,7 +313,7 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise<stri
313313
ssmParameterStoreTags,
314314
};
315315

316-
let createRunnersResult: CreateScaleUpRunnersResult;
316+
let createRunnersResult: CreateRunnerResult;
317317
try {
318318
createRunnersResult = await runnerProvider.createRunners({
319319
githubRunnerConfig,

0 commit comments

Comments
 (0)