From 6a9ef7f04ff5779374171f987447bcf4fba43a5b Mon Sep 17 00:00:00 2001 From: Harsh Singh Date: Tue, 17 Sep 2019 23:49:56 +0530 Subject: [PATCH] Move GetPodPriority from /scheduler/util to /api/pod --- pkg/api/v1/pod/util.go | 13 +++++- pkg/api/v1/pod/util_test.go | 38 +++++++++++++++++- pkg/kubelet/eviction/BUILD | 2 +- pkg/kubelet/eviction/helpers.go | 8 ++-- pkg/scheduler/core/BUILD | 2 + pkg/scheduler/core/extender_test.go | 7 ++-- pkg/scheduler/core/generic_scheduler.go | 19 ++++----- pkg/scheduler/internal/queue/BUILD | 1 + .../internal/queue/scheduling_queue.go | 9 +++-- .../internal/queue/scheduling_queue_test.go | 6 +-- pkg/scheduler/util/BUILD | 4 +- pkg/scheduler/util/utils.go | 27 ++++--------- pkg/scheduler/util/utils_test.go | 40 +------------------ 13 files changed, 91 insertions(+), 85 deletions(-) diff --git a/pkg/api/v1/pod/util.go b/pkg/api/v1/pod/util.go index f240f439341..1457e37ed86 100644 --- a/pkg/api/v1/pod/util.go +++ b/pkg/api/v1/pod/util.go @@ -20,7 +20,7 @@ import ( "fmt" "time" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" utilfeature "k8s.io/apiserver/pkg/util/feature" @@ -323,3 +323,14 @@ func UpdatePodCondition(status *v1.PodStatus, condition *v1.PodCondition) bool { // Return true if one of the fields have changed. return !isEqual } + +// GetPodPriority returns priority of the given pod. +func GetPodPriority(pod *v1.Pod) int32 { + if pod.Spec.Priority != nil { + return *pod.Spec.Priority + } + // When priority of a running pod is nil, it means it was created at a time + // that there was no global default priority class and the priority class + // name of the pod was empty. So, we resolve to the static default priority. + return 0 +} diff --git a/pkg/api/v1/pod/util_test.go b/pkg/api/v1/pod/util_test.go index ddf198bc32e..d3f8ebdea62 100644 --- a/pkg/api/v1/pod/util_test.go +++ b/pkg/api/v1/pod/util_test.go @@ -23,7 +23,7 @@ import ( "time" "github.com/stretchr/testify/assert" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/sets" @@ -770,3 +770,39 @@ func TestUpdatePodCondition(t *testing.T) { assert.Equal(t, test.expected, resultStatus, test.desc) } } + +// TestGetPodPriority tests GetPodPriority function. +func TestGetPodPriority(t *testing.T) { + p := int32(20) + tests := []struct { + name string + pod *v1.Pod + expectedPriority int32 + }{ + { + name: "no priority pod resolves to static default priority", + pod: &v1.Pod{ + Spec: v1.PodSpec{Containers: []v1.Container{ + {Name: "container", Image: "image"}}, + }, + }, + expectedPriority: 0, + }, + { + name: "pod with priority resolves correctly", + pod: &v1.Pod{ + Spec: v1.PodSpec{Containers: []v1.Container{ + {Name: "container", Image: "image"}}, + Priority: &p, + }, + }, + expectedPriority: p, + }, + } + for _, test := range tests { + if GetPodPriority(test.pod) != test.expectedPriority { + t.Errorf("expected pod priority: %v, got %v", test.expectedPriority, GetPodPriority(test.pod)) + } + + } +} diff --git a/pkg/kubelet/eviction/BUILD b/pkg/kubelet/eviction/BUILD index ef281bfc6fc..7231cff9cc1 100644 --- a/pkg/kubelet/eviction/BUILD +++ b/pkg/kubelet/eviction/BUILD @@ -48,6 +48,7 @@ go_library( ], importpath = "k8s.io/kubernetes/pkg/kubelet/eviction", deps = [ + "//pkg/api/v1/pod:go_default_library", "//pkg/api/v1/resource:go_default_library", "//pkg/apis/core/v1/helper:go_default_library", "//pkg/apis/core/v1/helper/qos:go_default_library", @@ -61,7 +62,6 @@ go_library( "//pkg/kubelet/types:go_default_library", "//pkg/kubelet/util/format:go_default_library", "//pkg/scheduler/api:go_default_library", - "//pkg/scheduler/util:go_default_library", "//pkg/volume/util:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", diff --git a/pkg/kubelet/eviction/helpers.go b/pkg/kubelet/eviction/helpers.go index dfdb8ce3b60..d008f9af395 100644 --- a/pkg/kubelet/eviction/helpers.go +++ b/pkg/kubelet/eviction/helpers.go @@ -23,14 +23,14 @@ import ( "strings" "time" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/klog" + "k8s.io/kubernetes/pkg/api/v1/pod" v1resource "k8s.io/kubernetes/pkg/api/v1/resource" statsapi "k8s.io/kubernetes/pkg/kubelet/apis/stats/v1alpha1" evictionapi "k8s.io/kubernetes/pkg/kubelet/eviction/api" kubetypes "k8s.io/kubernetes/pkg/kubelet/types" - schedulerutils "k8s.io/kubernetes/pkg/scheduler/util" volumeutils "k8s.io/kubernetes/pkg/volume/util" ) @@ -512,8 +512,8 @@ func (ms *multiSorter) Less(i, j int) bool { // priority compares pods by Priority, if priority is enabled. func priority(p1, p2 *v1.Pod) int { - priority1 := schedulerutils.GetPodPriority(p1) - priority2 := schedulerutils.GetPodPriority(p2) + priority1 := pod.GetPodPriority(p1) + priority2 := pod.GetPodPriority(p2) if priority1 == priority2 { return 0 } diff --git a/pkg/scheduler/core/BUILD b/pkg/scheduler/core/BUILD index b4274a22ffe..a4d448455e2 100644 --- a/pkg/scheduler/core/BUILD +++ b/pkg/scheduler/core/BUILD @@ -9,6 +9,7 @@ go_library( importpath = "k8s.io/kubernetes/pkg/scheduler/core", visibility = ["//visibility:public"], deps = [ + "//pkg/api/v1/pod:go_default_library", "//pkg/scheduler/algorithm:go_default_library", "//pkg/scheduler/algorithm/predicates:go_default_library", "//pkg/scheduler/algorithm/priorities:go_default_library", @@ -43,6 +44,7 @@ go_test( ], embed = [":go_default_library"], deps = [ + "//pkg/api/v1/pod:go_default_library", "//pkg/scheduler/algorithm:go_default_library", "//pkg/scheduler/algorithm/predicates:go_default_library", "//pkg/scheduler/algorithm/priorities:go_default_library", diff --git a/pkg/scheduler/core/extender_test.go b/pkg/scheduler/core/extender_test.go index 84dfa5ae8d8..e4bf47c50c1 100644 --- a/pkg/scheduler/core/extender_test.go +++ b/pkg/scheduler/core/extender_test.go @@ -22,11 +22,12 @@ import ( "testing" "time" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" + podutil "k8s.io/kubernetes/pkg/api/v1/pod" "k8s.io/kubernetes/pkg/scheduler/algorithm" "k8s.io/kubernetes/pkg/scheduler/algorithm/predicates" "k8s.io/kubernetes/pkg/scheduler/algorithm/priorities" @@ -210,9 +211,9 @@ func (f *FakeExtender) selectVictimsOnNodeByExtender( } // As the first step, remove all the lower priority pods from the node and // check if the given pod can be scheduled. - podPriority := util.GetPodPriority(pod) + podPriority := podutil.GetPodPriority(pod) for _, p := range nodeInfoCopy.Pods() { - if util.GetPodPriority(p) < podPriority { + if podutil.GetPodPriority(p) < podPriority { potentialVictims.Items = append(potentialVictims.Items, p) removePod(p) } diff --git a/pkg/scheduler/core/generic_scheduler.go b/pkg/scheduler/core/generic_scheduler.go index fb24bf098a2..0de2ae5f400 100644 --- a/pkg/scheduler/core/generic_scheduler.go +++ b/pkg/scheduler/core/generic_scheduler.go @@ -36,6 +36,7 @@ import ( "k8s.io/apimachinery/pkg/util/errors" corelisters "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/util/workqueue" + podutil "k8s.io/kubernetes/pkg/api/v1/pod" "k8s.io/kubernetes/pkg/scheduler/algorithm" "k8s.io/kubernetes/pkg/scheduler/algorithm/predicates" "k8s.io/kubernetes/pkg/scheduler/algorithm/priorities" @@ -427,9 +428,9 @@ func (g *genericScheduler) getLowerPriorityNominatedPods(pod *v1.Pod, nodeName s } var lowerPriorityPods []*v1.Pod - podPriority := util.GetPodPriority(pod) + podPriority := podutil.GetPodPriority(pod) for _, p := range pods { - if util.GetPodPriority(p) < podPriority { + if podutil.GetPodPriority(p) < podPriority { lowerPriorityPods = append(lowerPriorityPods, p) } } @@ -586,7 +587,7 @@ func addNominatedPods(pod *v1.Pod, meta predicates.PredicateMetadata, nodeInfoOut := nodeInfo.Clone() podsAdded := false for _, p := range nominatedPods { - if util.GetPodPriority(p) >= util.GetPodPriority(pod) && p.UID != pod.UID { + if podutil.GetPodPriority(p) >= podutil.GetPodPriority(pod) && p.UID != pod.UID { nodeInfoOut.AddPod(p) if metaOut != nil { if err := metaOut.AddPod(p, nodeInfoOut); err != nil { @@ -905,7 +906,7 @@ func pickOneNodeForPreemption(nodesToVictims map[*v1.Node]*schedulerapi.Victims) node := minNodes1[i] victims := nodesToVictims[node] // highestPodPriority is the highest priority among the victims on this node. - highestPodPriority := util.GetPodPriority(victims.Pods[0]) + highestPodPriority := podutil.GetPodPriority(victims.Pods[0]) if highestPodPriority < minHighestPriority { minHighestPriority = highestPodPriority lenNodes2 = 0 @@ -931,7 +932,7 @@ func pickOneNodeForPreemption(nodesToVictims map[*v1.Node]*schedulerapi.Victims) // needed so that a node with a few pods with negative priority is not // picked over a node with a smaller number of pods with the same negative // priority (and similar scenarios). - sumPriorities += int64(util.GetPodPriority(pod)) + int64(math.MaxInt32+1) + sumPriorities += int64(podutil.GetPodPriority(pod)) + int64(math.MaxInt32+1) } if sumPriorities < minSumPriorities { minSumPriorities = sumPriorities @@ -1122,9 +1123,9 @@ func (g *genericScheduler) selectVictimsOnNode( } // As the first step, remove all the lower priority pods from the node and // check if the given pod can be scheduled. - podPriority := util.GetPodPriority(pod) + podPriority := podutil.GetPodPriority(pod) for _, p := range nodeInfoCopy.Pods() { - if util.GetPodPriority(p) < podPriority { + if podutil.GetPodPriority(p) < podPriority { potentialVictims.Items = append(potentialVictims.Items, p) if err := removePod(p); err != nil { return nil, 0, false @@ -1229,9 +1230,9 @@ func podEligibleToPreemptOthers(pod *v1.Pod, nodeNameToInfo map[string]*schedule nomNodeName := pod.Status.NominatedNodeName if len(nomNodeName) > 0 { if nodeInfo, found := nodeNameToInfo[nomNodeName]; found { - podPriority := util.GetPodPriority(pod) + podPriority := podutil.GetPodPriority(pod) for _, p := range nodeInfo.Pods() { - if p.DeletionTimestamp != nil && util.GetPodPriority(p) < podPriority { + if p.DeletionTimestamp != nil && podutil.GetPodPriority(p) < podPriority { // There is a terminating pod on the nominated node. return false } diff --git a/pkg/scheduler/internal/queue/BUILD b/pkg/scheduler/internal/queue/BUILD index 915e557e4ad..19d02afa1e0 100644 --- a/pkg/scheduler/internal/queue/BUILD +++ b/pkg/scheduler/internal/queue/BUILD @@ -9,6 +9,7 @@ go_library( importpath = "k8s.io/kubernetes/pkg/scheduler/internal/queue", visibility = ["//pkg/scheduler:__subpackages__"], deps = [ + "//pkg/api/v1/pod:go_default_library", "//pkg/scheduler/algorithm/predicates:go_default_library", "//pkg/scheduler/algorithm/priorities/util:go_default_library", "//pkg/scheduler/framework/v1alpha1:go_default_library", diff --git a/pkg/scheduler/internal/queue/scheduling_queue.go b/pkg/scheduler/internal/queue/scheduling_queue.go index 14e22aaafb8..64debc829f7 100644 --- a/pkg/scheduler/internal/queue/scheduling_queue.go +++ b/pkg/scheduler/internal/queue/scheduling_queue.go @@ -15,7 +15,7 @@ limitations under the License. */ // This file contains structures that implement scheduling queue types. -// Scheduling queues hold pods waiting to be scheduled. This file implements a +// Scheduling queues hold pods waiting to be scheduled. This file implements a/ // priority queue which has two sub queues. One sub-queue holds pods that are // being considered for scheduling. This is called activeQ. Another queue holds // pods that are already tried and are determined to be unschedulable. The latter @@ -31,11 +31,12 @@ import ( "k8s.io/klog" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ktypes "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/tools/cache" + "k8s.io/kubernetes/pkg/api/v1/pod" "k8s.io/kubernetes/pkg/scheduler/algorithm/predicates" priorityutil "k8s.io/kubernetes/pkg/scheduler/algorithm/priorities/util" framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" @@ -154,8 +155,8 @@ func newPodInfoNoTimestamp(pod *v1.Pod) *framework.PodInfo { func activeQComp(podInfo1, podInfo2 interface{}) bool { pInfo1 := podInfo1.(*framework.PodInfo) pInfo2 := podInfo2.(*framework.PodInfo) - prio1 := util.GetPodPriority(pInfo1.Pod) - prio2 := util.GetPodPriority(pInfo2.Pod) + prio1 := pod.GetPodPriority(pInfo1.Pod) + prio2 := pod.GetPodPriority(pInfo2.Pod) return (prio1 > prio2) || (prio1 == prio2 && pInfo1.Timestamp.Before(pInfo2.Timestamp)) } diff --git a/pkg/scheduler/internal/queue/scheduling_queue_test.go b/pkg/scheduler/internal/queue/scheduling_queue_test.go index 779f69d9f13..a083d862ee9 100644 --- a/pkg/scheduler/internal/queue/scheduling_queue_test.go +++ b/pkg/scheduler/internal/queue/scheduling_queue_test.go @@ -157,8 +157,8 @@ type fakeFramework struct{} func (*fakeFramework) QueueSortFunc() framework.LessFunc { return func(podInfo1, podInfo2 *framework.PodInfo) bool { - prio1 := util.GetPodPriority(podInfo1.Pod) - prio2 := util.GetPodPriority(podInfo2.Pod) + prio1 := podutil.GetPodPriority(podInfo1.Pod) + prio2 := podutil.GetPodPriority(podInfo2.Pod) return prio1 < prio2 } } @@ -855,7 +855,7 @@ func TestPodFailedSchedulingMultipleTimesDoesNotBlockNewerPod(t *testing.T) { // Add an unschedulable pod to a priority queue. // This makes a situation that the pod was tried to schedule - // and had been determined unschedulable so far. + // and had been determined unschedulable so far unschedulablePod := v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "test-pod-unscheduled", diff --git a/pkg/scheduler/util/BUILD b/pkg/scheduler/util/BUILD index 3d1a71aa14b..61b523aea93 100644 --- a/pkg/scheduler/util/BUILD +++ b/pkg/scheduler/util/BUILD @@ -15,7 +15,7 @@ go_test( ], embed = [":go_default_library"], deps = [ - "//pkg/apis/scheduling:go_default_library", + "//pkg/api/v1/pod:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", ], @@ -31,7 +31,7 @@ go_library( ], importpath = "k8s.io/kubernetes/pkg/scheduler/util", deps = [ - "//pkg/apis/scheduling:go_default_library", + "//pkg/api/v1/pod:go_default_library", "//pkg/scheduler/api:go_default_library", "//pkg/scheduler/metrics:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", diff --git a/pkg/scheduler/util/utils.go b/pkg/scheduler/util/utils.go index 1a46bfcf754..e6b349fba0e 100644 --- a/pkg/scheduler/util/utils.go +++ b/pkg/scheduler/util/utils.go @@ -20,10 +20,10 @@ import ( "sort" "time" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/klog" - "k8s.io/kubernetes/pkg/apis/scheduling" + podutil "k8s.io/kubernetes/pkg/api/v1/pod" "k8s.io/kubernetes/pkg/scheduler/api" ) @@ -49,17 +49,6 @@ func GetPodFullName(pod *v1.Pod) string { return pod.Name + "_" + pod.Namespace } -// GetPodPriority returns priority of the given pod. -func GetPodPriority(pod *v1.Pod) int32 { - if pod.Spec.Priority != nil { - return *pod.Spec.Priority - } - // When priority of a running pod is nil, it means it was created at a time - // that there was no global default priority class and the priority class - // name of the pod was empty. So, we resolve to the static default priority. - return scheduling.DefaultPriorityWhenNoDefaultClassExists -} - // GetPodStartTime returns start time of the given pod. func GetPodStartTime(pod *v1.Pod) *metav1.Time { if pod.Status.StartTime != nil { @@ -83,15 +72,15 @@ func GetEarliestPodStartTime(victims *api.Victims) *metav1.Time { } earliestPodStartTime := GetPodStartTime(victims.Pods[0]) - highestPriority := GetPodPriority(victims.Pods[0]) + maxPriority := podutil.GetPodPriority(victims.Pods[0]) for _, pod := range victims.Pods { - if GetPodPriority(pod) == highestPriority { + if podutil.GetPodPriority(pod) == maxPriority { if GetPodStartTime(pod).Before(earliestPodStartTime) { earliestPodStartTime = GetPodStartTime(pod) } - } else if GetPodPriority(pod) > highestPriority { - highestPriority = GetPodPriority(pod) + } else if podutil.GetPodPriority(pod) > maxPriority { + maxPriority = podutil.GetPodPriority(pod) earliestPodStartTime = GetPodStartTime(pod) } } @@ -132,8 +121,8 @@ func (l *SortableList) Sort() { // It takes arguments of the type "interface{}" to be used with SortableList, // but expects those arguments to be *v1.Pod. func MoreImportantPod(pod1, pod2 interface{}) bool { - p1 := GetPodPriority(pod1.(*v1.Pod)) - p2 := GetPodPriority(pod2.(*v1.Pod)) + p1 := podutil.GetPodPriority(pod1.(*v1.Pod)) + p2 := podutil.GetPodPriority(pod2.(*v1.Pod)) if p1 != p2 { return p1 > p2 } diff --git a/pkg/scheduler/util/utils_test.go b/pkg/scheduler/util/utils_test.go index 206e5424c84..af29a98792b 100644 --- a/pkg/scheduler/util/utils_test.go +++ b/pkg/scheduler/util/utils_test.go @@ -22,50 +22,14 @@ import ( "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/diff" - "k8s.io/kubernetes/pkg/apis/scheduling" + "k8s.io/kubernetes/pkg/api/v1/pod" ) -// TestGetPodPriority tests GetPodPriority function. -func TestGetPodPriority(t *testing.T) { - p := int32(20) - tests := []struct { - name string - pod *v1.Pod - expectedPriority int32 - }{ - { - name: "no priority pod resolves to static default priority", - pod: &v1.Pod{ - Spec: v1.PodSpec{Containers: []v1.Container{ - {Name: "container", Image: "image"}}, - }, - }, - expectedPriority: scheduling.DefaultPriorityWhenNoDefaultClassExists, - }, - { - name: "pod with priority resolves correctly", - pod: &v1.Pod{ - Spec: v1.PodSpec{Containers: []v1.Container{ - {Name: "container", Image: "image"}}, - Priority: &p, - }, - }, - expectedPriority: p, - }, - } - for _, test := range tests { - if GetPodPriority(test.pod) != test.expectedPriority { - t.Errorf("expected pod priority: %v, got %v", test.expectedPriority, GetPodPriority(test.pod)) - } - - } -} - // TestSortableList tests SortableList by storing pods in the list and sorting // them by their priority. func TestSortableList(t *testing.T) { higherPriority := func(pod1, pod2 interface{}) bool { - return GetPodPriority(pod1.(*v1.Pod)) > GetPodPriority(pod2.(*v1.Pod)) + return pod.GetPodPriority(pod1.(*v1.Pod)) > pod.GetPodPriority(pod2.(*v1.Pod)) } podList := SortableList{CompFunc: higherPriority} // Add a few Pods with different priorities from lowest to highest priority.