From c38b9a72348fc7b56428ab73b17544c6edc69e0e Mon Sep 17 00:00:00 2001 From: kurok <22548029+kurok@users.noreply.github.com> Date: Thu, 2 Jul 2026 19:55:12 +0100 Subject: [PATCH] =?UTF-8?q?feat:=20capacity=20resilience=20=E2=80=94=20ord?= =?UTF-8?q?ered=20fallback=20across=20subnets/AZs=20and=20instance=20types?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A single subnet-id + ec2-instance-type is a single point of failure: when that AZ has no capacity for that type, RunInstances fails and the whole workflow with it. Both inputs now accept comma-separated ordered fallback lists (single values behave byte-identically): ec2-instance-type: c7i.4xlarge,c6i.4xlarge,m7i.4xlarge subnet-id: subnet-aaa,subnet-bbb # different AZs - launchWithFallback walks type × subnet in order — every subnet/AZ is tried before downgrading the instance type (placement is cheaper than a hardware change). Capacity errors advance the chain; non-capacity errors (invalid AMI, auth, quota InstanceLimitExceeded) abort immediately so a misconfig doesn't burn the whole matrix; transient API errors retry within the cell. - classifyRunError buckets RunInstances errors into capacity / transient / fatal (spot capacity codes already included for #39). - retry.js withRetry gains an optional shouldRetry predicate (default unchanged), so a cell retries only transient errors. - New outputs instance-type-used / subnet-id-used report what actually launched. - Each failed placement logs a warn line (type, subnet, code); exhaustion fails with a summary of every attempt. Tests: classifyRunError buckets, launchWithFallback ordering (first-try, mid- chain, full-matrix exhaustion, fatal-abort, quota message), parseCsv, and withRetry shouldRetry (142 tests total). README "Capacity fallback" section + inputs/outputs tables and action.yml updated. Note: spot × fallback composition lands with #39 (spot support); the capacity codes and chain are already spot-ready. Closes #40 Signed-off-by: kurok <22548029+kurok@users.noreply.github.com> --- README.md | 24 ++++++- action.yml | 17 ++++- dist/index.js | 154 ++++++++++++++++++++++++++++++++++++----- src/aws.js | 116 +++++++++++++++++++++++++++---- src/index.js | 19 +++-- src/retry.js | 7 ++ src/utils.js | 12 ++++ tests/fallback.test.js | 78 +++++++++++++++++++++ tests/retry.test.js | 20 ++++++ tests/utils.test.js | 21 +++++- 10 files changed, 427 insertions(+), 41 deletions(-) create mode 100644 tests/fallback.test.js diff --git a/README.md b/README.md index f3216381..dd419e47 100644 --- a/README.md +++ b/README.md @@ -307,8 +307,8 @@ Now you're ready to go! | `ec2-image-id` | Required for `start` mode, unless `ec2-image-filters` is set. | EC2 Image Id (AMI).

The new runner will be launched from this image.

Only **yum-based** AMIs are supported (Amazon Linux 2023 tested; AL2 / RHEL-family in principle). See the [Supported operating systems](#on-demand-self-hosted-aws-ec2-runner-for-github-actions) notice at the top of this README. | | `ec2-image-filters` | Optional. Used only with the `start` mode. | Stringified JSON array of EC2 `DescribeImages` filters used to look up the AMI when `ec2-image-id` is not provided.

Example: `[{"Name": "name", "Values": ["al2023-ami-*-x86_64"]}]`. The most recently created matching image is used. | | `ec2-image-owner` | Optional. Used only with the `start` mode. | Scopes the `ec2-image-filters` AMI lookup to specific owners (AWS account IDs, `self`, `amazon`, or `aws-marketplace`). | -| `ec2-instance-type` | Required if you use the `start` mode. | EC2 Instance Type. | -| `subnet-id` | Required if you use the `start` mode. | VPC Subnet Id.

The subnet should belong to the same VPC as the specified security group. | +| `ec2-instance-type` | Required if you use the `start` mode. | EC2 Instance Type.

Accepts a comma-separated **ordered fallback list** (e.g. `c7i.4xlarge,c6i.4xlarge,m7i.4xlarge`) — see [Capacity fallback](#capacity-fallback-across-azs-and-instance-types). A single value behaves as before. | +| `subnet-id` | Required if you use the `start` mode. | VPC Subnet Id.

The subnet should belong to the same VPC as the specified security group.

Accepts a comma-separated **ordered fallback list** of subnets (typically across AZs), e.g. `subnet-aaa,subnet-bbb`. | | `security-group-id` | Required if you use the `start` mode. | EC2 Security Group Id.

The security group should belong to the same VPC as the specified subnet.

Only the outbound traffic for port 443 should be allowed. No inbound traffic is required. | | `label` | Required if you use the `stop` mode. | Name of the unique label assigned to the runner.

The label is provided by the output of the action in the `start` mode.

The label is used to remove the runner from GitHub when the runner is not needed anymore. | | `ec2-instance-id` | Required if you use the `stop` mode. | EC2 Instance Id of the created runner.

The id is provided by the output of the action in the `start` mode.

The id is used to terminate the EC2 instance when the runner is not needed anymore. | @@ -345,6 +345,8 @@ We recommend using [aws-actions/configure-aws-credentials](https://github.com/aw | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `label` | Name of the unique label assigned to the runner.

The label is used in two cases:
- to use as the input of `runs-on` property for the following jobs;
- to remove the runner from GitHub when it is not needed anymore. | | `ec2-instance-id` | EC2 Instance Id of the created runner.

The id is used to terminate the EC2 instance when the runner is not needed anymore. | +| `instance-type-used` | The EC2 instance type actually launched. With a [capacity-fallback](#capacity-fallback-across-azs-and-instance-types) list this may differ from your first choice. | +| `subnet-id-used` | The subnet the runner was actually launched into. With a capacity-fallback list this may differ from your first choice. | ### Example @@ -419,6 +421,24 @@ In [this discussion](https://github.com/machulav/ec2-github-runner/discussions/1 If you use this action in your workflow, feel free to add your story there as well 🙌 +## Capacity fallback across AZs and instance types + +A single `subnet-id` + `ec2-instance-type` means a single point of failure: when that AZ has no capacity for that type — routine for larger/GPU types — `RunInstances` fails and the whole workflow fails with it. Pass comma-separated ordered lists and the action walks them until a launch succeeds: + +```yml +- name: Start EC2 runner + uses: namecheap/ec2-github-runner@v3 + with: + mode: start + # ... other inputs ... + ec2-instance-type: c7i.4xlarge,c6i.4xlarge,m7i.4xlarge + subnet-id: subnet-aaa,subnet-bbb # different AZs +``` + +**Order:** for each instance type, every subnet/AZ is tried before downgrading to the next type (placement is cheaper than a hardware change). On an insufficient-capacity error the action advances to the next cell; **non-capacity errors** (invalid AMI, auth, or a quota like `InstanceLimitExceeded`) fail immediately so a misconfiguration doesn't burn through the whole matrix. Transient API errors are retried within each cell. Each failed placement logs a warning line (type, subnet, error code); full exhaustion fails with a summary of every attempt. + +The `instance-type-used` and `subnet-id-used` outputs report what actually launched. Single values keep the original single-attempt behavior. + ## Disk space for Docker workloads The runner inherits the AMI's root volume size — 8 GiB on Amazon Linux 2023. Docker-based CI exhausts that almost immediately (a couple of large images plus build cache), and the job dies with `no space left on device` — one of the most common self-hosted-runner failures. Size the root volume for your workload: diff --git a/action.yml b/action.yml index 90f2c81f..cafd86e6 100644 --- a/action.yml +++ b/action.yml @@ -40,13 +40,18 @@ inputs: required: false ec2-instance-type: description: >- - EC2 Instance Type. - This input is required if you use the 'start' mode. + EC2 Instance Type. This input is required if you use the 'start' mode. + Accepts a comma-separated ordered fallback list (e.g. + 'c7i.4xlarge,c6i.4xlarge,m7i.4xlarge'): on an insufficient-capacity + error the action tries the next type. A single value behaves as before. required: false subnet-id: description: >- VPC Subnet Id. The subnet should belong to the same VPC as the specified security group. This input is required if you use the 'start' mode. + Accepts a comma-separated ordered fallback list of subnets (typically + in different AZs, e.g. 'subnet-aaa,subnet-bbb'): the action exhausts + all subnets for an instance type before moving to the next type. required: false security-group-id: description: >- @@ -193,6 +198,14 @@ outputs: description: >- EC2 Instance Id of the created runner. The id is used to terminate the EC2 instance when the runner is not needed anymore. + instance-type-used: + description: >- + The EC2 instance type that was actually launched (start mode). With a + capacity-fallback list this may differ from the first choice. + subnet-id-used: + description: >- + The subnet the runner was actually launched into (start mode). With a + capacity-fallback list this may differ from the first choice. runs: using: node24 main: ./dist/index.js diff --git a/dist/index.js b/dist/index.js index 7a339a02..f5a3b2a9 100644 --- a/dist/index.js +++ b/dist/index.js @@ -104663,9 +104663,82 @@ const core = __nccwpck_require__(7484); const config = __nccwpck_require__(1283); const log = __nccwpck_require__(7223); const { withRetry } = __nccwpck_require__(6759); -const { sortByCreationDate } = __nccwpck_require__(5804); +const { sortByCreationDate, parseCsv } = __nccwpck_require__(5804); const checksums = __nccwpck_require__(2874); +// RunInstances error classification for the capacity-fallback chain. +// - capacity: this placement (type × subnet/AZ) has no capacity right now — +// advance to the next cell in the chain. +// - transient: a retryable API-layer hiccup — retry the same cell. +// - fatal (default): misconfiguration or quota — abort immediately so a +// bad config doesn't burn through the whole matrix. +const CAPACITY_ERROR_CODES = new Set([ + 'InsufficientInstanceCapacity', + 'InsufficientHostCapacity', + 'InsufficientReservedInstanceCapacity', + 'Unsupported', + 'SpotMaxPriceTooLow', // spot capacity (composes with #39) + 'MaxSpotInstanceCountExceeded', +]); +const TRANSIENT_ERROR_CODES = new Set([ + 'RequestLimitExceeded', + 'Throttling', + 'ThrottlingException', + 'InternalError', + 'InternalFailure', + 'ServiceUnavailable', + 'Unavailable', +]); + +function classifyRunError(error) { + const name = error && error.name; + if (CAPACITY_ERROR_CODES.has(name)) { + return 'capacity'; + } + if (TRANSIENT_ERROR_CODES.has(name)) { + return 'transient'; + } + return 'fatal'; +} + +// Walk the instance-type × subnet fallback chain until one placement +// succeeds. Order: for each instance type, try every subnet/AZ before +// downgrading the type (placement is cheaper than a hardware change). On a +// capacity error, advance to the next cell; on a quota or any other fatal +// error, abort immediately (a misconfig must not burn the whole matrix). +// `attempt(instanceType, subnetId)` is injected (returns the instance id or +// throws), keeping the ordering logic testable without the AWS SDK. +async function launchWithFallback(attempt, instanceTypes, subnetIds) { + const failures = []; + for (const instanceType of instanceTypes) { + for (const subnetId of subnetIds) { + try { + const instanceId = await attempt(instanceType, subnetId); + return { instanceId, instanceType, subnetId }; + } catch (error) { + const kind = classifyRunError(error); + if (kind === 'capacity') { + failures.push({ instanceType, subnetId, code: error.name || 'Unknown' }); + log.warn('run_instance_fallback', { instance_type: instanceType, subnet_id: subnetId, error_code: error.name, message: error.message }); + continue; + } + if (error.name === 'InstanceLimitExceeded') { + throw new Error( + `RunInstances hit an account quota (InstanceLimitExceeded) launching ${instanceType} in ${subnetId}: ${error.message}. ` + + 'This is a service limit, not a capacity shortage — request a quota increase. The fallback chain was not continued.', + { cause: error }, + ); + } + throw error; + } + } + } + const summary = failures.map((f) => `${f.instanceType}/${f.subnetId}=${f.code}`).join('; '); + const error = new Error(`All ${failures.length} placement attempt(s) failed due to insufficient capacity: ${summary}`); + error.capacityExhausted = true; + throw error; +} + // Instance tag the bootstrap script writes to phone home its progress. // The start action polls it to fail fast on cloud-init errors instead of // waiting out the full registration timeout. See buildUserData(). @@ -105007,13 +105080,13 @@ async function startEc2Instance(label, githubRegistrationToken) { const resolved = await resolveImage(client); config.input.ec2ImageId = resolved.id; + // InstanceType and SubnetId are injected per attempt by the fallback + // chain (see below), so they are intentionally absent from the base. const params = { ImageId: config.input.ec2ImageId, - InstanceType: config.input.ec2InstanceType, MinCount: 1, MaxCount: 1, UserData: Buffer.from(userData).toString('base64'), - SubnetId: config.input.subnetId, SecurityGroupIds: [config.input.securityGroupId], IamInstanceProfile: { Name: config.input.iamRoleName }, TagSpecifications: buildTagSpecifications(label, new Date().toISOString()), @@ -105056,26 +105129,43 @@ async function startEc2Instance(label, githubRegistrationToken) { } } - let ec2InstanceId; - const runStart = Date.now(); + // Capacity fallback: walk instance-type × subnet/AZ in order, advancing + // on capacity errors and retrying only transient API errors within each + // cell. Single-value inputs collapse to a one-cell chain (byte-identical + // to the pre-fallback behavior). + const instanceTypes = parseCsv(config.input.ec2InstanceType); + const subnetIds = parseCsv(config.input.subnetId); log.info('run_instance', { ami_id: config.input.ec2ImageId, - instance_type: config.input.ec2InstanceType, - subnet_id: config.input.subnetId, + instance_types: instanceTypes, + subnet_ids: subnetIds, sg_id: config.input.securityGroupId, iam_role: config.input.iamRoleName || null, label, }); + + const attempt = async (instanceType, subnetId) => { + const attemptParams = { ...params, InstanceType: instanceType, SubnetId: subnetId }; + const result = await withRetry( + 'run_instance', + () => client.send(new RunInstancesCommand(attemptParams)), + { shouldRetry: (error) => classifyRunError(error) === 'transient' }, + ); + return result.Instances[0].InstanceId; + }; + + let placement; + const runStart = Date.now(); try { - const result = await client.send(new RunInstancesCommand(params)); - ec2InstanceId = result.Instances[0].InstanceId; - log.info('run_instance', { instance_id: ec2InstanceId, elapsed_ms: Date.now() - runStart }); - core.info(`AWS EC2 instance ${ec2InstanceId} is started`); + placement = await launchWithFallback(attempt, instanceTypes, subnetIds); } catch (error) { log.error('run_instance', { error: error.name, message: error.message }); core.error('AWS EC2 instance starting error'); throw error; } + const ec2InstanceId = placement.instanceId; + log.info('run_instance', { instance_id: ec2InstanceId, instance_type: placement.instanceType, subnet_id: placement.subnetId, elapsed_ms: Date.now() - runStart }); + core.info(`AWS EC2 instance ${ec2InstanceId} is started (type ${placement.instanceType}, subnet ${placement.subnetId})`); if (config.input.eipAllocationId) { await waitForInstanceRunning(ec2InstanceId); @@ -105092,7 +105182,7 @@ async function startEc2Instance(label, githubRegistrationToken) { } } - return ec2InstanceId; + return { instanceId: ec2InstanceId, instanceType: placement.instanceType, subnetId: placement.subnetId }; } async function terminateInstanceById(ec2InstanceId) { @@ -105269,6 +105359,8 @@ module.exports = { handleStartFailure, listManagedInstances, // Exported for unit testing. + classifyRunError, + launchWithFallback, buildRootDeviceMapping, wantsRootDeviceMapping, buildVolumeOpts, @@ -105763,6 +105855,10 @@ async function withRetry(step, fn, opts = {}) { const attempts = opts.attempts || 3; const baseMs = opts.baseMs || 2000; const maxMs = opts.maxMs || 10000; + // Optional predicate — when it returns false the error is re-thrown + // immediately without further retries. Defaults to retrying everything, + // preserving the original behavior for existing callers. + const shouldRetry = opts.shouldRetry || (() => true); let lastError; for (let i = 1; i <= attempts; i++) { @@ -105770,6 +105866,9 @@ async function withRetry(step, fn, opts = {}) { return await fn(); } catch (error) { lastError = error; + if (!shouldRetry(error)) { + throw error; + } if (i === attempts) { log.error(`${step}_retry`, { attempt: i, @@ -105867,8 +105966,20 @@ function sortByCreationDate(data) { }); } +// Parse a comma-separated input into a trimmed, non-empty list. A single +// value yields a one-element list, so callers behave identically whether the +// user passed "subnet-a" or "subnet-a,subnet-b". Empty/whitespace entries +// (e.g. trailing commas) are dropped. +function parseCsv(value) { + if (!value) { + return []; + } + return value.split(',').map((v) => v.trim()).filter((v) => v.length > 0); +} + module.exports = { sortByCreationDate, + parseCsv, } @@ -110813,14 +110924,20 @@ const core = __nccwpck_require__(7484); // v1.2.6 still emits the deprecated '::set-output name=X::Y' workflow // command; GitHub runners now surface that as a warning. Bypass the // legacy path — modern runners always set GITHUB_OUTPUT. -function setOutput(label, ec2InstanceId) { +function setOutput(label, placement) { + const { instanceId, instanceType, subnetId } = placement; const outputFile = process.env.GITHUB_OUTPUT; if (outputFile) { - fs.appendFileSync(outputFile, `label=${label}${os.EOL}ec2-instance-id=${ec2InstanceId}${os.EOL}`); + fs.appendFileSync( + outputFile, + `label=${label}${os.EOL}ec2-instance-id=${instanceId}${os.EOL}instance-type-used=${instanceType}${os.EOL}subnet-id-used=${subnetId}${os.EOL}`, + ); return; } core.setOutput('label', label); - core.setOutput('ec2-instance-id', ec2InstanceId); + core.setOutput('ec2-instance-id', instanceId); + core.setOutput('instance-type-used', instanceType); + core.setOutput('subnet-id-used', subnetId); } async function start() { @@ -110829,8 +110946,9 @@ async function start() { log.debug('start_inputs', config.input); // sanitized inside log.js const label = config.generateUniqueLabel(); const githubRegistrationToken = await gh.getRegistrationToken(); - const ec2InstanceId = await aws.startEc2Instance(label, githubRegistrationToken); - setOutput(label, ec2InstanceId); + const placement = await aws.startEc2Instance(label, githubRegistrationToken); + const ec2InstanceId = placement.instanceId; + setOutput(label, placement); await aws.waitForInstanceRunning(ec2InstanceId); // Watch the bootstrap phone-home tag and GitHub registration together. @@ -110847,7 +110965,7 @@ async function start() { await aws.handleStartFailure(ec2InstanceId, { redactValues: [githubRegistrationToken] }); throw waitError; } - log.info('start', { label, instance_id: ec2InstanceId, outcome: 'registered' }); + log.info('start', { label, instance_id: ec2InstanceId, instance_type: placement.instanceType, subnet_id: placement.subnetId, outcome: 'registered' }); } finally { core.endGroup(); } diff --git a/src/aws.js b/src/aws.js index 4653ff20..26a07a1d 100644 --- a/src/aws.js +++ b/src/aws.js @@ -13,9 +13,82 @@ const core = require('@actions/core'); const config = require('./config'); const log = require('./log'); const { withRetry } = require('./retry'); -const { sortByCreationDate } = require('./utils'); +const { sortByCreationDate, parseCsv } = require('./utils'); const checksums = require('./runner-checksums'); +// RunInstances error classification for the capacity-fallback chain. +// - capacity: this placement (type × subnet/AZ) has no capacity right now — +// advance to the next cell in the chain. +// - transient: a retryable API-layer hiccup — retry the same cell. +// - fatal (default): misconfiguration or quota — abort immediately so a +// bad config doesn't burn through the whole matrix. +const CAPACITY_ERROR_CODES = new Set([ + 'InsufficientInstanceCapacity', + 'InsufficientHostCapacity', + 'InsufficientReservedInstanceCapacity', + 'Unsupported', + 'SpotMaxPriceTooLow', // spot capacity (composes with #39) + 'MaxSpotInstanceCountExceeded', +]); +const TRANSIENT_ERROR_CODES = new Set([ + 'RequestLimitExceeded', + 'Throttling', + 'ThrottlingException', + 'InternalError', + 'InternalFailure', + 'ServiceUnavailable', + 'Unavailable', +]); + +function classifyRunError(error) { + const name = error && error.name; + if (CAPACITY_ERROR_CODES.has(name)) { + return 'capacity'; + } + if (TRANSIENT_ERROR_CODES.has(name)) { + return 'transient'; + } + return 'fatal'; +} + +// Walk the instance-type × subnet fallback chain until one placement +// succeeds. Order: for each instance type, try every subnet/AZ before +// downgrading the type (placement is cheaper than a hardware change). On a +// capacity error, advance to the next cell; on a quota or any other fatal +// error, abort immediately (a misconfig must not burn the whole matrix). +// `attempt(instanceType, subnetId)` is injected (returns the instance id or +// throws), keeping the ordering logic testable without the AWS SDK. +async function launchWithFallback(attempt, instanceTypes, subnetIds) { + const failures = []; + for (const instanceType of instanceTypes) { + for (const subnetId of subnetIds) { + try { + const instanceId = await attempt(instanceType, subnetId); + return { instanceId, instanceType, subnetId }; + } catch (error) { + const kind = classifyRunError(error); + if (kind === 'capacity') { + failures.push({ instanceType, subnetId, code: error.name || 'Unknown' }); + log.warn('run_instance_fallback', { instance_type: instanceType, subnet_id: subnetId, error_code: error.name, message: error.message }); + continue; + } + if (error.name === 'InstanceLimitExceeded') { + throw new Error( + `RunInstances hit an account quota (InstanceLimitExceeded) launching ${instanceType} in ${subnetId}: ${error.message}. ` + + 'This is a service limit, not a capacity shortage — request a quota increase. The fallback chain was not continued.', + { cause: error }, + ); + } + throw error; + } + } + } + const summary = failures.map((f) => `${f.instanceType}/${f.subnetId}=${f.code}`).join('; '); + const error = new Error(`All ${failures.length} placement attempt(s) failed due to insufficient capacity: ${summary}`); + error.capacityExhausted = true; + throw error; +} + // Instance tag the bootstrap script writes to phone home its progress. // The start action polls it to fail fast on cloud-init errors instead of // waiting out the full registration timeout. See buildUserData(). @@ -357,13 +430,13 @@ async function startEc2Instance(label, githubRegistrationToken) { const resolved = await resolveImage(client); config.input.ec2ImageId = resolved.id; + // InstanceType and SubnetId are injected per attempt by the fallback + // chain (see below), so they are intentionally absent from the base. const params = { ImageId: config.input.ec2ImageId, - InstanceType: config.input.ec2InstanceType, MinCount: 1, MaxCount: 1, UserData: Buffer.from(userData).toString('base64'), - SubnetId: config.input.subnetId, SecurityGroupIds: [config.input.securityGroupId], IamInstanceProfile: { Name: config.input.iamRoleName }, TagSpecifications: buildTagSpecifications(label, new Date().toISOString()), @@ -406,26 +479,43 @@ async function startEc2Instance(label, githubRegistrationToken) { } } - let ec2InstanceId; - const runStart = Date.now(); + // Capacity fallback: walk instance-type × subnet/AZ in order, advancing + // on capacity errors and retrying only transient API errors within each + // cell. Single-value inputs collapse to a one-cell chain (byte-identical + // to the pre-fallback behavior). + const instanceTypes = parseCsv(config.input.ec2InstanceType); + const subnetIds = parseCsv(config.input.subnetId); log.info('run_instance', { ami_id: config.input.ec2ImageId, - instance_type: config.input.ec2InstanceType, - subnet_id: config.input.subnetId, + instance_types: instanceTypes, + subnet_ids: subnetIds, sg_id: config.input.securityGroupId, iam_role: config.input.iamRoleName || null, label, }); + + const attempt = async (instanceType, subnetId) => { + const attemptParams = { ...params, InstanceType: instanceType, SubnetId: subnetId }; + const result = await withRetry( + 'run_instance', + () => client.send(new RunInstancesCommand(attemptParams)), + { shouldRetry: (error) => classifyRunError(error) === 'transient' }, + ); + return result.Instances[0].InstanceId; + }; + + let placement; + const runStart = Date.now(); try { - const result = await client.send(new RunInstancesCommand(params)); - ec2InstanceId = result.Instances[0].InstanceId; - log.info('run_instance', { instance_id: ec2InstanceId, elapsed_ms: Date.now() - runStart }); - core.info(`AWS EC2 instance ${ec2InstanceId} is started`); + placement = await launchWithFallback(attempt, instanceTypes, subnetIds); } catch (error) { log.error('run_instance', { error: error.name, message: error.message }); core.error('AWS EC2 instance starting error'); throw error; } + const ec2InstanceId = placement.instanceId; + log.info('run_instance', { instance_id: ec2InstanceId, instance_type: placement.instanceType, subnet_id: placement.subnetId, elapsed_ms: Date.now() - runStart }); + core.info(`AWS EC2 instance ${ec2InstanceId} is started (type ${placement.instanceType}, subnet ${placement.subnetId})`); if (config.input.eipAllocationId) { await waitForInstanceRunning(ec2InstanceId); @@ -442,7 +532,7 @@ async function startEc2Instance(label, githubRegistrationToken) { } } - return ec2InstanceId; + return { instanceId: ec2InstanceId, instanceType: placement.instanceType, subnetId: placement.subnetId }; } async function terminateInstanceById(ec2InstanceId) { @@ -619,6 +709,8 @@ module.exports = { handleStartFailure, listManagedInstances, // Exported for unit testing. + classifyRunError, + launchWithFallback, buildRootDeviceMapping, wantsRootDeviceMapping, buildVolumeOpts, diff --git a/src/index.js b/src/index.js index 75ebc585..81eb4e7e 100644 --- a/src/index.js +++ b/src/index.js @@ -27,14 +27,20 @@ const core = require('@actions/core'); // v1.2.6 still emits the deprecated '::set-output name=X::Y' workflow // command; GitHub runners now surface that as a warning. Bypass the // legacy path — modern runners always set GITHUB_OUTPUT. -function setOutput(label, ec2InstanceId) { +function setOutput(label, placement) { + const { instanceId, instanceType, subnetId } = placement; const outputFile = process.env.GITHUB_OUTPUT; if (outputFile) { - fs.appendFileSync(outputFile, `label=${label}${os.EOL}ec2-instance-id=${ec2InstanceId}${os.EOL}`); + fs.appendFileSync( + outputFile, + `label=${label}${os.EOL}ec2-instance-id=${instanceId}${os.EOL}instance-type-used=${instanceType}${os.EOL}subnet-id-used=${subnetId}${os.EOL}`, + ); return; } core.setOutput('label', label); - core.setOutput('ec2-instance-id', ec2InstanceId); + core.setOutput('ec2-instance-id', instanceId); + core.setOutput('instance-type-used', instanceType); + core.setOutput('subnet-id-used', subnetId); } async function start() { @@ -43,8 +49,9 @@ async function start() { log.debug('start_inputs', config.input); // sanitized inside log.js const label = config.generateUniqueLabel(); const githubRegistrationToken = await gh.getRegistrationToken(); - const ec2InstanceId = await aws.startEc2Instance(label, githubRegistrationToken); - setOutput(label, ec2InstanceId); + const placement = await aws.startEc2Instance(label, githubRegistrationToken); + const ec2InstanceId = placement.instanceId; + setOutput(label, placement); await aws.waitForInstanceRunning(ec2InstanceId); // Watch the bootstrap phone-home tag and GitHub registration together. @@ -61,7 +68,7 @@ async function start() { await aws.handleStartFailure(ec2InstanceId, { redactValues: [githubRegistrationToken] }); throw waitError; } - log.info('start', { label, instance_id: ec2InstanceId, outcome: 'registered' }); + log.info('start', { label, instance_id: ec2InstanceId, instance_type: placement.instanceType, subnet_id: placement.subnetId, outcome: 'registered' }); } finally { core.endGroup(); } diff --git a/src/retry.js b/src/retry.js index 2b4d449b..685ad9c1 100644 --- a/src/retry.js +++ b/src/retry.js @@ -11,6 +11,10 @@ async function withRetry(step, fn, opts = {}) { const attempts = opts.attempts || 3; const baseMs = opts.baseMs || 2000; const maxMs = opts.maxMs || 10000; + // Optional predicate — when it returns false the error is re-thrown + // immediately without further retries. Defaults to retrying everything, + // preserving the original behavior for existing callers. + const shouldRetry = opts.shouldRetry || (() => true); let lastError; for (let i = 1; i <= attempts; i++) { @@ -18,6 +22,9 @@ async function withRetry(step, fn, opts = {}) { return await fn(); } catch (error) { lastError = error; + if (!shouldRetry(error)) { + throw error; + } if (i === attempts) { log.error(`${step}_retry`, { attempt: i, diff --git a/src/utils.js b/src/utils.js index 9a3ba781..0f8de412 100644 --- a/src/utils.js +++ b/src/utils.js @@ -17,6 +17,18 @@ function sortByCreationDate(data) { }); } +// Parse a comma-separated input into a trimmed, non-empty list. A single +// value yields a one-element list, so callers behave identically whether the +// user passed "subnet-a" or "subnet-a,subnet-b". Empty/whitespace entries +// (e.g. trailing commas) are dropped. +function parseCsv(value) { + if (!value) { + return []; + } + return value.split(',').map((v) => v.trim()).filter((v) => v.length > 0); +} + module.exports = { sortByCreationDate, + parseCsv, } diff --git a/tests/fallback.test.js b/tests/fallback.test.js new file mode 100644 index 00000000..d87a3708 --- /dev/null +++ b/tests/fallback.test.js @@ -0,0 +1,78 @@ +// Tests for the capacity-fallback chain: classifyRunError and the +// launchWithFallback walker. launchWithFallback takes an injected attempt() +// so the ordering logic is tested without the AWS SDK. config + core are +// mocked because aws.js reaches log.js at require time. +jest.mock('../src/config', () => ({ + input: { mode: 'start', debug: 'false' }, + githubContext: { owner: 'o', repo: 'r' }, +})); +jest.mock('@actions/core', () => ({ + info: jest.fn(), warning: jest.fn(), error: jest.fn(), setFailed: jest.fn(), getInput: jest.fn(), + startGroup: jest.fn(), endGroup: jest.fn(), +})); + +const { classifyRunError, launchWithFallback } = require('../src/aws'); + +const err = (name) => Object.assign(new Error(name), { name }); +const capacity = () => err('InsufficientInstanceCapacity'); + +describe('classifyRunError', () => { + test('capacity codes classify as capacity', () => { + for (const n of ['InsufficientInstanceCapacity', 'InsufficientHostCapacity', 'Unsupported', 'SpotMaxPriceTooLow']) { + expect(classifyRunError(err(n))).toBe('capacity'); + } + }); + + test('transient codes classify as transient', () => { + for (const n of ['RequestLimitExceeded', 'Throttling', 'InternalError', 'ServiceUnavailable']) { + expect(classifyRunError(err(n))).toBe('transient'); + } + }); + + test('everything else — including quota — classifies as fatal', () => { + for (const n of ['InvalidAMIID.NotFound', 'UnauthorizedOperation', 'InstanceLimitExceeded', 'InvalidParameterValue']) { + expect(classifyRunError(err(n))).toBe('fatal'); + } + }); +}); + +describe('launchWithFallback', () => { + test('returns on the first cell when it succeeds (no fallback)', async () => { + const attempt = jest.fn().mockResolvedValue('i-1'); + const result = await launchWithFallback(attempt, ['t1', 't2'], ['s1', 's2']); + expect(result).toEqual({ instanceId: 'i-1', instanceType: 't1', subnetId: 's1' }); + expect(attempt).toHaveBeenCalledTimes(1); + }); + + test('exhausts all subnets for a type before advancing to the next type', async () => { + const attempt = jest.fn() + .mockRejectedValueOnce(capacity()) // t1/s1 + .mockRejectedValueOnce(capacity()) // t1/s2 + .mockResolvedValueOnce('i-9'); // t2/s1 + const result = await launchWithFallback(attempt, ['t1', 't2'], ['s1', 's2']); + expect(result).toEqual({ instanceId: 'i-9', instanceType: 't2', subnetId: 's1' }); + expect(attempt.mock.calls).toEqual([['t1', 's1'], ['t1', 's2'], ['t2', 's1']]); + }); + + test('walks the full matrix in order and throws a capacity summary on exhaustion', async () => { + const attempt = jest.fn().mockRejectedValue(capacity()); + await expect(launchWithFallback(attempt, ['t1', 't2'], ['s1', 's2'])).rejects.toMatchObject({ capacityExhausted: true }); + expect(attempt.mock.calls).toEqual([['t1', 's1'], ['t1', 's2'], ['t2', 's1'], ['t2', 's2']]); + // The summary names every attempted cell + its error code. + await expect(launchWithFallback(jest.fn().mockRejectedValue(capacity()), ['t1'], ['s1'])) + .rejects.toThrow(/t1\/s1=InsufficientInstanceCapacity/); + }); + + test('aborts immediately on a fatal (non-capacity) error', async () => { + const attempt = jest.fn() + .mockRejectedValueOnce(err('InvalidAMIID.NotFound')); + await expect(launchWithFallback(attempt, ['t1', 't2'], ['s1', 's2'])).rejects.toThrow('InvalidAMIID.NotFound'); + expect(attempt).toHaveBeenCalledTimes(1); // no further cells tried + }); + + test('gives a targeted message for a quota error and stops the chain', async () => { + const attempt = jest.fn().mockRejectedValueOnce(err('InstanceLimitExceeded')); + await expect(launchWithFallback(attempt, ['t1', 't2'], ['s1'])).rejects.toThrow(/quota.*InstanceLimitExceeded/); + expect(attempt).toHaveBeenCalledTimes(1); + }); +}); diff --git a/tests/retry.test.js b/tests/retry.test.js index 7b4cedfb..7cded55e 100644 --- a/tests/retry.test.js +++ b/tests/retry.test.js @@ -52,6 +52,26 @@ describe('withRetry', () => { expect(final).toMatchObject({ step: 'test_step_retry', attempt: 3, exhausted: true }); }); + test('shouldRetry:false re-throws immediately without retrying', async () => { + const { withRetry } = load(); + const fatal = Object.assign(new Error('bad config'), { name: 'InvalidAMIID.NotFound' }); + const fn = jest.fn().mockRejectedValue(fatal); + await expect( + withRetry('test_step', fn, { attempts: 3, baseMs: 1, shouldRetry: () => false }), + ).rejects.toThrow('bad config'); + expect(fn).toHaveBeenCalledTimes(1); + expect(coreMock.warning).not.toHaveBeenCalled(); + }); + + test('shouldRetry:true keeps retrying (default behavior preserved)', async () => { + const { withRetry } = load(); + const fn = jest.fn().mockRejectedValueOnce(new Error('t')).mockResolvedValue('ok'); + await expect( + withRetry('test_step', fn, { attempts: 3, baseMs: 1, shouldRetry: () => true }), + ).resolves.toBe('ok'); + expect(fn).toHaveBeenCalledTimes(2); + }); + test('backoff caps at maxMs', async () => { const { withRetry } = load(); const fn = jest.fn() diff --git a/tests/utils.test.js b/tests/utils.test.js index f3f095cc..3abf9e6e 100644 --- a/tests/utils.test.js +++ b/tests/utils.test.js @@ -1,4 +1,23 @@ -const { sortByCreationDate } = require('../src/utils'); +const { sortByCreationDate, parseCsv } = require('../src/utils'); + +describe('parseCsv', () => { + test('splits and trims a comma-separated list', () => { + expect(parseCsv('a, b ,c')).toEqual(['a', 'b', 'c']); + }); + + test('returns a one-element list for a single value (byte-identical path)', () => { + expect(parseCsv('subnet-a')).toEqual(['subnet-a']); + }); + + test('drops empty entries from trailing/double commas', () => { + expect(parseCsv('a,,b,')).toEqual(['a', 'b']); + }); + + test('returns an empty array for empty / undefined input', () => { + expect(parseCsv('')).toEqual([]); + expect(parseCsv(undefined)).toEqual([]); + }); +}); describe('sortByCreationDate', () => { test('sorts images by CreationDate descending (newest first)', () => {