From ee393b4e06cc642f2e30232b51e8d0abe715e78e Mon Sep 17 00:00:00 2001 From: Ahmad Diaa Date: Tue, 28 Aug 2018 22:22:06 +0200 Subject: [PATCH] addressed reviewer comments --- .../algorithm/predicates/metadata.go | 7 ++- .../algorithm/predicates/predicates.go | 43 ++++++++----------- 2 files changed, 23 insertions(+), 27 deletions(-) diff --git a/pkg/scheduler/algorithm/predicates/metadata.go b/pkg/scheduler/algorithm/predicates/metadata.go index 69ab1c24e9..f3dd8c895d 100644 --- a/pkg/scheduler/algorithm/predicates/metadata.go +++ b/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} diff --git a/pkg/scheduler/algorithm/predicates/predicates.go b/pkg/scheduler/algorithm/predicates/predicates.go index 79cb008413..d67aafdc50 100644 --- a/pkg/scheduler/algorithm/predicates/predicates.go +++ b/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 } }