feat(scale-down): Update Owner Logic (philips-labs#1065)

* feat(scale-down): Update Owner Logic * Update expect count * Removing org runner flag Prettier * Refactoring * Fixing resolved conflicts * Terminate legacy runners (philips-labs#2) * Terminate legacy runners * Update modules/runners/lambdas/runners/src/scale-runners/scale-down.ts Co-authored-by: Gertjan Maas <gertjan@maas.codes> * Move find index to new function * Removing old comment Co-authored-by: Gertjan Maas <gertjan@maas.codes> * Update modules/runners/lambdas/runners/src/scale-runners/scale-down.ts Co-authored-by: Niek Palm <npalm@users.noreply.github.com> * Addressing feedback * Shouldn't need to update original runner list anymore * Fixing case for legacy conversion * Update var descr * Add boot time check Co-authored-by: Gertjan Maas <gertjan@maas.codes> Co-authored-by: Niek Palm <npalm@users.noreply.github.com>
surminus · Aug 31, 2021 · ba2536b · ba2536b
1 parent f7f194d
commit ba2536b
Show file tree

Hide file tree

Showing 12 changed files with 613 additions and 575 deletions.
diff --git a/README.md b/README.md
diff --git a/modules/runners/README.md b/modules/runners/README.md
@@ -87,15 +87,15 @@ No Modules.
 
 | Name | Description | Type | Default | Required |
 |------|-------------|------|---------|:--------:|
-| ami\_filter | List of maps used to create the AMI filter for the action runner AMI. | `map(list(string))` | <pre>{<br>  "name": [<br>    "amzn2-ami-hvm-2.*-x86_64-ebs"<br>  ]<br>}</pre> | no |
+| ami\_filter | Map of lists used to create the AMI filter for the action runner AMI. | `map(list(string))` | <pre>{<br>  "name": [<br>    "amzn2-ami-hvm-2.*-x86_64-ebs"<br>  ]<br>}</pre> | no |
 | ami\_owners | The list of owners used to select the AMI of action runner instances. | `list(string)` | <pre>[<br>  "amazon"<br>]</pre> | no |
 | aws\_region | AWS region. | `string` | n/a | yes |
 | block\_device\_mappings | The EC2 instance block device configuration. Takes the following keys: `device_name`, `delete_on_termination`, `volume_type`, `volume_size`, `encrypted`, `iops` | `map(string)` | `{}` | no |
 | cloudwatch\_config | (optional) Replaces the module default cloudwatch log config. See https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch-Agent-Configuration-File-Details.html for details. | `string` | `null` | no |
-| create\_service\_linked\_role\_spot | (optional) create the serviced linked role for spot instances that is required by the scale-up lambda. | `bool` | `false` | no |
+| create\_service\_linked\_role\_spot | (optional) create the service linked role for spot instances that is required by the scale-up lambda. | `bool` | `false` | no |
 | enable\_cloudwatch\_agent | Enabling the cloudwatch agent on the ec2 runner instances, the runner contains default config. Configuration can be overridden via `cloudwatch_config`. | `bool` | `true` | no |
 | enable\_organization\_runners | n/a | `bool` | n/a | yes |
-| enable\_ssm\_on\_runners | Enable to allow access the runner instances for debugging purposes via SSM. Note that this adds additional permissions to the runner instances. | `bool` | n/a | yes |
+| enable\_ssm\_on\_runners | Enable to allow access to the runner instances for debugging purposes via SSM. Note that this adds additional permissions to the runner instances. | `bool` | n/a | yes |
 | environment | A name that identifies the environment, used as prefix and for tagging. | `string` | n/a | yes |
 | ghes\_url | GitHub Enterprise Server URL. DO NOT SET IF USING PUBLIC GITHUB | `string` | `null` | no |
 | github\_app\_parameters | Parameter Store for GitHub App Parameters. | <pre>object({<br>    key_base64    = map(string)<br>    id            = map(string)<br>    client_id     = map(string)<br>    client_secret = map(string)<br>  })</pre> | n/a | yes |
@@ -114,16 +114,17 @@ No Modules.
 | logging\_retention\_in\_days | Specifies the number of days you want to retain log events for the lambda log group. Possible values are: 0, 1, 3, 5, 7, 14, 30, 60, 90, 120, 150, 180, 365, 400, 545, 731, 1827, and 3653. | `number` | `180` | no |
 | market\_options | Market options for the action runner instances. | `string` | `"spot"` | no |
 | minimum\_running\_time\_in\_minutes | The time an ec2 action runner should be running at minimum before terminated if non busy. | `number` | `5` | no |
-| overrides | This maps provides the possibility to override some defaults. The following attributes are supported: `name_sg` overwrite the `Name` tag for all security groups created by this module. `name_runner_agent_instance` override the `Name` tag for the ec2 instance defined in the auto launch configuration. `name_docker_machine_runners` override the `Name` tag spot instances created by the runner agent. | `map(string)` | <pre>{<br>  "name_runner": "",<br>  "name_sg": ""<br>}</pre> | no |
-| role\_path | The path that will be added to the role, if not set the environment name will be used. | `string` | `null` | no |
+| overrides | This map provides the possibility to override some defaults. The following attributes are supported: `name_sg` overrides the `Name` tag for all security groups created by this module. `name_runner_agent_instance` overrides the `Name` tag for the ec2 instance defined in the auto launch configuration. `name_docker_machine_runners` overrides the `Name` tag spot instances created by the runner agent. | `map(string)` | <pre>{<br>  "name_runner": "",<br>  "name_sg": ""<br>}</pre> | no |
+| role\_path | The path that will be added to the role; if not set, the environment name will be used. | `string` | `null` | no |
 | role\_permissions\_boundary | Permissions boundary that will be added to the created role for the lambda. | `string` | `null` | no |
 | runner\_additional\_security\_group\_ids | (optional) List of additional security groups IDs to apply to the runner | `list(string)` | `[]` | no |
 | runner\_architecture | The platform architecture of the runner instance\_type. | `string` | `"x64"` | no |
 | runner\_as\_root | Run the action runner under the root user. | `bool` | `false` | no |
+| runner\_boot\_time\_in\_minutes | The minimum time for an EC2 runner to boot and register as a runner. | `number` | `5` | no |
 | runner\_extra\_labels | Extra labels for the runners (GitHub). Separate each label by a comma | `string` | `""` | no |
 | runner\_group\_name | Name of the runner group. | `string` | `"Default"` | no |
 | runner\_iam\_role\_managed\_policy\_arns | Attach AWS or customer-managed IAM policies (by ARN) to the runner IAM role | `list(string)` | `[]` | no |
-| runner\_log\_files | (optional) List of logfiles to send to cloudwatch, will only be used if `enable_cloudwatch_agent` is set to true. Object description: `log_group_name`: Name of the log group, `prefix_log_group`: If true, the log group name will be prefixed with `/github-self-hosted-runners/<var.environment>`, `file_path`: path to the log file, `log_stream_name`: name of the log stream. | <pre>list(object({<br>    log_group_name   = string<br>    prefix_log_group = bool<br>    file_path        = string<br>    log_stream_name  = string<br>  }))</pre> | <pre>[<br>  {<br>    "file_path": "/var/log/messages",<br>    "log_group_name": "messages",<br>    "log_stream_name": "{instance_id}",<br>    "prefix_log_group": true<br>  },<br>  {<br>    "file_path": "/var/log/user-data.log",<br>    "log_group_name": "user_data",<br>    "log_stream_name": "{instance_id}",<br>    "prefix_log_group": true<br>  },<br>  {<br>    "file_path": "/home/ec2-user/actions-runner/_diag/Runner_**.log",<br>    "log_group_name": "runner",<br>    "log_stream_name": "{instance_id}",<br>    "prefix_log_group": true<br>  }<br>]</pre> | no |
+| runner\_log\_files | (optional) List of logfiles to send to CloudWatch, will only be used if `enable_cloudwatch_agent` is set to true. Object description: `log_group_name`: Name of the log group, `prefix_log_group`: If true, the log group name will be prefixed with `/github-self-hosted-runners/<var.environment>`, `file_path`: path to the log file, `log_stream_name`: name of the log stream. | <pre>list(object({<br>    log_group_name   = string<br>    prefix_log_group = bool<br>    file_path        = string<br>    log_stream_name  = string<br>  }))</pre> | <pre>[<br>  {<br>    "file_path": "/var/log/messages",<br>    "log_group_name": "messages",<br>    "log_stream_name": "{instance_id}",<br>    "prefix_log_group": true<br>  },<br>  {<br>    "file_path": "/var/log/user-data.log",<br>    "log_group_name": "user_data",<br>    "log_stream_name": "{instance_id}",<br>    "prefix_log_group": true<br>  },<br>  {<br>    "file_path": "/home/ec2-user/actions-runner/_diag/Runner_**.log",<br>    "log_group_name": "runner",<br>    "log_stream_name": "{instance_id}",<br>    "prefix_log_group": true<br>  }<br>]</pre> | no |
 | runners\_lambda\_s3\_key | S3 key for runners lambda function. Required if using S3 bucket to specify lambdas. | `any` | `null` | no |
 | runners\_lambda\_s3\_object\_version | S3 object version for runners lambda function. Useful if S3 versioning is enabled on source bucket. | `any` | `null` | no |
 | runners\_maximum\_count | The maximum number of runners that will be created. | `number` | `3` | no |
@@ -133,8 +134,8 @@ No Modules.
 | sqs\_build\_queue | SQS queue to consume accepted build events. | <pre>object({<br>    arn = string<br>  })</pre> | n/a | yes |
 | subnet\_ids | List of subnets in which the action runners will be launched, the subnets needs to be subnets in the `vpc_id`. | `list(string)` | n/a | yes |
 | tags | Map of tags that will be added to created resources. By default resources will be tagged with name and environment. | `map(string)` | `{}` | no |
-| userdata\_post\_install | User-data script snippet to insert after GitHub acton runner install | `string` | `""` | no |
-| userdata\_pre\_install | User-data script snippet to insert before GitHub acton runner install | `string` | `""` | no |
+| userdata\_post\_install | User-data script snippet to insert after GitHub action runner install | `string` | `""` | no |
+| userdata\_pre\_install | User-data script snippet to insert before GitHub action runner install | `string` | `""` | no |
 | userdata\_template | Alternative user-data template, replacing the default template. By providing your own user\_data you have to take care of installing all required software, including the action runner. Variables userdata\_pre/post\_install are ignored. | `string` | `null` | no |
 | volume\_size | Size of runner volume | `number` | `30` | no |
 | vpc\_id | The VPC for the security groups. | `string` | n/a | yes |

diff --git a/modules/runners/lambdas/runners/src/scale-runners/cache.ts b/modules/runners/lambdas/runners/src/scale-runners/cache.ts
@@ -0,0 +1,10 @@
+import { Octokit } from '@octokit/rest';
+
+export type UnboxPromise<T> = T extends Promise<infer U> ? U : T;
+
+export type GhRunners = UnboxPromise<ReturnType<Octokit['actions']['listSelfHostedRunnersForRepo']>>['data']['runners'];
+
+export class githubCache {
+  static clients: Map<string, Octokit> = new Map();
+  static runners: Map<string, GhRunners> = new Map();
+}
diff --git a/modules/runners/lambdas/runners/src/scale-runners/runners.test.ts b/modules/runners/lambdas/runners/src/scale-runners/runners.test.ts
@@ -1,4 +1,4 @@
-import { listRunners, createRunner, terminateRunner, RunnerInfo } from './runners';
+import { listEC2Runners, createRunner, terminateRunner, RunnerInfo } from './runners';
 
 const mockEC2 = { describeInstances: jest.fn(), runInstances: jest.fn(), terminateInstances: jest.fn() };
 const mockSSM = { putParameter: jest.fn() };
@@ -25,17 +25,17 @@ describe('list instances', () => {
               LaunchTime: new Date('2020-10-10T14:48:00.000+09:00'),
               InstanceId: 'i-1234',
               Tags: [
-                { Key: 'Repo', Value: 'CoderToCat/hello-world' },
-                { Key: 'Org', Value: 'CoderToCat' },
                 { Key: 'Application', Value: 'github-action-runner' },
+                { Key: 'Type', Value: 'Org' },
+                { Key: 'Owner', Value: 'CoderToCat' },
               ],
             },
             {
               LaunchTime: new Date('2020-10-11T14:48:00.000+09:00'),
               InstanceId: 'i-5678',
               Tags: [
-                { Key: 'Repo', Value: REPO_NAME },
-                { Key: 'Org', Value: ORG_NAME },
+                { Key: 'Owner', Value: REPO_NAME },
+                { Key: 'Type', Value: 'Repo' },
                 { Key: 'Application', Value: 'github-action-runner' },
               ],
             },
@@ -47,51 +47,53 @@ describe('list instances', () => {
   });
 
   it('returns a list of instances', async () => {
-    const resp = await listRunners();
+    const resp = await listEC2Runners();
     expect(resp.length).toBe(2);
     expect(resp).toContainEqual({
       instanceId: 'i-1234',
       launchTime: new Date('2020-10-10T14:48:00.000+09:00'),
-      repo: 'CoderToCat/hello-world',
-      org: 'CoderToCat',
+      type: 'Org',
+      owner: 'CoderToCat',
     });
     expect(resp).toContainEqual({
       instanceId: 'i-5678',
       launchTime: new Date('2020-10-11T14:48:00.000+09:00'),
-      repo: REPO_NAME,
-      org: ORG_NAME,
+      type: 'Repo',
+      owner: REPO_NAME,
     });
   });
 
   it('calls EC2 describe instances', async () => {
-    await listRunners();
+    await listEC2Runners();
     expect(mockEC2.describeInstances).toBeCalled();
   });
 
   it('filters instances on repo name', async () => {
-    await listRunners({ runnerType: 'Repo', runnerOwner: REPO_NAME, environment: undefined });
+    await listEC2Runners({ runnerType: 'Repo', runnerOwner: REPO_NAME, environment: undefined });
     expect(mockEC2.describeInstances).toBeCalledWith({
       Filters: [
         { Name: 'tag:Application', Values: ['github-action-runner'] },
         { Name: 'instance-state-name', Values: ['running', 'pending'] },
-        { Name: 'tag:Repo', Values: [REPO_NAME] },
+        { Name: 'tag:Type', Values: ['Repo'] },
+        { Name: 'tag:Owner', Values: [REPO_NAME] },
       ],
     });
   });
 
   it('filters instances on org name', async () => {
-    await listRunners({ runnerType: 'Org', runnerOwner: ORG_NAME, environment: undefined });
+    await listEC2Runners({ runnerType: 'Org', runnerOwner: ORG_NAME, environment: undefined });
     expect(mockEC2.describeInstances).toBeCalledWith({
       Filters: [
         { Name: 'tag:Application', Values: ['github-action-runner'] },
         { Name: 'instance-state-name', Values: ['running', 'pending'] },
-        { Name: 'tag:Org', Values: [ORG_NAME] },
+        { Name: 'tag:Type', Values: ['Org'] },
+        { Name: 'tag:Owner', Values: [ORG_NAME] },
       ],
     });
   });
 
-  it('filters instances on org name', async () => {
-    await listRunners({ environment: ENVIRONMENT });
+  it('filters instances on environment', async () => {
+    await listEC2Runners({ environment: ENVIRONMENT });
     expect(mockEC2.describeInstances).toBeCalledWith({
       Filters: [
         { Name: 'tag:Application', Values: ['github-action-runner'] },
@@ -112,8 +114,10 @@ describe('terminate runner', () => {
   it('calls terminate instances with the right instance ids', async () => {
     const runner: RunnerInfo = {
       instanceId: 'instance-2',
+      owner: 'owner-2',
+      type: 'Repo',
     };
-    await terminateRunner(runner);
+    await terminateRunner(runner.instanceId);
 
     expect(mockEC2.terminateInstances).toBeCalledWith({ InstanceIds: [runner.instanceId] });
   });
@@ -156,7 +160,8 @@ describe('create runner', () => {
           ResourceType: 'instance',
           Tags: [
             { Key: 'Application', Value: 'github-action-runner' },
-            { Key: 'Repo', Value: REPO_NAME },
+            { Key: 'Type', Value: 'Repo' },
+            { Key: 'Owner', Value: REPO_NAME },
           ],
         },
       ],
@@ -183,7 +188,8 @@ describe('create runner', () => {
           ResourceType: 'instance',
           Tags: [
             { Key: 'Application', Value: 'github-action-runner' },
-            { Key: 'Org', Value: ORG_NAME },
+            { Key: 'Type', Value: 'Org' },
+            { Key: 'Owner', Value: ORG_NAME },
           ],
         },
       ],

diff --git a/modules/runners/lambdas/runners/src/scale-runners/runners.ts b/modules/runners/lambdas/runners/src/scale-runners/runners.ts
@@ -1,12 +1,21 @@
 import { EC2, SSM } from 'aws-sdk';
 
-export interface RunnerInfo {
+export interface RunnerList {
   instanceId: string;
   launchTime?: Date;
+  owner?: string;
+  type?: string;
   repo?: string;
   org?: string;
 }
 
+export interface RunnerInfo {
+  instanceId: string;
+  launchTime?: Date;
+  owner: string;
+  type: string;
+}
+
 export interface ListRunnerFilters {
   runnerType?: 'Org' | 'Repo';
   runnerOwner?: string;
@@ -20,7 +29,7 @@ export interface RunnerInputParameters {
   runnerOwner: string;
 }
 
-export async function listRunners(filters: ListRunnerFilters | undefined = undefined): Promise<RunnerInfo[]> {
+export async function listEC2Runners(filters: ListRunnerFilters | undefined = undefined): Promise<RunnerList[]> {
   const ec2 = new EC2();
   const ec2Filters = [
     { Name: 'tag:Application', Values: ['github-action-runner'] },
@@ -31,20 +40,23 @@ export async function listRunners(filters: ListRunnerFilters | undefined = undef
       ec2Filters.push({ Name: 'tag:Environment', Values: [filters.environment] });
     }
     if (filters.runnerType && filters.runnerOwner) {
-      ec2Filters.push({ Name: `tag:${filters.runnerType}`, Values: [filters.runnerOwner] });
+      ec2Filters.push({ Name: `tag:Type`, Values: [filters.runnerType] });
+      ec2Filters.push({ Name: `tag:Owner`, Values: [filters.runnerOwner] });
     }
   }
   const runningInstances = await ec2.describeInstances({ Filters: ec2Filters }).promise();
-  const runners: RunnerInfo[] = [];
+  const runners: RunnerList[] = [];
   if (runningInstances.Reservations) {
     for (const r of runningInstances.Reservations) {
       if (r.Instances) {
         for (const i of r.Instances) {
           runners.push({
             instanceId: i.InstanceId as string,
             launchTime: i.LaunchTime,
-            repo: i.Tags?.find((e) => e.Key === 'Repo')?.Value,
-            org: i.Tags?.find((e) => e.Key === 'Org')?.Value,
+            owner: i.Tags?.find((e) => e.Key === 'Owner')?.Value as string,
+            type: i.Tags?.find((e) => e.Key === 'Type')?.Value as string,
+            repo: i.Tags?.find((e) => e.Key === 'Repo')?.Value as string,
+            org: i.Tags?.find((e) => e.Key === 'Org')?.Value as string,
           });
         }
       }
@@ -53,14 +65,14 @@ export async function listRunners(filters: ListRunnerFilters | undefined = undef
   return runners;
 }
 
-export async function terminateRunner(runner: RunnerInfo): Promise<void> {
+export async function terminateRunner(instanceId: string): Promise<void> {
   const ec2 = new EC2();
   await ec2
     .terminateInstances({
-      InstanceIds: [runner.instanceId],
+      InstanceIds: [instanceId],
     })
     .promise();
-  console.debug('Runner terminated.' + runner.instanceId);
+  console.debug(`Runner ${instanceId} has been terminated.`);
 }
 
 export async function createRunner(runnerParameters: RunnerInputParameters, launchTemplateName: string): Promise<void> {
@@ -99,10 +111,8 @@ function getInstanceParams(
         ResourceType: 'instance',
         Tags: [
           { Key: 'Application', Value: 'github-action-runner' },
-          {
-            Key: runnerParameters.runnerType,
-            Value: runnerParameters.runnerOwner,
-          },
+          { Key: 'Type', Value: runnerParameters.runnerType },
+          { Key: 'Owner', Value: runnerParameters.runnerOwner },
         ],
       },
     ],