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-cdk/aws-ecs-patterns: Add ability to set healthCheck for QueueProcessingFargateService #15636

Closed
paya-cz opened this issue Jul 18, 2021 · 4 comments · Fixed by #18219
Closed
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library effort/small Small work item – less than a day of effort feature/enhancement A new API to make things easier or more intuitive. A catch-all for general feature requests. feature-request A feature should be added or improved. p2

Comments

@paya-cz
Copy link

paya-cz commented Jul 18, 2021

QueueProcessingFargateService currently does not offer healthCheck options property, even though it is very simple to add it here:

this.taskDefinition.addContainer(containerName, {

So, there is no good way to set a healthCheck on a container.

Use Case

I use healthcheck to make sure the tasks are healthy. Fargate ignores HEALTHCHECK instruction in Dockerfile, so I am forced to supply the healthCheck instruction via CDK. However, QueueProcessingFargateService does not let me.

Proposed Solution

Add a healthcheck property here:

export interface QueueProcessingFargateServiceProps extends QueueProcessingServiceBaseProps {

Pass it to the container here:

this.taskDefinition.addContainer(containerName, {

Workaround

This is not nice, but works.

// @ts-ignore
service.taskDefinition.defaultContainer!.props.healthCheck = <ecs.HealthCheck>{
    command: ['CMD', 'node /app/healthcheck.js'],
    interval: cdk.Duration.seconds(15),
    retries: 3,
    timeout: cdk.Duration.seconds(5),
};

This is a 🚀 Feature Request

@paya-cz paya-cz added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jul 18, 2021
@github-actions github-actions bot added the @aws-cdk/aws-ecs-patterns Related to ecs-patterns library label Jul 18, 2021
@paya-cz
Copy link
Author

paya-cz commented Jul 23, 2021

Same problem for ScheduledFargateTask:

this.taskDefinition.addContainer('ScheduledContainer', {

@peterwoodworth
Copy link
Contributor

Hey @paya-cz, sorry for the slow response

Would you like to submit a PR for this feature request yourself? I can't guarantee anyone from the team will be able to tackle this anytime soon

@peterwoodworth peterwoodworth added feature/enhancement A new API to make things easier or more intuitive. A catch-all for general feature requests. and removed needs-triage This issue or PR still needs to be triaged. labels Sep 20, 2021
@peterwoodworth peterwoodworth added p2 effort/small Small work item – less than a day of effort labels Sep 20, 2021
@paya-cz
Copy link
Author

paya-cz commented Oct 5, 2021

This issue seems to be somewhat related to #8320, perhaps both issues could be tackled at the same time one day.

@mergify mergify bot closed this as completed in #18219 Jan 25, 2022
mergify bot pushed a commit that referenced this issue Jan 25, 2022
…reating QueueProcessingFargateService (#18219)

The `QueueProcessingFargateService` construct internally creates a service Container with the default healthcheck that cannot currently be overriden by the user. This change allows users to optionally provide a custom healthcheck that can be passed on to the internal ECS container definition.

fixes #15636

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

LukvonStrom pushed a commit to LukvonStrom/aws-cdk that referenced this issue Jan 26, 2022
…reating QueueProcessingFargateService (aws#18219)

The `QueueProcessingFargateService` construct internally creates a service Container with the default healthcheck that cannot currently be overriden by the user. This change allows users to optionally provide a custom healthcheck that can be passed on to the internal ECS container definition.

fixes aws#15636

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
…reating QueueProcessingFargateService (aws#18219)

The `QueueProcessingFargateService` construct internally creates a service Container with the default healthcheck that cannot currently be overriden by the user. This change allows users to optionally provide a custom healthcheck that can be passed on to the internal ECS container definition.

fixes aws#15636

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
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/enhancement A new API to make things easier or more intuitive. A catch-all for general feature requests. feature-request A feature should be added or improved. p2
Projects
None yet
5 participants