aws-cdk
aws-cdk copied to clipboard
feat(ecs): add `maxSwap` and `swappiness` properties to LinuxParameters
Add support to MaxSwap and Swappiness attributes in the LinuxParameters construct.
Closes #18460
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
Thanks for your review @madeline-k
- Have you deployed an ECS service with these parameters? and does it actually work as expected? I am asking because of these comments: #18460 (comment) and #18460 (comment). If there is some actions users need to take for this to work after deployment, we need to document it. Or, hopefully do it for them in the cfn template.
I deployed a test stack and the parameters were correctly defined in the resulting task definition. However, I had no chance to check if swap memory was available inside the container and I did not figure out how, actually. Maybe I should ssh into the cluster instance and exec a free command on the running test container?
- Please include an integration test.
I developed an integration test, but I have no permissions to create the VPC that I need for the ECS cluster in my company account, thus I cannot run the full integ test. What is the policy here? Should I commit the test without the corresponding .expected.json template? Wouldn't this make integ regression tests fail in CI?
I developed an integration test, but I have no permissions to create the VPC that I need for the ECS cluster in my company account, thus I cannot run the full integ test. What is the policy here? Should I commit the test without the corresponding .expected.json template? Wouldn't this make integ regression tests fail in CI?
You're correct, you can't commit an integration test without the corresponding .expected.json template. Maybe I can help out here. Can you add the integration test to this PR? then I can run it and generate the expected.json template, and I'll push that to your branch.
Ok, I’ll try to post it tomorrow
// packages/@aws-cdk/aws-ecs/test/ec2/integ.swap-parameters.ts
import * as ec2 from '@aws-cdk/aws-ec2';
import * as cdk from '@aws-cdk/core';
import * as ecs from '../../lib';
import { LinuxParameters } from '../../lib';
const app = new cdk.App();
const stack = new cdk.Stack(app, 'aws-ecs-integ');
const vpc = new ec2.Vpc(stack, 'Vpc', { maxAzs: 2 });
// ECS cluster to host EC2 task
const cluster = new ecs.Cluster(stack, 'EcsCluster', { vpc });
cluster.addCapacity('DefaultAutoScalingGroup', {
instanceType: new ec2.InstanceType('t2.micro'),
});
// define task to run the container
const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDefinition', {
networkMode: ecs.NetworkMode.AWS_VPC,
});
// define linux parameters to enable swap
const linuxParameters = new LinuxParameters(stack, 'LinuxParameters', {
maxSwap: 5e3,
swappiness: 90,
});
// define container with linux parameters
new ecs.ContainerDefinition(stack, 'Container', {
image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
linuxParameters,
memoryLimitMiB: 256,
taskDefinition,
});
// define a service to run the task definition
new ecs.Ec2Service(stack, 'Service', {
cluster,
taskDefinition,
});
app.synth();
@flavioleggio Apologies for the long wait on this. I've run the test that you wrote and pushed the result to an expected file. Please take a look and ensure that this matches what you expect to see as output. Go ahead and assign this to me as a reviewer if everything looks good to you and the checks all pass.
@flavioleggio Well, this is my bad. I didn't notice that the integration test was the old style and not the new. We'll need to update to the new style. I'll take care of that and run them for the expected output within the next week.
@Mergifyio update
update
✅ Branch has been successfully updated
@Mergifyio update
update
✅ Branch has been successfully updated
@mrgrain Can you take another look? I've re-updated the tests.
This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.
Hi @TheRealAmazonKendra
I updated the integration test snapshot with the new version and fulfilled your request on the parameter type. I think this is ready for approval.
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).
AWS CodeBuild CI Report
- CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
- Commit ID: 3b2fb8c6384b63cd399eb6e187ab42ceac431116
- Result: SUCCEEDED
- Build Logs (available for 30 days)
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).