Merge pull request terraform-google-modules#313 from taylorludwig/bug…

…fix/311-autoscaling_node_count

Don't set initial_node_count when autoscaling disabled on node pool

CHANGELOG.md

@@ -12,6 +12,7 @@ Extending the adopted spec, each change should have a link to its corresponding
 
 * Support for Shielded Nodes beta feature via `enabled_shielded_nodes` variable. [#300]
 * Support for setting node_locations on node pools. [#303]
+* Fix for specifying  `node_count` on node pools when autoscaling is disabled. [#313]
 
 ## [v5.1.1] - 2019-10-25
 

autogen/cluster.tf

@@ -320,16 +320,18 @@ resource "google_container_node_pool" "pools" {
     "version",
     local.node_version,
   )
-  initial_node_count = lookup(
+
+  initial_node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? lookup(
     var.node_pools[count.index],
     "initial_node_count",
-    lookup(var.node_pools[count.index], "min_count", 1),
-  )
+    lookup(var.node_pools[count.index], "min_count", 1)
+  ) : null
+
   {% if beta_cluster %}
   max_pods_per_node = lookup(var.node_pools[count.index], "max_pods_per_node", null)
   {% endif %}
 
-  node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? null : lookup(var.node_pools[count.index], "min_count", 1)
+  node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? null : lookup(var.node_pools[count.index], "node_count", 1)
 
   dynamic "autoscaling" {
     for_each = lookup(var.node_pools[count.index], "autoscaling", true) ? [var.node_pools[count.index]] : []

cluster.tf

@@ -137,13 +137,15 @@ resource "google_container_node_pool" "pools" {
     "version",
     local.node_version,
   )
-  initial_node_count = lookup(
+
+  initial_node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? lookup(
     var.node_pools[count.index],
     "initial_node_count",
-    lookup(var.node_pools[count.index], "min_count", 1),
-  )
+    lookup(var.node_pools[count.index], "min_count", 1)
+  ) : null
+
 
-  node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? null : lookup(var.node_pools[count.index], "min_count", 1)
+  node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? null : lookup(var.node_pools[count.index], "node_count", 1)
 
   dynamic "autoscaling" {
     for_each = lookup(var.node_pools[count.index], "autoscaling", true) ? [var.node_pools[count.index]] : []

examples/node_pool/main.tf

@@ -60,10 +60,10 @@ module "gke" {
     },
     {
       name            = "pool-03"
-      node_locations  = "us-east4-b,us-east4-c"
+      node_locations  = "${var.region}-b,${var.region}-c"
+      autoscaling     = false
+      node_count      = 2
       machine_type    = "n1-standard-2"
-      min_count       = 1
-      max_count       = 2
       disk_type       = "pd-standard"
       image_type      = "COS"
       auto_upgrade    = true

modules/beta-private-cluster-update-variant/cluster.tf

@@ -292,14 +292,16 @@ resource "google_container_node_pool" "pools" {
     "version",
     local.node_version,
   )
-  initial_node_count = lookup(
+
+  initial_node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? lookup(
     var.node_pools[count.index],
     "initial_node_count",
-    lookup(var.node_pools[count.index], "min_count", 1),
-  )
+    lookup(var.node_pools[count.index], "min_count", 1)
+  ) : null
+
   max_pods_per_node = lookup(var.node_pools[count.index], "max_pods_per_node", null)
 
-  node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? null : lookup(var.node_pools[count.index], "min_count", 1)
+  node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? null : lookup(var.node_pools[count.index], "node_count", 1)
 
   dynamic "autoscaling" {
     for_each = lookup(var.node_pools[count.index], "autoscaling", true) ? [var.node_pools[count.index]] : []

modules/beta-private-cluster/cluster.tf

@@ -220,14 +220,16 @@ resource "google_container_node_pool" "pools" {
     "version",
     local.node_version,
   )
-  initial_node_count = lookup(
+
+  initial_node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? lookup(
     var.node_pools[count.index],
     "initial_node_count",
-    lookup(var.node_pools[count.index], "min_count", 1),
-  )
+    lookup(var.node_pools[count.index], "min_count", 1)
+  ) : null
+
   max_pods_per_node = lookup(var.node_pools[count.index], "max_pods_per_node", null)
 
-  node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? null : lookup(var.node_pools[count.index], "min_count", 1)
+  node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? null : lookup(var.node_pools[count.index], "node_count", 1)
 
   dynamic "autoscaling" {
     for_each = lookup(var.node_pools[count.index], "autoscaling", true) ? [var.node_pools[count.index]] : []

modules/beta-public-cluster/cluster.tf

@@ -215,14 +215,16 @@ resource "google_container_node_pool" "pools" {
     "version",
     local.node_version,
   )
-  initial_node_count = lookup(
+
+  initial_node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? lookup(
     var.node_pools[count.index],
     "initial_node_count",
-    lookup(var.node_pools[count.index], "min_count", 1),
-  )
+    lookup(var.node_pools[count.index], "min_count", 1)
+  ) : null
+
   max_pods_per_node = lookup(var.node_pools[count.index], "max_pods_per_node", null)
 
-  node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? null : lookup(var.node_pools[count.index], "min_count", 1)
+  node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? null : lookup(var.node_pools[count.index], "node_count", 1)
 
   dynamic "autoscaling" {
     for_each = lookup(var.node_pools[count.index], "autoscaling", true) ? [var.node_pools[count.index]] : []

modules/private-cluster-update-variant/cluster.tf

@@ -214,13 +214,15 @@ resource "google_container_node_pool" "pools" {
     "version",
     local.node_version,
   )
-  initial_node_count = lookup(
+
+  initial_node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? lookup(
     var.node_pools[count.index],
     "initial_node_count",
-    lookup(var.node_pools[count.index], "min_count", 1),
-  )
+    lookup(var.node_pools[count.index], "min_count", 1)
+  ) : null
+
 
-  node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? null : lookup(var.node_pools[count.index], "min_count", 1)
+  node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? null : lookup(var.node_pools[count.index], "node_count", 1)
 
   dynamic "autoscaling" {
     for_each = lookup(var.node_pools[count.index], "autoscaling", true) ? [var.node_pools[count.index]] : []

modules/private-cluster/cluster.tf

@@ -142,13 +142,15 @@ resource "google_container_node_pool" "pools" {
     "version",
     local.node_version,
   )
-  initial_node_count = lookup(
+
+  initial_node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? lookup(
     var.node_pools[count.index],
     "initial_node_count",
-    lookup(var.node_pools[count.index], "min_count", 1),
-  )
+    lookup(var.node_pools[count.index], "min_count", 1)
+  ) : null
+
 
-  node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? null : lookup(var.node_pools[count.index], "min_count", 1)
+  node_count = lookup(var.node_pools[count.index], "autoscaling", true) ? null : lookup(var.node_pools[count.index], "node_count", 1)
 
   dynamic "autoscaling" {
     for_each = lookup(var.node_pools[count.index], "autoscaling", true) ? [var.node_pools[count.index]] : []

test/fixtures/shared/variables.tf

@@ -20,13 +20,13 @@ variable "project_id" {
 
 variable "region" {
   description = "The GCP region to create and test resources in"
-  default     = "us-east4"
+  default     = "us-central1"
 }
 
 variable "zones" {
   type        = list(string)
   description = "The GCP zones to create and test resources in, for applicable tests"
-  default     = ["us-east4-a", "us-east4-b", "us-east4-c"]
+  default     = ["us-central1-a", "us-central1-b", "us-central1-c"]
 }
 
 variable "compute_engine_service_account" {
@@ -36,4 +36,3 @@ variable "compute_engine_service_account" {
 variable "registry_project_id" {
   description = "Project to use for granting access to the GCR registry, if requested"
 }
-

test/integration/node_pool/controls/gcloud.rb

@@ -279,6 +279,7 @@
           )
         end
       end
+
       describe "pool-03" do
         it "exists" do
           expect(data['nodePools']).to include(
@@ -287,6 +288,7 @@
             )
           )
         end
+
         it "is the expected machine type" do
           expect(data['nodePools']).to include(
             including(
@@ -298,8 +300,8 @@
           )
         end
 
-        it "has autoscaling enabled" do
-          expect(data['nodePools']).to include(
+        it "has autoscaling disabled" do
+          expect(data['nodePools']).not_to include(
             including(
               "name" => "pool-03",
               "autoscaling" => including(
@@ -309,13 +311,11 @@
           )
         end
 
-        it "has the expected minimum node count" do
+        it "has the expected node count" do
           expect(data['nodePools']).to include(
             including(
               "name" => "pool-03",
-              "autoscaling" => including(
-                "minNodeCount" => 1,
-              ),
+              "initialNodeCount" => 2
             )
           )
         end
@@ -342,9 +342,39 @@
           )
         end
 
+        it "has the expected labels" do
+          expect(data['nodePools']).to include(
+            including(
+              "name" => "pool-03",
+              "config" => including(
+                "labels" => {
+                  "all-pools-example" => "true",
+                  "cluster_name" => cluster_name,
+                  "node_pool" => "pool-03",
+                },
+              ),
+            )
+          )
+        end
+
+        it "has the expected network tags" do
+          expect(data['nodePools']).to include(
+            including(
+              "name" => "pool-03",
+              "config" => including(
+                "tags" => match_array([
+                  "all-node-example",
+                  "gke-#{cluster_name}",
+                  "gke-#{cluster_name}-pool-03",
+                ]),
+              ),
+            )
+          )
+        end
       end
     end
   end
+
   describe command("gcloud beta --project=#{project_id} container clusters --zone=#{location} describe #{cluster_name} --format=json") do
     its(:exit_status) { should eq 0 }
     its(:stderr) { should eq '' }
@@ -362,8 +392,8 @@
         including(
           "name" => "pool-03",
           "locations" => match_array([
-            "us-east4-b",
-            "us-east4-c",
+            "us-central1-b",
+            "us-central1-c",
           ]),
         )
       )

test/integration/node_pool/controls/kubectl.rb

@@ -72,6 +72,21 @@
           all_nodes.select { |n| n.metadata.labels.node_pool == "pool-02" }
         end
 
+        it "has the expected taints" do
+          expect(taints).to include(
+            {
+              effect: "PreferNoSchedule",
+              key: "all-pools-example",
+              value: "true",
+            }
+          )
+        end
+      end
+      describe "pool-03" do
+        let(:nodes) do
+          all_nodes.select { |n| n.metadata.labels.node_pool == "pool-03" }
+        end
+
         it "has the expected taints" do
           expect(taints).to include(
             {