Adding tests for Kubelet pod update functions

pkg/kubelet/types/BUILD

@@ -35,10 +35,10 @@ go_test(
     ],
     embed = [":go_default_library"],
     deps = [
+        "//pkg/apis/scheduling:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/github.com/stretchr/testify/assert:go_default_library",
-        "//vendor/github.com/stretchr/testify/require:go_default_library",
     ],
 )
 

pkg/kubelet/types/pod_update_test.go

@@ -20,69 +20,131 @@ import (
 	"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/kubernetes/pkg/apis/scheduling"
 )
 
-func TestGetValidatedSources(t *testing.T) {
+var (
-	// Empty.
+	systemPriority      = int32(scheduling.SystemCriticalPriority)
-	sources, err := GetValidatedSources([]string{""})
+	systemPriorityUpper = systemPriority + 1000
-	require.NoError(t, err)
+)
-	require.Len(t, sources, 0)
 
-	// Success.
+// getTestPod generates a new instance of an empty test Pod
-	sources, err = GetValidatedSources([]string{FileSource, ApiserverSource})
+func getTestPod(annotations map[string]string, podPriority *int32) *v1.Pod {
-	require.NoError(t, err)
+	pod := v1.Pod{
-	require.Len(t, sources, 2)
+		TypeMeta: metav1.TypeMeta{
-
+			Kind:       "Pod",
-	// All.
+			APIVersion: "v1",
-	sources, err = GetValidatedSources([]string{AllSource})
-	require.NoError(t, err)
-	require.Len(t, sources, 3)
-
-	// Unknown source.
-	_, err = GetValidatedSources([]string{"taco"})
-	require.Error(t, err)
-}
-
-func TestGetPodSource(t *testing.T) {
-	cases := []struct {
-		pod         v1.Pod
-		expected    string
-		errExpected bool
-	}{
-		{
-			pod:         v1.Pod{},
-			expected:    "",
-			errExpected: true,
 		},
-		{
-			pod: v1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
-					Annotations: map[string]string{
+			Name:      "foo",
-						"kubernetes.io/config.source": "host-ipc-sources",
+			Namespace: "default",
-					},
-				},
-			},
-			expected:    "host-ipc-sources",
-			errExpected: false,
 		},
 	}
-	for i, data := range cases {
+	// Set pod Priority in Spec if exists
-		source, err := GetPodSource(&data.pod)
+	if podPriority != nil {
-		if data.errExpected {
+		pod.Spec = v1.PodSpec{
+			Priority: podPriority,
+		}
+	}
+	// Set annotations if exists
+	if annotations != nil {
+		pod.Annotations = annotations
+	}
+	return &pod
+}
+
+func configSourceAnnotation(source string) map[string]string {
+	return map[string]string{ConfigSourceAnnotationKey: source}
+}
+
+func configMirrorAnnotation() map[string]string {
+	return map[string]string{ConfigMirrorAnnotationKey: "true"}
+}
+
+func TestGetValidatedSources(t *testing.T) {
+	tests := []struct {
+		name        string
+		sources     []string
+		errExpected bool
+		sourcesLen  int
+	}{
+		{
+			name:        "empty source",
+			sources:     []string{""},
+			errExpected: false,
+			sourcesLen:  0,
+		},
+		{
+			name:        "file and apiserver source",
+			sources:     []string{FileSource, ApiserverSource},
+			errExpected: false,
+			sourcesLen:  2,
+		},
+		{
+			name:        "all source",
+			sources:     []string{AllSource},
+			errExpected: false,
+			sourcesLen:  3,
+		},
+		{
+			name:        "unknown source",
+			sources:     []string{"unknown"},
+			errExpected: true,
+			sourcesLen:  0,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			sources, err := GetValidatedSources(test.sources)
+			if test.errExpected {
 				assert.Error(t, err)
 			} else {
 				assert.NoError(t, err)
 			}
-		assert.Equal(t, data.expected, source, "test[%d]", i)
+			assert.Len(t, sources, test.sourcesLen)
-		t.Logf("Test case [%d]", i)
+		})
+	}
+}
+
+func TestGetPodSource(t *testing.T) {
+	tests := []struct {
+		name        string
+		pod         *v1.Pod
+		expected    string
+		errExpected bool
+	}{
+		{
+			name:        "cannot get pod source",
+			pod:         getTestPod(nil, nil),
+			expected:    "",
+			errExpected: true,
+		},
+		{
+			name:        "valid annotation returns the source",
+			pod:         getTestPod(configSourceAnnotation("host-ipc-sources"), nil),
+			expected:    "host-ipc-sources",
+			errExpected: false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			source, err := GetPodSource(test.pod)
+			if test.errExpected {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+			assert.Equal(t, test.expected, source)
+		})
 	}
 }
 
 func TestString(t *testing.T) {
-	cases := []struct {
+	tests := []struct {
 		sp       SyncPodType
 		expected string
 	}{
@@ -107,34 +169,190 @@ func TestString(t *testing.T) {
 			expected: "unknown",
 		},
 	}
-	for i, data := range cases {
+	for _, test := range tests {
-		syncPodString := data.sp.String()
+		t.Run(test.expected, func(t *testing.T) {
-		assert.Equal(t, data.expected, syncPodString, "test[%d]", i)
+			syncPodString := test.sp.String()
-		t.Logf("Test case [%d]", i)
+			assert.Equal(t, test.expected, syncPodString)
+		})
+	}
+}
+
+func TestIsMirrorPod(t *testing.T) {
+	tests := []struct {
+		name     string
+		pod      *v1.Pod
+		expected bool
+	}{
+		{
+			name:     "mirror pod",
+			pod:      getTestPod(configMirrorAnnotation(), nil),
+			expected: true,
+		},
+		{
+			name:     "not a mirror pod",
+			pod:      getTestPod(nil, nil),
+			expected: false,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			isMirrorPod := IsMirrorPod(test.pod)
+			assert.Equal(t, test.expected, isMirrorPod)
+		})
+	}
+}
+
+func TestIsStaticPod(t *testing.T) {
+	tests := []struct {
+		name     string
+		pod      *v1.Pod
+		expected bool
+	}{
+		{
+			name:     "static pod with file source",
+			pod:      getTestPod(configSourceAnnotation(FileSource), nil),
+			expected: true,
+		},
+		{
+			name:     "static pod with http source",
+			pod:      getTestPod(configSourceAnnotation(HTTPSource), nil),
+			expected: true,
+		},
+		{
+			name:     "static pod with api server source",
+			pod:      getTestPod(configSourceAnnotation(ApiserverSource), nil),
+			expected: false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			isStaticPod := IsStaticPod(test.pod)
+			assert.Equal(t, test.expected, isStaticPod)
+		})
+	}
+}
+
+func TestIsCriticalPod(t *testing.T) {
+	tests := []struct {
+		name     string
+		pod      *v1.Pod
+		expected bool
+	}{
+		{
+			name:     "critical pod with file source",
+			pod:      getTestPod(configSourceAnnotation(FileSource), nil),
+			expected: true,
+		},
+		{
+			name:     "critical pod with mirror annotation",
+			pod:      getTestPod(configMirrorAnnotation(), nil),
+			expected: true,
+		},
+		{
+			name:     "critical pod using system priority",
+			pod:      getTestPod(nil, &systemPriority),
+			expected: true,
+		},
+		{
+			name:     "critical pod using greater than system priority",
+			pod:      getTestPod(nil, &systemPriorityUpper),
+			expected: true,
+		},
+		{
+			name:     "not a critical pod with api server annotation",
+			pod:      getTestPod(configSourceAnnotation(ApiserverSource), nil),
+			expected: false,
+		},
+		{
+			name:     "not critical if not static, mirror or without a priority",
+			pod:      getTestPod(nil, nil),
+			expected: false,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			isCriticalPod := IsCriticalPod(test.pod)
+			assert.Equal(t, test.expected, isCriticalPod)
+		})
+	}
+}
+
+func TestPreemptable(t *testing.T) {
+	tests := []struct {
+		name      string
+		preemptor *v1.Pod
+		preemptee *v1.Pod
+		expected  bool
+	}{
+		{
+			name:      "a critical preemptor pod preempts a non critical pod",
+			preemptor: getTestPod(configSourceAnnotation(FileSource), nil),
+			preemptee: getTestPod(nil, nil),
+			expected:  true,
+		},
+		{
+			name:      "a preemptor pod with higher priority preempts a critical pod",
+			preemptor: getTestPod(configSourceAnnotation(FileSource), &systemPriorityUpper),
+			preemptee: getTestPod(configSourceAnnotation(FileSource), &systemPriority),
+			expected:  true,
+		},
+		{
+			name:      "a not critical pod with higher priority preempts a critical pod",
+			preemptor: getTestPod(configSourceAnnotation(ApiserverSource), &systemPriorityUpper),
+			preemptee: getTestPod(configSourceAnnotation(FileSource), &systemPriority),
+			expected:  true,
+		},
+		{
+			name:      "a critical pod with less priority do not preempts a critical pod",
+			preemptor: getTestPod(configSourceAnnotation(FileSource), &systemPriority),
+			preemptee: getTestPod(configSourceAnnotation(FileSource), &systemPriorityUpper),
+			expected:  false,
+		},
+		{
+			name:      "a critical pod without priority do not preempts a critical pod without priority",
+			preemptor: getTestPod(configSourceAnnotation(FileSource), nil),
+			preemptee: getTestPod(configSourceAnnotation(FileSource), nil),
+			expected:  false,
+		},
+		{
+			name:      "a critical pod with priority do not preempts a critical pod with the same priority",
+			preemptor: getTestPod(configSourceAnnotation(FileSource), &systemPriority),
+			preemptee: getTestPod(configSourceAnnotation(FileSource), &systemPriority),
+			expected:  false,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			isPreemtable := Preemptable(test.preemptor, test.preemptee)
+			assert.Equal(t, test.expected, isPreemtable)
+		})
 	}
 }
 
 func TestIsCriticalPodBasedOnPriority(t *testing.T) {
 	tests := []struct {
 		priority int32
-		description string
+		name     string
 		expected bool
 	}{
 		{
-			priority:    int32(2000000001),
+			name:     "a system critical pod",
-			description: "A system critical pod",
+			priority: systemPriority,
 			expected: true,
 		},
 		{
-			priority:    int32(1000000000),
+			name:     "a non system critical pod",
-			description: "A non system critical pod",
+			priority: scheduling.HighestUserDefinablePriority,
 			expected: false,
 		},
 	}
 	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
 			actual := IsCriticalPodBasedOnPriority(test.priority)
 			if actual != test.expected {
-			t.Errorf("IsCriticalPodBased on priority should have returned %v for test %v but got %v", test.expected, test.description, actual)
+				t.Errorf("IsCriticalPodBased on priority should have returned %v for test %v but got %v", test.expected, test.name, actual)
 			}
+		})
 	}
 }