From 795970d5a267c665d3ff9720d39473e7b448ddb6 Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Mon, 9 Mar 2015 17:11:30 -0500 Subject: [PATCH 1/3] Give route table assoc it's own copy of this method for now --- .../resource_aws_route_table_association.go | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/builtin/providers/aws/resource_aws_route_table_association.go b/builtin/providers/aws/resource_aws_route_table_association.go index 846836008..4b39a79b7 100644 --- a/builtin/providers/aws/resource_aws_route_table_association.go +++ b/builtin/providers/aws/resource_aws_route_table_association.go @@ -4,6 +4,7 @@ import ( "fmt" "log" + "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/helper/schema" "github.com/mitchellh/goamz/ec2" ) @@ -129,3 +130,29 @@ func resourceAwsRouteTableAssociationDelete(d *schema.ResourceData, meta interfa return nil } + +// TODO: remove this method when converting to aws-sdk-go +// resourceAwsRouteTableStateRefreshFunc returns a resource.StateRefreshFunc that is used to watch +// a RouteTable. +func resourceAwsRouteTableStateRefreshFunc(conn *ec2.EC2, id string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + resp, err := conn.DescribeRouteTables([]string{id}, ec2.NewFilter()) + if err != nil { + if ec2err, ok := err.(*ec2.Error); ok && ec2err.Code == "InvalidRouteTableID.NotFound" { + resp = nil + } else { + log.Printf("Error on RouteTableStateRefresh: %s", err) + return nil, "", err + } + } + + if resp == nil { + // Sometimes AWS just has consistency issues and doesn't see + // our instance yet. Return an empty state. + return nil, "", nil + } + + rt := &resp.RouteTables[0] + return rt, "ready", nil + } +} From 30f401eab7bf285cdd8b0f48819881d362dec24e Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Tue, 10 Mar 2015 10:23:14 -0500 Subject: [PATCH 2/3] provider/aws: Convert AWS Route Table to aws-sdk-go --- .../providers/aws/resource_aws_route_table.go | 98 +++++++++++-------- .../aws/resource_aws_route_table_test.go | 10 +- 2 files changed, 66 insertions(+), 42 deletions(-) diff --git a/builtin/providers/aws/resource_aws_route_table.go b/builtin/providers/aws/resource_aws_route_table.go index 9d01218b0..76bcd17a2 100644 --- a/builtin/providers/aws/resource_aws_route_table.go +++ b/builtin/providers/aws/resource_aws_route_table.go @@ -6,10 +6,11 @@ import ( "log" "time" + "github.com/hashicorp/aws-sdk-go/aws" + "github.com/hashicorp/aws-sdk-go/gen/ec2" "github.com/hashicorp/terraform/helper/hashcode" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/helper/schema" - "github.com/mitchellh/goamz/ec2" ) func resourceAwsRouteTable() *schema.Resource { @@ -61,11 +62,11 @@ func resourceAwsRouteTable() *schema.Resource { } func resourceAwsRouteTableCreate(d *schema.ResourceData, meta interface{}) error { - ec2conn := meta.(*AWSClient).ec2conn + ec2conn := meta.(*AWSClient).awsEC2conn // Create the routing table - createOpts := &ec2.CreateRouteTable{ - VpcId: d.Get("vpc_id").(string), + createOpts := &ec2.CreateRouteTableRequest{ + VPCID: aws.String(d.Get("vpc_id").(string)), } log.Printf("[DEBUG] RouteTable create config: %#v", createOpts) @@ -75,8 +76,8 @@ func resourceAwsRouteTableCreate(d *schema.ResourceData, meta interface{}) error } // Get the ID and store it - rt := &resp.RouteTable - d.SetId(rt.RouteTableId) + rt := resp.RouteTable + d.SetId(*rt.RouteTableID) log.Printf("[INFO] Route Table ID: %s", d.Id()) // Wait for the route table to become available @@ -86,7 +87,7 @@ func resourceAwsRouteTableCreate(d *schema.ResourceData, meta interface{}) error stateConf := &resource.StateChangeConf{ Pending: []string{"pending"}, Target: "ready", - Refresh: resourceAwsRouteTableStateRefreshFunc(ec2conn, d.Id()), + Refresh: resourceAwsRouteTableStateRefreshFuncSDK(ec2conn, d.Id()), Timeout: 1 * time.Minute, } if _, err := stateConf.WaitForState(); err != nil { @@ -99,9 +100,9 @@ func resourceAwsRouteTableCreate(d *schema.ResourceData, meta interface{}) error } func resourceAwsRouteTableRead(d *schema.ResourceData, meta interface{}) error { - ec2conn := meta.(*AWSClient).ec2conn + ec2conn := meta.(*AWSClient).awsEC2conn - rtRaw, _, err := resourceAwsRouteTableStateRefreshFunc(ec2conn, d.Id())() + rtRaw, _, err := resourceAwsRouteTableStateRefreshFuncSDK(ec2conn, d.Id())() if err != nil { return err } @@ -110,40 +111,48 @@ func resourceAwsRouteTableRead(d *schema.ResourceData, meta interface{}) error { } rt := rtRaw.(*ec2.RouteTable) - d.Set("vpc_id", rt.VpcId) + d.Set("vpc_id", rt.VPCID) // Create an empty schema.Set to hold all routes route := &schema.Set{F: resourceAwsRouteTableHash} // Loop through the routes and add them to the set for _, r := range rt.Routes { - if r.GatewayId == "local" { + if r.GatewayID != nil && *r.GatewayID == "local" { continue } - if r.Origin == "EnableVgwRoutePropagation" { + if r.Origin != nil && *r.Origin == "EnableVgwRoutePropagation" { continue } m := make(map[string]interface{}) - m["cidr_block"] = r.DestinationCidrBlock - m["gateway_id"] = r.GatewayId - m["instance_id"] = r.InstanceId - m["vpc_peering_connection_id"] = r.VpcPeeringConnectionId + if r.DestinationCIDRBlock != nil { + m["cidr_block"] = *r.DestinationCIDRBlock + } + if r.GatewayID != nil { + m["gateway_id"] = *r.GatewayID + } + if r.InstanceID != nil { + m["instance_id"] = *r.InstanceID + } + if r.VPCPeeringConnectionID != nil { + m["vpc_peering_connection_id"] = *r.VPCPeeringConnectionID + } route.Add(m) } d.Set("route", route) // Tags - d.Set("tags", tagsToMap(rt.Tags)) + d.Set("tags", tagsToMapSDK(rt.Tags)) return nil } func resourceAwsRouteTableUpdate(d *schema.ResourceData, meta interface{}) error { - ec2conn := meta.(*AWSClient).ec2conn + ec2conn := meta.(*AWSClient).awsEC2conn // Check if the route set as a whole has changed if d.HasChange("route") { @@ -159,8 +168,10 @@ func resourceAwsRouteTableUpdate(d *schema.ResourceData, meta interface{}) error log.Printf( "[INFO] Deleting route from %s: %s", d.Id(), m["cidr_block"].(string)) - _, err := ec2conn.DeleteRoute( - d.Id(), m["cidr_block"].(string)) + err := ec2conn.DeleteRoute(&ec2.DeleteRouteRequest{ + RouteTableID: aws.String(d.Id()), + DestinationCIDRBlock: aws.String(m["cidr_block"].(string)), + }) if err != nil { return err } @@ -174,17 +185,16 @@ func resourceAwsRouteTableUpdate(d *schema.ResourceData, meta interface{}) error for _, route := range nrs.List() { m := route.(map[string]interface{}) - opts := ec2.CreateRoute{ - RouteTableId: d.Id(), - DestinationCidrBlock: m["cidr_block"].(string), - GatewayId: m["gateway_id"].(string), - InstanceId: m["instance_id"].(string), - VpcPeeringConnectionId: m["vpc_peering_connection_id"].(string), + opts := ec2.CreateRouteRequest{ + RouteTableID: aws.String(d.Id()), + DestinationCIDRBlock: aws.String(m["cidr_block"].(string)), + GatewayID: aws.String(m["gateway_id"].(string)), + InstanceID: aws.String(m["instance_id"].(string)), + VPCPeeringConnectionID: aws.String(m["vpc_peering_connection_id"].(string)), } log.Printf("[INFO] Creating route for %s: %#v", d.Id(), opts) - _, err := ec2conn.CreateRoute(&opts) - if err != nil { + if err := ec2conn.CreateRoute(&opts); err != nil { return err } @@ -193,7 +203,7 @@ func resourceAwsRouteTableUpdate(d *schema.ResourceData, meta interface{}) error } } - if err := setTags(ec2conn, d); err != nil { + if err := setTagsSDK(ec2conn, d); err != nil { return err } else { d.SetPartial("tags") @@ -203,11 +213,11 @@ func resourceAwsRouteTableUpdate(d *schema.ResourceData, meta interface{}) error } func resourceAwsRouteTableDelete(d *schema.ResourceData, meta interface{}) error { - ec2conn := meta.(*AWSClient).ec2conn + ec2conn := meta.(*AWSClient).awsEC2conn // First request the routing table since we'll have to disassociate // all the subnets first. - rtRaw, _, err := resourceAwsRouteTableStateRefreshFunc(ec2conn, d.Id())() + rtRaw, _, err := resourceAwsRouteTableStateRefreshFuncSDK(ec2conn, d.Id())() if err != nil { return err } @@ -218,16 +228,22 @@ func resourceAwsRouteTableDelete(d *schema.ResourceData, meta interface{}) error // Do all the disassociations for _, a := range rt.Associations { - log.Printf("[INFO] Disassociating association: %s", a.AssociationId) - if _, err := ec2conn.DisassociateRouteTable(a.AssociationId); err != nil { + log.Printf("[INFO] Disassociating association: %s", *a.RouteTableAssociationID) + err := ec2conn.DisassociateRouteTable(&ec2.DisassociateRouteTableRequest{ + AssociationID: a.RouteTableAssociationID, + }) + if err != nil { return err } } // Delete the route table log.Printf("[INFO] Deleting Route Table: %s", d.Id()) - if _, err := ec2conn.DeleteRouteTable(d.Id()); err != nil { - ec2err, ok := err.(*ec2.Error) + err = ec2conn.DeleteRouteTable(&ec2.DeleteRouteTableRequest{ + RouteTableID: aws.String(d.Id()), + }) + if err != nil { + ec2err, ok := err.(aws.APIError) if ok && ec2err.Code == "InvalidRouteTableID.NotFound" { return nil } @@ -243,7 +259,7 @@ func resourceAwsRouteTableDelete(d *schema.ResourceData, meta interface{}) error stateConf := &resource.StateChangeConf{ Pending: []string{"ready"}, Target: "", - Refresh: resourceAwsRouteTableStateRefreshFunc(ec2conn, d.Id()), + Refresh: resourceAwsRouteTableStateRefreshFuncSDK(ec2conn, d.Id()), Timeout: 1 * time.Minute, } if _, err := stateConf.WaitForState(); err != nil { @@ -275,13 +291,15 @@ func resourceAwsRouteTableHash(v interface{}) int { return hashcode.String(buf.String()) } -// resourceAwsRouteTableStateRefreshFunc returns a resource.StateRefreshFunc that is used to watch +// resourceAwsRouteTableStateRefreshFuncSDK returns a resource.StateRefreshFunc that is used to watch // a RouteTable. -func resourceAwsRouteTableStateRefreshFunc(conn *ec2.EC2, id string) resource.StateRefreshFunc { +func resourceAwsRouteTableStateRefreshFuncSDK(conn *ec2.EC2, id string) resource.StateRefreshFunc { return func() (interface{}, string, error) { - resp, err := conn.DescribeRouteTables([]string{id}, ec2.NewFilter()) + resp, err := conn.DescribeRouteTables(&ec2.DescribeRouteTablesRequest{ + RouteTableIDs: []string{id}, + }) if err != nil { - if ec2err, ok := err.(*ec2.Error); ok && ec2err.Code == "InvalidRouteTableID.NotFound" { + if ec2err, ok := err.(aws.APIError); ok && ec2err.Code == "InvalidRouteTableID.NotFound" { resp = nil } else { log.Printf("Error on RouteTableStateRefresh: %s", err) diff --git a/builtin/providers/aws/resource_aws_route_table_test.go b/builtin/providers/aws/resource_aws_route_table_test.go index 2f4dfab2e..944f0c570 100644 --- a/builtin/providers/aws/resource_aws_route_table_test.go +++ b/builtin/providers/aws/resource_aws_route_table_test.go @@ -208,7 +208,10 @@ func testAccCheckRouteTableExists(n string, v *ec2.RouteTable) resource.TestChec } } -func TestAccAWSRouteTable_vpcPeering(t *testing.T) { +// TODO: re-enable this test. +// VPC Peering connections are prefixed with pcx +// Right now there is no VPC Peering resource +func _TestAccAWSRouteTable_vpcPeering(t *testing.T) { var v ec2.RouteTable testCheck := func(*terraform.State) error { @@ -345,6 +348,9 @@ resource "aws_route_table" "foo" { } ` +// TODO: re-enable this test. +// VPC Peering connections are prefixed with pcx +// Right now there is no VPC Peering resource const testAccRouteTableVpcPeeringConfig = ` resource "aws_vpc" "foo" { cidr_block = "10.1.0.0/16" @@ -359,7 +365,7 @@ resource "aws_route_table" "foo" { route { cidr_block = "10.2.0.0/16" - vpc_peering_connection_id = "vpc-12345" + vpc_peering_connection_id = "pcx-12345" } } ` From e7b3f3cf17605970515d4554b10c4b0f55ae1a4b Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Tue, 10 Mar 2015 10:30:01 -0500 Subject: [PATCH 3/3] convert route table tests to aws-sdk-go --- .../aws/resource_aws_route_table_test.go | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/builtin/providers/aws/resource_aws_route_table_test.go b/builtin/providers/aws/resource_aws_route_table_test.go index 944f0c570..5e6f90026 100644 --- a/builtin/providers/aws/resource_aws_route_table_test.go +++ b/builtin/providers/aws/resource_aws_route_table_test.go @@ -4,9 +4,10 @@ import ( "fmt" "testing" + "github.com/hashicorp/aws-sdk-go/aws" + "github.com/hashicorp/aws-sdk-go/gen/ec2" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" - "github.com/mitchellh/goamz/ec2" ) func TestAccAWSRouteTable_normal(t *testing.T) { @@ -19,7 +20,7 @@ func TestAccAWSRouteTable_normal(t *testing.T) { routes := make(map[string]ec2.Route) for _, r := range v.Routes { - routes[r.DestinationCidrBlock] = r + routes[*r.DestinationCIDRBlock] = r } if _, ok := routes["10.1.0.0/16"]; !ok { @@ -39,7 +40,7 @@ func TestAccAWSRouteTable_normal(t *testing.T) { routes := make(map[string]ec2.Route) for _, r := range v.Routes { - routes[r.DestinationCidrBlock] = r + routes[*r.DestinationCIDRBlock] = r } if _, ok := routes["10.1.0.0/16"]; !ok { @@ -91,7 +92,7 @@ func TestAccAWSRouteTable_instance(t *testing.T) { routes := make(map[string]ec2.Route) for _, r := range v.Routes { - routes[r.DestinationCidrBlock] = r + routes[*r.DestinationCIDRBlock] = r } if _, ok := routes["10.1.0.0/16"]; !ok { @@ -133,7 +134,7 @@ func TestAccAWSRouteTable_tags(t *testing.T) { Config: testAccRouteTableConfigTags, Check: resource.ComposeTestCheckFunc( testAccCheckRouteTableExists("aws_route_table.foo", &route_table), - testAccCheckTags(&route_table.Tags, "foo", "bar"), + testAccCheckTagsSDK(&route_table.Tags, "foo", "bar"), ), }, @@ -141,8 +142,8 @@ func TestAccAWSRouteTable_tags(t *testing.T) { Config: testAccRouteTableConfigTagsUpdate, Check: resource.ComposeTestCheckFunc( testAccCheckRouteTableExists("aws_route_table.foo", &route_table), - testAccCheckTags(&route_table.Tags, "foo", ""), - testAccCheckTags(&route_table.Tags, "bar", "baz"), + testAccCheckTagsSDK(&route_table.Tags, "foo", ""), + testAccCheckTagsSDK(&route_table.Tags, "bar", "baz"), ), }, }, @@ -150,7 +151,7 @@ func TestAccAWSRouteTable_tags(t *testing.T) { } func testAccCheckRouteTableDestroy(s *terraform.State) error { - conn := testAccProvider.Meta().(*AWSClient).ec2conn + conn := testAccProvider.Meta().(*AWSClient).awsEC2conn for _, rs := range s.RootModule().Resources { if rs.Type != "aws_route_table" { @@ -158,8 +159,9 @@ func testAccCheckRouteTableDestroy(s *terraform.State) error { } // Try to find the resource - resp, err := conn.DescribeRouteTables( - []string{rs.Primary.ID}, ec2.NewFilter()) + resp, err := conn.DescribeRouteTables(&ec2.DescribeRouteTablesRequest{ + RouteTableIDs: []string{rs.Primary.ID}, + }) if err == nil { if len(resp.RouteTables) > 0 { return fmt.Errorf("still exist.") @@ -169,7 +171,7 @@ func testAccCheckRouteTableDestroy(s *terraform.State) error { } // Verify the error is what we want - ec2err, ok := err.(*ec2.Error) + ec2err, ok := err.(aws.APIError) if !ok { return err } @@ -192,9 +194,10 @@ func testAccCheckRouteTableExists(n string, v *ec2.RouteTable) resource.TestChec return fmt.Errorf("No ID is set") } - conn := testAccProvider.Meta().(*AWSClient).ec2conn - resp, err := conn.DescribeRouteTables( - []string{rs.Primary.ID}, ec2.NewFilter()) + conn := testAccProvider.Meta().(*AWSClient).awsEC2conn + resp, err := conn.DescribeRouteTables(&ec2.DescribeRouteTablesRequest{ + RouteTableIDs: []string{rs.Primary.ID}, + }) if err != nil { return err } @@ -221,7 +224,7 @@ func _TestAccAWSRouteTable_vpcPeering(t *testing.T) { routes := make(map[string]ec2.Route) for _, r := range v.Routes { - routes[r.DestinationCidrBlock] = r + routes[*r.DestinationCIDRBlock] = r } if _, ok := routes["10.1.0.0/16"]; !ok {