[review] cleanup, round 2

google/resource_container_cluster.go

@@ -341,6 +341,7 @@ func resourceContainerCluster() *schema.Resource {
 			"ip_allocation_policy": {
 				Type:     schema.TypeList,
 				Optional: true,
+				ForceNew: true,
 				MaxItems: 1,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
@@ -493,6 +494,13 @@ func resourceContainerClusterCreate(d *schema.ResourceData, meta interface{}) er
 		cluster.NodePools = nodePools
 	}
 
+	if v, ok := d.GetOk("ip_allocation_policy"); ok {
+		cluster.IpAllocationPolicy, err = expandIPAllocationPolicy(v)
+		if err != nil {
+			return err
+		}
+	}
+
 	req := &container.CreateClusterRequest{
 		Cluster: cluster,
 	}
@@ -609,8 +617,8 @@ func resourceContainerClusterRead(d *schema.ResourceData, meta interface{}) erro
 	}
 	d.Set("node_pool", nps)
 
-	if cluster.IpAllocationPolicy != nil && cluster.IpAllocationPolicy.UseIpAliases {
-		d.Set("ip_allocation_policy", flattenIPAllocationPolicy(cluster.IpAllocationPolicy))
+	if err := d.Set("ip_allocation_policy", flattenIPAllocationPolicy(cluster.IpAllocationPolicy)); err != nil {
+		return err
 	}
 
 	if igUrls, err := getInstanceGroupUrlsFromManagerUrls(config, cluster.InstanceGroupUrls); err != nil {
@@ -952,21 +960,21 @@ func expandClusterAddonsConfig(configured interface{}) *container.AddonsConfig {
 func expandIPAllocationPolicy(configured interface{}) (*container.IPAllocationPolicy, error) {
 	ap := &container.IPAllocationPolicy{}
 	if len(configured.([]interface{})) > 0 {
-		config := configured.([]interface{})[0].(map[string]interface{})
-
-		ap.UseIpAliases = true
-		if v, ok := config["cluster_secondary_range_name"]; ok {
-			ap.ClusterSecondaryRangeName = v.(string)
-		}
+		if config, ok := configured.([]interface{})[0].(map[string]interface{}); ok {
+			ap.UseIpAliases = true
+			if v, ok := config["cluster_secondary_range_name"]; ok {
+				ap.ClusterSecondaryRangeName = v.(string)
+			}
 
-		if v, ok := config["services_secondary_range_name"]; ok {
-			ap.ServicesSecondaryRangeName = v.(string)
-		}
+			if v, ok := config["services_secondary_range_name"]; ok {
+				ap.ServicesSecondaryRangeName = v.(string)
+			}
 
-		if ap.UseIpAliases &&
-			(ap.ClusterSecondaryRangeName == "" || ap.ServicesSecondaryRangeName == "") {
+			if ap.UseIpAliases &&
+				(ap.ClusterSecondaryRangeName == "" || ap.ServicesSecondaryRangeName == "") {
 
-			return nil, fmt.Errorf("clusters using IP aliases must specify secondary ranges.")
+				return nil, fmt.Errorf("clusters using IP aliases must specify secondary ranges.")
+			}
 		}
 	}
 

google/resource_container_cluster_test.go

@@ -598,13 +598,15 @@ func TestAccContainerCluster_withMaintenanceWindow(t *testing.T) {
 func TestAccContainerCluster_withIPAllocationPolicy(t *testing.T) {
 	t.Parallel()
 
+	cluster := fmt.Sprintf("cluster-test-%s", acctest.RandString(10))
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckContainerClusterDestroy,
 		Steps: []resource.TestStep{
 			{
 				Config: testAccContainerCluster_withIPAllocationPolicy(
+					cluster,
 					map[string]string{
 						"pods":     "10.1.0.0/16",
 						"services": "10.2.0.0/20",
@@ -617,10 +619,15 @@ func TestAccContainerCluster_withIPAllocationPolicy(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckContainerCluster(
 						"google_container_cluster.with_ip_allocation_policy"),
+					resource.TestCheckResourceAttr("google_container_cluster.with_ip_allocation_policy",
+						"ip_allocation_policy.0.cluster_secondary_range_name", "pods"),
+					resource.TestCheckResourceAttr("google_container_cluster.with_ip_allocation_policy",
+						"ip_allocation_policy.0.services_secondary_range_name", "services"),
 				),
 			},
 			{
 				Config: testAccContainerCluster_withIPAllocationPolicy(
+					cluster,
 					map[string]string{
 						"pods":     "10.1.0.0/16",
 						"services": "10.2.0.0/20",
@@ -631,6 +638,7 @@ func TestAccContainerCluster_withIPAllocationPolicy(t *testing.T) {
 			},
 			{
 				Config: testAccContainerCluster_withIPAllocationPolicy(
+					cluster,
 					map[string]string{
 						"pods": "10.1.0.0/16",
 					},
@@ -639,7 +647,7 @@ func TestAccContainerCluster_withIPAllocationPolicy(t *testing.T) {
 						"services_secondary_range_name": "services",
 					},
 				),
-				ExpectError: regexp.MustCompile("secondary ranges are missing on the cluster's subnetwork"),
+				ExpectError: regexp.MustCompile("services secondary range \"pods\" not found in subnet"),
 			},
 		},
 	})
@@ -759,6 +767,11 @@ func testAccCheckContainerCluster(n string) resource.TestCheckFunc {
 			clusterTests = append(clusterTests, clusterTestField{"maintenance_policy.0.daily_maintenance_window.0.duration", cluster.MaintenancePolicy.Window.DailyMaintenanceWindow.Duration})
 		}
 
+		if cluster.IpAllocationPolicy != nil && cluster.IpAllocationPolicy.UseIpAliases {
+			clusterTests = append(clusterTests, clusterTestField{"ip_allocation_policy.0.cluster_secondary_range_name", cluster.IpAllocationPolicy.ClusterSecondaryRangeName})
+			clusterTests = append(clusterTests, clusterTestField{"ip_allocation_policy.0.services_secondary_range_name", cluster.IpAllocationPolicy.ServicesSecondaryRangeName})
+		}
+
 		for i, np := range cluster.NodePools {
 			prefix := fmt.Sprintf("node_pool.%d.", i)
 			clusterTests = append(clusterTests, clusterTestField{prefix + "name", np.Name})
@@ -1531,7 +1544,7 @@ resource "google_container_cluster" "with_maintenance_window" {
 }`, acctest.RandString(10), startTime)
 }
 
-func testAccContainerCluster_withIPAllocationPolicy(ranges, policy map[string]string) string {
+func testAccContainerCluster_withIPAllocationPolicy(cluster string, ranges, policy map[string]string) string {
 
 	var secondaryRanges bytes.Buffer
 	for rangeName, cidr := range ranges {
@@ -1564,7 +1577,7 @@ resource "google_compute_subnetwork" "container_subnetwork" {
 }
 
 resource "google_container_cluster" "with_ip_allocation_policy" {
-	name = "with-ip-allocation-policy"
+	name = "%s"
 	zone = "us-central1-a"
 
 	network = "${google_compute_network.container_network.name}"
@@ -1574,5 +1587,5 @@ resource "google_container_cluster" "with_ip_allocation_policy" {
 	ip_allocation_policy {
 	    %s
 	}
-}`, acctest.RandString(10), secondaryRanges.String(), ipAllocationPolicy.String())
+}`, acctest.RandString(10), secondaryRanges.String(), cluster, ipAllocationPolicy.String())
 }

website/docs/r/container_cluster.html.markdown

@@ -191,12 +191,12 @@ The `ip_allocation_policy` block supports:
 * `cluster_secondary_range_name` - (Optional) The name of the secondary range to be
     used as for the cluster CIDR block. The secondary range will be used for pod IP
     addresses. This must be an existing secondary range associated with the cluster
-    subnetwork. This field is only applicable with `use_ip_aliases`.
+    subnetwork.
 
 * `services_secondary_range_name` - (Optional) The name of the secondary range to be
     used as for the services CIDR block.  The secondary range will be used for service
     ClusterIPs. This must be an existing secondary range associated with the cluster
-    subnetwork. This field is only applicable with `use_ip_aliases`.
+    subnetwork.
 
 The `master_auth` block supports: