Update NLB Health check parameters to match docs, allow NLB un/healthy counts to be different #733
Conversation
…y counts to be different
|
| @@ -261,57 +261,6 @@ def set_new_resource_outputs(self, output_definition): | |||
| return value | |||
|
|
|||
|
|
|||
| def validate_tcp_health_counts(props): | |||
There was a problem hiding this comment.
Why are we removing this function? Is it totally unused and I should have deleted it long time ago?
There was a problem hiding this comment.
https://github.com/compose-x/ecs_composex/blob/main/ecs_composex/elbv2/elbv2_ecs.py#L312
validate_tcp_health_counts is only ever used inside function fix_nlb_settings which we're also removing below (cuz NLB doesn't need those NLB-specific validations for its Healthcheck params)
There was a problem hiding this comment.
Have you tried then to have HealthCheckIntervalSeconds set 6 or 7 then and did it work? (by work, I mean successful deployment to AWS via CFN)
The point of these validations / fix functions is to catch incorrect input and correct it with best effort.
I must have missed that news on the change of health check possible for NLBs/ALBs.
There was a problem hiding this comment.
@louismollick any insignts on this? did you try creating the NLB with invalid interval values for example? if so, what happened?
thank you
There was a problem hiding this comment.
@JohnPreston Hey! Sorry for the radio silence, I got sidetracked
I edited and deployed the nginx example from your repo to show the NLB values:
main...louismollick:ecs_composex:elbv2ParamsFixDemo
healthcheck: 80:TCP:2:10:300:120
^^ here the HealthCheckIntervalSeconds is 300 and HealthCheckTimeoutSeconds is 120
I deployed from this branch using poetry run ecs-compose-x up -f docker-compose.yaml -f aws-compose-x.yaml -n testNLB
Below are some screenshots from the results in AWS, it even shows these values when you go to edit them in Target Group healthcheck settings:
Also attached the template here:
template.json
There was a problem hiding this comment.
Thanks so much. There I thought I was keeping up with all the changes getting the announcement news but there we go, that's some nice changes AWS did!
Thanks @louismollick
Fixes #732
I haven't tested this change ; but seems like an expected change given NLB was updated in November 2022:
https://aws.amazon.com/about-aws/whats-new/2022/11/elastic-load-balancing-capabilities-application-availability/
For example the CDK had to update their validations too:
aws/aws-cdk#26023
New values are as described in the docs:
https://docs.aws.amazon.com/elasticloadbalancing/latest/network/target-group-health-checks.html