Merge pull request #8039 from johngmyers/validation-test-public

Increase validation test coverage

pkg/validation/BUILD.bazel

@@ -29,9 +29,13 @@ go_test(
     deps = [
         "//pkg/apis/kops:go_default_library",
         "//pkg/cloudinstances:go_default_library",
+        "//upup/pkg/fi:go_default_library",
+        "//upup/pkg/fi/cloudup/awsup:go_default_library",
+        "//vendor/github.com/stretchr/testify/assert:go_default_library",
+        "//vendor/github.com/stretchr/testify/require: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/apimachinery/pkg/runtime:go_default_library",
         "//vendor/k8s.io/client-go/kubernetes/fake:go_default_library",
     ],
 )

pkg/validation/validate_cluster_test.go

@@ -17,17 +17,101 @@ limitations under the License.
 package validation
 
 import (
-	"fmt"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/client-go/kubernetes"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes/fake"
 	kopsapi "k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/cloudinstances"
+	"k8s.io/kops/upup/pkg/fi"
+	"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
 )
 
+type MockCloud struct {
+	awsup.MockAWSCloud
+	Groups map[string]*cloudinstances.CloudInstanceGroup
+
+	t                      *testing.T
+	expectedCluster        *kopsapi.Cluster
+	expectedInstanceGroups []kopsapi.InstanceGroup
+}
+
+var _ fi.Cloud = (*MockCloud)(nil)
+
+func BuildMockCloud(t *testing.T, groups map[string]*cloudinstances.CloudInstanceGroup, expectedCluster *kopsapi.Cluster, expectedInstanceGroups []kopsapi.InstanceGroup) *MockCloud {
+	m := MockCloud{
+		MockAWSCloud:           *awsup.BuildMockAWSCloud("us-east-1", "abc"),
+		Groups:                 groups,
+		t:                      t,
+		expectedCluster:        expectedCluster,
+		expectedInstanceGroups: expectedInstanceGroups,
+	}
+	return &m
+}
+
+func (c *MockCloud) GetCloudGroups(cluster *kopsapi.Cluster, instancegroups []*kopsapi.InstanceGroup, warnUnmatched bool, nodes []v1.Node) (map[string]*cloudinstances.CloudInstanceGroup, error) {
+	assert.Equal(c.t, c.expectedCluster, cluster, "cluster")
+
+	var igs = make([]kopsapi.InstanceGroup, 0, len(instancegroups))
+	for _, ig := range instancegroups {
+		igs = append(igs, *ig)
+	}
+	assert.ElementsMatch(c.t, c.expectedInstanceGroups, igs)
+
+	// TODO assert nodes contains all the nodes in the mock kubernetes.Interface?
+
+	return c.Groups, nil
+}
+
+func testValidate(t *testing.T, groups map[string]*cloudinstances.CloudInstanceGroup, objects []runtime.Object) (*ValidationCluster, error) {
+	cluster := &kopsapi.Cluster{
+		ObjectMeta: metav1.ObjectMeta{Name: "testcluster.k8s.local"},
+	}
+
+	instanceGroups := make([]kopsapi.InstanceGroup, 0, len(groups))
+	objects = append([]runtime.Object(nil), objects...)
+	for _, g := range groups {
+		instanceGroups = append(instanceGroups, *g.InstanceGroup)
+		for _, member := range g.Ready {
+			node := member.Node
+			if node != nil {
+				objects = append(objects, node)
+			}
+		}
+		for _, member := range g.NeedUpdate {
+			node := member.Node
+			if node != nil {
+				objects = append(objects, node)
+			}
+		}
+	}
+
+	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...))
+	if err != nil {
+		return nil, err
+	}
+	return validator.Validate()
+}
+
 func Test_ValidateNodesNotEnough(t *testing.T) {
 	groups := make(map[string]*cloudinstances.CloudInstanceGroup)
 	groups["node-1"] = &cloudinstances.CloudInstanceGroup{
@@ -67,40 +151,86 @@ func Test_ValidateNodesNotEnough(t *testing.T) {
 		},
 	}
 
-	{
-		v := &ValidationCluster{}
+	t.Run("too few nodes", func(t *testing.T) {
 		groups["node-1"].MinSize = 3
-		v.validateNodes(groups)
-		if len(v.Failures) != 2 {
+		v, err := testValidate(t, groups, nil)
+		require.NoError(t, err)
+		if !assert.Len(t, v.Failures, 2) {
 			printDebug(t, v)
-			t.Fatal("Too few nodes not caught")
 		}
-	}
+	})
 
-	{
+	t.Run("not ready node", func(t *testing.T) {
 		groups["node-1"].MinSize = 2
-		v := &ValidationCluster{}
-		v.validateNodes(groups)
-		if len(v.Failures) != 1 {
+		v, err := testValidate(t, groups, nil)
+		require.NoError(t, err)
+		if !assert.Len(t, v.Failures, 1) {
 			printDebug(t, v)
-			t.Fatal("Not ready node not caught")
 		}
-	}
+	})
 
-	{
+	t.Run("unexpected errors", func(t *testing.T) {
 		groups["node-1"].NeedUpdate[0].Node.Status.Conditions[0].Status = v1.ConditionTrue
-		v := &ValidationCluster{}
-		v.validateNodes(groups)
-		if len(v.Failures) != 0 {
+		v, err := testValidate(t, groups, nil)
+		require.NoError(t, err)
+		if !assert.Empty(t, v.Failures) {
 			printDebug(t, v)
-			t.Fatal("unexpected errors")
 		}
+	})
+}
+
+func Test_ValidateNoComponentFailures(t *testing.T) {
+	v, err := testValidate(t, nil, []runtime.Object{
+		&v1.ComponentStatus{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "testcomponent",
+			},
+			Conditions: []v1.ComponentCondition{
+				{
+					Status: v1.ConditionTrue,
+				},
+			},
+		},
+	})
+
+	require.NoError(t, err)
+	assert.Empty(t, v.Failures)
+}
+
+func Test_ValidateComponentFailure(t *testing.T) {
+	for _, status := range []v1.ConditionStatus{
+		v1.ConditionFalse,
+		v1.ConditionUnknown,
+	} {
+		t.Run(string(status), func(t *testing.T) {
+			v, err := testValidate(t, nil, []runtime.Object{
+				&v1.ComponentStatus{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "testcomponent",
+					},
+					Conditions: []v1.ComponentCondition{
+						{
+							Status: status,
+						},
+					},
+				},
+			})
+
+			require.NoError(t, err)
+			if !assert.Len(t, v.Failures, 1) ||
+				!assert.Equal(t, &ValidationError{
+					Kind:    "ComponentStatus",
+					Name:    "testcomponent",
+					Message: "component \"testcomponent\" is unhealthy",
+				}, v.Failures[0]) {
+				printDebug(t, v)
+			}
+		})
 	}
 }
 
 func Test_ValidateNoPodFailures(t *testing.T) {
-	v := &ValidationCluster{}
-	err := v.collectPodFailures(dummyPodClient(
+	v, err := testValidate(t, nil, makePodList(
 		[]map[string]string{
 			{
 				"name":  "pod1",
@@ -115,35 +245,52 @@ func Test_ValidateNoPodFailures(t *testing.T) {
 		},
 	))
 
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
-
-	if len(v.Failures) != 0 {
-		fmt.Printf("failures: %+v\n", v.Failures)
-		t.Fatal("no failures expected")
-	}
+	require.NoError(t, err)
+	assert.Empty(t, v.Failures)
 }
 
 func Test_ValidatePodFailure(t *testing.T) {
-	v := &ValidationCluster{}
-	err := v.collectPodFailures(dummyPodClient(
-		[]map[string]string{
-			{
-				"name":  "pod1",
-				"ready": "false",
-				"phase": string(v1.PodRunning),
+	for _, tc := range []struct {
+		name     string
+		phase    v1.PodPhase
+		expected ValidationError
+	}{
+		{
+			name:  "pending",
+			phase: v1.PodPending,
+			expected: ValidationError{
+				Kind:    "Pod",
+				Name:    "kube-system/pod1",
+				Message: "kube-system pod \"pod1\" is pending",
 			},
 		},
-	))
+		{
+			name:  "notready",
+			phase: v1.PodRunning,
+			expected: ValidationError{
+				Kind:    "Pod",
+				Name:    "kube-system/pod1",
+				Message: "kube-system pod \"pod1\" is not ready (container1,container2)",
+			},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			v, err := testValidate(t, nil, makePodList(
+				[]map[string]string{
+					{
+						"name":  "pod1",
+						"ready": "false",
+						"phase": string(tc.phase),
+					},
+				},
+			))
 
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
-
-	if len(v.Failures) != 1 || v.Failures[0].Name != "kube-system/pod1" {
-		printDebug(t, v)
-		t.Fatal("pod1 failure expected")
+			require.NoError(t, err)
+			if !assert.Len(t, v.Failures, 1) ||
+				!assert.Equal(t, &tc.expected, v.Failures[0]) {
+				printDebug(t, v)
+			}
+		})
 	}
 }
 
@@ -154,10 +301,6 @@ func printDebug(t *testing.T, v *ValidationCluster) {
 	}
 }
 
-func dummyPodClient(pods []map[string]string) kubernetes.Interface {
-	return fake.NewSimpleClientset(makePodList(pods))
-}
-
 func dummyPod(podMap map[string]string) v1.Pod {
 	return v1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
@@ -169,6 +312,11 @@ func dummyPod(podMap map[string]string) v1.Pod {
 			Phase: v1.PodPhase(podMap["phase"]),
 			ContainerStatuses: []v1.ContainerStatus{
 				{
+					Name:  "container1",
+					Ready: podMap["ready"] == "true",
+				},
+				{
+					Name:  "container2",
 					Ready: podMap["ready"] == "true",
 				},
 			},
@@ -177,12 +325,13 @@ func dummyPod(podMap map[string]string) v1.Pod {
 }
 
 // MakePodList constructs api.PodList from a list of pod attributes
-func makePodList(pods []map[string]string) *v1.PodList {
-	var list v1.PodList
+func makePodList(pods []map[string]string) []runtime.Object {
+	var list []runtime.Object
 	for _, pod := range pods {
-		list.Items = append(list.Items, dummyPod(pod))
+		p := dummyPod(pod)
+		list = append(list, &p)
 	}
-	return &list
+	return list
 }
 
 func Test_ValidateBastionNodes(t *testing.T) {
@@ -192,9 +341,6 @@ func Test_ValidateBastionNodes(t *testing.T) {
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "ig1",
 			},
-			Spec: kopsapi.InstanceGroupSpec{
-				Role: kopsapi.InstanceGroupRoleNode,
-			},
 		},
 		Ready: []*cloudinstances.CloudInstanceGroupMember{
 			{
@@ -205,28 +351,25 @@ func Test_ValidateBastionNodes(t *testing.T) {
 	}
 
 	// When an instancegroup's nodes are not ready, that is an error
-	{
-		v := &ValidationCluster{}
+	t.Run("instancegroup's nodes not ready", func(t *testing.T) {
 		groups["ig1"].InstanceGroup.Spec.Role = kopsapi.InstanceGroupRoleNode
-		v.validateNodes(groups)
-		if len(v.Failures) != 1 {
+		v, err := testValidate(t, groups, nil)
+		require.NoError(t, err)
+		if !assert.Len(t, v.Failures, 1) {
 			printDebug(t, v)
-			t.Fatal("Nodes are expected to join cluster")
-		} else if v.Failures[0].Message != "machine \"i-00001\" has not yet joined cluster" {
+		} else if !assert.Equal(t, "machine \"i-00001\" has not yet joined cluster", v.Failures[0].Message) {
 			printDebug(t, v)
-			t.Fatalf("unexpected validation failure: %+v", v.Failures[0])
 		}
-	}
+	})
 
 	// Except for a bastion instancegroup - those are not expected to join as nodes
-	{
-		v := &ValidationCluster{}
+	t.Run("bastion instancegroup nodes not ready", func(t *testing.T) {
 		groups["ig1"].InstanceGroup.Spec.Role = kopsapi.InstanceGroupRoleBastion
-		v.validateNodes(groups)
-		if len(v.Failures) != 0 {
+		v, err := testValidate(t, groups, nil)
+		require.NoError(t, err)
+		if !assert.Empty(t, v.Failures, "Bastion nodes are not expected to join cluster") {
 			printDebug(t, v)
-			t.Fatal("Bastion nodes are not expected to join cluster")
 		}
-	}
+	})
 
 }

vendor/github.com/stretchr/testify/require/BUILD.bazel

@@ -0,0 +1,16 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+    name = "go_default_library",
+    srcs = [
+        "doc.go",
+        "forward_requirements.go",
+        "require.go",
+        "require_forward.go",
+        "requirements.go",
+    ],
+    importmap = "k8s.io/kops/vendor/github.com/stretchr/testify/require",
+    importpath = "github.com/stretchr/testify/require",
+    visibility = ["//visibility:public"],
+    deps = ["//vendor/github.com/stretchr/testify/assert:go_default_library"],
+)

vendor/github.com/stretchr/testify/require/doc.go

@@ -0,0 +1,28 @@
+// Package require implements the same assertions as the `assert` package but
+// stops test execution when a test fails.
+//
+// Example Usage
+//
+// The following is a complete example using require in a standard test function:
+//    import (
+//      "testing"
+//      "github.com/stretchr/testify/require"
+//    )
+//
+//    func TestSomething(t *testing.T) {
+//
+//      var a string = "Hello"
+//      var b string = "Hello"
+//
+//      require.Equal(t, a, b, "The two words should be the same.")
+//
+//    }
+//
+// Assertions
+//
+// The `require` package have same global functions as in the `assert` package,
+// but instead of returning a boolean result they call `t.FailNow()`.
+//
+// Every assertion function also takes an optional string message as the final argument,
+// allowing custom error messages to be appended to the message the assertion method outputs.
+package require

vendor/github.com/stretchr/testify/require/forward_requirements.go

@@ -0,0 +1,16 @@
+package require
+
+// Assertions provides assertion methods around the
+// TestingT interface.
+type Assertions struct {
+	t TestingT
+}
+
+// New makes a new Assertions object for the specified TestingT.
+func New(t TestingT) *Assertions {
+	return &Assertions{
+		t: t,
+	}
+}
+
+//go:generate go run ../_codegen/main.go -output-package=require -template=require_forward.go.tmpl -include-format-funcs

vendor/github.com/stretchr/testify/require/require.go.tmpl

@@ -0,0 +1,6 @@
+{{.Comment}}
+func {{.DocInfo.Name}}(t TestingT, {{.Params}}) {
+	if h, ok := t.(tHelper); ok { h.Helper() }
+	if assert.{{.DocInfo.Name}}(t, {{.ForwardedParams}}) { return }
+	t.FailNow()
+}

vendor/github.com/stretchr/testify/require/require_forward.go.tmpl

@@ -0,0 +1,5 @@
+{{.CommentWithoutT "a"}}
+func (a *Assertions) {{.DocInfo.Name}}({{.Params}}) {
+	if h, ok := a.t.(tHelper); ok { h.Helper() }
+	{{.DocInfo.Name}}(a.t, {{.ForwardedParams}})
+}

vendor/github.com/stretchr/testify/require/requirements.go

@@ -0,0 +1,29 @@
+package require
+
+// TestingT is an interface wrapper around *testing.T
+type TestingT interface {
+	Errorf(format string, args ...interface{})
+	FailNow()
+}
+
+type tHelper interface {
+	Helper()
+}
+
+// ComparisonAssertionFunc is a common function prototype when comparing two values.  Can be useful
+// for table driven tests.
+type ComparisonAssertionFunc func(TestingT, interface{}, interface{}, ...interface{})
+
+// ValueAssertionFunc is a common function prototype when validating a single value.  Can be useful
+// for table driven tests.
+type ValueAssertionFunc func(TestingT, interface{}, ...interface{})
+
+// BoolAssertionFunc is a common function prototype when validating a bool value.  Can be useful
+// for table driven tests.
+type BoolAssertionFunc func(TestingT, bool, ...interface{})
+
+// ErrorAssertionFunc is a common function prototype when validating an error value.  Can be useful
+// for table driven tests.
+type ErrorAssertionFunc func(TestingT, error, ...interface{})
+
+//go:generate go run ../_codegen/main.go -output-package=require -template=require.go.tmpl -include-format-funcs

vendor/modules.txt

@@ -424,6 +424,7 @@ github.com/spotinst/spotinst-sdk-go/spotinst/util/uritemplates
 github.com/spotinst/spotinst-sdk-go/service/ocean/providers/gcp
 # github.com/stretchr/testify v1.4.0
 github.com/stretchr/testify/assert
+github.com/stretchr/testify/require
 # github.com/urfave/cli v1.20.0
 github.com/urfave/cli
 # github.com/vmware/govmomi v0.20.1