-
Notifications
You must be signed in to change notification settings - Fork 597
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
The task definition is configured to use a dynamic host port, but the target group has a health check port specified #817
Comments
Interesting. So this looks associated to the ECS Task Definition? So we would have to associate the ECS Task to the ALB being used? Validating that ECS is using Port 0 which means |
So many odd little quirks across the services. This one failed the pipe
line, so I thought why not try an issue. The template might have the info
you need to make a check. Let me know if I can help.
…On Wed, Apr 10, 2019, 8:32 PM Kevin DeJong ***@***.***> wrote:
Interesting. So this looks associated to the ECS Task Definition? So we
would have to associate the ECS Task to the ALB being used? Validating that
ECS is using Port 0 which means HealthCheckPort: 'traffic-port'
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ecs-taskdefinition-containerdefinitions-portmappings.html#cfn-ecs-taskdefinition-containerdefinition-portmappings-hostport
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#817 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABjykKWQkGYwanetjxTsdujk-o1yJ9QCks5vfoKWgaJpZM4cibYA>
.
|
Yes please create issues for this. These are the types of things we are trying to detect. We do have rules that compare values across resources so we should be able to check for this. We would need the ALB resource in the same template to do this work though. |
Of course cdk has the same behaviour. Using "traffic-port", as suggested, fixes the error. ...
healthCheck: {
protocol: elb.Protocol.HTTP,
path: '/healthz',
port: "traffic-port",
healthyThresholdCount: 2,
unhealthyThresholdCount: 2,
interval: cdk.Duration.seconds(10),
timeout: cdk.Duration.seconds(5)
}
... |
This one got me, too. Even though the default HealthCheckPort is 'traffic-port', you have to specify it anyways, or the update fails when using dynamic host ports. Is there any intent to add this check to the standard rules? |
Working on trying to replicate this issue. This doesn't give me an error. I'm wondering if there was a change in how the API works? taskdefinition:
Type: 'AWS::ECS::TaskDefinition'
Properties:
....
PortMappings:
- ContainerPort: 80
HostPort: 0
....
ECSTG:
Type: 'AWS::ElasticLoadBalancingV2::TargetGroup'
DependsOn: ECSALB
Properties:
HealthCheckIntervalSeconds: 10
HealthCheckPath: /
HealthCheckProtocol: HTTP
HealthCheckTimeoutSeconds: 5
HealthyThresholdCount: 2
Name: ECSTG
Port: 80
Protocol: HTTP
UnhealthyThresholdCount: 2
VpcId: !Ref VpcId |
Was able to replicate this by adding |
Also found while investigating is multiple containers in the same task cannot have the same HostPort Container port 80 is used in more than one port mapping for container simple-app (but the task is created fine. This is only a failure when doing the service). Logic looks like matching ContainerName to the ContainerDefinitions. Then if the PortMappings has a matching ContainerPort and HostPort is 0 then we can validate the TargetGroup configuration. |
cfn-lint version: (
cfn-lint --version
)Description of issue.
When updating a stack I found that there is no check for this
Please provide as much information as possible:
working template,
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-elasticloadbalancingv2-targetgroup.html#cfn-elasticloadbalancingv2-targetgroup-healthcheckport
cfn-lint
and/or the latest Spec filesCfn-lint uses the CloudFormation Resource Specifications as the base to do validation. These files are included as part of the application version. Please update to the latest version of
cfn-lint
or update the spec files manually (cfn-lint -u
)The text was updated successfully, but these errors were encountered: