provider/aws: Initial support for Application Load Balancers #8254

jen20 · 2016-08-17T12:53:44Z

This pull request contains the first two resources for Application Load Balancers ("ALB" or "elbv2"), aws_alb and aws_alb_target_group.

The resources necessary will be split across a number of pull requests to facilitate review.

Part of #8137.

stack72 · 2016-08-17T12:58:55Z

builtin/providers/aws/resource_aws_alb.go

+		return fmt.Errorf("Unable to find ALB: %#v", describeResp.LoadBalancers)
+	}
+
+	alb := describeResp.LoadBalancers[0]


Can we guarantee that our ALB is the first in the collection?

Yes - otherwise the error above would trigger.

stack72 · 2016-08-17T13:10:39Z

builtin/providers/aws/resource_aws_alb.go

+		return err
+	}
+
+	log.Printf("[INFO] ALB ID: %s", d.Id())


You are using d.Id before setting it below

stack72 · 2016-08-17T13:21:16Z

builtin/providers/aws/resource_aws_alb.go

+			},
+
+			"security_groups": {
+				Type:     schema.TypeSet,


If this cannot be modified, then this should be ForceNew: true

Good catch, corrected.

stack72 · 2016-08-17T13:26:58Z

builtin/providers/aws/resource_aws_alb_target_group.go

+
+func validateAwsAlbTargetGroupPort(v interface{}, k string) (ws []string, errors []error) {
+	port := v.(int)
+	if port < 0 || port > 65536 {


Should be <1 not <0

radeksimko · 2016-08-17T13:51:14Z

builtin/providers/aws/resource_aws_alb.go

+
+	log.Printf("[DEBUG] ALB create configuration: %#v", elbOpts)
+	var albArn string
+	err := resource.Retry(1*time.Minute, func() *resource.RetryError {


Why exactly are we retrying this operation if there's no RetryableError?

radeksimko · 2016-08-17T13:59:11Z

builtin/providers/aws/resource_aws_alb.go

+	}
+
+	accessLogMap := map[string]interface{}{}
+	for _, attr := range attributesResp.Attributes {


Wouldn't this be easier to maintain going forward if we just had a single attributes (TypeMap) field instead of trying to do this conversion?

Admittedly we'd probably still need to have some kind of key-filtering because of #8104

I don't think we can easily do that and retain the same semantics? Here we construct the required structure to set on access_log - the other attributes work on different prefixes? We could have a single attributes map in the schema, but I think this is more explicit and closer to expectations at the moment.

the other attributes work on different prefixes

yeah, it would mean making the map keys more verbose, which goes against UX, so forget what I said. This looks 👌

radeksimko · 2016-08-17T14:28:17Z

@jen20 This is looking pretty good.

I tried running the attached acceptance tests and one failed:

TF_ACC=1 go test ./builtin/providers/aws -v -run=AccAWSALB -timeout 120m
=== RUN   TestAccAWSALBTargetGroup_basic
--- PASS: TestAccAWSALBTargetGroup_basic (34.37s)
=== RUN   TestAccAWSALBTargetGroup_updateHealthCheck
--- PASS: TestAccAWSALBTargetGroup_updateHealthCheck (53.44s)
=== RUN   TestAccAWSALB_basic
--- PASS: TestAccAWSALB_basic (46.35s)
=== RUN   TestAccAWSALB_accesslogs
--- FAIL: TestAccAWSALB_accesslogs (53.77s)
        testing.go:265: Step 1 error: Error applying: 1 error(s) occurred:

            * aws_s3_bucket.logs: Error creating S3 bucket: InvalidBucketName: The specified bucket is not valid.
                status code: 400, request id: B37F0544044DE86D
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    187.957s

jen20 · 2016-08-17T14:46:21Z

@radeksimko Yup, fixing that up at the moment.

This commit adds a resource, acceptance tests and documentation for the new Application Load Balancer (aws_alb). We choose to use the name alb over the package name, elbv2, in order to avoid confusion. This is the first in a series of commits to fully support the new resources necessary for Application Load Balancers.

This commit adds a resource, acceptance tests and documentation for the Target Groups for Application Load Balancers. This is the second in a series of commits to fully support the new resources necessary for Application Load Balancers.

jen20 · 2016-08-17T15:05:43Z

› envchain aws.terraform make testacc TEST=./builtin/providers/aws TESTARGS="-run TestAccAWSALB"
==> Checking that code complies with gofmt requirements...
/Users/James/Code/go/bin/stringer
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/08/17 15:54:22 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run TestAccAWSALB -timeout 120m
=== RUN   TestAccAWSALBTargetGroup_basic
--- PASS: TestAccAWSALBTargetGroup_basic (36.57s)
=== RUN   TestAccAWSALBTargetGroup_updateHealthCheck
--- PASS: TestAccAWSALBTargetGroup_updateHealthCheck (59.23s)
=== RUN   TestAccAWSALB_basic
--- PASS: TestAccAWSALB_basic (51.25s)
=== RUN   TestAccAWSALB_accesslogs
--- PASS: TestAccAWSALB_accesslogs (93.51s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    240.584s

ghost · 2020-04-23T02:08:07Z

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

deps: Add AWS ELBv2 package

5ff28c8

jen20 added enhancement provider/aws labels Aug 17, 2016

provider/aws: Add ELBv2 to AWS config

ebdfe76

jen20 force-pushed the f-aws-application-lb branch 2 times, most recently from 197ef40 to 0145099 Compare August 17, 2016 12:56

stack72 reviewed Aug 17, 2016
View reviewed changes

jen20 force-pushed the f-aws-application-lb branch from 0145099 to db22a82 Compare August 17, 2016 13:00

stack72 reviewed Aug 17, 2016
View reviewed changes

jen20 force-pushed the f-aws-application-lb branch 2 times, most recently from 697869f to da4f09c Compare August 17, 2016 13:17

stack72 reviewed Aug 17, 2016
View reviewed changes

jen20 force-pushed the f-aws-application-lb branch from da4f09c to 9ea782b Compare August 17, 2016 13:22

stack72 reviewed Aug 17, 2016
View reviewed changes

jen20 force-pushed the f-aws-application-lb branch 6 times, most recently from b4361ce to 5276d21 Compare August 17, 2016 13:39

radeksimko reviewed Aug 17, 2016
View reviewed changes

jen20 force-pushed the f-aws-application-lb branch from 5276d21 to 6619bc2 Compare August 17, 2016 13:54

radeksimko reviewed Aug 17, 2016
View reviewed changes

jen20 force-pushed the f-aws-application-lb branch from 6619bc2 to 3d83c41 Compare August 17, 2016 14:24

jen20 added 2 commits August 17, 2016 15:48

provider/aws: Add aws_alb_target_group resource

531a976

This commit adds a resource, acceptance tests and documentation for the Target Groups for Application Load Balancers. This is the second in a series of commits to fully support the new resources necessary for Application Load Balancers.

jen20 force-pushed the f-aws-application-lb branch from 3d83c41 to 531a976 Compare August 17, 2016 14:48

jen20 merged commit f5f3154 into master Aug 17, 2016

jen20 deleted the f-aws-application-lb branch August 17, 2016 15:05

ghost locked and limited conversation to collaborators Apr 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

provider/aws: Initial support for Application Load Balancers #8254

provider/aws: Initial support for Application Load Balancers #8254

jen20 commented Aug 17, 2016

stack72 Aug 17, 2016

jen20 Aug 17, 2016

stack72 Aug 17, 2016

stack72 Aug 17, 2016

jen20 Aug 17, 2016

stack72 Aug 17, 2016

radeksimko Aug 17, 2016

radeksimko Aug 17, 2016

jen20 Aug 17, 2016

radeksimko Aug 17, 2016

radeksimko commented Aug 17, 2016 •

edited

Loading

jen20 commented Aug 17, 2016

jen20 commented Aug 17, 2016

ghost commented Apr 23, 2020

provider/aws: Initial support for Application Load Balancers #8254

provider/aws: Initial support for Application Load Balancers #8254

Conversation

jen20 commented Aug 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

radeksimko commented Aug 17, 2016 • edited Loading

jen20 commented Aug 17, 2016

jen20 commented Aug 17, 2016

ghost commented Apr 23, 2020

radeksimko commented Aug 17, 2016 •

edited

Loading