Skip to content

Commit

Permalink
container: fix updates for node_config.gcfs_config
Browse files Browse the repository at this point in the history
- 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 #19482
Follow up to GoogleCloudPlatform#11553
  • Loading branch information
wyardley committed Sep 14, 2024
1 parent 6d67e34 commit 3042a94
Show file tree
Hide file tree
Showing 5 changed files with 167 additions and 17 deletions.
15 changes: 8 additions & 7 deletions mmv1/third_party/terraform/services/container/node_config.go.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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" {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 = ""
}
Expand All @@ -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",
Expand Down

0 comments on commit 3042a94

Please sign in to comment.