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.FargateTaskDefinition: pidmode Task unsupported #29995

Closed
vduits opened this issue Apr 29, 2024 · 2 comments · Fixed by #30020
Closed

aws_ecs.FargateTaskDefinition: pidmode Task unsupported #29995

vduits opened this issue Apr 29, 2024 · 2 comments · Fixed by #30020
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@vduits
Copy link

vduits commented Apr 29, 2024

Describe the bug

Reading the pidmode announcement here, the cloudformation description for pidMode here. It explains task is the only option. But currently the FargateTaskDefinition only allows host.

Changing this to host will result in a failed deployment, because host isn't allowed on Fargate (linux) tasks.

Expected Behavior

Consistency between documentation and usable options reflected by synth/deployments in CDK.
Task is the one that is the only one actually supported.

Current Behavior

Error: 'pidMode' can only be set to 'host' for Fargate containers, got: 'task'.
at new FargateTaskDefinition (C:\projects\push-gateway-cdk\node_modules\aws-cdk-lib\aws-ecs\lib\fargate\fargate-task-definition.js:1:2562)

But using the host variant will results in:

Deployment failed: Error: The stack named XXXX failed to deploy: UPDATE_ROLLBACK_COMPLETE: Resource handler returned message: "Invalid request provided: Create TaskDefinition: Tasks using the Fargate launch type do not support pidMode 'host'. The supported value for pidMode is 'task'.

Reproduction Steps

const ftdTask = new ecs.FargateTaskDefinition(stack, `task-that-will-fail-to-synth`, {
    cpu: 1024,
    memoryLimitMiB: 2048,
    pidMode: PidMode.TASK,
});
const ftdHost = new ecs.FargateTaskDefinition(stack, `task-that-will-fail-to-deploy`, {
    cpu: 1024,
    memoryLimitMiB: 2048,
    pidMode: PidMode.HOST,
});

Possible Solution

Switch the checks from pidMode.HOST to pidMode.TASK.

Additional Information/Context

No response

CDK CLI Version

2.139.0

Framework Version

No response

Node.js Version

v20.12.0

OS

windows

Language

TypeScript

Language Version

No response

Other information

(OS is windows for where I ran CDK, the containers are linux as those are the only one allowing pidMode anyway)

@vduits vduits added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 29, 2024
@github-actions github-actions bot added the @aws-cdk/aws-ecs Related to Amazon Elastic Container label Apr 29, 2024
@pahud
Copy link
Contributor

pahud commented Apr 29, 2024

yap, looks like we should remove that check.

Are you able to get it deployed using this workaround?

    const task = new ecs.FargateTaskDefinition(this, `task-that-will-fail-to-deploy`, {
      cpu: 1024,
      memoryLimitMiB: 2048,
      pidMode: ecs.PidMode.HOST,
    });
    (task.node.defaultChild as ecs.CfnTaskDefinition).addPropertyOverride('PidMode', 'task');

We just need to make sure removing that check would not cause other issues.

If it works great with you, feel free to submit a tiny PR for this. Thank you.

@pahud pahud added p2 effort/medium Medium work item – several days of effort effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. effort/medium Medium work item – several days of effort labels Apr 29, 2024
@mergify mergify bot closed this as completed in #30020 May 10, 2024
mergify bot pushed a commit that referenced this issue May 10, 2024
#30020)

### Issue # (if applicable)

Closes #29995.

### Reason for this change

Only the `task` option is allowed for [`pidMode`](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ecs-taskdefinition.html#cfn-ecs-taskdefinition-pidmode) on Linux-based Fargate tasks.

### Description of changes

This PR builds on the changes introduced in #29670 but fixes the handling of `pidMode` so that it matches the behavior allowed by CloudFormation and described in the [AWS User Guide](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ecs-taskdefinition.html#cfn-ecs-taskdefinition-pidmode).

### Description of how you validated changes

Updated the existing tests so that `task` is the only allowable `pidMode` setting if a Fargate task's OS is Linux-based.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container bug This issue is a bug. effort/small Small work item – less than a day of effort p2
Projects
None yet
2 participants