Skip to content

fix(scale): Refactor Runner Type and Owner #871

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 15, 2021
Merged

fix(scale): Refactor Runner Type and Owner #871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd4ab7b
fix(scale): Refactor Runner Type and Owner
mcaulifn Jun 4, 2021
80ebe92
`environment` should not be optional
mcaulifn Jun 4, 2021
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
69 changes: 30 additions & 39 deletions modules/runners/lambdas/runners/src/scale-runners/runners.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { listRunners, RunnerInfo, createRunner } from './runners';
import { EC2, SSM } from 'aws-sdk';
import { listRunners, createRunner } from './runners';

const mockEC2 = { describeInstances: jest.fn(), runInstances: jest.fn() };
const mockSSM = { putParameter: jest.fn() };
Expand All @@ -8,6 +7,10 @@ jest.mock('aws-sdk', () => ({
SSM: jest.fn().mockImplementation(() => mockSSM),
}));

const ORG_NAME = 'SomeAwesomeCoder';
const REPO_NAME = `${ORG_NAME}/some-amazing-library`;
const ENVIRONMENT = 'unit-test-environment';

describe('list instances', () => {
const mockDescribeInstances = { promise: jest.fn() };
beforeEach(() => {
Expand All @@ -30,8 +33,8 @@ describe('list instances', () => {
LaunchTime: new Date('2020-10-11T14:48:00.000+09:00'),
InstanceId: 'i-5678',
Tags: [
{ Key: 'Repo', Value: 'SomeAwesomeCoder/some-amazing-library' },
{ Key: 'Org', Value: 'SomeAwesomeCoder' },
{ Key: 'Repo', Value: REPO_NAME },
{ Key: 'Org', Value: ORG_NAME },
{ Key: 'Application', Value: 'github-action-runner' },
],
},
Expand All @@ -54,8 +57,8 @@ describe('list instances', () => {
expect(resp).toContainEqual({
instanceId: 'i-5678',
launchTime: new Date('2020-10-11T14:48:00.000+09:00'),
repo: 'SomeAwesomeCoder/some-amazing-library',
org: 'SomeAwesomeCoder',
repo: REPO_NAME,
org: ORG_NAME,
});
});

Expand All @@ -65,46 +68,34 @@ describe('list instances', () => {
});

it('filters instances on repo name', async () => {
await listRunners({ repoName: 'SomeAwesomeCoder/some-amazing-library' });
await listRunners({ runnerType: 'Repo', runnerOwner: REPO_NAME, environment: undefined });
expect(mockEC2.describeInstances).toBeCalledWith({
Filters: [
{ Name: 'tag:Application', Values: ['github-action-runner'] },
{ Name: 'instance-state-name', Values: ['running', 'pending'] },
{ Name: 'tag:Repo', Values: ['SomeAwesomeCoder/some-amazing-library'] },
{ Name: 'tag:Repo', Values: [REPO_NAME] },
],
});
});

it('filters instances on org name', async () => {
await listRunners({ orgName: 'SomeAwesomeCoder' });
await listRunners({ runnerType: 'Org', runnerOwner: ORG_NAME, environment: undefined });
expect(mockEC2.describeInstances).toBeCalledWith({
Filters: [
{ Name: 'tag:Application', Values: ['github-action-runner'] },
{ Name: 'instance-state-name', Values: ['running', 'pending'] },
{ Name: 'tag:Org', Values: ['SomeAwesomeCoder'] },
{ Name: 'tag:Org', Values: [ORG_NAME] },
],
});
});

it('filters instances on org name', async () => {
await listRunners({ environment: 'unit-test-environment' });
expect(mockEC2.describeInstances).toBeCalledWith({
Filters: [
{ Name: 'tag:Application', Values: ['github-action-runner'] },
{ Name: 'instance-state-name', Values: ['running', 'pending'] },
{ Name: 'tag:Environment', Values: ['unit-test-environment'] },
],
});
});

it('filters instances on both org name and repo name', async () => {
await listRunners({ orgName: 'SomeAwesomeCoder', repoName: 'SomeAwesomeCoder/some-amazing-library' });
await listRunners({ environment: ENVIRONMENT });
expect(mockEC2.describeInstances).toBeCalledWith({
Filters: [
{ Name: 'tag:Application', Values: ['github-action-runner'] },
{ Name: 'instance-state-name', Values: ['running', 'pending'] },
{ Name: 'tag:Repo', Values: ['SomeAwesomeCoder/some-amazing-library'] },
{ Name: 'tag:Org', Values: ['SomeAwesomeCoder'] },
{ Name: 'tag:Environment', Values: [ENVIRONMENT] },
],
});
});
Expand Down Expand Up @@ -132,9 +123,9 @@ describe('create runner', () => {
it('calls run instances with the correct config for repo', async () => {
await createRunner({
runnerConfig: 'bla',
environment: 'unit-test-env',
repoName: 'SomeAwesomeCoder/some-amazing-library',
orgName: undefined,
environment: ENVIRONMENT,
runnerType: 'Repo',
runnerOwner: REPO_NAME
});
expect(mockEC2.runInstances).toBeCalledWith({
MaxCount: 1,
Expand All @@ -146,7 +137,7 @@ describe('create runner', () => {
ResourceType: 'instance',
Tags: [
{ Key: 'Application', Value: 'github-action-runner' },
{ Key: 'Repo', Value: 'SomeAwesomeCoder/some-amazing-library' },
{ Key: 'Repo', Value: REPO_NAME },
],
},
],
Expand All @@ -156,9 +147,9 @@ describe('create runner', () => {
it('calls run instances with the correct config for org', async () => {
await createRunner({
runnerConfig: 'bla',
environment: 'unit-test-env',
repoName: undefined,
orgName: 'SomeAwesomeCoder',
environment: ENVIRONMENT,
runnerType: 'Org',
runnerOwner: ORG_NAME,
});
expect(mockEC2.runInstances).toBeCalledWith({
MaxCount: 1,
Expand All @@ -170,7 +161,7 @@ describe('create runner', () => {
ResourceType: 'instance',
Tags: [
{ Key: 'Application', Value: 'github-action-runner' },
{ Key: 'Org', Value: 'SomeAwesomeCoder' },
{ Key: 'Org', Value: ORG_NAME },
],
},
],
Expand All @@ -180,12 +171,12 @@ describe('create runner', () => {
it('creates ssm parameters for each created instance', async () => {
await createRunner({
runnerConfig: 'bla',
environment: 'unit-test-env',
repoName: undefined,
orgName: 'SomeAwesomeCoder',
environment: ENVIRONMENT,
runnerType: 'Org',
runnerOwner: ORG_NAME,
});
expect(mockSSM.putParameter).toBeCalledWith({
Name: 'unit-test-env-i-1234',
Name: `${ENVIRONMENT}-i-1234`,
Value: 'bla',
Type: 'SecureString',
});
Expand All @@ -197,9 +188,9 @@ describe('create runner', () => {
});
await createRunner({
runnerConfig: 'bla',
environment: 'unit-test-env',
repoName: undefined,
orgName: 'SomeAwesomeCoder',
environment: ENVIRONMENT,
runnerType: 'Org',
runnerOwner: ORG_NAME,
});
expect(mockSSM.putParameter).not.toBeCalled();
});
Expand Down
21 changes: 9 additions & 12 deletions modules/runners/lambdas/runners/src/scale-runners/runners.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ export interface RunnerInfo {
}

export interface ListRunnerFilters {
repoName?: string;
orgName?: string;
environment?: string;
runnerType?: 'Org' | 'Repo';
runnerOwner?: string;
environment: string | undefined;
}

export async function listRunners(filters: ListRunnerFilters | undefined = undefined): Promise<RunnerInfo[]> {
Expand All @@ -23,11 +23,8 @@ export async function listRunners(filters: ListRunnerFilters | undefined = undef
if (filters.environment !== undefined) {
ec2Filters.push({ Name: 'tag:Environment', Values: [filters.environment] });
}
if (filters.repoName !== undefined) {
ec2Filters.push({ Name: 'tag:Repo', Values: [filters.repoName] });
}
if (filters.orgName !== undefined) {
ec2Filters.push({ Name: 'tag:Org', Values: [filters.orgName] });
if (filters.runnerType && filters.runnerOwner) {
ec2Filters.push({ Name: `tag:${filters.runnerType}`, Values: [filters.runnerOwner] });
}
}
const runningInstances = await ec2.describeInstances({ Filters: ec2Filters }).promise();
Expand All @@ -52,8 +49,8 @@ export async function listRunners(filters: ListRunnerFilters | undefined = undef
export interface RunnerInputParameters {
runnerConfig: string;
environment: string;
repoName?: string;
orgName?: string;
runnerType: 'Org' | 'Repo';
runnerOwner: string;
}

export async function terminateRunner(runner: RunnerInfo): Promise<void> {
Expand Down Expand Up @@ -89,8 +86,8 @@ export async function createRunner(runnerParameters: RunnerInputParameters): Pro
Tags: [
{ Key: 'Application', Value: 'github-action-runner' },
{
Key: runnerParameters.orgName ? 'Org' : 'Repo',
Value: runnerParameters.orgName ? runnerParameters.orgName : runnerParameters.repoName,
Key: runnerParameters.runnerType,
Value: runnerParameters.runnerOwner
},
],
},
Expand Down
2 changes: 1 addition & 1 deletion modules/runners/lambdas/runners/src/scale-runners/scale-down.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export async function scaleDown(): Promise<void> {
// list and sort runners, newest first. This ensure we keep the newest runners longer.
const runners = (
await listRunners({
environment: environment,
environment
})
).sort((a, b): number => {
if (a.launchTime === undefined) return 1;
Expand Down
44 changes: 24 additions & 20 deletions modules/runners/lambdas/runners/src/scale-runners/scale-up.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ describe('scaleUp with GHES', () => {
await scaleUp('aws:sqs', TEST_DATA);
expect(listRunners).toBeCalledWith({
environment: 'unit-test-environment',
repoName: undefined,
runnerType: 'Org',
runnerOwner: TEST_DATA.repositoryOwner
});
});

Expand Down Expand Up @@ -174,8 +175,8 @@ describe('scaleUp with GHES', () => {
expect(createRunner).toBeCalledWith({
environment: 'unit-test-environment',
runnerConfig: `--url https://github.enterprise.something/${TEST_DATA.repositoryOwner} --token 1234abcd `,
orgName: TEST_DATA.repositoryOwner,
repoName: undefined,
runnerType: 'Org',
runnerOwner: TEST_DATA.repositoryOwner,
});
});

Expand All @@ -187,8 +188,8 @@ describe('scaleUp with GHES', () => {
environment: 'unit-test-environment',
runnerConfig: `--url https://github.enterprise.something/${TEST_DATA.repositoryOwner} ` +
`--token 1234abcd --labels label1,label2 --runnergroup TEST_GROUP`,
orgName: TEST_DATA.repositoryOwner,
repoName: undefined,
runnerType: 'Org',
runnerOwner: TEST_DATA.repositoryOwner,
});
});
});
Expand All @@ -202,7 +203,8 @@ describe('scaleUp with GHES', () => {
await scaleUp('aws:sqs', TEST_DATA);
expect(listRunners).toBeCalledWith({
environment: 'unit-test-environment',
repoName: `${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`,
runnerType: 'Repo',
runnerOwner: `${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`,
});
});

Expand Down Expand Up @@ -253,8 +255,8 @@ describe('scaleUp with GHES', () => {
runnerConfig: `--url ` +
`https://github.enterprise.something/${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName} ` +
`--token 1234abcd --labels label1,label2`,
orgName: undefined,
repoName: `${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`,
runnerType: 'Repo',
runnerOwner: `${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`,
});
});

Expand All @@ -267,8 +269,8 @@ describe('scaleUp with GHES', () => {
runnerConfig: `--url ` +
`https://github.enterprise.something/${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName} ` +
`--token 1234abcd --labels label1,label2`,
orgName: undefined,
repoName: `${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`,
runnerType: 'Repo',
runnerOwner: `${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`,
});
});
});
Expand Down Expand Up @@ -322,7 +324,8 @@ describe('scaleUp with public GH', () => {
await scaleUp('aws:sqs', TEST_DATA);
expect(listRunners).toBeCalledWith({
environment: 'unit-test-environment',
repoName: undefined,
runnerType: 'Org',
runnerOwner: TEST_DATA.repositoryOwner
});
});

Expand All @@ -345,8 +348,8 @@ describe('scaleUp with public GH', () => {
expect(createRunner).toBeCalledWith({
environment: 'unit-test-environment',
runnerConfig: `--url https://github.com/${TEST_DATA.repositoryOwner} --token 1234abcd `,
orgName: TEST_DATA.repositoryOwner,
repoName: undefined,
runnerType: 'Org',
runnerOwner: TEST_DATA.repositoryOwner
});
});

Expand All @@ -358,8 +361,8 @@ describe('scaleUp with public GH', () => {
environment: 'unit-test-environment',
runnerConfig: `--url https://github.com/${TEST_DATA.repositoryOwner} ` +
`--token 1234abcd --labels label1,label2 --runnergroup TEST_GROUP`,
orgName: TEST_DATA.repositoryOwner,
repoName: undefined,
runnerType: 'Org',
runnerOwner: TEST_DATA.repositoryOwner
});
});
});
Expand All @@ -373,7 +376,8 @@ describe('scaleUp with public GH', () => {
await scaleUp('aws:sqs', TEST_DATA);
expect(listRunners).toBeCalledWith({
environment: 'unit-test-environment',
repoName: `${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`,
runnerType: 'Repo',
runnerOwner: `${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`,
});
});

Expand Down Expand Up @@ -414,8 +418,8 @@ describe('scaleUp with public GH', () => {
environment: 'unit-test-environment',
runnerConfig: `--url https://github.com/${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName} ` +
`--token 1234abcd --labels label1,label2`,
orgName: undefined,
repoName: `${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`,
runnerType: 'Repo',
runnerOwner: `${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`,
});
});

Expand All @@ -427,8 +431,8 @@ describe('scaleUp with public GH', () => {
environment: 'unit-test-environment',
runnerConfig: `--url https://github.com/${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName} ` +
`--token 1234abcd --labels label1,label2`,
orgName: undefined,
repoName: `${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`,
runnerType: 'Repo',
runnerOwner: `${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`,
});
});
});
Expand Down
Loading