Merge pull request #5153 from abhgupta/abhgupta-dev

Using only pods within the same namespace for spreading logic

pkg/scheduler/predicates.go

@@ -264,9 +264,16 @@ func (s *ServiceAffinity) CheckServiceAffinity(pod api.Pod, existingPods []api.P
 			if err != nil {
 				return false, err
 			}
-			if len(servicePods) > 0 {
+			// consider only the pods that belong to the same namespace
+			nsServicePods := []api.Pod{}
+			for _, nsPod := range servicePods {
+				if nsPod.Namespace == pod.Namespace {
+					nsServicePods = append(nsServicePods, nsPod)
+				}
+			}
+			if len(nsServicePods) > 0 {
 				// consider any service pod and fetch the minion its hosted on
-				otherMinion, err := s.nodeInfo.GetNodeInfo(servicePods[0].Status.Host)
+				otherMinion, err := s.nodeInfo.GetNodeInfo(nsServicePods[0].Status.Host)
 				if err != nil {
 					return false, err
 				}

pkg/scheduler/predicates_test.go

@@ -542,6 +542,33 @@ func TestServiceAffinity(t *testing.T) {
 			labels:   []string{"region"},
 			test:     "service pod on different minion, region mismatch",
 		},
+		{
+			pod:      api.Pod{ObjectMeta: api.ObjectMeta{Labels: selector, Namespace: "ns1"}},
+			pods:     []api.Pod{{Status: api.PodStatus{Host: "machine3"}, ObjectMeta: api.ObjectMeta{Labels: selector, Namespace: "ns1"}}},
+			node:     "machine1",
+			services: []api.Service{{Spec: api.ServiceSpec{Selector: selector}, ObjectMeta: api.ObjectMeta{Namespace: "ns2"}}},
+			fits:     true,
+			labels:   []string{"region"},
+			test:     "service in different namespace, region mismatch",
+		},
+		{
+			pod:      api.Pod{ObjectMeta: api.ObjectMeta{Labels: selector, Namespace: "ns1"}},
+			pods:     []api.Pod{{Status: api.PodStatus{Host: "machine3"}, ObjectMeta: api.ObjectMeta{Labels: selector, Namespace: "ns2"}}},
+			node:     "machine1",
+			services: []api.Service{{Spec: api.ServiceSpec{Selector: selector}, ObjectMeta: api.ObjectMeta{Namespace: "ns1"}}},
+			fits:     true,
+			labels:   []string{"region"},
+			test:     "pod in different namespace, region mismatch",
+		},
+		{
+			pod:      api.Pod{ObjectMeta: api.ObjectMeta{Labels: selector, Namespace: "ns1"}},
+			pods:     []api.Pod{{Status: api.PodStatus{Host: "machine3"}, ObjectMeta: api.ObjectMeta{Labels: selector, Namespace: "ns1"}}},
+			node:     "machine1",
+			services: []api.Service{{Spec: api.ServiceSpec{Selector: selector}, ObjectMeta: api.ObjectMeta{Namespace: "ns1"}}},
+			fits:     false,
+			labels:   []string{"region"},
+			test:     "service and pod in same namespace, region mismatch",
+		},
 		{
 			pod:      api.Pod{ObjectMeta: api.ObjectMeta{Labels: selector}},
 			pods:     []api.Pod{{Status: api.PodStatus{Host: "machine2"}, ObjectMeta: api.ObjectMeta{Labels: selector}}},

pkg/scheduler/spreading.go

@@ -36,18 +36,23 @@ func NewServiceSpreadPriority(serviceLister ServiceLister) PriorityFunction {
 // on the same machine.
 func (s *ServiceSpread) CalculateSpreadPriority(pod api.Pod, podLister PodLister, minionLister MinionLister) (HostPriorityList, error) {
 	var maxCount int
-	var pods []api.Pod
+	var nsServicePods []api.Pod
-	var err error
 
 	services, err := s.serviceLister.GetPodServices(pod)
 	if err == nil {
 		// just use the first service and get the other pods within the service
 		// TODO: a separate predicate can be created that tries to handle all services for the pod
 		selector := labels.SelectorFromSet(services[0].Spec.Selector)
-		pods, err = podLister.List(selector)
+		pods, err := podLister.List(selector)
 		if err != nil {
 			return nil, err
 		}
+		// consider only the pods that belong to the same namespace
+		for _, nsPod := range pods {
+			if nsPod.Namespace == pod.Namespace {
+				nsServicePods = append(nsServicePods, nsPod)
+			}
+		}
 	}
 
 	minions, err := minionLister.List()
@@ -56,8 +61,8 @@ func (s *ServiceSpread) CalculateSpreadPriority(pod api.Pod, podLister PodLister
 	}
 
 	counts := map[string]int{}
-	if len(pods) > 0 {
+	if len(nsServicePods) > 0 {
-		for _, pod := range pods {
+		for _, pod := range nsServicePods {
 			counts[pod.Status.Host]++
 			// Compute the maximum number of pods hosted on any minion
 			if counts[pod.Status.Host] > maxCount {
@@ -97,17 +102,23 @@ func NewServiceAntiAffinityPriority(serviceLister ServiceLister, label string) P
 // on machines with the same value for a particular label.
 // The label to be considered is provided to the struct (ServiceAntiAffinity).
 func (s *ServiceAntiAffinity) CalculateAntiAffinityPriority(pod api.Pod, podLister PodLister, minionLister MinionLister) (HostPriorityList, error) {
-	var pods []api.Pod
+	var nsServicePods []api.Pod
 
 	services, err := s.serviceLister.GetPodServices(pod)
 	if err == nil {
 		// just use the first service and get the other pods within the service
 		// TODO: a separate predicate can be created that tries to handle all services for the pod
 		selector := labels.SelectorFromSet(services[0].Spec.Selector)
-		pods, err = podLister.List(selector)
+		pods, err := podLister.List(selector)
 		if err != nil {
 			return nil, err
 		}
+		// consider only the pods that belong to the same namespace
+		for _, nsPod := range pods {
+			if nsPod.Namespace == pod.Namespace {
+				nsServicePods = append(nsServicePods, nsPod)
+			}
+		}
 	}
 
 	minions, err := minionLister.List()
@@ -128,7 +139,7 @@ func (s *ServiceAntiAffinity) CalculateAntiAffinityPriority(pod api.Pod, podList
 	}
 
 	podCounts := map[string]int{}
-	for _, pod := range pods {
+	for _, pod := range nsServicePods {
 		label, exists := labeledMinions[pod.Status.Host]
 		if !exists {
 			continue
@@ -136,7 +147,7 @@ func (s *ServiceAntiAffinity) CalculateAntiAffinityPriority(pod api.Pod, podList
 		podCounts[label]++
 	}
 
-	numServicePods := len(pods)
+	numServicePods := len(nsServicePods)
 	result := []HostPriority{}
 	//score int - scale of 0-10
 	// 0 being the lowest priority and 10 being the highest

pkg/scheduler/spreading_test.go

@@ -78,6 +78,47 @@ func TestServiceSpreadPriority(t *testing.T) {
 			expectedList: []HostPriority{{"machine1", 10}, {"machine2", 0}},
 			test:         "two pods, one service pod",
 		},
+		{
+			pod: api.Pod{ObjectMeta: api.ObjectMeta{Labels: labels1}},
+			pods: []api.Pod{
+				{Status: zone1Status, ObjectMeta: api.ObjectMeta{Labels: labels2}},
+				{Status: zone1Status, ObjectMeta: api.ObjectMeta{Labels: labels1, Namespace: api.NamespaceDefault}},
+				{Status: zone1Status, ObjectMeta: api.ObjectMeta{Labels: labels1, Namespace: "ns1"}},
+				{Status: zone2Status, ObjectMeta: api.ObjectMeta{Labels: labels1}},
+				{Status: zone2Status, ObjectMeta: api.ObjectMeta{Labels: labels2}},
+			},
+			nodes:        []string{"machine1", "machine2"},
+			services:     []api.Service{{Spec: api.ServiceSpec{Selector: labels1}}},
+			expectedList: []HostPriority{{"machine1", 10}, {"machine2", 0}},
+			test:         "five pods, one service pod in no namespace",
+		},
+		{
+			pod: api.Pod{ObjectMeta: api.ObjectMeta{Labels: labels1, Namespace: api.NamespaceDefault}},
+			pods: []api.Pod{
+				{Status: zone1Status, ObjectMeta: api.ObjectMeta{Labels: labels1}},
+				{Status: zone1Status, ObjectMeta: api.ObjectMeta{Labels: labels1, Namespace: "ns1"}},
+				{Status: zone2Status, ObjectMeta: api.ObjectMeta{Labels: labels1, Namespace: api.NamespaceDefault}},
+				{Status: zone2Status, ObjectMeta: api.ObjectMeta{Labels: labels2}},
+			},
+			nodes:        []string{"machine1", "machine2"},
+			services:     []api.Service{{Spec: api.ServiceSpec{Selector: labels1}, ObjectMeta: api.ObjectMeta{Namespace: api.NamespaceDefault}}},
+			expectedList: []HostPriority{{"machine1", 10}, {"machine2", 0}},
+			test:         "four pods, one service pod in default namespace",
+		},
+		{
+			pod: api.Pod{ObjectMeta: api.ObjectMeta{Labels: labels1, Namespace: "ns1"}},
+			pods: []api.Pod{
+				{Status: zone1Status, ObjectMeta: api.ObjectMeta{Labels: labels1}},
+				{Status: zone1Status, ObjectMeta: api.ObjectMeta{Labels: labels1, Namespace: api.NamespaceDefault}},
+				{Status: zone1Status, ObjectMeta: api.ObjectMeta{Labels: labels1, Namespace: "ns2"}},
+				{Status: zone2Status, ObjectMeta: api.ObjectMeta{Labels: labels1, Namespace: "ns1"}},
+				{Status: zone2Status, ObjectMeta: api.ObjectMeta{Labels: labels2}},
+			},
+			nodes:        []string{"machine1", "machine2"},
+			services:     []api.Service{{Spec: api.ServiceSpec{Selector: labels1}, ObjectMeta: api.ObjectMeta{Namespace: "ns1"}}},
+			expectedList: []HostPriority{{"machine1", 10}, {"machine2", 0}},
+			test:         "five pods, one service pod in specific namespace",
+		},
 		{
 			pod: api.Pod{ObjectMeta: api.ObjectMeta{Labels: labels1}},
 			pods: []api.Pod{
@@ -223,6 +264,21 @@ func TestZoneSpreadPriority(t *testing.T) {
 				{"machine01", 0}, {"machine02", 0}},
 			test: "three pods, two service pods on different machines",
 		},
+		{
+			pod: api.Pod{ObjectMeta: api.ObjectMeta{Labels: labels1, Namespace: api.NamespaceDefault}},
+			pods: []api.Pod{
+				{Status: zone1Status, ObjectMeta: api.ObjectMeta{Labels: labels1}},
+				{Status: zone1Status, ObjectMeta: api.ObjectMeta{Labels: labels1, Namespace: api.NamespaceDefault}},
+				{Status: zone2Status, ObjectMeta: api.ObjectMeta{Labels: labels1}},
+				{Status: zone2Status, ObjectMeta: api.ObjectMeta{Labels: labels1, Namespace: "ns1"}},
+			},
+			nodes:    labeledNodes,
+			services: []api.Service{{Spec: api.ServiceSpec{Selector: labels1}, ObjectMeta: api.ObjectMeta{Namespace: api.NamespaceDefault}}},
+			expectedList: []HostPriority{{"machine11", 0}, {"machine12", 0},
+				{"machine21", 10}, {"machine22", 10},
+				{"machine01", 0}, {"machine02", 0}},
+			test: "three service label match pods in different namespaces",
+		},
 		{
 			pod: api.Pod{ObjectMeta: api.ObjectMeta{Labels: labels1}},
 			pods: []api.Pod{