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

(aws_ecs_patterns): container-level cpu & memory props for ApplicationLoadBalancedFargateService #20638

Open
1 of 2 tasks
Eryn-mk opened this issue Jun 6, 2022 · 2 comments · May be fixed by #30920
Open
1 of 2 tasks
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@Eryn-mk
Copy link

Eryn-mk commented Jun 6, 2022

Describe the feature

Surface the container-level cpu and container-level memoryLimitMiB property of ContainerDefinitionOptions as part of the ApplicationLoadBalancedFargateService construct.

Currently those two properties are not accessible when creating an ApplicationLoadBalancedFargateService. (see code)

Use Case

I want to deploy a sidecar container to the service container, within the same task definition. Right now the issue I observe is, the service container is set a cpu value that equals to the task cpu value (named cpu in ApplicationLoadBalancedFargateService construct prop), thus I get error because the sum of sidecar container cpu and service container cpu exceeds the task cpu limit.

Proposed Solution

const service = new ApplicationLoadBalancedFargateService(this, 'Service',  {
    cpu: 4096 ,
    memoryLimitMiB: 7168, 
    containerCpu: 2048,
    containerMemoryLimitMiB: 6144,
    ...
}

or maybe passing in an object

const service = new ApplicationLoadBalancedFargateService(this, 'Service',  {
    cpu: 4096 ,
    memoryLimitMiB: 7168, 
    containerDefinitionOptions : {
        containerCpu: 2048,
        containerMemoryLimitMiB: 6144,
        ...
    }
    ...
}

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2

Environment details (OS name and version, etc.)

Linux

@Eryn-mk Eryn-mk added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jun 6, 2022
@github-actions github-actions bot added the @aws-cdk/aws-ecs-patterns Related to ecs-patterns library label Jun 6, 2022
@peterwoodworth peterwoodworth added good first issue Related to contributions. See CONTRIBUTING.md p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jun 10, 2022
@peterwoodworth
Copy link
Contributor

Generally our higher level constructs are supposed to be abstracting away finer details - but we should still offer ways to configure important settings like these without resorting to escape hatches.

We accept contributions. Check out our contributing guide if you're interested - there's a low chance the team will be able to address this soon but we'd be happy to review a PR 🙂

@pahud
Copy link
Contributor

pahud commented Sep 28, 2022

@Eryn-mk

You should be able to define the taskDefinition in the props and add multiple container definitions in the taskDefinition so you don't have to surface the container definitions to the ApplicationLoadBalancedFargateServiceProps.

Check out this sample.

@peterwoodworth I think we can convert this issue to discussion and I will be happy to offer more sample codes if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
4 participants