addressed reviewer comments

pkg/scheduler/algorithm/predicates/metadata.go

@@ -244,9 +244,11 @@ func (meta *predicateMetadata) AddPod(addedPod *v1.Pod, nodeInfo *schedulercache
 	podNodeName := addedPod.Spec.NodeName
 	if affinity != nil && len(podNodeName) > 0 {
 		podNode := nodeInfo.Node()
-		affinityTerms := GetPodAffinityTerms(affinity.PodAffinity)
-		antiAffinityTerms := GetPodAntiAffinityTerms(affinity.PodAntiAffinity)
+		// It is assumed that when the added pod matches affinity of the meta.pod, all the terms must match,
+		// this should be changed when the implementation of targetPodMatchesAffinityOfPod/podMatchesAffinityTermProperties
+		// is changed
 		if targetPodMatchesAffinityOfPod(meta.pod, addedPod) {
+			affinityTerms := GetPodAffinityTerms(affinity.PodAffinity)
 			for _, term := range affinityTerms {
 				if topologyValue, ok := podNode.Labels[term.TopologyKey]; ok {
 					pair := topologyPair{key: term.TopologyKey, value: topologyValue}
@@ -255,6 +257,7 @@ func (meta *predicateMetadata) AddPod(addedPod *v1.Pod, nodeInfo *schedulercache
 			}
 		}
 		if targetPodMatchesAntiAffinityOfPod(meta.pod, addedPod) {
+			antiAffinityTerms := GetPodAntiAffinityTerms(affinity.PodAntiAffinity)
 			for _, term := range antiAffinityTerms {
 				if topologyValue, ok := podNode.Labels[term.TopologyKey]; ok {
 					pair := topologyPair{key: term.TopologyKey, value: topologyValue}

pkg/scheduler/algorithm/predicates/predicates.go

@@ -1393,23 +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 *topologyPairsMaps, nodeInfo *schedulercache.NodeInfo, terms []v1.PodAffinityTerm) (bool, error) {
-	podNameToMatchingTermsCount := make(map[string]int)
+//  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()
-	podTermsCount := len(terms)
 	for _, term := range terms {
-		pair := topologyPair{key: term.TopologyKey, value: node.Labels[term.TopologyKey]}
-		for existingPod := range targetPods.topologyPairToPods[pair] {
-			existingPodFullName := schedutil.GetPodFullName(existingPod)
-			podNameToMatchingTermsCount[existingPodFullName] = podNameToMatchingTermsCount[existingPodFullName] + 1
-			if podNameToMatchingTermsCount[existingPodFullName] == podTermsCount {
-				return true, nil
+		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.
@@ -1422,20 +1420,15 @@ func (c *PodAffinityChecker) satisfiesPodsAffinityAntiAffinity(pod *v1.Pod,
 	}
 	if predicateMeta, ok := meta.(*predicateMetadata); ok {
 		// Check all affinity terms.
-		matchingPods := predicateMeta.topologyPairsPotentialAffinityPods
+		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.topologyPairToPods) == 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
@@ -1444,12 +1437,12 @@ func (c *PodAffinityChecker) satisfiesPodsAffinityAntiAffinity(pod *v1.Pod,
 		}
 
 		// Check all anti-affinity terms.
-		matchingPods = predicateMeta.topologyPairsPotentialAntiAffinityPods
+		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
 			}
 		}