Skip to content

Commit c7494c1

Browse files
jensenboxclaudeSRE EngineerPaperclip-PaperclipBrend-Smits
authored
feat(termination-watcher): deregister runners from GitHub on EC2 termination (#5055)
## Summary Extends the existing termination-watcher Lambda to deregister GitHub Actions runners from GitHub when their EC2 instances terminate. This prevents stale "offline" runner entries from accumulating in the organization/repository — a long-standing issue (#804, #1006, #2939) affecting all users of the module. ### How it works 1. When an EC2 instance terminates, the Lambda reads the `ghr:Owner` and `ghr:Type` tags from the instance 2. Authenticates to GitHub using the module's existing App credentials (SSM parameters) 3. Finds the runner by instance ID in the runner name, then calls the delete API 4. Errors are logged but never fail the Lambda — metrics collection continues unaffected ### What's included **Lambda changes:** - `deregister.ts` — GitHub API deregistration logic reusing the module's existing auth pattern (`createAppAuth` → installation token) - Wired into both `termination.ts` (BidEvictedEvent) and `termination-warning.ts` (Spot Interruption Warning) - `ConfigResolver.ts` — adds `enableRunnerDeregistration` and `ghesApiUrl` config from env vars - 295-line test suite covering org/repo runners, not-found cases, disabled feature, and error handling **Terraform changes:** - Passes GitHub App SSM parameter ARNs through the module chain to the termination-watcher - Adds SSM `GetParameter` IAM policy when deregistration is enabled - Adds `PARAMETER_GITHUB_APP_ID_NAME`, `PARAMETER_GITHUB_APP_KEY_BASE64_NAME`, `ENABLE_RUNNER_DEREGISTRATION`, and `GHES_URL` environment variables to both Lambda functions - Adds an `EC2 Instance State-change Notification` EventBridge rule (state: `shutting-down`) that catches **all** termination types — not just spot-specific events. This covers scale-down, manual termination, ASG termination, and spot reclamation. **New variables on `instance_termination_watcher`:** - `enable_runner_deregistration` (bool, default `false`) ### Design decisions - **Opt-in**: Disabled by default to avoid breaking existing deployments. Enable with `enable_runner_deregistration = true`. - **Reuses existing auth pattern**: Same `@octokit/auth-app` + SSM approach used by the control-plane Lambda. - **Reuses existing Lambda**: The state-change EventBridge rule targets the same notification Lambda rather than creating a new one, since both event types provide `detail['instance-id']`. - **Graceful failure**: All deregistration errors are caught and logged. If the runner is already removed, it logs and returns. The Lambda never fails due to deregistration issues. - **Supports Org and Repo runners**: Reads `ghr:Type` tag to determine the correct API endpoint. - **GHES compatible**: Passes through the `ghes_url` variable for GitHub Enterprise Server deployments. ### Testing - 44 unit tests pass (7 test files), including the new `deregister.test.ts` - Tested in production: manually terminated a runner instance → Lambda triggered within seconds → runner successfully deregistered from GitHub org Fixes #804 --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: SRE Engineer <sre-engineer@paperclip.ing> Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Brend Smits <brend.smits@philips.com>
1 parent 79b73a8 commit c7494c1

28 files changed

Lines changed: 1101 additions & 72 deletions

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ Join our discord community via [this invite link](https://discord.gg/bxgXW8jJGh)
146146
| <a name="input_instance_max_spot_price"></a> [instance\_max\_spot\_price](#input\_instance\_max\_spot\_price) | Max price price for spot instances per hour. This variable will be passed to the create fleet as max spot price for the fleet. | `string` | `null` | no |
147147
| <a name="input_instance_profile_path"></a> [instance\_profile\_path](#input\_instance\_profile\_path) | The path that will be added to the instance\_profile, if not set the environment name will be used. | `string` | `null` | no |
148148
| <a name="input_instance_target_capacity_type"></a> [instance\_target\_capacity\_type](#input\_instance\_target\_capacity\_type) | Default lifecycle used for runner instances, can be either `spot` or `on-demand`. | `string` | `"spot"` | no |
149-
| <a name="input_instance_termination_watcher"></a> [instance\_termination\_watcher](#input\_instance\_termination\_watcher) | Configuration for the instance termination watcher. This feature is Beta, changes will not trigger a major release as long in beta.<br/><br/>`enable`: Enable or disable the spot termination watcher.<br/>'features': Enable or disable features of the termination watcher.<br/>`memory_size`: Memory size limit in MB of the lambda.<br/>`s3_key`: S3 key for syncer lambda function. Required if using S3 bucket to specify lambdas.<br/>`s3_object_version`: S3 object version for syncer lambda function. Useful if S3 versioning is enabled on source bucket.<br/>`timeout`: Time out of the lambda in seconds.<br/>`zip`: File location of the lambda zip file. | <pre>object({<br/> enable = optional(bool, false)<br/> features = optional(object({<br/> enable_spot_termination_handler = optional(bool, true)<br/> enable_spot_termination_notification_watcher = optional(bool, true)<br/> }), {})<br/> memory_size = optional(number, null)<br/> s3_key = optional(string, null)<br/> s3_object_version = optional(string, null)<br/> timeout = optional(number, null)<br/> zip = optional(string, null)<br/> })</pre> | `{}` | no |
149+
| <a name="input_instance_termination_watcher"></a> [instance\_termination\_watcher](#input\_instance\_termination\_watcher) | Configuration for the instance termination watcher. This feature is Beta, changes will not trigger a major release as long in beta.<br/><br/>`enable`: Enable or disable the spot termination watcher.<br/>'features': Enable or disable features of the termination watcher.<br/>`enable_runner_deregistration`: Enable or disable deregistering the runner from GitHub when its EC2 instance is terminated.<br/>`memory_size`: Memory size limit in MB of the lambda.<br/>`s3_key`: S3 key for syncer lambda function. Required if using S3 bucket to specify lambdas.<br/>`s3_object_version`: S3 object version for syncer lambda function. Useful if S3 versioning is enabled on source bucket.<br/>`timeout`: Time out of the lambda in seconds.<br/>`zip`: File location of the lambda zip file. | <pre>object({<br/> enable = optional(bool, false)<br/> features = optional(object({<br/> enable_spot_termination_handler = optional(bool, true)<br/> enable_spot_termination_notification_watcher = optional(bool, true)<br/> }), {})<br/> enable_runner_deregistration = optional(bool, true)<br/> memory_size = optional(number, null)<br/> s3_key = optional(string, null)<br/> s3_object_version = optional(string, null)<br/> timeout = optional(number, null)<br/> zip = optional(string, null)<br/> })</pre> | `{}` | no |
150150
| <a name="input_instance_types"></a> [instance\_types](#input\_instance\_types) | List of instance types for the action runner. Defaults are based on runner\_os (al2023 for linux, macOS Sequoia for osx, Windows Server Core for win). | `list(string)` | <pre>[<br/> "m5.large",<br/> "c5.large"<br/>]</pre> | no |
151151
| <a name="input_job_queue_retention_in_seconds"></a> [job\_queue\_retention\_in\_seconds](#input\_job\_queue\_retention\_in\_seconds) | The number of seconds the job is held in the queue before it is purged. | `number` | `86400` | no |
152152
| <a name="input_job_retry"></a> [job\_retry](#input\_job\_retry) | Experimental! Can be removed / changed without trigger a major release.Configure job retries. The configuration enables job retries (for ephemeral runners). After creating the instances a message will be published to a job retry queue. The job retry check lambda is checking after a delay if the job is queued. If not the message will be published again on the scale-up (build queue). Using this feature can impact the rate limit of the GitHub app.<br/><br/>`enable`: Enable or disable the job retry feature.<br/>`delay_in_seconds`: The delay in seconds before the job retry check lambda will check the job status.<br/>`delay_backoff`: The backoff factor for the delay.<br/>`lambda_memory_size`: Memory size limit in MB for the job retry check lambda.<br/>`lambda_timeout`: Time out of the job retry check lambda in seconds.<br/>`max_attempts`: The maximum number of attempts to retry the job. | <pre>object({<br/> enable = optional(bool, false)<br/> delay_in_seconds = optional(number, 300)<br/> delay_backoff = optional(number, 2)<br/> lambda_memory_size = optional(number, 256)<br/> lambda_timeout = optional(number, 30)<br/> max_attempts = optional(number, 1)<br/> })</pre> | `{}` | no |

lambdas/functions/termination-watcher/package.json

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,15 @@
2424
},
2525
"dependencies": {
2626
"@aws-github-runner/aws-powertools-util": "*",
27+
"@aws-github-runner/aws-ssm-util": "*",
2728
"@aws-sdk/client-ec2": "^3.1009.0",
28-
"@middy/core": "^6.4.5"
29+
"@aws-sdk/client-sqs": "^3.1009.0",
30+
"@middy/core": "^6.4.5",
31+
"@octokit/auth-app": "8.2.0",
32+
"@octokit/core": "7.0.6",
33+
"@octokit/plugin-throttling": "11.0.3",
34+
"@octokit/request": "^9.2.2",
35+
"@octokit/rest": "22.0.1"
2936
},
3037
"nx": {
3138
"includedScripts": [

lambdas/functions/termination-watcher/src/ConfigResolver.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ describe('Test ConfigResolver', () => {
3737
delete process.env.ENABLE_METRICS_SPOT_WARNING;
3838
delete process.env.PREFIX;
3939
delete process.env.TAG_FILTERS;
40+
delete process.env.ENABLE_RUNNER_DEREGISTRATION;
41+
delete process.env.GHES_URL;
4042
});
4143

4244
it(description, async () => {
@@ -55,4 +57,29 @@ describe('Test ConfigResolver', () => {
5557
expect(config.tagFilters).toEqual(output.tagFilters);
5658
});
5759
});
60+
61+
describe('runner deregistration config', () => {
62+
beforeEach(() => {
63+
delete process.env.ENABLE_RUNNER_DEREGISTRATION;
64+
delete process.env.GHES_URL;
65+
});
66+
67+
it('should default to disabled', () => {
68+
const config = new Config();
69+
expect(config.enableRunnerDeregistration).toBe(false);
70+
expect(config.ghesApiUrl).toBe('');
71+
});
72+
73+
it('should enable deregistration when env var is true', () => {
74+
process.env.ENABLE_RUNNER_DEREGISTRATION = 'true';
75+
const config = new Config();
76+
expect(config.enableRunnerDeregistration).toBe(true);
77+
});
78+
79+
it('should set GHES URL when provided', () => {
80+
process.env.GHES_URL = 'https://github.internal.co/api/v3';
81+
const config = new Config();
82+
expect(config.ghesApiUrl).toBe('https://github.internal.co/api/v3');
83+
});
84+
});
5885
});

lambdas/functions/termination-watcher/src/ConfigResolver.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ export class Config {
55
createSpotTerminationMetric: boolean;
66
tagFilters: Record<string, string>;
77
prefix: string;
8+
enableRunnerDeregistration: boolean;
9+
ghesApiUrl: string;
810

911
constructor() {
1012
const logger = createChildLogger('config-resolver');
@@ -14,6 +16,8 @@ export class Config {
1416
this.createSpotWarningMetric = process.env.ENABLE_METRICS_SPOT_WARNING === 'true';
1517
this.createSpotTerminationMetric = process.env.ENABLE_METRICS_SPOT_TERMINATION === 'true';
1618
this.prefix = process.env.PREFIX ?? '';
19+
this.enableRunnerDeregistration = process.env.ENABLE_RUNNER_DEREGISTRATION === 'true';
20+
this.ghesApiUrl = process.env.GHES_URL ?? '';
1721
this.tagFilters = { 'ghr:environment': this.prefix };
1822

1923
const rawTagFilters = process.env.TAG_FILTERS;
Lines changed: 295 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,295 @@
1+
import { Instance } from '@aws-sdk/client-ec2';
2+
import { describe, it, expect, beforeEach, vi } from 'vitest';
3+
import { deregisterRunner, createThrottleOptions } from './deregister';
4+
import { Config } from './ConfigResolver';
5+
import type { EndpointDefaults } from '@octokit/types';
6+
7+
const mockGetParameter = vi.fn();
8+
vi.mock('@aws-github-runner/aws-ssm-util', () => ({
9+
getParameter: (...args: unknown[]) => mockGetParameter(...args),
10+
}));
11+
12+
const mockCreateAppAuth = vi.fn();
13+
vi.mock('@octokit/auth-app', () => ({
14+
createAppAuth: (...args: unknown[]) => mockCreateAppAuth(...args),
15+
}));
16+
17+
const mockPaginate = {
18+
iterator: vi.fn(),
19+
};
20+
21+
const mockActions = {
22+
listSelfHostedRunnersForOrg: vi.fn(),
23+
listSelfHostedRunnersForRepo: vi.fn(),
24+
deleteSelfHostedRunnerFromOrg: vi.fn(),
25+
deleteSelfHostedRunnerFromRepo: vi.fn(),
26+
};
27+
28+
const mockApps = {
29+
getOrgInstallation: vi.fn(),
30+
getRepoInstallation: vi.fn(),
31+
};
32+
33+
function MockOctokit() {
34+
return {
35+
actions: mockActions,
36+
apps: mockApps,
37+
paginate: mockPaginate,
38+
};
39+
}
40+
MockOctokit.plugin = vi.fn().mockReturnValue(MockOctokit);
41+
42+
vi.mock('@octokit/rest', () => ({
43+
Octokit: MockOctokit,
44+
}));
45+
46+
vi.mock('@octokit/plugin-throttling', () => ({
47+
throttling: vi.fn(),
48+
}));
49+
50+
vi.mock('@octokit/request', () => ({
51+
request: {
52+
defaults: vi.fn().mockReturnValue(vi.fn()),
53+
},
54+
}));
55+
56+
const baseConfig: Config = {
57+
createSpotWarningMetric: false,
58+
createSpotTerminationMetric: true,
59+
tagFilters: { 'ghr:environment': 'test' },
60+
prefix: 'runners',
61+
enableRunnerDeregistration: true,
62+
ghesApiUrl: '',
63+
};
64+
65+
const orgInstance: Instance = {
66+
InstanceId: 'i-12345678901234567',
67+
InstanceType: 't2.micro',
68+
Tags: [
69+
{ Key: 'Name', Value: 'test-instance' },
70+
{ Key: 'ghr:environment', Value: 'test' },
71+
{ Key: 'ghr:Owner', Value: 'test-org' },
72+
{ Key: 'ghr:Type', Value: 'Org' },
73+
],
74+
State: { Name: 'running' },
75+
LaunchTime: new Date('2021-01-01'),
76+
};
77+
78+
const repoInstance: Instance = {
79+
InstanceId: 'i-repo12345678901234',
80+
InstanceType: 't2.micro',
81+
Tags: [
82+
{ Key: 'Name', Value: 'test-repo-instance' },
83+
{ Key: 'ghr:environment', Value: 'test' },
84+
{ Key: 'ghr:Owner', Value: 'test-org/test-repo' },
85+
{ Key: 'ghr:Type', Value: 'Repo' },
86+
],
87+
State: { Name: 'running' },
88+
LaunchTime: new Date('2021-01-01'),
89+
};
90+
91+
function setupAuthMocks() {
92+
const appPrivateKey = Buffer.from('fake-private-key').toString('base64');
93+
mockGetParameter.mockImplementation((name: string) => {
94+
if (name === 'github-app-id') return Promise.resolve('12345');
95+
if (name === 'github-app-key') return Promise.resolve(appPrivateKey);
96+
return Promise.reject(new Error(`Unknown parameter: ${name}`));
97+
});
98+
99+
// App auth returns app token
100+
const mockAuth = vi.fn();
101+
mockAuth.mockImplementation((opts: { type: string }) => {
102+
if (opts.type === 'app') {
103+
return Promise.resolve({ token: 'app-token' });
104+
}
105+
return Promise.resolve({ token: 'installation-token' });
106+
});
107+
mockCreateAppAuth.mockReturnValue(mockAuth);
108+
}
109+
110+
describe('deregisterRunner', () => {
111+
beforeEach(() => {
112+
vi.clearAllMocks();
113+
process.env.PARAMETER_GITHUB_APP_ID_NAME = 'github-app-id';
114+
process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME = 'github-app-key';
115+
setupAuthMocks();
116+
});
117+
118+
it('should skip deregistration when disabled', async () => {
119+
await deregisterRunner(orgInstance, { ...baseConfig, enableRunnerDeregistration: false });
120+
expect(mockGetParameter).not.toHaveBeenCalled();
121+
});
122+
123+
it('should skip deregistration when instance ID is missing', async () => {
124+
const instance: Instance = { ...orgInstance, InstanceId: undefined };
125+
await deregisterRunner(instance, baseConfig);
126+
expect(mockGetParameter).not.toHaveBeenCalled();
127+
});
128+
129+
it('should skip deregistration when ghr:Owner tag is missing', async () => {
130+
const instance: Instance = {
131+
...orgInstance,
132+
Tags: [{ Key: 'Name', Value: 'test' }],
133+
};
134+
await deregisterRunner(instance, baseConfig);
135+
// Auth should not be called since we bail early
136+
expect(mockCreateAppAuth).not.toHaveBeenCalled();
137+
});
138+
139+
it('should deregister an org runner successfully', async () => {
140+
mockApps.getOrgInstallation.mockResolvedValue({ data: { id: 999 } });
141+
142+
async function* fakeIterator() {
143+
yield { data: [{ id: 42, name: `runner-i-12345678901234567` }] };
144+
}
145+
mockPaginate.iterator.mockReturnValue(fakeIterator());
146+
147+
mockActions.deleteSelfHostedRunnerFromOrg.mockResolvedValue({});
148+
149+
await deregisterRunner(orgInstance, baseConfig);
150+
151+
expect(mockApps.getOrgInstallation).toHaveBeenCalledWith({ org: 'test-org' });
152+
expect(mockActions.deleteSelfHostedRunnerFromOrg).toHaveBeenCalledWith({
153+
org: 'test-org',
154+
runner_id: 42,
155+
});
156+
});
157+
158+
it('should deregister a repo runner successfully', async () => {
159+
mockApps.getRepoInstallation.mockResolvedValue({ data: { id: 888 } });
160+
161+
async function* fakeIterator() {
162+
yield { data: [{ id: 55, name: `runner-i-repo12345678901234` }] };
163+
}
164+
mockPaginate.iterator.mockReturnValue(fakeIterator());
165+
166+
mockActions.deleteSelfHostedRunnerFromRepo.mockResolvedValue({});
167+
168+
await deregisterRunner(repoInstance, baseConfig);
169+
170+
expect(mockApps.getRepoInstallation).toHaveBeenCalledWith({ owner: 'test-org', repo: 'test-repo' });
171+
expect(mockActions.deleteSelfHostedRunnerFromRepo).toHaveBeenCalledWith({
172+
owner: 'test-org',
173+
repo: 'test-repo',
174+
runner_id: 55,
175+
});
176+
});
177+
178+
it('should handle runner not found gracefully', async () => {
179+
mockApps.getOrgInstallation.mockResolvedValue({ data: { id: 999 } });
180+
181+
async function* fakeIterator() {
182+
yield { data: [{ id: 42, name: 'runner-other-instance' }] };
183+
}
184+
mockPaginate.iterator.mockReturnValue(fakeIterator());
185+
186+
await deregisterRunner(orgInstance, baseConfig);
187+
188+
expect(mockActions.deleteSelfHostedRunnerFromOrg).not.toHaveBeenCalled();
189+
});
190+
191+
it('should handle GitHub API errors gracefully', async () => {
192+
mockApps.getOrgInstallation.mockRejectedValue(new Error('GitHub API error'));
193+
194+
await deregisterRunner(orgInstance, baseConfig);
195+
196+
// Should not throw — error is caught internally
197+
expect(mockActions.deleteSelfHostedRunnerFromOrg).not.toHaveBeenCalled();
198+
});
199+
200+
it('should default to Org runner type when ghr:Type tag is missing', async () => {
201+
const instance: Instance = {
202+
...orgInstance,
203+
Tags: [
204+
{ Key: 'ghr:environment', Value: 'test' },
205+
{ Key: 'ghr:Owner', Value: 'test-org' },
206+
],
207+
};
208+
209+
mockApps.getOrgInstallation.mockResolvedValue({ data: { id: 999 } });
210+
211+
async function* fakeIterator() {
212+
yield { data: [{ id: 42, name: `runner-i-12345678901234567` }] };
213+
}
214+
mockPaginate.iterator.mockReturnValue(fakeIterator());
215+
216+
mockActions.deleteSelfHostedRunnerFromOrg.mockResolvedValue({});
217+
218+
await deregisterRunner(instance, baseConfig);
219+
220+
expect(mockApps.getOrgInstallation).toHaveBeenCalledWith({ org: 'test-org' });
221+
expect(mockActions.deleteSelfHostedRunnerFromOrg).toHaveBeenCalledWith({
222+
org: 'test-org',
223+
runner_id: 42,
224+
});
225+
});
226+
227+
it('should use GHES API URL when configured', async () => {
228+
const ghesConfig = { ...baseConfig, ghesApiUrl: 'https://github.internal.co/api/v3' };
229+
230+
mockApps.getOrgInstallation.mockResolvedValue({ data: { id: 999 } });
231+
232+
async function* fakeIterator() {
233+
yield { data: [{ id: 42, name: `runner-i-12345678901234567` }] };
234+
}
235+
mockPaginate.iterator.mockReturnValue(fakeIterator());
236+
237+
mockActions.deleteSelfHostedRunnerFromOrg.mockResolvedValue({});
238+
239+
await deregisterRunner(orgInstance, ghesConfig);
240+
241+
expect(mockActions.deleteSelfHostedRunnerFromOrg).toHaveBeenCalled();
242+
});
243+
244+
it('should paginate through multiple pages to find runner', async () => {
245+
mockApps.getOrgInstallation.mockResolvedValue({ data: { id: 999 } });
246+
247+
async function* fakeIterator() {
248+
yield { data: [{ id: 1, name: 'runner-other-1' }] };
249+
yield { data: [{ id: 2, name: 'runner-other-2' }] };
250+
yield { data: [{ id: 42, name: `runner-i-12345678901234567` }] };
251+
}
252+
mockPaginate.iterator.mockReturnValue(fakeIterator());
253+
254+
mockActions.deleteSelfHostedRunnerFromOrg.mockResolvedValue({});
255+
256+
await deregisterRunner(orgInstance, baseConfig);
257+
258+
expect(mockActions.deleteSelfHostedRunnerFromOrg).toHaveBeenCalledWith({
259+
org: 'test-org',
260+
runner_id: 42,
261+
});
262+
});
263+
264+
it('should handle repo runner not found gracefully', async () => {
265+
mockApps.getRepoInstallation.mockResolvedValue({ data: { id: 888 } });
266+
267+
async function* fakeIterator() {
268+
yield { data: [{ id: 99, name: 'runner-other-instance' }] };
269+
}
270+
mockPaginate.iterator.mockReturnValue(fakeIterator());
271+
272+
await deregisterRunner(repoInstance, baseConfig);
273+
274+
expect(mockActions.deleteSelfHostedRunnerFromRepo).not.toHaveBeenCalled();
275+
});
276+
277+
it('should handle instance with no tags', async () => {
278+
const instance: Instance = {
279+
InstanceId: 'i-12345678901234567',
280+
Tags: undefined,
281+
};
282+
await deregisterRunner(instance, baseConfig);
283+
expect(mockCreateAppAuth).not.toHaveBeenCalled();
284+
});
285+
});
286+
287+
describe('createThrottleOptions', () => {
288+
it('should return false for rate limit and log warning', () => {
289+
const options = createThrottleOptions();
290+
const endpointDefaults = { method: 'GET', url: '/test' } as Required<EndpointDefaults>;
291+
292+
expect(options.onRateLimit(60, endpointDefaults)).toBe(false);
293+
expect(options.onSecondaryRateLimit(60, endpointDefaults)).toBe(false);
294+
});
295+
});

0 commit comments

Comments
 (0)