Add new attribute slow_start to aws_lb_target_group #4661

bboerst · 2018-05-25T17:35:06Z

Changes proposed in this pull request:

Add new attribute slow_start to associated resources and tests. This change requires v1.13.55 of the aws-sdk-go package.

Output from acceptance testing:

 $ make test
==> Checking that code complies with gofmt requirements...
go test ./... -timeout=30s -parallel=4
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1.857s

bflad

Hi @bboerst -- thanks for this contribution! I left some initial comments below. Please let us know if you have any questions.

bflad · 2018-05-25T19:10:30Z

aws/resource_aws_alb_target_group_test.go

@@ -65,6 +65,7 @@ func TestAccAWSALBTargetGroup_basic(t *testing.T) {
 					resource.TestCheckResourceAttr("aws_alb_target_group.test", "protocol", "HTTPS"),
 					resource.TestCheckResourceAttrSet("aws_alb_target_group.test", "vpc_id"),
 					resource.TestCheckResourceAttr("aws_alb_target_group.test", "deregistration_delay", "200"),
+					resource.TestCheckResourceAttr("aws_alb_target_group.test", "slow_start", "0"),


We only need to check slow_start being equal to "0" in the _basic resource test and its configuration as the default applies to all configurations unless overridden. Speaking of which, we should have a completely separate acceptance test and configuration that actually sets this to a non-0 value and preferably tries to update it as well so we know that functionality works.

bflad · 2018-05-25T19:10:55Z

vendor/github.com/aws/aws-sdk-go/service/elbv2/api.go

@@ -7586,6 +7586,10 @@ type TargetGroupAttribute struct {
 	//    from draining to unused. The range is 0-3600 seconds. The default value
 	//    is 300 seconds.
 	//
+	//    * slow_start.duration_seconds - The amount time for targets to warm up


This is failing CI due to:

$ make vendor-status The following packages are missing or modified locally: github.com/aws/aws-sdk-go/service/elbv2 Error: status failed for 1 package(s) make: *** [vendor-status] Error 2 The command "make vendor-status" exited with 2.

When it comes to vendor updates we usually:

Keep them separate from other code updates

Use govendor to update the dependencies (for now)

I'll go ahead and submit the v1.13.55 update PR right now then you can rebase this PR on that.

Dependency PR submitted: #4663

The full SDK update has been merged into master 👍

bflad · 2018-05-25T19:12:16Z

aws/resource_aws_alb_target_group_test.go

@@ -484,6 +490,7 @@ func testAccAWSALBTargetGroupConfig_basic(targetGroupName string) string {
  vpc_id = "${aws_vpc.test.id}"

  deregistration_delay = 200
+  slow_start = 0


We should not include the default value in all the test configurations. It should only be specified in either configurations that actually require the setting or if the default is overridden.

bboerst · 2018-05-25T23:18:27Z

@bflad This should be all set. Please double-check the acceptance tests that I wrote for this; this is my first time contributing to this project...

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSALBTargetGroup_setAndUpdateSlowStart'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSALBTargetGroup_setAndUpdateSlowStart -timeout 120m
=== RUN   TestAccAWSALBTargetGroup_setAndUpdateSlowStart
--- PASS: TestAccAWSALBTargetGroup_setAndUpdateSlowStart (39.04s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	39.094s

bflad · 2018-05-30T12:29:24Z

I'm seeing a bunch of the other acceptance testing failing, e.g.

=== RUN   TestAccAWSLBTargetGroup_basic
--- FAIL: TestAccAWSLBTargetGroup_basic (2.59s)
    testing.go:518: Step 0 error: config is invalid: aws_lb_target_group.test: "slow_start" contains an invalid Slow Start Duration "0". Valid intervals are 30-900 or 0 to disable.

bflad

Still getting acceptance testing failures, but at this point I'll just fix it up on merge so we can get this released. 🚀

bflad · 2018-05-30T22:58:37Z

aws/data_source_aws_lb_target_group_test.go

@@ -27,6 +27,7 @@ func TestAccDataSourceAWSALBTargetGroup_basic(t *testing.T) {
 					resource.TestCheckResourceAttr("data.aws_lb_target_group.alb_tg_test_with_arn", "protocol", "HTTP"),
 					resource.TestCheckResourceAttrSet("data.aws_lb_target_group.alb_tg_test_with_arn", "vpc_id"),
 					resource.TestCheckResourceAttr("data.aws_lb_target_group.alb_tg_test_with_arn", "deregistration_delay", "300"),
+					resource.TestCheckResourceAttr("data.aws_lb_target_group.alb_tg_test_with_arn", "slow_start", "0"),


The data source testing is failing because d.Set("slow_start", ...) is never called in the resource read function. This would normally be caught in the resource acceptance testing with a TestCase that calls ImportStateVerify: true.

=== RUN TestAccDataSourceAWSALBTargetGroup_basic --- FAIL: TestAccDataSourceAWSALBTargetGroup_basic (494.14s) testing.go:518: Step 0 error: Check failed: 2 error(s) occurred: * Check 9/39 error: data.aws_lb_target_group.alb_tg_test_with_arn: Attribute 'slow_start' not found * Check 28/39 error: data.aws_lb_target_group.alb_tg_test_with_name: Attribute 'slow_start' not found

bflad · 2018-05-30T23:33:38Z

After adding this to flattenAwsLbTargetGroupResource():

		case "slow_start.duration_seconds":
			slowStart, err := strconv.Atoi(aws.StringValue(attr.Value))
			if err != nil {
				return fmt.Errorf("Error converting slow_start.duration_seconds to int: %s", aws.StringValue(attr.Value))
			}
			d.Set("slow_start", slowStart)
		}

All tests are green:

35 tests passed (all tests)
=== RUN   TestAccAWSALBTargetGroup_namePrefix
--- PASS: TestAccAWSALBTargetGroup_namePrefix (94.10s)
=== RUN   TestAccAWSALBTargetGroup_generatedName
--- PASS: TestAccAWSALBTargetGroup_generatedName (96.18s)
=== RUN   TestAccAWSALBTargetGroup_updateHealthCheck
--- PASS: TestAccAWSALBTargetGroup_updateHealthCheck (106.66s)
=== RUN   TestAccAWSALBTargetGroup_basic
--- PASS: TestAccAWSALBTargetGroup_basic (130.07s)
=== RUN   TestAccAWSLBTargetGroup_namePrefix
--- PASS: TestAccAWSLBTargetGroup_namePrefix (32.27s)
=== RUN   TestAccAWSLBTargetGroup_networkLB_TargetGroup
--- PASS: TestAccAWSLBTargetGroup_networkLB_TargetGroup (139.69s)
=== RUN   TestAccAWSLBTargetGroup_basic
--- PASS: TestAccAWSLBTargetGroup_basic (166.38s)
=== RUN   TestAccAWSALBTargetGroup_changePortForceNew
--- PASS: TestAccAWSALBTargetGroup_changePortForceNew (199.31s)
=== RUN   TestAccAWSLBTargetGroup_generatedName
--- PASS: TestAccAWSLBTargetGroup_generatedName (75.91s)
=== RUN   TestAccAWSALBTargetGroup_changeVpcForceNew
--- PASS: TestAccAWSALBTargetGroup_changeVpcForceNew (218.15s)
=== RUN   TestAccAWSLBTargetGroupAttachmentBackwardsCompatibility
--- PASS: TestAccAWSLBTargetGroupAttachmentBackwardsCompatibility (286.98s)
=== RUN   TestAccAWSLBTargetGroup_changePortForceNew
--- PASS: TestAccAWSLBTargetGroup_changePortForceNew (128.56s)
=== RUN   TestAccAWSLBTargetGroupBackwardsCompatibility
--- PASS: TestAccAWSLBTargetGroupBackwardsCompatibility (206.84s)
=== RUN   TestAccAWSALBTargetGroupAttachment_ipAddress
--- PASS: TestAccAWSALBTargetGroupAttachment_ipAddress (332.15s)
=== RUN   TestAccAWSALBTargetGroup_changeNameForceNew
--- PASS: TestAccAWSALBTargetGroup_changeNameForceNew (357.31s)
=== RUN   TestAccAWSLBTargetGroup_stickinessWithTCPEnabledShouldError
--- PASS: TestAccAWSLBTargetGroup_stickinessWithTCPEnabledShouldError (14.96s)
=== RUN   TestAccAWSLBTargetGroup_stickinessWithTCPDisabled
--- PASS: TestAccAWSLBTargetGroup_stickinessWithTCPDisabled (43.72s)
=== RUN   TestAccAWSLBTargetGroup_updateSticknessEnabled
--- PASS: TestAccAWSLBTargetGroup_updateSticknessEnabled (91.40s)
=== RUN   TestAccAWSALBTargetGroup_changeProtocolForceNew
--- PASS: TestAccAWSALBTargetGroup_changeProtocolForceNew (385.95s)
=== RUN   TestAccAWSLBTargetGroup_defaults_network
--- PASS: TestAccAWSLBTargetGroup_defaults_network (104.52s)
=== RUN   TestAccAWSLBTargetGroup_networkLB_TargetGroupWithProxy
--- PASS: TestAccAWSLBTargetGroup_networkLB_TargetGroupWithProxy (414.87s)
=== RUN   TestAccAWSLBTargetGroup_updateHealthCheck
--- PASS: TestAccAWSLBTargetGroup_updateHealthCheck (204.26s)
=== RUN   TestAccAWSLBTargetGroup_defaults_application
--- PASS: TestAccAWSLBTargetGroup_defaults_application (131.64s)
=== RUN   TestAccAWSALBTargetGroup_tags
--- PASS: TestAccAWSALBTargetGroup_tags (430.31s)
=== RUN   TestAccAWSLBTargetGroupAttachment_basic
--- PASS: TestAccAWSLBTargetGroupAttachment_basic (450.62s)
=== RUN   TestAccAWSLBTargetGroup_changeProtocolForceNew
--- PASS: TestAccAWSLBTargetGroup_changeProtocolForceNew (312.78s)
=== RUN   TestAccAWSLBTargetGroup_changeNameForceNew
--- PASS: TestAccAWSLBTargetGroup_changeNameForceNew (320.00s)
=== RUN   TestAccAWSALBTargetGroup_setAndUpdateSlowStart
--- PASS: TestAccAWSALBTargetGroup_setAndUpdateSlowStart (489.66s)
=== RUN   TestAccAWSLBTargetGroup_tags
--- PASS: TestAccAWSLBTargetGroup_tags (294.67s)
=== RUN   TestAccAWSLBTargetGroupAttachment_withoutPort
--- PASS: TestAccAWSLBTargetGroupAttachment_withoutPort (508.37s)
=== RUN   TestAccAWSALBTargetGroup_updateSticknessEnabled
--- PASS: TestAccAWSALBTargetGroup_updateSticknessEnabled (514.49s)
=== RUN   TestAccAWSLBTargetGroup_TCP_HTTPHealthCheck
--- PASS: TestAccAWSLBTargetGroup_TCP_HTTPHealthCheck (451.66s)
=== RUN   TestAccAWSLBTargetGroup_changeVpcForceNew
--- PASS: TestAccAWSLBTargetGroup_changeVpcForceNew (351.60s)
=== RUN   TestAccDataSourceAWSALBTargetGroup_basic
--- PASS: TestAccDataSourceAWSALBTargetGroup_basic (571.08s)
=== RUN   TestAccDataSourceAWSLBTargetGroupBackwardsCompatibility
--- PASS: TestAccDataSourceAWSLBTargetGroupBackwardsCompatibility (784.47s)

bflad · 2018-05-31T04:06:53Z

This has been released in version 1.21.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

ghost · 2020-04-05T17:45:55Z

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!

ghost added size/M Managed by automation to categorize the size of a PR. dependencies Used to indicate dependency changes. labels May 25, 2018

bflad reviewed May 25, 2018

View reviewed changes

bflad added enhancement Requests to existing resources that expand the functionality or scope. service/elbv2 Issues and PRs that pertain to the elbv2 service. labels May 25, 2018

ghost added size/M Managed by automation to categorize the size of a PR. and removed dependencies Used to indicate dependency changes. labels May 25, 2018

Adding slow_start to required resources and tests.

9ce06f9

ghost added the size/M Managed by automation to categorize the size of a PR. label May 25, 2018

bboerst added 2 commits May 25, 2018 15:07

Removing slow_start attr check from tests where it is not needed.

004b014

Adding an acceptance test that sets slow_start to 30 then 60.

bf285ee

ghost added size/L Managed by automation to categorize the size of a PR. and removed size/M Managed by automation to categorize the size of a PR. labels May 25, 2018

Removing more slow_start attributes that were added incorrectly.

0a0f055

ghost added the size/L Managed by automation to categorize the size of a PR. label May 25, 2018

Fixing slow_start validation comment for correctness.

5ad9339

Fixed slow_start validation error.

5430ab3

bflad reviewed May 30, 2018

View reviewed changes

bflad merged commit 5430ab3 into hashicorp:master May 30, 2018

bflad added this to the v1.21.0 milestone May 30, 2018

bflad added a commit that referenced this pull request May 30, 2018

Update CHANGELOG for #4661

a45075e

ghost locked and limited conversation to collaborators Apr 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new attribute slow_start to aws_lb_target_group #4661

Add new attribute slow_start to aws_lb_target_group #4661

bboerst commented May 25, 2018 •

edited

Loading

bflad left a comment

bflad May 25, 2018

bflad May 25, 2018

bflad May 25, 2018

bflad May 25, 2018

bflad May 25, 2018

bboerst commented May 25, 2018 •

edited

Loading

bflad commented May 30, 2018

bflad left a comment

bflad May 30, 2018

bflad commented May 30, 2018

bflad commented May 31, 2018

ghost commented Apr 5, 2020

Add new attribute slow_start to aws_lb_target_group #4661

Add new attribute slow_start to aws_lb_target_group #4661

Conversation

bboerst commented May 25, 2018 • edited Loading

bflad left a comment

Choose a reason for hiding this comment

bflad May 25, 2018

Choose a reason for hiding this comment

bflad May 25, 2018

Choose a reason for hiding this comment

bflad May 25, 2018

Choose a reason for hiding this comment

bflad May 25, 2018

Choose a reason for hiding this comment

bflad May 25, 2018

Choose a reason for hiding this comment

bboerst commented May 25, 2018 • edited Loading

bflad commented May 30, 2018

bflad left a comment

Choose a reason for hiding this comment

bflad May 30, 2018

Choose a reason for hiding this comment

bflad commented May 30, 2018

bflad commented May 31, 2018

ghost commented Apr 5, 2020

bboerst commented May 25, 2018 •

edited

Loading

bboerst commented May 25, 2018 •

edited

Loading