fix: Don't delete busy runners (philips-labs#1832)

* Don't attempt to delete busy runners * fix idleCounter logic * adjust tests to cover the new case * remove debugging code Co-authored-by: Samuel Barabas <samuel@owee.de>
mcaulifn · Mar 25, 2022 · 0e9b083 · 0e9b083
1 parent c17303d
commit 0e9b083
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 7 deletions.
diff --git a/modules/runners/lambdas/runners/src/scale-runners/scale-down.test.ts b/modules/runners/lambdas/runners/src/scale-runners/scale-down.test.ts
@@ -143,32 +143,51 @@ const DEFAULT_RUNNERS_ORIGINAL = [
     launchTime: moment(new Date()).toDate(),
     repo: `${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`,
   },
+  {
+    instanceId: 'i-busy-112',
+    launchTime: moment(new Date())
+      .subtract(minimumRunningTimeInMinutes + 27, 'minutes')
+      .toDate(),
+    type: 'Org',
+    owner: TEST_DATA.repositoryOwner,
+  },
 ];
 
 const DEFAULT_REGISTERED_RUNNERS = [
   {
     id: 101,
     name: 'i-idle-101',
+    busy: false,
   },
   {
     id: 102,
     name: 'i-idle-102',
+    busy: false,
   },
   {
     id: 103,
     name: 'i-oldest-idle-103',
+    busy: false,
   },
   {
     id: 104,
     name: 'i-oldest-idle-104',
+    busy: false,
   },
   {
     id: 105,
     name: 'i-running-105',
+    busy: false,
   },
   {
     id: 106,
     name: 'i-running-106',
+    busy: false,
+  },
+  {
+    id: 112,
+    name: 'i-busy-112',
+    busy: true,
   },
 ];
 
@@ -260,7 +279,8 @@ describe('scaleDown', () => {
     );
 
     RUNNERS_ALL_REMOVED = DEFAULT_RUNNERS_ORG.filter(
-      (r) => !r.instanceId.includes('running') && !r.instanceId.includes('registered'),
+      (r) =>
+        !r.instanceId.includes('running') && !r.instanceId.includes('registered') && !r.instanceId.includes('busy'),
     );
     DEFAULT_RUNNERS_ORPHANED = DEFAULT_RUNNERS_ORIGINAL.filter(
       (r) => r.instanceId.includes('orphan') && !r.instanceId.includes('not-registered'),

diff --git a/modules/runners/lambdas/runners/src/scale-runners/scale-down.ts b/modules/runners/lambdas/runners/src/scale-runners/scale-down.ts
@@ -111,10 +111,7 @@ async function removeRunner(ec2runner: RunnerInfo, ghRunnerId: number): Promise<
       logger.error(`Failed to de-register GitHub runner: ${result.status}`, LogFields.print());
     }
   } catch (e) {
-    logger.info(
-      `Runner '${ec2runner.instanceId}' cannot be de-registered, most likely the runner is active.`,
-      LogFields.print(),
-    );
+    logger.error(`Runner '${ec2runner.instanceId}' cannot be de-registered. Error: ${e}`, LogFields.print());
   }
 }
 
@@ -135,10 +132,10 @@ async function evaluateAndRemoveRunners(
       const ghRunners = await listGitHubRunners(ec2Runner);
       const ghRunner = ghRunners.find((runner) => runner.name === ec2Runner.instanceId);
       if (ghRunner) {
-        if (runnerMinimumTimeExceeded(ec2Runner)) {
+        if (!ghRunner.busy && runnerMinimumTimeExceeded(ec2Runner)) {
           if (idleCounter > 0) {
             idleCounter--;
-            logger.info(`Runner '${ec2Runner.instanceId}' will kept idle.`, LogFields.print());
+            logger.info(`Runner '${ec2Runner.instanceId}' will be kept idle.`, LogFields.print());
           } else {
             logger.info(`Runner '${ec2Runner.instanceId}' will be terminated.`, LogFields.print());
             await removeRunner(ec2Runner, ghRunner.id);