Tweak max unhealthy nodes to make max 99% + add more tests

Author: vincentportella

docs/configuration/nodegroup.md

@@ -298,4 +298,6 @@ This field is required.
 
 The maximum percentage of unhealthy nodes in the test set from `health_check_newest_nodes_percent`. Beyond this threshold all scaling activity is paused and unhealthy nodes are flushed out.
 
+> **Note:** The valid range for `max_unhealthy_nodes_percent` is `0%` to `99%`.
+
 This is an optional field. If not set, it will default to `0%`.

pkg/controller/controller_test.go

@@ -2,6 +2,7 @@ package controller
 
 import (
 	"context"
+	"fmt"
 	"testing"
 	"time"
 
@@ -442,3 +443,112 @@ func TestTaintUnhealthyInstances(t *testing.T) {
 		})
 	}
 }
+
+func TestIsNodegroupHealthy(t *testing.T) {
+	c := &Controller{}
+	now := time.Now()
+	gracePeriod := 10 * time.Minute
+
+	buildNodes := func(total int, unhealthy int, creationTime time.Time) []*v1.Node {
+		nodes := make([]*v1.Node, total)
+		for i := 0; i < total; i++ {
+			notReady := i < unhealthy
+			nodes[i] = test.BuildTestNode(test.NodeOpts{
+				Name:     fmt.Sprintf("n%d", i+1),
+				NotReady: notReady,
+				Creation: creationTime,
+			})
+		}
+		return nodes
+	}
+
+	oldCreationTime := now.Add(-2 * gracePeriod)
+	newCreationTime := now.Add(-gracePeriod / 2)
+
+	tests := []struct {
+		name    string
+		state   *NodeGroupState
+		nodes   []*v1.Node
+		healthy bool
+	}{
+		{
+			name: "0 nodes, healthy",
+			state: &NodeGroupState{
+				Opts: NodeGroupOptions{
+					unhealthyNodeGracePeriodDuration: gracePeriod,
+					HealthCheckNewestNodesPercent:    100,
+					MaxUnhealthyNodesPercent:         50,
+				},
+			},
+			nodes:   buildNodes(0, 0, oldCreationTime),
+			healthy: true,
+		},
+		{
+			name: "0 nodes, max unhealthy 0%, healthy",
+			state: &NodeGroupState{
+				Opts: NodeGroupOptions{
+					unhealthyNodeGracePeriodDuration: gracePeriod,
+					HealthCheckNewestNodesPercent:    100,
+					MaxUnhealthyNodesPercent:         0,
+				},
+			},
+			nodes:   buildNodes(0, 0, oldCreationTime),
+			healthy: true,
+		},
+		{
+			name: "4 nodes, all nodes too new, healthy",
+			state: &NodeGroupState{
+				Opts: NodeGroupOptions{
+					unhealthyNodeGracePeriodDuration: gracePeriod,
+					HealthCheckNewestNodesPercent:    100,
+					MaxUnhealthyNodesPercent:         50,
+				},
+			},
+			nodes:   buildNodes(4, 4, newCreationTime),
+			healthy: true,
+		},
+		{
+			name: "4 nodes, all nodes old, unhealthy",
+			state: &NodeGroupState{
+				Opts: NodeGroupOptions{
+					unhealthyNodeGracePeriodDuration: gracePeriod,
+					HealthCheckNewestNodesPercent:    100,
+					MaxUnhealthyNodesPercent:         50,
+				},
+			},
+			nodes:   buildNodes(4, 4, oldCreationTime),
+			healthy: false,
+		},
+		{
+			name: "4 nodes, all healthy, max unhealthy 0%, healthy",
+			state: &NodeGroupState{
+				Opts: NodeGroupOptions{
+					unhealthyNodeGracePeriodDuration: gracePeriod,
+					HealthCheckNewestNodesPercent:    100,
+					MaxUnhealthyNodesPercent:         0,
+				},
+			},
+			nodes:   buildNodes(4, 0, oldCreationTime),
+			healthy: true,
+		},
+		{
+			name: "4 nodes, all unhealthy, max unhealthy 100%, healthy",
+			state: &NodeGroupState{
+				Opts: NodeGroupOptions{
+					unhealthyNodeGracePeriodDuration: gracePeriod,
+					HealthCheckNewestNodesPercent:    100,
+					MaxUnhealthyNodesPercent:         99,
+				},
+			},
+			nodes:   buildNodes(4, 4, oldCreationTime),
+			healthy: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := c.isNodegroupHealthy(tt.state, tt.nodes)
+			assert.Equal(t, tt.healthy, got)
+		})
+	}
+}

pkg/controller/node_group.go

@@ -151,7 +151,7 @@ func ValidateNodeGroup(nodegroup NodeGroupOptions) []error {
 		checkThat(nodegroup.HealthCheckNewestNodesPercent > 0, "health_check_newest_nodes_percent must be greater than 0")
 		checkThat(nodegroup.HealthCheckNewestNodesPercent <= 100, "health_check_newest_nodes_percent must be less than or equal to 100")
 		checkThat(nodegroup.MaxUnhealthyNodesPercent >= 0, "max_unhealthy_nodes_percent must be greater than or equal to 0")
-		checkThat(nodegroup.MaxUnhealthyNodesPercent <= 100, "max_unhealthy_nodes_percent must be less than or equal to 100")
+		checkThat(nodegroup.MaxUnhealthyNodesPercent < 100, "max_unhealthy_nodes_percent must be less than 100")
 	}
 
 	return problems

pkg/controller/node_group_test.go

@@ -506,7 +506,7 @@ func TestValidateNodeGroup(t *testing.T) {
 					ScaleUpCoolDownPeriod:              "55m",
 					UnhealthyNodeGracePeriod:           "500h",
 					HealthCheckNewestNodesPercent:      1,
-					MaxUnhealthyNodesPercent:           100,
+					MaxUnhealthyNodesPercent:           99,
 				},
 			},
 			nil,
@@ -560,7 +560,7 @@ func TestValidateNodeGroup(t *testing.T) {
 					MaxNodeAge:                         "bla",
 					UnhealthyNodeGracePeriod:           "bla",
 					HealthCheckNewestNodesPercent:      101,
-					MaxUnhealthyNodesPercent:           101,
+					MaxUnhealthyNodesPercent:           100,
 				},
 			},
 			[]string{
@@ -573,7 +573,7 @@ func TestValidateNodeGroup(t *testing.T) {
 				"max_node_age failed to parse into a time.Duration. Set to '0' or '' to disable, or a positive Go duration to enable.",
 				"unhealthy_node_grace_period failed to parse into a time.Duration. check your formatting.",
 				"health_check_newest_nodes_percent must be less than or equal to 100",
-				"max_unhealthy_nodes_percent must be less than or equal to 100",
+				"max_unhealthy_nodes_percent must be less than 100",
 			},
 		},
 		{