container: make cpu_manager_policy optional in kubelet_config (#11572)

Co-authored-by: Stephen Lewis (Burrows) <[email protected]>

[upstream:c920b8fc0cd06c1450c7da57434418b1fa307ebc]

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

Author: modular-magician

.changelog/11572.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+container: make `cpu_manager_policy` optional in `kubelet_config`
+```

google-beta/services/container/node_config.go

@@ -590,7 +590,7 @@ func schemaNodeConfig() *schema.Schema {
 						Schema: map[string]*schema.Schema{
 							"cpu_manager_policy": {
 								Type:         schema.TypeString,
-								Required:     true,
+								Optional:     true,
 								ValidateFunc: validation.StringInSlice([]string{"static", "none", ""}, false),
 								Description:  `Control the CPU management policy on the node.`,
 							},

google-beta/services/container/resource_container_cluster_test.go

@@ -1525,6 +1525,40 @@ func TestAccContainerCluster_withNodeConfig(t *testing.T) {
 	})
 }
 
+// Note: Updates for these are currently known to be broken (b/361634104), and
+// so are not tested here.
+// They can probably be made similar to, or consolidated with,
+// TestAccContainerCluster_withInsecureKubeletReadonlyPortEnabledInNodeConfigUpdates
+// after that's resolved.
+func TestAccContainerCluster_withNodeConfigKubeletConfigSettings(t *testing.T) {
+	t.Parallel()
+	clusterName := fmt.Sprintf("tf-test-cluster-%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_withNodeConfigKubeletConfigSettings(clusterName, networkName, subnetworkName),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						acctest.ExpectNoDelete(),
+					},
+				},
+			},
+			{
+				ResourceName:            "google_container_cluster.with_node_config_kubelet_config_settings",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"deletion_protection"},
+			},
+		},
+	})
+}
+
 // This is for node_config.kubelet_config, which affects the default node-pool
 // (default-pool) when created via the google_container_cluster resource
 func TestAccContainerCluster_withInsecureKubeletReadonlyPortEnabledInNodeConfigUpdates(t *testing.T) {
@@ -6620,6 +6654,28 @@ resource "google_container_cluster" "with_node_config" {
 `, clusterName, networkName, subnetworkName)
 }
 
+func testAccContainerCluster_withNodeConfigKubeletConfigSettings(clusterName, networkName, subnetworkName string) string {
+	return fmt.Sprintf(`
+resource "google_container_cluster" "with_node_config_kubelet_config_settings" {
+  name               = "%s"
+  location           = "us-central1-f"
+  initial_node_count = 1
+
+  node_config {
+    kubelet_config {
+      cpu_manager_policy   = "static"
+      cpu_cfs_quota        = true
+      cpu_cfs_quota_period = "100ms"
+      pod_pids_limit       = 2048
+    }
+  }
+  deletion_protection = false
+  network             = "%s"
+  subnetwork          = "%s"
+}
+`, clusterName, networkName, subnetworkName)
+}
+
 func testAccContainerCluster_withInsecureKubeletReadonlyPortEnabledInNodeConfig(clusterName, networkName, subnetworkName, insecureKubeletReadonlyPortEnabled string) string {
 	return fmt.Sprintf(`
 resource "google_container_cluster" "with_insecure_kubelet_readonly_port_enabled_in_node_config" {
@@ -6629,9 +6685,6 @@ resource "google_container_cluster" "with_insecure_kubelet_readonly_port_enabled
 
   node_config {
     kubelet_config {
-      # Must be set when kubelet_config is, but causes permadrift unless set to
-      # undocumented empty value
-      cpu_manager_policy                     = ""
       insecure_kubelet_readonly_port_enabled = "%s"
     }
   }

website/docs/r/container_cluster.html.markdown

@@ -1290,9 +1290,9 @@ Enables monitoring and attestation of the boot integrity of the instance. The at
 
 <a name="nested_kubelet_config"></a>The `kubelet_config` block supports:
 
-* `cpu_manager_policy` - (Required) The CPU management policy on the node. See
+* `cpu_manager_policy` - (Optional) The CPU management policy on the node. See
 [K8S CPU Management Policies](https://kubernetes.io/docs/tasks/administer-cluster/cpu-management-policies/).
-One of `"none"` or `"static"`. Defaults to `none` when `kubelet_config` is unset.
+One of `"none"` or `"static"`. If unset (or set to the empty string `""`), the API will treat the field as if set to "none".
 
 * `cpu_cfs_quota` - (Optional) If true, enables CPU CFS quota enforcement for
 containers that specify CPU limits.
@@ -1302,11 +1302,6 @@ as a sequence of decimal numbers, each with optional fraction and a unit suffix,
 such as `"300ms"`. Valid time units are "ns", "us" (or "µs"), "ms", "s", "m",
 "h". The value must be a positive duration.
 
--> Note: At the time of writing (2020/08/18) the GKE API rejects the `none`
-value and accepts an invalid `default` value instead. While this remains true,
-not specifying the `kubelet_config` block should be the equivalent of specifying
-`none`.
-
 * `insecure_kubelet_readonly_port_enabled` - (Optional) Controls whether the kubelet read-only port is enabled. It is strongly recommended to set this to `FALSE`. Possible values: `TRUE`, `FALSE`.
 
 * `pod_pids_limit` - (Optional) Controls the maximum number of processes allowed to run in a pod. The value must be greater than or equal to 1024 and less than 4194304.