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.

Sign up for GitHub

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

Jump to bottom

aws_elasticloadbalancingv2: Incorrect statement that NLB health check healthy/unhealthy counts need to be the same #32365

Closed
vcschapp opened this issue Dec 2, 2024 · 2 comments · Fixed by #32387
Closed

aws_elasticloadbalancingv2: Incorrect statement that NLB health check healthy/unhealthy counts need to be the same #32365

vcschapp opened this issue Dec 2, 2024 · 2 comments · Fixed by #32387
Labels
@aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 documentation This is a problem with documentation. effort/small Small work item – less than a day of effort p2

Comments

@vcschapp
Copy link

vcschapp commented Dec 2, 2024 •
edited
Loading

Describe the issue

The documentation for the unhealthyThresholdCount member of the ELBv2 HealthCheck interface says that for NLBs the unhealthy threshold count must be the same as the healthy threshold count:

unhealthyThresholdCount?

Type: number (optional, default: 2)

The number of consecutive health check failures required before considering a target unhealthy.

For Application Load Balancers, the default is 2. For Network Load Balancers, this value must be the same as the healthy threshold count.

This does not appear to be accurate.

  • When I create an NLB target group manually in the AWS EC2 console and configure the health check manually, I can set the healthy/unhealthy counts to different values.
  • There is no reference to this limitation in the CloudFormation documentation for AWS::ElasticLoadBalancingV2::TargetGroup or any of its member attributes.
  • There is no attempt to validate this constraint in the CDK TypeScript code.

image

Links

  1. Impacted CDK documentation page: https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_elasticloadbalancingv2.HealthCheck.html#unhealthythresholdcount
  2. Documentation source in GitHub. https://github.com/aws/aws-cdk/blob/v2.170.0/packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/shared/base-target-group.ts#L135-L136
  3. Relevant CloudFormation documentation: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-elasticloadbalancingv2-targetgroup.html#cfn-elasticloadbalancingv2-targetgroup-unhealthythresholdcount

CDK Version

Last checked in documentation for v2.170.0.
@vcschapp vcschapp added documentation This is a problem with documentation. needs-triage This issue or PR still needs to be triaged. labels Dec 2, 2024
@github-actions github-actions bot added the @aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 label Dec 2, 2024
@khushail khushail added investigating This issue is being investigated and/or work is in progress to resolve the issue. p2 and removed needs-triage This issue or PR still needs to be triaged. labels Dec 3, 2024
@khushail khushail self-assigned this Dec 3, 2024
@khushail
Copy link
Contributor

khushail commented Dec 5, 2024

Hi @vcschapp , thanks for reporting this.

I see that on the AWS Docs page, both the range is mentioned as 2-10 while the defaults differ in case of both UnhealthyThresholdCount and HealthyThresholdCount (https://docs.aws.amazon.com/elasticloadbalancing/latest/network/target-group-health-checks.html) -

Screenshot 2024-12-04 at 5 20 08 PM

and the same is mentioned in Cloudformation docs as well -https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-elasticloadbalancingv2-targetgroup.html#cfn-elasticloadbalancingv2-targetgroup-healthythresholdcount

So it might be misleading to write such statement in CDK doc and should be corrected -

aws-cdk/packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/shared/base-target-group.ts

Line 136 in a95560c

* Balancers, this value must be the same as the healthy threshold count.

@khushail khushail added effort/small Small work item – less than a day of effort and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Dec 5, 2024
@khushail khushail removed their assignment Dec 5, 2024
@khushail khushail mentioned this issue Dec 5, 2024
Merged
1 task
@mergify mergify bot closed this as completed in #32387 Dec 9, 2024
mergify bot pushed a commit that referenced this issue Dec 9, 2024
@khushail
docs(elasticloadbalancingv2): removed statement reg unhealthyThreshol…
453045b 
…dCount (#32387)

### Issue # (if applicable)

Closes #32365 .

### Reason for this change

There is no mention of `unHealthyThresholdCount` being the same with `healthyThresholdCount` in case of NLB in ELBV2 as stated in given docs - 

- cloudformation docs - https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-elasticloadbalancingv2-targetgroup.html#cfn-elasticloadbalancingv2-targetgroup-healthythresholdcount
- AWS Docs -https://docs.aws.amazon.com/elasticloadbalancing/latest/network/target-group-health-checks.html


### Description of changes

removed the in-line comment


### Checklist
- [ ] 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*
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 9, 2024

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
@aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 documentation This is a problem with documentation. effort/small Small work item – less than a day of effort p2
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

docs(elasticloadbalancingv2): removed statement reg unhealthyThresholdCount aws/aws-cdk
2 participants
@vcschapp @khushail