container: fix updates for node_config.gcfs_config

- Handle updates properly for `node_config.gcfs_config` - Make `node_config.gcfs_config.enabled` optional and computed vs. required - Add tests to make sure we test the case where it's defined in `google_container_cluster.node_config`, as well as testing updates for both that and the `google_container_node_pool` resource. Fixes hashicorp/terraform-provider-google#19482 Follow up to GoogleCloudPlatform#11553
wyardley · Sep 14, 2024 · 58a8ca8 · 58a8ca8
1 parent 6d67e34
commit 58a8ca8
Show file tree

Hide file tree

Showing 6 changed files with 168 additions and 18 deletions.
diff --git a/mmv1/third_party/terraform/services/container/node_config.go.erb b/mmv1/third_party/terraform/services/container/node_config.go.erb
@@ -101,21 +101,22 @@ func schemaLoggingVariant() *schema.Schema {
 }
 
 func schemaGcfsConfig() *schema.Schema {
-        return &schema.Schema{
-                Type:     schema.TypeList,
-		Optional: true,
-		MaxItems: 1,
+	return &schema.Schema{
+		Type:        schema.TypeList,
+		Optional:    true,
+		MaxItems:    1,
 		Description: `GCFS configuration for this node.`,
-		Elem: &schema.Resource{
+		Elem:        &schema.Resource{
 			Schema: map[string]*schema.Schema{
 				"enabled": {
 					Type:        schema.TypeBool,
-					Required:    true,
+					Optional:    true,
+					Computed:    true,
 					Description: `Whether or not GCFS is enabled`,
 				},
 			},
 		},
-        }
+	}
 }
 
 func schemaNodeConfig() *schema.Schema {

diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster.go.erb
@@ -3883,6 +3883,55 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er
 				log.Printf("[INFO] GKE cluster %s: default-pool setting for insecure_kubelet_readonly_port_enabled updated to %s", d.Id(), it)
       }
 		}
+
+		if d.HasChange("node_config.0.gcfs_config") {
+
+			defaultPool := "default-pool"
+
+			timeout := d.Timeout(schema.TimeoutCreate)
+
+			nodePoolInfo, err := extractNodePoolInformationFromCluster(d, config, clusterName)
+			if err != nil {
+				return err
+			}
+
+			// Acquire write-lock on nodepool.
+			npLockKey := nodePoolInfo.nodePoolLockKey(defaultPool)
+
+			gcfsEnabled := d.Get("node_config.0.gcfs_config.0.enabled").(bool)
+
+			// While we're getting the value from the drepcated field in
+			// node_config.kubelet_config, the actual setting that needs to be updated
+			// is on the default nodepool.
+			req := &container.UpdateNodePoolRequest{
+				Name: defaultPool,
+				GcfsConfig: &container.GcfsConfig{
+					Enabled: gcfsEnabled,
+				},
+			}
+
+			updateF := func() error {
+				clusterNodePoolsUpdateCall := config.NewContainerClient(userAgent).Projects.Locations.Clusters.NodePools.Update(nodePoolInfo.fullyQualifiedName(defaultPool), req)
+				if config.UserProjectOverride {
+					clusterNodePoolsUpdateCall.Header().Add("X-Goog-User-Project", nodePoolInfo.project)
+				}
+				op, err := clusterNodePoolsUpdateCall.Do()
+				if err != nil {
+					return err
+				}
+
+				// Wait until it's updated
+				return ContainerOperationWait(config, op, nodePoolInfo.project, nodePoolInfo.location,
+					"updating GKE node pool gcfs_config", userAgent, timeout)
+			}
+
+			if err := retryWhileIncompatibleOperation(timeout, npLockKey, updateF); err != nil {
+				return err
+			}
+
+			log.Printf("[INFO] GKE cluster %s: %s setting for gcfs_config updated to %t", d.Id(), defaultPool, gcfsEnabled)
+		}
+
 	}
 
 	if d.HasChange("notification_config") {

diff --git a/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_cluster_test.go.erb
@@ -1536,6 +1536,49 @@ func TestAccContainerCluster_withNodeConfig(t *testing.T) {
 	})
 }
 
+func TestAccContainerCluster_withNodeConfigGcfsConfig(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_withNodeConfigGcfsConfig(clusterName, networkName, subnetworkName, false),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						acctest.ExpectNoDelete(),
+					},
+				},
+			},
+			{
+				ResourceName:            "google_container_cluster.with_node_config_gcfs_config",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"deletion_protection"},
+			},
+			{
+				Config: testAccContainerCluster_withNodeConfigGcfsConfig(clusterName, networkName, subnetworkName, true),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						acctest.ExpectNoDelete(),
+					},
+				},
+			},
+			{
+				ResourceName:            "google_container_cluster.with_node_config_gcfs_config",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"deletion_protection"},
+			},
+		},
+	})
+}
+
 // 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,
@@ -6693,6 +6736,26 @@ resource "google_container_cluster" "with_node_config" {
 `, clusterName, networkName, subnetworkName)
 }
 
+func testAccContainerCluster_withNodeConfigGcfsConfig(clusterName, networkName, subnetworkName string, enabled bool) string {
+	return fmt.Sprintf(`
+resource "google_container_cluster" "with_node_config_gcfs_config" {
+  name               = "%s"
+  location           = "us-central1-f"
+  initial_node_count = 1
+
+  node_config {
+    gcfs_config {
+      enabled = %t
+    }
+  }
+
+  deletion_protection = false
+  network             = "%s"
+  subnetwork          = "%s"
+}
+`, clusterName, enabled, networkName, subnetworkName)
+}
+
 func testAccContainerCluster_withNodeConfigKubeletConfigSettings(clusterName, networkName, subnetworkName string) string {
 	return fmt.Sprintf(`
 resource "google_container_cluster" "with_node_config_kubelet_config_settings" {

diff --git a/mmv1/third_party/terraform/services/container/resource_container_node_pool.go.erb b/mmv1/third_party/terraform/services/container/resource_container_node_pool.go.erb
@@ -1787,6 +1787,39 @@ func nodePoolUpdate(d *schema.ResourceData, meta interface{}, nodePoolInfo *Node
 			log.Printf("[INFO] Updated workload_metadata_config for node pool %s", name)
 		}
 
+		if d.HasChange(prefix + "node_config.0.gcfs_config") {
+			gcfsEnabled := bool(d.Get(prefix + "node_config.0.gcfs_config.0.enabled").(bool))
+			req := &container.UpdateNodePoolRequest{
+				NodePoolId: name,
+				GcfsConfig: &container.GcfsConfig{
+					Enabled: gcfsEnabled,
+				},
+			}
+			updateF := func() error {
+				clusterNodePoolsUpdateCall := config.NewContainerClient(userAgent).Projects.Locations.Clusters.NodePools.Update(nodePoolInfo.fullyQualifiedName(name),req)
+				if config.UserProjectOverride {
+					clusterNodePoolsUpdateCall.Header().Add("X-Goog-User-Project", nodePoolInfo.project)
+				}
+				op, err := clusterNodePoolsUpdateCall.Do()
+				if err != nil {
+					return err
+				}
+
+				// Wait until it's updated
+				return ContainerOperationWait(config, op,
+					nodePoolInfo.project,
+					nodePoolInfo.location,
+					"updating GKE node pool gcfs_config", userAgent,
+					timeout)
+			}
+
+			if err := retryWhileIncompatibleOperation(timeout, npLockKey, updateF); err != nil {
+					return err
+			}
+
+			log.Printf("[INFO] Updated gcfs_config for node pool %s", name)
+		}
+
 		if d.HasChange(prefix + "node_config.0.kubelet_config") {
 			req := &container.UpdateNodePoolRequest{
 				NodePoolId: name,

diff --git a/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb b/mmv1/third_party/terraform/services/container/resource_container_node_pool_test.go.erb
@@ -1675,9 +1675,9 @@ resource "google_container_node_pool" "np" {
   node_config {
     machine_type = "n1-standard-8"
     image_type = "COS_CONTAINERD"
-	gcfs_config {
-  		enabled = true
-	}
+    gcfs_config {
+      enabled = true
+    }
     secondary_boot_disks {
       disk_image = ""
       mode = "CONTAINER_IMAGE_CACHE"
@@ -1694,9 +1694,9 @@ resource "google_container_node_pool" "np-no-mode" {
   node_config {
     machine_type = "n1-standard-8"
     image_type = "COS_CONTAINERD"
-	gcfs_config {
-  		enabled = true
-	}
+    gcfs_config {
+      enabled = true
+    }
     secondary_boot_disks {
       disk_image = ""
     }
@@ -1720,10 +1720,14 @@ func TestAccContainerNodePool_gcfsConfig(t *testing.T) {
 		Steps: []resource.TestStep{
 			{
 				Config: testAccContainerNodePool_gcfsConfig(cluster, np, networkName, subnetworkName, true),
-				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr("google_container_node_pool.np",
-						"node_config.0.gcfs_config.0.enabled", "true"),
-				),
+			},
+			{
+				ResourceName:            "google_container_node_pool.np",
+				ImportState:             true,
+				ImportStateVerify:       true,
+			},
+			{
+				Config: testAccContainerNodePool_gcfsConfig(cluster, np, networkName, subnetworkName, false),
 			},
 			{
 				ResourceName:            "google_container_node_pool.np",

diff --git a/mmv1/third_party/terraform/website/docs/r/container_cluster.html.markdown b/mmv1/third_party/terraform/website/docs/r/container_cluster.html.markdown
@@ -1038,7 +1038,7 @@ sole_tenant_config {
 
 <a name="nested_gcfs_config"></a>The `gcfs_config` block supports:
 
-* `enabled` (Required) - Whether or not the Google Container Filesystem (GCFS) is enabled
+* `enabled` (Optional) - Whether or not the Google Container Filesystem (GCFS) is enabled
 
 <a name="nested_gvnic"></a>The `gvnic` block supports: