Skip to content

Commit cd7ca8f

Browse files
committed
feat: add retry logic to Octokit client and add ScaleUp tests
1 parent 7e33d64 commit cd7ca8f

4 files changed

Lines changed: 117 additions & 1 deletion

File tree

lambdas/functions/control-plane/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
"@middy/core": "^6.4.5",
3939
"@octokit/auth-app": "8.1.2",
4040
"@octokit/core": "7.0.6",
41+
"@octokit/plugin-retry": "8.0.3",
4142
"@octokit/plugin-throttling": "11.0.3",
4243
"@octokit/rest": "22.0.1",
4344
"cron-parser": "^5.4.0"

lambdas/functions/control-plane/src/github/auth.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ type StrategyOptions = {
1818
};
1919
import { request } from '@octokit/request';
2020
import { Octokit } from '@octokit/rest';
21+
import { retry } from '@octokit/plugin-retry';
2122
import { throttling } from '@octokit/plugin-throttling';
2223
import { createChildLogger } from '@aws-github-runner/aws-powertools-util';
2324
import { getParameter } from '@aws-github-runner/aws-ssm-util';
@@ -26,7 +27,7 @@ import { EndpointDefaults } from '@octokit/types';
2627
const logger = createChildLogger('gh-auth');
2728

2829
export async function createOctokitClient(token: string, ghesApiUrl = ''): Promise<Octokit> {
29-
const CustomOctokit = Octokit.plugin(throttling);
30+
const CustomOctokit = Octokit.plugin(retry, throttling);
3031
const ocktokitOptions: OctokitOptions = {
3132
auth: token,
3233
};
@@ -38,6 +39,17 @@ export async function createOctokitClient(token: string, ghesApiUrl = ''): Promi
3839
return new CustomOctokit({
3940
...ocktokitOptions,
4041
userAgent: process.env.USER_AGENT || 'github-aws-runners',
42+
retry: {
43+
onRetry: (retryCount: number, error: Error, request: any) => {
44+
logger.warn('GitHub API request retry attempt', {
45+
retryCount,
46+
method: request.method,
47+
url: request.url,
48+
error: error.message,
49+
status: (error as any).status,
50+
});
51+
},
52+
},
4153
throttle: {
4254
onRateLimit: (retryAfter: number, options: Required<EndpointDefaults>) => {
4355
logger.warn(

lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,95 @@ describe('scaleUp with GHES', () => {
410410
});
411411
});
412412

413+
it('should handle retryable errors with error handling logic', async () => {
414+
process.env.RUNNERS_MAXIMUM_COUNT = '5';
415+
mockCreateRunner.mockImplementation(async () => {
416+
return ['i-instance-1', 'i-instance-2'];
417+
});
418+
mockListRunners.mockImplementation(async () => {
419+
return [];
420+
});
421+
422+
let callCount = 0;
423+
mockOctokit.actions.generateRunnerJitconfigForOrg.mockImplementation(({ name }) => {
424+
callCount++;
425+
if (name === 'unit-test-i-instance-1') {
426+
const error = new Error('Internal Server Error') as any;
427+
error.status = 500;
428+
error.response = {
429+
status: 500,
430+
data: { message: 'Internal server error' },
431+
};
432+
throw error;
433+
}
434+
return {
435+
data: {
436+
runner: { id: 9876543210 },
437+
encoded_jit_config: `TEST_JIT_CONFIG_${name}`,
438+
},
439+
headers: {},
440+
};
441+
});
442+
443+
await scaleUpModule.scaleUp(TEST_DATA);
444+
445+
expect(mockSSMClient).toHaveReceivedCommandWith(PutParameterCommand, {
446+
Name: '/github-action-runners/default/runners/config/i-instance-2',
447+
Value: 'TEST_JIT_CONFIG_unit-test-i-instance-2',
448+
Type: 'SecureString',
449+
Tags: [{ Key: 'InstanceId', Value: 'i-instance-2' }],
450+
});
451+
452+
expect(mockSSMClient).not.toHaveReceivedCommandWith(PutParameterCommand, {
453+
Name: '/github-action-runners/default/runners/config/i-instance-1',
454+
});
455+
});
456+
457+
it('should handle non-retryable 4xx errors gracefully', async () => {
458+
process.env.RUNNERS_MAXIMUM_COUNT = '5';
459+
mockCreateRunner.mockImplementation(async () => {
460+
return ['i-instance-1', 'i-instance-2'];
461+
});
462+
mockListRunners.mockImplementation(async () => {
463+
return [];
464+
});
465+
466+
let callCount = 0;
467+
mockOctokit.actions.generateRunnerJitconfigForOrg.mockImplementation(({ name }) => {
468+
callCount++;
469+
if (name === 'unit-test-i-instance-1') {
470+
// 404 is not retryable - will fail immediately
471+
const error = new Error('Not Found') as any;
472+
error.status = 404;
473+
error.response = {
474+
status: 404,
475+
data: { message: 'Resource not found' },
476+
};
477+
throw error;
478+
}
479+
return {
480+
data: {
481+
runner: { id: 9876543210 },
482+
encoded_jit_config: `TEST_JIT_CONFIG_${name}`,
483+
},
484+
headers: {},
485+
};
486+
});
487+
488+
await scaleUpModule.scaleUp(TEST_DATA);
489+
490+
expect(mockSSMClient).toHaveReceivedCommandWith(PutParameterCommand, {
491+
Name: '/github-action-runners/default/runners/config/i-instance-2',
492+
Value: 'TEST_JIT_CONFIG_unit-test-i-instance-2',
493+
Type: 'SecureString',
494+
Tags: [{ Key: 'InstanceId', Value: 'i-instance-2' }],
495+
});
496+
497+
expect(mockSSMClient).not.toHaveReceivedCommandWith(PutParameterCommand, {
498+
Name: '/github-action-runners/default/runners/config/i-instance-1',
499+
});
500+
});
501+
413502
it.each(RUNNER_TYPES)(
414503
'calls create start runner config of 40' + ' instances (ssm rate limit condition) to test time delay ',
415504
async (type: RunnerType) => {

lambdas/yarn.lock

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ __metadata:
155155
"@middy/core": "npm:^6.4.5"
156156
"@octokit/auth-app": "npm:8.1.2"
157157
"@octokit/core": "npm:7.0.6"
158+
"@octokit/plugin-retry": "npm:8.0.3"
158159
"@octokit/plugin-throttling": "npm:11.0.3"
159160
"@octokit/rest": "npm:22.0.1"
160161
"@octokit/types": "npm:^16.0.0"
@@ -3685,6 +3686,19 @@ __metadata:
36853686
languageName: node
36863687
linkType: hard
36873688

3689+
"@octokit/plugin-retry@npm:8.0.3":
3690+
version: 8.0.3
3691+
resolution: "@octokit/plugin-retry@npm:8.0.3"
3692+
dependencies:
3693+
"@octokit/request-error": "npm:^7.0.2"
3694+
"@octokit/types": "npm:^16.0.0"
3695+
bottleneck: "npm:^2.15.3"
3696+
peerDependencies:
3697+
"@octokit/core": ">=7"
3698+
checksum: 10c0/24d35d85f750f9e3e52f63b8ddd8fc8aa7bdd946c77b9ea4d6894d026c5c2c69109e8de3880a9970c906f624eb777c7d0c0a2072e6d41dadc7b36cce104b978c
3699+
languageName: node
3700+
linkType: hard
3701+
36883702
"@octokit/plugin-throttling@npm:11.0.3":
36893703
version: 11.0.3
36903704
resolution: "@octokit/plugin-throttling@npm:11.0.3"

0 commit comments

Comments
 (0)