feat(ecs-service-extensions): Target tracking policies for Service Extensions#17101
Conversation
|
|
packages/@aws-cdk-containers/ecs-service-extensions/lib/extensions/http-load-balancer.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk-containers/ecs-service-extensions/lib/extensions/http-load-balancer.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk-containers/ecs-service-extensions/lib/service.ts
Outdated
Show resolved
Hide resolved
| }); | ||
|
|
||
| // THEN | ||
| expect(stack).toHaveResourceLike('AWS::ApplicationAutoScaling::ScalingPolicy', { |
There was a problem hiding this comment.
Can we somehow print that maxTaskCount is 5 here?
There was a problem hiding this comment.
Agreed probably can't tell from this resource but the other ones like ecs service
There was a problem hiding this comment.
Right, I meant to say can we have an expect statement to print that maxTaskCount is 5
There was a problem hiding this comment.
Hmm the unit test is usually meant to check if the CFN template we generate is expected, instead of if the construct itself. maxTaskCount is a field of the construct.
There was a problem hiding this comment.
Yes I didn't include a check for maxCapacity of the ScalableTarget here because we configure it in the service construct but it makes sense to add a check for it here as well! Will update!
| new Service(stack, 'my-service', { | ||
| environment, | ||
| serviceDescription, | ||
| autoScaleTaskCount: { |
There was a problem hiding this comment.
What happens if we send autoScaleTaskCount: {}
There was a problem hiding this comment.
maxTaskCount is a required field. If autoScaleTaskCount is undefined then we error out i guess? https://github.com/aws/aws-cdk/pull/17101/files#diff-77a32c641e0209aa8836c64a4f58132a141ad27eab31be695d247e8f4015a8a0R135
There was a problem hiding this comment.
But does it consider {} as undefined is the question
There was a problem hiding this comment.
{} is not undefined and it will be invalid input and not pass the static check i think
There was a problem hiding this comment.
Hmm that makes sense 👍🏼
paragbhingre
left a comment
There was a problem hiding this comment.
Looks great to me, just have couple of questions. 🎉
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…tensions (#17101) ---- This PR adds `desiredCount`, `targetCpuUtilization` and `targetMemoryUtilization` to the service construct. It also adds `requestsPerTarget` to the `HttpLoadBalancerExtension` props to allow adding target tracking policy based on the ALB request count. It will be followed by another PR to configure queue auto scaling for the SQS Queues in the `QueueExtension`. *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…ension (#17802) ---- This PR deprecates one of the existing `scaleOnCpuUtilization` extension. We recommend users to configure task auto scaling using the [`autoScaleTaskCount`](https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk-containers/ecs-service-extensions/lib/service.ts#L61) in the `Service` construct. Related PR: #17101 *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…tensions (aws#17101) ---- This PR adds `desiredCount`, `targetCpuUtilization` and `targetMemoryUtilization` to the service construct. It also adds `requestsPerTarget` to the `HttpLoadBalancerExtension` props to allow adding target tracking policy based on the ALB request count. It will be followed by another PR to configure queue auto scaling for the SQS Queues in the `QueueExtension`. *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…ension (aws#17802) ---- This PR deprecates one of the existing `scaleOnCpuUtilization` extension. We recommend users to configure task auto scaling using the [`autoScaleTaskCount`](https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk-containers/ecs-service-extensions/lib/service.ts#L61) in the `Service` construct. Related PR: aws#17101 *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This PR adds
desiredCount,targetCpuUtilizationandtargetMemoryUtilizationto the service construct. It also addsrequestsPerTargetto theHttpLoadBalancerExtensionprops to allow adding target tracking policy based on the ALB request count.It will be followed by another PR to configure queue auto scaling for the SQS Queues in the
QueueExtension.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license