Skip to content

Commit 34d63fb

Browse files
edersonbrilhanteedersonbrilhante
authored
refactor(runners): introduce provider plugin framework (github-aws-runners#5234)
## Description Introduce a shared runner-provider plugin framework so a provider is enabled once and its webhook, scale-up, scale-down, pool, and runner-resource capabilities are available through one registry facade. - Move the EC2 provider implementation into the shared `runner-providers` library. - Add a single provider configuration and capability registry for webhook and control-plane consumers. - Add reusable scale-up, scale-down, and pool contract tests that run for every enabled provider lane. - Add a provider skeleton that documents the interfaces required to implement another lane. This reduces the number of control-plane and webhook locations that must be changed when adding a runner provider. ## Follow-up work Some tests still need to move to the control-plane package instead of remaining with the EC2 plugin. The affected files contain groups of tests, so this cleanup requires moving individual test cases rather than relocating an entire block or file. It will be handled in a follow-up PR to keep this refactor reviewable. This is why some EC2 provider tests currently import from `../../../../../../functions/control-plane`. Those dependencies should be removed in the follow-up PR. ## Test Plan - Ran the `runner-providers:test` Nx target. - Ran the `control-plane:test` Nx target. - Ran Prettier checks for the runner-provider library. - Ran ESLint for the runner-provider library. ## Related Issues Follow-up: github-aws-runners#5236.
1 parent 2394f84 commit 34d63fb

67 files changed

Lines changed: 1282 additions & 442 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎lambdas/functions/control-plane/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
"dependencies": {
3333
"@aws-github-runner/aws-powertools-util": "*",
3434
"@aws-github-runner/aws-ssm-util": "*",
35-
"@aws-github-runner/runner-provider": "*",
35+
"@aws-github-runner/runner-providers": "*",
3636
"@aws-lambda-powertools/parameters": "^2.31.0",
3737
"@aws-sdk/client-ec2": "^3.1009.0",
3838
"@aws-sdk/client-sqs": "^3.1009.0",

‎lambdas/functions/control-plane/src/control-plane-providers.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { createControlPlaneProviderRegistry } from '@aws-github-runner/runner-providers/control-plane';
2+
import { runnerProviderTypes } from '@aws-github-runner/runner-providers/provider-types';
3+
4+
import { createStartRunnerConfig } from './scale-runners/github-runner';
5+
6+
export const controlPlaneProviderRegistry = createControlPlaneProviderRegistry(createStartRunnerConfig);
7+
8+
export { runnerProviderTypes };

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

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import type { Octokit } from '@octokit/rest';
2+
import type { RunnerProviderType } from '@aws-github-runner/runner-providers/provider-types';
3+
import { beforeEach, vi } from 'vitest';
4+
5+
import { definePoolContractTests } from '../test/runner-provider-contracts/pool';
6+
import { providerTypes } from '../test/runner-provider-contracts/provider-types';
7+
import * as ghAuth from '../github/auth';
8+
import { controlPlaneProviderRegistry } from '../control-plane-providers';
9+
import * as githubRunner from '../scale-runners/github-runner';
10+
import { adjust } from './pool';
11+
import type { PoolRunnerProvider } from './pool-provider';
12+
13+
vi.mock('../github/auth', () => ({
14+
createGithubAppAuth: vi.fn(),
15+
createGithubInstallationAuth: vi.fn(),
16+
createOctokitClient: vi.fn(),
17+
}));
18+
19+
vi.mock('../scale-runners/github-runner', () => ({
20+
createStartRunnerConfig: vi.fn(),
21+
getGitHubEnterpriseApiUrl: vi.fn(),
22+
validateSsmParameterStoreTags: vi.fn(),
23+
}));
24+
25+
const mockedAppAuth = vi.mocked(ghAuth.createGithubAppAuth);
26+
const mockedInstallationAuth = vi.mocked(ghAuth.createGithubInstallationAuth);
27+
const mockedCreateClient = vi.mocked(ghAuth.createOctokitClient);
28+
const mockedResolveCapability = vi.spyOn(controlPlaneProviderRegistry, 'capability');
29+
30+
const githubClient = {
31+
actions: { listSelfHostedRunnersForOrg: vi.fn() },
32+
apps: { getOrgInstallation: vi.fn() },
33+
paginate: vi.fn(),
34+
} as unknown as Octokit;
35+
36+
const cleanEnv = process.env;
37+
38+
const lanes = providerTypes.map((type) => ({
39+
provider: {
40+
type,
41+
listRunners: vi.fn(),
42+
countAvailableRunners: vi.fn(),
43+
createRunners: vi.fn(),
44+
} satisfies PoolRunnerProvider,
45+
}));
46+
47+
beforeEach(() => {
48+
vi.clearAllMocks();
49+
process.env = { ...cleanEnv };
50+
51+
mockedAppAuth.mockResolvedValue({ type: 'app', token: 'app-token', appId: 1, expiresAt: 'some-date' });
52+
mockedInstallationAuth.mockResolvedValue({
53+
type: 'token',
54+
tokenType: 'installation',
55+
token: 'installation-token',
56+
createdAt: 'some-date',
57+
expiresAt: 'some-date',
58+
permissions: {},
59+
repositorySelection: 'selected',
60+
installationId: 2,
61+
});
62+
mockedCreateClient.mockResolvedValue(githubClient);
63+
vi.mocked(githubRunner.getGitHubEnterpriseApiUrl).mockReturnValue({ ghesApiUrl: '', ghesBaseUrl: '' });
64+
vi.mocked(githubRunner.validateSsmParameterStoreTags).mockReturnValue([]);
65+
vi.mocked(githubClient.apps.getOrgInstallation).mockResolvedValue({ data: { id: 2 } } as never);
66+
vi.mocked(githubClient.paginate).mockResolvedValue([]);
67+
});
68+
69+
definePoolContractTests<RunnerProviderType>({
70+
adjust,
71+
githubInstallationClient: githubClient,
72+
lanes,
73+
resolveCapability: mockedResolveCapability,
74+
});

‎lambdas/functions/control-plane/src/pool/pool-provider.ts‎

Lines changed: 6 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,6 @@
1-
import type { Octokit } from '@octokit/rest';
2-
import type { RunnerProvider } from '@aws-github-runner/runner-provider';
3-
4-
import type { CreateGitHubRunnerConfig, GitHubRunnerType } from '../scale-runners/types';
5-
6-
export interface RunnerStatus {
7-
busy: boolean;
8-
status: string;
9-
}
10-
11-
export interface ListPoolRunnersInput {
12-
environment: string;
13-
runnerOwner: string;
14-
runnerType: GitHubRunnerType;
15-
}
16-
17-
export interface CreatePoolRunnersInput {
18-
githubRunnerConfig: CreateGitHubRunnerConfig;
19-
numberOfRunners: number;
20-
githubInstallationClient: Octokit;
21-
}
22-
23-
export interface PoolRunnerProvider<TRunner = unknown> extends RunnerProvider {
24-
listRunners(input: ListPoolRunnersInput): Promise<TRunner[]>;
25-
countAvailableRunners(
26-
runners: TRunner[],
27-
runnerStatus: Map<string, RunnerStatus>,
28-
includeBusyRunners: boolean,
29-
): number;
30-
createRunners(input: CreatePoolRunnersInput): Promise<string[]>;
31-
}
1+
export type {
2+
CreatePoolRunnersInput,
3+
ListPoolRunnersInput,
4+
PoolRunnerProvider,
5+
RunnerStatus,
6+
} from '@aws-github-runner/runner-providers/core';

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

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ import { Octokit } from '@octokit/rest';
22
import moment from 'moment-timezone';
33
import * as nock from 'nock';
44

5-
import { listEC2Runners } from '../aws/ec2-runners';
5+
import { createRunners } from '@aws-github-runner/runner-providers/aws/ec2/control-plane/runner-config';
6+
import { listEC2Runners } from '@aws-github-runner/runner-providers/aws/ec2/control-plane/runners';
67
import * as ghAuth from '../github/auth';
7-
import { createRunners } from '../scale-runners/ec2';
88
import { getGitHubEnterpriseApiUrl } from '../scale-runners/github-runner';
99
import { adjust } from './pool';
1010
import { describe, it, expect, beforeEach, vi, MockedClass } from 'vitest';
@@ -26,7 +26,7 @@ vi.mock('@octokit/rest', () => ({
2626
}),
2727
}));
2828

29-
vi.mock('./../aws/ec2-runners', async () => ({
29+
vi.mock('@aws-github-runner/runner-providers/aws/ec2/control-plane/runners', async () => ({
3030
listEC2Runners: vi.fn(),
3131
// Include any other functions from the module that might be used
3232
bootTimeExceeded: vi.fn(),
@@ -37,12 +37,13 @@ vi.mock('./../github/auth', async () => ({
3737
createOctokitClient: vi.fn(),
3838
}));
3939

40-
vi.mock('../scale-runners/ec2', async (importOriginal) => ({
41-
...(await importOriginal<typeof import('../scale-runners/ec2')>()),
40+
vi.mock('@aws-github-runner/runner-providers/aws/ec2/control-plane/runner-config', async (importOriginal) => ({
41+
...(await importOriginal<typeof import('@aws-github-runner/runner-providers/aws/ec2/control-plane/runner-config')>()),
4242
createRunners: vi.fn(),
4343
}));
4444

4545
vi.mock('../scale-runners/github-runner', async () => ({
46+
createStartRunnerConfig: vi.fn(),
4647
getGitHubEnterpriseApiUrl: vi.fn().mockReturnValue({
4748
ghesApiUrl: '',
4849
ghesBaseUrl: '',
@@ -206,6 +207,7 @@ describe('Test simple pool.', () => {
206207
expect.anything(),
207208
1,
208209
expect.anything(),
210+
expect.anything(),
209211
'pool-lambda',
210212
);
211213
});
@@ -223,6 +225,7 @@ describe('Test simple pool.', () => {
223225
expect.anything(),
224226
8,
225227
expect.anything(),
228+
expect.anything(),
226229
'pool-lambda',
227230
);
228231
});
@@ -234,6 +237,7 @@ describe('Test simple pool.', () => {
234237
expect.anything(),
235238
8,
236239
expect.anything(),
240+
expect.anything(),
237241
'pool-lambda',
238242
);
239243
});
@@ -323,6 +327,7 @@ describe('Test simple pool.', () => {
323327
expect.anything(),
324328
3,
325329
expect.anything(),
330+
expect.anything(),
326331
'pool-lambda',
327332
);
328333
});
@@ -344,6 +349,7 @@ describe('Test simple pool.', () => {
344349
expect.anything(),
345350
3,
346351
expect.anything(),
352+
expect.anything(),
347353
'pool-lambda',
348354
);
349355
});
@@ -400,6 +406,7 @@ describe('Test simple pool.', () => {
400406
expect.anything(),
401407
1,
402408
expect.anything(),
409+
expect.anything(),
403410
'pool-lambda',
404411
);
405412
});
@@ -437,6 +444,7 @@ describe('Test simple pool.', () => {
437444
expect.anything(),
438445
2,
439446
expect.anything(),
447+
expect.anything(),
440448
'pool-lambda',
441449
);
442450
});
@@ -451,6 +459,7 @@ describe('Test simple pool.', () => {
451459
expect.anything(),
452460
1,
453461
expect.anything(),
462+
expect.anything(),
454463
'pool-lambda',
455464
);
456465
});
@@ -464,6 +473,7 @@ describe('Test simple pool.', () => {
464473
expect.anything(),
465474
8,
466475
expect.anything(),
476+
expect.anything(),
467477
'pool-lambda',
468478
);
469479
});
@@ -488,6 +498,7 @@ describe('Test simple pool.', () => {
488498
expect.anything(),
489499
2,
490500
expect.anything(),
501+
expect.anything(),
491502
'pool-lambda',
492503
);
493504
});

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import { Octokit } from '@octokit/rest';
22
import { createChildLogger } from '@aws-github-runner/aws-powertools-util';
3-
import { resolveRunnerProviderType } from '@aws-github-runner/runner-provider';
3+
import { resolveRunnerProviderType } from '@aws-github-runner/runner-providers/provider-types';
44
import yn from 'yn';
55

66
import { createGithubAppAuth, createGithubInstallationAuth, createOctokitClient } from '../github/auth';
7-
import { createPoolRunnerProvider } from '../runner-provider-registry';
7+
import { controlPlaneProviderRegistry } from '../control-plane-providers';
88
import { getGitHubEnterpriseApiUrl, validateSsmParameterStoreTags } from '../scale-runners/github-runner';
99
import type { RunnerStatus } from './pool-provider';
1010

@@ -17,7 +17,10 @@ export interface PoolEvent {
1717

1818
export async function adjust(event: PoolEvent): Promise<void> {
1919
const runnerProviderType = resolveRunnerProviderType(event.type);
20-
const runnerProvider = createPoolRunnerProvider(runnerProviderType);
20+
const runnerProvider = {
21+
...controlPlaneProviderRegistry.capability(runnerProviderType, 'pool')(),
22+
type: runnerProviderType,
23+
};
2124
logger.info(`Checking current ${runnerProvider.type} pool size against pool of size: ${event.poolSize}`);
2225
const runnerLabels = process.env.RUNNER_LABELS || '';
2326
const runnerGroup = process.env.RUNNER_GROUP_NAME || '';

‎lambdas/functions/control-plane/src/runner-provider-registry.test.ts‎

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

‎lambdas/functions/control-plane/src/runner-provider-registry.ts‎

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

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import type { RunnerProviderType } from '@aws-github-runner/runner-providers/provider-types';
2+
import { beforeEach, vi } from 'vitest';
3+
4+
import { providerTypes } from '../test/runner-provider-contracts/provider-types';
5+
import { defineScaleDownContractTests } from '../test/runner-provider-contracts/scale-down';
6+
import { controlPlaneProviderRegistry } from '../control-plane-providers';
7+
import { scaleDown } from './scale-down';
8+
import type { ScaleDownRunnerProvider } from './scale-down-provider';
9+
10+
const mockedResolveCapability = vi.spyOn(controlPlaneProviderRegistry, 'capability');
11+
12+
const cleanEnv = process.env;
13+
14+
const lanes = providerTypes.map((type) => ({
15+
provider: {
16+
type,
17+
list: vi.fn(),
18+
bootTimeExceeded: vi.fn(),
19+
markOrphan: vi.fn(),
20+
unmarkOrphan: vi.fn(),
21+
terminate: vi.fn(),
22+
} satisfies ScaleDownRunnerProvider,
23+
}));
24+
25+
beforeEach(() => {
26+
vi.clearAllMocks();
27+
process.env = { ...cleanEnv };
28+
});
29+
30+
defineScaleDownContractTests<RunnerProviderType>({
31+
lanes,
32+
resolveCapability: mockedResolveCapability,
33+
scaleDown,
34+
});

0 commit comments

Comments
 (0)