fix: ECS Task needs memory and CPU overrides when invoked from Pipe #171
Conversation
oli-pr
changed the title
|
@antonbabenko @bryantbiggs please could you take another look at this when you have a moment and let me have your thoughts. Thank you |
| @@ -0,0 +1,161 @@ | |||
| provider "aws" { | |||
There was a problem hiding this comment.
why do we need a net new example for this?
There was a problem hiding this comment.
why do we need a net new example for this?
Happy to merge this with the other pipes example but will need to add the ECS cluster and the policy attachment required as the workaround if that works for you?
There was a problem hiding this comment.
you're not explaining the issues very well and instead rushing to solutions. I think this PR is more of a feature request (we should support all of the arguments provided by the underlying resources, so adding them is great) - but you are stating its an issue because you have not:
- Specified CPU/memory on your task definition you are passing in https://github.com/terraform-aws-modules/terraform-aws-eventbridge/pull/171/files#diff-b744c49b4f2afd82f6463977a09297c70645ac52940d5834537fe70afd8623fbR136
- The modules in use are not making any assumptions about a minimum CPU/memory
Which is why you are seeing an error - the EventBridge service is trying to invoke the task but it can't without some idea of what amount of CPU/memory is required
Thats one issue I see (*I think - again, not very well flushed out here)
The other one is that of permissions - its not clear why we are adding permissions in the example. Is this a module deficiency? If so, how have we proven that - the module has a very similar set of permissions so is there a bug somewhere or misconfiguration?
There was a problem hiding this comment.
-
Without this change when the task gets run container overrides were provided which set both the CPU and Mem to zero rendering ECS target on a pipe as unusable. I think you are implying I have a possible usage error, I will go away and check this - sorry if I missed something. This was supposed to be a fix for ECS Pipe Target - ECS Task CPU and Memory are zero and invalid #170
-
In trying to fix this I also had to workaround this issue (to construct the test case) which I don't yet have a fix for, again if this is some kind of usage issue I apologise in advance. Incorrect IAM Policy when using ECS Target for Pipe #169
There was a problem hiding this comment.
If these are usage issues then I will look to see if there is something I can do to improve an example. As it stands I don't consider this a "feature request" but things I have had to put in place in our environment as I was unable to get an ECS target working in Event Bridge pipes without them.
There was a problem hiding this comment.
Looking at the implementation log for this I don't understand currently why this commit was made:
There isn't an associated example with it so I am not 100pc sure but it seems like ECS Task target is not currently usable IMO
|
|
||
| container_definitions = { | ||
| hello-world = { | ||
| image = "public.ecr.aws/docker/library/hello-world:latest", |
There was a problem hiding this comment.
@bryantbiggs The task def does include the CPU and memory settings? Have I misunderstood your concern?
{
"taskDefinitionArn": "arn:aws:ecs:eu-west-1:<REDACTED>:task-definition/hello-world:8",
"containerDefinitions": [
{
"name": "hello-world",
"image": "public.ecr.aws/docker/library/hello-world:latest",
"cpu": 256,
"memory": 512,
"portMappings": [],
"essential": true,
"restartPolicy": {
"enabled": true,
"ignoredExitCodes": []
},
"environment": [],
"mountPoints": [],
"volumesFrom": [],
"linuxParameters": {
"initProcessEnabled": false
},
"startTimeout": 30,
"stopTimeout": 120,
"versionConsistency": "disabled",
"privileged": false,
"readonlyRootFilesystem": true,
"interactive": false,
"pseudoTerminal": false,
"logConfiguration": {
"logDriver": "awslogs",
"options": {
"awslogs-group": "/aws/ecs/hello-world/hello-world",
"awslogs-region": "eu-west-1",
"awslogs-stream-prefix": "ecs"
}
},
"systemControls": []
}
],
"family": "hello-world",
"taskRoleArn": "arn:aws:iam::<REDACTED>:role/hello-world-20250729154228887200000004",
"executionRoleArn": "arn:aws:iam::<REDACTED>:role/hello-world-20250729154228887000000003",
"networkMode": "awsvpc",
"revision": 8,
"volumes": [],
"status": "ACTIVE",
"requiresAttributes": [
{
"name": "com.amazonaws.ecs.capability.logging-driver.awslogs"
},
{
"name": "ecs.capability.execution-role-awslogs"
},
{
"name": "com.amazonaws.ecs.capability.docker-remote-api.1.19"
},
{
"name": "ecs.capability.container-restart-policy"
},
{
"name": "com.amazonaws.ecs.capability.docker-remote-api.1.17"
},
{
"name": "com.amazonaws.ecs.capability.task-iam-role"
},
{
"name": "ecs.capability.container-ordering"
},
{
"name": "com.amazonaws.ecs.capability.docker-remote-api.1.18"
},
{
"name": "ecs.capability.task-eni"
}
],
"placementConstraints": [],
"compatibilities": [
"EC2",
"FARGATE"
],
"requiresCompatibilities": [
"FARGATE"
],
"cpu": "1024",
"memory": "2048",
"runtimePlatform": {
"cpuArchitecture": "X86_64",
"operatingSystemFamily": "LINUX"
},
"registeredAt": "2025-07-29T15:42:30.350Z",
"registeredBy": "<REDACTED>",
"tags": []
}
There was a problem hiding this comment.
@bryantbiggs @antonbabenko please could I ask for a little more of your time on this? I am very happy to re-work the test code to use an existing example if that is a better fit. I still think this is a fix rather than a feature request as I have been unable to get an ECS Target working with pipes as recorded by this issue #170
If I use the existing pipes example I would need to add the ecs cluster necessary to run it and a workaround for this issue #169
If you're happy with that I will make those revisions.
I think on investigation this issue is actually related to this in the provider:
Description
Adds CPU and Memory to task parameters so container overrides does not set them to zero meaning task cannot run.
Motivation and Context
See this issue
Breaking Changes
This may be a breaking change for any pipes that are configured with this module that use ECS Targets but as far as I can tell the functionality is broken.
How Has This Been Tested?
examples/*to demonstrate and validate my change(s)examples/*projectspre-commit run -aon my pull request