feat(ecs): add maxSwap and swappiness properties to LinuxParameters

[flavioleggio]

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

[gitpod-io]

[madeline-k]

Thanks for making this contribution, @flavioleggio!

Some requests:

1. Please add validation somewhere for the restrictions of maxSwap and swappiness - e.g. min/max values, if swappiness is supplied but maxSwap is not then swapping won't occur, these values can't be used with Fargate tasks
2. Have you deployed an ECS service with these parameters? and does it actually work as expected? I am asking because of these comments: aws-ecs: Support MaxSwap and Swappiness in LinuxParameters #18460 (comment) and aws-ecs: Support MaxSwap and Swappiness in LinuxParameters #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.
3. Please include an integration test.

Pull request has been modified.

[flavioleggio]

Thanks for your review @madeline-k

2. 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?

3. 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?

[madeline-k]

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.

[flavioleggio]

Ok, I’ll try to post it tomorrow

[flavioleggio]

// 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();

[TheRealAmazonKendra]

@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.

[TheRealAmazonKendra]

@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.

[TheRealAmazonKendra]

@Mergifyio update

[mrgrain]

@TheRealAmazonKendra Looks good now, I think we need the Token changes and then the PR is ready.

[madeline-k]

I only have one requested change here for this PR.

Changing types is a breaking change, so we need that added before the PR is merged.
In an ideal world, I think there would be more validation for this - error message when maxSwap is omitted, but swappiness is provided, and verify that these params can only be used when EC2 launch type is used. But we can add more validation in future PRs. I wouldn't hold this one for that.

Pull request has been modified.

[TheRealAmazonKendra]

@Mergifyio update

[mergify]

update

✅ Branch has been successfully updated

[TheRealAmazonKendra]

@mrgrain Can you take another look? I've re-updated the tests.

[TheRealAmazonKendra]

I didn't notice the update in here to sharedMemorySize. We can't actually do that because it's a breaking change.

[aws-cdk-automation]

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.

Pull request has been modified.

[flavioleggio]

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.

[TheRealAmazonKendra]

Looks great! Thanks for all your work on this!

[mergify]

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-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 3b2fb8c
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

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).