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

NLB: NetworkTargetGroup does not depend on NetworkListener #1139

Closed
inoutch opened this issue Nov 10, 2018 · 4 comments · Fixed by #1146
Closed

NLB: NetworkTargetGroup does not depend on NetworkListener #1139

inoutch opened this issue Nov 10, 2018 · 4 comments · Fixed by #1146
Labels
bug This issue is a bug.

Comments

@inoutch
Copy link

inoutch commented Nov 10, 2018

I tried to deploy NLB with ECS Fargate, but I couldn't deploy Service of ECS because the following reason.

The target group with targetGroupArn arn:aws:elasticloadbalancing:ap-northeast-1:*****:targetgroup/*****/***** does not have an associated load balancer. (Service: AmazonECS; Status Code: 400; Error Code: InvalidParameterException; Request ID: *****)

I found that NetworkTargetGroup does not to depend on NetworkListener.
In addition, I found not to call targetGroup.registerListener(this) in only NLB.

https://github.com/awslabs/aws-cdk/blob/6f2569fd51e7b6152830d40b146ff5f2d607e4c2/packages/%40aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-listener.ts#L69
https://github.com/awslabs/aws-cdk/blob/6f2569fd51e7b6152830d40b146ff5f2d607e4c2/packages/%40aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-listener.ts#L60
https://github.com/awslabs/aws-cdk/blob/6f2569fd51e7b6152830d40b146ff5f2d607e4c2/packages/%40aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-listener.ts#L38-L43

I think it should call this registerListener function as the same way as ALB.

https://github.com/awslabs/aws-cdk/blob/6f2569fd51e7b6152830d40b146ff5f2d607e4c2/packages/%40aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener.ts#L252-L255

Is this bug?, or is there any reasons in only NLB?

@inoutch inoutch changed the title [NLB] NetworkTargetGroup does not depend on NetworkListener NLB: NetworkTargetGroup does not depend on NetworkListener Nov 10, 2018
@rix0rrr
Copy link
Contributor

rix0rrr commented Nov 10, 2018

That's a bug. Thanks

@rix0rrr rix0rrr added bug This issue is a bug. and removed bug This issue is a bug. labels Nov 11, 2018
@rix0rrr
Copy link
Contributor

rix0rrr commented Nov 11, 2018

Ooh, I totally misunderstood what the error report was about.

Could you show some of your code please?

@rix0rrr
Copy link
Contributor

rix0rrr commented Nov 11, 2018

@rix0rrr rix0rrr added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Nov 11, 2018
@inoutch
Copy link
Author

inoutch commented Nov 12, 2018

My source code where the error occurred is general constitution using NetworkLoadBalancer. I think the same error occurs replacing ALB with NLB in the test(integ.lb-awsvpc-nw) you pasted.

I think there is a cause in addTargetGroups(called by addTargets) method of NetworkListener class. In the addTargetGroups, _addDefaultTargetGroup is called, but I think that targetGroup.registerListener method should also be called together. By calling the registerListener method, NetworkTargetGroup depends on NetworkListener. This implementation is only in ALB, not in NLB. Therefore ALB succeed.

It is another small problem, but also I report that unhealthyThresholdCount of TargetGroup properties is not reflected.

Sorry for my bad english.

@rix0rrr rix0rrr added bug This issue is a bug. and removed response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Nov 12, 2018
rix0rrr added a commit that referenced this issue Nov 12, 2018
Fix taking a listener dependency for Network Load Balancers.

Fixes #1139.
rix0rrr added a commit that referenced this issue Nov 12, 2018
Fix taking a listener dependency for Network Load Balancers.

Fixes #1139.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants