Skip to content

Commit aa4554a

Browse files
jeanschmidtjeanschmidt
authored
max_available in scale-config.yaml with negative values disable max enforcing (#5806)
1 parent 99b02dd commit aa4554a

File tree

4 files changed

+181
-12
lines changed

4 files changed

+181
-12
lines changed

‎terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/gh-runners.test.ts‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -669,7 +669,6 @@ runner_types:
669669
instance_type: c5.2xlarge
670670
os: linux
671671
min_available: 1
672-
max_available: 1
673672
disk_size: 150
674673
is_ephemeral: false
675674
linux.4xlarge:
@@ -690,7 +689,7 @@ runner_types:
690689
instance_type: c5.2xlarge
691690
os: linux
692691
min_available: 1
693-
max_available: 1
692+
max_available: -1
694693
disk_size: 150
695694
is_ephemeral: false
696695
variants:
@@ -700,7 +699,6 @@ runner_types:
700699
instance_type: c5.2xlarge
701700
os: linux
702701
min_available: 1
703-
max_available: 1
704702
disk_size: 150
705703
is_ephemeral: false
706704
variants:
@@ -714,7 +712,6 @@ runner_types:
714712
runnerTypeName: 'linux.2xlarge',
715713
instance_type: 'c5.2xlarge',
716714
os: 'linux',
717-
max_available: 1,
718715
min_available: 1,
719716
disk_size: 150,
720717
is_ephemeral: false,
@@ -775,7 +772,7 @@ runner_types:
775772
runnerTypeName: 'lf.linux.4xlarge',
776773
instance_type: 'c5.2xlarge',
777774
os: 'linux',
778-
max_available: 1,
775+
max_available: -1,
779776
min_available: 1,
780777
disk_size: 150,
781778
is_ephemeral: false,
@@ -787,7 +784,7 @@ runner_types:
787784
runnerTypeName: 'lf.ephemeral.linux.4xlarge',
788785
instance_type: 'c5.2xlarge',
789786
os: 'linux',
790-
max_available: 1,
787+
max_available: -1,
791788
min_available: 1,
792789
disk_size: 150,
793790
is_ephemeral: true,
@@ -799,7 +796,6 @@ runner_types:
799796
runnerTypeName: 'lf.c.linux.4xlarge',
800797
instance_type: 'c5.2xlarge',
801798
os: 'linux',
802-
max_available: 1,
803799
min_available: 1,
804800
disk_size: 150,
805801
is_ephemeral: false,
@@ -811,7 +807,6 @@ runner_types:
811807
runnerTypeName: 'lf.c.ephemeral.linux.4xlarge',
812808
instance_type: 'c5.2xlarge',
813809
os: 'linux',
814-
max_available: 1,
815810
min_available: 1,
816811
disk_size: 150,
817812
is_ephemeral: true,

‎terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/runners.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ export interface RunnerType extends RunnerTypeOptional {
4343
disk_size: number;
4444
instance_type: string;
4545
is_ephemeral: boolean;
46-
max_available: number;
4746
os: string;
4847
runnerTypeName: string;
4948
}

‎terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up.test.ts‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,177 @@ describe('scaleUp', () => {
417417
expect(mockedCreateRegistrationTokenForRepo).toBeCalledWith(repo, metrics, 2);
418418
});
419419

420+
it('don`t have sufficient runners, max_available is negative', async (): Promise<void> => {
421+
const config = {
422+
...baseCfg,
423+
environment: 'config.environ',
424+
ghesUrlHost: 'https://github.com',
425+
minAvailableRunners: 10,
426+
runnersExtraLabels: 'extra-label',
427+
};
428+
jest.spyOn(Config, 'Instance', 'get').mockImplementation(() => config as unknown as Config);
429+
const repo = { repo: 'repo', owner: 'owner' };
430+
const payload = {
431+
id: 10,
432+
eventType: 'event',
433+
repositoryName: 'repo',
434+
repositoryOwner: 'owner',
435+
installationId: 2,
436+
};
437+
const token = 'AGDGADUWG113';
438+
const runnerType1 = {
439+
instance_type: 'instance_type',
440+
os: 'os',
441+
max_available: -1,
442+
disk_size: 113,
443+
runnerTypeName: 'linux.2xlarge',
444+
is_ephemeral: false,
445+
};
446+
447+
mocked(getRunnerTypes).mockResolvedValue(new Map([['linux.2xlarge', runnerType1]]));
448+
mocked(listGithubRunnersRepo).mockResolvedValue([
449+
{
450+
id: 3,
451+
name: 'name-01',
452+
os: 'linux',
453+
status: 'live',
454+
busy: false,
455+
labels: [
456+
{
457+
id: 113,
458+
name: 'linux.2xlarge',
459+
type: 'read-only',
460+
},
461+
],
462+
},
463+
{
464+
id: 33,
465+
name: 'name-02',
466+
os: 'linux',
467+
status: 'live',
468+
busy: false,
469+
labels: [
470+
{
471+
id: 113,
472+
name: 'linux.2xlarge',
473+
type: 'read-only',
474+
},
475+
],
476+
},
477+
]);
478+
const mockedCreateRegistrationTokenForRepo = mocked(createRegistrationTokenRepo).mockResolvedValue(token);
479+
const mockedCreateRunner = mocked(createRunner);
480+
481+
await scaleUp('aws:sqs', payload, metrics);
482+
483+
expect(mockedCreateRunner).toBeCalledTimes(1);
484+
expect(mockedCreateRunner).toBeCalledWith(
485+
{
486+
environment: config.environment,
487+
runnerConfig: expect.any(Function),
488+
repoName: 'owner/repo',
489+
runnerType: runnerType1,
490+
},
491+
metrics,
492+
);
493+
494+
expect(await mockedCreateRunner.mock.calls[0][0].runnerConfig(config.awsRegion, false)).toEqual(
495+
`--url ${config.ghesUrlHost}/owner/repo --token ${token} --labels AWS:${config.awsRegion},linux.2xlarge,` +
496+
`extra-label `,
497+
);
498+
expect(await mockedCreateRunner.mock.calls[0][0].runnerConfig(config.awsRegion, true)).toEqual(
499+
`--url ${config.ghesUrlHost}/owner/repo --token ${token} --labels AWS:${config.awsRegion},linux.2xlarge,` +
500+
`experimental.ami,extra-label --ephemeral`,
501+
);
502+
expect(mockedCreateRegistrationTokenForRepo).toBeCalledTimes(2);
503+
expect(mockedCreateRegistrationTokenForRepo).toBeCalledWith(repo, metrics, 2);
504+
});
505+
506+
it('don`t have sufficient runners, max_available is undefined', async (): Promise<void> => {
507+
const config = {
508+
...baseCfg,
509+
environment: 'config.environ',
510+
ghesUrlHost: 'https://github.com',
511+
minAvailableRunners: 10,
512+
runnersExtraLabels: 'extra-label',
513+
};
514+
jest.spyOn(Config, 'Instance', 'get').mockImplementation(() => config as unknown as Config);
515+
const repo = { repo: 'repo', owner: 'owner' };
516+
const payload = {
517+
id: 10,
518+
eventType: 'event',
519+
repositoryName: 'repo',
520+
repositoryOwner: 'owner',
521+
installationId: 2,
522+
};
523+
const token = 'AGDGADUWG113';
524+
const runnerType1 = {
525+
instance_type: 'instance_type',
526+
os: 'os',
527+
disk_size: 113,
528+
runnerTypeName: 'linux.2xlarge',
529+
is_ephemeral: false,
530+
};
531+
532+
mocked(getRunnerTypes).mockResolvedValue(new Map([['linux.2xlarge', runnerType1]]));
533+
mocked(listGithubRunnersRepo).mockResolvedValue([
534+
{
535+
id: 3,
536+
name: 'name-01',
537+
os: 'linux',
538+
status: 'live',
539+
busy: false,
540+
labels: [
541+
{
542+
id: 113,
543+
name: 'linux.2xlarge',
544+
type: 'read-only',
545+
},
546+
],
547+
},
548+
{
549+
id: 33,
550+
name: 'name-02',
551+
os: 'linux',
552+
status: 'live',
553+
busy: false,
554+
labels: [
555+
{
556+
id: 113,
557+
name: 'linux.2xlarge',
558+
type: 'read-only',
559+
},
560+
],
561+
},
562+
]);
563+
const mockedCreateRegistrationTokenForRepo = mocked(createRegistrationTokenRepo).mockResolvedValue(token);
564+
const mockedCreateRunner = mocked(createRunner);
565+
566+
await scaleUp('aws:sqs', payload, metrics);
567+
568+
expect(mockedCreateRunner).toBeCalledTimes(1);
569+
expect(mockedCreateRunner).toBeCalledWith(
570+
{
571+
environment: config.environment,
572+
runnerConfig: expect.any(Function),
573+
repoName: 'owner/repo',
574+
runnerType: runnerType1,
575+
},
576+
metrics,
577+
);
578+
579+
expect(await mockedCreateRunner.mock.calls[0][0].runnerConfig(config.awsRegion, false)).toEqual(
580+
`--url ${config.ghesUrlHost}/owner/repo --token ${token} --labels AWS:${config.awsRegion},linux.2xlarge,` +
581+
`extra-label `,
582+
);
583+
expect(await mockedCreateRunner.mock.calls[0][0].runnerConfig(config.awsRegion, true)).toEqual(
584+
`--url ${config.ghesUrlHost}/owner/repo --token ${token} --labels AWS:${config.awsRegion},linux.2xlarge,` +
585+
`experimental.ami,extra-label --ephemeral`,
586+
);
587+
expect(mockedCreateRegistrationTokenForRepo).toBeCalledTimes(2);
588+
expect(mockedCreateRegistrationTokenForRepo).toBeCalledWith(repo, metrics, 2);
589+
});
590+
420591
it('runners are offline', async () => {
421592
const config = {
422593
...baseCfg,

‎terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ async function allRunnersBusy(
210210
runnerType: string,
211211
repo: Repo,
212212
isEphemeral: boolean,
213-
maxAvailable: number,
213+
maxAvailable: number | undefined,
214214
metrics: ScaleUpMetrics,
215215
): Promise<number> {
216216
const ghRunners = Config.Instance.enableOrganizationRunners
@@ -233,14 +233,18 @@ async function allRunnersBusy(
233233
}
234234

235235
// If a runner isn't ephemeral then maxAvailable should be applied
236-
if (!isEphemeral && runnersWithLabel.length >= maxAvailable) {
236+
if (!isEphemeral && maxAvailable !== undefined && maxAvailable >= 0 && runnersWithLabel.length >= maxAvailable) {
237237
/* istanbul ignore next */
238238
if (Config.Instance.enableOrganizationRunners) {
239239
metrics.ghRunnersOrgMaxHit(repo.owner, runnerType);
240240
} else {
241241
metrics.ghRunnersRepoMaxHit(repo, runnerType);
242242
}
243-
console.info(`Max runners hit [${runnerType}], ${busyCount}/${runnersWithLabel.length}/${ghRunners.length}`);
243+
244+
console.info(
245+
`Max runners hit [${runnerType}], ${busyCount}/${runnersWithLabel.length}/${ghRunners.length} - Limit enforced`,
246+
);
247+
244248
return 0;
245249
}
246250

0 commit comments

Comments
 (0)