Merge pull request #67788 from mohamed-mehany/inter-pod-affinity-optimization

pkg/scheduler/algorithm/predicates/metadata_test.go

@@ -52,13 +52,6 @@ func (s sortableServices) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
 
 var _ = sort.Interface(&sortableServices{})
 
-func sortNodePodMap(np map[string][]*v1.Pod) {
-	for _, pl := range np {
-		sortablePods := sortablePods(pl)
-		sort.Sort(sortablePods)
-	}
-}
-
 // predicateMetadataEquivalent returns true if the two metadata are equivalent.
 // Note: this function does not compare podRequest.
 func predicateMetadataEquivalent(meta1, meta2 *predicateMetadata) error {
@@ -77,15 +70,11 @@ func predicateMetadataEquivalent(meta1, meta2 *predicateMetadata) error {
 	for !reflect.DeepEqual(meta1.podPorts, meta2.podPorts) {
 		return fmt.Errorf("podPorts are not equal")
 	}
-	sortNodePodMap(meta1.nodeNameToMatchingAffinityPods)
-	sortNodePodMap(meta2.nodeNameToMatchingAffinityPods)
-	if !reflect.DeepEqual(meta1.nodeNameToMatchingAffinityPods, meta2.nodeNameToMatchingAffinityPods) {
-		return fmt.Errorf("nodeNameToMatchingAffinityPods are not euqal")
+	if !reflect.DeepEqual(meta1.topologyPairsPotentialAffinityPods, meta2.topologyPairsPotentialAffinityPods) {
+		return fmt.Errorf("topologyPairsPotentialAffinityPods are not equal")
 	}
-	sortNodePodMap(meta1.nodeNameToMatchingAntiAffinityPods)
-	sortNodePodMap(meta2.nodeNameToMatchingAntiAffinityPods)
-	if !reflect.DeepEqual(meta1.nodeNameToMatchingAntiAffinityPods, meta2.nodeNameToMatchingAntiAffinityPods) {
-		return fmt.Errorf("nodeNameToMatchingAntiAffinityPods are not euqal")
+	if !reflect.DeepEqual(meta1.topologyPairsPotentialAntiAffinityPods, meta2.topologyPairsPotentialAntiAffinityPods) {
+		return fmt.Errorf("topologyPairsPotentialAntiAffinityPods are not equal")
 	}
 	if !reflect.DeepEqual(meta1.topologyPairsAntiAffinityPodsMap.podToTopologyPairs,
 		meta2.topologyPairsAntiAffinityPodsMap.podToTopologyPairs) {
@@ -454,42 +443,71 @@ func TestPredicateMetadata_ShallowCopy(t *testing.T) {
 				},
 			},
 		},
-		nodeNameToMatchingAffinityPods: map[string][]*v1.Pod{
-			"nodeA": {
-				&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p1", Labels: selector1},
-					Spec: v1.PodSpec{NodeName: "nodeA"},
+		topologyPairsPotentialAffinityPods: &topologyPairsMaps{
+			topologyPairToPods: map[topologyPair]podSet{
+				{key: "name", value: "nodeA"}: {
+					&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p1", Labels: selector1},
+						Spec: v1.PodSpec{NodeName: "nodeA"},
+					}: struct{}{},
+				},
+				{key: "name", value: "nodeC"}: {
+					&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p2"},
+						Spec: v1.PodSpec{
+							NodeName: "nodeC",
+						},
+					}: struct{}{},
+					&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p6", Labels: selector1},
+						Spec: v1.PodSpec{NodeName: "nodeC"},
+					}: struct{}{},
 				},
 			},
-			"nodeC": {
-				&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p2"},
-					Spec: v1.PodSpec{
-						NodeName: "nodeC",
-					},
+			podToTopologyPairs: map[string]topologyPairSet{
+				"p1_": {
+					topologyPair{key: "name", value: "nodeA"}: struct{}{},
 				},
-				&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p6", Labels: selector1},
-					Spec: v1.PodSpec{NodeName: "nodeC"},
+				"p2_": {
+					topologyPair{key: "name", value: "nodeC"}: struct{}{},
+				},
+				"p6_": {
+					topologyPair{key: "name", value: "nodeC"}: struct{}{},
 				},
 			},
 		},
-		nodeNameToMatchingAntiAffinityPods: map[string][]*v1.Pod{
-			"nodeN": {
-				&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p1", Labels: selector1},
-					Spec: v1.PodSpec{NodeName: "nodeN"},
-				},
-				&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p2"},
-					Spec: v1.PodSpec{
-						NodeName: "nodeM",
-					},
+		topologyPairsPotentialAntiAffinityPods: &topologyPairsMaps{
+			topologyPairToPods: map[topologyPair]podSet{
+				{key: "name", value: "nodeN"}: {
+					&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p1", Labels: selector1},
+						Spec: v1.PodSpec{NodeName: "nodeN"},
+					}: struct{}{},
+					&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p2"},
+						Spec: v1.PodSpec{
+							NodeName: "nodeM",
+						},
+					}: struct{}{},
+					&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p3"},
+						Spec: v1.PodSpec{
+							NodeName: "nodeM",
+						},
+					}: struct{}{},
 				},
-				&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p3"},
-					Spec: v1.PodSpec{
-						NodeName: "nodeM",
-					},
+				{key: "name", value: "nodeM"}: {
+					&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p6", Labels: selector1},
+						Spec: v1.PodSpec{NodeName: "nodeM"},
+					}: struct{}{},
 				},
 			},
-			"nodeM": {
-				&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p6", Labels: selector1},
-					Spec: v1.PodSpec{NodeName: "nodeM"},
+			podToTopologyPairs: map[string]topologyPairSet{
+				"p1_": {
+					topologyPair{key: "name", value: "nodeN"}: struct{}{},
+				},
+				"p2_": {
+					topologyPair{key: "name", value: "nodeN"}: struct{}{},
+				},
+				"p3_": {
+					topologyPair{key: "name", value: "nodeN"}: struct{}{},
+				},
+				"p6_": {
+					topologyPair{key: "name", value: "nodeM"}: struct{}{},
 				},
 			},
 		},

pkg/scheduler/algorithm/predicates/predicates.go

@@ -1393,30 +1393,21 @@ func (c *PodAffinityChecker) satisfiesExistingPodsAntiAffinity(pod *v1.Pod, meta
 	return nil, nil
 }
 
-// anyPodsMatchingTopologyTerms checks whether any of the nodes given via
-// "targetPods" matches topology of all the "terms" for the give "pod" and "nodeInfo".
-func (c *PodAffinityChecker) anyPodsMatchingTopologyTerms(pod *v1.Pod, targetPods map[string][]*v1.Pod, nodeInfo *schedulercache.NodeInfo, terms []v1.PodAffinityTerm) (bool, error) {
-	for nodeName, targetPods := range targetPods {
-		targetPodNodeInfo, err := c.info.GetNodeInfo(nodeName)
-		if err != nil {
-			return false, err
-		}
-		if len(targetPods) > 0 {
-			allTermsMatched := true
-			for _, term := range terms {
-				if !priorityutil.NodesHaveSameTopologyKey(nodeInfo.Node(), targetPodNodeInfo, term.TopologyKey) {
-					allTermsMatched = false
-					break
-				}
-			}
-			if allTermsMatched {
-				// We have 1 or more pods on the target node that have already matched namespace and selector
-				// and all of the terms topologies matched the target node. So, there is at least 1 matching pod on the node.
-				return true, nil
+//  nodeMatchesTopologyTerms checks whether "nodeInfo" matches
+//  topology of all the "terms" for the given "pod".
+func (c *PodAffinityChecker) nodeMatchesTopologyTerms(pod *v1.Pod, topologyPairs *topologyPairsMaps, nodeInfo *schedulercache.NodeInfo, terms []v1.PodAffinityTerm) bool {
+	node := nodeInfo.Node()
+	for _, term := range terms {
+		if topologyValue, ok := node.Labels[term.TopologyKey]; ok {
+			pair := topologyPair{key: term.TopologyKey, value: topologyValue}
+			if _, ok := topologyPairs.topologyPairToPods[pair]; !ok {
+				return false
 			}
+		} else {
+			return false
 		}
 	}
-	return false, nil
+	return true
 }
 
 // Checks if scheduling the pod onto this node would break any rules of this pod.
@@ -1429,20 +1420,15 @@ func (c *PodAffinityChecker) satisfiesPodsAffinityAntiAffinity(pod *v1.Pod,
 	}
 	if predicateMeta, ok := meta.(*predicateMetadata); ok {
 		// Check all affinity terms.
-		matchingPods := predicateMeta.nodeNameToMatchingAffinityPods
+		topologyPairsPotentialAffinityPods := predicateMeta.topologyPairsPotentialAffinityPods
 		if affinityTerms := GetPodAffinityTerms(affinity.PodAffinity); len(affinityTerms) > 0 {
-			matchExists, err := c.anyPodsMatchingTopologyTerms(pod, matchingPods, nodeInfo, affinityTerms)
-			if err != nil {
-				errMessage := fmt.Sprintf("Cannot schedule pod %+v onto node %v, because of PodAffinity, err: %v", podName(pod), node.Name, err)
-				glog.Errorf(errMessage)
-				return ErrPodAffinityRulesNotMatch, errors.New(errMessage)
-			}
+			matchExists := c.nodeMatchesTopologyTerms(pod, topologyPairsPotentialAffinityPods, nodeInfo, affinityTerms)
 			if !matchExists {
 				// This pod may the first pod in a series that have affinity to themselves. In order
 				// to not leave such pods in pending state forever, we check that if no other pod
 				// in the cluster matches the namespace and selector of this pod and the pod matches
 				// its own terms, then we allow the pod to pass the affinity check.
-				if !(len(matchingPods) == 0 && targetPodMatchesAffinityOfPod(pod, pod)) {
+				if !(len(topologyPairsPotentialAffinityPods.topologyPairToPods) == 0 && targetPodMatchesAffinityOfPod(pod, pod)) {
 					glog.V(10).Infof("Cannot schedule pod %+v onto node %v, because of PodAffinity",
 						podName(pod), node.Name)
 					return ErrPodAffinityRulesNotMatch, nil
@@ -1451,12 +1437,12 @@ func (c *PodAffinityChecker) satisfiesPodsAffinityAntiAffinity(pod *v1.Pod,
 		}
 
 		// Check all anti-affinity terms.
-		matchingPods = predicateMeta.nodeNameToMatchingAntiAffinityPods
+		topologyPairsPotentialAntiAffinityPods := predicateMeta.topologyPairsPotentialAntiAffinityPods
 		if antiAffinityTerms := GetPodAntiAffinityTerms(affinity.PodAntiAffinity); len(antiAffinityTerms) > 0 {
-			matchExists, err := c.anyPodsMatchingTopologyTerms(pod, matchingPods, nodeInfo, antiAffinityTerms)
-			if err != nil || matchExists {
-				glog.V(10).Infof("Cannot schedule pod %+v onto node %v, because of PodAntiAffinity, err: %v",
-					podName(pod), node.Name, err)
+			matchExists := c.nodeMatchesTopologyTerms(pod, topologyPairsPotentialAntiAffinityPods, nodeInfo, antiAffinityTerms)
+			if matchExists {
+				glog.V(10).Infof("Cannot schedule pod %+v onto node %v, because of PodAntiAffinity",
+					podName(pod), node.Name)
 				return ErrPodAntiAffinityRulesNotMatch, nil
 			}
 		}