From 23d48f01d64ecf8e6db0fec27dd4df97395bcd67 Mon Sep 17 00:00:00 2001
From: John Gardiner Myers <jgmyers@proofpoint.com>
Date: Mon, 11 May 2020 20:55:38 -0700
Subject: [PATCH] Return cluster validation failure if ASG missing

---
 pkg/validation/validate_cluster.go      | 17 +++++-
 pkg/validation/validate_cluster_test.go | 72 ++++++++++++++++++++-----
 2 files changed, 74 insertions(+), 15 deletions(-)

diff --git a/pkg/validation/validate_cluster.go b/pkg/validation/validate_cluster.go
index 58ac90df61622..3a190be7566b0 100644
--- a/pkg/validation/validate_cluster.go
+++ b/pkg/validation/validate_cluster.go
@@ -166,7 +166,7 @@ func (v *clusterValidatorImpl) Validate() (*ValidationCluster, error) {
 	if err != nil {
 		return nil, err
 	}
-	readyNodes := validation.validateNodes(cloudGroups)
+	readyNodes := validation.validateNodes(cloudGroups, v.instanceGroups)
 
 	if err := validation.collectComponentFailures(ctx, v.k8sClient); err != nil {
 		return nil, fmt.Errorf("cannot get component status for %q: %v", clusterName, err)
@@ -286,13 +286,16 @@ func (v *ValidationCluster) collectPodFailures(ctx context.Context, client kuber
 	return nil
 }
 
-func (v *ValidationCluster) validateNodes(cloudGroups map[string]*cloudinstances.CloudInstanceGroup) []v1.Node {
+func (v *ValidationCluster) validateNodes(cloudGroups map[string]*cloudinstances.CloudInstanceGroup, groups []*kops.InstanceGroup) []v1.Node {
 	var readyNodes []v1.Node
+	groupsSeen := map[string]bool{}
+
 	for _, cloudGroup := range cloudGroups {
 		var allMembers []*cloudinstances.CloudInstanceGroupMember
 		allMembers = append(allMembers, cloudGroup.Ready...)
 		allMembers = append(allMembers, cloudGroup.NeedUpdate...)
 
+		groupsSeen[cloudGroup.InstanceGroup.Name] = true
 		numNodes := 0
 		for _, m := range allMembers {
 			if !m.Detached {
@@ -374,5 +377,15 @@ func (v *ValidationCluster) validateNodes(cloudGroups map[string]*cloudinstances
 		}
 	}
 
+	for _, ig := range groups {
+		if !groupsSeen[ig.Name] {
+			v.addError(&ValidationError{
+				Kind:    "InstanceGroup",
+				Name:    ig.Name,
+				Message: fmt.Sprintf("InstanceGroup %q is missing from the cloud provider", ig.Name),
+			})
+		}
+	}
+
 	return readyNodes
 }
diff --git a/pkg/validation/validate_cluster_test.go b/pkg/validation/validate_cluster_test.go
index 8829148876782..66ea64c753a49 100644
--- a/pkg/validation/validate_cluster_test.go
+++ b/pkg/validation/validate_cluster_test.go
@@ -73,6 +73,34 @@ func testValidate(t *testing.T, groups map[string]*cloudinstances.CloudInstanceG
 		ObjectMeta: metav1.ObjectMeta{Name: "testcluster.k8s.local"},
 	}
 
+	if len(groups) == 0 {
+		groups = make(map[string]*cloudinstances.CloudInstanceGroup)
+		groups["master-1"] = &cloudinstances.CloudInstanceGroup{
+			InstanceGroup: &kopsapi.InstanceGroup{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "master-1",
+				},
+				Spec: kopsapi.InstanceGroupSpec{
+					Role: kopsapi.InstanceGroupRoleMaster,
+				},
+			},
+			MinSize: 1,
+			Ready: []*cloudinstances.CloudInstanceGroupMember{
+				{
+					ID: "i-00001",
+					Node: &v1.Node{
+						ObjectMeta: metav1.ObjectMeta{Name: "master-1a"},
+						Status: v1.NodeStatus{
+							Conditions: []v1.NodeCondition{
+								{Type: "Ready", Status: v1.ConditionTrue},
+							},
+						},
+					},
+				},
+			},
+		}
+	}
+
 	instanceGroups := make([]kopsapi.InstanceGroup, 0, len(groups))
 	objects = append([]runtime.Object(nil), objects...)
 	for _, g := range groups {
@@ -91,19 +119,6 @@ func testValidate(t *testing.T, groups map[string]*cloudinstances.CloudInstanceG
 		}
 	}
 
-	if len(instanceGroups) == 0 {
-		instanceGroups = []kopsapi.InstanceGroup{
-			{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "master-1",
-				},
-				Spec: kopsapi.InstanceGroupSpec{
-					Role: kopsapi.InstanceGroupRoleMaster,
-				},
-			},
-		}
-	}
-
 	mockcloud := BuildMockCloud(t, groups, cluster, instanceGroups)
 
 	validator, err := NewClusterValidator(cluster, mockcloud, &kopsapi.InstanceGroupList{Items: instanceGroups}, fake.NewSimpleClientset(objects...))
@@ -113,6 +128,37 @@ func testValidate(t *testing.T, groups map[string]*cloudinstances.CloudInstanceG
 	return validator.Validate()
 }
 
+func Test_ValidateCloudGroupMissing(t *testing.T) {
+	cluster := &kopsapi.Cluster{
+		ObjectMeta: metav1.ObjectMeta{Name: "testcluster.k8s.local"},
+	}
+	instanceGroups := []kopsapi.InstanceGroup{
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "node-1",
+			},
+			Spec: kopsapi.InstanceGroupSpec{
+				Role: kopsapi.InstanceGroupRoleNode,
+			},
+		},
+	}
+
+	mockcloud := BuildMockCloud(t, nil, cluster, instanceGroups)
+
+	validator, err := NewClusterValidator(cluster, mockcloud, &kopsapi.InstanceGroupList{Items: instanceGroups}, fake.NewSimpleClientset())
+	require.NoError(t, err)
+	v, err := validator.Validate()
+	require.NoError(t, err)
+	if !assert.Len(t, v.Failures, 1) ||
+		!assert.Equal(t, &ValidationError{
+			Kind:    "InstanceGroup",
+			Name:    "node-1",
+			Message: "InstanceGroup \"node-1\" is missing from the cloud provider",
+		}, v.Failures[0]) {
+		printDebug(t, v)
+	}
+}
+
 func Test_ValidateNodesNotEnough(t *testing.T) {
 	groups := make(map[string]*cloudinstances.CloudInstanceGroup)
 	groups["node-1"] = &cloudinstances.CloudInstanceGroup{