Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't set initial_node_count when autoscaling disabled on node pool #313

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
10 changes: 6 additions & 4 deletions autogen/cluster.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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]] : []
Expand Down
10 changes: 6 additions & 4 deletions cluster.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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]] : []
Expand Down
6 changes: 3 additions & 3 deletions examples/node_pool/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 6 additions & 4 deletions modules/beta-private-cluster-update-variant/cluster.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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]] : []
Expand Down
10 changes: 6 additions & 4 deletions modules/beta-private-cluster/cluster.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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]] : []
Expand Down
10 changes: 6 additions & 4 deletions modules/beta-public-cluster/cluster.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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]] : []
Expand Down
10 changes: 6 additions & 4 deletions modules/private-cluster-update-variant/cluster.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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]] : []
Expand Down
10 changes: 6 additions & 4 deletions modules/private-cluster/cluster.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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]] : []
Expand Down
5 changes: 2 additions & 3 deletions test/fixtures/shared/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand All @@ -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"
}

46 changes: 38 additions & 8 deletions test/integration/node_pool/controls/gcloud.rb
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@
)
end
end

describe "pool-03" do
it "exists" do
expect(data['nodePools']).to include(
Expand All @@ -287,6 +288,7 @@
)
)
end

it "is the expected machine type" do
expect(data['nodePools']).to include(
including(
Expand All @@ -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(
Expand All @@ -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
Expand All @@ -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 '' }
Expand All @@ -362,8 +392,8 @@
including(
"name" => "pool-03",
"locations" => match_array([
"us-east4-b",
"us-east4-c",
"us-central1-b",
"us-central1-c",
]),
)
)
Expand Down
15 changes: 15 additions & 0 deletions test/integration/node_pool/controls/kubectl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(
{
Expand Down