Add subnetwork resource ACC tests, additional doc updates

builtin/providers/google/provider.go

@@ -146,6 +146,8 @@ func validateCredentials(v interface{}, k string) (warnings []string, errors []e
 	return
 }
 
+// FIXME: not sure this is the best place for this
+// Given a Google zone (e.g. us-central1-f) this func returns the Region, us-central1 in this example.
 func getRegionFromZone(zone string) string {
 	if zone != "" && len(zone) > 2 {
 		region := zone[:len(zone)-2]

builtin/providers/google/provider_test.go

@@ -50,3 +50,11 @@ func testAccPreCheck(t *testing.T) {
 		t.Fatal("GOOGLE_REGION must be set to us-central1 for acceptance tests")
 	}
 }
+
+func TestProvider_getRegionFromZone(t *testing.T) {
+	expected := "us-central1"
+	actual := getRegionFromZone("us-central1-f")
+	if expected != actual {
+		t.Fatalf("Region (%s) did not match expected value: %s", actual, expected)
+	}
+}

builtin/providers/google/resource_compute_network.go

@@ -14,7 +14,6 @@ func resourceComputeNetwork() *schema.Resource {
 		Create: resourceComputeNetworkCreate,
 		Read:   resourceComputeNetworkRead,
 		Delete: resourceComputeNetworkDelete,
-		Update: resourceComputeNetworkUpdate,
 
 		Schema: map[string]*schema.Schema{
 			"name": &schema.Schema{
@@ -27,7 +26,7 @@ func resourceComputeNetwork() *schema.Resource {
 				Type:       schema.TypeString,
 				Optional:   true,
 				ForceNew:   true,
-				Deprecated: "Please use custom subnetworks instead",
+				Deprecated: "Please use google_compute_subnetwork resources instead.",
 			},
 
 			"gateway_ipv4": &schema.Schema{
@@ -38,12 +37,15 @@ func resourceComputeNetwork() *schema.Resource {
 			"auto_create_subnetworks": &schema.Schema{
 				Type:     schema.TypeBool,
 				Optional: true,
-				Default:  false, // copy behaviour of Google Cloud GUI and gcloud tool
+				ForceNew: true,
+				Default:  false, // TODO: ideally should be true to match Google's default behaviour, but this causes backward
+				// compatibility issue with existing terraform configs
 			},
 
 			"description": &schema.Schema{
 				Type:     schema.TypeString,
 				Optional: true,
+				ForceNew: true,
 			},
 
 			"self_link": &schema.Schema{
@@ -59,11 +61,11 @@ func resourceComputeNetworkCreate(d *schema.ResourceData, meta interface{}) erro
 
 	//
 	// Possible modes:
-	// -1- Legacy mode - Create a network in the legacy mode. ipv4_range is set. auto_create_subnetworks must be false
+	// - 1 Legacy mode - Create a network in the legacy mode. ipv4_range is set. auto_create_subnetworks must be false
-	//     and not sent in reqest, and subnetworks empty
+	//     and not sent in request
-	// -2- Distributed Mode - Create a new generation network that supports subnetworks:
+	// - 2 Distributed Mode - Create a new generation network that supports subnetworks:
-	//   2.a - Auto subnet mode - auto_create_subnetworks = true, Google will generate 1 subnetwork per region
+	//   - 2.a - Auto subnet mode - auto_create_subnetworks = true, Google will generate 1 subnetwork per region
-	//   2.b - Custom subnet mode - auto_create_subnetworks = false & ipv4_range not set,
+	//   - 2.b - Custom subnet mode - auto_create_subnetworks = false & ipv4_range not set,
 	//
 	ipv4range := d.Get("ipv4_range").(string)
 	autoCreateSubnetworks := d.Get("auto_create_subnetworks").(bool)
@@ -83,7 +85,8 @@ func resourceComputeNetworkCreate(d *schema.ResourceData, meta interface{}) erro
 		log.Printf("[DEBUG] Setting IPv4Range (%#V) for legacy network mode", v.(string))
 		network.IPv4Range = v.(string)
 	} else {
-		// custom subnet mode, so make sure AutoCreateSubnetworks field is included in request
+		// custom subnet mode, so make sure AutoCreateSubnetworks field is included in request otherwise
+		// google will create a network in legacy mode.
 		network.ForceSendFields = []string{"AutoCreateSubnetworks"}
 	}
 
@@ -102,7 +105,7 @@ func resourceComputeNetworkCreate(d *schema.ResourceData, meta interface{}) erro
 		return err
 	}
 
-	return nil
+	return resourceComputeNetworkRead(d, meta)
 }
 
 func resourceComputeNetworkRead(d *schema.ResourceData, meta interface{}) error {
@@ -124,10 +127,6 @@ func resourceComputeNetworkRead(d *schema.ResourceData, meta interface{}) error
 
 	d.Set("gateway_ipv4", network.GatewayIPv4)
 	d.Set("self_link", network.SelfLink)
-	for i, v := range network.Subnetworks {
-		prefix := fmt.Sprintf("subnetwork_links.%d", i)
-		d.Set(prefix, v)
-	}
 
 	return nil
 }
@@ -150,8 +149,3 @@ func resourceComputeNetworkDelete(d *schema.ResourceData, meta interface{}) erro
 	d.SetId("")
 	return nil
 }
-
-func resourceComputeNetworkUpdate(d *schema.ResourceData, meta interface{}) error {
-	//config := meta.(*Config)
-	return nil
-}

builtin/providers/google/resource_compute_network_test.go

@@ -42,6 +42,8 @@ func TestAccComputeNetwork_auto_subnet(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckComputeNetworkExists(
 						"google_compute_network.bar", &network),
+					testAccCheckComputeNetworkIsAutoSubnet(
+						"google_compute_network.bar", &network),
 				),
 			},
 		},
@@ -116,6 +118,28 @@ func testAccCheckComputeNetworkExists(n string, network *compute.Network) resour
 	}
 }
 
+func testAccCheckComputeNetworkIsAutoSubnet(n string, network *compute.Network) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		config := testAccProvider.Meta().(*Config)
+
+		found, err := config.clientCompute.Networks.Get(
+			config.Project, network.Name).Do()
+		if err != nil {
+			return err
+		}
+
+		if !found.AutoCreateSubnetworks {
+			return fmt.Errorf("should have AutoCreateSubnetworks = true")
+		}
+
+		if found.IPv4Range != "" {
+			return fmt.Errorf("should not have IPv4Range")
+		}
+
+		return nil
+	}
+}
+
 func testAccCheckComputeNetworkIsCustomSubnet(n string, network *compute.Network) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		config := testAccProvider.Meta().(*Config)

builtin/providers/google/resource_compute_subnetwork.go

@@ -7,6 +7,7 @@ import (
 	"github.com/hashicorp/terraform/helper/schema"
 	"google.golang.org/api/compute/v1"
 	"google.golang.org/api/googleapi"
+	"strings"
 )
 
 func resourceComputeSubnetwork() *schema.Resource {
@@ -63,6 +64,13 @@ func createSubnetID(s *compute.Subnetwork) string {
 	return fmt.Sprintf("%s/%s", s.Region, s.Name)
 }
 
+func splitSubnetID(id string) (region string, name string) {
+	parts := strings.Split(id, "/")
+	region = parts[0]
+	name = parts[1]
+	return
+}
+
 func resourceComputeSubnetworkCreate(d *schema.ResourceData, meta interface{}) error {
 	config := meta.(*Config)
 
@@ -83,8 +91,11 @@ func resourceComputeSubnetworkCreate(d *schema.ResourceData, meta interface{}) e
 		return fmt.Errorf("Error creating subnetwork: %s", err)
 	}
 
-	// It probably maybe worked, so store the ID now
+	// It probably maybe worked, so store the ID now. ID is a combination of region + subnetwork
-	// Subnetwork name is not guaranteed to be unique in a project, but must be unique within a region
+	// name because subnetwork names are not unique in a project, per the Google docs:
+	// "When creating a new subnetwork, its name has to be unique in that project for that region, even across networks.
+	// The same name can appear twice in a project, as long as each one is in a different region."
+	// https://cloud.google.com/compute/docs/subnetworks
 	subnetwork.Region = region
 	d.SetId(createSubnetID(subnetwork))
 
@@ -125,14 +136,14 @@ func resourceComputeSubnetworkDelete(d *schema.ResourceData, meta interface{}) e
 	config := meta.(*Config)
 	region := d.Get("region").(string)
 
-	// Delete the network
+	// Delete the subnetwork
 	op, err := config.clientCompute.Subnetworks.Delete(
 		config.Project, region, d.Get("name").(string)).Do()
 	if err != nil {
-		return fmt.Errorf("Error deleting network: %s", err)
+		return fmt.Errorf("Error deleting subnetwork: %s", err)
 	}
 
-	err = computeOperationWaitRegion(config, op, region, "Deleting Network")
+	err = computeOperationWaitRegion(config, op, region, "Deleting Subnetwork")
 	if err != nil {
 		return err
 	}

builtin/providers/google/resource_compute_subnetwork_test.go

@@ -0,0 +1,92 @@
+package google
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/hashicorp/terraform/helper/acctest"
+	"github.com/hashicorp/terraform/helper/resource"
+	"github.com/hashicorp/terraform/terraform"
+	"google.golang.org/api/compute/v1"
+)
+
+func TestAccComputeSubnetwork_basic(t *testing.T) {
+	var subnetwork compute.Subnetwork
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckComputeSubnetworkDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccComputeSubnetwork_basic,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckComputeSubnetworkExists(
+						"google_compute_subnetwork.foobar", &subnetwork),
+				),
+			},
+		},
+	})
+}
+
+func testAccCheckComputeSubnetworkDestroy(s *terraform.State) error {
+	config := testAccProvider.Meta().(*Config)
+
+	for _, rs := range s.RootModule().Resources {
+		if rs.Type != "google_compute_subnetwork" {
+			continue
+		}
+
+		region, subnet_name := splitSubnetID(rs.Primary.ID)
+		_, err := config.clientCompute.Subnetworks.Get(
+			config.Project, region, subnet_name).Do()
+		if err == nil {
+			return fmt.Errorf("Network still exists")
+		}
+	}
+
+	return nil
+}
+
+func testAccCheckComputeSubnetworkExists(n string, subnetwork *compute.Subnetwork) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		rs, ok := s.RootModule().Resources[n]
+		if !ok {
+			return fmt.Errorf("Not found: %s", n)
+		}
+
+		if rs.Primary.ID == "" {
+			return fmt.Errorf("No ID is set")
+		}
+
+		config := testAccProvider.Meta().(*Config)
+
+		region, subnet_name := splitSubnetID(rs.Primary.ID)
+		found, err := config.clientCompute.Subnetworks.Get(
+			config.Project, region, subnet_name).Do()
+		if err != nil {
+			return err
+		}
+
+		if found.Name != subnet_name {
+			return fmt.Errorf("Subnetwork not found")
+		}
+
+		*subnetwork = *found
+
+		return nil
+	}
+}
+
+var testAccComputeSubnetwork_basic = fmt.Sprintf(`
+resource "google_compute_network" "custom-test" {
+	name = "network-test-%s"
+	auto_create_subnetworks = false
+}
+
+resource "google_compute_subnetwork" "foobar" {
+	name = "subnetwork-test-%s"
+	ip_cidr_range = "10.0.0.0/16"
+	region = "us-central1"
+	network = "${google_compute_network.custom-test.self_link}"
+}`, acctest.RandString(10), acctest.RandString(10))

website/source/docs/providers/google/r/compute_instance.html.markdown

@@ -124,7 +124,7 @@ The `network_interface` block supports:
     `network` or `subnetwork` must be provided.
     
 *  `subnetwork` - (Optional) the name of the subnetwork to attach this interface to. The subnetwork
-     must exist in the same region this instance is to be created in. Either `network`
+     must exist in the same region this instance will be created in. Either `network`
      or `subnetwork` must be provided.
 
 * `access_config` - (Optional) Access configurations, i.e. IPs via which this instance can be

website/source/docs/providers/google/r/compute_instance_template.html.markdown

@@ -137,7 +137,12 @@ The `disk` block supports:
 
 The `network_interface` block supports:
 
-* `network` - (Required) The name of the network to attach this interface to.
+* `network` - (Optional) The name of the network to attach this interface to. Only use `network`
+   attribute for Legacy or Auto subnetted networks, otherwise use `subnetwork` instead.
+
+* `subnetwork` - (Optional) the name of the subnetwork to attach this interface to. The subnetwork
+   must exist in the same region this instance will be created in. Either `network`
+   or `subnetwork` must be provided.
 
 * `access_config` - (Optional) Access configurations, i.e. IPs via which this instance can be
   accessed via the Internet.  Omit to ensure that the instance is not accessible from the Internet

website/source/docs/providers/google/r/compute_vpn_tunnel.html.markdown

@@ -103,6 +103,10 @@ The following arguments are supported:
 * `ike_version` - (Optional) Either version 1 or 2. Default is 2.
     Changing this forces a new resource to be created.
     
+* `local_traffic_selector` - (Optional) Specifies which CIDR ranges are announced
+    to the VPN peer. Mandatory if the VPN gateway is attached to a custom subnetted
+    network. Refer to Google documentation for more information.
+
 ## Attributes Reference
 
 The following attributes are exported: