Load balancer fixes #1884

dblooman · 2017-10-13T16:26:44Z

The loadbalancer changes don't allow for a NLB to be created because the NLB uses TCP only, with most of the code only allowing HTTP or HTTPS.

I think there is probably some additional code coverage that can be added here, especially as NLB's are much more restrictive than ALB/ELB

Example

resource "aws_lb" "main" {
  name               = "test-nlb-tf"
  internal           = false
  load_balancer_type = "network"
  subnets            = ["subnet-123456"]

  enable_deletion_protection = true

  tags {
    Environment = "test"
  }
}

resource "aws_lb_target_group" "test" {
  name     = "tf-example"
  port     = 8085
  protocol = "TCP"
  vpc_id   = "vpc-c333ccc3"

  health_check {
    interval            = 30
    port                = "traffic-port"
    timeout             = 10
    protocol            = "TCP"
    healthy_threshold   = 3
    unhealthy_threshold = 3
  }
}

resource "aws_lb_listener" "front_end" {
  load_balancer_arn = "${aws_lb.main.id}"
  port              = "80"
  protocol          = "TCP"

  default_action {
    target_group_arn = "${aws_lb_target_group.test.id}"
    type             = "forward"
  }
}

resource "aws_lb_target_group_attachment" "test" {
  target_group_arn = "${aws_lb_target_group.test.arn}"
  target_id        = "i-0000000000000"
  port             = 80
}

code4mankind · 2017-10-17T06:28:45Z

This should be marked as a bug, not an enhancement - aws_lb of type "network" is unusable, unable to create aws_lb_listener with "TCP" protocol.

aandriyc · 2017-10-17T14:27:29Z

aws/resource_aws_lb_target_group.go

@@ -420,7 +427,7 @@ func validateAwsLbTargetGroupPort(v interface{}, k string) (ws []string, errors

 func validateAwsLbTargetGroupProtocol(v interface{}, k string) (ws []string, errors []error) {
 	protocol := strings.ToLower(v.(string))
-	if protocol == "http" || protocol == "https" {
+	if protocol == "http" || protocol == "https" || protocol == "tcp" {
 		return
 	}



Validation error message below needs to be updated

aandriyc · 2017-10-17T14:27:44Z

aws/resource_aws_lb_target_group.go

@@ -402,7 +409,7 @@ func validateAwsLbTargetGroupHealthCheckTimeout(v interface{}, k string) (ws []s

 func validateAwsLbTargetGroupHealthCheckProtocol(v interface{}, k string) (ws []string, errors []error) {
 	value := strings.ToLower(v.(string))
-	if value == "http" || value == "https" {
+	if value == "http" || value == "https" || value == "tcp" {
 		return
 	}



Validation error message below needs to be updated

elsbrock · 2017-10-17T19:23:27Z

Cool! I think it's missing the possibility to attach the LB to a certain subnet / AZ.

dblooman · 2017-10-18T13:22:48Z

@else My first comment is wrong, i removed the subnet ID from my example by mistake, if you try to create a LB without a subnet, it will fail. You are required to attach at least one subnet to a network load balancer. I have also seen that you cannot update a NLB with additional subnets after creation, handled in #1941

c2h5oh · 2017-10-25T15:43:11Z

Is there any more work left here? This is a blocker for us and I'll help if necessary

jwinter · 2017-10-26T00:12:37Z

This is minor, but I terraform applyed a target group with the same health check as the example:

resource "aws_lb_target_group" "test" {
  name     = "tf-example"
  port     = 8085
  protocol = "TCP"
  vpc_id   = "vpc-c333ccc3"

  health_check {
    interval            = 30
    port                = "traffic-port"
    timeout             = 10
    protocol            = "TCP"
    healthy_threshold   = 3
    unhealthy_threshold = 3
  }
}

After the apply succeeded, a subsequent terraform plan showed a change pending:

Terraform will perform the following actions:

  ~ module.ads.aws_lb_target_group.dataservice-target-group
      health_check.0.path:                   "" => "/"

I expected this plan not to show any changes since I had just applied.

I believe this is because the Default path for the health check is "/". The health-check-path is only for HTTP/HTTPS health checks, not TCP.

c2h5oh · 2017-10-30T16:06:56Z

There is one more - checker for healthcheck defaults to checking HTTP 200 response code, which is only supposed to happen for HTTP/HTTPS healthchecks http://docs.aws.amazon.com/elasticloadbalancing/latest/network/target-group-health-checks.html

dblooman · 2017-10-30T16:17:58Z

@c2h5oh in my branch or in master? I removed the default in my branch, so it shouldn't impact it anymore. @jwinter Removed default path, though im not actually sure if this PR is going to be merged or not, we'll have to see, @stack72 or @radeksimko could perhaps give us an update as current NLB is pretty unusable in terraform right now.

c2h5oh · 2017-10-30T16:24:13Z

Right, it's fixed in your branch. I wish we could just write a validator for entire HealthCheck, but https://github.com/hashicorp/terraform/blob/master/helper/schema/schema.go#L188 suggests we can't.

@stack72 or @radeksimko could perhaps give us an update as current NLB is pretty unusable in terraform right now.

Totally unusable.

dblooman · 2017-10-30T16:48:49Z

I actually think the best approach would have been to keep the load balancer types separate. They have quite a lot of differences that have made this more complex than it needed to be. I'm sure there are additional things that have not been picked up so far, but once these fixes go in, we may see edge cases not picked up by tests.

c2h5oh · 2017-10-30T17:34:43Z

Ideally schema.Schema would have been an interface, not a struct and you could just compose the types of LB from a common struct and type specific one.

nathanielks · 2017-10-30T20:09:26Z

@DaveBlooman maybe we should keep them separate? It's obvious implementation wasn't as simple as it appeared to be at the outset. It makes sense to me to keep them separate as they require different things. Even though AWS sees them as one resource, functionally they're different.

c2h5oh · 2017-10-31T16:47:13Z

@nathanielks the aws_lb is almost the same for both types - when load_balancer_type == "network" you can't change subnets after creation is the sole difference. aws_lb_target_group_attachment is the same between the two

The big differences are in:

aws_lb_listener

network: TCP only, no TLS support, can have elastic IP assigned, has only a single rule - which target group should it forward the traffic to
application: HTTP/HTTPS only, TLS support, no elastic IP support, custom routing rules supported

basically no overlap

aws_lb_target_group":

network: TCP only, TCP healthcheck (port optional, no path) no sticky sessions
application: HTTP/HTTPS only, HTTP/HTTPS healthcheck (port optional, path required with default) checking HTTP response code, sticky sessions support

again, no overlap

So IMHO it makes sense to split listener and target into _network_ and _application_ variants while keeping the rest.

nathanielks · 2017-10-31T20:10:41Z

@c2h5oh access_logs are also different: alb's have them and nlb's don't.

OferE · 2017-11-01T09:15:35Z

hi - since this is blocking us - is there a way to get a binary for this version?

strongoose · 2017-11-01T11:13:39Z

@OferE you can just pull the branch and build your own binary pretty easily.

dblooman · 2017-11-01T14:08:32Z

@OferE I have made a release on my fork, assuming its macOS/Linux you wanted. https://github.com/DaveBlooman/terraform-provider-aws/releases/tag/dev

OferE · 2017-11-01T17:11:25Z

Thank u so much!! you are amazing

Elethiomel · 2017-11-02T12:18:14Z

@DaveBlooman : you're a lifesaver.

Telling · 2017-11-06T15:11:37Z

I just tried to setup a new NLB using the updated AWS provider provided by @DaveBlooman. I am not able to get a NLB up and running using it. It fails with:

Error: Error applying plan:

1 error(s) occurred:

* module.rabbitmq.aws_lb.lb: 1 error(s) occurred:

* aws_lb.lb: Failure Setting ALB Subnets: InvalidConfigurationRequest: SetSubnets is not supported for load balancers of type 'network'
        status code: 400, request id: eb422135-c302-11e7-b4bc-6b437721a612

This is my code:

# Load balancing

resource "aws_lb" "lb" {
  name               = "rabbitmq-old"
  internal           = true
  subnets            = ["subnet-9d4f83f5"]
  load_balancer_type = "network"
}

resource "aws_alb_target_group" "target" {
  name     = "rabbitmq-old"
  port     = "5672"
  protocol = "TCP"
  vpc_id   = "${var.vpc_id}"
}

resource "aws_alb_listener" "listener" {
  load_balancer_arn = "${aws_lb.lb.arn}"
  port              = "5672"
  protocol          = "TCP"

  default_action {
    target_group_arn = "${aws_alb_target_group.target.arn}"
    type             = "forward"
  }
}

And to clarify, I'm using the NLB with an ASG like so:

# Auto Scaling Group

resource "aws_autoscaling_group" "scaling_group" {
  name                = "rabbitmq-old"
  vpc_zone_identifier = ["${aws_subnet.subnet.id}"]
  target_group_arns   = ["${aws_alb_target_group.target.arn}"]

  health_check_type    = "EC2"
  launch_configuration = "${aws_launch_configuration.launch_config.name}"
  desired_capacity     = "${var.scaling_desired}"
  max_size             = "${var.scaling_max}"
  min_size             = "${var.scaling_min}"
}

Which I believe should be fine. Am I doing something wrong or is this just a bug?

c2h5oh · 2017-11-06T17:38:10Z

@DaveBlooman one more:
Error creating LB Target Group: InvalidConfigurationRequest: Custom health check timeouts are not supported for health checks for target groups with the TCP protocol - ~~I guess we need to get rid of the timeout default in health check~~ nope, it has to be exactly 10 seconds.

edit2: interval has to be exactly 30s, timeout exactly 10s

dblooman · 2017-11-10T17:28:36Z

@c2h5oh So you can change the interval, you can set it to 10 or 30s, but the timeout is set to 10s. I have made some changes to this PR so that it should cover the health check block correctly.

Here is some sample TF for those having issues with ASG's and NLB's, just fill in the locals and deploy an app running on 8080.

locals {
  subnet_id     = "subnet-"
  vpc_id        = "vpc-"
  ami_id        = "ami-"
  key_pair      = ""
  instance_type = "t2.micro"
}

resource "aws_lb" "demo" {
  name                       = "demo-lb"
  internal                   = false
  load_balancer_type         = "network"
  enable_deletion_protection = false

  subnet_mapping {
    subnet_id     = "${local.subnet_id}"
    allocation_id = "${aws_eip.demo.id}"
  }

  tags {
    Environment = "demo"
  }
}

resource "aws_lb_target_group" "demo" {
  name     = "nlb-example"
  port     = 8080
  protocol = "TCP"
  vpc_id   = "${local.vpc_id}"

  health_check {
    interval          = 30
    port              = 8080
    protocol          = "TCP"
    healthy_threshold = 3
  }
}

resource "aws_lb_listener" "demo" {
  load_balancer_arn = "${aws_lb.demo.id}"
  port              = "80"
  protocol          = "TCP"

  default_action {
    target_group_arn = "${aws_lb_target_group.demo.id}"
    type             = "forward"
  }
}

resource "aws_eip" "demo" {}

resource "aws_autoscaling_group" "demo" {
  name                 = "testing-asg"
  vpc_zone_identifier  = ["${local.subnet_id}"]
  target_group_arns    = ["${aws_lb_target_group.demo.arn}"]
  health_check_type    = "EC2"
  launch_configuration = "${aws_launch_configuration.demo.name}"
  desired_capacity     = 1
  max_size             = 1
  min_size             = 1
}

resource "aws_launch_configuration" "demo" {
  name_prefix     = "demo-"
  image_id        = "${local.ami_id}"
  instance_type   = "${local.instance_type}"
  key_name        = "${local.key_pair}"
  security_groups = ["${aws_security_group.demo.id}"]

  lifecycle {
    create_before_destroy = true
  }
}

resource "aws_security_group" "demo" {
  vpc_id      = "${local.vpc_id}"
  name        = "demo-nlb"
  description = "demo-nlb"

  tags {
    Name = "demo-nlb"
  }
}

resource "aws_security_group_rule" "demo_ssh_in" {
  type              = "ingress"
  from_port         = 22
  to_port           = 22
  protocol          = "tcp"
  cidr_blocks       = ["0.0.0.0/0"]
  security_group_id = "${aws_security_group.demo.id}"
}

resource "aws_security_group_rule" "nlb_port_in" {
  type      = "ingress"
  from_port = 8080
  to_port   = 8080
  protocol  = "tcp"

  cidr_blocks       = ["0.0.0.0/0"]
  security_group_id = "${aws_security_group.demo.id}"
}

resource "aws_security_group_rule" "traffic_out" {
  type      = "egress"
  from_port = 0
  to_port   = 0
  protocol  = "-1"

  cidr_blocks       = ["0.0.0.0/0"]
  security_group_id = "${aws_security_group.demo.id}"
}

catsby

I made some notes below. I believe I'll build of this branch and do the remaining work, since everyone here has already contributed a lot and we're a bit behind on addressing this

catsby · 2017-11-10T18:10:37Z

aws/resource_aws_lb_target_group_test.go

@@ -794,6 +794,39 @@ resource "aws_vpc" "test" {
 }`, targetGroupName)
 }

+func testAccAWSLBTargetGroupConfig_typeTCP(targetGroupName string) string {


This configuration isn't actually used in a test, was the corresponding test omitted?

catsby · 2017-11-10T18:11:38Z

aws/resource_aws_lb_target_group_test.go

+
+  deregistration_delay = 200
+
+  stickiness {


stickiness is only applicable to Application Load balancers:

http://docs.aws.amazon.com/elasticloadbalancing/latest/APIReference/API_TargetGroupAttribute.html

We should take this opportunity to document that as well

catsby · 2017-11-10T18:55:21Z

I've opened #2251 to supersede of this one. It's a continuation with just a commit on top that adds a test and updates the docs. That PR will allow Network Load balancers to use Target Groups. Please let me know what you think! I'm going to close this one for now

AnishAnil · 2017-11-17T09:36:05Z

Hello Terraform,

This is a very critical blocker for our automation and we are ending up getting the same error.
The Application load balancer and classic load balancer works flawlessly but network LB fails wil the error.

Error applying plan:

1 error(s) occurred:

aws_lb.test: 1 error(s) occurred:
aws_lb.test: Failure Setting ALB Subnets: InvalidConfigurationRequest: SetSubnets is not supported for load bala
ncers of type 'network'
status code: 400, request id: 11cbb172-cb7a-11e7-9498-cbadb6f85381
Error applying plan:

1 error(s) occurred:

aws_lb.test: 1 error(s) occurred:
aws_lb.test: Failure Setting ALB Subnets: InvalidConfigurationRequest: SetSubnets is not supported for load bala
ncers of type 'network'
status code: 400, request id: 11cbb172-cb7a-11e7-9498-cbadb6f85381

My Code*
resource "aws_lb" "test" {
name = "test-lb-tf"
idle_timeout = 60
load_balancer_type = "network"
internal = false
subnets = ["${aws_subnet.main-public-2.id}", "${aws_subnet.main-public-1.id}"]

enable_deletion_protection = false

tags {
Environment = "production"
}
}

NOTE: The Load balancer is actually created and is attached to multiple Availability zone as mentioned in the subnet. But the error simply fails the automation.

Is there an ETA when this will be resolved or any suggestions to circumvent this behavior.....

Thank you

radeksimko · 2017-11-17T10:02:08Z

@AnishAnil this was shipped yesterday in 1.3.0

AnishAnil · 2017-11-17T10:17:41Z

I just updated Terraform and tested and ended up with the same error.

The only differnec being it asks to confirm before i apply the plan and after the plan it applied the "Error" is in red while the rest of the message is in plain white text.

Error: Error applying plan: <"Error" in this statement is in RED and is the only differnece>

1 error(s) occurred:

aws_lb.test: 1 error(s) occurred:
aws_lb.test: Failure Setting ALB Subnets: InvalidConfigurationRequest: SetSubnets is not supported for load bala
ncers of type 'network'
status code: 400, request id: b801a52e-cb7f-11e7-9498-cbadb6f85381

Terraform does not automatically rollback in the face of errors.Instead, your Terraform state file has been partially updated with any resources that successfully completed. Please address the error above and apply again to incremental

terraform version
Terraform v0.11.0

provider.aws v1.2.0

Is this the intended behavior.........or is there any separete mentod to upgrade to 1.3??

Ninir · 2017-11-17T10:33:40Z

HI @AnishAnil

Since Terraform 0.10, providers (aws, google, azure, rabbitmq, etc..) are not in their own repository, like the one you are currently seeing. This brings a lot of pros: you don't need the core release to get new fixes & features as each one of them has its own lifecycle and thus is built & released independently.

Yesterday, we released version 1.3.0 of the AWS Provider, which contains the new features & fixes.

It seems you updated Terraform only, which is a great thing! however, you will only get those new features once you run: terraform init -upgrade (this will work most of the time, except if you specified the provider version in the Terraform configuration).

Can you do that and tell us how it goes?

Thanks!

AnishAnil · 2017-11-17T11:27:40Z

@Ninir, Thank you n it works now.

jasonkuehl · 2017-12-14T20:39:48Z

I'm having the same issue where TCP is not a valid option for protocol.

Error: aws_lb_target_group.testexternal: "protocol" must be either "HTTP" or "HTTPS"

resource "aws_lb" "testexternal" {
  name                        = "testserver"

  load_balancer_type          = "network"
  internal                    = false
  subnets                     = ["${module.subnet.ELB-subnet-ids}"]
  enable_deletion_protection  = true
}

resource "aws_lb_target_group" "testexternal" {
  name     = "testexternal"
  protocol = "TCP"
  port     = 22
  vpc_id      = "${aws_vpc.bla.id}"

  health_check {
      healthy_threshold   = 10
      unhealthy_threshold = 2
      interval            = 10
      timeout             = 3
  }
}

resource "aws_lb" "testexternal" {
  name                        = "testserver"

  load_balancer_type          = "network"
  internal                    = false
  subnets                     = ["${module.subnet.ELB-subnet-ids}"]
  enable_deletion_protection  = true
}

resource "aws_lb_listener" "testexternal" {
  load_balancer_arn = "${aws_lb.testexternal.arn}"
  protocol          = "TCP"
  port              = "22"

  default_action {
    target_group_arn = "${aws_lb_target_group.testexternal.arn}"
    type             = "forward"
  }
}

resource "aws_lb_target_group_attachment" "testexternal" {
  target_group_arn = "${aws_lb_target_group.testexternal.arn}"
  target_id        = "${aws_instance.testserver-001.id}"
  port             = 22
}

ghost · 2020-04-10T17:16:21Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

dblooman mentioned this pull request Oct 13, 2017

Impossible to create LBs of type network #1838

Closed

radeksimko added the enhancement Requests to existing resources that expand the functionality or scope. label Oct 14, 2017

alextrofymenko mentioned this pull request Oct 16, 2017

Target group should support TCP protocol #1914

Closed

Ginja mentioned this pull request Oct 17, 2017

lb_target_group not compatible with aws_lb of type network #1912

Closed

aandriyc reviewed Oct 17, 2017

View reviewed changes

itsdalmo mentioned this pull request Nov 3, 2017

Fix container modules once AWS provider supports NLB TeliaSoneraNorge/divx-terraform-modules#46

Closed

keichan34 mentioned this pull request Nov 10, 2017

Add support for NLB target groups. #2242

Closed

load balancer fixes

0d9e713

catsby reviewed Nov 10, 2017

View reviewed changes

catsby mentioned this pull request Nov 10, 2017

r/lb_target_group: fix compatibility with Network Load Balancers #2251

Merged

catsby closed this Nov 10, 2017

ghost locked and limited conversation to collaborators Apr 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Load balancer fixes #1884

Load balancer fixes #1884

dblooman commented Oct 13, 2017 •

edited

Loading

code4mankind commented Oct 17, 2017

aandriyc Oct 17, 2017

aandriyc Oct 17, 2017

elsbrock commented Oct 17, 2017

dblooman commented Oct 18, 2017

c2h5oh commented Oct 25, 2017

jwinter commented Oct 26, 2017

c2h5oh commented Oct 30, 2017

dblooman commented Oct 30, 2017

c2h5oh commented Oct 30, 2017

dblooman commented Oct 30, 2017

c2h5oh commented Oct 30, 2017

nathanielks commented Oct 30, 2017

c2h5oh commented Oct 31, 2017

nathanielks commented Oct 31, 2017

OferE commented Nov 1, 2017

strongoose commented Nov 1, 2017

dblooman commented Nov 1, 2017

OferE commented Nov 1, 2017

Elethiomel commented Nov 2, 2017

Telling commented Nov 6, 2017 •

edited

Loading

c2h5oh commented Nov 6, 2017 •

edited

Loading

dblooman commented Nov 10, 2017

catsby left a comment

catsby Nov 10, 2017

catsby Nov 10, 2017

catsby commented Nov 10, 2017

AnishAnil commented Nov 17, 2017

radeksimko commented Nov 17, 2017

AnishAnil commented Nov 17, 2017

Ninir commented Nov 17, 2017 •

edited

Loading

AnishAnil commented Nov 17, 2017

jasonkuehl commented Dec 14, 2017 •

edited

Loading

ghost commented Apr 10, 2020

Load balancer fixes #1884

Load balancer fixes #1884

Conversation

dblooman commented Oct 13, 2017 • edited Loading

code4mankind commented Oct 17, 2017

aandriyc Oct 17, 2017

Choose a reason for hiding this comment

aandriyc Oct 17, 2017

Choose a reason for hiding this comment

elsbrock commented Oct 17, 2017

dblooman commented Oct 18, 2017

c2h5oh commented Oct 25, 2017

jwinter commented Oct 26, 2017

c2h5oh commented Oct 30, 2017

dblooman commented Oct 30, 2017

c2h5oh commented Oct 30, 2017

dblooman commented Oct 30, 2017

c2h5oh commented Oct 30, 2017

nathanielks commented Oct 30, 2017

c2h5oh commented Oct 31, 2017

nathanielks commented Oct 31, 2017

OferE commented Nov 1, 2017

strongoose commented Nov 1, 2017

dblooman commented Nov 1, 2017

OferE commented Nov 1, 2017

Elethiomel commented Nov 2, 2017

Telling commented Nov 6, 2017 • edited Loading

c2h5oh commented Nov 6, 2017 • edited Loading

dblooman commented Nov 10, 2017

catsby left a comment

Choose a reason for hiding this comment

catsby Nov 10, 2017

Choose a reason for hiding this comment

catsby Nov 10, 2017

Choose a reason for hiding this comment

catsby commented Nov 10, 2017

AnishAnil commented Nov 17, 2017

radeksimko commented Nov 17, 2017

AnishAnil commented Nov 17, 2017

Ninir commented Nov 17, 2017 • edited Loading

AnishAnil commented Nov 17, 2017

jasonkuehl commented Dec 14, 2017 • edited Loading

ghost commented Apr 10, 2020

dblooman commented Oct 13, 2017 •

edited

Loading

Telling commented Nov 6, 2017 •

edited

Loading

c2h5oh commented Nov 6, 2017 •

edited

Loading

Ninir commented Nov 17, 2017 •

edited

Loading

jasonkuehl commented Dec 14, 2017 •

edited

Loading