feat: warm pool — stop/start idle runners for fast restart - #5204
Draft
Brend-Smits wants to merge 20 commits into
Draft
feat: warm pool — stop/start idle runners for fast restart#5204Brend-Smits wants to merge 20 commits into
Brend-Smits wants to merge 20 commits into
Conversation
- ADR documenting the warm pool concept with three instance states (hot/warm/cold) - Implementation plan with phased approach - User-facing warm pool configuration guide Signed-off-by: Brend Smits <brend.smits@philips.com>
…, IAM - Add warm_pool_config and pool_strategy variables to root, runners, and multi-runner modules - Create DynamoDB table with GSI by-owner and TTL for warm pool state - Add IAM policies for scale-down, scale-up, and pool lambdas (DynamoDB + EC2 stop/start) - Wire WARM_POOL_CONFIG, WARM_POOL_TABLE_NAME, POOL_STRATEGY env vars to all lambdas - Add enable_warm_pool metric toggle - Cross-variable validation: pool_strategy=warm requires warm_pool_config.enabled=true Signed-off-by: Brend Smits <brend.smits@philips.com>
- Add warm-pool.ts with DynamoDB operations (add, remove, get, list, count) - Add stopRunner() and startRunner() to runners.ts - Add emitWarmPoolMetric() for CloudWatch metric publishing - Export getWarmPoolConfig() and getPoolStrategy() helpers Signed-off-by: Brend Smits <brend.smits@philips.com>
- Modify removeRunner() to stop instances into warm pool when enabled - Add DynamoDB record on stop with ghr:warm-pool-member tag - Respect maxWarmInstances cap (terminate if full) - Add evictStaleWarmInstances() for periodic cleanup: - Evict instances exceeding maxWarmAgeHours - Evict instances exceeding maxWarmInstances count - Emit WarmPoolSize metric after eviction Signed-off-by: Brend Smits <brend.smits@philips.com>
- Add findAndStartWarmRunners() to query DynamoDB and start stopped instances - Integrate warm pool lookup before cold-launching new runners - Handle org-level fallback when repo-level owner has no warm instances - Write runner registration config (SSM) for restarted warm instances - Tag started instances with ghr:started-from-warm-pool, remove ghr:warm-pool-member - Emit WarmPoolInstanceStarted/WarmPoolStartFailed metrics - Self-heal stale DynamoDB records on start failure Signed-off-by: Brend Smits <brend.smits@philips.com>
- Pool counts warm (stopped) instances toward pool target when pool_strategy=warm - Pool tries to start warm instances before cold-launching new ones - Remaining deficit is filled by createRunners (cold start) - Scale-down will stop excess idle runners into warm pool after they run Signed-off-by: Brend Smits <brend.smits@philips.com>
- Test disabled/hot strategy returns empty - Test starting and tagging warm instances - Test failure handling (skip failed, cleanup DynamoDB) - Test org-level fallback for repo-level owners - Test multiple instance start up to count - Test tag failure as non-fatal (best-effort) Signed-off-by: Brend Smits <brend.smits@philips.com>
Scale-down eviction tests: - Skip eviction when warm pool disabled - Evict instances exceeding max age - Evict instances exceeding max count - Emit WarmPoolSize metric after eviction Pool warm strategy tests: - Count warm instances toward pool target - Try warm instances before cold launching - Skip cold launch when warm satisfies full top-up - Don't count warm instances with hot strategy Signed-off-by: Brend Smits <brend.smits@philips.com>
- Add amiId field to WarmPoolEntry interface and DynamoDB storage - Store current AMI ID (from SSM parameter) when stopping instances - Evict warm instances with stale AMI during eviction cycle - Add @aws-sdk/client-dynamodb to control-plane dependencies - Pass AMI_ID_SSM_PARAMETER_NAME env var to scale-down lambda - Grant scale-down IAM permission to read AMI SSM parameter Signed-off-by: Brend Smits <brend.smits@philips.com>
Add findAndStartWarmRunners and warm-pool module mocks to existing pool.test.ts to prevent failures from new imports. Signed-off-by: Brend Smits <brend.smits@philips.com>
- Verify DynamoDB table not created when disabled (default) - Verify DynamoDB table created with correct schema when enabled - Verify IAM policies attached to scale-down, scale-up, and pool roles - Verify pool_strategy defaults to 'hot' - Verify warm strategy + enabled config creates resources Signed-off-by: Brend Smits <brend.smits@philips.com>
- Measure and emit start latency (ms) for warm instance starts - Emit WarmPoolEvicted metric count during eviction cycles - Use MetricUnit.Milliseconds for latency metric Signed-off-by: Brend Smits <brend.smits@philips.com>
When pool_strategy=warm and new instances are cold-launched: - Wait warmPoolReadyDelaySeconds (default 30s) after creation - Re-check GitHub runner status for each new instance - If runner picked up a job: leave running, tag ghr:warm-pool-grace-hit - If runner is idle: stop instance and add to warm pool This enables the pool to maintain warm (stopped) instances at zero idle compute cost while giving jobs a chance to claim new runners. Signed-off-by: Brend Smits <brend.smits@philips.com>
Signed-off-by: Brend Smits <brend.smits@philips.com>
Contributor
Dependency ReviewThe following issues were found:
License Issueslambdas/functions/control-plane/package.json
OpenSSF ScorecardScorecard details
Scanned Files
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a “warm pool” lifecycle to the Terraform AWS GitHub Runner module so idle runners can be stopped (EBS-only cost) and later restarted quickly, instead of being terminated and cold-launched.
Changes:
- Introduces
warm_pool_configandpool_strategyinputs and wires them through root, runners, pool, and multi-runner modules. - Provisions a DynamoDB warm-pool state table plus IAM policies, and passes warm-pool env vars into scale-up/scale-down/pool lambdas.
- Implements warm-pool stop/start and eviction logic in the control-plane lambdas, adds unit + Terraform tests, and documents the design (docs + ADRs).
Reviewed changes
Copilot reviewed 33 out of 34 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| variables.tf | Adds root-level warm-pool inputs and warm-pool metrics toggle. |
| README.md | Documents new inputs and updates instance allocation strategy wording. |
| modules/termination-watcher/termination/README.md | Regenerates docs to include new IAM policy resource. |
| modules/termination-watcher/notification/README.md | Regenerates docs to include new event rules/targets/permissions/policy resources. |
| modules/runners/warm-pool.tf | Creates warm-pool DynamoDB table + IAM inline policies (new file). |
| modules/runners/variables.tf | Adds warm-pool inputs and metrics toggle in runners module. |
| modules/runners/tests/warm-pool.tftest.hcl | Adds Terraform test coverage for warm-pool resources and validation. |
| modules/runners/scale-up.tf | Passes warm-pool env vars to scale-up lambda. |
| modules/runners/scale-down.tf | Passes warm-pool env vars to scale-down lambda and adds SSM read policy attachment for AMI staleness logic. |
| modules/runners/README.md | Regenerates runners module docs for warm-pool resources/inputs. |
| modules/runners/pool/variables.tf | Extends pool submodule config object to include warm-pool settings. |
| modules/runners/pool/README.md | Regenerates pool submodule docs for extended config schema. |
| modules/runners/pool/main.tf | Passes warm-pool env vars to pool lambda. |
| modules/runners/pool.tf | Wires warm-pool table/config into the pool submodule. |
| modules/runners/policies/lambda-warm-pool.json | New IAM policy template for DynamoDB + EC2 stop/start/tag actions. |
| modules/multi-runner/variables.tf | Adds warm-pool fields to multi-runner config type. |
| modules/multi-runner/runners.tf | Passes warm-pool inputs through to each runners module instance. |
| modules/multi-runner/README.md | Regenerates multi-runner docs to include warm-pool config fields. |
| mkdocs.yaml | Adds warm-pool page to documentation nav. |
| main.tf | Wires root warm-pool inputs into the runners module. |
| lambdas/yarn.lock | Adds @aws-sdk/client-dynamodb dependency lock entries. |
| lambdas/functions/control-plane/src/scale-runners/scale-up.ts | Adds warm instance lookup/start path before cold launching. |
| lambdas/functions/control-plane/src/scale-runners/scale-down.ts | Stops idle instances into warm pool and adds warm eviction logic. |
| lambdas/functions/control-plane/src/scale-runners/scale-down-warm-pool.test.ts | Adds unit tests for eviction behavior. |
| lambdas/functions/control-plane/src/scale-runners/find-and-start-warm-runners.test.ts | Adds unit tests for warm start behavior. |
| lambdas/functions/control-plane/src/pool/pool.ts | Updates pool logic to account for warm instances and implement warm-strategy grace period stop. |
| lambdas/functions/control-plane/src/pool/pool.test.ts | Updates existing pool tests for new wiring/mocks. |
| lambdas/functions/control-plane/src/pool/pool-warm-strategy.test.ts | Adds pool warm-strategy-specific unit tests. |
| lambdas/functions/control-plane/src/aws/warm-pool.ts | New DynamoDB warm-pool state client + metric emitter. |
| lambdas/functions/control-plane/src/aws/runners.ts | Adds EC2 start/stop helpers for warm pool. |
| lambdas/functions/control-plane/package.json | Adds @aws-sdk/client-dynamodb dependency. |
| docs/warm-pool.md | Adds feature documentation and configuration guide. |
| docs/adr/001-warm-pool-implementation-plan.md | Adds detailed phased implementation plan ADR. |
| docs/adr/001-warm-pool-hibernation.md | Adds warm-pool decision ADR and design rationale. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove unused imports in test files (CodeQL warnings) - Move check block to variable validation (TF >= 1.3 compat) - Gate scale-up warm starts on warmPoolConfig.enabled only, not poolStrategy (scale-up should try warm starts regardless of pool lambda strategy) - Wrap scale-down warm pool stop in try/catch with fallback to terminate on failure - Include stopped instances in eviction owner lookup - Add AMI ID to pool-created warm instances for staleness eviction - Make removeFromWarmPool atomic with conditional delete to prevent concurrent scale-up race conditions - Add test for concurrent claim scenario Signed-off-by: Brend Smits <brend.smits@philips.com>
Cross-variable references in variable validation blocks require Terraform 1.9+. The repo supports >= 1.3.0, so this breaks CI on TF 1.5.6. The constraint (pool_strategy=warm requires enabled=true) is documented in the variable description and enforced at runtime by the lambda. Signed-off-by: Brend Smits <brend.smits@philips.com>
Contributor
|
Great idea. |
Contributor
Author
|
@copilot resolve the merge conflicts in this pull request. There is a major refactor ongoing on several parts in the control plane. Take this into account and make the needed changes |
…ernation # Conflicts: # lambdas/functions/control-plane/src/pool/pool.test.ts # lambdas/functions/control-plane/src/pool/pool.ts # lambdas/functions/control-plane/src/scale-runners/scale-down.ts # lambdas/functions/control-plane/src/scale-runners/scale-up.ts # main.tf # modules/multi-runner/README.md # modules/runners/pool/README.md # modules/runners/pool/main.tf # modules/runners/scale-down.tf # variables.tf
Contributor
Merge conflicts resolved in 3cbd134, rebasing the warm pool onto the ongoing control-plane provider refactor. Rather than patching the old monolithic lambdas, I adopted main's provider-based architecture as the base and re-integrated the warm pool behind the new provider interfaces so it stays EC2-specific:
All 570 control-plane unit tests pass (warm-pool tests ported to the new provider files), and tsc, eslint, prettier, and |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 37 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
modules/runners/variables.tf:923
pool_strategy = "warm"is described as requiringwarm_pool_config.enabled = true, but there’s no validation enforcing this invariant. Without it, users can configure an unsupported combination and the lambdas will receivePOOL_STRATEGY=warmwithout a warm-pool table/policy being created.
variables.tf:860pool_strategyis documented as requiringwarm_pool_config.enabled = truewhen set towarm, but this is not enforced. This allows invalid configurations to plan/apply and then fail at runtime when warm-pool resources/env vars are missing.
lambdas/functions/control-plane/src/scale-runners/ec2-scale-up.ts:140startWarmInstancescurrently iterates DynamoDB query results in the order returned bylistWarmInstancesByOwner, which is configured asScanIndexForward: true(oldest first). This causes scale-up to start the stalest warm instances first, conflicting with the docs/ADR intent of preferring the freshest stopped instance.
const startedInstances: string[] = [];
lambdas/functions/control-plane/src/pool/ec2-pool.ts:128
- When a runner becomes busy during the warm-strategy grace window, the code emits
WarmPoolInstanceStarted. That metric name implies a warm-pool restart, but this path is a normal (cold) start that simply avoided being stopped. This will inflate warm-start counts and make metrics misleading.
if (status?.busy) {
// Runner picked up a job during the grace window — leave it running.
logger.info(`Runner '${instanceId}' picked up a job during grace period, leaving running`);
await tag(instanceId, [{ Key: 'ghr:warm-pool-grace-hit', Value: 'true' }]).catch(() => {
/* best-effort */
});
emitWarmPoolMetric('WarmPoolInstanceStarted', 1, { Owner: runnerOwner });
} else {
| "Resource": "*", | ||
| "Condition": { | ||
| "StringEquals": { | ||
| "ec2:ResourceTag/gh:environment": "${environment}" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds a warm pool feature that reduces runner startup latency from 2–5 minutes to 10–30 seconds by stopping idle instances instead of terminating them.
Three-tier instance lifecycle:
Key components:
pool_strategy = "warm"for zero-idle-compute deploymentsWarmPoolStartLatency,WarmPoolEvicted, etc.See docs/warm-pool.md and ADR-001 for full design.
Test Plan
warm-pool.tftest.hcl)Related Issues
N/A — new feature