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.

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

Add peer_region to vpc peering connection #2508

Merged
merged 10 commits into from
Jan 8, 2018

Conversation

kl4w
Copy link
Contributor

@kl4w kl4w commented Dec 1, 2017

#2484

TF_ACC=1 go test ./aws -v -run=TestAccAWSVPCPeeringConnection_region -timeout 120m                                                                                                                                     ~/D/k/r/g/s/g/t/terraform-provider-aws inter-region-vpc-peer
=== RUN   TestAccAWSVPCPeeringConnection_region
--- PASS: TestAccAWSVPCPeeringConnection_region (27.57s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	27.615s

@ewbankkit
Copy link
Contributor

@kl4w
Copy link
Contributor Author

kl4w commented Dec 1, 2017

Should I do it as part of this PR, or separate it?

@radeksimko radeksimko added the enhancement Requests to existing resources that expand the functionality or scope. label Dec 1, 2017
@ewbankkit
Copy link
Contributor

Seems like a pretty small change so I'd say do it as part of this PR.

@kl4w
Copy link
Contributor Author

kl4w commented Dec 2, 2017

rebased and found an issue (not exactly sure why the test before didn't pick this up).
TF_ACC=1 go test ./aws -v -run=TestAccAWSVPCPeeringConnection_region -timeout 120m

Error: Check failed: Found the VPC Peering Connection in an unexpected state: {
		  AccepterVpcInfo: {
		    OwnerId: "1234",
		    Region: "us-east-1",
		    VpcId: "vpc-5d80e825"
		  },
		  RequesterVpcInfo: {
		    OwnerId: "1234",
		    Region: "us-west-2",
		    VpcId: "vpc-71879817"
		  },
		  Status: {
		    Code: "deleting",
		    Message: "Deleting Initiated by 1234"
		  },
		  VpcPeeringConnectionId: "pcx-068dcbfdf19cf099d"
		}

may be a timeout issue?

@kl4w
Copy link
Contributor Author

kl4w commented Dec 3, 2017

Found a couple of issues and would like some input for the below errors:

TF_ACC=1 go test ./aws -v -run=TestAccAWSVPCPeeringConnection -timeout 120m
=== RUN   TestAccAWSVPCPeeringConnection_importBasic
--- PASS: TestAccAWSVPCPeeringConnection_importBasic (68.59s)
=== RUN   TestAccAWSVPCPeeringConnectionAccepter_sameRegion
--- PASS: TestAccAWSVPCPeeringConnectionAccepter_sameRegion (45.91s)
=== RUN   TestAccAWSVPCPeeringConnectionAccepter_differentRegion
--- FAIL: TestAccAWSVPCPeeringConnectionAccepter_differentRegion (25.63s)
	testing.go:503: Step 0 error: Error applying: 1 error(s) occurred:

		* aws_vpc_peering_connection_accepter.peer: 1 error(s) occurred:

		* aws_vpc_peering_connection_accepter.peer: Unable to accept VPC Peering Connection: OperationNotPermitted: Incorrect region (us-west-2) specified for this request. VPC peering connection pcx-0e1b4b8ba14e6e2a9 must be accepted in region us-east-1
			status code: 400, request id: fe232759-0bb6-4987-8b04-5dfa8857b60d
=== RUN   TestAccAWSVPCPeeringConnection_basic
--- PASS: TestAccAWSVPCPeeringConnection_basic (38.28s)
=== RUN   TestAccAWSVPCPeeringConnection_plan
--- PASS: TestAccAWSVPCPeeringConnection_plan (34.98s)
=== RUN   TestAccAWSVPCPeeringConnection_tags
--- PASS: TestAccAWSVPCPeeringConnection_tags (40.90s)
=== RUN   TestAccAWSVPCPeeringConnection_options
--- PASS: TestAccAWSVPCPeeringConnection_options (64.33s)
=== RUN   TestAccAWSVPCPeeringConnection_failedState
--- PASS: TestAccAWSVPCPeeringConnection_failedState (24.87s)
=== RUN   TestAccAWSVPCPeeringConnection_peerRegionAndAutoAccept
--- PASS: TestAccAWSVPCPeeringConnection_peerRegionAndAutoAccept (21.43s)
=== RUN   TestAccAWSVPCPeeringConnection_region
--- FAIL: TestAccAWSVPCPeeringConnection_region (37.76s)
	testing.go:563: Error destroying resource! WARNING: Dangling resources
		may exist. The full state and error is shown below.

		Error: Check failed: Found the VPC Peering Connection in an unexpected state: {
		  AccepterVpcInfo: {
		    OwnerId: "blah",
		    Region: "us-east-1",
		    VpcId: "vpc-7b6d0703"
		  },
		  RequesterVpcInfo: {
		    OwnerId: "blah",
		    Region: "us-west-2",
		    VpcId: "vpc-bdb1afdb"
		  },
		  Status: {
		    Code: "rejected",
		    Message: "Rejected by blah"
		  },
		  VpcPeeringConnectionId: "pcx-0b0721f1be30fd4c0"
		}

		State: <no state>
FAIL
exit status 1
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	402.734s

TF_LOG=DEBUG TF_ACC=1 go test ./aws -v -run=TestAccAWSVPCPeeringConnection_region -timeout 120m
The dangling resource seems to happen when attempting to delete aws_vpc.bar. The request looks like it's going to eu-west-2 rather than us-east-1

-----------------------------------------------------
2017/12/03 09:34:47 [DEBUG] [aws-sdk-go] DEBUG: Request ec2/DeleteVpc Details:
---[ REQUEST POST-SIGN ]-----------------------------
POST / HTTP/1.1
Host: ec2.us-west-2.amazonaws.com
User-Agent: aws-sdk-go/1.12.38 (go1.9.2; darwin; amd64) APN/1.0 HashiCorp/1.0 Terraform/0.11.0-beta1
Authorization: AWS4-HMAC-SHA256 Credential=blah/20171203/us-west-2/ec2/aws4_request

TF_LOG=DEBUG TF_ACC=1 go test ./aws -v -run=TestAccAWSVPCPeeringConnectionAccepter_differentRegion -timeout 120m

when attempting to call accept aws_vpc_peering_connection_accepter.peer the request seems to be going to eu-west-2 rather than us-east-1.

2017/12/03 09:45:14 [DEBUG] [aws-sdk-go] DEBUG: Request ec2/AcceptVpcPeeringConnection Details:
---[ REQUEST POST-SIGN ]-----------------------------
POST / HTTP/1.1
Host: ec2.us-west-2.amazonaws.com
User-Agent: aws-sdk-go/1.12.38 (go1.9.2; darwin; amd64) APN/1.0 HashiCorp/1.0 Terraform/0.11.0-beta1
Authorization: AWS4-HMAC-SHA256 Credential=blah/20171203/us-west-2/ec2/aws4_request

@kl4w
Copy link
Contributor Author

kl4w commented Dec 8, 2017

TF_ACC=1 go test ./aws -v -run=TestAccAWSVPCPeeringConnection -timeout 120m                                                                                                                                         ~/D/k/r/g/s/g/t/terraform-provider-aws inter-region-vpc-peer
=== RUN   TestAccAWSVPCPeeringConnection_importBasic
--- PASS: TestAccAWSVPCPeeringConnection_importBasic (25.47s)
=== RUN   TestAccAWSVPCPeeringConnectionAccepter_sameRegion
--- PASS: TestAccAWSVPCPeeringConnectionAccepter_sameRegion (25.34s)
=== RUN   TestAccAWSVPCPeeringConnectionAccepter_differentRegion
--- PASS: TestAccAWSVPCPeeringConnectionAccepter_differentRegion (31.64s)
=== RUN   TestAccAWSVPCPeeringConnection_basic
--- PASS: TestAccAWSVPCPeeringConnection_basic (23.71s)
=== RUN   TestAccAWSVPCPeeringConnection_plan
--- PASS: TestAccAWSVPCPeeringConnection_plan (22.57s)
=== RUN   TestAccAWSVPCPeeringConnection_tags
--- PASS: TestAccAWSVPCPeeringConnection_tags (25.69s)
=== RUN   TestAccAWSVPCPeeringConnection_options
--- PASS: TestAccAWSVPCPeeringConnection_options (40.32s)
=== RUN   TestAccAWSVPCPeeringConnection_failedState
--- PASS: TestAccAWSVPCPeeringConnection_failedState (15.18s)
=== RUN   TestAccAWSVPCPeeringConnection_peerRegionAndAutoAccept
--- PASS: TestAccAWSVPCPeeringConnection_peerRegionAndAutoAccept (12.69s)
=== RUN   TestAccAWSVPCPeeringConnection_region
--- PASS: TestAccAWSVPCPeeringConnection_region (26.77s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	249.420s

@scrothers
Copy link
Contributor

Is this going to make it in soon? Or is there documentation somewhere on how/when PRs get merged by Hashicorp?

@jmvbxx
Copy link

jmvbxx commented Dec 18, 2017

Is there an update on this issue? Any idea when it's going to be merged?

if _, ok := d.GetOk("auto_accept"); ok {
return fmt.Errorf("peer_region cannot be set whilst auto_accept is true when creating a vpc peering connection")
}
createOpts.SetPeerRegion(v.(string))
Copy link
Contributor

Choose a reason for hiding this comment

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

The convention has been to address the structure's field directly, i.e.
createOpts.PeerRegion = aws.String(v.(string))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ewbankkit Missed that, thanks!

About to push the change up, should I rebase and recomment the test results as well?

@erikbor
Copy link

erikbor commented Jan 5, 2018

Any updates on when this will be merged? Thanks!

@kl4w
Copy link
Contributor Author

kl4w commented Jan 5, 2018

@ewbankkit how is this PR looking? Is this ready to be merged?

@ewbankkit
Copy link
Contributor

I have no further comments but it is not my call to make.

@scrothers
Copy link
Contributor

@radeksimko You seem to be the one doing some of the most recent merges in PRs. Can you please provide some insight into how PRs are merged and when?

It would help us understand the process so we don't seem so noisy or bothersome :)

@jen20 jen20 added the reinvent label Jan 8, 2018
@jen20
Copy link
Contributor

jen20 commented Jan 8, 2018

This looks good to me, and the tests pass. There are a few minor things which I will fix up in a supplementary commit locally before merging.

However, it turns out that the resources for the acceptor are misspelled accepter - we should address that before the next release by adding aliases for the correct naming, and leave the current one for backward compatibility.

@jen20 jen20 merged commit eb3085f into hashicorp:master Jan 8, 2018
@bflad
Copy link
Contributor

bflad commented Jan 12, 2018

This has been released in terraform-provider-aws version 1.7.0. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

scrothers added a commit to scrothers/terraform-provider-aws that referenced this pull request Jan 12, 2018
The previous phrase "AWS only supports VPC peering within the same AWS region." existed in the documentation, it's no longer accurate since hashicorp#2508 opened up multi-regional peering capabilities and was merged and released in provider version 1.7.
@kl4w kl4w deleted the inter-region-vpc-peer branch April 20, 2018 13:57
@ghost
Copy link

ghost commented Apr 6, 2020

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 ghost locked and limited conversation to collaborators Apr 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants