Add support for tagging aws_dx_lag and aws_dx_connection resources #2990

ewbankkit · 2018-01-14T22:25:40Z

Acceptance tests:

make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsDxLag_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAwsDxLag_ -timeout 120m
=== RUN   TestAccAwsDxLag_basic
--- PASS: TestAccAwsDxLag_basic (38.46s)
=== RUN   TestAccAwsDxLag_tags
--- PASS: TestAccAwsDxLag_tags (50.75s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	89.222s

make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsDxConnection_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAwsDxConnection_ -timeout 120m
=== RUN   TestAccAwsDxConnection_basic
--- PASS: TestAccAwsDxConnection_basic (25.39s)
=== RUN   TestAccAwsDxConnection_tags
--- PASS: TestAccAwsDxConnection_tags (39.78s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	65.177s

ewbankkit · 2018-01-14T22:30:59Z

Added arn as an attribute to aws_dx_lag and aws_dx_connection as this is required for the tagging operations
Added some extra error checking for non-existent/already deleted LAGs and connections

radeksimko

Hi @ewbankkit
this looks overall very good. Just one question about connection state and a bunch of nitpicks, the former being a blocker.

radeksimko · 2018-02-12T15:37:05Z

aws/resource_aws_dx_connection.go

+		log.Printf("[WARN] Direct Connect connection (%s) not found, removing from state", d.Id())
+		d.SetId("")
+		return nil
+	}


I'd be ok with removing the resource from state if it's in ConnectionStateDeleted, but I think we should keep it in the state if it's still ConnectionStateDeleting or ConnectionStateRejected. The deletion may not finish successfully and rejection != deleted resource, AFAIK?

Yes, agreed. I'll fix.

radeksimko · 2018-02-12T15:40:24Z

aws/resource_aws_dx_connection_test.go

+  bandwidth = "1Gbps"
+  location = "EqSe2"
+}
+    `, n)


Nitpick: indentation.

radeksimko · 2018-02-12T15:40:31Z

aws/resource_aws_dx_connection_test.go

+    Usage = "original"
+  }
+}
+    `, n)


Nitpick: indentation.

radeksimko · 2018-02-12T15:40:39Z

aws/resource_aws_dx_connection_test.go

+    Usage = "changed"
+  }
+}
+    `, n)


Nitpick: indentation.

radeksimko · 2018-02-12T15:41:35Z

aws/resource_aws_dx_lag.go

 	}
+	terminalStates := map[string]bool{
+		directconnect.LagStateDeleted:  true,
+		directconnect.LagStateDeleting: true,


As above, I think it would be safer to only catch LagStateDeleted here.

radeksimko · 2018-02-12T15:42:50Z

aws/resource_aws_dx_lag_test.go

+  number_of_connections = 2
+  force_destroy = true
+}
+    `, n)


Nitpick: Indentation.

radeksimko · 2018-02-12T15:42:58Z

aws/resource_aws_dx_lag_test.go

+    Usage = "original"
+  }
+}
+    `, n)


Nitpick: Indentation.

radeksimko · 2018-02-12T15:43:02Z

aws/resource_aws_dx_lag_test.go

+    Usage = "changed"
+  }
+}
+    `, n)


Nitpick: Indentation.

… state.

ewbankkit · 2018-02-13T14:33:32Z

Re-ran acceptance tests after the change to aws/resource_aws_dx_connection.go:

make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsDxConnection_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAwsDxConnection_ -timeout 120m
=== RUN   TestAccAwsDxConnection_basic
--- PASS: TestAccAwsDxConnection_basic (26.48s)
=== RUN   TestAccAwsDxConnection_tags
--- PASS: TestAccAwsDxConnection_tags (45.95s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	81.800s

radeksimko

I just removed the check for Deleting state in LAG too, as discussed and pushed to your branch so we can get this in. I hope you don't mind.

👍

bflad · 2018-02-27T12:51:10Z

This has been released in version 1.10.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-04T17:20:48Z

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!

Add support for tagging aws_dx_lag and aws_dx_connection resources.

5281c8b

Ninir added the enhancement Requests to existing resources that expand the functionality or scope. label Jan 15, 2018

radeksimko added the service/directconnect Issues and PRs that pertain to the directconnect service. label Jan 16, 2018

radeksimko suggested changes Feb 12, 2018

View reviewed changes

radeksimko added the waiting-response Maintainers are waiting on response from community or contributor. label Feb 12, 2018

Kit Ewbank added 2 commits February 13, 2018 09:25

Fix indentation for configuration generation.

60a9812

Remove Direct Connect connection from state only if it's in 'Deleted'…

4a2a9a2

… state.

ghost added the size/XL Managed by automation to categorize the size of a PR. label Feb 13, 2018

radeksimko removed the waiting-response Maintainers are waiting on response from community or contributor. label Feb 13, 2018

Do not remove LAG if it's in Deleting state

3a2988d

ghost added the size/XL Managed by automation to categorize the size of a PR. label Feb 13, 2018

radeksimko approved these changes Feb 13, 2018

View reviewed changes

radeksimko merged commit 1787b2b into hashicorp:master Feb 13, 2018

bflad added this to the v1.10.0 milestone Feb 27, 2018

ewbankkit deleted the issue-2989 branch August 3, 2018 14:16

ghost locked and limited conversation to collaborators Apr 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for tagging aws_dx_lag and aws_dx_connection resources #2990

Add support for tagging aws_dx_lag and aws_dx_connection resources #2990

ewbankkit commented Jan 14, 2018

ewbankkit commented Jan 14, 2018

radeksimko left a comment

radeksimko Feb 12, 2018

ewbankkit Feb 13, 2018

radeksimko Feb 12, 2018

ewbankkit Feb 13, 2018

radeksimko Feb 12, 2018

ewbankkit Feb 13, 2018

radeksimko Feb 12, 2018

ewbankkit Feb 13, 2018

radeksimko Feb 12, 2018

radeksimko Feb 12, 2018

ewbankkit Feb 13, 2018

radeksimko Feb 12, 2018

ewbankkit Feb 13, 2018

radeksimko Feb 12, 2018

ewbankkit Feb 13, 2018

ewbankkit commented Feb 13, 2018

radeksimko left a comment

bflad commented Feb 27, 2018

ghost commented Apr 4, 2020

Add support for tagging aws_dx_lag and aws_dx_connection resources #2990

Add support for tagging aws_dx_lag and aws_dx_connection resources #2990

Conversation

ewbankkit commented Jan 14, 2018

ewbankkit commented Jan 14, 2018

radeksimko left a 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

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

ewbankkit commented Feb 13, 2018

radeksimko left a comment

Choose a reason for hiding this comment

bflad commented Feb 27, 2018

ghost commented Apr 4, 2020