diff --git a/README.md b/README.md index f097d3ebd2..dde6d0f6fd 100644 --- a/README.md +++ b/README.md @@ -228,7 +228,7 @@ Join our discord community via [this invite link](https://discord.gg/bxgXW8jJGh) | [scale\_up\_reserved\_concurrent\_executions](#input\_scale\_up\_reserved\_concurrent\_executions) | Amount of reserved concurrent executions for the scale-up lambda function. A value of 0 disables lambda from being triggered and -1 removes any concurrency limitations. | `number` | `1` | no | | [ssm\_paths](#input\_ssm\_paths) | The root path used in SSM to store configuration and secrets. |
object({
root = optional(string, "github-action-runners")
app = optional(string, "app")
runners = optional(string, "runners")
webhook = optional(string, "webhook")
use_prefix = optional(bool, true)
}) | `{}` | no |
| [state\_event\_rule\_binaries\_syncer](#input\_state\_event\_rule\_binaries\_syncer) | Option to disable EventBridge Lambda trigger for the binary syncer, useful to stop automatic updates of binary distribution | `string` | `"ENABLED"` | no |
-| [subnet\_ids](#input\_subnet\_ids) | List of subnets in which the action runner instances will be launched. The subnets need to exist in the configured VPC (`vpc_id`), and must reside in different availability zones (see https://github.com/github-aws-runners/terraform-aws-github-runner/issues/2904) | `list(string)` | n/a | yes |
+| [subnet\_ids](#input\_subnet\_ids) | List of subnets in which the action runner instances will be launched. The subnets must exist in the configured VPC (`vpc_id`). | `list(string)` | n/a | yes |
| [syncer\_lambda\_s3\_key](#input\_syncer\_lambda\_s3\_key) | S3 key for syncer lambda function. Required if using an S3 bucket to specify lambdas. | `string` | `null` | no |
| [syncer\_lambda\_s3\_object\_version](#input\_syncer\_lambda\_s3\_object\_version) | S3 object version for syncer lambda function. Useful if S3 versioning is enabled on source bucket. | `string` | `null` | no |
| [tags](#input\_tags) | Map of tags that will be added to created resources. By default resources will be tagged with name and environment. | `map(string)` | `{}` | no |
diff --git a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runners.test.ts b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runners.test.ts
index 738c6da13d..b52d75fe37 100644
--- a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runners.test.ts
+++ b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runners.test.ts
@@ -9,6 +9,8 @@ import {
DeleteTagsCommand,
DescribeInstancesCommand,
type DescribeInstancesResult,
+ DescribeSubnetsCommand,
+ type DescribeSubnetsResult,
EC2Client,
FleetOnDemandAllocationStrategy,
RunInstancesCommand,
@@ -77,6 +79,18 @@ const mockRunningInstancesJit: DescribeInstancesResult = {
},
],
};
+const mockDefaultSubnets: DescribeSubnetsResult = {
+ Subnets: [
+ { SubnetId: 'subnet-123', AvailabilityZoneId: 'euw1-az1' },
+ { SubnetId: 'subnet-456', AvailabilityZoneId: 'euw1-az2' },
+ ],
+};
+
+function getSubnetIdsFromFleetRequest(request: CreateFleetCommandInput): string[] {
+ const overrides = request.LaunchTemplateConfigs?.[0].Overrides ?? [];
+ const subnetIds = overrides.flatMap(({ SubnetId }) => (SubnetId ? [SubnetId] : []));
+ return [...new Set(subnetIds)];
+}
describe('list instances', () => {
beforeEach(() => {
@@ -338,6 +352,7 @@ describe('create runner', () => {
mockEC2Client.reset();
mockSSMClient.reset();
+ mockEC2Client.on(DescribeSubnetsCommand).resolves(mockDefaultSubnets);
mockEC2Client.on(CreateFleetCommand).resolves({ Instances: [{ InstanceIds: ['i-1234'] }] });
mockSSMClient.on(GetParameterCommand).resolves({});
});
@@ -345,6 +360,10 @@ describe('create runner', () => {
it.each(RUNNER_TYPES)('calls create fleet of 1 instance with the default config for %p', async (type: RunnerType) => {
await createRunner(createRunnerConfig({ ...defaultRunnerConfig, type: type }));
+ expect(mockEC2Client).toHaveReceivedCommandWith(DescribeSubnetsCommand, {
+ SubnetIds: ['subnet-123', 'subnet-456'],
+ });
+ expect(mockEC2Client).toHaveReceivedCommandTimes(CreateFleetCommand, 1);
expect(mockEC2Client).toHaveReceivedCommandWith(CreateFleetCommand, {
...expectedCreateFleetRequest({
...defaultExpectedFleetRequestValues,
@@ -353,6 +372,71 @@ describe('create runner', () => {
});
});
+ it('partitions batch capacity across subnet sets that contain at most one subnet per Availability Zone', async () => {
+ mockEC2Client.on(DescribeSubnetsCommand).resolves({
+ Subnets: [
+ { SubnetId: 'subnet-123', AvailabilityZoneId: 'euw1-az1' },
+ { SubnetId: 'subnet-456', AvailabilityZoneId: 'euw1-az1' },
+ { SubnetId: 'subnet-789', AvailabilityZoneId: 'euw1-az2' },
+ ],
+ });
+ mockEC2Client
+ .on(CreateFleetCommand)
+ .resolvesOnce({ Instances: [{ InstanceIds: ['i-1234'] }] })
+ .resolvesOnce({ Instances: [{ InstanceIds: ['i-5678'] }] });
+
+ const result = await createRunner({
+ ...createRunnerConfig(defaultRunnerConfig),
+ numberOfRunners: 2,
+ subnets: ['subnet-123', 'subnet-456', 'subnet-789'],
+ });
+
+ expect(result).toEqual({
+ instances: ['i-1234', 'i-5678'],
+ retryableErrorCount: 0,
+ nonRetryableErrorCount: 0,
+ });
+ expect(mockEC2Client).toHaveReceivedCommandTimes(CreateFleetCommand, 2);
+ const fleetRequests = mockEC2Client.commandCalls(CreateFleetCommand).map(({ args: [command] }) => command.input);
+ const subnetSets = fleetRequests.map(getSubnetIdsFromFleetRequest);
+ expect(subnetSets.map((subnets) => subnets.sort()).sort()).toEqual(
+ [
+ ['subnet-123', 'subnet-789'],
+ ['subnet-456', 'subnet-789'],
+ ].sort(),
+ );
+ });
+
+ it('carries retryable subnet address failures to the next same-AZ subnet set', async () => {
+ mockEC2Client.on(DescribeSubnetsCommand).resolves({
+ Subnets: [
+ { SubnetId: 'subnet-123', AvailabilityZoneId: 'euw1-az1' },
+ { SubnetId: 'subnet-456', AvailabilityZoneId: 'euw1-az1' },
+ ],
+ });
+ mockEC2Client
+ .on(CreateFleetCommand)
+ .resolvesOnce({ Errors: [{ ErrorCode: 'InsufficientFreeAddressesInSubnet' }] })
+ .resolvesOnce({ Instances: [{ InstanceIds: ['i-1234'] }] });
+
+ const result = await createRunner(
+ createRunnerConfig({
+ ...defaultRunnerConfig,
+ scaleErrors: [...defaultRunnerConfig.scaleErrors, 'InsufficientFreeAddressesInSubnet'],
+ }),
+ );
+
+ expect(result).toEqual({
+ instances: ['i-1234'],
+ retryableErrorCount: 0,
+ nonRetryableErrorCount: 0,
+ });
+ expect(mockEC2Client).toHaveReceivedCommandTimes(CreateFleetCommand, 2);
+ const fleetRequests = mockEC2Client.commandCalls(CreateFleetCommand).map(({ args: [command] }) => command.input);
+ const attemptedSubnets = fleetRequests.flatMap(getSubnetIdsFromFleetRequest).sort();
+ expect(attemptedSubnets).toEqual(['subnet-123', 'subnet-456']);
+ });
+
it('calls create fleet of 2 instances with the correct config for org ', async () => {
const instances = [{ InstanceIds: ['i-1234', 'i-5678'] }];
@@ -545,6 +629,7 @@ describe('create runner', () => {
},
});
+ expect(mockEC2Client).not.toHaveReceivedCommand(DescribeSubnetsCommand);
expect(mockEC2Client).toHaveReceivedCommandWith(CreateFleetCommand, {
LaunchTemplateConfigs: [
{
@@ -767,6 +852,7 @@ describe('create runner with errors', () => {
mockEC2Client.reset();
mockSSMClient.reset();
+ mockEC2Client.on(DescribeSubnetsCommand).resolves(mockDefaultSubnets);
mockSSMClient.on(PutParameterCommand).resolves({});
mockSSMClient.on(GetParameterCommand).resolves({});
mockEC2Client.on(CreateFleetCommand).resolves({ Instances: [] });
@@ -1013,6 +1099,7 @@ describe('create runner with errors fail over to OnDemand', () => {
mockEC2Client.reset();
mockSSMClient.reset();
+ mockEC2Client.on(DescribeSubnetsCommand).resolves(mockDefaultSubnets);
mockSSMClient.on(PutParameterCommand).resolves({});
mockSSMClient.on(GetParameterCommand).resolves({});
mockEC2Client.on(CreateFleetCommand).resolves({ Instances: [] });
@@ -1032,7 +1119,7 @@ describe('create runner with errors fail over to OnDemand', () => {
expect(mockEC2Client).toHaveReceivedCommandTimes(CreateFleetCommand, 2);
// first call with spot failure
- expect(mockEC2Client).toHaveReceivedNthCommandWith(1, CreateFleetCommand, {
+ expect(mockEC2Client).toHaveReceivedNthSpecificCommandWith(1, CreateFleetCommand, {
...expectedCreateFleetRequest({
...defaultExpectedFleetRequestValues,
totalTargetCapacity: 1,
@@ -1041,7 +1128,7 @@ describe('create runner with errors fail over to OnDemand', () => {
});
// second call with with OnDemand fallback, allocation strategy defaults to lowest-price
- expect(mockEC2Client).toHaveReceivedNthCommandWith(2, CreateFleetCommand, {
+ expect(mockEC2Client).toHaveReceivedNthSpecificCommandWith(2, CreateFleetCommand, {
...expectedCreateFleetRequest({
...defaultExpectedFleetRequestValues,
totalTargetCapacity: 1,
@@ -1079,7 +1166,7 @@ describe('create runner with errors fail over to OnDemand', () => {
expect(mockEC2Client).toHaveReceivedCommandTimes(CreateFleetCommand, 2);
// first call with spot failure
- expect(mockEC2Client).toHaveReceivedNthCommandWith(1, CreateFleetCommand, {
+ expect(mockEC2Client).toHaveReceivedNthSpecificCommandWith(1, CreateFleetCommand, {
...expectedCreateFleetRequest({
...defaultExpectedFleetRequestValues,
totalTargetCapacity: 2,
@@ -1088,7 +1175,7 @@ describe('create runner with errors fail over to OnDemand', () => {
});
// second call with with OnDemand failback, capacity is reduced by 1, allocation strategy defaults to lowest-price
- expect(mockEC2Client).toHaveReceivedNthCommandWith(2, CreateFleetCommand, {
+ expect(mockEC2Client).toHaveReceivedNthSpecificCommandWith(2, CreateFleetCommand, {
...expectedCreateFleetRequest({
...defaultExpectedFleetRequestValues,
totalTargetCapacity: 1,
@@ -1113,7 +1200,7 @@ describe('create runner with errors fail over to OnDemand', () => {
expect(mockEC2Client).toHaveReceivedCommandTimes(CreateFleetCommand, 1);
// first call with spot failure
- expect(mockEC2Client).toHaveReceivedNthCommandWith(1, CreateFleetCommand, {
+ expect(mockEC2Client).toHaveReceivedNthSpecificCommandWith(1, CreateFleetCommand, {
...expectedCreateFleetRequest({
...defaultExpectedFleetRequestValues,
totalTargetCapacity: 2,
@@ -1328,6 +1415,7 @@ describe('create runner with useDedicatedHost', () => {
mockEC2Client.reset();
mockSSMClient.reset();
+ mockEC2Client.on(DescribeSubnetsCommand).resolves(mockDefaultSubnets);
mockEC2Client.on(RunInstancesCommand).resolves({
Instances: [{ InstanceId: 'i-dedicated-1' }],
});
@@ -1344,6 +1432,7 @@ describe('create runner with useDedicatedHost', () => {
});
expect(mockEC2Client).toHaveReceivedCommand(RunInstancesCommand);
expect(mockEC2Client).not.toHaveReceivedCommand(CreateFleetCommand);
+ expect(mockEC2Client).not.toHaveReceivedCommand(DescribeSubnetsCommand);
});
it('uses CreateFleet when useDedicatedHost is false', async () => {
diff --git a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runners.ts b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runners.ts
index 050804cce1..cb446751e6 100644
--- a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runners.ts
+++ b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runners.ts
@@ -6,6 +6,7 @@ import {
DeleteTagsCommand,
DescribeInstancesCommand,
DescribeInstancesResult,
+ DescribeSubnetsCommand,
RunInstancesCommand,
type RunInstancesCommandInput,
RunInstancesCommandOutput,
@@ -32,6 +33,11 @@ interface Ec2Filter {
Values: string[];
}
+interface SubnetAllocation {
+ subnets: string[];
+ targetCapacity: number;
+}
+
type FleetError = NonNullableobject({
root = optional(string, "github-action-runners")
app = optional(string, "app")
runners = optional(string, "runners")
webhook = optional(string, "webhook")
}) | `{}` | no |
| [state\_event\_rule\_binaries\_syncer](#input\_state\_event\_rule\_binaries\_syncer) | Option to disable EventBridge Lambda trigger for the binary syncer, useful to stop automatic updates of binary distribution | `string` | `"ENABLED"` | no |
-| [subnet\_ids](#input\_subnet\_ids) | List of subnets in which the action runners will be launched, the subnets needs to be subnets in the `vpc_id`. | `list(string)` | n/a | yes |
+| [subnet\_ids](#input\_subnet\_ids) | List of subnets in which the action runner instances will be launched. The subnets must exist in the configured VPC (`vpc_id`). | `list(string)` | n/a | yes |
| [syncer\_lambda\_s3\_key](#input\_syncer\_lambda\_s3\_key) | S3 key for syncer lambda function. Required if using S3 bucket to specify lambdas. | `string` | `null` | no |
| [syncer\_lambda\_s3\_object\_version](#input\_syncer\_lambda\_s3\_object\_version) | S3 object version for syncer lambda function. Useful if S3 versioning is enabled on source bucket. | `string` | `null` | no |
| [tags](#input\_tags) | Map of tags that will be added to created resources. By default resources will be tagged with name and environment. | `map(string)` | `{}` | no |
diff --git a/modules/multi-runner/variables.tf b/modules/multi-runner/variables.tf
index a47cd2a83c..64cf0aae93 100644
--- a/modules/multi-runner/variables.tf
+++ b/modules/multi-runner/variables.tf
@@ -589,7 +589,7 @@ variable "vpc_id" {
}
variable "subnet_ids" {
- description = "List of subnets in which the action runners will be launched, the subnets needs to be subnets in the `vpc_id`."
+ description = "List of subnets in which the action runner instances will be launched. The subnets must exist in the configured VPC (`vpc_id`)."
type = list(string)
}
diff --git a/modules/runners/README.md b/modules/runners/README.md
index d228615edd..429c4e6c0c 100644
--- a/modules/runners/README.md
+++ b/modules/runners/README.md
@@ -232,7 +232,7 @@ yarn run dist
| [sqs\_build\_queue](#input\_sqs\_build\_queue) | SQS queue to consume accepted build events. | object({
arn = string
url = string
}) | n/a | yes |
| [ssm\_housekeeper](#input\_ssm\_housekeeper) | Configuration for the SSM housekeeper lambda. This lambda deletes token / JIT config from SSM.object({
schedule_expression = optional(string, "rate(1 day)")
state = optional(string, "ENABLED")
lambda_memory_size = optional(number, 512)
lambda_timeout = optional(number, 60)
config = object({
tokenPath = optional(string)
minimumDaysOld = optional(number, 1)
dryRun = optional(bool, false)
})
}) | {
"config": {}
} | no |
| [ssm\_paths](#input\_ssm\_paths) | The root path used in SSM to store configuration and secrets. | object({
root = string
tokens = string
config = string
}) | n/a | yes |
-| [subnet\_ids](#input\_subnet\_ids) | List of subnets in which the action runners will be launched, the subnets needs to be subnets in the `vpc_id`. | `list(string)` | n/a | yes |
+| [subnet\_ids](#input\_subnet\_ids) | List of subnets in which the action runner instances will be launched. The subnets must exist in the configured VPC (`vpc_id`). | `list(string)` | n/a | yes |
| [tags](#input\_tags) | Map of tags that will be added to created resources. By default resources will be tagged with name. | `map(string)` | `{}` | no |
| [tracing\_config](#input\_tracing\_config) | Configuration for lambda tracing. | object({
mode = optional(string, null)
capture_http_requests = optional(bool, false)
capture_error = optional(bool, false)
}) | `{}` | no |
| [use\_dedicated\_host](#input\_use\_dedicated\_host) | Experimental! Can be removed / changed without trigger a major release. Whether to use EC2 dedicated hosts for the runners. Needed for macos runners Note that using dedicated hosts can increase cost significantly. | `bool` | `false` | no |
diff --git a/modules/runners/policies/lambda-scale-up.json b/modules/runners/policies/lambda-scale-up.json
index 851ecc34f5..a12ee66183 100644
--- a/modules/runners/policies/lambda-scale-up.json
+++ b/modules/runners/policies/lambda-scale-up.json
@@ -5,6 +5,7 @@
"Effect": "Allow",
"Action": [
"ec2:DescribeInstances",
+ "ec2:DescribeSubnets",
"ec2:DescribeLaunchTemplateVersions",
"ec2:DescribeTags",
"ec2:RunInstances",
diff --git a/modules/runners/pool/policies/lambda-pool.json b/modules/runners/pool/policies/lambda-pool.json
index 51afd73b50..5b16a2ebf4 100644
--- a/modules/runners/pool/policies/lambda-pool.json
+++ b/modules/runners/pool/policies/lambda-pool.json
@@ -5,6 +5,7 @@
"Effect": "Allow",
"Action": [
"ec2:DescribeInstances",
+ "ec2:DescribeSubnets",
"ec2:DescribeTags",
"ec2:RunInstances",
"ec2:CreateFleet",
diff --git a/modules/runners/variables.tf b/modules/runners/variables.tf
index 946f9abf30..97561472dc 100644
--- a/modules/runners/variables.tf
+++ b/modules/runners/variables.tf
@@ -31,7 +31,7 @@ variable "vpc_id" {
}
variable "subnet_ids" {
- description = "List of subnets in which the action runners will be launched, the subnets needs to be subnets in the `vpc_id`."
+ description = "List of subnets in which the action runner instances will be launched. The subnets must exist in the configured VPC (`vpc_id`)."
type = list(string)
}
diff --git a/variables.tf b/variables.tf
index c4e1e9b5cf..2de4182ed6 100644
--- a/variables.tf
+++ b/variables.tf
@@ -9,7 +9,7 @@ variable "vpc_id" {
}
variable "subnet_ids" {
- description = "List of subnets in which the action runner instances will be launched. The subnets need to exist in the configured VPC (`vpc_id`), and must reside in different availability zones (see https://github.com/github-aws-runners/terraform-aws-github-runner/issues/2904)"
+ description = "List of subnets in which the action runner instances will be launched. The subnets must exist in the configured VPC (`vpc_id`)."
type = list(string)
}