fix: node_pools_cgroup_mode var does not correctly handle the "all" map key (#2224)

DrFaust92 · apeabody · web-flow · commit 53b1a50b0e7e · 2025-01-02T12:06:31.000-08:00
Signed-off-by: drfaust92 &lt;ilia.lazebnik@gmail.com&gt;
Co-authored-by: Andrew Peabody &lt;andrewpeabody@google.com&gt;
diff --git a/autogen/main/cluster.tf.tmpl b/autogen/main/cluster.tf.tmpl
@@ -1111,6 +1111,7 @@ resource "google_container_node_pool" "windows_pools" {
       for_each = length(merge(
         local.node_pools_linux_node_configs_sysctls["all"],
         local.node_pools_linux_node_configs_sysctls[each.value["name"]],
+        local.node_pools_cgroup_mode["all"] == "" ? {} : { cgroup = local.node_pools_cgroup_mode["all"] },
         local.node_pools_cgroup_mode[each.value["name"]] == "" ? {} : {cgroup = local.node_pools_cgroup_mode[each.value["name"]]}
       )) != 0 ? [1] : []
 
@@ -1119,7 +1120,7 @@ resource "google_container_node_pool" "windows_pools" {
           local.node_pools_linux_node_configs_sysctls["all"],
           local.node_pools_linux_node_configs_sysctls[each.value["name"]]
         )
-        cgroup_mode = local.node_pools_cgroup_mode[each.value["name"]] == "" ? null : local.node_pools_cgroup_mode[each.value["name"]]
+        cgroup_mode = coalesce(local.node_pools_cgroup_mode[each.value["name"]], local.node_pools_cgroup_mode["all"], null)
       }
     }
     {% endif %}
diff --git a/autogen/main/variables_defaults.tf.tmpl b/autogen/main/variables_defaults.tf.tmpl
@@ -128,14 +128,15 @@ locals {
     ),
     var.node_pools_linux_node_configs_sysctls
   )
-   node_pools_cgroup_mode = merge(
-    { all = "" },
-    { default-node-pool = "" },
-    zipmap(
-      [for node_pool in var.node_pools : node_pool["name"]],
-      [for node_pool in var.node_pools : ""]
-    ),
-    var.node_pools_cgroup_mode
+
+  node_pools_cgroup_mode = merge(
+   { all = "" },
+   { default-node-pool = "" },
+   zipmap(
+     [for node_pool in var.node_pools : node_pool["name"]],
+     [for node_pool in var.node_pools : ""]
+   ),
+   var.node_pools_cgroup_mode
   )
 }
 {% endif %}
diff --git a/cluster.tf b/cluster.tf
@@ -804,6 +804,7 @@ resource "google_container_node_pool" "pools" {
       for_each = length(merge(
         local.node_pools_linux_node_configs_sysctls["all"],
         local.node_pools_linux_node_configs_sysctls[each.value["name"]],
+        local.node_pools_cgroup_mode["all"] == "" ? {} : { cgroup = local.node_pools_cgroup_mode["all"] },
         local.node_pools_cgroup_mode[each.value["name"]] == "" ? {} : { cgroup = local.node_pools_cgroup_mode[each.value["name"]] }
       )) != 0 ? [1] : []
 
@@ -812,7 +813,7 @@ resource "google_container_node_pool" "pools" {
           local.node_pools_linux_node_configs_sysctls["all"],
           local.node_pools_linux_node_configs_sysctls[each.value["name"]]
         )
-        cgroup_mode = local.node_pools_cgroup_mode[each.value["name"]] == "" ? null : local.node_pools_cgroup_mode[each.value["name"]]
+        cgroup_mode = coalesce(local.node_pools_cgroup_mode[each.value["name"]], local.node_pools_cgroup_mode["all"], null)
       }
     }
 
diff --git a/examples/node_pool/main.tf b/examples/node_pool/main.tf
@@ -150,4 +150,9 @@ module "gke" {
       "net.core.netdev_max_backlog" = "20000"
     }
   }
+
+  node_pools_cgroup_mode = {
+    all     = "CGROUP_MODE_V1"
+    pool-01 = "CGROUP_MODE_V2"
+  }
 }
diff --git a/modules/beta-private-cluster-update-variant/cluster.tf b/modules/beta-private-cluster-update-variant/cluster.tf
@@ -975,6 +975,7 @@ resource "google_container_node_pool" "pools" {
       for_each = length(merge(
         local.node_pools_linux_node_configs_sysctls["all"],
         local.node_pools_linux_node_configs_sysctls[each.value["name"]],
+        local.node_pools_cgroup_mode["all"] == "" ? {} : { cgroup = local.node_pools_cgroup_mode["all"] },
         local.node_pools_cgroup_mode[each.value["name"]] == "" ? {} : { cgroup = local.node_pools_cgroup_mode[each.value["name"]] }
       )) != 0 ? [1] : []
 
@@ -983,7 +984,7 @@ resource "google_container_node_pool" "pools" {
           local.node_pools_linux_node_configs_sysctls["all"],
           local.node_pools_linux_node_configs_sysctls[each.value["name"]]
         )
-        cgroup_mode = local.node_pools_cgroup_mode[each.value["name"]] == "" ? null : local.node_pools_cgroup_mode[each.value["name"]]
+        cgroup_mode = coalesce(local.node_pools_cgroup_mode[each.value["name"]], local.node_pools_cgroup_mode["all"], null)
       }
     }
 
diff --git a/modules/beta-private-cluster-update-variant/variables_defaults.tf b/modules/beta-private-cluster-update-variant/variables_defaults.tf
@@ -127,6 +127,7 @@ locals {
     ),
     var.node_pools_linux_node_configs_sysctls
   )
+
   node_pools_cgroup_mode = merge(
     { all = "" },
     { default-node-pool = "" },
diff --git a/modules/beta-private-cluster/cluster.tf b/modules/beta-private-cluster/cluster.tf
@@ -894,6 +894,7 @@ resource "google_container_node_pool" "pools" {
       for_each = length(merge(
         local.node_pools_linux_node_configs_sysctls["all"],
         local.node_pools_linux_node_configs_sysctls[each.value["name"]],
+        local.node_pools_cgroup_mode["all"] == "" ? {} : { cgroup = local.node_pools_cgroup_mode["all"] },
         local.node_pools_cgroup_mode[each.value["name"]] == "" ? {} : { cgroup = local.node_pools_cgroup_mode[each.value["name"]] }
       )) != 0 ? [1] : []
 
@@ -902,7 +903,7 @@ resource "google_container_node_pool" "pools" {
           local.node_pools_linux_node_configs_sysctls["all"],
           local.node_pools_linux_node_configs_sysctls[each.value["name"]]
         )
-        cgroup_mode = local.node_pools_cgroup_mode[each.value["name"]] == "" ? null : local.node_pools_cgroup_mode[each.value["name"]]
+        cgroup_mode = coalesce(local.node_pools_cgroup_mode[each.value["name"]], local.node_pools_cgroup_mode["all"], null)
       }
     }
 
diff --git a/modules/beta-private-cluster/variables_defaults.tf b/modules/beta-private-cluster/variables_defaults.tf
@@ -127,6 +127,7 @@ locals {
     ),
     var.node_pools_linux_node_configs_sysctls
   )
+
   node_pools_cgroup_mode = merge(
     { all = "" },
     { default-node-pool = "" },
diff --git a/modules/beta-public-cluster-update-variant/cluster.tf b/modules/beta-public-cluster-update-variant/cluster.tf
@@ -945,6 +945,7 @@ resource "google_container_node_pool" "pools" {
       for_each = length(merge(
         local.node_pools_linux_node_configs_sysctls["all"],
         local.node_pools_linux_node_configs_sysctls[each.value["name"]],
+        local.node_pools_cgroup_mode["all"] == "" ? {} : { cgroup = local.node_pools_cgroup_mode["all"] },
         local.node_pools_cgroup_mode[each.value["name"]] == "" ? {} : { cgroup = local.node_pools_cgroup_mode[each.value["name"]] }
       )) != 0 ? [1] : []
 
@@ -953,7 +954,7 @@ resource "google_container_node_pool" "pools" {
           local.node_pools_linux_node_configs_sysctls["all"],
           local.node_pools_linux_node_configs_sysctls[each.value["name"]]
         )
-        cgroup_mode = local.node_pools_cgroup_mode[each.value["name"]] == "" ? null : local.node_pools_cgroup_mode[each.value["name"]]
+        cgroup_mode = coalesce(local.node_pools_cgroup_mode[each.value["name"]], local.node_pools_cgroup_mode["all"], null)
       }
     }
 
diff --git a/modules/beta-public-cluster-update-variant/variables_defaults.tf b/modules/beta-public-cluster-update-variant/variables_defaults.tf
@@ -127,6 +127,7 @@ locals {
     ),
     var.node_pools_linux_node_configs_sysctls
   )
+
   node_pools_cgroup_mode = merge(
     { all = "" },
     { default-node-pool = "" },
diff --git a/modules/beta-public-cluster/cluster.tf b/modules/beta-public-cluster/cluster.tf
@@ -864,6 +864,7 @@ resource "google_container_node_pool" "pools" {
       for_each = length(merge(
         local.node_pools_linux_node_configs_sysctls["all"],
         local.node_pools_linux_node_configs_sysctls[each.value["name"]],
+        local.node_pools_cgroup_mode["all"] == "" ? {} : { cgroup = local.node_pools_cgroup_mode["all"] },
         local.node_pools_cgroup_mode[each.value["name"]] == "" ? {} : { cgroup = local.node_pools_cgroup_mode[each.value["name"]] }
       )) != 0 ? [1] : []
 
@@ -872,7 +873,7 @@ resource "google_container_node_pool" "pools" {
           local.node_pools_linux_node_configs_sysctls["all"],
           local.node_pools_linux_node_configs_sysctls[each.value["name"]]
         )
-        cgroup_mode = local.node_pools_cgroup_mode[each.value["name"]] == "" ? null : local.node_pools_cgroup_mode[each.value["name"]]
+        cgroup_mode = coalesce(local.node_pools_cgroup_mode[each.value["name"]], local.node_pools_cgroup_mode["all"], null)
       }
     }
 
diff --git a/modules/beta-public-cluster/variables_defaults.tf b/modules/beta-public-cluster/variables_defaults.tf
@@ -127,6 +127,7 @@ locals {
     ),
     var.node_pools_linux_node_configs_sysctls
   )
+
   node_pools_cgroup_mode = merge(
     { all = "" },
     { default-node-pool = "" },
diff --git a/modules/private-cluster-update-variant/cluster.tf b/modules/private-cluster-update-variant/cluster.tf
@@ -914,6 +914,7 @@ resource "google_container_node_pool" "pools" {
       for_each = length(merge(
         local.node_pools_linux_node_configs_sysctls["all"],
         local.node_pools_linux_node_configs_sysctls[each.value["name"]],
+        local.node_pools_cgroup_mode["all"] == "" ? {} : { cgroup = local.node_pools_cgroup_mode["all"] },
         local.node_pools_cgroup_mode[each.value["name"]] == "" ? {} : { cgroup = local.node_pools_cgroup_mode[each.value["name"]] }
       )) != 0 ? [1] : []
 
@@ -922,7 +923,7 @@ resource "google_container_node_pool" "pools" {
           local.node_pools_linux_node_configs_sysctls["all"],
           local.node_pools_linux_node_configs_sysctls[each.value["name"]]
         )
-        cgroup_mode = local.node_pools_cgroup_mode[each.value["name"]] == "" ? null : local.node_pools_cgroup_mode[each.value["name"]]
+        cgroup_mode = coalesce(local.node_pools_cgroup_mode[each.value["name"]], local.node_pools_cgroup_mode["all"], null)
       }
     }
 
diff --git a/modules/private-cluster-update-variant/variables_defaults.tf b/modules/private-cluster-update-variant/variables_defaults.tf
@@ -127,6 +127,7 @@ locals {
     ),
     var.node_pools_linux_node_configs_sysctls
   )
+
   node_pools_cgroup_mode = merge(
     { all = "" },
     { default-node-pool = "" },
diff --git a/modules/private-cluster/cluster.tf b/modules/private-cluster/cluster.tf
@@ -834,6 +834,7 @@ resource "google_container_node_pool" "pools" {
       for_each = length(merge(
         local.node_pools_linux_node_configs_sysctls["all"],
         local.node_pools_linux_node_configs_sysctls[each.value["name"]],
+        local.node_pools_cgroup_mode["all"] == "" ? {} : { cgroup = local.node_pools_cgroup_mode["all"] },
         local.node_pools_cgroup_mode[each.value["name"]] == "" ? {} : { cgroup = local.node_pools_cgroup_mode[each.value["name"]] }
       )) != 0 ? [1] : []
 
@@ -842,7 +843,7 @@ resource "google_container_node_pool" "pools" {
           local.node_pools_linux_node_configs_sysctls["all"],
           local.node_pools_linux_node_configs_sysctls[each.value["name"]]
         )
-        cgroup_mode = local.node_pools_cgroup_mode[each.value["name"]] == "" ? null : local.node_pools_cgroup_mode[each.value["name"]]
+        cgroup_mode = coalesce(local.node_pools_cgroup_mode[each.value["name"]], local.node_pools_cgroup_mode["all"], null)
       }
     }
 
diff --git a/modules/private-cluster/variables_defaults.tf b/modules/private-cluster/variables_defaults.tf
@@ -127,6 +127,7 @@ locals {
     ),
     var.node_pools_linux_node_configs_sysctls
   )
+
   node_pools_cgroup_mode = merge(
     { all = "" },
     { default-node-pool = "" },
diff --git a/test/integration/node_pool/testdata/TestNodePool.json b/test/integration/node_pool/testdata/TestNodePool.json
@@ -270,7 +270,7 @@
       "config": {
         "diskSizeGb": 100,
         "diskType": "pd-standard",
-        "effectiveCgroupMode": "EFFECTIVE_CGROUP_MODE_V2",
+        "effectiveCgroupMode": "EFFECTIVE_CGROUP_MODE_V1",
         "imageType": "COS_CONTAINERD",
         "machineType": "e2-medium",
         "metadata": {
@@ -425,7 +425,7 @@
         ],
         "diskSizeGb": 30,
         "diskType": "pd-standard",
-        "effectiveCgroupMode": "EFFECTIVE_CGROUP_MODE_V2",
+        "effectiveCgroupMode": "EFFECTIVE_CGROUP_MODE_V1",
         "imageType": "COS_CONTAINERD",
         "labels": {
           "all-pools-example": "true",
@@ -505,7 +505,7 @@
       "config": {
         "diskSizeGb": 100,
         "diskType": "pd-standard",
-        "effectiveCgroupMode": "EFFECTIVE_CGROUP_MODE_V2",
+        "effectiveCgroupMode": "EFFECTIVE_CGROUP_MODE_V1",
         "imageType": "COS_CONTAINERD",
         "kubeletConfig": {
           "cpuCfsQuota": true,
@@ -603,7 +603,7 @@
       "config": {
         "diskSizeGb": 100,
         "diskType": "pd-standard",
-        "effectiveCgroupMode": "EFFECTIVE_CGROUP_MODE_V2",
+        "effectiveCgroupMode": "EFFECTIVE_CGROUP_MODE_V1",
         "imageType": "COS_CONTAINERD",
         "labels": {
           "all-pools-example": "true",
@@ -698,7 +698,7 @@
         },
         "diskSizeGb": 100,
         "diskType": "pd-standard",
-        "effectiveCgroupMode": "EFFECTIVE_CGROUP_MODE_V2",
+        "effectiveCgroupMode": "EFFECTIVE_CGROUP_MODE_V1",
         "imageType": "COS_CONTAINERD",
         "labels": {
           "all-pools-example": "true",
diff --git a/variables_defaults.tf b/variables_defaults.tf
@@ -127,6 +127,7 @@ locals {
     ),
     var.node_pools_linux_node_configs_sysctls
   )
+
   node_pools_cgroup_mode = merge(
     { all = "" },
     { default-node-pool = "" },

Original file line number	Diff line number	Diff line change
`@@ -1111,6 +1111,7 @@ resource "google_container_node_pool" "windows_pools" {`
`1111`	`1111`	`for_each = length(merge(`
`1112`	`1112`	`local.node_pools_linux_node_configs_sysctls["all"],`
`1113`	`1113`	`local.node_pools_linux_node_configs_sysctls[each.value["name"]],`
	`1114`	`+ local.node_pools_cgroup_mode["all"] == "" ? {} : { cgroup = local.node_pools_cgroup_mode["all"] },`
`1114`	`1115`	`local.node_pools_cgroup_mode[each.value["name"]] == "" ? {} : {cgroup = local.node_pools_cgroup_mode[each.value["name"]]}`
`1115`	`1116`	`)) != 0 ? [1] : []`
`1116`	`1117`
`@@ -1119,7 +1120,7 @@ resource "google_container_node_pool" "windows_pools" {`
`1119`	`1120`	`local.node_pools_linux_node_configs_sysctls["all"],`
`1120`	`1121`	`local.node_pools_linux_node_configs_sysctls[each.value["name"]]`
`1121`	`1122`	`)`
`1122`		`- cgroup_mode = local.node_pools_cgroup_mode[each.value["name"]] == "" ? null : local.node_pools_cgroup_mode[each.value["name"]]`
	`1123`	`+ cgroup_mode = coalesce(local.node_pools_cgroup_mode[each.value["name"]], local.node_pools_cgroup_mode["all"], null)`
`1123`	`1124`	`}`
`1124`	`1125`	`}`
`1125`	`1126`	`{% endif %}`
Original file line number	Diff line number	Diff line change
`@@ -804,6 +804,7 @@ resource "google_container_node_pool" "pools" {`
`804`	`804`	`for_each = length(merge(`
`805`	`805`	`local.node_pools_linux_node_configs_sysctls["all"],`
`806`	`806`	`local.node_pools_linux_node_configs_sysctls[each.value["name"]],`
	`807`	`+ local.node_pools_cgroup_mode["all"] == "" ? {} : { cgroup = local.node_pools_cgroup_mode["all"] },`
`807`	`808`	`local.node_pools_cgroup_mode[each.value["name"]] == "" ? {} : { cgroup = local.node_pools_cgroup_mode[each.value["name"]] }`
`808`	`809`	`)) != 0 ? [1] : []`
`809`	`810`
`@@ -812,7 +813,7 @@ resource "google_container_node_pool" "pools" {`
`812`	`813`	`local.node_pools_linux_node_configs_sysctls["all"],`
`813`	`814`	`local.node_pools_linux_node_configs_sysctls[each.value["name"]]`
`814`	`815`	`)`
`815`		`- cgroup_mode = local.node_pools_cgroup_mode[each.value["name"]] == "" ? null : local.node_pools_cgroup_mode[each.value["name"]]`
	`816`	`+ cgroup_mode = coalesce(local.node_pools_cgroup_mode[each.value["name"]], local.node_pools_cgroup_mode["all"], null)`
`816`	`817`	`}`
`817`	`818`	`}`
`818`	`819`
Original file line number	Diff line number	Diff line change
`@@ -150,4 +150,9 @@ module "gke" {`
`150`	`150`	`"net.core.netdev_max_backlog" = "20000"`
`151`	`151`	`}`
`152`	`152`	`}`
	`153`	`+`
	`154`	`+ node_pools_cgroup_mode = {`
	`155`	`+ all = "CGROUP_MODE_V1"`
	`156`	`+ pool-01 = "CGROUP_MODE_V2"`
	`157`	`+ }`
`153`	`158`	`}`
Original file line number	Diff line number	Diff line change
`@@ -975,6 +975,7 @@ resource "google_container_node_pool" "pools" {`
`975`	`975`	`for_each = length(merge(`
`976`	`976`	`local.node_pools_linux_node_configs_sysctls["all"],`
`977`	`977`	`local.node_pools_linux_node_configs_sysctls[each.value["name"]],`
	`978`	`+ local.node_pools_cgroup_mode["all"] == "" ? {} : { cgroup = local.node_pools_cgroup_mode["all"] },`
`978`	`979`	`local.node_pools_cgroup_mode[each.value["name"]] == "" ? {} : { cgroup = local.node_pools_cgroup_mode[each.value["name"]] }`
`979`	`980`	`)) != 0 ? [1] : []`
`980`	`981`
`@@ -983,7 +984,7 @@ resource "google_container_node_pool" "pools" {`
`983`	`984`	`local.node_pools_linux_node_configs_sysctls["all"],`
`984`	`985`	`local.node_pools_linux_node_configs_sysctls[each.value["name"]]`
`985`	`986`	`)`
`986`		`- cgroup_mode = local.node_pools_cgroup_mode[each.value["name"]] == "" ? null : local.node_pools_cgroup_mode[each.value["name"]]`
	`987`	`+ cgroup_mode = coalesce(local.node_pools_cgroup_mode[each.value["name"]], local.node_pools_cgroup_mode["all"], null)`
`987`	`988`	`}`
`988`	`989`	`}`
`989`	`990`
Original file line number	Diff line number	Diff line change
`@@ -127,6 +127,7 @@ locals {`
`127`	`127`	`),`
`128`	`128`	`var.node_pools_linux_node_configs_sysctls`
`129`	`129`	`)`
	`130`	`+`
`130`	`131`	`node_pools_cgroup_mode = merge(`
`131`	`132`	`{ all = "" },`
`132`	`133`	`{ default-node-pool = "" },`
Original file line number	Diff line number	Diff line change
`@@ -894,6 +894,7 @@ resource "google_container_node_pool" "pools" {`
`894`	`894`	`for_each = length(merge(`
`895`	`895`	`local.node_pools_linux_node_configs_sysctls["all"],`
`896`	`896`	`local.node_pools_linux_node_configs_sysctls[each.value["name"]],`
	`897`	`+ local.node_pools_cgroup_mode["all"] == "" ? {} : { cgroup = local.node_pools_cgroup_mode["all"] },`
`897`	`898`	`local.node_pools_cgroup_mode[each.value["name"]] == "" ? {} : { cgroup = local.node_pools_cgroup_mode[each.value["name"]] }`
`898`	`899`	`)) != 0 ? [1] : []`
`899`	`900`
`@@ -902,7 +903,7 @@ resource "google_container_node_pool" "pools" {`
`902`	`903`	`local.node_pools_linux_node_configs_sysctls["all"],`
`903`	`904`	`local.node_pools_linux_node_configs_sysctls[each.value["name"]]`
`904`	`905`	`)`
`905`		`- cgroup_mode = local.node_pools_cgroup_mode[each.value["name"]] == "" ? null : local.node_pools_cgroup_mode[each.value["name"]]`
	`906`	`+ cgroup_mode = coalesce(local.node_pools_cgroup_mode[each.value["name"]], local.node_pools_cgroup_mode["all"], null)`
`906`	`907`	`}`
`907`	`908`	`}`
`908`	`909`
Original file line number	Diff line number	Diff line change
`@@ -945,6 +945,7 @@ resource "google_container_node_pool" "pools" {`
`945`	`945`	`for_each = length(merge(`
`946`	`946`	`local.node_pools_linux_node_configs_sysctls["all"],`
`947`	`947`	`local.node_pools_linux_node_configs_sysctls[each.value["name"]],`
	`948`	`+ local.node_pools_cgroup_mode["all"] == "" ? {} : { cgroup = local.node_pools_cgroup_mode["all"] },`
`948`	`949`	`local.node_pools_cgroup_mode[each.value["name"]] == "" ? {} : { cgroup = local.node_pools_cgroup_mode[each.value["name"]] }`
`949`	`950`	`)) != 0 ? [1] : []`
`950`	`951`
`@@ -953,7 +954,7 @@ resource "google_container_node_pool" "pools" {`
`953`	`954`	`local.node_pools_linux_node_configs_sysctls["all"],`
`954`	`955`	`local.node_pools_linux_node_configs_sysctls[each.value["name"]]`
`955`	`956`	`)`
`956`		`- cgroup_mode = local.node_pools_cgroup_mode[each.value["name"]] == "" ? null : local.node_pools_cgroup_mode[each.value["name"]]`
	`957`	`+ cgroup_mode = coalesce(local.node_pools_cgroup_mode[each.value["name"]], local.node_pools_cgroup_mode["all"], null)`
`957`	`958`	`}`
`958`	`959`	`}`
`959`	`960`