provider/aws: Make sure that VPC Peering Connection in a failed state returns an error. #9038

kwilczynski · 2016-09-25T14:42:08Z

This commit adds simple logic which allows for a VPC Peering Connection
that is in a failed state (e.g. due to an overlapping IP address ranges,
etc.), to report such failed state as an error, rather then waiting for
the time out to occur.

Signed-off-by: Krzysztof Wilczynski [email protected]

kwilczynski · 2016-09-25T14:42:44Z

Resolves #8933.

kwilczynski · 2016-09-25T14:43:00Z

The acceptance test is passing:

 make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSVPCPeeringConnection'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/09/25 15:29:23 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSVPCPeeringConnection -timeout 120m
=== RUN   TestAccAWSVPCPeeringConnection_importBasic
--- PASS: TestAccAWSVPCPeeringConnection_importBasic (42.63s)
=== RUN   TestAccAWSVPCPeeringConnection_basic
--- PASS: TestAccAWSVPCPeeringConnection_basic (44.20s)
=== RUN   TestAccAWSVPCPeeringConnection_plan
--- PASS: TestAccAWSVPCPeeringConnection_plan (42.61s)
=== RUN   TestAccAWSVPCPeeringConnection_tags
--- PASS: TestAccAWSVPCPeeringConnection_tags (46.10s)
=== RUN   TestAccAWSVPCPeeringConnection_options
--- PASS: TestAccAWSVPCPeeringConnection_options (75.21s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    250.764s

kwilczynski · 2016-09-25T14:43:37Z

I need to add a test that covers the failed state.

…ror. This commit adds simple logic which allows for a VPC Peering Connection that is in a failed state (e.g. due to an overlapping IP address ranges, etc.), to report such failed state as an error, rather then waiting for the time out to occur. Signed-off-by: Krzysztof Wilczynski <[email protected]>

kwilczynski · 2016-09-25T14:56:40Z

Generally, the idea is to bubble to error, as per:

$ ./terraform apply
aws_vpc.bar: Refreshing state... (ID: vpc-9be100f3)
aws_vpc.foo: Refreshing state... (ID: vpc-9ae100f2)
aws_vpc_peering_connection.foo: Refreshing state... (ID: pcx-6161b308)
aws_vpc_peering_connection.foo: Creating...
  accept_status: "" => "<computed>"
  accepter.#:    "" => "<computed>"
  auto_accept:   "" => "true"
  peer_owner_id: "" => "635543228030"
  peer_vpc_id:   "" => "vpc-9be100f3"
  requester.#:   "" => "<computed>"
  status:        "" => "<computed>"
  vpc_id:        "" => "vpc-9ae100f2"
Error applying plan:

1 error(s) occurred:

* aws_vpc_peering_connection.foo: Error waiting for VPC Peering Connection (pcx-5661b33f) to become available: Failed due to incorrect VPC-ID, Account ID, or overlapping CIDR range

Terraform does not automatically rollback in the face of errors.
Instead, your Terraform state file has been partially updated with
any resources that successfully completed. Please address the error
above and apply again to incrementally change your infrastructure.

Above has overlapping IP address ranges (CIDR), and so rightfully it fails.

kwilczynski · 2016-09-25T16:24:08Z

I am not sure how to test for a failure here. I've added a test, but it does not seem to execute, for example:

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSVPCPeeringConnection_failedState'
==> Checking that code complies with gofmt requirements...
:go generate $(go list ./... | grep -v /terraform/vendor/)
2016/09/25 17:17:34 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSVPCPeeringConnection_failedState -timeout 120m
=== RUN   TestAccAWSVPCPeeringConnection_failedState
--- FAIL: TestAccAWSVPCPeeringConnection_failedState (28.66s)
    testing.go:265: Step 0 error: Error applying: 1 error(s) occurred:

        * aws_vpc_peering_connection.foo: Error waiting for VPC Peering Connection (pcx-b1ec55d8) to become available: Failed due to incorrect VPC-ID, Account ID, or overlapping CIDR range
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    28.678s
make: *** [testacc] Error 1

Maybe it is not possible to do it.

Signed-off-by: Krzysztof Wilczynski <[email protected]>

kwilczynski · 2016-09-26T08:24:49Z

I've added a test that checks for failure, and the acceptance test is passing:

 make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSVPCPeeringConnection_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/09/26 09:11:31 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSVPCPeeringConnection_ -timeout 120m
=== RUN   TestAccAWSVPCPeeringConnection_importBasic
--- PASS: TestAccAWSVPCPeeringConnection_importBasic (37.24s)
=== RUN   TestAccAWSVPCPeeringConnection_basic
--- PASS: TestAccAWSVPCPeeringConnection_basic (38.33s)
=== RUN   TestAccAWSVPCPeeringConnection_plan
--- PASS: TestAccAWSVPCPeeringConnection_plan (36.11s)
=== RUN   TestAccAWSVPCPeeringConnection_tags
--- PASS: TestAccAWSVPCPeeringConnection_tags (38.53s)
=== RUN   TestAccAWSVPCPeeringConnection_options
--- PASS: TestAccAWSVPCPeeringConnection_options (66.57s)
=== RUN   TestAccAWSVPCPeeringConnection_failedState
--- PASS: TestAccAWSVPCPeeringConnection_failedState (23.81s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    240.613s

kwilczynski · 2016-09-26T08:25:00Z

Over to you @stack72 🚀

stack72 · 2016-09-26T08:31:04Z

builtin/providers/aws/resource_aws_vpc_peering_connection.go

 				Type:     schema.TypeString,
 				Optional: true,
 				ForceNew: true,
 				Computed: true,
 			},
-			"peer_vpc_id": &schema.Schema{
+			"peer_vpc_id": {


Love the cleanup :)

stack72 · 2016-09-26T08:31:14Z

builtin/providers/aws/resource_aws_vpc_peering_connection.go

-		return fmt.Errorf(
-			"Error waiting for VPC Peering Connection (%s) to become available: %s",
-			d.Id(), err)
+		return errwrap.Wrapf(fmt.Sprintf(


It does feel a bit cumbersome with the fmt.Sprintf, though.

stack72 · 2016-09-26T08:32:21Z

builtin/providers/aws/resource_aws_vpc_peering_connection.go

@@ -139,23 +142,23 @@ func resourceAwsVPCPeeringRead(d *schema.ResourceData, meta interface{}) error {
 	// When the VPC Peering Connection is pending acceptance,
 	// the details about accepter and/or requester peering
 	// options would not be included in the response.
-	if pc.AccepterVpcInfo != nil && pc.AccepterVpcInfo.PeeringOptions != nil {
+	if pc.AccepterVpcInfo.PeeringOptions != nil {


Can we guarantee that pc.AccepterVpcInfo != nil?

There is no need for this extra check. Above, we set items that are being read from the structure too which are essential e.g. d.Set("peer_vpc_id", pc.AccepterVpcInfo.VpcId). Should this fail it would be long before we even get to peering options.

stack72 · 2016-09-27T16:56:51Z

Walked through this review with @kwilczynski and he pointed out the rationale behind what has been done here. The tests pass so this looks good to merge:

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSVPCPeeringConnection_'                                                                                                    ✹
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/09/27 17:50:11 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSVPCPeeringConnection_ -timeout 120m
=== RUN   TestAccAWSVPCPeeringConnection_importBasic
--- PASS: TestAccAWSVPCPeeringConnection_importBasic (39.83s)
=== RUN   TestAccAWSVPCPeeringConnection_basic
--- PASS: TestAccAWSVPCPeeringConnection_basic (41.60s)
=== RUN   TestAccAWSVPCPeeringConnection_plan
--- PASS: TestAccAWSVPCPeeringConnection_plan (39.13s)
=== RUN   TestAccAWSVPCPeeringConnection_tags
--- PASS: TestAccAWSVPCPeeringConnection_tags (43.97s)
=== RUN   TestAccAWSVPCPeeringConnection_options
--- PASS: TestAccAWSVPCPeeringConnection_options (68.40s)
=== RUN   TestAccAWSVPCPeeringConnection_failedState
--- PASS: TestAccAWSVPCPeeringConnection_failedState (26.34s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    259.297s

ghost · 2020-04-22T01:51:03Z

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

kwilczynski changed the title ~~Make sure that VPC Peering Connection in a failed state returns an error.~~ provider/aws: Make sure that VPC Peering Connection in a failed state returns an error. Sep 25, 2016

kwilczynski changed the title ~~provider/aws: Make sure that VPC Peering Connection in a failed state returns an error.~~ [WIP] provider/aws: Make sure that VPC Peering Connection in a failed state returns an error. Sep 25, 2016

kwilczynski force-pushed the feature/error-reporting-aws_vpc_peering_connection branch from a7a87da to 9859064 Compare September 25, 2016 14:46

kwilczynski force-pushed the feature/error-reporting-aws_vpc_peering_connection branch from 9859064 to a58650c Compare September 25, 2016 14:50

jen20 added bug provider/aws labels Sep 25, 2016

Add test to check for failed state of the VPC Peering Connection.

a2a2de5

Signed-off-by: Krzysztof Wilczynski <[email protected]>

kwilczynski changed the title ~~[WIP] provider/aws: Make sure that VPC Peering Connection in a failed state returns an error.~~ provider/aws: Make sure that VPC Peering Connection in a failed state returns an error. Sep 26, 2016

stack72 reviewed Sep 26, 2016

View reviewed changes

stack72 merged commit 1cf9f41 into hashicorp:master Sep 27, 2016

stack72 mentioned this pull request Sep 27, 2016

Terraform does not bubble up VPC Peering errors #8933

Closed

ghost locked and limited conversation to collaborators Apr 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

provider/aws: Make sure that VPC Peering Connection in a failed state returns an error. #9038

provider/aws: Make sure that VPC Peering Connection in a failed state returns an error. #9038

kwilczynski commented Sep 25, 2016

kwilczynski commented Sep 25, 2016

kwilczynski commented Sep 25, 2016

kwilczynski commented Sep 25, 2016

kwilczynski commented Sep 25, 2016

kwilczynski commented Sep 25, 2016

kwilczynski commented Sep 26, 2016

kwilczynski commented Sep 26, 2016

stack72 Sep 26, 2016

stack72 Sep 26, 2016

kwilczynski Sep 26, 2016

kwilczynski Sep 26, 2016

stack72 Sep 26, 2016

kwilczynski Sep 26, 2016 •

edited

Loading

stack72 commented Sep 27, 2016

ghost commented Apr 22, 2020

provider/aws: Make sure that VPC Peering Connection in a failed state returns an error. #9038

provider/aws: Make sure that VPC Peering Connection in a failed state returns an error. #9038

Conversation

kwilczynski commented Sep 25, 2016

kwilczynski commented Sep 25, 2016

kwilczynski commented Sep 25, 2016

kwilczynski commented Sep 25, 2016

kwilczynski commented Sep 25, 2016

kwilczynski commented Sep 25, 2016

kwilczynski commented Sep 26, 2016

kwilczynski commented Sep 26, 2016

stack72 Sep 26, 2016

Choose a reason for hiding this comment

stack72 Sep 26, 2016

Choose a reason for hiding this comment

kwilczynski Sep 26, 2016

Choose a reason for hiding this comment

kwilczynski Sep 26, 2016

Choose a reason for hiding this comment

stack72 Sep 26, 2016

Choose a reason for hiding this comment

kwilczynski Sep 26, 2016 • edited Loading

Choose a reason for hiding this comment

stack72 commented Sep 27, 2016

ghost commented Apr 22, 2020

kwilczynski Sep 26, 2016 •

edited

Loading