Skip to content

Commit

Permalink
Remove offline runners during scaleDown (#4228)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jeanschmidt
jeanschmidt authored May 30, 2023
1 parent 39dc73b commit b81ebb8
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 30 deletions.
4 changes: 2 additions & 2 deletions ...aform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/metrics.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ describe('./metrics', () => {
});

const timeDiff = spyDate.mock.results[1].value - spyDate.mock.results[0].value;
expect(timeDiff).toBeGreaterThanOrEqual(waitMs);
expect(timeDiff).toBeGreaterThanOrEqual(waitMs - 5);
expect(successFn).toBeCalledWith(timeDiff);
expect(failFn).not.toBeCalled();
});
Expand All @@ -172,7 +172,7 @@ describe('./metrics', () => {
).rejects.toThrowError();

const timeDiff = spyDate.mock.results[1].value - spyDate.mock.results[0].value;
expect(timeDiff).toBeGreaterThanOrEqual(waitMs);
expect(timeDiff).toBeGreaterThanOrEqual(waitMs - 5);
expect(successFn).not.toBeCalled();
expect(failFn).toBeCalledWith(timeDiff);
});
Expand Down
36 changes: 36 additions & 0 deletions terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/metrics.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1221,6 +1221,42 @@ export class ScaleDownMetrics extends Metrics {
}
}

/* istanbul ignore next */
runnerGhOfflineFoundRepo(repo: Repo, total: number) {
const dimensions = this.getRepoDim(repo);
this.addEntry('run.ghRunner.perRepo.offline.found', total, dimensions);
}

/* istanbul ignore next */
runnerGhOfflineRemovedRepo(repo: Repo) {
const dimensions = this.getRepoDim(repo);
this.countEntry('run.ghRunner.perRepo.offline.removed.success', 1, dimensions);
}

/* istanbul ignore next */
runnerGhOfflineRemovedFailureRepo(repo: Repo) {
const dimensions = this.getRepoDim(repo);
this.countEntry('run.ghRunner.perRepo.offline.removed.failure', 1, dimensions);
}

/* istanbul ignore next */
runnerGhOfflineFoundOrg(org: string, total: number) {
const dimensions = new Map([['Org', org]]);
this.addEntry('run.ghRunner.perOrg.offline.found', total, dimensions);
}

/* istanbul ignore next */
runnerGhOfflineRemovedOrg(org: string) {
const dimensions = new Map([['Org', org]]);
this.countEntry('run.ghRunner.perOrg.offline.removed.success', 1, dimensions);
}

/* istanbul ignore next */
runnerGhOfflineRemovedFailureOrg(org: string) {
const dimensions = new Map([['Org', org]]);
this.countEntry('run.ghRunner.perOrg.offline.removed.failure', 1, dimensions);
}

/* istanbul ignore next */
runnerTerminateSuccess(ec2Runner: RunnerInfo) {
this.countEntry('run.ec2Runners.terminate.total');
Expand Down
76 changes: 48 additions & 28 deletions ...rm-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-down.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,18 +165,20 @@ describe('scale-down', () => {
['keep-lt-min-no-ghrunner', { is_ephemeral: false } as RunnerType],
]);
const ghRunners = [
mockRunner({ id: '0001', name: 'keep-this-not-min-time-01', busy: false }),
mockRunner({ id: '0002', name: 'keep-this-not-min-time-02', busy: false }),
mockRunner({ id: '0003', name: 'keep-this-is-busy-01', busy: true }),
mockRunner({ id: '0004', name: 'keep-this-is-busy-02', busy: true }),
mockRunner({ id: '0005', name: 'keep-this-not-min-time-03', busy: false }),
mockRunner({ id: '0006', name: 'keep-this-is-busy-03', busy: true }),
mockRunner({ id: '0007', name: 'remove-ephemeral-01-fail-ghr', busy: false }),
mockRunner({ id: '0008', name: 'keep-min-runners-not-oldest-01', busy: false }),
mockRunner({ id: '0009', name: 'keep-min-runners-oldest-01', busy: false }),
mockRunner({ id: '0010', name: 'keep-min-runners-not-oldest-02', busy: false }),
mockRunner({ id: '0011', name: 'keep-min-runners-oldest-02', busy: false }),
mockRunner({ id: '0012', name: 'keep-lt-min-no-ghrunner-01', busy: false }),
mockRunner({ id: '0001', name: 'keep-this-not-min-time-01', busy: false, status: 'online' }),
mockRunner({ id: '0002', name: 'keep-this-not-min-time-02', busy: false, status: 'online' }),
mockRunner({ id: '0003', name: 'keep-this-is-busy-01', busy: true, status: 'online' }),
mockRunner({ id: '0004', name: 'keep-this-is-busy-02', busy: true, status: 'online' }),
mockRunner({ id: '0005', name: 'keep-this-not-min-time-03', busy: false, status: 'online' }),
mockRunner({ id: '0006', name: 'keep-this-is-busy-03', busy: true, status: 'online' }),
mockRunner({ id: '0007', name: 'remove-ephemeral-01-fail-ghr', busy: false, status: 'online' }),
mockRunner({ id: '0008', name: 'keep-min-runners-not-oldest-01', busy: false, status: 'online' }),
mockRunner({ id: '0009', name: 'keep-min-runners-oldest-01', busy: false, status: 'online' }),
mockRunner({ id: '0010', name: 'keep-min-runners-not-oldest-02', busy: false, status: 'online' }),
mockRunner({ id: '0011', name: 'keep-min-runners-oldest-02', busy: false, status: 'online' }),
mockRunner({ id: '0012', name: 'keep-lt-min-no-ghrunner-01', busy: false, status: 'online' }),
mockRunner({ id: '0013', name: 'remove-offline-01', busy: false, status: 'offline' }),
mockRunner({ id: '0014', name: 'remove-offline-02', busy: false, status: 'offline' }),
] as GhRunners;
const listRunnersRet = [
{
Expand Down Expand Up @@ -420,13 +422,13 @@ describe('scale-down', () => {
expect(mockedListRunners).toBeCalledTimes(1);
expect(mockedListRunners).toBeCalledWith(metrics, { environment: environment });

expect(mockedListGithubRunnersOrg).toBeCalledTimes(15);
expect(mockedListGithubRunnersOrg).toBeCalledTimes(16);
expect(mockedListGithubRunnersOrg).toBeCalledWith(theOrg, metrics);

expect(mockedGetRunnerTypes).toBeCalledTimes(4);
expect(mockedGetRunnerTypes).toBeCalledWith({ owner: theOrg, repo: scaleConfigRepo }, metrics);

expect(mockedRemoveGithubRunnerOrg).toBeCalledTimes(3);
expect(mockedRemoveGithubRunnerOrg).toBeCalledTimes(5);
{
const { awsR, ghR } = getRunnerPair('keep-min-runners-oldest-02');
expect(mockedRemoveGithubRunnerOrg).toBeCalledWith(ghR.id, awsR.org as string, metrics);
Expand All @@ -439,6 +441,14 @@ describe('scale-down', () => {
const { awsR, ghR } = getRunnerPair('remove-ephemeral-01-fail-ghr');
expect(mockedRemoveGithubRunnerOrg).toBeCalledWith(ghR.id, awsR.org as string, metrics);
}
{
const { ghR } = getRunnerPair('remove-offline-01');
expect(mockedRemoveGithubRunnerOrg).toBeCalledWith(ghR.id, theOrg, metrics);
}
{
const { ghR } = getRunnerPair('remove-offline-02');
expect(mockedRemoveGithubRunnerOrg).toBeCalledWith(ghR.id, theOrg, metrics);
}

expect(mockedTerminateRunner).toBeCalledTimes(5);
{
Expand Down Expand Up @@ -478,18 +488,20 @@ describe('scale-down', () => {
['keep-lt-min-no-ghrunner', { is_ephemeral: false } as RunnerType],
]);
const ghRunners = [
mockRunner({ id: '0001', name: 'keep-this-not-min-time-01', busy: false }),
mockRunner({ id: '0002', name: 'keep-this-not-min-time-02', busy: false }),
mockRunner({ id: '0003', name: 'keep-this-is-busy-01', busy: true }),
mockRunner({ id: '0004', name: 'keep-this-is-busy-02', busy: true }),
mockRunner({ id: '0005', name: 'keep-this-not-min-time-03', busy: false }),
mockRunner({ id: '0006', name: 'keep-this-is-busy-03', busy: true }),
mockRunner({ id: '0007', name: 'remove-ephemeral-01-fail-ghr', busy: false }),
mockRunner({ id: '0008', name: 'keep-min-runners-not-oldest-01', busy: false }),
mockRunner({ id: '0009', name: 'keep-min-runners-oldest-01', busy: false }),
mockRunner({ id: '0010', name: 'keep-min-runners-not-oldest-02', busy: false }),
mockRunner({ id: '0011', name: 'keep-min-runners-oldest-02', busy: false }),
mockRunner({ id: '0012', name: 'keep-lt-min-no-ghrunner-01', busy: false }),
mockRunner({ id: '0001', name: 'keep-this-not-min-time-01', busy: false, status: 'online' }),
mockRunner({ id: '0002', name: 'keep-this-not-min-time-02', busy: false, status: 'online' }),
mockRunner({ id: '0003', name: 'keep-this-is-busy-01', busy: true, status: 'online' }),
mockRunner({ id: '0004', name: 'keep-this-is-busy-02', busy: true, status: 'online' }),
mockRunner({ id: '0005', name: 'keep-this-not-min-time-03', busy: false, status: 'online' }),
mockRunner({ id: '0006', name: 'keep-this-is-busy-03', busy: true, status: 'online' }),
mockRunner({ id: '0007', name: 'remove-ephemeral-01-fail-ghr', busy: false, status: 'online' }),
mockRunner({ id: '0008', name: 'keep-min-runners-not-oldest-01', busy: false, status: 'online' }),
mockRunner({ id: '0009', name: 'keep-min-runners-oldest-01', busy: false, status: 'online' }),
mockRunner({ id: '0010', name: 'keep-min-runners-not-oldest-02', busy: false, status: 'online' }),
mockRunner({ id: '0011', name: 'keep-min-runners-oldest-02', busy: false, status: 'online' }),
mockRunner({ id: '0012', name: 'keep-lt-min-no-ghrunner-01', busy: false, status: 'online' }),
mockRunner({ id: '0013', name: 'remove-offline-01', busy: false, status: 'offline' }),
mockRunner({ id: '0014', name: 'remove-offline-02', busy: false, status: 'offline' }),
] as GhRunners;
const listRunnersRet = [
{
Expand Down Expand Up @@ -728,13 +740,13 @@ describe('scale-down', () => {
expect(mockedListRunners).toBeCalledTimes(1);
expect(mockedListRunners).toBeCalledWith(metrics, { environment: environment });

expect(mockedListGithubRunnersRepo).toBeCalledTimes(15);
expect(mockedListGithubRunnersRepo).toBeCalledTimes(16);
expect(mockedListGithubRunnersRepo).toBeCalledWith(repo, metrics);

expect(mockedGetRunnerTypes).toBeCalledTimes(4);
expect(mockedGetRunnerTypes).toBeCalledWith(repo, metrics);

expect(mockedRemoveGithubRunnerRepo).toBeCalledTimes(3);
expect(mockedRemoveGithubRunnerRepo).toBeCalledTimes(5);
{
const { ghR } = getRunnerPair('keep-min-runners-oldest-02');
expect(mockedRemoveGithubRunnerRepo).toBeCalledWith(ghR.id, repo, metrics);
Expand All @@ -747,6 +759,14 @@ describe('scale-down', () => {
const { ghR } = getRunnerPair('remove-ephemeral-01-fail-ghr');
expect(mockedRemoveGithubRunnerRepo).toBeCalledWith(ghR.id, repo, metrics);
}
{
const { ghR } = getRunnerPair('remove-offline-01');
expect(mockedRemoveGithubRunnerRepo).toBeCalledWith(ghR.id, repo, metrics);
}
{
const { ghR } = getRunnerPair('remove-offline-02');
expect(mockedRemoveGithubRunnerRepo).toBeCalledWith(ghR.id, repo, metrics);
}

expect(mockedTerminateRunner).toBeCalledTimes(5);
{
Expand Down
42 changes: 42 additions & 0 deletions terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-down.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,17 @@ export async function scaleDown(): Promise<void> {
return;
}

const foundOrgs = new Set<string>();
const foundRepos = new Set<string>();

for (const [runnerType, runners] of shuffleArrayInPlace(Array.from(runnersDict.entries()))) {
if (runners.length < 1 || runners[0].runnerType === undefined || runnerType === undefined) continue;

const ghRunnersRemovable: Array<[RunnerInfo, GhRunner | undefined]> = [];
for (const ec2runner of runners) {
// REPO assigned runners
if (ec2runner.repo !== undefined) {
foundRepos.add(ec2runner.repo);
const ghRunner = await getGHRunnerRepo(ec2runner, metrics);
// if configured to repo, don't mess with organization runners
if (!Config.Instance.enableOrganizationRunners) {
Expand All @@ -68,6 +72,7 @@ export async function scaleDown(): Promise<void> {
}
// ORG assigned runners
} else if (ec2runner.org !== undefined) {
foundOrgs.add(ec2runner.org);
const ghRunner = await getGHRunnerOrg(ec2runner, metrics);
// if configured to org, don't mess with repo runners
if (Config.Instance.enableOrganizationRunners) {
Expand Down Expand Up @@ -170,6 +175,43 @@ export async function scaleDown(): Promise<void> {
}
}
}

if (Config.Instance.enableOrganizationRunners) {
for (const org of foundOrgs) {
const offlineGhRunners = (await listGithubRunnersOrg(org, metrics)).filter(
(r) => r.status.toLowerCase() === 'offline',
);
metrics.runnerGhOfflineFoundOrg(org, offlineGhRunners.length);

for (const ghRunner of offlineGhRunners) {
try {
await removeGithubRunnerOrg(ghRunner.id, org, metrics);
metrics.runnerGhOfflineRemovedOrg(org);
} catch (e) {
console.warn(`Failed to remove offline runner ${ghRunner.id} for org ${org}`, e);
metrics.runnerGhOfflineRemovedFailureOrg(org);
}
}
}
} else {
for (const repoString of foundRepos) {
const repo = getRepo(repoString);
const offlineGhRunners = (await listGithubRunnersRepo(repo, metrics)).filter(
(r) => r.status.toLowerCase() === 'offline',
);
metrics.runnerGhOfflineFoundRepo(repo, offlineGhRunners.length);

for (const ghRunner of offlineGhRunners) {
try {
await removeGithubRunnerRepo(ghRunner.id, repo, metrics);
metrics.runnerGhOfflineRemovedRepo(repo);
} catch (e) {
console.warn(`Failed to remove offline runner ${ghRunner.id} for repo ${repo}`, e);
metrics.runnerGhOfflineRemovedFailureRepo(repo);
}
}
}
}
} catch (e) {
/* istanbul ignore next */
metrics.exception();
Expand Down

0 comments on commit b81ebb8

Please sign in to comment.