feat(pool): Allow to optionally include busy runners into the pool

'pool_config' is useful when one wants to ensure that a certain number
of runners is running within given time periods.
However, when 'pool_config' is used in a combination with persistent runners,
the pool lambda tends to spin up new runner instance when it sees that some
existing runners are online, but busy.

As a result, with frequent pool lambda checks, the total size of the runners
pool tends to grow more than desired (without respecting
'runners_maximum_count', which is only considered by the 'scale_up' lambda).

These changes introduce a new 'pool_include_busy_runners' module variable
to make it possbile to include all online runners (both idle and busy)
into the pool, so that the pool's lambda only tops up the pool if not
enough runners are online.

Author: rvem

lambdas/functions/control-plane/src/pool/pool.test.ts

@@ -336,4 +336,22 @@ describe('Test simple pool.', () => {
       expect(createRunners).toHaveBeenCalledWith(expect.anything(), expect.anything(), 1, expect.anything());
     });
   });
+
+  describe('With INCLUDE_BUSY_RUNNERS enabled', () => {
+    beforeEach(() => {
+      process.env.INCLUDE_BUSY_RUNNERS = 'true';
+    });
+
+    it('Should not top up when pool size matches runners including busy online runners.', async () => {
+      // Without INCLUDE_BUSY_RUNNERS: 2 in pool (i-1-idle, i-4-idle-older). With it: 3 (adds i-2-busy).
+      await adjust({ poolSize: 3 });
+      expect(createRunners).not.toHaveBeenCalled();
+    });
+
+    it('Should top up by two runners when pool size is 5 and busy runners count toward the pool.', async () => {
+      await adjust({ poolSize: 5 });
+      // 3 in pool (idle, busy, older idle); need 2 more
+      expect(createRunners).toHaveBeenCalledWith(expect.anything(), expect.anything(), 2, expect.anything());
+    });
+  });
 });

lambdas/functions/control-plane/src/pool/pool.ts

@@ -47,6 +47,7 @@ export async function adjust(event: PoolEvent): Promise<void> {
       ? validateSsmParameterStoreTags(process.env.SSM_PARAMETER_STORE_TAGS)
       : [];
   const scaleErrors = JSON.parse(process.env.SCALE_ERRORS) as [string];
+  const includeBusyRunners = yn(process.env.INCLUDE_BUSY_RUNNERS, { default: false });
 
   const { ghesApiUrl, ghesBaseUrl } = getGitHubEnterpriseApiUrl();
 
@@ -69,7 +70,7 @@ export async function adjust(event: PoolEvent): Promise<void> {
     statuses: ['running'],
   });
 
-  const numberOfRunnersInPool = calculatePooSize(ec2runners, runnerStatusses);
+  const numberOfRunnersInPool = calculatePooSize(ec2runners, runnerStatusses, includeBusyRunners);
   const topUp = event.poolSize - numberOfRunnersInPool;
 
   if (topUp > 0) {
@@ -123,12 +124,16 @@ async function getInstallationId(ghesApiUrl: string, org: string): Promise<numbe
   ).data.id;
 }
 
-function calculatePooSize(ec2runners: RunnerList[], runnerStatus: Map<string, RunnerStatus>): number {
+function calculatePooSize(
+  ec2runners: RunnerList[],
+  runnerStatus: Map<string, RunnerStatus>,
+  includeBusyRunners: boolean,
+): number {
   // Runner should be considered idle if it is still booting, or is idle in GitHub
   let numberOfRunnersInPool = 0;
   for (const ec2Instance of ec2runners) {
     if (
-      runnerStatus.get(ec2Instance.instanceId)?.busy === false &&
+      (runnerStatus.get(ec2Instance.instanceId)?.busy === false || includeBusyRunners) &&
       runnerStatus.get(ec2Instance.instanceId)?.status === 'online'
     ) {
       numberOfRunnersInPool++;

main.tf

@@ -272,6 +272,7 @@ module "runners" {
   pool_lambda_timeout                        = var.pool_lambda_timeout
   pool_runner_owner                          = var.pool_runner_owner
   pool_lambda_reserved_concurrent_executions = var.pool_lambda_reserved_concurrent_executions
+  pool_include_busy_runners                  = var.pool_include_busy_runners
 
   ssm_housekeeper = var.runners_ssm_housekeeper
   ebs_optimized   = var.runners_ebs_optimized

modules/runners/pool.tf

@@ -37,6 +37,7 @@ module "pool" {
       parameter_store_tags           = local.parameter_store_tags
     }
     pool                      = var.pool_config
+    include_busy_runners      = var.pool_include_busy_runners
     role_path                 = local.role_path
     role_permissions_boundary = var.role_permissions_boundary
     runner = {

modules/runners/pool/main.tf

@@ -49,6 +49,7 @@ resource "aws_lambda_function" "pool" {
       ENABLE_ON_DEMAND_FAILOVER_FOR_ERRORS     = jsonencode(var.config.runner.enable_on_demand_failover_for_errors)
       SSM_PARAMETER_STORE_TAGS                 = var.config.lambda.parameter_store_tags
       SCALE_ERRORS                             = jsonencode(var.config.runner.scale_errors)
+      INCLUDE_BUSY_RUNNERS                     = var.config.include_busy_runners
     }
   }
 

modules/runners/pool/variables.tf

@@ -57,6 +57,7 @@ variable "config" {
       schedule_expression_timezone = string
       size                         = number
     }))
+    include_busy_runners                 = bool
     role_permissions_boundary            = string
     kms_key_arn                          = string
     ami_kms_key_arn                      = string

modules/runners/variables.tf

@@ -572,6 +572,12 @@ variable "pool_config" {
   default = []
 }
 
+variable "pool_include_busy_runners" {
+  description = "Include busy runners in the pool calculation. By default busy runners are not included in the pool."
+  type        = bool
+  default     = false
+}
+
 variable "disable_runner_autoupdate" {
   description = "Disable the auto update of the github runner agent. Be aware there is a grace period of 30 days, see also the [GitHub article](https://github.blog/changelog/2022-02-01-github-actions-self-hosted-runners-can-now-disable-automatic-updates/)"
   type        = bool

variables.tf

@@ -765,6 +765,12 @@ variable "pool_config" {
   default = []
 }
 
+variable "pool_include_busy_runners" {
+  description = "Include busy runners in the pool calculation. By default busy runners are not included in the pool."
+  type        = bool
+  default     = false
+}
+
 variable "aws_partition" {
   description = "(optiona) partition in the arn namespace to use if not 'aws'"
   type        = string