From 10f89efc2051c7a6a943e416aac12a1a0d291901 Mon Sep 17 00:00:00 2001 From: Niek Palm Date: Wed, 21 Dec 2022 20:58:14 +0100 Subject: [PATCH] fix(runners): Remove Application legacy tag (#2705) * Support legacy `Application` tag key * Reverting test * fix(runners): Remove legacy Application tag. * fix formatter error Co-authored-by: Nathaniel McAuliffe --- .../lambdas/runners/src/aws/runners.test.ts | 57 +++---------------- .../lambdas/runners/src/aws/runners.ts | 5 +- 2 files changed, 9 insertions(+), 53 deletions(-) diff --git a/modules/runners/lambdas/runners/src/aws/runners.test.ts b/modules/runners/lambdas/runners/src/aws/runners.test.ts index 8ff566296f..966cc98d7d 100644 --- a/modules/runners/lambdas/runners/src/aws/runners.test.ts +++ b/modules/runners/lambdas/runners/src/aws/runners.test.ts @@ -35,23 +35,6 @@ const mockRunningInstances: AWS.EC2.DescribeInstancesResult = { }, ], }; -const mockRunningInstancesLegacy: AWS.EC2.DescribeInstancesResult = { - Reservations: [ - { - Instances: [ - { - LaunchTime: new Date('2020-10-11T14:48:00.000+09:00'), - InstanceId: 'i-5678', - Tags: [ - { Key: 'Owner', Value: REPO_NAME }, - { Key: 'Type', Value: 'Repo' }, - { Key: 'Application', Value: 'github-action-runner' }, - ], - }, - ], - }, - ], -}; describe('list instances', () => { beforeEach(() => { @@ -60,37 +43,25 @@ describe('list instances', () => { }); it('returns a list of instances', async () => { - mockDescribeInstances.promise - .mockReturnValueOnce(mockRunningInstances) - .mockReturnValueOnce(mockRunningInstancesLegacy); + mockDescribeInstances.promise.mockReturnValue(mockRunningInstances); const resp = await listEC2Runners(); - expect(resp.length).toBe(2); + expect(resp.length).toBe(1); expect(resp).toContainEqual({ instanceId: 'i-1234', launchTime: new Date('2020-10-10T14:48:00.000+09:00'), type: 'Org', owner: 'CoderToCat', }); - expect(resp).toContainEqual({ - instanceId: 'i-5678', - launchTime: new Date('2020-10-11T14:48:00.000+09:00'), - type: 'Repo', - owner: REPO_NAME, - }); }); it('calls EC2 describe instances', async () => { - mockDescribeInstances.promise - .mockReturnValueOnce(mockRunningInstances) - .mockReturnValueOnce(mockRunningInstancesLegacy); + mockDescribeInstances.promise.mockReturnValueOnce(mockRunningInstances); await listEC2Runners(); expect(mockEC2.describeInstances).toBeCalled(); }); it('filters instances on repo name', async () => { - mockDescribeInstances.promise - .mockReturnValueOnce(mockRunningInstances) - .mockReturnValueOnce(mockRunningInstancesLegacy); + mockDescribeInstances.promise.mockReturnValueOnce(mockRunningInstances); await listEC2Runners({ runnerType: 'Repo', runnerOwner: REPO_NAME, environment: undefined }); expect(mockEC2.describeInstances).toBeCalledWith({ Filters: [ @@ -100,20 +71,10 @@ describe('list instances', () => { { Name: 'tag:ghr:Application', Values: ['github-action-runner'] }, ], }); - expect(mockEC2.describeInstances).toBeCalledWith({ - Filters: [ - { Name: 'instance-state-name', Values: ['running', 'pending'] }, - { Name: 'tag:Type', Values: ['Repo'] }, - { Name: 'tag:Owner', Values: [REPO_NAME] }, - { Name: 'tag:Application', Values: ['github-action-runner'] }, - ], - }); }); it('filters instances on org name', async () => { - mockDescribeInstances.promise - .mockReturnValueOnce(mockRunningInstances) - .mockReturnValueOnce(mockRunningInstancesLegacy); + mockDescribeInstances.promise.mockReturnValueOnce(mockRunningInstances); await listEC2Runners({ runnerType: 'Org', runnerOwner: ORG_NAME, environment: undefined }); expect(mockEC2.describeInstances).toBeCalledWith({ Filters: [ @@ -126,9 +87,7 @@ describe('list instances', () => { }); it('filters instances on environment', async () => { - mockDescribeInstances.promise - .mockReturnValueOnce(mockRunningInstances) - .mockReturnValueOnce(mockRunningInstancesLegacy); + mockDescribeInstances.promise.mockReturnValueOnce(mockRunningInstances); await listEC2Runners({ environment: ENVIRONMENT }); expect(mockEC2.describeInstances).toBeCalledWith({ Filters: [ @@ -156,7 +115,7 @@ describe('list instances', () => { }, ], }; - mockDescribeInstances.promise.mockReturnValueOnce(noInstances).mockReturnValueOnce(noInstances); + mockDescribeInstances.promise.mockReturnValueOnce(noInstances); const resp = await listEC2Runners(); expect(resp.length).toBe(0); }); @@ -175,7 +134,7 @@ describe('list instances', () => { }, ], }; - mockDescribeInstances.promise.mockReturnValueOnce(noInstances).mockReturnValue({}); + mockDescribeInstances.promise.mockReturnValueOnce(noInstances); const resp = await listEC2Runners(); expect(resp.length).toBe(1); }); diff --git a/modules/runners/lambdas/runners/src/aws/runners.ts b/modules/runners/lambdas/runners/src/aws/runners.ts index fb4154d073..395bdb9240 100644 --- a/modules/runners/lambdas/runners/src/aws/runners.ts +++ b/modules/runners/lambdas/runners/src/aws/runners.ts @@ -74,10 +74,7 @@ function constructFilters(filters?: ListRunnerFilters): Ec2Filter[][] { } } - // ***Deprecation Notice*** - // Support for legacy `Application` tag keys - // will be removed in next major release. - for (const key of ['tag:ghr:Application', 'tag:Application']) { + for (const key of ['tag:ghr:Application']) { const filter = [...ec2FiltersBase]; filter.push({ Name: key, Values: ['github-action-runner'] }); ec2Filters.push(filter);