From 0e9b083ec99b228037acca4477e680deb6343bb7 Mon Sep 17 00:00:00 2001
From: Timo Ulich <ulich@users.noreply.github.com>
Date: Fri, 25 Mar 2022 14:58:04 +0100
Subject: [PATCH] fix: Don't delete busy runners (#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>
---
 .../src/scale-runners/scale-down.test.ts      | 22 ++++++++++++++++++-
 .../runners/src/scale-runners/scale-down.ts   |  9 +++-----
 2 files changed, 24 insertions(+), 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
index e17afd79ed..45f8afd8b9 100644
--- 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
index 1c48af50e7..df4ee1fe97 100644
--- 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);