diff --git a/pkg/kubelet/types/BUILD b/pkg/kubelet/types/BUILD index 437153301de..2f4e1abe191 100644 --- a/pkg/kubelet/types/BUILD +++ b/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", ], ) diff --git a/pkg/kubelet/types/pod_update_test.go b/pkg/kubelet/types/pod_update_test.go index faf27a38439..529f32e81b6 100644 --- a/pkg/kubelet/types/pod_update_test.go +++ b/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" ) +var ( + systemPriority = int32(scheduling.SystemCriticalPriority) + systemPriorityUpper = systemPriority + 1000 +) + +// getTestPod generates a new instance of an empty test Pod +func getTestPod(annotations map[string]string, podPriority *int32) *v1.Pod { + pod := v1.Pod{ + TypeMeta: metav1.TypeMeta{ + Kind: "Pod", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "default", + }, + } + // Set pod Priority in Spec if exists + if podPriority != nil { + 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) { - // Empty. - sources, err := GetValidatedSources([]string{""}) - require.NoError(t, err) - require.Len(t, sources, 0) + 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, + }, + } - // Success. - sources, err = GetValidatedSources([]string{FileSource, ApiserverSource}) - require.NoError(t, err) - require.Len(t, sources, 2) - - // All. - sources, err = GetValidatedSources([]string{AllSource}) - require.NoError(t, err) - require.Len(t, sources, 3) - - // Unknown source. - _, err = GetValidatedSources([]string{"taco"}) - require.Error(t, err) + 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.Len(t, sources, test.sourcesLen) + }) + } } func TestGetPodSource(t *testing.T) { - cases := []struct { - pod v1.Pod + tests := []struct { + name string + pod *v1.Pod expected string errExpected bool }{ { - pod: v1.Pod{}, + name: "cannot get pod source", + pod: getTestPod(nil, nil), expected: "", errExpected: true, }, { - pod: v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - "kubernetes.io/config.source": "host-ipc-sources", - }, - }, - }, + name: "valid annotation returns the source", + pod: getTestPod(configSourceAnnotation("host-ipc-sources"), nil), expected: "host-ipc-sources", errExpected: false, }, } - for i, data := range cases { - source, err := GetPodSource(&data.pod) - if data.errExpected { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - assert.Equal(t, data.expected, source, "test[%d]", i) - t.Logf("Test case [%d]", i) + + 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 { - syncPodString := data.sp.String() - assert.Equal(t, data.expected, syncPodString, "test[%d]", i) - t.Logf("Test case [%d]", i) + for _, test := range tests { + t.Run(test.expected, func(t *testing.T) { + syncPodString := test.sp.String() + 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 - expected bool + priority int32 + name string + expected bool }{ { - priority: int32(2000000001), - description: "A system critical pod", - expected: true, + name: "a system critical pod", + priority: systemPriority, + expected: true, }, { - priority: int32(1000000000), - description: "A non system critical pod", - expected: false, + name: "a non system critical pod", + priority: scheduling.HighestUserDefinablePriority, + expected: false, }, } for _, test := range tests { - 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.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.name, actual) + } + }) } }