Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(batch): ephemeralStorage property on job definitions #25399

Merged
merged 31 commits into from
Jun 20, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
fe9d5ce
add ephemeralStorage property and unit test
sumupitchayan May 2, 2023
3854e10
add ephemeralStorage in README
sumupitchayan May 2, 2023
81efde2
add new prop to integ test
sumupitchayan May 2, 2023
0ec9174
Merge branch 'main' into sumughan/batch-add-ephemeralstorage-prop
sumupitchayan May 3, 2023
2af1f44
add new prop to IEcsContainerDefinition
sumupitchayan May 3, 2023
22e06ab
move ephemeral storage to Fargate class
sumupitchayan May 4, 2023
5d51fb2
add ephemeralStorage to CloudFormation and fix test format
sumupitchayan May 4, 2023
3d07bd1
update integ test snapshots
sumupitchayan May 4, 2023
1699e71
remove README typo
sumupitchayan May 4, 2023
c023e0e
remove test comments
sumupitchayan May 4, 2023
b71a742
Merge branch 'main' into sumughan/batch-add-ephemeralstorage-prop
sumupitchayan May 4, 2023
f95a4c8
Merge branch 'main' into sumughan/batch-add-ephemeralstorage-prop
sumupitchayan May 5, 2023
84c5040
Update packages/@aws-cdk/aws-batch-alpha/lib/ecs-container-definition.ts
sumupitchayan May 5, 2023
2719d0c
Update packages/@aws-cdk/aws-batch-alpha/lib/ecs-container-definition.ts
sumupitchayan May 5, 2023
cdc7006
Update packages/@aws-cdk/aws-batch-alpha/test/ecs-container-definitio…
sumupitchayan May 5, 2023
bdfb89a
rename to ephemeralStorageSize
sumupitchayan May 8, 2023
03b220c
finish renaming to ephemeralStorageSize
sumupitchayan May 8, 2023
8218d4c
add validation and unit test
sumupitchayan May 8, 2023
58b2ce5
make prop undefined by default
sumupitchayan May 8, 2023
c8023c3
refactor code for breaking change, moving executionRole property to E…
sumupitchayan May 9, 2023
c2e5e1d
Merge branch 'main' into sumughan/batch-add-ephemeralstorage-prop
sumupitchayan May 9, 2023
d341a1f
Kaizen suggestion - writing 20 GiB for default
sumupitchayan May 9, 2023
06e4dab
kaizen suggestion: simplify test case and use throw error('') instead…
sumupitchayan May 9, 2023
d36b2b3
add test adding ephemeralStorage as a Token
sumupitchayan May 9, 2023
06529b6
Merge branch 'main' into sumughan/batch-add-ephemeralstorage-prop
sumupitchayan May 9, 2023
72ace6e
fix calvin formatting suggestions
sumupitchayan Jun 8, 2023
62728cd
Update packages/@aws-cdk/aws-batch-alpha/lib/ecs-container-definition.ts
sumupitchayan Jun 8, 2023
b0c672b
Update packages/@aws-cdk/aws-batch-alpha/lib/ecs-container-definition.ts
sumupitchayan Jun 8, 2023
ef9727d
fix build failure by adding quotes within error expected error msg
sumupitchayan Jun 20, 2023
b025fbd
kaizen suggestion, remove creating new app in unit test
sumupitchayan Jun 20, 2023
97fded2
Merge branch 'main' into sumughan/batch-add-ephemeralstorage-prop
mergify[bot] Jun 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -935,8 +935,12 @@ export class EcsFargateContainerDefinition extends EcsContainerDefinitionBase im
this.ephemeralStorageSize = props.ephemeralStorageSize;

// validates ephemeralStorageSize is within limits
if (props.ephemeralStorageSize && (props.ephemeralStorageSize.toGibibytes() > 200 || props.ephemeralStorageSize.toGibibytes() < 21)) {
throw new Error('Ephemeral Storage Size must be within 21 to 200 GiB.');
if (props.ephemeralStorageSize) {
if (props.ephemeralStorageSize.toGibibytes() > 200) {
throw new Error(`ECS Fargate container ${id} specifies ephemeralStorageSize at ${props.ephemeralStorageSize.toGibibytes()} > 200 GB`);
sumupitchayan marked this conversation as resolved.
Show resolved Hide resolved
} else if (props.ephemeralStorageSize.toGibibytes() < 21) {
throw new Error(`ECS Fargate container ${id} specifies ephemeralStorageSize at ${props.ephemeralStorageSize.toGibibytes()} < 21 GB`);
sumupitchayan marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ const defaultExpectedProps: CfnJobDefinitionProps = {
},
};

let app: App;
let stack: Stack;
let pascalCaseExpectedProps: any;

Expand Down Expand Up @@ -760,7 +759,7 @@ describe('EC2 containers', () => {
describe('Fargate containers', () => {
// GIVEN
beforeEach(() => {
app = new App();
const app = new App();
stack = new Stack(app);
pascalCaseExpectedProps = capitalizePropertyNames(stack, defaultExpectedProps);
});
Expand Down Expand Up @@ -855,14 +854,14 @@ describe('Fargate containers', () => {
fargatePlatformVersion: ecs.FargatePlatformVersion.LATEST,
ephemeralStorageSize: Size.gibibytes(19),
}),
})).toThrow('Ephemeral Storage Size must be within 21 to 200 GiB.');
})).toThrow('ECS Fargate container EcsFargateContainer specifies ephemeralStorageSize at 19 < 21 GB');

expect(() => new EcsJobDefinition(stack, 'ECSJobDefn2', {
container: new EcsFargateContainerDefinition(stack, 'EcsFargateContainer2', {
...defaultContainerProps,
fargatePlatformVersion: ecs.FargatePlatformVersion.LATEST,
ephemeralStorageSize: Size.gibibytes(201),
}),
})).toThrow('Ephemeral Storage Size must be within 21 to 200 GiB.');
})).toThrow('ECS Fargate container EcsFargateContainer2 specifies ephemeralStorageSize at 201 > 200 GB');
});
});