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

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

Merged
merged 4 commits into from
Nov 13, 2017
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0d9e713
load balancer fixes
dblooman Oct 13, 2017
76a3ce9
r/lb_target_group: fix compatibility with Network Load Balancers
catsby Nov 10, 2017
c93f98a
fix typo
catsby Nov 11, 2017
3310363
apply feedback
catsby Nov 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 41 additions & 27 deletions aws/resource_aws_lb_target_group.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ func resourceAwsLbTargetGroup() *schema.Resource {
"path": {
Type: schema.TypeString,
Optional: true,
Default: "/",
ValidateFunc: validateAwsLbTargetGroupHealthCheckPath,
},

Expand All @@ -146,27 +145,26 @@ func resourceAwsLbTargetGroup() *schema.Resource {
"timeout": {
Type: schema.TypeInt,
Optional: true,
Default: 5,
Default: 10,
ValidateFunc: validateAwsLbTargetGroupHealthCheckTimeout,
},

"healthy_threshold": {
Type: schema.TypeInt,
Optional: true,
Default: 5,
Default: 3,
ValidateFunc: validateAwsLbTargetGroupHealthCheckHealthyThreshold,
},

"matcher": {
Type: schema.TypeString,
Optional: true,
Default: "200",
},

"unhealthy_threshold": {
Type: schema.TypeInt,
Optional: true,
Default: 2,
Default: 3,
ValidateFunc: validateAwsLbTargetGroupHealthCheckHealthyThreshold,
},
},
Expand Down Expand Up @@ -201,14 +199,17 @@ func resourceAwsLbTargetGroupCreate(d *schema.ResourceData, meta interface{}) er
healthCheck := healthChecks[0].(map[string]interface{})

params.HealthCheckIntervalSeconds = aws.Int64(int64(healthCheck["interval"].(int)))
params.HealthCheckPath = aws.String(healthCheck["path"].(string))
params.HealthCheckPort = aws.String(healthCheck["port"].(string))
params.HealthCheckProtocol = aws.String(healthCheck["protocol"].(string))
params.HealthCheckTimeoutSeconds = aws.Int64(int64(healthCheck["timeout"].(int)))
params.HealthyThresholdCount = aws.Int64(int64(healthCheck["healthy_threshold"].(int)))
params.UnhealthyThresholdCount = aws.Int64(int64(healthCheck["unhealthy_threshold"].(int)))
params.Matcher = &elbv2.Matcher{
HttpCode: aws.String(healthCheck["matcher"].(string)),

if *params.Protocol != "TCP" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be a bit more future-proof and cleaner if we checked each argument separately regardless of the protocol. That said it would probably require removing all defaults from these fields and making them computed, which introduces some other potential troubles during updates.

The downside of the current solution is that we're silently ignoring certain arguments without ever telling the user which may backfire if AWS decides to support any of these arguments in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed this isn't ideal, but I'm going to go forward as is. Part of me wonders if the real solution would have been to keep ALB and LB as separate resources entirely 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed on that point, It is a shame to make a breaking change, but as more specific features are added to each type, they will diverge more. Let's see what aws do at re: invent

params.HealthCheckTimeoutSeconds = aws.Int64(int64(healthCheck["timeout"].(int)))
params.HealthCheckPath = aws.String(healthCheck["path"].(string))
params.Matcher = &elbv2.Matcher{
HttpCode: aws.String(healthCheck["matcher"].(string)),
}
params.UnhealthyThresholdCount = aws.Int64(int64(healthCheck["unhealthy_threshold"].(int)))
}
}

Expand Down Expand Up @@ -264,17 +265,22 @@ func resourceAwsLbTargetGroupUpdate(d *schema.ResourceData, meta interface{}) er
healthCheck := healthChecks[0].(map[string]interface{})

params = &elbv2.ModifyTargetGroupInput{
TargetGroupArn: aws.String(d.Id()),
HealthCheckIntervalSeconds: aws.Int64(int64(healthCheck["interval"].(int))),
HealthCheckPath: aws.String(healthCheck["path"].(string)),
HealthCheckPort: aws.String(healthCheck["port"].(string)),
HealthCheckProtocol: aws.String(healthCheck["protocol"].(string)),
HealthCheckTimeoutSeconds: aws.Int64(int64(healthCheck["timeout"].(int))),
HealthyThresholdCount: aws.Int64(int64(healthCheck["healthy_threshold"].(int))),
UnhealthyThresholdCount: aws.Int64(int64(healthCheck["unhealthy_threshold"].(int))),
Matcher: &elbv2.Matcher{
TargetGroupArn: aws.String(d.Id()),
HealthCheckPort: aws.String(healthCheck["port"].(string)),
HealthCheckProtocol: aws.String(healthCheck["protocol"].(string)),
HealthyThresholdCount: aws.Int64(int64(healthCheck["healthy_threshold"].(int))),
}

healthCheckProtocol := strings.ToLower(healthCheck["protocol"].(string))

if healthCheckProtocol != "tcp" {
params.Matcher = &elbv2.Matcher{
HttpCode: aws.String(healthCheck["matcher"].(string)),
},
}
params.HealthCheckPath = aws.String(healthCheck["path"].(string))
params.HealthCheckIntervalSeconds = aws.Int64(int64(healthCheck["interval"].(int)))
params.UnhealthyThresholdCount = aws.Int64(int64(healthCheck["unhealthy_threshold"].(int)))
params.HealthCheckTimeoutSeconds = aws.Int64(int64(healthCheck["timeout"].(int)))
}
} else {
params = &elbv2.ModifyTargetGroupInput{
Expand Down Expand Up @@ -402,11 +408,11 @@ 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
}

errors = append(errors, fmt.Errorf("%q must be either %q or %q", k, "HTTP", "HTTPS"))
errors = append(errors, fmt.Errorf("%q must be either %q, %q or %q", k, "HTTP", "HTTPS", "TCP"))
return
}

Expand All @@ -420,11 +426,11 @@ 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
}

errors = append(errors, fmt.Errorf("%q must be either %q or %q", k, "HTTP", "HTTPS"))
errors = append(errors, fmt.Errorf("%q must be either %q, %q or %q", k, "HTTP", "HTTPS", "TCP"))
return
}

Expand Down Expand Up @@ -479,14 +485,22 @@ func flattenAwsLbTargetGroupResource(d *schema.ResourceData, meta interface{}, t

healthCheck := make(map[string]interface{})
healthCheck["interval"] = *targetGroup.HealthCheckIntervalSeconds
healthCheck["path"] = *targetGroup.HealthCheckPath
healthCheck["port"] = *targetGroup.HealthCheckPort
healthCheck["protocol"] = *targetGroup.HealthCheckProtocol
healthCheck["timeout"] = *targetGroup.HealthCheckTimeoutSeconds
healthCheck["healthy_threshold"] = *targetGroup.HealthyThresholdCount
healthCheck["unhealthy_threshold"] = *targetGroup.UnhealthyThresholdCount
healthCheck["matcher"] = *targetGroup.Matcher.HttpCode
d.Set("health_check", []interface{}{healthCheck})

if targetGroup.HealthCheckPath != nil {
healthCheck["path"] = *targetGroup.HealthCheckPath
}
if targetGroup.Matcher.HttpCode != nil {
healthCheck["matcher"] = *targetGroup.Matcher.HttpCode
}

if err := d.Set("health_check", []interface{}{healthCheck}); err != nil {
log.Printf("[WARN] Error setting health check: %s", err)
}

attrResp, err := elbconn.DescribeTargetGroupAttributes(&elbv2.DescribeTargetGroupAttributesInput{
TargetGroupArn: aws.String(d.Id()),
Expand Down
67 changes: 67 additions & 0 deletions aws/resource_aws_lb_target_group_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,41 @@ func TestAccAWSLBTargetGroup_basic(t *testing.T) {
})
}

func TestAccAWSLBTargetGroup_networkLB_TargetGroup(t *testing.T) {
var conf elbv2.TargetGroup
targetGroupName := fmt.Sprintf("test-target-group-%s", acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum))

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
IDRefreshName: "aws_lb_target_group.test",
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSLBTargetGroupDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSLBTargetGroupConfig_typeTCP(targetGroupName),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckAWSLBTargetGroupExists("aws_lb_target_group.test", &conf),
resource.TestCheckResourceAttrSet("aws_lb_target_group.test", "arn"),
resource.TestCheckResourceAttr("aws_lb_target_group.test", "name", targetGroupName),
resource.TestCheckResourceAttr("aws_lb_target_group.test", "port", "8082"),
resource.TestCheckResourceAttr("aws_lb_target_group.test", "protocol", "TCP"),
resource.TestCheckResourceAttrSet("aws_lb_target_group.test", "vpc_id"),
resource.TestCheckResourceAttr("aws_lb_target_group.test", "deregistration_delay", "200"),
resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.#", "1"),
resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.0.interval", "30"),
resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.0.port", "traffic-port"),
resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.0.protocol", "TCP"),
resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.0.timeout", "10"),
resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.0.healthy_threshold", "3"),
resource.TestCheckResourceAttr("aws_lb_target_group.test", "health_check.0.unhealthy_threshold", "3"),
resource.TestCheckResourceAttr("aws_lb_target_group.test", "tags.%", "1"),
resource.TestCheckResourceAttr("aws_lb_target_group.test", "tags.Name", "TestAcc_networkLB_TargetGroup"),
),
},
},
})
}

func TestAccAWSLBTargetGroupBackwardsCompatibility(t *testing.T) {
var conf elbv2.TargetGroup
targetGroupName := fmt.Sprintf("test-target-group-%s", acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum))
Expand Down Expand Up @@ -794,6 +829,38 @@ resource "aws_vpc" "test" {
}`, targetGroupName)
}

func testAccAWSLBTargetGroupConfig_typeTCP(targetGroupName string) string {
return fmt.Sprintf(`resource "aws_lb_target_group" "test" {
name = "%s"
port = 8082
protocol = "TCP"
vpc_id = "${aws_vpc.test.id}"

deregistration_delay = 200

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

tags {
Name = "TestAcc_networkLB_TargetGroup"
}
}

resource "aws_vpc" "test" {
cidr_block = "10.0.0.0/16"

tags {
Name = "TestAcc_networkLB_TargetGroup"
}
}`, targetGroupName)
}

func testAccAWSLBTargetGroupConfig_stickiness(targetGroupName string, addStickinessBlock bool, enabled bool) string {
var stickinessBlock string

Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/lb_target_group.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ The following arguments are supported:
* `protocol` - (Required) The protocol to use for routing traffic to the targets.
* `vpc_id` - (Required) The identifier of the VPC in which to create the target group.
* `deregistration_delay` - (Optional) The amount time for Elastic Load Balancing to wait before changing the state of a deregistering target from draining to unused. The range is 0-3600 seconds. The default value is 300 seconds.
* `stickiness` - (Optional) A Stickiness block. Stickiness blocks are documented below.
* `stickiness` - (Optional) A Stickiness block. Stickiness blocks are documented below. `stickiness` is only valid if used with Load Balancers of type `Application`
* `health_check` - (Optional) A Health Check block. Health Check blocks are documented below.
* `tags` - (Optional) A mapping of tags to assign to the resource.

Expand Down