refactor(runners): split runner provider layout#5203
Conversation
Dependency ReviewThe following issues were found:
License Issueslambdas/functions/control-plane/package.json
lambdas/functions/webhook/package.json
OpenSSF Scorecard
Scanned Files
|
37edc84 to
ef4f7e7
Compare
There was a problem hiding this comment.
Pull request overview
Refactors the runner orchestration code to be provider-based (while keeping EC2 as the only/default provider), splitting EC2-specific logic into dedicated modules and introducing provider registries for scale-up, scale-down, and pool management. This also renames the dynamic label policy configuration to be AWS/provider-scoped in preparation for future non-EC2 providers.
Changes:
- Introduces provider interfaces/registries and moves EC2 scale-up, scale-down, pool, and dynamic-label handling behind EC2 provider implementations.
- Renames dynamic label policy configuration from EC2-specific to AWS/provider-scoped (
ec2_*→aws_*) across Terraform and webhook config. - Adds explicit/default EC2 provider type wiring in scheduler/pool inputs and scale-up lambda environment.
Reviewed changes
Copilot reviewed 58 out of 60 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| variables.tf | Adds provider type to idle config objects; renames dynamic label policy input to aws_dynamic_labels_policy; updates scale error description text. |
| README.md | Regenerates input docs reflecting aws_dynamic_labels_policy, idle config type, and updated allocation strategy description. |
| main.tf | Wires webhook matcher config to aws_dynamic_labels_policy. |
| docs/configuration.md | Updates examples/docs for idle config provider type and renamed dynamic label policy variable/fields. |
| modules/webhook/variables.tf | Renames matcher policy field to awsDynamicLabelsPolicy in module input schema/docs. |
| modules/webhook/README.md | Regenerates module docs for awsDynamicLabelsPolicy. |
| modules/termination-watcher/termination/README.md | Regenerates docs to include new policy resource listing. |
| modules/termination-watcher/notification/README.md | Regenerates docs to include additional event/policy resources. |
| modules/runners/variables.tf | Adds provider type to idle config objects; updates scale error description text. |
| modules/runners/scale-up.tf | Adds explicit RUNNER_PROVIDER_TYPE=ec2 environment variable to scale-up lambda. |
| modules/runners/README.md | Regenerates docs for idle config type, allocation strategy text, and new inputs. |
| modules/runners/pool/README.md | Updates pool module config schema docs (adds instance type priorities). |
| modules/runners/pool/main.tf | Adds type: "ec2" to scheduler input payload. |
| modules/multi-runner/variables.tf | Adds provider type to idle config objects; renames matcher policy field to awsDynamicLabelsPolicy; updates scale error description text. |
| modules/multi-runner/README.md | Regenerates docs reflecting idle config type and awsDynamicLabelsPolicy rename. |
| lambdas/functions/webhook/src/sqs/index.ts | Renames matcher policy type usage to awsDynamicLabelsPolicy and introduces optional runner provider field on queue config. |
| lambdas/functions/webhook/src/runners/labels.ts | Extracts/centralizes workflow label splitting + sanitization + label matching logic. |
| lambdas/functions/webhook/src/runners/ec2-dynamic-labels.ts | Implements EC2-specific dynamic label queue selection using the new provider strategy pattern. |
| lambdas/functions/webhook/src/runners/ec2-dynamic-labels-policy.ts | Refactors EC2 policy typing to reuse AWS policy types while retaining EC2 policy evaluation behavior. |
| lambdas/functions/webhook/src/runners/dynamic-labels-policy.test.ts | Updates tests to reference EC2 policy module path after refactor. |
| lambdas/functions/webhook/src/runners/dispatch.ts | Refactors webhook dispatch logic to use shared label helpers and provider-based dynamic label selection. |
| lambdas/functions/webhook/src/runners/dispatch.test.ts | Updates dispatch tests for awsDynamicLabelsPolicy and adds coverage for default EC2 provider selection. |
| lambdas/functions/webhook/src/runners/aws-dynamic-labels.ts | Adds provider-based dynamic label selection orchestration (currently only EC2 strategy registered). |
| lambdas/functions/webhook/src/runners/aws-dynamic-labels-provider.ts | Defines the provider strategy interface for dynamic label queue selection. |
| lambdas/functions/webhook/src/runners/aws-dynamic-labels-policy.ts | Introduces provider-agnostic AWS dynamic label policy type definitions. |
| lambdas/functions/control-plane/src/scale-runners/types.ts | Extracts shared types from scale-up into a dedicated types module. |
| lambdas/functions/control-plane/src/scale-runners/ScaleError.ts | Updates type import source to the new shared types module. |
| lambdas/functions/control-plane/src/scale-runners/ScaleError.test.ts | Updates type import source to the new shared types module. |
| lambdas/functions/control-plane/src/scale-runners/scale-up.ts | Refactors scale-up to use provider registry + GitHub runner helpers and EC2 provider implementation. |
| lambdas/functions/control-plane/src/scale-runners/scale-up-provider.ts | Defines the scale-up provider interface and strategy creation contract. |
| lambdas/functions/control-plane/src/scale-runners/scale-up-provider-registry.ts | Adds registry for scale-up provider strategies (EC2 default). |
| lambdas/functions/control-plane/src/scale-runners/scale-up-config.ts | Adds helper for choosing scale-up provider type from env/default. |
| lambdas/functions/control-plane/src/scale-runners/scale-down.ts | Refactors scale-down to use provider registry, provider runner abstraction, and GitHub runner helper utilities. |
| lambdas/functions/control-plane/src/scale-runners/scale-down.test.ts | Updates scale-down tests for renamed EC2 runner modules and mocks. |
| lambdas/functions/control-plane/src/scale-runners/scale-down-provider.ts | Introduces provider interface for listing/marking/terminating runners during scale-down. |
| lambdas/functions/control-plane/src/scale-runners/scale-down-provider-registry.ts | Adds registry for scale-down provider strategies (EC2 default). |
| lambdas/functions/control-plane/src/scale-runners/scale-down-config.ts | Adds optional provider type to scale-down config and logic to enforce a single provider type per config. |
| lambdas/functions/control-plane/src/scale-runners/scale-down-config.test.ts | Adds unit tests for determining scale-down provider type. |
| lambdas/functions/control-plane/src/scale-runners/job-retry.ts | Updates imports to use GitHub runner helpers and shared types. |
| lambdas/functions/control-plane/src/scale-runners/job-retry.test.ts | Updates type imports to use the new shared types module. |
| lambdas/functions/control-plane/src/scale-runners/github-runner.ts | Extracts shared GitHub runner registration/JIT/token logic from scale-up into reusable helper module. |
| lambdas/functions/control-plane/src/scale-runners/ec2.ts | Extracts EC2-specific runner creation + metadata tagging into its own module. |
| lambdas/functions/control-plane/src/scale-runners/ec2-scale-up.ts | Implements EC2 scale-up provider strategy including dynamic EC2 override label parsing integration. |
| lambdas/functions/control-plane/src/scale-runners/ec2-scale-down.ts | Implements EC2 scale-down provider strategy (list/terminate/orphan tag operations). |
| lambdas/functions/control-plane/src/scale-runners/ec2-labels.ts | Extracts EC2 override label parsing and launch template block device lookup into a dedicated module. |
| lambdas/functions/control-plane/src/pool/pool.ts | Refactors pool adjuster to use provider registry + provider-specific logic and requires provider type in pool events. |
| lambdas/functions/control-plane/src/pool/pool.test.ts | Updates pool tests to use provider-driven pool logic and adds tests for EC2 pool provider behavior. |
| lambdas/functions/control-plane/src/pool/pool-provider.ts | Introduces provider interface for pool runner listing/counting/creation. |
| lambdas/functions/control-plane/src/pool/pool-provider-registry.ts | Adds registry for pool provider strategies (EC2 default). |
| lambdas/functions/control-plane/src/pool/ec2-pool.ts | Implements EC2 pool provider (list running runners, count “available”, create runners). |
| lambdas/functions/control-plane/src/modules.d.ts | Adds optional RUNNER_PROVIDER_TYPE env var typing. |
| lambdas/functions/control-plane/src/local-pool.ts | Updates local pool runner to include explicit type: 'ec2' in the event. |
| lambdas/functions/control-plane/src/lambda.ts | Updates imports to use shared types and refactored scale-up module. |
| lambdas/functions/control-plane/src/lambda.test.ts | Updates adjust pool tests to include pool event type. |
| lambdas/functions/control-plane/src/github/octokit.ts | Updates type import source to shared types module. |
| lambdas/functions/control-plane/src/github/octokit.test.ts | Updates type import source to shared types module. |
| lambdas/functions/control-plane/src/aws/runners.test.ts | Updates imports to renamed EC2 runner modules and shared types. |
| lambdas/functions/control-plane/src/aws/ec2-runners.ts | Updates internal type import path for renamed EC2 runners declarations. |
| lambdas/functions/control-plane/src/aws/ec2-runners.d.ts | Updates type import source to shared types module. |
Comments suppressed due to low confidence (1)
modules/webhook/variables.tf:40
- The
runner_matcher_configvalidation currently usestry(var.runner_matcher_config.matcherConfig.priority, 999), butrunner_matcher_configis a map so this expression errors andtry(...)always falls back to 999. As a result, invalidpriorityvalues in individual matcher entries won’t be validated. Consider validating over all map values instead.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Brend-Smits
left a comment
There was a problem hiding this comment.
I love the idea for this, I need a bit more time to review the changes though.
Have you been running this change in your environment?
My reviews are a little bit more delayed at the moment due to my vacation.
guicaulada
left a comment
There was a problem hiding this comment.
Overall this looks good to me and pretty clean for 2,000+ moved lines!
The provider seams are sensibly placed for future MicroVM work.
Two things I think need to happen before merge: a rebase that ports #5078 (pool_include_busy_runners) into the new pool provider, and a default for the pool event's type so existing scheduler payloads don't silently disable pool top-up mid-upgrade.
The SSM policy fail-open is worth a bridge or a loud warning even under the experimental banner. Happy to re-review after the rebase.
One other thing I'd like to see is a shared contract between scale-up, scale-down and pool. Currently adding a second provider means implementing three subtly different contracts. I'd like to see a shared "RunnerProvider" contract across all three. A lot of the review comments I made seem to point towards a missing opportunity to share some types and implementation. They diverge just slightly and seem hard to maintain, making changes to the RunnerProvider contract currently means changing multiple places where it's kind of re-implemented.
e0b048a to
e6d42f5
Compare
Description
Refactors the existing EC2 runner implementation behind provider-specific boundaries while preserving EC2 as the only supported/default runner provider in this PR.
This prepares the codebase for future AWS runner providers without adding MicroVM support yet:
ec2-runners.ts,ec2-runners.d.ts)ec2_dynamic_labels_policy/ec2DynamicLabelsPolicytoaws_dynamic_labels_policy/awsDynamicLabelsPolicyidle_configremains provider-neutralTest Plan
Replace your config to use aws_dynamic_labels_policy and run apply. Everything should work as today
Related Issues
N/A