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

Author: rileykarson

mmv1/third_party/terraform/services/container/node_config.go.tmpl

@@ -1,10 +1,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"
 
@@ -659,6 +662,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.`,
 							},
@@ -1213,7 +1217,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.
@@ -1496,6 +1500,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 {
@@ -1640,7 +1672,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)

mmv1/third_party/terraform/services/container/resource_container_cluster.go.tmpl

@@ -2868,15 +2868,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 {

mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.tmpl

@@ -6803,6 +6803,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]
@@ -14974,3 +15010,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)
+}

mmv1/third_party/terraform/services/container/resource_container_node_pool.go.tmpl

@@ -1136,7 +1136,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")),