make vpc-native clusters the default (for new clusters) (#9067) (#6402)

Signed-off-by: Modular Magician <[email protected]>

Author: modular-magician

.changelog/9067.txt

@@ -0,0 +1,3 @@
+```release-note:breaking-change
+container: newly created `google_container_cluster` resources now default to VPC-native instead of routes-based.
+```

google-beta/services/container/resource_container_cluster.go

@@ -1567,13 +1567,14 @@ func ResourceContainerCluster() *schema.Resource {
 				},
 			},
 
+			// Defaults to "VPC_NATIVE" during create only
 			"networking_mode": {
 				Type:         schema.TypeString,
 				Optional:     true,
 				Computed:     true,
 				ForceNew:     true,
 				ValidateFunc: validation.StringInSlice([]string{"VPC_NATIVE", "ROUTES"}, false),
-				Description:  `Determines whether alias IPs or routes will be used for pod IPs in the cluster.`,
+				Description:  `Determines whether alias IPs or routes will be used for pod IPs in the cluster. Defaults to VPC_NATIVE for new clusters.`,
 			},
 
 			"remove_default_node_pool": {
@@ -2086,6 +2087,21 @@ func resourceContainerClusterCreate(d *schema.ResourceData, meta interface{}) er
 
 	clusterName := d.Get("name").(string)
 
+	// Default to VPC_NATIVE mode during initial creation
+	// This solution (a conditional default) should not be considered to set a precedent on its own.
+	// If you're considering a similar approach on a different resource, strongly weigh making the field required.
+	// GKE tends to require exceptional handling in general- and this default was a breaking change in their API
+	// that was compounded on by numerous product developments afterwards. We have not seen a similar case
+	// since, after several years.
+	networkingMode := d.Get("networking_mode").(string)
+	clusterIpv4Cidr := d.Get("cluster_ipv4_cidr").(string)
+	if networkingMode == "" && clusterIpv4Cidr == "" {
+		err := d.Set("networking_mode", "VPC_NATIVE")
+		if err != nil {
+			return fmt.Errorf("Error setting networking mode during creation: %s", err)
+		}
+	}
+
 	ipAllocationBlock, err := expandIPAllocationPolicy(d.Get("ip_allocation_policy"), d.Get("networking_mode").(string), d.Get("enable_autopilot").(bool))
 	if err != nil {
 		return err
@@ -4246,10 +4262,7 @@ func expandIPAllocationPolicy(configured interface{}, networkingMode string, aut
 	l := configured.([]interface{})
 	if len(l) == 0 || l[0] == nil {
 		if networkingMode == "VPC_NATIVE" {
-			if autopilot {
-				return nil, nil
-			}
-			return nil, fmt.Errorf("`ip_allocation_policy` block is required for VPC_NATIVE clusters.")
+			return nil, nil
 		}
 		return &container.IPAllocationPolicy{
 			UseIpAliases:    false,

google-beta/services/container/resource_container_cluster_test.go

@@ -28,6 +28,7 @@ func TestAccContainerCluster_basic(t *testing.T) {
 				Check: resource.ComposeAggregateTestCheckFunc(
 					resource.TestCheckResourceAttrSet("google_container_cluster.primary", "services_ipv4_cidr"),
 					resource.TestCheckResourceAttrSet("google_container_cluster.primary", "self_link"),
+					resource.TestCheckResourceAttr("google_container_cluster.primary", "networking_mode", "VPC_NATIVE"),
 				),
 			},
 			{
@@ -57,21 +58,31 @@ func TestAccContainerCluster_basic(t *testing.T) {
 func TestAccContainerCluster_networkingModeRoutes(t *testing.T) {
 	t.Parallel()
 
-	clusterName := fmt.Sprintf("tf-test-cluster-%s", acctest.RandString(t, 10))
+	firstClusterName := fmt.Sprintf("tf-test-cluster-%s", acctest.RandString(t, 10))
+	secondClusterName := fmt.Sprintf("tf-test-cluster-%s", acctest.RandString(t, 10))
 	acctest.VcrTest(t, resource.TestCase{
 		PreCheck:                 func() { acctest.AccTestPreCheck(t) },
 		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
 		CheckDestroy:             testAccCheckContainerClusterDestroyProducer(t),
 		Steps: []resource.TestStep{
 			{
-				Config: testAccContainerCluster_networkingModeRoutes(clusterName),
+				Config: testAccContainerCluster_networkingModeRoutes(firstClusterName, secondClusterName),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("google_container_cluster.primary", "networking_mode", "ROUTES"),
+					resource.TestCheckResourceAttr("google_container_cluster.secondary", "networking_mode", "ROUTES")),
 			},
 			{
 				ResourceName:            "google_container_cluster.primary",
 				ImportState:             true,
 				ImportStateVerify:       true,
 				ImportStateVerifyIgnore: []string{"deletion_protection"},
 			},
+			{
+				ResourceName:            "google_container_cluster.secondary",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"deletion_protection"},
+			},
 		},
 	})
 }
@@ -2693,6 +2704,9 @@ func TestAccContainerCluster_withAutopilot(t *testing.T) {
 		Steps: []resource.TestStep{
 			{
 				Config: testAccContainerCluster_withAutopilot(pid, containerNetName, clusterName, "us-central1", true, false, ""),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("google_container_cluster.with_autopilot", "networking_mode", "VPC_NATIVE"),
+				),
 			},
 			{
 				ResourceName:            "google_container_cluster.with_autopilot",
@@ -4260,7 +4274,7 @@ resource "google_container_cluster" "primary" {
 `, name)
 }
 
-func testAccContainerCluster_networkingModeRoutes(name string) string {
+func testAccContainerCluster_networkingModeRoutes(firstName, secondName string) string {
 	return fmt.Sprintf(`
 resource "google_container_cluster" "primary" {
   name               = "%s"
@@ -4269,7 +4283,15 @@ resource "google_container_cluster" "primary" {
   networking_mode    = "ROUTES"
   deletion_protection = false
 }
-`, name)
+
+resource "google_container_cluster" "secondary" {
+	name               = "%s"
+	location           = "us-central1-a"
+	initial_node_count = 1
+	cluster_ipv4_cidr  = "10.96.0.0/14"
+	deletion_protection = false
+  }
+`, firstName, secondName)
 }
 
 func testAccContainerCluster_misc(name string) string {

website/docs/guides/version_5_upgrade.html.markdown

@@ -452,6 +452,34 @@ Terraform from destroying or recreating the cluster.
 **`deletion_protection` does NOT prevent deletion outside of Terraform.**
 To destroy a `google_container_cluster`, this field must be explicitly set to `false`.
 
+### `networking_mode` defaults to `VPC_NATIVE` for newly created clusters
+
+New clusters will default to `VPC_NATIVE` which enables [IP aliasing](https://cloud.google.com/kubernetes-engine/docs/how-to/ip-aliases). Previously, `google_container_cluster` would default to using routes as
+the networking mode unless `ip_allocation_policy` policy was set. Now, `networking_mode` will
+default to `VPC_NATIVE` and `ip_allocation_policy` will be set by the server if unset in
+configuration. Existing clusters should not be affected.
+
+#### New Minimal Config for VPC-native cluster
+
+```hcl
+resource "google_container_cluster" "primary" {
+  name               = "my_cluster"
+  location           = "us-central1-a"
+  initial_node_count = 1
+}
+```
+
+#### New Minimal Config for Routes-based cluster
+
+```hcl
+resource "google_container_cluster" "primary" {
+  name               = "my_cluster"
+  location           = "us-central1-a"
+  initial_node_count = 1
+  networking_mode    = "ROUTES"
+}
+```
+
 ### `enable_binary_authorization` is now removed
 
 `enable_binary_authorization` has been removed in favor of `binary_authorization.enabled`.

website/docs/r/container_cluster.html.markdown

@@ -136,7 +136,7 @@ set to `true`).
 * `cluster_ipv4_cidr` - (Optional) The IP address range of the Kubernetes pods
 in this cluster in CIDR notation (e.g. `10.96.0.0/14`). Leave blank to have one
 automatically chosen or specify a `/14` block in `10.0.0.0/8`. This field will
-only work for routes-based clusters, where `ip_allocation_policy` is not defined.
+default a new cluster to routes-based, where `ip_allocation_policy` is not defined.
 
 * `cluster_autoscaling` - (Optional)
 Per-cluster configuration of Node Auto-Provisioning with Cluster Autoscaler to
@@ -194,13 +194,11 @@ set this to a value of at least `1`, alongside setting
 `remove_default_node_pool` to `true`.
 
 * `ip_allocation_policy` - (Optional) Configuration of cluster IP allocation for
-VPC-native clusters. Adding this block enables [IP aliasing](https://cloud.google.com/kubernetes-engine/docs/how-to/ip-aliases),
-making the cluster VPC-native instead of routes-based. Structure is [documented
-below](#nested_ip_allocation_policy).
+VPC-native clusters. If this block is unset during creation, it will be set by the GKE backend. 
+Structure is [documented below](#nested_ip_allocation_policy).
 
 * `networking_mode` - (Optional) Determines whether alias IPs or routes will be used for pod IPs in the cluster.
-Options are `VPC_NATIVE` or `ROUTES`. `VPC_NATIVE` enables [IP aliasing](https://cloud.google.com/kubernetes-engine/docs/how-to/ip-aliases),
-and requires the `ip_allocation_policy` block to be defined. By default, when this field is unspecified and no `ip_allocation_policy` blocks are set, GKE will create a `ROUTES`-based cluster.
+Options are `VPC_NATIVE` or `ROUTES`. `VPC_NATIVE` enables [IP aliasing](https://cloud.google.com/kubernetes-engine/docs/how-to/ip-aliases). Newly created clusters will default to `VPC_NATIVE`.
 
 * `logging_config` - (Optional) Logging configuration for the cluster.
     Structure is [documented below](#nested_logging_config).