provider/aws: Add support for Network Loadbalancers #1629

stack72 · 2017-09-09T17:49:53Z

In terraform, we had the idea of an alb. In AWS this doesn't exist. ALBs
are actually Load balancers of type application

Therefore, the first part of this PR adds a new parameter to ALBs called
load_balancer_type. We default this to application to follow the
same idea as the current behaviour

The next part of the PR will then change the idea of an alb -> lb

In order to preserve backwards compatibility, we have added another
resource name to the same schema type. This means we effectively have an
alias of aws_alb and aws_lb. This includes updating all of the tests
to make sure and remove the idea of ALB and rename to LB and then we
will add a check to make sure we can still check that an ALB can be
created in the old resource

robinbowes · 2017-09-09T20:55:47Z

I like this approach.

stack72 · 2017-09-10T19:25:51Z

ALB -> LB Rename:

% make testacc TEST=./aws TESTARGS='-run=TestAccAWSLB_'                                                                                                                     2 ↵ ✹
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSLB_ -timeout 120m
=== RUN   TestAccAWSLB_basic
--- PASS: TestAccAWSLB_basic (257.40s)
=== RUN   TestAccAWSLB_generatedName
--- PASS: TestAccAWSLB_generatedName (250.24s)
=== RUN   TestAccAWSLB_generatesNameForZeroValue
--- PASS: TestAccAWSLB_generatesNameForZeroValue (241.45s)
=== RUN   TestAccAWSLB_namePrefix
--- PASS: TestAccAWSLB_namePrefix (224.39s)
=== RUN   TestAccAWSLB_tags
--- PASS: TestAccAWSLB_tags (306.51s)
=== RUN   TestAccAWSLB_updatedSecurityGroups
--- PASS: TestAccAWSLB_updatedSecurityGroups (324.80s)
=== RUN   TestAccAWSLB_updatedSubnets
--- PASS: TestAccAWSLB_updatedSubnets (310.78s)
=== RUN   TestAccAWSLB_updatedIpAddressType
--- PASS: TestAccAWSLB_updatedIpAddressType (328.16s)
=== RUN   TestAccAWSLB_noSecurityGroup
--- PASS: TestAccAWSLB_noSecurityGroup (266.48s)
=== RUN   TestAccAWSLB_accesslogs
--- PASS: TestAccAWSLB_accesslogs (454.85s)
PASS
ok	github.com/terraform-providers/terraform-provider-aws/aws	2639.407s

stack72 · 2017-09-10T19:26:17Z

ALB Target Group -> LB Target Group Rename:

% make testacc TEST=./aws TESTARGS='-run=TestAccAWSLBTargetGroup'                                                                                                               ✹
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSLBTargetGroup -timeout 120m
=== RUN   TestAccAWSLBTargetGroupAttachment_basic
--- PASS: TestAccAWSLBTargetGroupAttachment_basic (147.89s)
=== RUN   TestAccAWSLBTargetGroupAttachment_withoutPort
--- PASS: TestAccAWSLBTargetGroupAttachment_withoutPort (151.83s)
=== RUN   TestAccAWSLBTargetGroup_basic
--- PASS: TestAccAWSLBTargetGroup_basic (61.20s)
=== RUN   TestAccAWSLBTargetGroup_namePrefix
--- PASS: TestAccAWSLBTargetGroup_namePrefix (58.53s)
=== RUN   TestAccAWSLBTargetGroup_generatedName
--- PASS: TestAccAWSLBTargetGroup_generatedName (58.85s)
=== RUN   TestAccAWSLBTargetGroup_changeNameForceNew
--- PASS: TestAccAWSLBTargetGroup_changeNameForceNew (100.24s)
=== RUN   TestAccAWSLBTargetGroup_changeProtocolForceNew
--- PASS: TestAccAWSLBTargetGroup_changeProtocolForceNew (116.85s)
=== RUN   TestAccAWSLBTargetGroup_changePortForceNew
--- PASS: TestAccAWSLBTargetGroup_changePortForceNew (99.64s)
=== RUN   TestAccAWSLBTargetGroup_changeVpcForceNew
--- PASS: TestAccAWSLBTargetGroup_changeVpcForceNew (95.69s)
=== RUN   TestAccAWSLBTargetGroup_tags
--- PASS: TestAccAWSLBTargetGroup_tags (98.38s)
=== RUN   TestAccAWSLBTargetGroup_updateHealthCheck
--- PASS: TestAccAWSLBTargetGroup_updateHealthCheck (97.87s)
=== RUN   TestAccAWSLBTargetGroup_updateSticknessEnabled
--- PASS: TestAccAWSLBTargetGroup_updateSticknessEnabled (136.35s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1223.354s

stack72 · 2017-09-10T19:27:01Z

ALB Target Group -> LB Target Group Rename:

% make testacc TEST=./aws TESTARGS='-run=TestAccAWSLBListener'                                                                                                                  ✹
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSLBListener -timeout 120m
=== RUN   TestAccAWSLBListenerRule_basic
--- PASS: TestAccAWSLBListenerRule_basic (250.35s)
=== RUN   TestAccAWSLBListenerRule_updateRulePriority
--- PASS: TestAccAWSLBListenerRule_updateRulePriority (322.16s)
=== RUN   TestAccAWSLBListenerRule_changeListenerRuleArnForcesNew
--- PASS: TestAccAWSLBListenerRule_changeListenerRuleArnForcesNew (312.03s)
=== RUN   TestAccAWSLBListenerRule_multipleConditionThrowsError
--- PASS: TestAccAWSLBListenerRule_multipleConditionThrowsError (2.60s)
=== RUN   TestAccAWSLBListener_basic
--- PASS: TestAccAWSLBListener_basic (239.81s)
=== RUN   TestAccAWSLBListener_https
--- FAIL: TestAccAWSLBListener_https (232.24s)
	testing.go:434: Step 0 error: Error applying: 1 error(s) occurred:

		* aws_iam_server_certificate.test_cert: 1 error(s) occurred:

		* aws_iam_server_certificate.test_cert: [WARN] Error uploading server certificate, error: MalformedCertificate: Certificate is no longer valid. The 'Not After' date restriction on the certificate has passed.
FAIL
exit status 1
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	1359.207s
make: *** [testacc] Error 1

I believe the cert issues are nothing to do with this PR

stack72 · 2017-09-10T19:27:31Z

ALB Datasource -> LB Datasource Rename:

% make testacc TEST=./aws TESTARGS='-run=TestAccDataSourceAWSLB'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccDataSourceAWSLB -timeout 120m
=== RUN   TestAccDataSourceAWSLBListener_basic
--- PASS: TestAccDataSourceAWSLBListener_basic (253.82s)
=== RUN   TestAccDataSourceAWSLBListener_https
--- FAIL: TestAccDataSourceAWSLBListener_https (242.25s)
	testing.go:434: Step 0 error: Error applying: 1 error(s) occurred:

		* aws_iam_server_certificate.test_cert: 1 error(s) occurred:

		* aws_iam_server_certificate.test_cert: [WARN] Error uploading server certificate, error: MalformedCertificate: Certificate is no longer valid. The 'Not After' date restriction on the certificate has passed.
=== RUN   TestAccDataSourceAWSLB_basic
--- PASS: TestAccDataSourceAWSLB_basic (265.85s)
FAIL
exit status 1
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	761.943s
make: *** [testacc] Error 1

Again, I believe the certs are nothing to do with this PR

stack72 · 2017-09-10T19:41:52Z

DataSource Backwards compatibility tests - i.e. using aws_alb rather than aws_lb

ALB:

=== RUN   TestAccDataSourceAWSLBBackwardsCompatibility
--- PASS: TestAccDataSourceAWSLBBackwardsCompatibility (263.95s)

ALB Listener:

=== RUN   TestAccDataSourceAWSLBListenerBackwardsCompatibility
--- PASS: TestAccDataSourceAWSLBListenerBackwardsCompatibility (274.05s)

ALB Target Group:

=== RUN   TestAccDataSourceAWSLBTargetGroupBackwardsCompatibility
--- PASS: TestAccDataSourceAWSLBTargetGroupBackwardsCompatibility (241.30s)

stack72 · 2017-09-10T19:58:51Z

Resource Backwards compatibility tests - i.e using aws_alb rather than aws_lb

ALB:

=== RUN   TestAccAWSLBBackwardsCompatibility
--- PASS: TestAccAWSLBBackwardsCompatibility (270.93s)

ALB Listener:

=== RUN   TestAccAWSLBListenerBackwardsCompatibility
--- PASS: TestAccAWSLBListenerBackwardsCompatibility (254.64s)

ALB Listener Rule:

=== RUN   TestAccAWSLBListenerRuleBackwardsCompatibility
--- PASS: TestAccAWSLBListenerRuleBackwardsCompatibility (254.01s)

ALB Target Group:

=== RUN   TestAccAWSLBTargetGroupBackwardsCompatibility
--- PASS: TestAccAWSLBTargetGroupBackwardsCompatibility (59.26s)

ALB Target Group Attachment:

=== RUN   TestAccAWSLBTargetGroupAttachmentBackwardsCompatibility
--- PASS: TestAccAWSLBTargetGroupAttachmentBackwardsCompatibility (163.89s)

stack72 · 2017-09-19T11:43:47Z

And finally....

Support for NLBs themselves:

=== RUN   TestAccAWSLB_networkLoadbalancer
--- PASS: TestAccAWSLB_networkLoadbalancer (283.88s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	283.922s

stack72 · 2017-09-19T11:47:29Z

@catsby / @radeksimko / @tombuildsstuff I really think this needs to be part of 1.0.0 as this is a potentially B/C due to the renaming - I have tried to cover all of the test cases as above but there may be 1 or 2 very small edge cases that I missed

luisdavim · 2017-09-20T09:26:28Z

I also like this approach, when can we have this merged?

Ninir · 2017-09-21T07:49:28Z

Hi @stack72,
'will defer to Clint, Radek or Tom as you suggested.

Seems awesome though! 👍

stack72 · 2017-09-21T09:15:00Z

@Ninir this is only as it's going to be a controversial change :(

Ninir · 2017-09-21T09:35:02Z

No worries! 😄
Just to let you know that I won't have a deeper look at it :)

dblooman · 2017-09-27T08:23:24Z

Can we get an idea when this will be merged and released?

catsby · 2017-09-27T14:25:14Z

Hi all! I apologize for the very delayed response here. We've taken a look and thus far are pretty confident there are little or no backwards incompatible or otherwise breaking changes here. Because we didn't see any BC or problems, we didn't include it in the 1.0.0 release; it was pretty big as it was and well overdue, so we wanted to minimize changes.

1.0.0 released today (September 27th, 2017). We plan to merge this soon (today/tomorrow most likely), and hopefully return to a bi-weekly release cadence for AWS. With the Provider split and separate, independent versions between core and providers, we could actually release sooner, and I know this is a pretty high demand feature, so we may release 1.0.1 even sooner. Unfortunately I can't promise a date 😞

Thanks for the patience, and thanks for all the work here, @stack72 !

luisdavim · 2017-09-27T14:41:07Z

releasing this sooner would be awesome, thanks @stack72 and @catsby

jwinter · 2017-09-27T18:39:57Z

I think this line needs to be changed to allow Target Groups with the protocol TCP: https://github.com/terraform-providers/terraform-provider-aws/pull/1629/files#diff-375aea487c27a6ada86edfd817ba2401R423

Based on the AWS docs here: http://docs.aws.amazon.com/elasticloadbalancing/latest/network/load-balancer-target-groups.html I think Network Load Balancers only support Target Groups with protocol TCP.

dblooman · 2017-09-28T08:36:15Z

I think releases should be often and small. Having the tip of master available is potentially a faster way of getting feedback as people won't have to compile themselves.

luisdavim · 2017-09-28T14:06:17Z

@stack72 I think @jwinter has a point, any comments?

lattwood · 2017-09-29T13:06:08Z

@DaveBlooman while this probably isn't the right place for the discussion, in the words of Kelsey hightower it would be dope if they released a version automatically after every merge, following the rules of semantic versioning. A guy can dream, eh?

We have adopted 1.0.0 of the AWS terraform provider, but also pulled in a PR adding support for NLB, which appears to be targeting a 1.0.1 release. (see hashicorp/terraform-provider-aws#1629) Naming is a little strange on these load balancers due to confusing decisions by both AWS and Terraform. For our modules mapped on top - we use: * `elasticloadbalancing` module supports ELB * `elasticloadbalancingv2` module supports ALB and NLB * `applicationloadbalancing` module supports ALB CloudFormation uses the first two names. Terraform uses an `elb` prefix for the first, an `lb` prefix for the second and an `alb` prefix for the third. They have effectively deprecated the third as part of the addition of the second group (in fact, those are just aliases now I believe).

radeksimko

@stack72 This is looking pretty good - thanks for all the work, esp. for updating the name across the whole codebase.

I left you one semi-important question + a few nitpicks, but overall I'm pretty much ready to merge this once you resolve conflicts.

radeksimko · 2017-10-02T09:48:06Z

aws/resource_aws_lb.go

@@ -295,7 +354,8 @@ func resourceAwsAlbUpdate(d *schema.ResourceData, meta interface{}) error {
 })
 }

- if d.HasChange("idle_timeout") {
+ // It's important to know that Idle timeout is not supported for Network Loadbalancers
+ if d.Get("load_balancer_type").(string) != "network" && d.HasChange("idle_timeout") {


Shouldn't we instead error out here and let the user know they made a mistake instead of silently ignoring it?

radeksimko · 2017-10-02T09:53:56Z

website/docs/d/lb.html.markdown


-Provides information about an Application Load Balancer.
+~> **Note:** `aws_alb` is know as `aws_lb`. The functionality is identical.


Small typo - know_n_

radeksimko · 2017-10-02T09:54:22Z

website/docs/d/lb_listener.html.markdown

+
+# aws_lb_listener
+
+~> **Note:** `aws_alb_listener` is know as `aws_lb_listener`. The functionality is identical.


^ nitpick - typo

radeksimko · 2017-10-02T09:54:46Z

website/docs/d/lb_target_group.html.markdown

+
+# aws_lb_target_group
+
+~> **Note:** `aws_alb_target_group` is know as `aws_lb_target_group`. The functionality is identical.


^ nitpick - typo

radeksimko · 2017-10-02T09:55:10Z

website/docs/d/lb_target_group.html.markdown

+## Argument Reference
+## Argument Reference
+## Argument Reference
+## Argument Reference


Nitpick - but I guess this header duplication wasn't intentional?

radeksimko · 2017-10-02T09:55:48Z

website/docs/r/lb_listener.html.markdown

-Provides an Application Load Balancer Listener resource.
+Provides a Load Balancer Listener resource.
+
+~> **Note:** `aws_alb_listener` is know as `aws_lb_listener`. The functionality is identical.


^ nitpick - typo

radeksimko · 2017-10-02T09:56:06Z

website/docs/r/lb_target_group_attachment.html.markdown

 target group

+~> **Note:** `aws_alb_target_group_attachment` is know as `aws_lb_target_group_attachment`. The functionality is identical.


^ nitpick - typo

stack72 · 2017-10-04T06:11:06Z

sorry @radeksimko, I accidentally closed this PR - with a branch delete. I just repushed the same code to a new PR #1806

When you are happy it's the same, I will make all of the changes you asked for

P.

ghost · 2020-04-11T17:12:31Z

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!

stack72 force-pushed the f-aws-nlb branch from 287849d to a13a706 Compare September 9, 2017 18:45

stack72 force-pushed the f-aws-nlb branch 2 times, most recently from d69a044 to 5505a60 Compare September 10, 2017 19:16

stack72 force-pushed the f-aws-nlb branch from 5505a60 to 84798f2 Compare September 10, 2017 20:04

stack72 changed the title ~~[WIP] provider/aws: Add support for Network Loadbalancers~~ provider/aws: Add support for Network Loadbalancers Sep 10, 2017

stack72 force-pushed the f-aws-nlb branch 2 times, most recently from 86dab53 to a253a4b Compare September 10, 2017 20:18

stack72 changed the title ~~provider/aws: Add support for Network Loadbalancers~~ [WIP] provider/aws: Add support for Network Loadbalancers Sep 10, 2017

itsdalmo mentioned this pull request Sep 11, 2017

ECS (container module): Test with classic ELB TeliaSoneraNorge/divx-terraform-modules#14

Closed

Ninir added the enhancement Requests to existing resources that expand the functionality or scope. label Sep 11, 2017

stack72 mentioned this pull request Sep 12, 2017

Add support for new NLB #1639

Closed

stack72 force-pushed the f-aws-nlb branch 3 times, most recently from 4bd990e to 8be9043 Compare September 19, 2017 11:43

stack72 force-pushed the f-aws-nlb branch from 8be9043 to ab6bd8b Compare September 19, 2017 11:45

tombuildsstuff requested a review from catsby September 19, 2017 11:45

stack72 changed the title ~~[WIP] provider/aws: Add support for Network Loadbalancers~~ provider/aws: Add support for Network Loadbalancers Sep 19, 2017

stack72 force-pushed the f-aws-nlb branch from ab6bd8b to 945527e Compare September 21, 2017 09:18

lukehoban mentioned this pull request Oct 1, 2017

Update to latest AWS terraform provider pulumi/pulumi-aws#37

Merged

radeksimko reviewed Oct 2, 2017

View reviewed changes

radeksimko added the waiting-response Maintainers are waiting on response from community or contributor. label Oct 2, 2017

stack72 closed this Oct 4, 2017

stack72 force-pushed the f-aws-nlb branch from 945527e to f4b158c Compare October 4, 2017 06:05

stack72 mentioned this pull request Oct 4, 2017

provider/aws: Add support for Network Loadbalancers #1806

Merged

ghost locked and limited conversation to collaborators Apr 11, 2020

breathingdust removed the waiting-response Maintainers are waiting on response from community or contributor. label Sep 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

provider/aws: Add support for Network Loadbalancers #1629

provider/aws: Add support for Network Loadbalancers #1629

stack72 commented Sep 9, 2017

robinbowes commented Sep 9, 2017

stack72 commented Sep 10, 2017

stack72 commented Sep 10, 2017

stack72 commented Sep 10, 2017

stack72 commented Sep 10, 2017 •

edited

Loading

stack72 commented Sep 10, 2017 •

edited

Loading

stack72 commented Sep 10, 2017

stack72 commented Sep 19, 2017

stack72 commented Sep 19, 2017

luisdavim commented Sep 20, 2017

Ninir commented Sep 21, 2017

stack72 commented Sep 21, 2017

Ninir commented Sep 21, 2017

dblooman commented Sep 27, 2017

catsby commented Sep 27, 2017

luisdavim commented Sep 27, 2017

jwinter commented Sep 27, 2017

dblooman commented Sep 28, 2017

luisdavim commented Sep 28, 2017

lattwood commented Sep 29, 2017

radeksimko left a comment

radeksimko Oct 2, 2017

radeksimko Oct 2, 2017

radeksimko Oct 2, 2017

radeksimko Oct 2, 2017

radeksimko Oct 2, 2017

radeksimko Oct 2, 2017

radeksimko Oct 2, 2017

stack72 commented Oct 4, 2017

ghost commented Apr 11, 2020


		Provides information about an Application Load Balancer.
		~> Note: `aws_alb` is know as `aws_lb`. The functionality is identical.


		# aws_lb_listener

		~> Note: `aws_alb_listener` is know as `aws_lb_listener`. The functionality is identical.


		# aws_lb_target_group

		~> Note: `aws_alb_target_group` is know as `aws_lb_target_group`. The functionality is identical.

		target group

		~> Note: `aws_alb_target_group_attachment` is know as `aws_lb_target_group_attachment`. The functionality is identical.

provider/aws: Add support for Network Loadbalancers #1629

provider/aws: Add support for Network Loadbalancers #1629

Conversation

stack72 commented Sep 9, 2017

robinbowes commented Sep 9, 2017

stack72 commented Sep 10, 2017

stack72 commented Sep 10, 2017

stack72 commented Sep 10, 2017

stack72 commented Sep 10, 2017 • edited Loading

stack72 commented Sep 10, 2017 • edited Loading

stack72 commented Sep 10, 2017

stack72 commented Sep 19, 2017

stack72 commented Sep 19, 2017

luisdavim commented Sep 20, 2017

Ninir commented Sep 21, 2017

stack72 commented Sep 21, 2017

Ninir commented Sep 21, 2017

dblooman commented Sep 27, 2017

catsby commented Sep 27, 2017

luisdavim commented Sep 27, 2017

jwinter commented Sep 27, 2017

dblooman commented Sep 28, 2017

luisdavim commented Sep 28, 2017

lattwood commented Sep 29, 2017

radeksimko left a comment

Choose a reason for hiding this comment

radeksimko Oct 2, 2017

Choose a reason for hiding this comment

radeksimko Oct 2, 2017

Choose a reason for hiding this comment

radeksimko Oct 2, 2017

Choose a reason for hiding this comment

radeksimko Oct 2, 2017

Choose a reason for hiding this comment

radeksimko Oct 2, 2017

Choose a reason for hiding this comment

radeksimko Oct 2, 2017

Choose a reason for hiding this comment

radeksimko Oct 2, 2017

Choose a reason for hiding this comment

stack72 commented Oct 4, 2017

ghost commented Apr 11, 2020

stack72 commented Sep 10, 2017 •

edited

Loading

stack72 commented Sep 10, 2017 •

edited

Loading