GKE: Conditionally send the cpu_cfs_quota field based on presence in config (#15268) (#24569)

[upstream:606f5f2041c291dc7bd864cd7170eea55001461c]

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

Author: modular-magician

.changelog/15268.txt

@@ -0,0 +1,7 @@
+```release-note:breaking-change
+container: `node_config` blocks that had set `kubelet_config` without explicitly setting `cpu_cfs_quota` implicitly set `cfu_cfs_quota` to `false` when unset. From this version onwards, an unset `cpu_cfs_quota` will instead match the API default of true `true`. Resources that are recreated will receive the new value; old resources are unaffected, and may change values by explicitly setting the intended one.
+```
+
+```release-note:bug
+container: Fixed the default for `node_config.kubelet_config.cpu_cfs_quota` on `google_container_cluster`, `google_container_node_pool`, `google_container_cluster.node_pool` to align with the API. Terraform will now send a `true` value when the field is unset on creation, and preserve any previously set value when unset. Explicitly set values will work as defined in configuration.
+```

google/services/container/node_config.go

@@ -17,10 +17,13 @@
 package container
 
 import (
+	"fmt"
 	"log"
+	"strconv"
 	"strings"
 	"time"
 
+	"github.com/hashicorp/go-cty/cty"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 
@@ -628,6 +631,7 @@ func schemaNodeConfig() *schema.Schema {
 							},
 							"cpu_cfs_quota": {
 								Type:        schema.TypeBool,
+								Computed:    true,
 								Optional:    true,
 								Description: `Enable CPU CFS quota enforcement for containers that specify CPU limits.`,
 							},
@@ -1181,7 +1185,7 @@ func expandNodeConfigDefaults(configured interface{}) *container.NodeConfigDefau
 	return nodeConfigDefaults
 }
 
-func expandNodeConfig(v interface{}) *container.NodeConfig {
+func expandNodeConfig(d *schema.ResourceData, prefix string, v interface{}) *container.NodeConfig {
 	nodeConfigs := v.([]interface{})
 	nc := &container.NodeConfig{
 		// Defaults can't be set on a list/set in the schema, so set the default on create here.
@@ -1448,6 +1452,34 @@ func expandNodeConfig(v interface{}) *container.NodeConfig {
 
 	if v, ok := nodeConfig["kubelet_config"]; ok {
 		nc.KubeletConfig = expandKubeletConfig(v)
+
+		// start cpu_cfs_quota fix https://github.com/hashicorp/terraform-provider-google/issues/15767
+		// this makes the field conditional on appearance in configuration. This allows the API `true` default
+		// to override null, where currently we force-send null as false, which is wrong.
+		rawConfigNPRoot := d.GetRawConfig()
+		// if we have a prefix, we're in `node_pool.N.` in GKE Cluster. Traverse the RawConfig object to reach that
+		// root, at which point local references work going forwards.
+		if prefix != "" {
+			parts := strings.Split(prefix, ".") // "node_pool.N." -> ["node_pool" "N", ""]
+			npIndex, err := strconv.Atoi(parts[1])
+			if err != nil { // no error return from expander
+				panic(fmt.Errorf("unexpected format for node pool path prefix: %w. value: %v", err, prefix))
+			}
+
+			rawConfigNPRoot = rawConfigNPRoot.GetAttr("node_pool").Index(cty.NumberIntVal(int64(npIndex)))
+		}
+
+		if vNC := rawConfigNPRoot.GetAttr("node_config"); vNC.LengthInt() > 0 {
+			if vKC := vNC.Index(cty.NumberIntVal(0)).GetAttr("kubelet_config"); vKC.LengthInt() > 0 {
+				v := vKC.Index(cty.NumberIntVal(0)).GetAttr("cpu_cfs_quota")
+				if v == cty.NullVal(cty.Bool) {
+					nc.KubeletConfig.CpuCfsQuota = true
+				} else if v.False() { // force-send explicit false to API
+					nc.KubeletConfig.ForceSendFields = append(nc.KubeletConfig.ForceSendFields, "CpuCfsQuota")
+				}
+			}
+		}
+		// end cpu_cfs_quota fix
 	}
 
 	if v, ok := nodeConfig["linux_node_config"]; ok {
@@ -1586,7 +1618,6 @@ func expandKubeletConfig(v interface{}) *container.NodeKubeletConfig {
 	}
 	if cpuCfsQuota, ok := cfg["cpu_cfs_quota"]; ok {
 		kConfig.CpuCfsQuota = cpuCfsQuota.(bool)
-		kConfig.ForceSendFields = append(kConfig.ForceSendFields, "CpuCfsQuota")
 	}
 	if cpuCfsQuotaPeriod, ok := cfg["cpu_cfs_quota_period"]; ok {
 		kConfig.CpuCfsQuotaPeriod = cpuCfsQuotaPeriod.(string)

google/services/container/resource_container_cluster.go

@@ -2675,15 +2675,15 @@ func resourceContainerClusterCreate(d *schema.ResourceData, meta interface{}) er
 	} else {
 		// Node Configs have default values that are set in the expand function,
 		// but can only be set if node pools are unspecified.
-		cluster.NodeConfig = expandNodeConfig([]interface{}{})
+		cluster.NodeConfig = expandNodeConfig(d, "", []interface{}{})
 	}
 
 	if v, ok := d.GetOk("node_pool_defaults"); ok {
 		cluster.NodePoolDefaults = expandNodePoolDefaults(v)
 	}
 
 	if v, ok := d.GetOk("node_config"); ok {
-		cluster.NodeConfig = expandNodeConfig(v)
+		cluster.NodeConfig = expandNodeConfig(d, "", v)
 	}
 
 	if v, ok := d.GetOk("authenticator_groups_config"); ok {

google/services/container/resource_container_cluster_test.go

@@ -6386,6 +6386,42 @@ func TestAccContainerCluster_additional_pod_ranges_config_on_update(t *testing.T
 	})
 }
 
+func TestAccContainerCluster_withCpuCfsQuotaPool(t *testing.T) {
+	t.Parallel()
+
+	clusterName := fmt.Sprintf("tf-test-cluster-%s", acctest.RandString(t, 10))
+	npName := fmt.Sprintf("tf-test-cluster-nodepool-%s", acctest.RandString(t, 10))
+	networkName := acctest.BootstrapSharedTestNetwork(t, "gke-cluster")
+	subnetworkName := acctest.BootstrapSubnet(t, "gke-cluster", networkName)
+
+	acctest.VcrTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.AccTestPreCheck(t) },
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+		CheckDestroy:             testAccCheckContainerClusterDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccContainerCluster_withCpuCfsQuotaPool(clusterName, npName, networkName, subnetworkName),
+			},
+			{
+				ResourceName:            "google_container_cluster.with_kubelet_config",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"deletion_protection"},
+			},
+			{
+				Config:   testAccContainerCluster_withCpuCfsQuotaPool2(clusterName, npName, networkName, subnetworkName),
+				PlanOnly: true,
+			},
+			{
+				ResourceName:            "google_container_cluster.with_kubelet_config",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"deletion_protection"},
+			},
+		},
+	})
+}
+
 func testAccContainerCluster_masterAuthorizedNetworksDisabled(t *testing.T, resource_name string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[resource_name]
@@ -13983,3 +14019,67 @@ resource "google_container_cluster" "with_kubelet_config" {
 }
 `, clusterName, networkName, subnetworkName, cpuManagerPolicy, memoryManagerPolicy, topologyManagerPolicy, topologyManagerScope)
 }
+
+func testAccContainerCluster_withCpuCfsQuotaPool(clusterName, npName, networkName, subnetworkName string) string {
+	return fmt.Sprintf(`
+resource "google_container_cluster" "with_kubelet_config" {
+  name               = %q
+  location           = "us-central1-a"
+  network            = %q
+  subnetwork         = %q
+  deletion_protection = false
+
+  node_pool {
+    name = "%s-1"
+    initial_node_count = 1
+    node_config {
+      kubelet_config {
+        # cpu_cfs_quota = true
+      }
+    }
+  }
+
+  node_pool {
+    name = "%s-2"
+    initial_node_count = 1
+    node_config {
+      kubelet_config {
+        cpu_cfs_quota = false
+      }
+    }
+  }
+}
+`, clusterName, networkName, subnetworkName, npName, npName)
+}
+
+func testAccContainerCluster_withCpuCfsQuotaPool2(clusterName, npName, networkName, subnetworkName string) string {
+	return fmt.Sprintf(`
+resource "google_container_cluster" "with_kubelet_config" {
+  name               = %q
+  location           = "us-central1-a"
+  network            = %q
+  subnetwork         = %q
+  deletion_protection = false
+
+  node_pool {
+    name = "%s-1"
+    initial_node_count = 1
+    node_config {
+      kubelet_config {
+        cpu_cfs_quota = true
+      }
+    }
+  }
+
+  node_pool {
+    name = "%s-2"
+    initial_node_count = 1
+    node_config {
+      kubelet_config {
+        cpu_cfs_quota = false
+      }
+    }
+  }
+}
+`, clusterName, networkName, subnetworkName, npName, npName)
+}

google/services/container/resource_container_node_pool.go

@@ -968,7 +968,7 @@ func expandNodePool(d *schema.ResourceData, prefix string) (*container.NodePool,
 	np := &container.NodePool{
 		Name:             name,
 		InitialNodeCount: int64(nodeCount),
-		Config:           expandNodeConfig(d.Get(prefix + "node_config")),
+		Config:           expandNodeConfig(d, prefix, d.Get(prefix+"node_config")),
 		Locations:        locations,
 		Version:          d.Get(prefix + "version").(string),
 		NetworkConfig:    expandNodeNetworkConfig(d.Get(prefix + "network_config")),