Add SQS-based deregistration retry for busy runners (C-243)

When GitHub returns 422 on runner deletion (runner executing a job),
instead of silently dropping the attempt, enqueue a retry message to
SQS with a 5-minute delay. By that time the EC2 instance has been
terminated and the runner appears offline, allowing clean deletion.

Changes:
- deregister.ts: send 422 failures to DEREGISTER_RETRY_QUEUE_URL SQS
  queue; add handleDeregisterRetry for processing retry messages
- lambda.ts: export deregisterRetry SQS handler
- package.json: add @aws-sdk/client-sqs dependency
- scale-down.ts: remove reconcileGitHubRunners polling (replaced by SQS)
- modules/multi-runner: add environment_variables to
  instance_termination_watcher variable and pass through to Lambda config
- modules/termination-watcher: merge caller-supplied environment_variables
  into notification and handler Lambda env var configs

Author: jensenbox

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

@@ -313,91 +313,6 @@ function filterRunners(ec2runners: RunnerList[]): RunnerInfo[] {
   return ec2runners.filter((ec2Runner) => ec2Runner.type && !ec2Runner.orphan) as RunnerInfo[];
 }
 
-async function reconcileGitHubRunners(environment: string, ec2Runners: RunnerList[]): Promise<void> {
-  const offlineThresholdMinutes = parseInt(process.env.OFFLINE_RUNNER_DEREGISTER_MINUTES ?? '10');
-  if (offlineThresholdMinutes <= 0) {
-    logger.debug('Offline runner reconciliation is disabled (threshold <= 0)');
-    return;
-  }
-
-  const ec2InstanceIds = new Set(ec2Runners.map((r) => r.instanceId));
-
-  // Build a set of unique owners/types from the EC2 runners we know about.
-  // If there are no EC2 runners, we still need at least one owner to query GitHub.
-  // Fall back to environment tags to find the org.
-  const ownerTypes = new Map<string, string>();
-  for (const r of ec2Runners) {
-    if (r.owner && r.type) {
-      ownerTypes.set(r.owner, r.type);
-    }
-  }
-
-  // If no EC2 runners exist, we can't determine the owner to query GitHub.
-  // This is fine — the scale-up Lambda will handle it once new runners register.
-  if (ownerTypes.size === 0) {
-    logger.debug('No EC2 runners with owner tags found, skipping GitHub runner reconciliation');
-    return;
-  }
-
-  for (const [owner, runnerType] of ownerTypes) {
-    try {
-      // Create a synthetic RunnerInfo to reuse the existing GitHub client helpers
-      const syntheticRunner: RunnerInfo = { instanceId: 'reconciler', owner, type: runnerType };
-      const ghRunners = await listGitHubRunners(syntheticRunner);
-
-      // Find GitHub runners whose name contains an environment prefix that matches ours,
-      // that are offline, and have no corresponding EC2 instance
-      const orphanedGhRunners = ghRunners.filter((ghRunner: { name: string; status: string; id: number }) => {
-        if (ghRunner.status !== 'offline') return false;
-        // Check if this runner's EC2 instance still exists
-        const matchesEc2 = Array.from(ec2InstanceIds).some((instanceId) => ghRunner.name.includes(instanceId));
-        return !matchesEc2;
-      });
-
-      if (orphanedGhRunners.length === 0) {
-        logger.debug(`No orphaned GitHub runners found for owner '${owner}'`);
-        continue;
-      }
-
-      logger.info(
-        `Found ${orphanedGhRunners.length} offline GitHub runner(s) with no EC2 instance for owner '${owner}'`,
-      );
-
-      const client = await getOrCreateOctokit(syntheticRunner);
-      for (const ghRunner of orphanedGhRunners) {
-        try {
-          if (runnerType === 'Org') {
-            await client.actions.deleteSelfHostedRunnerFromOrg({
-              org: owner,
-              runner_id: (ghRunner as { id: number }).id,
-            });
-          } else {
-            const [repoOwner, repo] = owner.split('/');
-            await client.actions.deleteSelfHostedRunnerFromRepo({
-              owner: repoOwner,
-              repo,
-              runner_id: (ghRunner as { id: number }).id,
-            });
-          }
-          logger.info(`Deregistered orphaned GitHub runner '${(ghRunner as { name: string }).name}' (ID: ${(ghRunner as { id: number }).id})`);
-        } catch (error) {
-          if (error instanceof RequestError && error.status === 422) {
-            logger.warn(
-              `Cannot deregister runner '${(ghRunner as { name: string }).name}' — still marked as busy. Will retry next cycle.`,
-            );
-          } else {
-            logger.error(`Failed to deregister orphaned runner '${(ghRunner as { name: string }).name}'`, {
-              error: error as Error,
-            });
-          }
-        }
-      }
-    } catch (error) {
-      logger.warn(`Failed to reconcile GitHub runners for owner '${owner}'`, { error: error as Error });
-    }
-  }
-}
-
 export async function scaleDown(): Promise<void> {
   githubCache.reset();
   const environment = process.env.ENVIRONMENT;
@@ -412,11 +327,6 @@ export async function scaleDown(): Promise<void> {
   logger.info(`Found: '${activeEc2RunnersCount}' active GitHub EC2 runner instances before clean-up.`);
   logger.debug(`Active GitHub EC2 runner instances: ${JSON.stringify(ec2Runners)}`);
 
-  // Reconcile: deregister GitHub runners whose EC2 instances no longer exist.
-  // This prevents deadlocks where offline ghost runners count toward the max,
-  // blocking scale-up from launching replacements.
-  await reconcileGitHubRunners(environment, ec2Runners);
-
   if (activeEc2RunnersCount === 0) {
     logger.debug(`No active runners found for environment: '${environment}'`);
     return;

lambdas/functions/termination-watcher/package.json

@@ -26,6 +26,7 @@
     "@aws-github-runner/aws-powertools-util": "*",
     "@aws-github-runner/aws-ssm-util": "*",
     "@aws-sdk/client-ec2": "^3.984.0",
+    "@aws-sdk/client-sqs": "^3.984.0",
     "@middy/core": "^6.4.5",
     "@octokit/auth-app": "8.2.0",
     "@octokit/core": "7.0.6",

lambdas/functions/termination-watcher/src/deregister.ts

@@ -3,11 +3,22 @@ import { Octokit } from '@octokit/rest';
 import { throttling } from '@octokit/plugin-throttling';
 import { request } from '@octokit/request';
 import { Instance } from '@aws-sdk/client-ec2';
+import { SQSClient, SendMessageCommand } from '@aws-sdk/client-sqs';
 import { createChildLogger } from '@aws-github-runner/aws-powertools-util';
 import { getParameter } from '@aws-github-runner/aws-ssm-util';
 import type { EndpointDefaults } from '@octokit/types';
 import type { Config } from './ConfigResolver';
 
+export interface DeregisterRetryMessage {
+  instanceId: string;
+  owner: string;
+  runnerType: string;
+  runnerId: number;
+  retryCount: number;
+}
+
+const sqsClient = new SQSClient({ region: process.env.AWS_REGION });
+
 const logger = createChildLogger('deregister');
 
 export function createThrottleOptions() {
@@ -198,20 +209,81 @@ export async function deregisterRunner(instance: Instance, config: Config): Prom
     });
   } catch (error) {
     // GitHub returns 422 when a runner is currently executing a job.
-    // The runner will become offline after the instance terminates, and the
-    // scale-down Lambda's reconciliation loop will clean it up on its next cycle.
+    // Queue a delayed retry — the instance will be terminated by EC2 shortly,
+    // and the runner will appear offline when we retry in 5 minutes.
     const isRunnerBusy = error instanceof Error && 'status' in error && (error as { status: number }).status === 422;
     if (isRunnerBusy) {
-      logger.warn('Runner is currently busy, cannot deregister now. Scale-down reconciliation will clean it up.', {
+      const queueUrl = process.env.DEREGISTER_RETRY_QUEUE_URL;
+      if (queueUrl) {
+        await queueDeregisterRetry(queueUrl, { instanceId, owner, runnerType, runnerId: 0, retryCount: 0 });
+        logger.warn('Runner is busy — queued deregistration retry in 5 minutes via SQS', { instanceId, owner });
+      } else {
+        logger.warn('Runner is busy and DEREGISTER_RETRY_QUEUE_URL is not set — deregistration skipped', {
+          instanceId,
+          owner,
+        });
+      }
+    } else {
+      logger.error('Failed to deregister runner from GitHub', {
         instanceId,
         owner,
+        error: error as Error,
+      });
+    }
+  }
+}
+
+async function queueDeregisterRetry(queueUrl: string, message: DeregisterRetryMessage): Promise<void> {
+  const command = new SendMessageCommand({
+    QueueUrl: queueUrl,
+    MessageBody: JSON.stringify(message),
+  });
+  await sqsClient.send(command);
+}
+
+export async function handleDeregisterRetry(queueUrl: string, message: DeregisterRetryMessage): Promise<void> {
+  const { instanceId, owner, runnerType, retryCount } = message;
+  logger.info('Processing deregistration retry from SQS', { instanceId, owner, runnerType, retryCount });
+
+  try {
+    const appOctokit = await createAuthenticatedClient('');
+    const installationOctokit = await createInstallationClient(appOctokit, owner, runnerType, '');
+
+    const runner = await findRunnerByInstanceId(installationOctokit, owner, instanceId, runnerType);
+    if (!runner) {
+      logger.info('Runner not found in GitHub — already deregistered or never registered', { instanceId, owner });
+      return;
+    }
+
+    await deleteRunner(installationOctokit, owner, runner.id, runnerType);
+    logger.info('Successfully deregistered runner via SQS retry', {
+      instanceId,
+      runnerId: runner.id,
+      runnerName: runner.name,
+      owner,
+      retryCount,
+    });
+  } catch (error) {
+    const isRunnerBusy = error instanceof Error && 'status' in error && (error as { status: number }).status === 422;
+    if (isRunnerBusy) {
+      // Re-enqueue for another retry — SQS maxReceiveCount DLQ will stop after 3 total attempts.
+      // Re-send explicitly so each retry resets the delay (SQS visibility timeout applies on re-receive,
+      // but re-sending gives us the full 5-minute DelaySeconds again).
+      await queueDeregisterRetry(queueUrl, { ...message, retryCount: retryCount + 1 });
+      logger.warn('Runner still busy on retry — re-queued for another attempt', {
+        instanceId,
+        owner,
+        retryCount: retryCount + 1,
       });
     } else {
-      logger.error('Failed to deregister runner from GitHub', {
+      logger.error('Failed to deregister runner on retry', {
         instanceId,
         owner,
+        retryCount,
         error: error as Error,
       });
+      // Re-throw so SQS treats this as a failure and routes to DLQ after maxReceiveCount
+      throw error;
     }
   }
 }

lambdas/functions/termination-watcher/src/lambda.ts

@@ -1,10 +1,11 @@
 import middy from '@middy/core';
 import { captureLambdaHandler, logger, metrics, setContext, tracer } from '@aws-github-runner/aws-powertools-util';
 import { logMetrics } from '@aws-lambda-powertools/metrics/middleware';
-import { Context } from 'aws-lambda';
+import { Context, SQSEvent } from 'aws-lambda';
 
 import { handle as handleTerminationWarning } from './termination-warning';
 import { handle as handleTermination } from './termination';
+import { handleDeregisterRetry, DeregisterRetryMessage } from './deregister';
 import { BidEvictedDetail, BidEvictedEvent, SpotInterruptionWarning, SpotTerminationDetail } from './types';
 import { Config } from './ConfigResolver';
 
@@ -37,6 +38,29 @@ export async function termination(event: BidEvictedEvent<BidEvictedDetail>, cont
   }
 }
 
+export async function deregisterRetry(event: SQSEvent, context: Context): Promise<void> {
+  setContext(context, 'lambda.ts');
+  logger.logEventIfEnabled(event);
+  logger.debug('Processing SQS deregister retry batch', { recordCount: event.Records.length });
+
+  const queueUrl = process.env.DEREGISTER_RETRY_QUEUE_URL;
+  if (!queueUrl) {
+    logger.error('DEREGISTER_RETRY_QUEUE_URL is not set — cannot process retry messages');
+    return;
+  }
+
+  for (const record of event.Records) {
+    try {
+      const message = JSON.parse(record.body) as DeregisterRetryMessage;
+      await handleDeregisterRetry(queueUrl, message);
+    } catch (e) {
+      logger.error(`Failed to process SQS record ${record.messageId}`, { error: e as Error });
+      // Re-throw to mark the message as failed so SQS can retry or route to DLQ
+      throw e;
+    }
+  }
+}
+
 const addMiddleware = () => {
   const middleware = middy(interruptionWarning);