diff --git a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/gh-runners.test.ts b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/gh-runners.test.ts index 31e3000eea..13bb7627a8 100644 --- a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/gh-runners.test.ts +++ b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/gh-runners.test.ts @@ -1,13 +1,14 @@ import { - GhRunners, createGitHubClientForRunnerInstallId, createGitHubClientForRunnerOrg, createGitHubClientForRunnerRepo, createRegistrationTokenOrg, createRegistrationTokenRepo, + getGitHubRateLimit, getRunnerOrg, getRunnerRepo, getRunnerTypes, + GhRunners, listGithubRunnersOrg, listGithubRunnersRepo, removeGithubRunnerOrg, @@ -20,7 +21,7 @@ import { ScaleUpMetrics } from './metrics'; import { Config } from './config'; import { Octokit } from '@octokit/rest'; import { mocked } from 'ts-jest/utils'; -import { locallyCached } from './cache'; +import { locallyCached, redisCached } from './cache'; import nock from 'nock'; const mockEC2 = { @@ -1081,3 +1082,51 @@ describe('createRegistrationToken', () => { }); }); }); + +describe('getGitHubRateLimit', () => { + const repo = { owner: 'owner', repo: 'repo' }; + + it('succeeds, make sure it is using the cache mechanism', async () => { + const mocckedRedisCache = mocked(redisCached).mockImplementationOnce((_, __, ___, ____, f) => f()); + const mockCreateGithubAuth = mocked(createGithubAuth); + const mockCreateOctoClient = mocked(createOctoClient); + const getRepoInstallation = jest.fn().mockResolvedValue({ + data: { id: 'mockReturnValueOnce1' }, + }); + const mockedOctokit = { + actions: { + deleteSelfHostedRunnerFromRepo: jest.fn().mockResolvedValue({ + status: 204, + }), + }, + apps: { getRepoInstallation: getRepoInstallation }, + rateLimit: { + get: jest.fn().mockResolvedValue({ + data: {}, + headers: { + 'x-ratelimit-limit': '5000', + 'x-ratelimit-remaining': '4999', + 'x-ratelimit-used': '1', + }, + status: 200, + }), + }, + }; + + mockCreateGithubAuth.mockResolvedValueOnce('token1'); + mockCreateOctoClient.mockReturnValueOnce(mockedOctokit as unknown as Octokit); + mockCreateGithubAuth.mockResolvedValueOnce('token2'); + mockCreateOctoClient.mockReturnValueOnce(mockedOctokit as unknown as Octokit); + + await resetGHRunnersCaches(); + const response = await getGitHubRateLimit(repo, metrics); + + expect(mocckedRedisCache).toBeCalledTimes(2); + expect(mockedOctokit.rateLimit.get).toBeCalledTimes(1); + expect(response).toEqual({ + limit: 5000, + remaining: 4999, + used: 1, + }); + }); +}); diff --git a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/gh-runners.ts b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/gh-runners.ts index 9185e394b5..627b34ad02 100644 --- a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/gh-runners.ts +++ b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/gh-runners.ts @@ -12,6 +12,12 @@ import YAML from 'yaml'; const ghMainClientCache = new LRU({ maxAge: 10 * 1000 }); const ghClientCache = new LRU({ maxAge: 10 * 1000 }); +export interface GHRateLimitInfo { + limit: number; + remaining: number; + used: number; +} + export async function resetGHRunnersCaches() { await redisClearCacheKeyPattern('ghRunners', ''); clearLocalCacheNamespace('ghRunners'); @@ -366,6 +372,7 @@ export async function getRunnerTypes( Array.from(result.keys()).forEach((key) => { const runnerType = result.get(key); + /* istanbul ignore next */ if (runnerType?.variants === undefined) { return; } @@ -373,6 +380,7 @@ export async function getRunnerTypes( if (runnerType.variants.size > 0) { Array.from(runnerType.variants.keys()).forEach((variant) => { const variantType = runnerType.variants?.get(variant); + /* istanbul ignore next */ if (!variantType) { return; } @@ -400,6 +408,7 @@ export async function getRunnerTypes( typeof runnerType.instance_type === 'string' && alphaNumericStr.test(runnerType.instance_type) && ['linux', 'windows'].includes(runnerType.os) && + /* istanbul ignore next */ (runnerType.labels?.every((label) => typeof label === 'string' && alphaNumericStr.test(label)) ?? true) && (typeof runnerType.disk_size === 'number' || runnerType.disk_size === undefined) && (typeof runnerType.min_available === 'number' || runnerType.min_available === undefined) && @@ -517,9 +526,9 @@ export async function createRegistrationTokenOrg( } } -export async function getGitHubRateLimit(repo: Repo, installationId: number, metrics: Metrics): Promise { +export async function getGitHubRateLimit(repo: Repo, metrics: Metrics): Promise { try { - const { used, limit, remaining } = await locallyCached('ghRunners', 'getGitHubRateLimit', 10, async () => { + return await redisCached('ghRunners', 'getGitHubRateLimit', 10, 0.5, async () => { try { const client = await createGitHubClientForRunnerRepo(repo, metrics); @@ -535,13 +544,13 @@ export async function getGitHubRateLimit(repo: Repo, installationId: number, met return { used, limit, remaining }; } catch (e) { + /* istanbul ignore next */ console.error(`[getGitHubRateLimit]: ${e}`); throw e; } }); - - metrics.gitHubRateLimitStats(limit, remaining, used); } catch (e) { + /* istanbul ignore next */ console.error(`[getGitHubRateLimit]: ${e}`); throw e; } diff --git a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up.test.ts b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up.test.ts index 18f7130f07..6e9d102a87 100644 --- a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up.test.ts +++ b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up.test.ts @@ -2,6 +2,7 @@ import { createRunner } from './runners'; import { createRegistrationTokenOrg, createRegistrationTokenRepo, + getGitHubRateLimit, getRunnerTypes, listGithubRunnersOrg, listGithubRunnersRepo, @@ -23,6 +24,8 @@ beforeEach(() => { jest.clearAllMocks(); jest.restoreAllMocks(); nock.disableNetConnect(); + + mocked(getGitHubRateLimit).mockResolvedValue({ limit: 5000, remaining: 4999, used: 1 }); }); const baseCfg = { diff --git a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up.ts b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up.ts index 2f458983d3..014c33c285 100644 --- a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up.ts +++ b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up.ts @@ -55,7 +55,13 @@ export async function scaleUp( metrics.runRepo(repo); metrics.run(); - getGitHubRateLimit(repo, Number(payload.installationId), metrics); + try { + const ghLimitInfo = await getGitHubRateLimit(repo, metrics); + metrics.gitHubRateLimitStats(ghLimitInfo.limit, ghLimitInfo.remaining, ghLimitInfo.used); + } catch (e) { + /* istanbul ignore next */ + console.error(`Error getting GitHub rate limit: ${e}`); + } const scaleConfigRepo = { owner: repo.owner,