Merge pull request #32471 from wojtek-t/use_controller_ref_for_avoid_priority

plugin/pkg/scheduler/algorithm/priorities/priorities.go

@@ -345,24 +345,16 @@ func fractionOfCapacity(requested, capacity int64) float64 {
 	return float64(requested) / float64(capacity)
 }
 
-type NodePreferAvoidPod struct {
-	controllerLister algorithm.ControllerLister
-	replicaSetLister algorithm.ReplicaSetLister
-}
-
-func NewNodePreferAvoidPodsPriority(controllerLister algorithm.ControllerLister, replicaSetLister algorithm.ReplicaSetLister) algorithm.PriorityFunction {
-	nodePreferAvoid := &NodePreferAvoidPod{
-		controllerLister: controllerLister,
-		replicaSetLister: replicaSetLister,
+func CalculateNodePreferAvoidPodsPriority(pod *api.Pod, nodeNameToInfo map[string]*schedulercache.NodeInfo, nodes []*api.Node) (schedulerapi.HostPriorityList, error) {
+	controllerRef := priorityutil.GetControllerRef(pod)
+	if controllerRef != nil {
+		// Ignore pods that are owned by other controller than ReplicationController
+		// or ReplicaSet.
+		if controllerRef.Kind != "ReplicationController" && controllerRef.Kind != "ReplicaSet" {
+			controllerRef = nil
 		}
-	return nodePreferAvoid.CalculateNodePreferAvoidPodsPriority
-}
-
-func (npa *NodePreferAvoidPod) CalculateNodePreferAvoidPodsPriority(pod *api.Pod, nodeNameToInfo map[string]*schedulercache.NodeInfo, nodes []*api.Node) (schedulerapi.HostPriorityList, error) {
-	// TODO: Once we have ownerReference fully implemented, use it to find controller for the pod.
-	rcs, _ := npa.controllerLister.GetPodControllers(pod)
-	rss, _ := npa.replicaSetLister.GetPodReplicaSets(pod)
-	if len(rcs) == 0 && len(rss) == 0 {
+	}
+	if controllerRef == nil {
 		result := make(schedulerapi.HostPriorityList, 0, len(nodes))
 		for _, node := range nodes {
 			result = append(result, schedulerapi.HostPriority{Host: node.Name, Score: 10})
@@ -381,17 +373,8 @@ func (npa *NodePreferAvoidPod) CalculateNodePreferAvoidPodsPriority(pod *api.Pod
 		avoidNode = false
 		for i := range avoids.PreferAvoidPods {
 			avoid := &avoids.PreferAvoidPods[i]
-			// TODO: Once we have controllerRef implemented there will be at most one owner
-			// of our pod. That said we won't even need loop theoretically. That said for
-			// code simplicity, we can get rid of all breaks.
-			// Also, we can simply compare fields from ownerRef with avoid.
-			for _, rc := range rcs {
-				if avoid.PodSignature.PodController.Kind == "ReplicationController" && avoid.PodSignature.PodController.UID == rc.UID {
-					avoidNode = true
-				}
-			}
-			for _, rs := range rss {
-				if avoid.PodSignature.PodController.Kind == "ReplicaSet" && avoid.PodSignature.PodController.UID == rs.UID {
+			if controllerRef != nil {
+				if avoid.PodSignature.PodController.Kind == controllerRef.Kind && avoid.PodSignature.PodController.UID == controllerRef.UID {
 					avoidNode = true
 				}
 			}

plugin/pkg/scheduler/algorithm/priorities/priorities_test.go

@@ -28,8 +28,6 @@ import (
 	"k8s.io/kubernetes/cmd/libs/go2idl/types"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/resource"
-	"k8s.io/kubernetes/pkg/api/unversioned"
-	"k8s.io/kubernetes/pkg/apis/extensions"
 	"k8s.io/kubernetes/pkg/util/codeinspector"
 	"k8s.io/kubernetes/plugin/pkg/scheduler/algorithm"
 	schedulerapi "k8s.io/kubernetes/plugin/pkg/scheduler/api"
@@ -1041,8 +1039,6 @@ func TestPrioritiesRegistered(t *testing.T) {
 }
 
 func TestNodePreferAvoidPriority(t *testing.T) {
-	label1 := map[string]string{"foo": "bar"}
-	label2 := map[string]string{"bar": "foo"}
 	annotations1 := map[string]string{
 		api.PreferAvoidPodsAnnotationKey: `
 							{
@@ -1094,24 +1090,20 @@ func TestNodePreferAvoidPriority(t *testing.T) {
 			ObjectMeta: api.ObjectMeta{Name: "machine3"},
 		},
 	}
+	trueVar := true
 	tests := []struct {
 		pod          *api.Pod
-		rcs          []api.ReplicationController
-		rss          []extensions.ReplicaSet
 		nodes        []*api.Node
 		expectedList schedulerapi.HostPriorityList
 		test         string
 	}{
 		{
-			pod: &api.Pod{ObjectMeta: api.ObjectMeta{Namespace: "default", Labels: label1}},
-			rcs: []api.ReplicationController{
-				{
+			pod: &api.Pod{
 				ObjectMeta: api.ObjectMeta{
 					Namespace: "default",
-						Name:      "foo",
-						UID:       "abcdef123456",
+					OwnerReferences: []api.OwnerReference{
+						{Kind: "ReplicationController", Name: "foo", UID: "abcdef123456", Controller: &trueVar},
 					},
-					Spec: api.ReplicationControllerSpec{Selector: label1},
 				},
 			},
 			nodes:        testNodes,
@@ -1119,49 +1111,48 @@ func TestNodePreferAvoidPriority(t *testing.T) {
 			test:         "pod managed by ReplicationController should avoid a node, this node get lowest priority score",
 		},
 		{
-			pod: &api.Pod{ObjectMeta: api.ObjectMeta{Namespace: "default", Labels: label2}},
-			rss: []extensions.ReplicaSet{
-				{
-					TypeMeta: unversioned.TypeMeta{
-						APIVersion: "v1",
-						Kind:       "ReplicaSet",
-					},
+			pod: &api.Pod{
 				ObjectMeta: api.ObjectMeta{
 					Namespace: "default",
-						Name:      "bar",
-						UID:       "qwert12345",
+					OwnerReferences: []api.OwnerReference{
+						{Kind: "RandomController", Name: "foo", UID: "abcdef123456", Controller: &trueVar},
 					},
-					Spec: extensions.ReplicaSetSpec{Selector: &unversioned.LabelSelector{MatchLabels: label2}},
 				},
 			},
 			nodes:        testNodes,
-			expectedList: []schedulerapi.HostPriority{{Host: "machine1", Score: 10}, {Host: "machine2", Score: 0}, {Host: "machine3", Score: 10}},
-			test:         "pod managed by ReplicaSet should avoid a node, this node get lowest priority score",
+			expectedList: []schedulerapi.HostPriority{{Host: "machine1", Score: 10}, {Host: "machine2", Score: 10}, {Host: "machine3", Score: 10}},
+			test:         "ownership by random controller should be ignored",
 		},
 		{
-			pod: &api.Pod{ObjectMeta: api.ObjectMeta{Namespace: "default"}},
-			rcs: []api.ReplicationController{
-				{
+			pod: &api.Pod{
 				ObjectMeta: api.ObjectMeta{
 					Namespace: "default",
-						Name:      "foo",
-						UID:       "abcdef123456",
+					OwnerReferences: []api.OwnerReference{
+						{Kind: "ReplicationController", Name: "foo", UID: "abcdef123456"},
 					},
-					Spec: api.ReplicationControllerSpec{Selector: label1},
 				},
 			},
 			nodes:        testNodes,
 			expectedList: []schedulerapi.HostPriority{{Host: "machine1", Score: 10}, {Host: "machine2", Score: 10}, {Host: "machine3", Score: 10}},
-			test:         "pod should not avoid these nodes, all nodes get highest priority score",
+			test:         "owner without Controller field set should be ignored",
+		},
+		{
+			pod: &api.Pod{
+				ObjectMeta: api.ObjectMeta{
+					Namespace: "default",
+					OwnerReferences: []api.OwnerReference{
+						{Kind: "ReplicaSet", Name: "foo", UID: "qwert12345", Controller: &trueVar},
+					},
+				},
+			},
+			nodes:        testNodes,
+			expectedList: []schedulerapi.HostPriority{{"machine1", 10}, {"machine2", 0}, {"machine3", 10}},
+			test:         "pod managed by ReplicaSet should avoid a node, this node get lowest priority score",
 		},
 	}
 
 	for _, test := range tests {
-		prioritizer := NodePreferAvoidPod{
-			controllerLister: algorithm.FakeControllerLister(test.rcs),
-			replicaSetLister: algorithm.FakeReplicaSetLister(test.rss),
-		}
-		list, err := prioritizer.CalculateNodePreferAvoidPodsPriority(test.pod, map[string]*schedulercache.NodeInfo{}, test.nodes)
+		list, err := CalculateNodePreferAvoidPodsPriority(test.pod, map[string]*schedulercache.NodeInfo{}, test.nodes)
 		if err != nil {
 			t.Errorf("unexpected error: %v", err)
 		}

plugin/pkg/scheduler/algorithm/priorities/util/util.go

+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+import (
+	"k8s.io/kubernetes/pkg/api"
+)
+
+func GetControllerRef(pod *api.Pod) *api.OwnerReference {
+	if len(pod.OwnerReferences) == 0 {
+		return nil
+	}
+	for i := range pod.OwnerReferences {
+		ref := &pod.OwnerReferences[i]
+		if ref.Controller != nil && *ref.Controller {
+			return ref
+		}
+	}
+	return nil
+}

plugin/pkg/scheduler/algorithmprovider/defaults/defaults.go

@@ -178,17 +178,9 @@ func defaultPriorities() sets.String {
 				Weight: 1,
 			},
 		),
-		factory.RegisterPriorityConfigFactory(
-			"NodePreferAvoidPodsPriority",
-			factory.PriorityConfigFactory{
-				Function: func(args factory.PluginFactoryArgs) algorithm.PriorityFunction {
-					return priorities.NewNodePreferAvoidPodsPriority(args.ControllerLister, args.ReplicaSetLister)
-				},
 		// Set this weight large enough to override all other priority functions.
 		// TODO: Figure out a better way to do this, maybe at same time as fixing #24720.
-				Weight: 10000,
-			},
-		),
+		factory.RegisterPriorityFunction("NodePreferAvoidPodsPriority", priorities.CalculateNodePreferAvoidPodsPriority, 10000),
 		factory.RegisterPriorityFunction("NodeAffinityPriority", priorities.CalculateNodeAffinityPriority, 1),
 		factory.RegisterPriorityFunction("TaintTolerationPriority", priorities.ComputeTaintTolerationPriority, 1),
 		// pods should be placed in the same topological domain (e.g. same node, same rack, same zone, same power domain, etc.)