New Resource: aws_dx_public_virtual_interface #3252

ewbankkit · 2018-02-04T23:17:09Z

aws/dx_vif.go and changes to aws/utils*.go shared with:

aws/tagsDX*.go copied from:

Add support for tagging aws_dx_lag and aws_dx_connection resources #2990

ewbankkit · 2018-02-04T23:27:28Z

Acceptance tests require an active Direct Connect connection which should be specified via the DX_CONNECTION_ID environment variable.

export DX_CONNECTION_ID=dxcon-xxxxxxxx
make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsDxPublicVirtualInterface_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAwsDxPublicVirtualInterface_ -timeout 120m
=== RUN   TestAccAwsDxPublicVirtualInterface_basic
--- PASS: TestAccAwsDxPublicVirtualInterface_basic (54.63s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	65.238s

radeksimko

Thanks for this PR @ewbankkit
It's looking good overall, just a few questions pending.

radeksimko · 2018-02-12T15:48:49Z

aws/dx_vif.go

+		"tags": tagsSchema(),
+	},
+)
+var dxVirtualInterfaceSchema = map[string]*schema.Schema{


Do you mind moving all the schema related code to the top of the file? It's just a convention we tend to follow.

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

aws/dx_vif.go

+	return isAWSErr(err, "DirectConnectClientException", "does not exist")
+}
+
+func dxVirtualInterfaceRead(d *schema.ResourceData, meta interface{}) (*directconnect.VirtualInterface, error) {


Do you mind reducing the interface to bare minimum here? e.g. (id string, conn *directconnect.DirectConnect)?

Exceptions apply, but ideally only CRUD should be responsible for modifying the state, i.e. calling d.SetId("").

Done. I also changed the checking to just remove the resource from state when the status is Deleted and not Deleting or Rejected, similar to #2990.

radeksimko · 2018-02-12T15:55:54Z

aws/resource_aws_dx_public_virtual_interface_test.go

+	connectionId := os.Getenv(key)
+	if connectionId == "" {
+		t.Skipf("Environment variable %s is not set", key)
+	}


Can't we just automatically create the DX connection as part of the test here?

The problem is that the connection needs to be in the Active state for any of the Virtual Interface functionality to succeed and activation of a connection requires sending a Letter of Activation to the network/CoLo provider and manual work on their side; i.e. plenty of time and money.
I have been testing on connections that already have active but aren't used for live traffic.

Hi @ewbankkit, we are just about to implement two new Direct Connect connections in eu-west-2, and in advance I created a test connection in a sandbox account so that I could map out the Virtual interfaces I needed. I have no intention of ever sending the LOA for this connection, but I am able to create and attach Virtual Interfaces to it, even though it is in the down state.

@X-Guardian What state does the Virtual Interface end up in? The resource won't be successfully created unless it ends up in available or verifying states.

@ewbankkit, I can create a Virtual Interface resource connected to either a DC Gateway or a VPG, and it creates successfully. Yes, it has a status of down of course, but does that matter?

@X-Guardian When I try and create a Direct Connect Connection and then a Public Virtual Interface on that connection in the acceptance test

resource "aws_dx_connection" "foo" { name = "%s" bandwidth = "1Gbps" location = "EqSe2" } resource "aws_dx_public_virtual_interface" "foo" { connection_id = "${aws_dx_connection.foo.id}" name = "%s" vlan = 4094 address_family = "ipv4" bgp_asn = 65352 customer_address = "175.45.176.1/30" amazon_address = "175.45.176.2/30" route_filter_prefixes = [ "210.52.109.0/24", "175.45.176.0/22" ] }

then the test fails

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsDxPublicVirtualInterface_' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws/ -v -run=TestAccAwsDxPublicVirtualInterface_ -timeout 120m === RUN TestAccAwsDxPublicVirtualInterface_basic --- FAIL: TestAccAwsDxPublicVirtualInterface_basic (20.73s) testing.go:518: Step 0 error: Error applying: 1 error(s) occurred: * aws_dx_public_virtual_interface.foo: 1 error(s) occurred: * aws_dx_public_virtual_interface.foo: Error creating Direct Connect public virtual interface: DirectConnectClientException: The specified Connection ID is not available status code: 400, request id: 0fb430bb-6c2f-11e8-8b43-895a753e5994 FAIL exit status 1 FAIL github.com/terraform-providers/terraform-provider-aws/aws 20.744s

The connection is in the requested state when the VIF is created on it.

@ewbankkit, when I have created a Direct Connect connection, it can take anything from 15 minutes to several hours to move on from the 'ordering' state, depending upon the partner chosen. Can you split your dx connection test from your virtual interface test?

@X-Guardian Yes, that's why the acceptance tests require the environment variable DX_CONNECTION_ID to be set to the ID of the connection created elsewhere.

radeksimko · 2018-02-12T15:56:11Z

aws/resource_aws_dx_public_virtual_interface_test.go

+	"175.45.176.0/22"
+  ]
+}
+    `, cid, n)


Nitpick: indentation.

radeksimko · 2018-02-12T15:56:16Z

aws/resource_aws_dx_public_virtual_interface_test.go

+    Environment = "test"
+  }
+}
+    `, cid, n)


Nitpick: indentation.

ewbankkit · 2018-02-13T16:26:57Z

I will propagate the changes to dx_vif.go to #3253, #3254 and #3255.

bcornils · 2018-05-10T20:51:19Z

Have we been able to address the conflicts here @bflad? @ewbankkit did you already look into that and I missed it? Beth

ewbankkit · 2018-05-11T20:48:04Z

Rebased and re-ran acceptance tests:

$ export DX_CONNECTION_ID=dxcon-xxxxxxxx
$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsDxPublicVirtualInterface_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAwsDxPublicVirtualInterface_ -timeout 120m
=== RUN   TestAccAwsDxPublicVirtualInterface_basic
--- PASS: TestAccAwsDxPublicVirtualInterface_basic (44.90s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws    45.528s

ewbankkit · 2018-05-11T20:50:20Z

@bcornils / @bflad: I have fixed the conflict and successfully run acceptance tests on a Direct Connection I have.
Once this PR is merged I can work on #3253, #3254 and #3255 which share the new dx_vif.go file and changes to utils.go and structure.go.

n3ph · 2018-06-20T14:30:32Z

I would love to see this merged, as this blocks the progress in #3255 and #3253..
We need this soooo badly..

ewbankkit · 2018-06-21T22:43:21Z

Rebased to remove conflict.

n3ph · 2018-06-22T17:19:07Z

Confirming acceptance tests in eu-central-1

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

ewbankkit · 2018-06-22T20:53:25Z

Changes after code review on #3253.
Acceptance tests:

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

n3ph · 2018-06-23T14:52:38Z

Acceptance tests in eu-central-1

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

n3ph · 2018-06-23T15:10:10Z

Here is a small test case

provider "aws" {
  max_retries = 3
  region      = "eu-central-1"
  profile     = "connect"
}

variable "connection_id" {
  default = "dxcon-zzzzzzzz"
}

resource "aws_dx_public_virtual_interface" "test" {
  connection_id = "${var.connection_id}"

  name           = "vif-test"
  vlan           = 1
  address_family = "ipv4"
  bgp_asn        = 65001

  customer_address = "175.45.176.1/30"
  amazon_address   = "175.45.176.2/30"

  route_filter_prefixes = [
    "210.52.109.0/24",
    "175.45.176.0/22",
  ]
}

$ terraform apply

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  + aws_dx_public_virtual_interface.test
      id:                               <computed>
      address_family:                   "ipv4"
      amazon_address:                   "175.45.176.2/30"
      arn:                              <computed>
      bgp_asn:                          "65001"
      bgp_auth_key:                     <computed>
      connection_id:                    "dxcon-zzzzzzzz"
      customer_address:                 "175.45.176.1/30"
      name:                             "vif-test"
      route_filter_prefixes.#:          "2"
      route_filter_prefixes.1752038751: "210.52.109.0/24"
      route_filter_prefixes.4290081960: "175.45.176.0/22"
      vlan:                             "1"


Plan: 1 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

aws_dx_public_virtual_interface.test: Creating...
  address_family:                   "" => "ipv4"
  amazon_address:                   "" => "175.45.176.2/30"
  arn:                              "" => "<computed>"
  bgp_asn:                          "" => "65001"
  bgp_auth_key:                     "" => "<computed>"
  connection_id:                    "" => "dxcon-zzzzzzzz"
  customer_address:                 "" => "175.45.176.1/30"
  name:                             "" => "vif-test"
  route_filter_prefixes.#:          "" => "2"
  route_filter_prefixes.1752038751: "" => "210.52.109.0/24"
  route_filter_prefixes.4290081960: "" => "175.45.176.0/22"
  vlan:                             "" => "1"
aws_dx_public_virtual_interface.test: Still creating... (10s elapsed)
aws_dx_public_virtual_interface.test: Creation complete after 11s (ID: dxvif-fhccct40)

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.

$ terraform apply
aws_dx_public_virtual_interface.test: Refreshing state... (ID: dxvif-fhccct40)

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

$ terraform destroy
aws_dx_public_virtual_interface.test: Refreshing state... (ID: dxvif-fhccct40)

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  - destroy

Terraform will perform the following actions:

  - aws_dx_public_virtual_interface.test


Plan: 0 to add, 0 to change, 1 to destroy.

Do you really want to destroy?
  Terraform will destroy all your managed infrastructure, as shown above.
  There is no undo. Only 'yes' will be accepted to confirm.

  Enter a value: yes

aws_dx_public_virtual_interface.test: Destroying... (ID: dxvif-fhccct40)
aws_dx_public_virtual_interface.test: Still destroying... (ID: dxvif-fhccct40, 10s elapsed)
aws_dx_public_virtual_interface.test: Destruction complete after 11s

Destroy complete! Resources: 1 destroyed.

bflad

Looks great, thanks @ewbankkit! I'll fix the merge conflicts from merging the other Direct Connect PRs 🚀

bflad · 2018-06-27T14:04:44Z

This has been released in version 1.25.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:12:41Z

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 the size/XL Managed by automation to categorize the size of a PR. label Feb 4, 2018

bflad added new-resource Introduces a new resource. service/directconnect Issues and PRs that pertain to the directconnect service. labels Feb 5, 2018

radeksimko reviewed Feb 12, 2018

View reviewed changes

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

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

radeksimko changed the title ~~Add 'aws_dx_public_virtual_interface' resource~~ New Resource: aws_dx_public_virtual_interface Mar 23, 2018

ewbankkit force-pushed the issue-3248 branch from f95c197 to f085537 Compare May 11, 2018 19:49

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

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

ewbankkit and others added 7 commits June 21, 2018 18:42

Add 'aws_dx_public_virtual_interface' resource.

34c1fdd

Fix indentation for configuration generation.

dad1c60

Move schema-related code to the top of 'dx_vif.go'.

48de747

Change interface to 'dxVirtualInterfaceRead'.

3175432

Minor refactoring.

bbd895e

Standardize resource name for testing.

9c9b676

Make consistent with Direct Connect gateway.

07324ec

ewbankkit force-pushed the issue-3248 branch from 5bcfac1 to 07324ec Compare June 21, 2018 22:43

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

Remove 'mergeSchemas'.

9fde05d

Changes after code review on hashicorp#3253.

adb1caa

bflad removed the waiting-response Maintainers are waiting on response from community or contributor. label Jun 22, 2018

bflad approved these changes Jun 25, 2018

View reviewed changes

bflad added this to the v1.25.0 milestone Jun 25, 2018

bflad merged commit adb1caa into hashicorp:master Jun 25, 2018

bflad added a commit that referenced this pull request Jun 25, 2018

Update CHANGELOG for #3252

fcae19c

ewbankkit deleted the issue-3248 branch June 25, 2018 14:01

bflad mentioned this pull request Jun 25, 2018

Add initial Direct Connect resources #876

Closed

ghost locked and limited conversation to collaborators Apr 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: aws_dx_public_virtual_interface #3252

New Resource: aws_dx_public_virtual_interface #3252

ewbankkit commented Feb 4, 2018 •

edited

Loading

ewbankkit commented Feb 4, 2018

radeksimko left a comment

radeksimko Feb 12, 2018

ewbankkit Feb 13, 2018

ewbankkit Feb 13, 2018

radeksimko Feb 12, 2018

ewbankkit Feb 13, 2018

ewbankkit Feb 13, 2018

radeksimko Feb 12, 2018

ewbankkit Feb 13, 2018

X-Guardian May 29, 2018

ewbankkit Jun 6, 2018

X-Guardian Jun 7, 2018

ewbankkit Jun 9, 2018 •

edited

Loading

X-Guardian Jun 10, 2018

ewbankkit Jun 11, 2018

radeksimko Feb 12, 2018

ewbankkit Feb 13, 2018

radeksimko Feb 12, 2018

ewbankkit Feb 13, 2018

ewbankkit commented Feb 13, 2018

bcornils commented May 10, 2018

ewbankkit commented May 11, 2018

ewbankkit commented May 11, 2018

n3ph commented Jun 20, 2018

ewbankkit commented Jun 21, 2018

n3ph commented Jun 22, 2018 •

edited

Loading

ewbankkit commented Jun 22, 2018

n3ph commented Jun 23, 2018 •

edited

Loading

n3ph commented Jun 23, 2018 •

edited

Loading

bflad left a comment

bflad commented Jun 27, 2018

ghost commented Apr 5, 2020

New Resource: aws_dx_public_virtual_interface #3252

New Resource: aws_dx_public_virtual_interface #3252

Conversation

ewbankkit commented Feb 4, 2018 • edited Loading

ewbankkit commented Feb 4, 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

ewbankkit Jun 9, 2018 • edited Loading

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

bcornils commented May 10, 2018

ewbankkit commented May 11, 2018

ewbankkit commented May 11, 2018

n3ph commented Jun 20, 2018

ewbankkit commented Jun 21, 2018

n3ph commented Jun 22, 2018 • edited Loading

ewbankkit commented Jun 22, 2018

n3ph commented Jun 23, 2018 • edited Loading

n3ph commented Jun 23, 2018 • edited Loading

bflad left a comment

Choose a reason for hiding this comment

bflad commented Jun 27, 2018

ghost commented Apr 5, 2020

ewbankkit commented Feb 4, 2018 •

edited

Loading

ewbankkit Jun 9, 2018 •

edited

Loading

n3ph commented Jun 22, 2018 •

edited

Loading

n3ph commented Jun 23, 2018 •

edited

Loading

n3ph commented Jun 23, 2018 •

edited

Loading