Skip to content

Commit dc6e8d8

Browse files
edersonbrilhanteedersonbrilhante
committed
refactor: remove custom friom variable and add default values
1 parent 3f3981c commit dc6e8d8

19 files changed

Lines changed: 70 additions & 72 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ Join our discord community via [this invite link](https://discord.gg/bxgXW8jJGh)
117117
| <a name="input_block_device_mappings"></a> [block\_device\_mappings](#input\_block\_device\_mappings) | The EC2 instance block device configuration. Takes the following keys: `device_name`, `delete_on_termination`, `volume_type`, `volume_size`, `encrypted`, `iops`, `throughput`, `kms_key_id`, `snapshot_id`. | <pre>list(object({<br/> delete_on_termination = optional(bool, true)<br/> device_name = optional(string, "/dev/xvda")<br/> encrypted = optional(bool, true)<br/> iops = optional(number)<br/> kms_key_id = optional(string)<br/> snapshot_id = optional(string)<br/> throughput = optional(number)<br/> volume_size = number<br/> volume_type = optional(string, "gp3")<br/> }))</pre> | <pre>[<br/> {<br/> "volume_size": 30<br/> }<br/>]</pre> | no |
118118
| <a name="input_cloudwatch_config"></a> [cloudwatch\_config](#input\_cloudwatch\_config) | (optional) Replaces the module's default cloudwatch log config. See https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch-Agent-Configuration-File-Details.html for details. | `string` | `null` | no |
119119
| <a name="input_create_service_linked_role_spot"></a> [create\_service\_linked\_role\_spot](#input\_create\_service\_linked\_role\_spot) | (optional) create the service linked role for spot instances that is required by the scale-up lambda. | `bool` | `false` | no |
120-
| <a name="input_custom_scale_errors"></a> [custom\_scale\_errors](#input\_custom\_scale\_errors) | List of aws error codesthat should trigger retry during scale up. This list will replace the default errors defined in the variable `defaultScaleErrors` in https://github.com/github-aws-runners/terraform-aws-github-runner/blob/main/lambdas/functions/control-plane/src/aws/runners.ts | `list(string)` | `[]` | no |
120+
| <a name="input_scale_errors"></a> [custom\_scale\_errors](#input\_custom\_scale\_errors) | List of aws error codesthat should trigger retry during scale up. This list will replace the default errors defined in the variable `defaultScaleErrors` in https://github.com/github-aws-runners/terraform-aws-github-runner/blob/main/lambdas/functions/control-plane/src/aws/runners.ts | `list(string)` | `[]` | no |
121121
| <a name="input_delay_webhook_event"></a> [delay\_webhook\_event](#input\_delay\_webhook\_event) | The number of seconds the event accepted by the webhook is invisible on the queue before the scale up lambda will receive the event. | `number` | `30` | no |
122122
| <a name="input_disable_runner_autoupdate"></a> [disable\_runner\_autoupdate](#input\_disable\_runner\_autoupdate) | Disable the auto update of the github runner agent. Be aware there is a grace period of 30 days, see also the [GitHub article](https://github.blog/changelog/2022-02-01-github-actions-self-hosted-runners-can-now-disable-automatic-updates/) | `bool` | `false` | no |
123123
| <a name="input_enable_ami_housekeeper"></a> [enable\_ami\_housekeeper](#input\_enable\_ami\_housekeeper) | Option to disable the lambda to clean up old AMIs. | `bool` | `false` | no |

‎lambdas/functions/control-plane/src/aws/runners.d.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,5 +45,5 @@ export interface RunnerInputParameters {
4545
amiIdSsmParameterName?: string;
4646
tracingEnabled?: boolean;
4747
onDemandFailoverOnError?: string[];
48-
customScaleErrors?: string[];
48+
scaleErrors: string[];
4949
}

‎lambdas/functions/control-plane/src/aws/runners.test.ts‎

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -461,19 +461,6 @@ describe('create runner with errors', () => {
461461
expect(mockSSMClient).not.toHaveReceivedCommand(PutParameterCommand);
462462
});
463463

464-
it('test ScaleError with custom scale error.', async () => {
465-
createFleetMockWithErrors(['CustomAWSError']);
466-
467-
await expect(
468-
createRunner(createRunnerConfig({ ...defaultRunnerConfig, customScaleErrors: ['CustomAWSError'] })),
469-
).rejects.toBeInstanceOf(ScaleError);
470-
expect(mockEC2Client).toHaveReceivedCommandWith(
471-
CreateFleetCommand,
472-
expectedCreateFleetRequest(defaultExpectedFleetRequestValues),
473-
);
474-
expect(mockSSMClient).not.toHaveReceivedCommand(PutParameterCommand);
475-
});
476-
477464
it('test ScaleError with multiple error.', async () => {
478465
createFleetMockWithErrors(['UnfulfillableCapacity', 'MaxSpotInstanceCountExceeded', 'NotMappedError']);
479466

@@ -716,7 +703,7 @@ interface RunnerConfig {
716703
amiIdSsmParameterName?: string;
717704
tracingEnabled?: boolean;
718705
onDemandFailoverOnError?: string[];
719-
customScaleErrors?: string[];
706+
scaleErrors: string[];
720707
}
721708

722709
function createRunnerConfig(runnerConfig: RunnerConfig): RunnerInputParameters {
@@ -736,7 +723,7 @@ function createRunnerConfig(runnerConfig: RunnerConfig): RunnerInputParameters {
736723
amiIdSsmParameterName: runnerConfig.amiIdSsmParameterName,
737724
tracingEnabled: runnerConfig.tracingEnabled,
738725
onDemandFailoverOnError: runnerConfig.onDemandFailoverOnError,
739-
customScaleErrors: runnerConfig.customScaleErrors,
726+
scaleErrors: runnerConfig.scaleErrors,
740727
};
741728
}
742729

‎lambdas/functions/control-plane/src/aws/runners.ts‎

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -197,23 +197,7 @@ async function processFleetResult(
197197
return instances;
198198
}
199199

200-
// Educated guess of errors that would make sense to retry based on the list
201-
// https://docs.aws.amazon.com/AWSEC2/latest/APIReference/errors-overview.html
202-
const defaultScaleErrors = [
203-
'UnfulfillableCapacity',
204-
'MaxSpotInstanceCountExceeded',
205-
'TargetCapacityLimitExceededException',
206-
'RequestLimitExceeded',
207-
'ResourceLimitExceeded',
208-
'MaxSpotInstanceCountExceeded',
209-
'MaxSpotFleetRequestCountExceeded',
210-
'InsufficientInstanceCapacity',
211-
];
212-
213-
const scaleErrors =
214-
runnerParameters.customScaleErrors && runnerParameters.customScaleErrors.length > 0
215-
? runnerParameters.customScaleErrors
216-
: defaultScaleErrors;
200+
const scaleErrors = runnerParameters.scaleErrors;
217201

218202
const failedCount = countScaleErrors(errors, scaleErrors);
219203
if (failedCount > 0) {

‎lambdas/functions/control-plane/src/modules.d.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ declare namespace NodeJS {
33
AWS_REGION: string;
44
ENABLE_METRIC_GITHUB_APP_RATE_LIMIT: string;
55
ENABLE_ON_DEMAND_FAILOVER_FOR_ERRORS: string;
6-
CUSTOM_SCALE_ERRORS: string;
6+
SCALE_ERRORS: string;
77
ENVIRONMENT: string;
88
GHES_URL: string;
99
JOB_RETRY_CONFIG: string;

‎lambdas/functions/control-plane/src/pool/pool.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,7 @@ export async function adjust(event: PoolEvent): Promise<void> {
4141
const onDemandFailoverOnError = process.env.ENABLE_ON_DEMAND_FAILOVER_FOR_ERRORS
4242
? (JSON.parse(process.env.ENABLE_ON_DEMAND_FAILOVER_FOR_ERRORS) as [string])
4343
: [];
44-
const customScaleErrors = process.env.CUSTOM_SCALE_ERRORS
45-
? (JSON.parse(process.env.CUSTOM_SCALE_ERRORS) as [string])
46-
: [];
44+
const scaleErrors = JSON.parse(process.env.SCALE_ERRORS) as [string];
4745

4846
const { ghesApiUrl, ghesBaseUrl } = getGitHubEnterpriseApiUrl();
4947

@@ -98,7 +96,7 @@ export async function adjust(event: PoolEvent): Promise<void> {
9896
amiIdSsmParameterName,
9997
tracingEnabled,
10098
onDemandFailoverOnError,
101-
customScaleErrors,
99+
scaleErrors,
102100
},
103101
topUp,
104102
githubInstallationClient,

‎lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ const EXPECTED_RUNNER_PARAMS: RunnerInputParameters = {
105105
subnets: ['subnet-123'],
106106
tracingEnabled: false,
107107
onDemandFailoverOnError: [],
108-
customScaleErrors: [],
108+
scaleErrors: ['UnfulfillableCapacity', 'MaxSpotInstanceCountExceeded', 'TargetCapacityLimitExceededException'],
109109
};
110110
let expectedRunnerParams: RunnerInputParameters;
111111

@@ -123,7 +123,8 @@ function setDefaults() {
123123
process.env.INSTANCE_TYPES = 'm5.large';
124124
process.env.INSTANCE_TARGET_CAPACITY_TYPE = 'spot';
125125
process.env.ENABLE_ON_DEMAND_FAILOVER = undefined;
126-
process.env.CUSTOM_SCALE_ERRORS = undefined;
126+
process.env.SCALE_ERRORS =
127+
'["UnfulfillableCapacity","MaxSpotInstanceCountExceeded","TargetCapacityLimitExceededException"]';
127128
}
128129

129130
beforeEach(() => {
@@ -813,11 +814,11 @@ describe('scaleUp with public GH', () => {
813814

814815
it('creates a runner with correct config and labels and custom scale errors enabled.', async () => {
815816
process.env.RUNNER_LABELS = 'label1,label2';
816-
process.env.CUSTOM_SCALE_ERRORS = JSON.stringify(['RequestLimitExceeded']);
817+
process.env.SCALE_ERRORS = JSON.stringify(['RequestLimitExceeded']);
817818
await scaleUpModule.scaleUp(TEST_DATA);
818819
expect(createRunner).toBeCalledWith({
819820
...expectedRunnerParams,
820-
customScaleErrors: ['RequestLimitExceeded'],
821+
scaleErrors: ['RequestLimitExceeded'],
821822
});
822823
});
823824

‎lambdas/functions/control-plane/src/scale-runners/scale-up.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ interface CreateEC2RunnerConfig {
6262
amiIdSsmParameterName?: string;
6363
tracingEnabled?: boolean;
6464
onDemandFailoverOnError?: string[];
65-
customScaleErrors?: string[];
65+
scaleErrors: string[];
6666
}
6767

6868
function generateRunnerServiceConfig(githubRunnerConfig: CreateGitHubRunnerConfig, token: string) {
@@ -256,9 +256,7 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise<stri
256256
const onDemandFailoverOnError = process.env.ENABLE_ON_DEMAND_FAILOVER_FOR_ERRORS
257257
? (JSON.parse(process.env.ENABLE_ON_DEMAND_FAILOVER_FOR_ERRORS) as [string])
258258
: [];
259-
const customScaleErrors = process.env.CUSTOM_SCALE_ERRORS
260-
? (JSON.parse(process.env.CUSTOM_SCALE_ERRORS) as [string])
261-
: [];
259+
const scaleErrors = JSON.parse(process.env.SCALE_ERRORS) as [string];
262260

263261
const { ghesApiUrl, ghesBaseUrl } = getGitHubEnterpriseApiUrl();
264262

@@ -435,7 +433,7 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise<stri
435433
amiIdSsmParameterName,
436434
tracingEnabled,
437435
onDemandFailoverOnError,
438-
customScaleErrors,
436+
scaleErrors,
439437
},
440438
newRunners,
441439
githubInstallationClient,

‎main.tf‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ module "runners" {
187187
enable_jit_config = var.enable_jit_config
188188
enable_job_queued_check = var.enable_job_queued_check
189189
enable_on_demand_failover_for_errors = var.enable_runner_on_demand_failover_for_errors
190-
custom_scale_errors = var.custom_scale_errors
190+
scale_errors = var.scale_errors
191191
disable_runner_autoupdate = var.disable_runner_autoupdate
192192
enable_managed_runner_security_group = var.enable_managed_runner_security_group
193193
enable_runner_detailed_monitoring = var.enable_runner_detailed_monitoring

‎modules/multi-runner/runners.tf‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ module "runners" {
3232
github_app_parameters = local.github_app_parameters
3333
ebs_optimized = each.value.runner_config.ebs_optimized
3434
enable_on_demand_failover_for_errors = each.value.runner_config.enable_on_demand_failover_for_errors
35-
custom_scale_errors = each.value.runner_config.custom_scale_errors
35+
scale_errors = each.value.runner_config.scale_errors
3636
enable_organization_runners = each.value.runner_config.enable_organization_runners
3737
enable_ephemeral_runners = each.value.runner_config.enable_ephemeral_runners
3838
enable_jit_config = each.value.runner_config.enable_jit_config

0 commit comments

Comments
 (0)