Skip to content

Commit

Permalink
make vpc-native clusters the default (for new clusters) (#9067)
Browse files Browse the repository at this point in the history
Signed-off-by: Modular Magician <magic-modules@google.com>
  • Loading branch information
modular-magician committed Sep 28, 2023
1 parent e61f77d commit f0ee6ee
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 15 deletions.
3 changes: 3 additions & 0 deletions .changelog/9067.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:breaking-change
container: newly created `google_container_cluster` resources now default to VPC-native instead of routes-based.
```
23 changes: 18 additions & 5 deletions google-beta/services/container/resource_container_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
30 changes: 26 additions & 4 deletions google-beta/services/container/resource_container_cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
),
},
{
Expand Down Expand Up @@ -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"},
},
},
})
}
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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"
Expand All @@ -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 {
Expand Down
28 changes: 28 additions & 0 deletions website/docs/guides/version_5_upgrade.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
10 changes: 4 additions & 6 deletions website/docs/r/container_cluster.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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).
Expand Down

0 comments on commit f0ee6ee

Please sign in to comment.