test: add handling for 404 errors in scale-down tests and improve error logging

Author: stuartp44

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

@@ -403,6 +403,106 @@ describe('Scale down runners', () => {
         expect(mockTerminateRunners).toHaveBeenCalledWith(orphanRunner.instanceId);
       });
 
+      it('Should handle 404 error when checking orphaned runner (JIT) - treat as orphaned', async () => {
+        // arrange
+        const orphanRunner = createRunnerTestData(
+          'orphan-jit-404',
+          type,
+          MINIMUM_BOOT_TIME + 1,
+          false,
+          true,
+          true, // should be terminated when 404
+          undefined,
+          1234567890,
+        );
+        const runners = [orphanRunner];
+
+        mockGitHubRunners([]);
+        mockAwsRunners(runners);
+
+        // Mock 404 error response
+        const error404 = new Error('Runner not found');
+        (error404 as any).status = 404;
+
+        if (type === 'Repo') {
+          mockOctokit.actions.getSelfHostedRunnerForRepo.mockRejectedValueOnce(error404);
+        } else {
+          mockOctokit.actions.getSelfHostedRunnerForOrg.mockRejectedValueOnce(error404);
+        }
+
+        // act
+        await scaleDown();
+
+        // assert - should terminate since 404 means runner doesn't exist on GitHub
+        expect(mockTerminateRunners).toHaveBeenCalledWith(orphanRunner.instanceId);
+      });
+
+      it('Should handle 404 error when checking runner busy state - treat as not busy', async () => {
+        // arrange
+        const runner = createRunnerTestData(
+          'runner-404',
+          type,
+          MINIMUM_TIME_RUNNING_IN_MINUTES + 1,
+          true,
+          false,
+          true, // should be terminated since not busy due to 404
+        );
+        const runners = [runner];
+
+        mockGitHubRunners(runners);
+        mockAwsRunners(runners);
+
+        // Mock 404 error response for busy state check
+        const error404 = new Error('Runner not found');
+        (error404 as any).status = 404;
+
+        if (type === 'Repo') {
+          mockOctokit.actions.getSelfHostedRunnerForRepo.mockRejectedValueOnce(error404);
+        } else {
+          mockOctokit.actions.getSelfHostedRunnerForOrg.mockRejectedValueOnce(error404);
+        }
+
+        // act
+        await scaleDown();
+
+        // assert - should terminate since 404 means runner is not busy
+        checkTerminated(runners);
+      });
+
+      it('Should re-throw non-404 errors when checking runner state', async () => {
+        // arrange
+        const orphanRunner = createRunnerTestData(
+          'orphan-error',
+          type,
+          MINIMUM_BOOT_TIME + 1,
+          false,
+          true,
+          false,
+          undefined,
+          1234567890,
+        );
+        const runners = [orphanRunner];
+
+        mockGitHubRunners([]);
+        mockAwsRunners(runners);
+
+        // Mock non-404 error response
+        const error500 = new Error('Internal server error');
+        (error500 as any).status = 500;
+
+        if (type === 'Repo') {
+          mockOctokit.actions.getSelfHostedRunnerForRepo.mockRejectedValueOnce(error500);
+        } else {
+          mockOctokit.actions.getSelfHostedRunnerForOrg.mockRejectedValueOnce(error500);
+        }
+
+        // act & assert - should not throw because error handling is in terminateOrphan
+        await expect(scaleDown()).resolves.not.toThrow();
+
+        // Should not terminate since the error was not a 404
+        expect(mockTerminateRunners).not.toHaveBeenCalledWith(orphanRunner.instanceId);
+      });
+
       it(`Should ignore errors when termination orphan fails.`, async () => {
         // setup
         const orphanRunner = createRunnerTestData('orphan-1', type, MINIMUM_BOOT_TIME + 1, false, true, true);

lambdas/functions/control-plane/src/scale-runners/scale-down.ts

@@ -55,25 +55,37 @@ async function getGitHubSelfHostedRunnerState(
   client: Octokit,
   ec2runner: RunnerInfo,
   runnerId: number,
-): Promise<RunnerState> {
-  const state =
-    ec2runner.type === 'Org'
-      ? await client.actions.getSelfHostedRunnerForOrg({
-          runner_id: runnerId,
-          org: ec2runner.owner,
-        })
-      : await client.actions.getSelfHostedRunnerForRepo({
-          runner_id: runnerId,
-          owner: ec2runner.owner.split('/')[0],
-          repo: ec2runner.owner.split('/')[1],
-        });
-  metricGitHubAppRateLimit(state.headers);
-
-  return state.data;
+): Promise<RunnerState | null> {
+  try {
+    const state =
+      ec2runner.type === 'Org'
+        ? await client.actions.getSelfHostedRunnerForOrg({
+            runner_id: runnerId,
+            org: ec2runner.owner,
+          })
+        : await client.actions.getSelfHostedRunnerForRepo({
+            runner_id: runnerId,
+            owner: ec2runner.owner.split('/')[0],
+            repo: ec2runner.owner.split('/')[1],
+          });
+    metricGitHubAppRateLimit(state.headers);
+
+    return state.data;
+  } catch (error: any) {
+    if (error.status === 404) {
+      logger.info(`Runner '${ec2runner.instanceId}' with GitHub Runner ID '${runnerId}' not found on GitHub (404)`);
+      return null;
+    }
+    throw error;
+  }
 }
 
 async function getGitHubRunnerBusyState(client: Octokit, ec2runner: RunnerInfo, runnerId: number): Promise<boolean> {
   const state = await getGitHubSelfHostedRunnerState(client, ec2runner, runnerId);
+  if (state === null) {
+    logger.info(`Runner '${ec2runner.instanceId}' - GitHub Runner ID '${runnerId}' - Not found on GitHub, treating as not busy`);
+    return false;
+  }
   logger.info(`Runner '${ec2runner.instanceId}' - GitHub Runner ID '${runnerId}' - Busy: ${state.busy}`);
   return state.busy;
 }
@@ -227,12 +239,18 @@ async function lastChanceCheckOrphanRunner(runner: RunnerList): Promise<boolean>
   const ec2Instance = runner as RunnerInfo;
   const state = await getGitHubSelfHostedRunnerState(client, ec2Instance, runnerId);
   let isOrphan = false;
-  logger.debug(
-    `Runner '${runner.instanceId}' is '${state.status}' and is currently '${state.busy ? 'busy' : 'idle'}'.`,
-  );
-  const isOfflineAndBusy = state.status === 'offline' && state.busy;
-  if (isOfflineAndBusy) {
+
+  if (state === null) {
+    logger.debug(`Runner '${runner.instanceId}' not found on GitHub, treating as orphaned.`);
     isOrphan = true;
+  } else {
+    logger.debug(
+      `Runner '${runner.instanceId}' is '${state.status}' and is currently '${state.busy ? 'busy' : 'idle'}'.`,
+    );
+    const isOfflineAndBusy = state.status === 'offline' && state.busy;
+    if (isOfflineAndBusy) {
+      isOrphan = true;
+    }
   }
   logger.info(`Runner '${runner.instanceId}' is judged to ${isOrphan ? 'be' : 'not be'} orphaned.`);
   return isOrphan;