Skip to content

Commit 4193ac0

Browse files
committed
Add unit test for scale down, add parameter to terraform, cleanup
1 parent 3839c94 commit 4193ac0

File tree

7 files changed

+252
-35
lines changed

7 files changed

+252
-35
lines changed

modules/runners/lambdas/scale-runners/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
"dependencies": {
2626
"@octokit/auth-app": "^2.4.5",
2727
"@octokit/rest": "^17.6.0",
28+
"moment": "^2.25.3",
2829
"yn": "^4.0.0"
2930
}
3031
}
Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
import { mocked } from 'ts-jest/utils';
2+
import { scaleDown } from './scale-down';
3+
import moment from 'moment';
4+
import { createAppAuth } from '@octokit/auth-app';
5+
import { Octokit } from '@octokit/rest';
6+
import { listRunners, terminateRunner } from './runners';
7+
8+
jest.mock('@octokit/auth-app', () => ({
9+
createAppAuth: jest.fn().mockImplementation(() => jest.fn().mockImplementation(() => ({ token: 'Blaat' }))),
10+
}));
11+
const mockOctokit = {
12+
apps: {
13+
getOrgInstallation: jest.fn(),
14+
getRepoInstallation: jest.fn(),
15+
},
16+
actions: {
17+
listSelfHostedRunnersForRepo: jest.fn(),
18+
listSelfHostedRunnersForOrg: jest.fn(),
19+
deleteSelfHostedRunnerFromOrg: jest.fn(),
20+
deleteSelfHostedRunnerFromRepo: jest.fn(),
21+
},
22+
};
23+
jest.mock('@octokit/rest', () => ({
24+
Octokit: jest.fn().mockImplementation(() => mockOctokit),
25+
}));
26+
27+
jest.mock('./runners');
28+
29+
export interface TestData {
30+
repositoryName: string;
31+
repositoryOwner: string;
32+
}
33+
34+
const environment = 'unit-test-environment';
35+
const TEST_DATA: TestData = {
36+
repositoryName: 'hello-world',
37+
repositoryOwner: 'Codertocat',
38+
};
39+
40+
const DEFAULT_RUNNERS = [
41+
{
42+
instanceId: 'i-idle-101',
43+
launchTime: new Date('2020-05-12T11:32:06.000Z'),
44+
repo: `${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`,
45+
org: undefined,
46+
},
47+
{
48+
instanceId: 'i-idle-102',
49+
launchTime: new Date('2020-05-12T10:32:06.000Z'),
50+
repo: `${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`,
51+
org: undefined,
52+
},
53+
{
54+
instanceId: 'i-running-103',
55+
launchTime: moment(new Date()).subtract(25, 'minutes').toDate(),
56+
repo: `doe/another-repo`,
57+
org: undefined,
58+
},
59+
{
60+
instanceId: 'i-not-registered-104',
61+
launchTime: moment(new Date()).subtract(5, 'minutes').toDate(),
62+
repo: `doe/another-repo`,
63+
org: undefined,
64+
},
65+
];
66+
67+
const DEFAULT_REGISTERED_RUNNERS: any = {
68+
data: {
69+
runners: [
70+
{
71+
id: 101,
72+
name: 'i-idle-101',
73+
},
74+
{
75+
id: 102,
76+
name: 'i-idle-101',
77+
},
78+
{
79+
id: 103,
80+
name: 'i-running-103',
81+
},
82+
],
83+
},
84+
};
85+
86+
describe('scaleDown', () => {
87+
beforeEach(() => {
88+
process.env.GITHUB_APP_KEY_BASE64 = 'TEST_CERTIFICATE_DATA';
89+
process.env.GITHUB_APP_ID = '1337';
90+
process.env.GITHUB_APP_CLIENT_ID = 'TEST_CLIENT_ID';
91+
process.env.GITHUB_APP_CLIENT_SECRET = 'TEST_CLIENT_SECRET';
92+
process.env.RUNNERS_MAXIMUM_COUNT = '3';
93+
process.env.ENVIRONMENT = environment;
94+
const minimumRunningTimeInMinutes = '15';
95+
process.env.MINIMUM_RUNNING_TIME_IN_MINUTES = minimumRunningTimeInMinutes;
96+
jest.clearAllMocks();
97+
mockOctokit.apps.getOrgInstallation.mockImplementation(() => ({
98+
data: {
99+
id: 'ORG',
100+
},
101+
}));
102+
mockOctokit.apps.getRepoInstallation.mockImplementation(() => ({
103+
data: {
104+
id: 'REPO',
105+
},
106+
}));
107+
108+
mockOctokit.actions.listSelfHostedRunnersForOrg.mockImplementation(() => {
109+
return DEFAULT_REGISTERED_RUNNERS;
110+
});
111+
mockOctokit.actions.listSelfHostedRunnersForRepo.mockImplementation(() => {
112+
return DEFAULT_REGISTERED_RUNNERS;
113+
});
114+
115+
function deRegisterRunnerGithub(id: number): any {}
116+
mockOctokit.actions.deleteSelfHostedRunnerFromRepo.mockImplementation((repo) => {
117+
if (repo.runner_id === 103) {
118+
throw Error();
119+
} else {
120+
return { status: 204 };
121+
}
122+
});
123+
mockOctokit.actions.deleteSelfHostedRunnerFromOrg.mockImplementation((repo) => {
124+
return repo.runner_id === 103 ? { status: 500 } : { status: 204 };
125+
});
126+
127+
const mockTerminateRunners = mocked(terminateRunner);
128+
mockTerminateRunners.mockImplementation(async () => {
129+
return;
130+
});
131+
});
132+
133+
describe('no runners running', () => {
134+
beforeAll(() => {
135+
const mockListRunners = mocked(listRunners);
136+
mockListRunners.mockImplementation(async () => []);
137+
});
138+
139+
it('No runners for repo.', async () => {
140+
process.env.ENABLE_ORGANIZATION_RUNNERS = 'false';
141+
await scaleDown();
142+
expect(listRunners).toBeCalledWith({
143+
environment: environment,
144+
});
145+
expect(terminateRunner).not;
146+
expect(mockOctokit.apps.getRepoInstallation).not;
147+
});
148+
149+
it('No runners for org.', async () => {
150+
process.env.ENABLE_ORGANIZATION_RUNNERS = 'true';
151+
await scaleDown();
152+
expect(listRunners).toBeCalledWith({
153+
environment: environment,
154+
});
155+
expect(terminateRunner).not;
156+
expect(mockOctokit.apps.getRepoInstallation).not;
157+
});
158+
});
159+
160+
describe('on repo level', () => {
161+
beforeAll(() => {
162+
process.env.ENABLE_ORGANIZATION_RUNNERS = 'false';
163+
const mockListRunners = mocked(listRunners);
164+
mockListRunners.mockImplementation(async () => {
165+
return DEFAULT_RUNNERS;
166+
});
167+
});
168+
169+
it('Terminate 2 of 4 runners for repo.', async () => {
170+
await scaleDown();
171+
expect(listRunners).toBeCalledWith({
172+
environment: environment,
173+
});
174+
175+
expect(mockOctokit.apps.getRepoInstallation).toBeCalled();
176+
177+
expect(terminateRunner).toBeCalledTimes(2);
178+
});
179+
});
180+
181+
describe('on org level', () => {
182+
beforeAll(() => {
183+
process.env.ENABLE_ORGANIZATION_RUNNERS = 'true';
184+
const mockListRunners = mocked(listRunners);
185+
mockListRunners.mockImplementation(async () => {
186+
return DEFAULT_RUNNERS;
187+
});
188+
});
189+
190+
it('Terminate 2 of 4 runners for org.', async () => {
191+
await scaleDown();
192+
expect(listRunners).toBeCalledWith({
193+
environment: environment,
194+
});
195+
196+
expect(mockOctokit.apps.getOrgInstallation).toBeCalled();
197+
198+
expect(terminateRunner).toBeCalledTimes(2);
199+
});
200+
});
201+
});

modules/runners/lambdas/scale-runners/src/scale-runners/scale-down.ts

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
import { createAppAuth } from '@octokit/auth-app';
21
import { Octokit } from '@octokit/rest';
32
import { AppAuth } from '@octokit/auth-app/dist-types/types';
43
import { listRunners, terminateRunner, RunnerInfo } from './runners';
54
import { createGithubAppAuth, createInstallationClient } from './scale-up';
65
import yn from 'yn';
6+
import moment from 'moment';
77

88
async function createAppClient(githubAppAuth: AppAuth): Promise<Octokit> {
99
const auth = await githubAppAuth({ type: 'app' });
@@ -16,17 +16,9 @@ interface Repo {
1616
}
1717

1818
function getRepo(runner: RunnerInfo, orgLevel: boolean): Repo {
19-
if (orgLevel) {
20-
return {
21-
repoOwner: runner.org as string,
22-
repoName: '',
23-
};
24-
} else {
25-
return {
26-
repoOwner: runner.repo?.split('/')[0] as string,
27-
repoName: runner.repo?.split('/')[1] as string,
28-
};
29-
}
19+
return orgLevel
20+
? { repoOwner: runner.org as string, repoName: '' }
21+
: { repoOwner: runner.repo?.split('/')[0] as string, repoName: runner.repo?.split('/')[1] as string };
3022
}
3123

3224
async function createGitHubClientForRunner(runner: RunnerInfo, orgLevel: boolean): Promise<Octokit> {
@@ -49,24 +41,30 @@ async function createGitHubClientForRunner(runner: RunnerInfo, orgLevel: boolean
4941
return createInstallationClient(createGithubAppAuth(installationId));
5042
}
5143

44+
function runnerMinimumTimeExceeded(runner: RunnerInfo, minimumRunningTimeInMinutes: string): boolean {
45+
const launchTimePlusMinimum = moment(runner.launchTime).utc().add(minimumRunningTimeInMinutes, 'minutes');
46+
const now = moment(new Date()).utc();
47+
return launchTimePlusMinimum < now;
48+
}
49+
5250
export async function scaleDown(): Promise<void> {
5351
const enableOrgLevel = yn(process.env.ENABLE_ORGANIZATION_RUNNERS, { default: true });
5452
const environment = process.env.ENVIRONMENT as string;
53+
const minimumRunningTimeInMinutes = process.env.MINIMUM_RUNNING_TIME_IN_MINUTES as string;
54+
5555
const runners = await listRunners({
5656
environment: environment,
5757
});
5858

59-
if (runners?.length === 0) {
59+
if (runners.length === 0) {
6060
console.debug(`No active runners found for environment: '${environment}'`);
6161
return;
6262
}
63-
console.log(runners);
6463

65-
for (const r of runners) {
66-
const githubAppClient = await createGitHubClientForRunner(r, enableOrgLevel);
64+
for (const ec2runner of runners) {
65+
const githubAppClient = await createGitHubClientForRunner(ec2runner, enableOrgLevel);
6766

68-
const repo = getRepo(r, enableOrgLevel);
69-
console.log(repo);
67+
const repo = getRepo(ec2runner, enableOrgLevel);
7068
const registered = enableOrgLevel
7169
? await githubAppClient.actions.listSelfHostedRunnersForOrg({
7270
org: repo.repoOwner,
@@ -75,25 +73,26 @@ export async function scaleDown(): Promise<void> {
7573
owner: repo.repoOwner,
7674
repo: repo.repoName,
7775
});
78-
console.log(registered);
7976

80-
console.log(registered.data.runners);
81-
for (const a of registered.data.runners) {
82-
const runnerName = a.name as string;
83-
if (runnerName === r.instanceId) {
77+
for (const ghRunner of registered.data.runners) {
78+
const runnerName = ghRunner.name as string;
79+
if (runnerName === ec2runner.instanceId && runnerMinimumTimeExceeded(ec2runner, minimumRunningTimeInMinutes)) {
8480
try {
8581
const result = enableOrgLevel
86-
? await githubAppClient.actions.deleteSelfHostedRunnerFromOrg({ runner_id: a.id, org: repo.repoOwner })
82+
? await githubAppClient.actions.deleteSelfHostedRunnerFromOrg({
83+
runner_id: ghRunner.id,
84+
org: repo.repoOwner,
85+
})
8786
: await githubAppClient.actions.deleteSelfHostedRunnerFromRepo({
88-
runner_id: a.id,
87+
runner_id: ghRunner.id,
8988
owner: repo.repoOwner,
9089
repo: repo.repoName,
9190
});
9291

93-
if (result?.status == 204) {
94-
await terminateRunner(r);
92+
if (result.status == 204) {
93+
await terminateRunner(ec2runner);
9594
console.info(
96-
`AWS runner instance '${r.instanceId}' is terminated and GitHub runner '${runnerName}' is de-registered.`,
95+
`AWS runner instance '${ec2runner.instanceId}' is terminated and GitHub runner '${runnerName}' is de-registered.`,
9796
);
9897
}
9998
} catch (e) {

modules/runners/lambdas/scale-runners/yarn.lock

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2896,6 +2896,11 @@ mkdirp@^0.5.1:
28962896
dependencies:
28972897
minimist "^1.2.5"
28982898

2899+
moment@^2.25.3:
2900+
version "2.25.3"
2901+
resolved "https://registry.yarnpkg.com/moment/-/moment-2.25.3.tgz#252ff41319cf41e47761a1a88cab30edfe9808c0"
2902+
integrity sha512-PuYv0PHxZvzc15Sp8ybUCoQ+xpyPWvjOuK72a5ovzp2LI32rJXOiIfyoFoYvG3s6EwwrdkMyWuRiEHSZRLJNdg==
2903+
28992904
29002905
version "2.0.0"
29012906
resolved "https://registry.yarnpkg.com/ms/-/ms-2.0.0.tgz#5608aeadfc00be6c2901df5f9861788de0d597c8"

modules/runners/scale-down.tf

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@ resource "aws_lambda_function" "scale_down" {
99

1010
environment {
1111
variables = {
12-
ENABLE_ORGANIZATION_RUNNERS = var.enable_organization_runners
13-
GITHUB_APP_KEY_BASE64 = var.github_app.key_base64
14-
GITHUB_APP_ID = var.github_app.id
15-
GITHUB_APP_CLIENT_ID = var.github_app.client_id
16-
GITHUB_APP_CLIENT_SECRET = var.github_app.client_secret
17-
ENVIRONMENT = var.environment
12+
ENABLE_ORGANIZATION_RUNNERS = var.enable_organization_runners
13+
MINIMUM_RUNNING_TIME_IN_MINUTES = var.minimum_running_time_in_minutes
14+
GITHUB_APP_KEY_BASE64 = var.github_app.key_base64
15+
GITHUB_APP_ID = var.github_app.id
16+
GITHUB_APP_CLIENT_ID = var.github_app.client_id
17+
GITHUB_APP_CLIENT_SECRET = var.github_app.client_secret
18+
ENVIRONMENT = var.environment
1819
}
1920
}
2021
}

modules/runners/variables.tf

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,3 +114,9 @@ variable "scale_down_schedule_expression" {
114114
type = string
115115
default = "cron(*/5 * * * ? *)"
116116
}
117+
118+
variable "minimum_running_time_in_minutes" {
119+
description = "The time an ec2 action runner should be running at minium before terminated if non busy."
120+
type = number
121+
default = 5
122+
}

variables.tf

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,8 @@ variable "scale_down_schedule_expression" {
4646
default = "cron(*/5 * * * ? *)"
4747
}
4848

49-
49+
variable "minimum_running_time_in_minutes" {
50+
description = "The time an ec2 action runner should be running at minium before terminated if non busy."
51+
type = number
52+
default = 5
53+
}

0 commit comments

Comments
 (0)