Specify number of NotReady instances in makeGroup() parameter

pkg/instancegroups/BUILD.bazel

@@ -43,6 +43,7 @@ go_test(
         "//vendor/github.com/stretchr/testify/assert:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/k8s.io/client-go/kubernetes:go_default_library",
         "//vendor/k8s.io/client-go/kubernetes/fake:go_default_library",
         "//vendor/k8s.io/client-go/testing:go_default_library",
     ],

pkg/instancegroups/rollingupdate_test.go

@@ -18,7 +18,6 @@ package instancegroups
 
 import (
 	"errors"
-	"fmt"
 	"strings"
 	"testing"
 	"time"
@@ -28,6 +27,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	v1 "k8s.io/api/core/v1"
 	v1meta "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/fake"
 	testingclient "k8s.io/client-go/testing"
 	"k8s.io/kops/cloudmock/aws/mockautoscaling"
@@ -42,7 +42,7 @@ const (
 	taintPatch  = "{\"spec\":{\"taints\":[{\"effect\":\"PreferNoSchedule\",\"key\":\"kops.k8s.io/scheduled-for-update\"}]}}"
 )
 
-func getTestSetup() (*RollingUpdateCluster, awsup.AWSCloud, *kopsapi.Cluster, map[string]*cloudinstances.CloudInstanceGroup) {
+func getTestSetup() (*RollingUpdateCluster, awsup.AWSCloud, *kopsapi.Cluster) {
 	k8sClient := fake.NewSimpleClientset()
 
 	mockcloud := awsup.BuildMockAWSCloud("us-east-1", "abc")
@@ -64,9 +64,7 @@ func getTestSetup() (*RollingUpdateCluster, awsup.AWSCloud, *kopsapi.Cluster, ma
 		ValidateSuccessDuration: 5 * time.Millisecond,
 	}
 
-	groups := getGroups(k8sClient, mockcloud)
+	return c, mockcloud, cluster
-
-	return c, mockcloud, cluster, groups
 }
 
 type successfulClusterValidator struct{}
@@ -104,7 +102,9 @@ func (v *assertNotCalledClusterValidator) Validate() (*validation.ValidationClus
 	return nil, errors.New("validator called unexpectedly")
 }
 
-func makeGroup(groups map[string]*cloudinstances.CloudInstanceGroup, k8sClient *fake.Clientset, cloud awsup.AWSCloud, name string, role kopsapi.InstanceGroupRole, count int) {
+func makeGroup(groups map[string]*cloudinstances.CloudInstanceGroup, k8sClient kubernetes.Interface, cloud awsup.AWSCloud, name string, role kopsapi.InstanceGroupRole, count int, needUpdate int) {
+	fakeClient := k8sClient.(*fake.Clientset)
+
 	groups[name] = &cloudinstances.CloudInstanceGroup{
 		InstanceGroup: &kopsapi.InstanceGroup{
 			ObjectMeta: v1meta.ObjectMeta{
@@ -130,12 +130,17 @@ func makeGroup(groups map[string]*cloudinstances.CloudInstanceGroup, k8sClient *
 			node = &v1.Node{
 				ObjectMeta: v1meta.ObjectMeta{Name: id + ".local"},
 			}
-			_ = k8sClient.Tracker().Add(node)
+			_ = fakeClient.Tracker().Add(node)
 		}
-		groups[name].Ready = append(groups[name].Ready, &cloudinstances.CloudInstanceGroupMember{
+		member := cloudinstances.CloudInstanceGroupMember{
 			ID:   id,
 			Node: node,
-		})
+		}
+		if i < needUpdate {
+			groups[name].NeedUpdate = append(groups[name].NeedUpdate, &member)
+		} else {
+			groups[name].Ready = append(groups[name].Ready, &member)
+		}
 		instanceIds = append(instanceIds, aws.String(id))
 	}
 	cloud.Autoscaling().AttachInstances(&autoscaling.AttachInstancesInput{
@@ -144,45 +149,28 @@ func makeGroup(groups map[string]*cloudinstances.CloudInstanceGroup, k8sClient *
 	})
 }
 
-func getGroups(k8sClient *fake.Clientset, cloud awsup.AWSCloud) map[string]*cloudinstances.CloudInstanceGroup {
+func getGroups(k8sClient kubernetes.Interface, cloud awsup.AWSCloud) map[string]*cloudinstances.CloudInstanceGroup {
 	groups := make(map[string]*cloudinstances.CloudInstanceGroup)
-	makeGroup(groups, k8sClient, cloud, "node-1", kopsapi.InstanceGroupRoleNode, 3)
+	makeGroup(groups, k8sClient, cloud, "node-1", kopsapi.InstanceGroupRoleNode, 3, 0)
-	makeGroup(groups, k8sClient, cloud, "node-2", kopsapi.InstanceGroupRoleNode, 3)
+	makeGroup(groups, k8sClient, cloud, "node-2", kopsapi.InstanceGroupRoleNode, 3, 0)
-	makeGroup(groups, k8sClient, cloud, "master-1", kopsapi.InstanceGroupRoleMaster, 2)
+	makeGroup(groups, k8sClient, cloud, "master-1", kopsapi.InstanceGroupRoleMaster, 2, 0)
-	makeGroup(groups, k8sClient, cloud, "bastion-1", kopsapi.InstanceGroupRoleBastion, 1)
+	makeGroup(groups, k8sClient, cloud, "bastion-1", kopsapi.InstanceGroupRoleBastion, 1, 0)
 	return groups
 }
 
-func markNeedUpdate(group *cloudinstances.CloudInstanceGroup, nodeIds ...string) {
+func getGroupsAllNeedUpdate(k8sClient kubernetes.Interface, cloud awsup.AWSCloud) map[string]*cloudinstances.CloudInstanceGroup {
-	for _, nodeId := range nodeIds {
+	groups := make(map[string]*cloudinstances.CloudInstanceGroup)
-		var newReady []*cloudinstances.CloudInstanceGroupMember
+	makeGroup(groups, k8sClient, cloud, "node-1", kopsapi.InstanceGroupRoleNode, 3, 3)
-		found := false
+	makeGroup(groups, k8sClient, cloud, "node-2", kopsapi.InstanceGroupRoleNode, 3, 3)
-		for _, member := range group.Ready {
+	makeGroup(groups, k8sClient, cloud, "master-1", kopsapi.InstanceGroupRoleMaster, 2, 2)
-			if member.ID == nodeId {
+	makeGroup(groups, k8sClient, cloud, "bastion-1", kopsapi.InstanceGroupRoleBastion, 1, 1)
-				group.NeedUpdate = append(group.NeedUpdate, member)
+	return groups
-				found = true
-			} else {
-				newReady = append(newReady, member)
-			}
-		}
-		group.Ready = newReady
-		if !found {
-			panic(fmt.Sprintf("didn't find nodeId %s in ready list", nodeId))
-		}
-	}
-}
-
-func markAllNeedUpdate(groups map[string]*cloudinstances.CloudInstanceGroup) {
-	markNeedUpdate(groups["node-1"], "node-1a", "node-1b", "node-1c")
-	markNeedUpdate(groups["node-2"], "node-2a", "node-2b", "node-2c")
-	markNeedUpdate(groups["master-1"], "master-1a", "master-1b")
-	markNeedUpdate(groups["bastion-1"], "bastion-1a")
 }
 
 func TestRollingUpdateAllNeedUpdate(t *testing.T) {
-	c, cloud, cluster, groups := getTestSetup()
+	c, cloud, cluster := getTestSetup()
 
-	markAllNeedUpdate(groups)
+	groups := getGroupsAllNeedUpdate(c.K8sClient, cloud)
 	err := c.RollingUpdate(groups, cluster, &kopsapi.InstanceGroupList{})
 	assert.NoError(t, err, "rolling update")
 
@@ -227,12 +215,12 @@ func TestRollingUpdateAllNeedUpdate(t *testing.T) {
 }
 
 func TestRollingUpdateAllNeedUpdateCloudonly(t *testing.T) {
-	c, cloud, cluster, groups := getTestSetup()
+	c, cloud, cluster := getTestSetup()
 
 	c.CloudOnly = true
 	c.ClusterValidator = &assertNotCalledClusterValidator{T: t}
 
-	markAllNeedUpdate(groups)
+	groups := getGroupsAllNeedUpdate(c.K8sClient, cloud)
 	err := c.RollingUpdate(groups, cluster, &kopsapi.InstanceGroupList{})
 	assert.NoError(t, err, "rolling update")
 
@@ -245,12 +233,12 @@ func TestRollingUpdateAllNeedUpdateCloudonly(t *testing.T) {
 }
 
 func TestRollingUpdateAllNeedUpdateNoFailOnValidate(t *testing.T) {
-	c, cloud, cluster, groups := getTestSetup()
+	c, cloud, cluster := getTestSetup()
 
 	c.FailOnValidate = false
 	c.ClusterValidator = &failingClusterValidator{}
 
-	markAllNeedUpdate(groups)
+	groups := getGroupsAllNeedUpdate(c.K8sClient, cloud)
 	err := c.RollingUpdate(groups, cluster, &kopsapi.InstanceGroupList{})
 	assert.NoError(t, err, "rolling update")
 
@@ -261,7 +249,8 @@ func TestRollingUpdateAllNeedUpdateNoFailOnValidate(t *testing.T) {
 }
 
 func TestRollingUpdateNoneNeedUpdate(t *testing.T) {
-	c, cloud, cluster, groups := getTestSetup()
+	c, cloud, cluster := getTestSetup()
+	groups := getGroups(c.K8sClient, cloud)
 
 	err := c.RollingUpdate(groups, cluster, &kopsapi.InstanceGroupList{})
 	assert.NoError(t, err, "rolling update")
@@ -275,7 +264,8 @@ func TestRollingUpdateNoneNeedUpdate(t *testing.T) {
 }
 
 func TestRollingUpdateNoneNeedUpdateWithForce(t *testing.T) {
-	c, cloud, cluster, groups := getTestSetup()
+	c, cloud, cluster := getTestSetup()
+	groups := getGroups(c.K8sClient, cloud)
 
 	c.Force = true
 
@@ -289,7 +279,7 @@ func TestRollingUpdateNoneNeedUpdateWithForce(t *testing.T) {
 }
 
 func TestRollingUpdateEmptyGroup(t *testing.T) {
-	c, cloud, _, _ := getTestSetup()
+	c, cloud, _ := getTestSetup()
 
 	groups := make(map[string]*cloudinstances.CloudInstanceGroup)
 
@@ -303,7 +293,8 @@ func TestRollingUpdateEmptyGroup(t *testing.T) {
 }
 
 func TestRollingUpdateUnknownRole(t *testing.T) {
-	c, cloud, cluster, groups := getTestSetup()
+	c, cloud, cluster := getTestSetup()
+	groups := getGroups(c.K8sClient, cloud)
 
 	groups["node-1"].InstanceGroup.Spec.Role = "Unknown"
 
@@ -317,11 +308,11 @@ func TestRollingUpdateUnknownRole(t *testing.T) {
 }
 
 func TestRollingUpdateAllNeedUpdateFailsValidation(t *testing.T) {
-	c, cloud, cluster, groups := getTestSetup()
+	c, cloud, cluster := getTestSetup()
 
 	c.ClusterValidator = &failingClusterValidator{}
 
-	markAllNeedUpdate(groups)
+	groups := getGroupsAllNeedUpdate(c.K8sClient, cloud)
 	err := c.RollingUpdate(groups, cluster, &kopsapi.InstanceGroupList{})
 	assert.Error(t, err, "rolling update")
 
@@ -332,11 +323,11 @@ func TestRollingUpdateAllNeedUpdateFailsValidation(t *testing.T) {
 }
 
 func TestRollingUpdateAllNeedUpdateErrorsValidation(t *testing.T) {
-	c, cloud, cluster, groups := getTestSetup()
+	c, cloud, cluster := getTestSetup()
 
 	c.ClusterValidator = &erroringClusterValidator{}
 
-	markAllNeedUpdate(groups)
+	groups := getGroupsAllNeedUpdate(c.K8sClient, cloud)
 	err := c.RollingUpdate(groups, cluster, &kopsapi.InstanceGroupList{})
 	assert.Error(t, err, "rolling update")
 
@@ -347,11 +338,12 @@ func TestRollingUpdateAllNeedUpdateErrorsValidation(t *testing.T) {
 }
 
 func TestRollingUpdateNodes1NeedsUpdateFailsValidation(t *testing.T) {
-	c, cloud, cluster, groups := getTestSetup()
+	c, cloud, cluster := getTestSetup()
 
 	c.ClusterValidator = &failingClusterValidator{}
 
-	markNeedUpdate(groups["node-1"], "node-1a", "node-1b", "node-1c")
+	groups := make(map[string]*cloudinstances.CloudInstanceGroup)
+	makeGroup(groups, c.K8sClient, cloud, "node-1", kopsapi.InstanceGroupRoleNode, 3, 3)
 	err := c.RollingUpdate(groups, cluster, &kopsapi.InstanceGroupList{})
 	assert.Error(t, err, "rolling update")
 
@@ -359,11 +351,12 @@ func TestRollingUpdateNodes1NeedsUpdateFailsValidation(t *testing.T) {
 }
 
 func TestRollingUpdateNodes1NeedsUpdateErrorsValidation(t *testing.T) {
-	c, cloud, cluster, groups := getTestSetup()
+	c, cloud, cluster := getTestSetup()
 
 	c.ClusterValidator = &erroringClusterValidator{}
 
-	markNeedUpdate(groups["node-1"], "node-1a", "node-1b", "node-1c")
+	groups := make(map[string]*cloudinstances.CloudInstanceGroup)
+	makeGroup(groups, c.K8sClient, cloud, "node-1", kopsapi.InstanceGroupRoleNode, 3, 3)
 	err := c.RollingUpdate(groups, cluster, &kopsapi.InstanceGroupList{})
 	assert.Error(t, err, "rolling update")
 
@@ -400,7 +393,7 @@ func (v *failAfterOneNodeClusterValidator) Validate() (*validation.ValidationClu
 }
 
 func TestRollingUpdateClusterFailsValidationAfterOneMaster(t *testing.T) {
-	c, cloud, cluster, groups := getTestSetup()
+	c, cloud, cluster := getTestSetup()
 
 	c.ClusterValidator = &failAfterOneNodeClusterValidator{
 		Cloud:       cloud,
@@ -408,7 +401,7 @@ func TestRollingUpdateClusterFailsValidationAfterOneMaster(t *testing.T) {
 		ReturnError: false,
 	}
 
-	markAllNeedUpdate(groups)
+	groups := getGroupsAllNeedUpdate(c.K8sClient, cloud)
 	err := c.RollingUpdate(groups, cluster, &kopsapi.InstanceGroupList{})
 	assert.Error(t, err, "rolling update")
 
@@ -419,7 +412,7 @@ func TestRollingUpdateClusterFailsValidationAfterOneMaster(t *testing.T) {
 }
 
 func TestRollingUpdateClusterErrorsValidationAfterOneMaster(t *testing.T) {
-	c, cloud, cluster, groups := getTestSetup()
+	c, cloud, cluster := getTestSetup()
 
 	c.ClusterValidator = &failAfterOneNodeClusterValidator{
 		Cloud:       cloud,
@@ -427,7 +420,7 @@ func TestRollingUpdateClusterErrorsValidationAfterOneMaster(t *testing.T) {
 		ReturnError: true,
 	}
 
-	markAllNeedUpdate(groups)
+	groups := getGroupsAllNeedUpdate(c.K8sClient, cloud)
 	err := c.RollingUpdate(groups, cluster, &kopsapi.InstanceGroupList{})
 	assert.Error(t, err, "rolling update")
 
@@ -438,7 +431,7 @@ func TestRollingUpdateClusterErrorsValidationAfterOneMaster(t *testing.T) {
 }
 
 func TestRollingUpdateClusterFailsValidationAfterOneNode(t *testing.T) {
-	c, cloud, cluster, groups := getTestSetup()
+	c, cloud, cluster := getTestSetup()
 
 	c.ClusterValidator = &failAfterOneNodeClusterValidator{
 		Cloud:       cloud,
@@ -446,7 +439,8 @@ func TestRollingUpdateClusterFailsValidationAfterOneNode(t *testing.T) {
 		ReturnError: false,
 	}
 
-	markNeedUpdate(groups["node-1"], "node-1a", "node-1b", "node-1c")
+	groups := make(map[string]*cloudinstances.CloudInstanceGroup)
+	makeGroup(groups, c.K8sClient, cloud, "node-1", kopsapi.InstanceGroupRoleNode, 3, 3)
 	err := c.RollingUpdate(groups, cluster, &kopsapi.InstanceGroupList{})
 	assert.Error(t, err, "rolling update")
 
@@ -454,7 +448,7 @@ func TestRollingUpdateClusterFailsValidationAfterOneNode(t *testing.T) {
 }
 
 func TestRollingUpdateClusterErrorsValidationAfterOneNode(t *testing.T) {
-	c, cloud, cluster, groups := getTestSetup()
+	c, cloud, cluster := getTestSetup()
 
 	c.ClusterValidator = &failAfterOneNodeClusterValidator{
 		Cloud:       cloud,
@@ -462,7 +456,8 @@ func TestRollingUpdateClusterErrorsValidationAfterOneNode(t *testing.T) {
 		ReturnError: true,
 	}
 
-	markNeedUpdate(groups["node-1"], "node-1a", "node-1b", "node-1c")
+	groups := make(map[string]*cloudinstances.CloudInstanceGroup)
+	makeGroup(groups, c.K8sClient, cloud, "node-1", kopsapi.InstanceGroupRoleNode, 3, 3)
 	err := c.RollingUpdate(groups, cluster, &kopsapi.InstanceGroupList{})
 	assert.Error(t, err, "rolling update")
 
@@ -505,7 +500,7 @@ func (v *flappingClusterValidator) Validate() (*validation.ValidationCluster, er
 }
 
 func TestRollingUpdateFlappingValidation(t *testing.T) {
-	c, cloud, cluster, groups := getTestSetup()
+	c, cloud, cluster := getTestSetup()
 
 	// This should only take a few milliseconds,
 	// but we have to pad to allow for random delays (e.g. GC)
@@ -517,7 +512,7 @@ func TestRollingUpdateFlappingValidation(t *testing.T) {
 		Cloud: cloud,
 	}
 
-	markAllNeedUpdate(groups)
+	groups := getGroupsAllNeedUpdate(c.K8sClient, cloud)
 	err := c.RollingUpdate(groups, cluster, &kopsapi.InstanceGroupList{})
 	assert.NoError(t, err, "rolling update")
 
@@ -548,7 +543,7 @@ func (v *failThreeTimesClusterValidator) Validate() (*validation.ValidationClust
 }
 
 func TestRollingUpdateValidatesAfterBastion(t *testing.T) {
-	c, cloud, cluster, groups := getTestSetup()
+	c, cloud, cluster := getTestSetup()
 
 	// This should only take a few milliseconds,
 	// but we have to pad to allow for random delays (e.g. GC)
@@ -557,7 +552,7 @@ func TestRollingUpdateValidatesAfterBastion(t *testing.T) {
 
 	c.ClusterValidator = &failThreeTimesClusterValidator{}
 
-	markAllNeedUpdate(groups)
+	groups := getGroupsAllNeedUpdate(c.K8sClient, cloud)
 	err := c.RollingUpdate(groups, cluster, &kopsapi.InstanceGroupList{})
 	assert.NoError(t, err, "rolling update")
 
@@ -568,9 +563,10 @@ func TestRollingUpdateValidatesAfterBastion(t *testing.T) {
 }
 
 func TestRollingUpdateTaintAllButOneNeedUpdate(t *testing.T) {
-	c, cloud, cluster, groups := getTestSetup()
+	c, cloud, cluster := getTestSetup()
 
-	markNeedUpdate(groups["node-1"], "node-1a", "node-1b")
+	groups := make(map[string]*cloudinstances.CloudInstanceGroup)
+	makeGroup(groups, c.K8sClient, cloud, "node-1", kopsapi.InstanceGroupRoleNode, 3, 2)
 	err := c.RollingUpdate(groups, cluster, &kopsapi.InstanceGroupList{})
 	assert.NoError(t, err, "rolling update")