Skip to content

Commit a50f0d5

Browse files
edersonbrilhanteedersonbrilhante
committed
refactor: remove custom friom variable and add default values
1 parent 9fb2553 commit a50f0d5

File tree

21 files changed

+74
-73
lines changed

21 files changed

+74
-73
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ Join our discord community via [this invite link](https://discord.gg/bxgXW8jJGh)
119119
| <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 |
120120
| <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 |
121121
| <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 |
122-
| <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 |
122+
| <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 |
123123
| <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 |
124124
| <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 |
125125
| <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
@@ -44,5 +44,5 @@ export interface RunnerInputParameters {
4444
amiIdSsmParameterName?: string;
4545
tracingEnabled?: boolean;
4646
onDemandFailoverOnError?: string[];
47-
customScaleErrors?: string[];
47+
scaleErrors: string[];
4848
}

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

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,7 @@ describe('create runner with errors', () => {
429429
allocationStrategy: SpotAllocationStrategy.CAPACITY_OPTIMIZED,
430430
capacityType: 'spot',
431431
type: 'Repo',
432+
scaleErrors: ['UnfulfillableCapacity', 'MaxSpotInstanceCountExceeded'],
432433
};
433434
const defaultExpectedFleetRequestValues: ExpectedFleetRequestValues = {
434435
type: 'Repo',
@@ -457,19 +458,6 @@ describe('create runner with errors', () => {
457458
expect(mockSSMClient).not.toHaveReceivedCommand(PutParameterCommand);
458459
});
459460

460-
it('test ScaleError with custom scale error.', async () => {
461-
createFleetMockWithErrors(['CustomAWSError']);
462-
463-
await expect(
464-
createRunner(createRunnerConfig({ ...defaultRunnerConfig, customScaleErrors: ['CustomAWSError'] })),
465-
).rejects.toBeInstanceOf(ScaleError);
466-
expect(mockEC2Client).toHaveReceivedCommandWith(
467-
CreateFleetCommand,
468-
expectedCreateFleetRequest(defaultExpectedFleetRequestValues),
469-
);
470-
expect(mockSSMClient).not.toHaveReceivedCommand(PutParameterCommand);
471-
});
472-
473461
it('test ScaleError with multiple error.', async () => {
474462
createFleetMockWithErrors(['UnfulfillableCapacity', 'MaxSpotInstanceCountExceeded', 'NotMappedError']);
475463

@@ -712,7 +700,7 @@ interface RunnerConfig {
712700
amiIdSsmParameterName?: string;
713701
tracingEnabled?: boolean;
714702
onDemandFailoverOnError?: string[];
715-
customScaleErrors?: string[];
703+
scaleErrors: string[];
716704
}
717705

718706
function createRunnerConfig(runnerConfig: RunnerConfig): RunnerInputParameters {
@@ -732,7 +720,7 @@ function createRunnerConfig(runnerConfig: RunnerConfig): RunnerInputParameters {
732720
amiIdSsmParameterName: runnerConfig.amiIdSsmParameterName,
733721
tracingEnabled: runnerConfig.tracingEnabled,
734722
onDemandFailoverOnError: runnerConfig.onDemandFailoverOnError,
735-
customScaleErrors: runnerConfig.customScaleErrors,
723+
scaleErrors: runnerConfig.scaleErrors,
736724
};
737725
}
738726

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

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

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

217201
const failedCount = countScaleErrors(errors, scaleErrors);
218202
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.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,8 @@ beforeEach(() => {
140140
process.env.INSTANCE_TARGET_CAPACITY_TYPE = 'spot';
141141
process.env.RUNNER_OWNER = ORG;
142142
process.env.RUNNER_BOOT_TIME_IN_MINUTES = MINIMUM_TIME_RUNNING.toString();
143+
process.env.SCALE_ERRORS =
144+
'["UnfulfillableCapacity","MaxSpotInstanceCountExceeded","TargetCapacityLimitExceededException"]';
143145

144146
const mockTokenReturnValue = {
145147
data: {

‎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

0 commit comments

Comments
 (0)