From 3fb6912d08194da623bd7680947b31c44359ef5a Mon Sep 17 00:00:00 2001 From: Mohamed Mehany <7327188+mohamed-mehany@users.noreply.github.com> Date: Mon, 30 Jul 2018 04:59:26 +0200 Subject: [PATCH 1/4] add topologyValue map to reduce search space --- .../algorithm/predicates/metadata.go | 32 +++++- .../algorithm/predicates/metadata_test.go | 4 + .../algorithm/predicates/predicates.go | 107 ++++++++++++------ 3 files changed, 108 insertions(+), 35 deletions(-) diff --git a/pkg/scheduler/algorithm/predicates/metadata.go b/pkg/scheduler/algorithm/predicates/metadata.go index 2b276ede61..80db0e65e4 100644 --- a/pkg/scheduler/algorithm/predicates/metadata.go +++ b/pkg/scheduler/algorithm/predicates/metadata.go @@ -54,6 +54,9 @@ type predicateMetadata struct { podPorts []*v1.ContainerPort //key is a pod full name with the anti-affinity rules. matchingAntiAffinityTerms map[string][]matchingPodAntiAffinityTerm + // A map of antiffinity terms' topology ke values to the pods' names + // that can potentially match the affinity rules of the pod + topologyValueToAntiAffinityPods map[string][]string // A map of node name to a list of Pods on the node that can potentially match // the affinity rules of the "pod". nodeNameToMatchingAffinityPods map[string][]*v1.Pod @@ -113,7 +116,7 @@ func (pfactory *PredicateMetadataFactory) GetMetadata(pod *v1.Pod, nodeNameToInf if pod == nil { return nil } - matchingTerms, err := getMatchingAntiAffinityTerms(pod, nodeNameToInfoMap) + matchingTerms, topologyValues, err := getMatchingAntiAffinityTerms(pod, nodeNameToInfoMap) if err != nil { return nil } @@ -130,6 +133,7 @@ func (pfactory *PredicateMetadataFactory) GetMetadata(pod *v1.Pod, nodeNameToInf matchingAntiAffinityTerms: matchingTerms, nodeNameToMatchingAffinityPods: affinityPods, nodeNameToMatchingAntiAffinityPods: antiAffinityPods, + topologyValueToAntiAffinityPods: topologyValues, } for predicateName, precomputeFunc := range predicateMetadataProducers { glog.V(10).Infof("Precompute: %v", predicateName) @@ -145,6 +149,21 @@ func (meta *predicateMetadata) RemovePod(deletedPod *v1.Pod) error { if deletedPodFullName == schedutil.GetPodFullName(meta.pod) { return fmt.Errorf("deletedPod and meta.pod must not be the same") } + + // Delete pod from matching topology values map + for _, term := range meta.matchingAntiAffinityTerms[deletedPodFullName] { + if topologyValue, ok := term.node.Labels[term.term.TopologyKey]; ok { + for index, podName := range meta.topologyValueToAntiAffinityPods[topologyValue] { + if podName == deletedPodFullName { + podsList := meta.topologyValueToAntiAffinityPods[topologyValue] + meta.topologyValueToAntiAffinityPods[topologyValue] = append(podsList[:index], + podsList[index+1:]...) + break + } + } + } + } + // Delete any anti-affinity rule from the deletedPod. delete(meta.matchingAntiAffinityTerms, deletedPodFullName) // Delete pod from the matching affinity or anti-affinity pods if exists. @@ -203,7 +222,7 @@ func (meta *predicateMetadata) AddPod(addedPod *v1.Pod, nodeInfo *schedulercache return fmt.Errorf("invalid node in nodeInfo") } // Add matching anti-affinity terms of the addedPod to the map. - podMatchingTerms, err := getMatchingAntiAffinityTermsOfExistingPod(meta.pod, addedPod, nodeInfo.Node()) + podMatchingTerms, podTopologyValuesToMatchingPods, err := getMatchingAntiAffinityTermsOfExistingPod(meta.pod, addedPod, nodeInfo.Node()) if err != nil { return err } @@ -215,6 +234,10 @@ func (meta *predicateMetadata) AddPod(addedPod *v1.Pod, nodeInfo *schedulercache } else { meta.matchingAntiAffinityTerms[addedPodFullName] = podMatchingTerms } + + for topologyValue, pods := range podTopologyValuesToMatchingPods { + meta.topologyValueToAntiAffinityPods[topologyValue] = append(meta.topologyValueToAntiAffinityPods[topologyValue], pods...) + } } // Add the pod to nodeNameToMatchingAffinityPods and nodeNameToMatchingAntiAffinityPods if needed. affinity := meta.pod.Spec.Affinity @@ -280,10 +303,15 @@ func (meta *predicateMetadata) ShallowCopy() algorithm.PredicateMetadata { for k, v := range meta.nodeNameToMatchingAntiAffinityPods { newPredMeta.nodeNameToMatchingAntiAffinityPods[k] = append([]*v1.Pod(nil), v...) } + newPredMeta.topologyValueToAntiAffinityPods = make(map[string][]string) + for k, v := range meta.topologyValueToAntiAffinityPods { + newPredMeta.topologyValueToAntiAffinityPods[k] = append([]string(nil), v...) + } newPredMeta.serviceAffinityMatchingPodServices = append([]*v1.Service(nil), meta.serviceAffinityMatchingPodServices...) newPredMeta.serviceAffinityMatchingPodList = append([]*v1.Pod(nil), meta.serviceAffinityMatchingPodList...) + return (algorithm.PredicateMetadata)(newPredMeta) } diff --git a/pkg/scheduler/algorithm/predicates/metadata_test.go b/pkg/scheduler/algorithm/predicates/metadata_test.go index fde740f745..84e7cc55c3 100644 --- a/pkg/scheduler/algorithm/predicates/metadata_test.go +++ b/pkg/scheduler/algorithm/predicates/metadata_test.go @@ -475,6 +475,10 @@ func TestPredicateMetadata_ShallowCopy(t *testing.T) { }, }, }, + topologyValueToAntiAffinityPods: map[string][]string{ + "machine1": {"p1", "p2"}, + "machine2": {"p3"}, + }, nodeNameToMatchingAffinityPods: map[string][]*v1.Pod{ "nodeA": { &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p1", Labels: selector1}, diff --git a/pkg/scheduler/algorithm/predicates/predicates.go b/pkg/scheduler/algorithm/predicates/predicates.go index 17d8f0591d..477bd0753b 100644 --- a/pkg/scheduler/algorithm/predicates/predicates.go +++ b/pkg/scheduler/algorithm/predicates/predicates.go @@ -1246,7 +1246,7 @@ func GetPodAntiAffinityTerms(podAntiAffinity *v1.PodAntiAffinity) (terms []v1.Po return terms } -func getMatchingAntiAffinityTerms(pod *v1.Pod, nodeInfoMap map[string]*schedulercache.NodeInfo) (map[string][]matchingPodAntiAffinityTerm, error) { +func getMatchingAntiAffinityTerms(pod *v1.Pod, nodeInfoMap map[string]*schedulercache.NodeInfo) (map[string][]matchingPodAntiAffinityTerm, map[string][]string, error) { allNodeNames := make([]string, 0, len(nodeInfoMap)) for name := range nodeInfoMap { allNodeNames = append(allNodeNames, name) @@ -1254,14 +1254,25 @@ func getMatchingAntiAffinityTerms(pod *v1.Pod, nodeInfoMap map[string]*scheduler var lock sync.Mutex var firstError error - result := make(map[string][]matchingPodAntiAffinityTerm) - appendResult := func(toAppend map[string][]matchingPodAntiAffinityTerm) { + podsToMatchingAntiAffinityTerms := make(map[string][]matchingPodAntiAffinityTerm) + topologyValuesToMatchingPods := make(map[string][]string) + + appendPodsMatchingAntiAffinityTerms := func(toAppend map[string][]matchingPodAntiAffinityTerm) { lock.Lock() defer lock.Unlock() for uid, terms := range toAppend { - result[uid] = append(result[uid], terms...) + podsToMatchingAntiAffinityTerms[uid] = append(podsToMatchingAntiAffinityTerms[uid], terms...) } } + + appendTopologyValuesMatchingPods := func(toAppend map[string][]string) { + lock.Lock() + defer lock.Unlock() + for topologyValue, pods := range toAppend { + topologyValuesToMatchingPods[topologyValue] = append(topologyValuesToMatchingPods[topologyValue], pods...) + } + } + catchError := func(err error) { lock.Lock() defer lock.Unlock() @@ -1277,7 +1288,9 @@ func getMatchingAntiAffinityTerms(pod *v1.Pod, nodeInfoMap map[string]*scheduler catchError(fmt.Errorf("node not found")) return } - nodeResult := make(map[string][]matchingPodAntiAffinityTerm) + nodePodsToMatchingAntiAffinityTerms := make(map[string][]matchingPodAntiAffinityTerm) + nodeTopologyValuesToMatchingPods := make(map[string][]string) + for _, existingPod := range nodeInfo.PodsWithAffinity() { affinity := existingPod.Spec.Affinity if affinity == nil { @@ -1292,40 +1305,55 @@ func getMatchingAntiAffinityTerms(pod *v1.Pod, nodeInfoMap map[string]*scheduler } if priorityutil.PodMatchesTermsNamespaceAndSelector(pod, namespaces, selector) { existingPodFullName := schedutil.GetPodFullName(existingPod) - nodeResult[existingPodFullName] = append( - nodeResult[existingPodFullName], + nodePodsToMatchingAntiAffinityTerms[existingPodFullName] = append( + nodePodsToMatchingAntiAffinityTerms[existingPodFullName], matchingPodAntiAffinityTerm{term: &term, node: node}) + + if topologyValue, ok := node.Labels[term.TopologyKey]; ok { + nodeTopologyValuesToMatchingPods[topologyValue] = append(nodeTopologyValuesToMatchingPods[topologyValue], existingPodFullName) + } } } } - if len(nodeResult) > 0 { - appendResult(nodeResult) + if len(nodePodsToMatchingAntiAffinityTerms) > 0 { + appendPodsMatchingAntiAffinityTerms(nodePodsToMatchingAntiAffinityTerms) + } + if len(nodeTopologyValuesToMatchingPods) > 0 { + appendTopologyValuesMatchingPods(nodeTopologyValuesToMatchingPods) } } workqueue.Parallelize(16, len(allNodeNames), processNode) - return result, firstError + return podsToMatchingAntiAffinityTerms, topologyValuesToMatchingPods, firstError } -func getMatchingAntiAffinityTermsOfExistingPod(newPod *v1.Pod, existingPod *v1.Pod, node *v1.Node) ([]matchingPodAntiAffinityTerm, error) { - var result []matchingPodAntiAffinityTerm +func getMatchingAntiAffinityTermsOfExistingPod(newPod *v1.Pod, existingPod *v1.Pod, node *v1.Node) ([]matchingPodAntiAffinityTerm, map[string][]string, error) { + var podMatchingTerms []matchingPodAntiAffinityTerm + topologyValuesToMatchingPods := make(map[string][]string) + affinity := existingPod.Spec.Affinity if affinity != nil && affinity.PodAntiAffinity != nil { for _, term := range GetPodAntiAffinityTerms(affinity.PodAntiAffinity) { namespaces := priorityutil.GetNamespacesFromPodAffinityTerm(existingPod, &term) selector, err := metav1.LabelSelectorAsSelector(term.LabelSelector) if err != nil { - return nil, err + return nil, nil, err } if priorityutil.PodMatchesTermsNamespaceAndSelector(newPod, namespaces, selector) { - result = append(result, matchingPodAntiAffinityTerm{term: &term, node: node}) + podMatchingTerms = append(podMatchingTerms, matchingPodAntiAffinityTerm{term: &term, node: node}) + existingPodFullName := schedutil.GetPodFullName(existingPod) + if topologyValue, ok := node.Labels[term.TopologyKey]; ok { + topologyValuesToMatchingPods[topologyValue] = append(topologyValuesToMatchingPods[topologyValue], existingPodFullName) + } } } } - return result, nil + return podMatchingTerms, topologyValuesToMatchingPods, nil } -func (c *PodAffinityChecker) getMatchingAntiAffinityTerms(pod *v1.Pod, allPods []*v1.Pod) (map[string][]matchingPodAntiAffinityTerm, error) { +func (c *PodAffinityChecker) getMatchingAntiAffinityTerms(pod *v1.Pod, allPods []*v1.Pod) (map[string][]matchingPodAntiAffinityTerm, map[string][]string, error) { result := make(map[string][]matchingPodAntiAffinityTerm) + topologyValuesToMatchingPods := make(map[string][]string) + for _, existingPod := range allPods { affinity := existingPod.Spec.Affinity if affinity != nil && affinity.PodAntiAffinity != nil { @@ -1335,19 +1363,22 @@ func (c *PodAffinityChecker) getMatchingAntiAffinityTerms(pod *v1.Pod, allPods [ glog.Errorf("Node not found, %v", existingPod.Spec.NodeName) continue } - return nil, err + return nil, nil, err } - existingPodMatchingTerms, err := getMatchingAntiAffinityTermsOfExistingPod(pod, existingPod, existingPodNode) + existingPodMatchingTerms, podTopologyValuesToMatchingPods, err := getMatchingAntiAffinityTermsOfExistingPod(pod, existingPod, existingPodNode) if err != nil { - return nil, err + return nil, nil, err } if len(existingPodMatchingTerms) > 0 { existingPodFullName := schedutil.GetPodFullName(existingPod) result[existingPodFullName] = existingPodMatchingTerms } + for topologyValue, pods := range podTopologyValuesToMatchingPods { + topologyValuesToMatchingPods[topologyValue] = append(topologyValuesToMatchingPods[topologyValue], pods...) + } } } - return result, nil + return result, topologyValuesToMatchingPods, nil } // Checks if scheduling the pod onto this node would break any anti-affinity @@ -1358,8 +1389,11 @@ func (c *PodAffinityChecker) satisfiesExistingPodsAntiAffinity(pod *v1.Pod, meta return ErrExistingPodsAntiAffinityRulesNotMatch, fmt.Errorf("Node is nil") } var matchingTerms map[string][]matchingPodAntiAffinityTerm + var topologyValuesToMatchingPods map[string][]string + if predicateMeta, ok := meta.(*predicateMetadata); ok { matchingTerms = predicateMeta.matchingAntiAffinityTerms + topologyValuesToMatchingPods = predicateMeta.topologyValueToAntiAffinityPods } else { // Filter out pods whose nodeName is equal to nodeInfo.node.Name, but are not // present in nodeInfo. Pods on other nodes pass the filter. @@ -1369,24 +1403,31 @@ func (c *PodAffinityChecker) satisfiesExistingPodsAntiAffinity(pod *v1.Pod, meta glog.Error(errMessage) return ErrExistingPodsAntiAffinityRulesNotMatch, errors.New(errMessage) } - if matchingTerms, err = c.getMatchingAntiAffinityTerms(pod, filteredPods); err != nil { + if matchingTerms, topologyValuesToMatchingPods, err = c.getMatchingAntiAffinityTerms(pod, filteredPods); err != nil { errMessage := fmt.Sprintf("Failed to get all terms that pod %+v matches, err: %+v", podName(pod), err) glog.Error(errMessage) return ErrExistingPodsAntiAffinityRulesNotMatch, errors.New(errMessage) } } - for _, terms := range matchingTerms { - for i := range terms { - term := &terms[i] - if len(term.term.TopologyKey) == 0 { - errMessage := fmt.Sprintf("Empty topologyKey is not allowed except for PreferredDuringScheduling pod anti-affinity") - glog.Error(errMessage) - return ErrExistingPodsAntiAffinityRulesNotMatch, errors.New(errMessage) - } - if priorityutil.NodesHaveSameTopologyKey(node, term.node, term.term.TopologyKey) { - glog.V(10).Infof("Cannot schedule pod %+v onto node %v,because of PodAntiAffinityTerm %v", - podName(pod), node.Name, term.term) - return ErrExistingPodsAntiAffinityRulesNotMatch, nil + + // Iterate over topology values, to get matching pods and get their matching terms to check for same topolgy key + // currently ignored if predicateMetadata is not precomputed + for _, topologyValue := range node.Labels { + potentialPods := topologyValuesToMatchingPods[topologyValue] + for _, matchingPod := range potentialPods { + podTerms := matchingTerms[matchingPod] + for i := range podTerms { + term := &podTerms[i] + if len(term.term.TopologyKey) == 0 { + errMessage := fmt.Sprintf("Empty topologyKey is not allowed except for PreferredDuringScheduling pod anti-affinity") + glog.Error(errMessage) + return ErrExistingPodsAntiAffinityRulesNotMatch, errors.New(errMessage) + } + if priorityutil.NodesHaveSameTopologyKey(node, term.node, term.term.TopologyKey) { + glog.V(10).Infof("Cannot schedule pod %+v onto node %v,because of PodAntiAffinityTerm %v", + podName(pod), node.Name, term.term) + return ErrExistingPodsAntiAffinityRulesNotMatch, nil + } } } } From f6659e454344775784d30ae15dd6995a62bb7012 Mon Sep 17 00:00:00 2001 From: Ahmad Diaa Date: Tue, 7 Aug 2018 03:17:21 +0200 Subject: [PATCH 2/4] further enhancements removing matchingTerms from metadata --- .../algorithm/predicates/metadata.go | 77 ++++++------ .../algorithm/predicates/metadata_test.go | 31 ++--- .../algorithm/predicates/predicates.go | 111 ++++++++---------- 3 files changed, 102 insertions(+), 117 deletions(-) diff --git a/pkg/scheduler/algorithm/predicates/metadata.go b/pkg/scheduler/algorithm/predicates/metadata.go index 80db0e65e4..ea822ac843 100644 --- a/pkg/scheduler/algorithm/predicates/metadata.go +++ b/pkg/scheduler/algorithm/predicates/metadata.go @@ -38,6 +38,12 @@ type PredicateMetadataFactory struct { podLister algorithm.PodLister } +// AntiAffinityTerm's topology key value used in predicate metadata +type topologyPair struct { + key string + value string +} + // Note that predicateMetadata and matchingPodAntiAffinityTerm need to be declared in the same file // due to the way declarations are processed in predicate declaration unit tests. type matchingPodAntiAffinityTerm struct { @@ -52,11 +58,11 @@ type predicateMetadata struct { podBestEffort bool podRequest *schedulercache.Resource podPorts []*v1.ContainerPort - //key is a pod full name with the anti-affinity rules. - matchingAntiAffinityTerms map[string][]matchingPodAntiAffinityTerm - // A map of antiffinity terms' topology ke values to the pods' names + // A map of antiffinity terms' topology pairs to the pods' // that can potentially match the affinity rules of the pod - topologyValueToAntiAffinityPods map[string][]string + topologyPairToAntiAffinityPods map[topologyPair][]*v1.Pod + // Reverse map for topologyPairToAntiAffinityPods to reduce deletion time + antiAffinityPodToTopologyPairs map[string][]topologyPair // A map of node name to a list of Pods on the node that can potentially match // the affinity rules of the "pod". nodeNameToMatchingAffinityPods map[string][]*v1.Pod @@ -116,7 +122,7 @@ func (pfactory *PredicateMetadataFactory) GetMetadata(pod *v1.Pod, nodeNameToInf if pod == nil { return nil } - matchingTerms, topologyValues, err := getMatchingAntiAffinityTerms(pod, nodeNameToInfoMap) + podToTopolgyPair, topologyPairToPods, err := getMatchingTopologyPairs(pod, nodeNameToInfoMap) if err != nil { return nil } @@ -130,10 +136,10 @@ func (pfactory *PredicateMetadataFactory) GetMetadata(pod *v1.Pod, nodeNameToInf podBestEffort: isPodBestEffort(pod), podRequest: GetResourceRequest(pod), podPorts: schedutil.GetContainerPorts(pod), - matchingAntiAffinityTerms: matchingTerms, nodeNameToMatchingAffinityPods: affinityPods, nodeNameToMatchingAntiAffinityPods: antiAffinityPods, - topologyValueToAntiAffinityPods: topologyValues, + topologyPairToAntiAffinityPods: topologyPairToPods, + antiAffinityPodToTopologyPairs: podToTopolgyPair, } for predicateName, precomputeFunc := range predicateMetadataProducers { glog.V(10).Infof("Precompute: %v", predicateName) @@ -149,23 +155,22 @@ func (meta *predicateMetadata) RemovePod(deletedPod *v1.Pod) error { if deletedPodFullName == schedutil.GetPodFullName(meta.pod) { return fmt.Errorf("deletedPod and meta.pod must not be the same") } - - // Delete pod from matching topology values map - for _, term := range meta.matchingAntiAffinityTerms[deletedPodFullName] { - if topologyValue, ok := term.node.Labels[term.term.TopologyKey]; ok { - for index, podName := range meta.topologyValueToAntiAffinityPods[topologyValue] { - if podName == deletedPodFullName { - podsList := meta.topologyValueToAntiAffinityPods[topologyValue] - meta.topologyValueToAntiAffinityPods[topologyValue] = append(podsList[:index], - podsList[index+1:]...) - break + // Delete pod from matching topology pairs map + for _, pair := range meta.antiAffinityPodToTopologyPairs[deletedPodFullName] { + for index, pod := range meta.topologyPairToAntiAffinityPods[pair] { + if schedutil.GetPodFullName(pod) == deletedPodFullName { + podsList := meta.topologyPairToAntiAffinityPods[pair] + podsList[index] = podsList[len(podsList)-1] + if len(podsList) <= 1 { + delete(meta.topologyPairToAntiAffinityPods, pair) + } else { + meta.topologyPairToAntiAffinityPods[pair] = podsList[:len(podsList)-1] } + break } } } - - // Delete any anti-affinity rule from the deletedPod. - delete(meta.matchingAntiAffinityTerms, deletedPodFullName) + delete(meta.antiAffinityPodToTopologyPairs, deletedPodFullName) // Delete pod from the matching affinity or anti-affinity pods if exists. affinity := meta.pod.Spec.Affinity podNodeName := deletedPod.Spec.NodeName @@ -222,21 +227,16 @@ func (meta *predicateMetadata) AddPod(addedPod *v1.Pod, nodeInfo *schedulercache return fmt.Errorf("invalid node in nodeInfo") } // Add matching anti-affinity terms of the addedPod to the map. - podMatchingTerms, podTopologyValuesToMatchingPods, err := getMatchingAntiAffinityTermsOfExistingPod(meta.pod, addedPod, nodeInfo.Node()) + matchingPodToTopologyPairs, podTopologyPairToMatchingPods, err := getMatchingTopologyPairsOfExistingPod(meta.pod, addedPod, nodeInfo.Node()) if err != nil { return err } - if len(podMatchingTerms) > 0 { - existingTerms, found := meta.matchingAntiAffinityTerms[addedPodFullName] - if found { - meta.matchingAntiAffinityTerms[addedPodFullName] = append(existingTerms, - podMatchingTerms...) - } else { - meta.matchingAntiAffinityTerms[addedPodFullName] = podMatchingTerms + if len(matchingPodToTopologyPairs) > 0 { + for pair, pods := range podTopologyPairToMatchingPods { + meta.topologyPairToAntiAffinityPods[pair] = append(meta.topologyPairToAntiAffinityPods[pair], pods...) } - - for topologyValue, pods := range podTopologyValuesToMatchingPods { - meta.topologyValueToAntiAffinityPods[topologyValue] = append(meta.topologyValueToAntiAffinityPods[topologyValue], pods...) + for pod, pairs := range matchingPodToTopologyPairs { + meta.antiAffinityPodToTopologyPairs[pod] = append(meta.antiAffinityPodToTopologyPairs[pod], pairs...) } } // Add the pod to nodeNameToMatchingAffinityPods and nodeNameToMatchingAntiAffinityPods if needed. @@ -291,10 +291,6 @@ func (meta *predicateMetadata) ShallowCopy() algorithm.PredicateMetadata { ignoredExtendedResources: meta.ignoredExtendedResources, } newPredMeta.podPorts = append([]*v1.ContainerPort(nil), meta.podPorts...) - newPredMeta.matchingAntiAffinityTerms = map[string][]matchingPodAntiAffinityTerm{} - for k, v := range meta.matchingAntiAffinityTerms { - newPredMeta.matchingAntiAffinityTerms[k] = append([]matchingPodAntiAffinityTerm(nil), v...) - } newPredMeta.nodeNameToMatchingAffinityPods = make(map[string][]*v1.Pod) for k, v := range meta.nodeNameToMatchingAffinityPods { newPredMeta.nodeNameToMatchingAffinityPods[k] = append([]*v1.Pod(nil), v...) @@ -303,15 +299,18 @@ func (meta *predicateMetadata) ShallowCopy() algorithm.PredicateMetadata { for k, v := range meta.nodeNameToMatchingAntiAffinityPods { newPredMeta.nodeNameToMatchingAntiAffinityPods[k] = append([]*v1.Pod(nil), v...) } - newPredMeta.topologyValueToAntiAffinityPods = make(map[string][]string) - for k, v := range meta.topologyValueToAntiAffinityPods { - newPredMeta.topologyValueToAntiAffinityPods[k] = append([]string(nil), v...) + newPredMeta.topologyPairToAntiAffinityPods = make(map[topologyPair][]*v1.Pod) + for k, v := range meta.topologyPairToAntiAffinityPods { + newPredMeta.topologyPairToAntiAffinityPods[k] = append([]*v1.Pod(nil), v...) + } + newPredMeta.antiAffinityPodToTopologyPairs = make(map[string][]topologyPair) + for k, v := range meta.antiAffinityPodToTopologyPairs { + newPredMeta.antiAffinityPodToTopologyPairs[k] = append([]topologyPair(nil), v...) } newPredMeta.serviceAffinityMatchingPodServices = append([]*v1.Service(nil), meta.serviceAffinityMatchingPodServices...) newPredMeta.serviceAffinityMatchingPodList = append([]*v1.Pod(nil), meta.serviceAffinityMatchingPodList...) - return (algorithm.PredicateMetadata)(newPredMeta) } diff --git a/pkg/scheduler/algorithm/predicates/metadata_test.go b/pkg/scheduler/algorithm/predicates/metadata_test.go index 84e7cc55c3..287a94d10c 100644 --- a/pkg/scheduler/algorithm/predicates/metadata_test.go +++ b/pkg/scheduler/algorithm/predicates/metadata_test.go @@ -113,11 +113,6 @@ func predicateMetadataEquivalent(meta1, meta2 *predicateMetadata) error { for !reflect.DeepEqual(meta1.podPorts, meta2.podPorts) { return fmt.Errorf("podPorts are not equal") } - sortAntiAffinityTerms(meta1.matchingAntiAffinityTerms) - sortAntiAffinityTerms(meta2.matchingAntiAffinityTerms) - if !reflect.DeepEqual(meta1.matchingAntiAffinityTerms, meta2.matchingAntiAffinityTerms) { - return fmt.Errorf("matchingAntiAffinityTerms are not euqal") - } sortNodePodMap(meta1.nodeNameToMatchingAffinityPods) sortNodePodMap(meta2.nodeNameToMatchingAffinityPods) if !reflect.DeepEqual(meta1.nodeNameToMatchingAffinityPods, meta2.nodeNameToMatchingAffinityPods) { @@ -465,19 +460,25 @@ func TestPredicateMetadata_ShallowCopy(t *testing.T) { HostIP: "1.2.3.4", }, }, - matchingAntiAffinityTerms: map[string][]matchingPodAntiAffinityTerm{ - "term1": { - { - term: &v1.PodAffinityTerm{TopologyKey: "node"}, - node: &v1.Node{ - ObjectMeta: metav1.ObjectMeta{Name: "machine1"}, - }, + topologyPairToAntiAffinityPods: map[topologyPair][]*v1.Pod{ + {key: "name", value: "machine1"}: { + &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p2", Labels: selector1}, + Spec: v1.PodSpec{NodeName: "nodeC"}, + }, + }, + {key: "name", value: "machine2"}: { + &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p1", Labels: selector1}, + Spec: v1.PodSpec{NodeName: "nodeA"}, }, }, }, - topologyValueToAntiAffinityPods: map[string][]string{ - "machine1": {"p1", "p2"}, - "machine2": {"p3"}, + antiAffinityPodToTopologyPairs: map[string][]topologyPair{ + "p2": { + topologyPair{key: "name", value: "machine1"}, + }, + "p1": { + topologyPair{key: "name", value: "machine2"}, + }, }, nodeNameToMatchingAffinityPods: map[string][]*v1.Pod{ "nodeA": { diff --git a/pkg/scheduler/algorithm/predicates/predicates.go b/pkg/scheduler/algorithm/predicates/predicates.go index 477bd0753b..b280c71e11 100644 --- a/pkg/scheduler/algorithm/predicates/predicates.go +++ b/pkg/scheduler/algorithm/predicates/predicates.go @@ -1246,7 +1246,7 @@ func GetPodAntiAffinityTerms(podAntiAffinity *v1.PodAntiAffinity) (terms []v1.Po return terms } -func getMatchingAntiAffinityTerms(pod *v1.Pod, nodeInfoMap map[string]*schedulercache.NodeInfo) (map[string][]matchingPodAntiAffinityTerm, map[string][]string, error) { +func getMatchingTopologyPairs(pod *v1.Pod, nodeInfoMap map[string]*schedulercache.NodeInfo) (map[string][]topologyPair, map[topologyPair][]*v1.Pod, error) { allNodeNames := make([]string, 0, len(nodeInfoMap)) for name := range nodeInfoMap { allNodeNames = append(allNodeNames, name) @@ -1254,25 +1254,25 @@ func getMatchingAntiAffinityTerms(pod *v1.Pod, nodeInfoMap map[string]*scheduler var lock sync.Mutex var firstError error - podsToMatchingAntiAffinityTerms := make(map[string][]matchingPodAntiAffinityTerm) - topologyValuesToMatchingPods := make(map[string][]string) - appendPodsMatchingAntiAffinityTerms := func(toAppend map[string][]matchingPodAntiAffinityTerm) { + topologyPairToMatchingPods := make(map[topologyPair][]*v1.Pod) + matchingPodToTopologyPair := make(map[string][]topologyPair) + + appendTopologyPairToMatchingPods := func(toAppend map[topologyPair][]*v1.Pod) { lock.Lock() defer lock.Unlock() - for uid, terms := range toAppend { - podsToMatchingAntiAffinityTerms[uid] = append(podsToMatchingAntiAffinityTerms[uid], terms...) + for pair, pods := range toAppend { + topologyPairToMatchingPods[pair] = append(topologyPairToMatchingPods[pair], pods...) } } - appendTopologyValuesMatchingPods := func(toAppend map[string][]string) { + appendMatchingPodToTopologyPair := func(toAppend map[string][]topologyPair) { lock.Lock() defer lock.Unlock() - for topologyValue, pods := range toAppend { - topologyValuesToMatchingPods[topologyValue] = append(topologyValuesToMatchingPods[topologyValue], pods...) + for pod, pairs := range toAppend { + matchingPodToTopologyPair[pod] = append(matchingPodToTopologyPair[pod], pairs...) } } - catchError := func(err error) { lock.Lock() defer lock.Unlock() @@ -1288,9 +1288,8 @@ func getMatchingAntiAffinityTerms(pod *v1.Pod, nodeInfoMap map[string]*scheduler catchError(fmt.Errorf("node not found")) return } - nodePodsToMatchingAntiAffinityTerms := make(map[string][]matchingPodAntiAffinityTerm) - nodeTopologyValuesToMatchingPods := make(map[string][]string) - + nodeTopologyPairToMatchingPods := make(map[topologyPair][]*v1.Pod) + nodeMatchingPodToTopologyPairs := make(map[string][]topologyPair) for _, existingPod := range nodeInfo.PodsWithAffinity() { affinity := existingPod.Spec.Affinity if affinity == nil { @@ -1304,31 +1303,27 @@ func getMatchingAntiAffinityTerms(pod *v1.Pod, nodeInfoMap map[string]*scheduler return } if priorityutil.PodMatchesTermsNamespaceAndSelector(pod, namespaces, selector) { - existingPodFullName := schedutil.GetPodFullName(existingPod) - nodePodsToMatchingAntiAffinityTerms[existingPodFullName] = append( - nodePodsToMatchingAntiAffinityTerms[existingPodFullName], - matchingPodAntiAffinityTerm{term: &term, node: node}) - if topologyValue, ok := node.Labels[term.TopologyKey]; ok { - nodeTopologyValuesToMatchingPods[topologyValue] = append(nodeTopologyValuesToMatchingPods[topologyValue], existingPodFullName) + pair := topologyPair{key: term.TopologyKey, value: topologyValue} + nodeTopologyPairToMatchingPods[pair] = append(nodeTopologyPairToMatchingPods[pair], existingPod) + existingPodFullName := schedutil.GetPodFullName(existingPod) + nodeMatchingPodToTopologyPairs[existingPodFullName] = append(nodeMatchingPodToTopologyPairs[existingPodFullName], pair) } } } } - if len(nodePodsToMatchingAntiAffinityTerms) > 0 { - appendPodsMatchingAntiAffinityTerms(nodePodsToMatchingAntiAffinityTerms) - } - if len(nodeTopologyValuesToMatchingPods) > 0 { - appendTopologyValuesMatchingPods(nodeTopologyValuesToMatchingPods) + if len(nodeTopologyPairToMatchingPods) > 0 { + appendTopologyPairToMatchingPods(nodeTopologyPairToMatchingPods) + appendMatchingPodToTopologyPair(nodeMatchingPodToTopologyPairs) } } workqueue.Parallelize(16, len(allNodeNames), processNode) - return podsToMatchingAntiAffinityTerms, topologyValuesToMatchingPods, firstError + return matchingPodToTopologyPair, topologyPairToMatchingPods, firstError } -func getMatchingAntiAffinityTermsOfExistingPod(newPod *v1.Pod, existingPod *v1.Pod, node *v1.Node) ([]matchingPodAntiAffinityTerm, map[string][]string, error) { - var podMatchingTerms []matchingPodAntiAffinityTerm - topologyValuesToMatchingPods := make(map[string][]string) +func getMatchingTopologyPairsOfExistingPod(newPod *v1.Pod, existingPod *v1.Pod, node *v1.Node) (map[string][]topologyPair, map[topologyPair][]*v1.Pod, error) { + topologyPairToMatchingPods := make(map[topologyPair][]*v1.Pod) + matchingPodToTopologyPairs := make(map[string][]topologyPair) affinity := existingPod.Spec.Affinity if affinity != nil && affinity.PodAntiAffinity != nil { @@ -1339,21 +1334,21 @@ func getMatchingAntiAffinityTermsOfExistingPod(newPod *v1.Pod, existingPod *v1.P return nil, nil, err } if priorityutil.PodMatchesTermsNamespaceAndSelector(newPod, namespaces, selector) { - podMatchingTerms = append(podMatchingTerms, matchingPodAntiAffinityTerm{term: &term, node: node}) - existingPodFullName := schedutil.GetPodFullName(existingPod) if topologyValue, ok := node.Labels[term.TopologyKey]; ok { - topologyValuesToMatchingPods[topologyValue] = append(topologyValuesToMatchingPods[topologyValue], existingPodFullName) + pair := topologyPair{key: term.TopologyKey, value: topologyValue} + topologyPairToMatchingPods[pair] = append(topologyPairToMatchingPods[pair], existingPod) + existingPodFullName := schedutil.GetPodFullName(existingPod) + matchingPodToTopologyPairs[existingPodFullName] = append(matchingPodToTopologyPairs[existingPodFullName], pair) } } } } - return podMatchingTerms, topologyValuesToMatchingPods, nil + return matchingPodToTopologyPairs, topologyPairToMatchingPods, nil } +func (c *PodAffinityChecker) getMatchingAntiAffinityTopologyPairs(pod *v1.Pod, allPods []*v1.Pod) (map[string][]topologyPair, map[topologyPair][]*v1.Pod, error) { -func (c *PodAffinityChecker) getMatchingAntiAffinityTerms(pod *v1.Pod, allPods []*v1.Pod) (map[string][]matchingPodAntiAffinityTerm, map[string][]string, error) { - result := make(map[string][]matchingPodAntiAffinityTerm) - topologyValuesToMatchingPods := make(map[string][]string) - + topologyPairToMatchingPods := make(map[topologyPair][]*v1.Pod) + matchingPodToTopologyPairs := make(map[string][]topologyPair) for _, existingPod := range allPods { affinity := existingPod.Spec.Affinity if affinity != nil && affinity.PodAntiAffinity != nil { @@ -1365,20 +1360,19 @@ func (c *PodAffinityChecker) getMatchingAntiAffinityTerms(pod *v1.Pod, allPods [ } return nil, nil, err } - existingPodMatchingTerms, podTopologyValuesToMatchingPods, err := getMatchingAntiAffinityTermsOfExistingPod(pod, existingPod, existingPodNode) + existingPodTopologyTerms, podTopologyPairToMatchingPods, err := getMatchingTopologyPairsOfExistingPod(pod, existingPod, existingPodNode) if err != nil { return nil, nil, err } - if len(existingPodMatchingTerms) > 0 { - existingPodFullName := schedutil.GetPodFullName(existingPod) - result[existingPodFullName] = existingPodMatchingTerms + for pair, pods := range podTopologyPairToMatchingPods { + topologyPairToMatchingPods[pair] = append(topologyPairToMatchingPods[pair], pods...) } - for topologyValue, pods := range podTopologyValuesToMatchingPods { - topologyValuesToMatchingPods[topologyValue] = append(topologyValuesToMatchingPods[topologyValue], pods...) + for pod, pairs := range existingPodTopologyTerms { + matchingPodToTopologyPairs[pod] = append(matchingPodToTopologyPairs[pod], pairs...) } } } - return result, topologyValuesToMatchingPods, nil + return matchingPodToTopologyPairs, topologyPairToMatchingPods, nil } // Checks if scheduling the pod onto this node would break any anti-affinity @@ -1388,12 +1382,10 @@ func (c *PodAffinityChecker) satisfiesExistingPodsAntiAffinity(pod *v1.Pod, meta if node == nil { return ErrExistingPodsAntiAffinityRulesNotMatch, fmt.Errorf("Node is nil") } - var matchingTerms map[string][]matchingPodAntiAffinityTerm - var topologyValuesToMatchingPods map[string][]string + var topologyPairToMatchingPods map[topologyPair][]*v1.Pod if predicateMeta, ok := meta.(*predicateMetadata); ok { - matchingTerms = predicateMeta.matchingAntiAffinityTerms - topologyValuesToMatchingPods = predicateMeta.topologyValueToAntiAffinityPods + topologyPairToMatchingPods = predicateMeta.topologyPairToAntiAffinityPods } else { // Filter out pods whose nodeName is equal to nodeInfo.node.Name, but are not // present in nodeInfo. Pods on other nodes pass the filter. @@ -1403,29 +1395,22 @@ func (c *PodAffinityChecker) satisfiesExistingPodsAntiAffinity(pod *v1.Pod, meta glog.Error(errMessage) return ErrExistingPodsAntiAffinityRulesNotMatch, errors.New(errMessage) } - if matchingTerms, topologyValuesToMatchingPods, err = c.getMatchingAntiAffinityTerms(pod, filteredPods); err != nil { + if _, topologyPairToMatchingPods, err = c.getMatchingAntiAffinityTopologyPairs(pod, filteredPods); err != nil { errMessage := fmt.Sprintf("Failed to get all terms that pod %+v matches, err: %+v", podName(pod), err) glog.Error(errMessage) return ErrExistingPodsAntiAffinityRulesNotMatch, errors.New(errMessage) } } - // Iterate over topology values, to get matching pods and get their matching terms to check for same topolgy key - // currently ignored if predicateMetadata is not precomputed - for _, topologyValue := range node.Labels { - potentialPods := topologyValuesToMatchingPods[topologyValue] - for _, matchingPod := range potentialPods { - podTerms := matchingTerms[matchingPod] - for i := range podTerms { - term := &podTerms[i] - if len(term.term.TopologyKey) == 0 { - errMessage := fmt.Sprintf("Empty topologyKey is not allowed except for PreferredDuringScheduling pod anti-affinity") - glog.Error(errMessage) - return ErrExistingPodsAntiAffinityRulesNotMatch, errors.New(errMessage) - } - if priorityutil.NodesHaveSameTopologyKey(node, term.node, term.term.TopologyKey) { + // Iterate over topology topology pairs to get any of the pods being affected by + // the scheduled pod anti-affinity rules + for topologyKey, topologyValue := range node.Labels { + if violatedPods, ok := topologyPairToMatchingPods[topologyPair{key: topologyKey, value: topologyValue}]; ok { + affinity := violatedPods[0].Spec.Affinity + for _, term := range GetPodAntiAffinityTerms(affinity.PodAntiAffinity) { + if term.TopologyKey == topologyKey { glog.V(10).Infof("Cannot schedule pod %+v onto node %v,because of PodAntiAffinityTerm %v", - podName(pod), node.Name, term.term) + podName(pod), node.Name, term) return ErrExistingPodsAntiAffinityRulesNotMatch, nil } } From 0f4c3064fd1abcb76a4d823d6454f55e1a62b119 Mon Sep 17 00:00:00 2001 From: Ahmad Diaa Date: Fri, 10 Aug 2018 00:06:45 +0200 Subject: [PATCH 3/4] created struct for topologyPairs maps --- .../algorithm/predicates/metadata.go | 110 +++++++++++------- .../algorithm/predicates/metadata_test.go | 89 +++++++------- .../algorithm/predicates/predicates.go | 94 ++++++--------- 3 files changed, 145 insertions(+), 148 deletions(-) diff --git a/pkg/scheduler/algorithm/predicates/metadata.go b/pkg/scheduler/algorithm/predicates/metadata.go index ea822ac843..0a87a17a68 100644 --- a/pkg/scheduler/algorithm/predicates/metadata.go +++ b/pkg/scheduler/algorithm/predicates/metadata.go @@ -51,6 +51,13 @@ type matchingPodAntiAffinityTerm struct { node *v1.Node } +// topologyPairsMaps keeps topologyPairToAntiAffinityPods and antiAffinityPodToTopologyPairs in sync +// as they are the inverse of each others. +type topologyPairsMaps struct { + topologyPairToPods map[topologyPair][]*v1.Pod + podToTopologyPairs map[string][]topologyPair +} + // NOTE: When new fields are added/removed or logic is changed, please make sure that // RemovePod, AddPod, and ShallowCopy functions are updated to work with the new changes. type predicateMetadata struct { @@ -58,11 +65,8 @@ type predicateMetadata struct { podBestEffort bool podRequest *schedulercache.Resource podPorts []*v1.ContainerPort - // A map of antiffinity terms' topology pairs to the pods' - // that can potentially match the affinity rules of the pod - topologyPairToAntiAffinityPods map[topologyPair][]*v1.Pod - // Reverse map for topologyPairToAntiAffinityPods to reduce deletion time - antiAffinityPodToTopologyPairs map[string][]topologyPair + + topologyPairsAntiAffinityPodsMap *topologyPairsMaps // A map of node name to a list of Pods on the node that can potentially match // the affinity rules of the "pod". nodeNameToMatchingAffinityPods map[string][]*v1.Pod @@ -122,7 +126,7 @@ func (pfactory *PredicateMetadataFactory) GetMetadata(pod *v1.Pod, nodeNameToInf if pod == nil { return nil } - podToTopolgyPair, topologyPairToPods, err := getMatchingTopologyPairs(pod, nodeNameToInfoMap) + topologyPairsMaps, err := getMatchingTopologyPairs(pod, nodeNameToInfoMap) if err != nil { return nil } @@ -138,8 +142,7 @@ func (pfactory *PredicateMetadataFactory) GetMetadata(pod *v1.Pod, nodeNameToInf podPorts: schedutil.GetContainerPorts(pod), nodeNameToMatchingAffinityPods: affinityPods, nodeNameToMatchingAntiAffinityPods: antiAffinityPods, - topologyPairToAntiAffinityPods: topologyPairToPods, - antiAffinityPodToTopologyPairs: podToTopolgyPair, + topologyPairsAntiAffinityPodsMap: topologyPairsMaps, } for predicateName, precomputeFunc := range predicateMetadataProducers { glog.V(10).Infof("Precompute: %v", predicateName) @@ -148,6 +151,47 @@ func (pfactory *PredicateMetadataFactory) GetMetadata(pod *v1.Pod, nodeNameToInf return predicateMetadata } +func (topologyPairsMaps *topologyPairsMaps) AddTopologyPair(pair topologyPair, pod *v1.Pod) { + found := false + for _, existingPod := range topologyPairsMaps.topologyPairToPods[pair] { + if existingPod == pod { + found = true + break + } + } + if !found { + topologyPairsMaps.topologyPairToPods[pair] = append(topologyPairsMaps.topologyPairToPods[pair], pod) + topologyPairsMaps.podToTopologyPairs[schedutil.GetPodFullName(pod)] = append(topologyPairsMaps.podToTopologyPairs[schedutil.GetPodFullName(pod)], pair) + } +} + +func (topologyPairsMaps *topologyPairsMaps) RemovePod(podName string) { + for _, pair := range topologyPairsMaps.podToTopologyPairs[podName] { + for index, pod := range topologyPairsMaps.topologyPairToPods[pair] { + if schedutil.GetPodFullName(pod) == podName { + podsList := topologyPairsMaps.topologyPairToPods[pair] + podsList[index] = podsList[len(podsList)-1] + if len(podsList) <= 1 { + delete(topologyPairsMaps.topologyPairToPods, pair) + } else { + topologyPairsMaps.topologyPairToPods[pair] = podsList[:len(podsList)-1] + } + break + } + } + } + delete(topologyPairsMaps.podToTopologyPairs, podName) +} + +func (topologyPairsMaps *topologyPairsMaps) appendMaps(toAppend *topologyPairsMaps) { + for pod, pairs := range toAppend.podToTopologyPairs { + topologyPairsMaps.podToTopologyPairs[pod] = append(topologyPairsMaps.podToTopologyPairs[pod], pairs...) + } + for pair, pods := range toAppend.topologyPairToPods { + topologyPairsMaps.topologyPairToPods[pair] = append(topologyPairsMaps.topologyPairToPods[pair], pods...) + } +} + // RemovePod changes predicateMetadata assuming that the given `deletedPod` is // deleted from the system. func (meta *predicateMetadata) RemovePod(deletedPod *v1.Pod) error { @@ -155,22 +199,7 @@ func (meta *predicateMetadata) RemovePod(deletedPod *v1.Pod) error { if deletedPodFullName == schedutil.GetPodFullName(meta.pod) { return fmt.Errorf("deletedPod and meta.pod must not be the same") } - // Delete pod from matching topology pairs map - for _, pair := range meta.antiAffinityPodToTopologyPairs[deletedPodFullName] { - for index, pod := range meta.topologyPairToAntiAffinityPods[pair] { - if schedutil.GetPodFullName(pod) == deletedPodFullName { - podsList := meta.topologyPairToAntiAffinityPods[pair] - podsList[index] = podsList[len(podsList)-1] - if len(podsList) <= 1 { - delete(meta.topologyPairToAntiAffinityPods, pair) - } else { - meta.topologyPairToAntiAffinityPods[pair] = podsList[:len(podsList)-1] - } - break - } - } - } - delete(meta.antiAffinityPodToTopologyPairs, deletedPodFullName) + meta.topologyPairsAntiAffinityPodsMap.RemovePod(deletedPodFullName) // Delete pod from the matching affinity or anti-affinity pods if exists. affinity := meta.pod.Spec.Affinity podNodeName := deletedPod.Spec.NodeName @@ -227,17 +256,12 @@ func (meta *predicateMetadata) AddPod(addedPod *v1.Pod, nodeInfo *schedulercache return fmt.Errorf("invalid node in nodeInfo") } // Add matching anti-affinity terms of the addedPod to the map. - matchingPodToTopologyPairs, podTopologyPairToMatchingPods, err := getMatchingTopologyPairsOfExistingPod(meta.pod, addedPod, nodeInfo.Node()) + topologyPairsMaps, err := getMatchingTopologyPairsOfExistingPod(meta.pod, addedPod, nodeInfo.Node()) if err != nil { return err } - if len(matchingPodToTopologyPairs) > 0 { - for pair, pods := range podTopologyPairToMatchingPods { - meta.topologyPairToAntiAffinityPods[pair] = append(meta.topologyPairToAntiAffinityPods[pair], pods...) - } - for pod, pairs := range matchingPodToTopologyPairs { - meta.antiAffinityPodToTopologyPairs[pod] = append(meta.antiAffinityPodToTopologyPairs[pod], pairs...) - } + if len(topologyPairsMaps.podToTopologyPairs) > 0 { + meta.topologyPairsAntiAffinityPodsMap.appendMaps(topologyPairsMaps) } // Add the pod to nodeNameToMatchingAffinityPods and nodeNameToMatchingAntiAffinityPods if needed. affinity := meta.pod.Spec.Affinity @@ -284,11 +308,12 @@ func (meta *predicateMetadata) AddPod(addedPod *v1.Pod, nodeInfo *schedulercache // its maps and slices, but it does not copy the contents of pointer values. func (meta *predicateMetadata) ShallowCopy() algorithm.PredicateMetadata { newPredMeta := &predicateMetadata{ - pod: meta.pod, - podBestEffort: meta.podBestEffort, - podRequest: meta.podRequest, - serviceAffinityInUse: meta.serviceAffinityInUse, - ignoredExtendedResources: meta.ignoredExtendedResources, + pod: meta.pod, + podBestEffort: meta.podBestEffort, + podRequest: meta.podRequest, + serviceAffinityInUse: meta.serviceAffinityInUse, + ignoredExtendedResources: meta.ignoredExtendedResources, + topologyPairsAntiAffinityPodsMap: meta.topologyPairsAntiAffinityPodsMap, } newPredMeta.podPorts = append([]*v1.ContainerPort(nil), meta.podPorts...) newPredMeta.nodeNameToMatchingAffinityPods = make(map[string][]*v1.Pod) @@ -299,14 +324,9 @@ func (meta *predicateMetadata) ShallowCopy() algorithm.PredicateMetadata { for k, v := range meta.nodeNameToMatchingAntiAffinityPods { newPredMeta.nodeNameToMatchingAntiAffinityPods[k] = append([]*v1.Pod(nil), v...) } - newPredMeta.topologyPairToAntiAffinityPods = make(map[topologyPair][]*v1.Pod) - for k, v := range meta.topologyPairToAntiAffinityPods { - newPredMeta.topologyPairToAntiAffinityPods[k] = append([]*v1.Pod(nil), v...) - } - newPredMeta.antiAffinityPodToTopologyPairs = make(map[string][]topologyPair) - for k, v := range meta.antiAffinityPodToTopologyPairs { - newPredMeta.antiAffinityPodToTopologyPairs[k] = append([]topologyPair(nil), v...) - } + newPredMeta.topologyPairsAntiAffinityPodsMap = &topologyPairsMaps{topologyPairToPods: make(map[topologyPair][]*v1.Pod), + podToTopologyPairs: make(map[string][]topologyPair)} + newPredMeta.topologyPairsAntiAffinityPodsMap.appendMaps(meta.topologyPairsAntiAffinityPodsMap) newPredMeta.serviceAffinityMatchingPodServices = append([]*v1.Service(nil), meta.serviceAffinityMatchingPodServices...) newPredMeta.serviceAffinityMatchingPodList = append([]*v1.Pod(nil), diff --git a/pkg/scheduler/algorithm/predicates/metadata_test.go b/pkg/scheduler/algorithm/predicates/metadata_test.go index 287a94d10c..0fc116e8a1 100644 --- a/pkg/scheduler/algorithm/predicates/metadata_test.go +++ b/pkg/scheduler/algorithm/predicates/metadata_test.go @@ -28,39 +28,30 @@ import ( schedulertesting "k8s.io/kubernetes/pkg/scheduler/testing" ) -// sortableAntiAffinityTerms lets us to sort anti-affinity terms. -type sortableAntiAffinityTerms []matchingPodAntiAffinityTerm +// sortableTopologyPairs lets us sort topology pairs +type sortableTopologyPairs []topologyPair -// Less establishes some ordering between two matchingPodAntiAffinityTerms for -// sorting. -func (s sortableAntiAffinityTerms) Less(i, j int) bool { +// Less establishes some ordering between two topologyPairs for sorting. +func (s sortableTopologyPairs) Less(i, j int) bool { t1, t2 := s[i], s[j] - if t1.node.Name != t2.node.Name { - return t1.node.Name < t2.node.Name - } - if len(t1.term.Namespaces) != len(t2.term.Namespaces) { - return len(t1.term.Namespaces) < len(t2.term.Namespaces) - } - if t1.term.TopologyKey != t2.term.TopologyKey { - return t1.term.TopologyKey < t2.term.TopologyKey - } - if len(t1.term.LabelSelector.MatchLabels) != len(t2.term.LabelSelector.MatchLabels) { - return len(t1.term.LabelSelector.MatchLabels) < len(t2.term.LabelSelector.MatchLabels) - } - return false -} -func (s sortableAntiAffinityTerms) Len() int { return len(s) } -func (s sortableAntiAffinityTerms) Swap(i, j int) { - s[i], s[j] = s[j], s[i] + return t1.key < t2.key || (t1.key == t2.key && t1.value < t2.value) } +func (s sortableTopologyPairs) Len() int { return len(s) } +func (s sortableTopologyPairs) Swap(i, j int) { s[i], s[j] = s[j], s[i] } -var _ = sort.Interface(sortableAntiAffinityTerms{}) +var _ = sort.Interface(sortableTopologyPairs{}) -func sortAntiAffinityTerms(terms map[string][]matchingPodAntiAffinityTerm) { - for k, v := range terms { - sortableTerms := sortableAntiAffinityTerms(v) +func sortTopologyPairs(pairs map[string][]topologyPair) { + for k, v := range pairs { + sortableTerms := sortableTopologyPairs(v) sort.Sort(sortableTerms) - terms[k] = sortableTerms + pairs[k] = sortableTerms + } +} +func sortTopologyPairPods(np map[topologyPair][]*v1.Pod) { + for _, pl := range np { + sortablePods := sortablePods(pl) + sort.Sort(sortablePods) } } @@ -123,6 +114,18 @@ func predicateMetadataEquivalent(meta1, meta2 *predicateMetadata) error { if !reflect.DeepEqual(meta1.nodeNameToMatchingAntiAffinityPods, meta2.nodeNameToMatchingAntiAffinityPods) { return fmt.Errorf("nodeNameToMatchingAntiAffinityPods are not euqal") } + sortTopologyPairs(meta1.topologyPairsAntiAffinityPodsMap.podToTopologyPairs) + sortTopologyPairs(meta2.topologyPairsAntiAffinityPodsMap.podToTopologyPairs) + if !reflect.DeepEqual(meta1.topologyPairsAntiAffinityPodsMap.podToTopologyPairs, + meta2.topologyPairsAntiAffinityPodsMap.podToTopologyPairs) { + return fmt.Errorf("topologyPairsAntiAffinityPodsMap.antiAffinityPodToTopologyPairs are not equal") + } + sortTopologyPairPods(meta1.topologyPairsAntiAffinityPodsMap.topologyPairToPods) + sortTopologyPairPods(meta2.topologyPairsAntiAffinityPodsMap.topologyPairToPods) + if !reflect.DeepEqual(meta1.topologyPairsAntiAffinityPodsMap.topologyPairToPods, + meta2.topologyPairsAntiAffinityPodsMap.topologyPairToPods) { + return fmt.Errorf("topologyPairsAntiAffinityPodsMap.topologyPairToAntiAffinityPods are not equal") + } if meta1.serviceAffinityInUse { sortablePods1 := sortablePods(meta1.serviceAffinityMatchingPodList) sort.Sort(sortablePods1) @@ -460,24 +463,26 @@ func TestPredicateMetadata_ShallowCopy(t *testing.T) { HostIP: "1.2.3.4", }, }, - topologyPairToAntiAffinityPods: map[topologyPair][]*v1.Pod{ - {key: "name", value: "machine1"}: { - &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p2", Labels: selector1}, - Spec: v1.PodSpec{NodeName: "nodeC"}, + topologyPairsAntiAffinityPodsMap: &topologyPairsMaps{ + topologyPairToPods: map[topologyPair][]*v1.Pod{ + {key: "name", value: "machine1"}: { + &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p2", Labels: selector1}, + Spec: v1.PodSpec{NodeName: "nodeC"}, + }, + }, + {key: "name", value: "machine2"}: { + &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p1", Labels: selector1}, + Spec: v1.PodSpec{NodeName: "nodeA"}, + }, }, }, - {key: "name", value: "machine2"}: { - &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p1", Labels: selector1}, - Spec: v1.PodSpec{NodeName: "nodeA"}, + podToTopologyPairs: map[string][]topologyPair{ + "p2": { + topologyPair{key: "name", value: "machine1"}, + }, + "p1": { + topologyPair{key: "name", value: "machine2"}, }, - }, - }, - antiAffinityPodToTopologyPairs: map[string][]topologyPair{ - "p2": { - topologyPair{key: "name", value: "machine1"}, - }, - "p1": { - topologyPair{key: "name", value: "machine2"}, }, }, nodeNameToMatchingAffinityPods: map[string][]*v1.Pod{ diff --git a/pkg/scheduler/algorithm/predicates/predicates.go b/pkg/scheduler/algorithm/predicates/predicates.go index b280c71e11..103d5ea7f5 100644 --- a/pkg/scheduler/algorithm/predicates/predicates.go +++ b/pkg/scheduler/algorithm/predicates/predicates.go @@ -1246,7 +1246,7 @@ func GetPodAntiAffinityTerms(podAntiAffinity *v1.PodAntiAffinity) (terms []v1.Po return terms } -func getMatchingTopologyPairs(pod *v1.Pod, nodeInfoMap map[string]*schedulercache.NodeInfo) (map[string][]topologyPair, map[topologyPair][]*v1.Pod, error) { +func getMatchingTopologyPairs(pod *v1.Pod, nodeInfoMap map[string]*schedulercache.NodeInfo) (*topologyPairsMaps, error) { allNodeNames := make([]string, 0, len(nodeInfoMap)) for name := range nodeInfoMap { allNodeNames = append(allNodeNames, name) @@ -1255,23 +1255,13 @@ func getMatchingTopologyPairs(pod *v1.Pod, nodeInfoMap map[string]*schedulercach var lock sync.Mutex var firstError error - topologyPairToMatchingPods := make(map[topologyPair][]*v1.Pod) - matchingPodToTopologyPair := make(map[string][]topologyPair) + topologyMaps := &topologyPairsMaps{topologyPairToPods: make(map[topologyPair][]*v1.Pod), + podToTopologyPairs: make(map[string][]topologyPair)} - appendTopologyPairToMatchingPods := func(toAppend map[topologyPair][]*v1.Pod) { + appendTopologyPairsMaps := func(toAppend *topologyPairsMaps) { lock.Lock() defer lock.Unlock() - for pair, pods := range toAppend { - topologyPairToMatchingPods[pair] = append(topologyPairToMatchingPods[pair], pods...) - } - } - - appendMatchingPodToTopologyPair := func(toAppend map[string][]topologyPair) { - lock.Lock() - defer lock.Unlock() - for pod, pairs := range toAppend { - matchingPodToTopologyPair[pod] = append(matchingPodToTopologyPair[pod], pairs...) - } + topologyMaps.appendMaps(toAppend) } catchError := func(err error) { lock.Lock() @@ -1288,8 +1278,8 @@ func getMatchingTopologyPairs(pod *v1.Pod, nodeInfoMap map[string]*schedulercach catchError(fmt.Errorf("node not found")) return } - nodeTopologyPairToMatchingPods := make(map[topologyPair][]*v1.Pod) - nodeMatchingPodToTopologyPairs := make(map[string][]topologyPair) + nodeTopologyMaps := &topologyPairsMaps{topologyPairToPods: make(map[topologyPair][]*v1.Pod), + podToTopologyPairs: make(map[string][]topologyPair)} for _, existingPod := range nodeInfo.PodsWithAffinity() { affinity := existingPod.Spec.Affinity if affinity == nil { @@ -1305,50 +1295,44 @@ func getMatchingTopologyPairs(pod *v1.Pod, nodeInfoMap map[string]*schedulercach if priorityutil.PodMatchesTermsNamespaceAndSelector(pod, namespaces, selector) { if topologyValue, ok := node.Labels[term.TopologyKey]; ok { pair := topologyPair{key: term.TopologyKey, value: topologyValue} - nodeTopologyPairToMatchingPods[pair] = append(nodeTopologyPairToMatchingPods[pair], existingPod) - existingPodFullName := schedutil.GetPodFullName(existingPod) - nodeMatchingPodToTopologyPairs[existingPodFullName] = append(nodeMatchingPodToTopologyPairs[existingPodFullName], pair) + nodeTopologyMaps.AddTopologyPair(pair, existingPod) } } } - } - if len(nodeTopologyPairToMatchingPods) > 0 { - appendTopologyPairToMatchingPods(nodeTopologyPairToMatchingPods) - appendMatchingPodToTopologyPair(nodeMatchingPodToTopologyPairs) + if len(nodeTopologyMaps.podToTopologyPairs) > 0 { + appendTopologyPairsMaps(nodeTopologyMaps) + } } } workqueue.Parallelize(16, len(allNodeNames), processNode) - return matchingPodToTopologyPair, topologyPairToMatchingPods, firstError + return topologyMaps, firstError } -func getMatchingTopologyPairsOfExistingPod(newPod *v1.Pod, existingPod *v1.Pod, node *v1.Node) (map[string][]topologyPair, map[topologyPair][]*v1.Pod, error) { - topologyPairToMatchingPods := make(map[topologyPair][]*v1.Pod) - matchingPodToTopologyPairs := make(map[string][]topologyPair) - +func getMatchingTopologyPairsOfExistingPod(newPod *v1.Pod, existingPod *v1.Pod, node *v1.Node) (*topologyPairsMaps, error) { + topologyMaps := &topologyPairsMaps{topologyPairToPods: make(map[topologyPair][]*v1.Pod), + podToTopologyPairs: make(map[string][]topologyPair)} affinity := existingPod.Spec.Affinity if affinity != nil && affinity.PodAntiAffinity != nil { for _, term := range GetPodAntiAffinityTerms(affinity.PodAntiAffinity) { namespaces := priorityutil.GetNamespacesFromPodAffinityTerm(existingPod, &term) selector, err := metav1.LabelSelectorAsSelector(term.LabelSelector) if err != nil { - return nil, nil, err + return nil, err } if priorityutil.PodMatchesTermsNamespaceAndSelector(newPod, namespaces, selector) { if topologyValue, ok := node.Labels[term.TopologyKey]; ok { pair := topologyPair{key: term.TopologyKey, value: topologyValue} - topologyPairToMatchingPods[pair] = append(topologyPairToMatchingPods[pair], existingPod) - existingPodFullName := schedutil.GetPodFullName(existingPod) - matchingPodToTopologyPairs[existingPodFullName] = append(matchingPodToTopologyPairs[existingPodFullName], pair) + topologyMaps.AddTopologyPair(pair, existingPod) } } } } - return matchingPodToTopologyPairs, topologyPairToMatchingPods, nil + return topologyMaps, nil } -func (c *PodAffinityChecker) getMatchingAntiAffinityTopologyPairs(pod *v1.Pod, allPods []*v1.Pod) (map[string][]topologyPair, map[topologyPair][]*v1.Pod, error) { +func (c *PodAffinityChecker) getMatchingAntiAffinityTopologyPairs(pod *v1.Pod, allPods []*v1.Pod) (*topologyPairsMaps, error) { + topologyMaps := &topologyPairsMaps{topologyPairToPods: make(map[topologyPair][]*v1.Pod), + podToTopologyPairs: make(map[string][]topologyPair)} - topologyPairToMatchingPods := make(map[topologyPair][]*v1.Pod) - matchingPodToTopologyPairs := make(map[string][]topologyPair) for _, existingPod := range allPods { affinity := existingPod.Spec.Affinity if affinity != nil && affinity.PodAntiAffinity != nil { @@ -1358,21 +1342,16 @@ func (c *PodAffinityChecker) getMatchingAntiAffinityTopologyPairs(pod *v1.Pod, a glog.Errorf("Node not found, %v", existingPod.Spec.NodeName) continue } - return nil, nil, err + return nil, err } - existingPodTopologyTerms, podTopologyPairToMatchingPods, err := getMatchingTopologyPairsOfExistingPod(pod, existingPod, existingPodNode) + existingPodsTopologyMaps, err := getMatchingTopologyPairsOfExistingPod(pod, existingPod, existingPodNode) if err != nil { - return nil, nil, err - } - for pair, pods := range podTopologyPairToMatchingPods { - topologyPairToMatchingPods[pair] = append(topologyPairToMatchingPods[pair], pods...) - } - for pod, pairs := range existingPodTopologyTerms { - matchingPodToTopologyPairs[pod] = append(matchingPodToTopologyPairs[pod], pairs...) + return nil, err } + topologyMaps.appendMaps(existingPodsTopologyMaps) } } - return matchingPodToTopologyPairs, topologyPairToMatchingPods, nil + return topologyMaps, nil } // Checks if scheduling the pod onto this node would break any anti-affinity @@ -1382,10 +1361,9 @@ func (c *PodAffinityChecker) satisfiesExistingPodsAntiAffinity(pod *v1.Pod, meta if node == nil { return ErrExistingPodsAntiAffinityRulesNotMatch, fmt.Errorf("Node is nil") } - var topologyPairToMatchingPods map[topologyPair][]*v1.Pod - + var topologyMaps *topologyPairsMaps if predicateMeta, ok := meta.(*predicateMetadata); ok { - topologyPairToMatchingPods = predicateMeta.topologyPairToAntiAffinityPods + topologyMaps = predicateMeta.topologyPairsAntiAffinityPodsMap } else { // Filter out pods whose nodeName is equal to nodeInfo.node.Name, but are not // present in nodeInfo. Pods on other nodes pass the filter. @@ -1395,25 +1373,19 @@ func (c *PodAffinityChecker) satisfiesExistingPodsAntiAffinity(pod *v1.Pod, meta glog.Error(errMessage) return ErrExistingPodsAntiAffinityRulesNotMatch, errors.New(errMessage) } - if _, topologyPairToMatchingPods, err = c.getMatchingAntiAffinityTopologyPairs(pod, filteredPods); err != nil { + if topologyMaps, err = c.getMatchingAntiAffinityTopologyPairs(pod, filteredPods); err != nil { errMessage := fmt.Sprintf("Failed to get all terms that pod %+v matches, err: %+v", podName(pod), err) glog.Error(errMessage) return ErrExistingPodsAntiAffinityRulesNotMatch, errors.New(errMessage) } } - // Iterate over topology topology pairs to get any of the pods being affected by + // Iterate over topology pairs to get any of the pods being affected by // the scheduled pod anti-affinity rules for topologyKey, topologyValue := range node.Labels { - if violatedPods, ok := topologyPairToMatchingPods[topologyPair{key: topologyKey, value: topologyValue}]; ok { - affinity := violatedPods[0].Spec.Affinity - for _, term := range GetPodAntiAffinityTerms(affinity.PodAntiAffinity) { - if term.TopologyKey == topologyKey { - glog.V(10).Infof("Cannot schedule pod %+v onto node %v,because of PodAntiAffinityTerm %v", - podName(pod), node.Name, term) - return ErrExistingPodsAntiAffinityRulesNotMatch, nil - } - } + if _, ok := topologyMaps.topologyPairToPods[topologyPair{key: topologyKey, value: topologyValue}]; ok { + glog.V(10).Infof("Cannot schedule pod %+v onto node %v", podName(pod), node.Name) + return ErrExistingPodsAntiAffinityRulesNotMatch, nil } } if glog.V(10) { From b4c7d190cd9a6770ea39c6b99a799e45aed1c5ce Mon Sep 17 00:00:00 2001 From: Ahmad Diaa Date: Fri, 17 Aug 2018 21:30:33 +0200 Subject: [PATCH 4/4] using set instead of lists for topologyPairsMaps attributes --- .../algorithm/predicates/metadata.go | 71 +++++++++---------- .../algorithm/predicates/metadata_test.go | 51 +++---------- .../algorithm/predicates/predicates.go | 18 ++--- 3 files changed, 52 insertions(+), 88 deletions(-) diff --git a/pkg/scheduler/algorithm/predicates/metadata.go b/pkg/scheduler/algorithm/predicates/metadata.go index 0a87a17a68..163ad90ba2 100644 --- a/pkg/scheduler/algorithm/predicates/metadata.go +++ b/pkg/scheduler/algorithm/predicates/metadata.go @@ -51,11 +51,15 @@ type matchingPodAntiAffinityTerm struct { node *v1.Node } +type podSet map[*v1.Pod]struct{} + +type topologyPairSet map[topologyPair]struct{} + // topologyPairsMaps keeps topologyPairToAntiAffinityPods and antiAffinityPodToTopologyPairs in sync // as they are the inverse of each others. type topologyPairsMaps struct { - topologyPairToPods map[topologyPair][]*v1.Pod - podToTopologyPairs map[string][]topologyPair + topologyPairToPods map[topologyPair]podSet + podToTopologyPairs map[string]topologyPairSet } // NOTE: When new fields are added/removed or logic is changed, please make sure that @@ -151,44 +155,40 @@ func (pfactory *PredicateMetadataFactory) GetMetadata(pod *v1.Pod, nodeNameToInf return predicateMetadata } -func (topologyPairsMaps *topologyPairsMaps) AddTopologyPair(pair topologyPair, pod *v1.Pod) { - found := false - for _, existingPod := range topologyPairsMaps.topologyPairToPods[pair] { - if existingPod == pod { - found = true - break - } - } - if !found { - topologyPairsMaps.topologyPairToPods[pair] = append(topologyPairsMaps.topologyPairToPods[pair], pod) - topologyPairsMaps.podToTopologyPairs[schedutil.GetPodFullName(pod)] = append(topologyPairsMaps.podToTopologyPairs[schedutil.GetPodFullName(pod)], pair) - } +// returns a pointer to a new topologyPairsMaps +func newTopologyPairsMaps() *topologyPairsMaps { + return &topologyPairsMaps{topologyPairToPods: make(map[topologyPair]podSet), + podToTopologyPairs: make(map[string]topologyPairSet)} } -func (topologyPairsMaps *topologyPairsMaps) RemovePod(podName string) { - for _, pair := range topologyPairsMaps.podToTopologyPairs[podName] { - for index, pod := range topologyPairsMaps.topologyPairToPods[pair] { - if schedutil.GetPodFullName(pod) == podName { - podsList := topologyPairsMaps.topologyPairToPods[pair] - podsList[index] = podsList[len(podsList)-1] - if len(podsList) <= 1 { - delete(topologyPairsMaps.topologyPairToPods, pair) - } else { - topologyPairsMaps.topologyPairToPods[pair] = podsList[:len(podsList)-1] - } - break - } +func (topologyPairsMaps *topologyPairsMaps) addTopologyPair(pair topologyPair, pod *v1.Pod) { + podFullName := schedutil.GetPodFullName(pod) + if topologyPairsMaps.topologyPairToPods[pair] == nil { + topologyPairsMaps.topologyPairToPods[pair] = make(map[*v1.Pod]struct{}) + } + topologyPairsMaps.topologyPairToPods[pair][pod] = struct{}{} + if topologyPairsMaps.podToTopologyPairs[podFullName] == nil { + topologyPairsMaps.podToTopologyPairs[podFullName] = make(map[topologyPair]struct{}) + } + topologyPairsMaps.podToTopologyPairs[podFullName][pair] = struct{}{} +} + +func (topologyPairsMaps *topologyPairsMaps) removePod(deletedPod *v1.Pod) { + deletedPodFullName := schedutil.GetPodFullName(deletedPod) + for pair := range topologyPairsMaps.podToTopologyPairs[deletedPodFullName] { + delete(topologyPairsMaps.topologyPairToPods[pair], deletedPod) + if len(topologyPairsMaps.topologyPairToPods[pair]) == 0 { + delete(topologyPairsMaps.topologyPairToPods, pair) } } - delete(topologyPairsMaps.podToTopologyPairs, podName) + delete(topologyPairsMaps.podToTopologyPairs, deletedPodFullName) } func (topologyPairsMaps *topologyPairsMaps) appendMaps(toAppend *topologyPairsMaps) { - for pod, pairs := range toAppend.podToTopologyPairs { - topologyPairsMaps.podToTopologyPairs[pod] = append(topologyPairsMaps.podToTopologyPairs[pod], pairs...) - } - for pair, pods := range toAppend.topologyPairToPods { - topologyPairsMaps.topologyPairToPods[pair] = append(topologyPairsMaps.topologyPairToPods[pair], pods...) + for pair := range toAppend.topologyPairToPods { + for pod := range toAppend.topologyPairToPods[pair] { + topologyPairsMaps.addTopologyPair(pair, pod) + } } } @@ -199,7 +199,7 @@ func (meta *predicateMetadata) RemovePod(deletedPod *v1.Pod) error { if deletedPodFullName == schedutil.GetPodFullName(meta.pod) { return fmt.Errorf("deletedPod and meta.pod must not be the same") } - meta.topologyPairsAntiAffinityPodsMap.RemovePod(deletedPodFullName) + meta.topologyPairsAntiAffinityPodsMap.removePod(deletedPod) // Delete pod from the matching affinity or anti-affinity pods if exists. affinity := meta.pod.Spec.Affinity podNodeName := deletedPod.Spec.NodeName @@ -324,8 +324,7 @@ func (meta *predicateMetadata) ShallowCopy() algorithm.PredicateMetadata { for k, v := range meta.nodeNameToMatchingAntiAffinityPods { newPredMeta.nodeNameToMatchingAntiAffinityPods[k] = append([]*v1.Pod(nil), v...) } - newPredMeta.topologyPairsAntiAffinityPodsMap = &topologyPairsMaps{topologyPairToPods: make(map[topologyPair][]*v1.Pod), - podToTopologyPairs: make(map[string][]topologyPair)} + newPredMeta.topologyPairsAntiAffinityPodsMap = newTopologyPairsMaps() newPredMeta.topologyPairsAntiAffinityPodsMap.appendMaps(meta.topologyPairsAntiAffinityPodsMap) newPredMeta.serviceAffinityMatchingPodServices = append([]*v1.Service(nil), meta.serviceAffinityMatchingPodServices...) diff --git a/pkg/scheduler/algorithm/predicates/metadata_test.go b/pkg/scheduler/algorithm/predicates/metadata_test.go index 0fc116e8a1..04da8ed5c0 100644 --- a/pkg/scheduler/algorithm/predicates/metadata_test.go +++ b/pkg/scheduler/algorithm/predicates/metadata_test.go @@ -28,33 +28,6 @@ import ( schedulertesting "k8s.io/kubernetes/pkg/scheduler/testing" ) -// sortableTopologyPairs lets us sort topology pairs -type sortableTopologyPairs []topologyPair - -// Less establishes some ordering between two topologyPairs for sorting. -func (s sortableTopologyPairs) Less(i, j int) bool { - t1, t2 := s[i], s[j] - return t1.key < t2.key || (t1.key == t2.key && t1.value < t2.value) -} -func (s sortableTopologyPairs) Len() int { return len(s) } -func (s sortableTopologyPairs) Swap(i, j int) { s[i], s[j] = s[j], s[i] } - -var _ = sort.Interface(sortableTopologyPairs{}) - -func sortTopologyPairs(pairs map[string][]topologyPair) { - for k, v := range pairs { - sortableTerms := sortableTopologyPairs(v) - sort.Sort(sortableTerms) - pairs[k] = sortableTerms - } -} -func sortTopologyPairPods(np map[topologyPair][]*v1.Pod) { - for _, pl := range np { - sortablePods := sortablePods(pl) - sort.Sort(sortablePods) - } -} - // sortablePods lets us to sort pods. type sortablePods []*v1.Pod @@ -114,17 +87,13 @@ func predicateMetadataEquivalent(meta1, meta2 *predicateMetadata) error { if !reflect.DeepEqual(meta1.nodeNameToMatchingAntiAffinityPods, meta2.nodeNameToMatchingAntiAffinityPods) { return fmt.Errorf("nodeNameToMatchingAntiAffinityPods are not euqal") } - sortTopologyPairs(meta1.topologyPairsAntiAffinityPodsMap.podToTopologyPairs) - sortTopologyPairs(meta2.topologyPairsAntiAffinityPodsMap.podToTopologyPairs) if !reflect.DeepEqual(meta1.topologyPairsAntiAffinityPodsMap.podToTopologyPairs, meta2.topologyPairsAntiAffinityPodsMap.podToTopologyPairs) { - return fmt.Errorf("topologyPairsAntiAffinityPodsMap.antiAffinityPodToTopologyPairs are not equal") + return fmt.Errorf("topologyPairsAntiAffinityPodsMap.podToTopologyPairs are not equal") } - sortTopologyPairPods(meta1.topologyPairsAntiAffinityPodsMap.topologyPairToPods) - sortTopologyPairPods(meta2.topologyPairsAntiAffinityPodsMap.topologyPairToPods) if !reflect.DeepEqual(meta1.topologyPairsAntiAffinityPodsMap.topologyPairToPods, meta2.topologyPairsAntiAffinityPodsMap.topologyPairToPods) { - return fmt.Errorf("topologyPairsAntiAffinityPodsMap.topologyPairToAntiAffinityPods are not equal") + return fmt.Errorf("topologyPairsAntiAffinityPodsMap.topologyPairToPods are not equal") } if meta1.serviceAffinityInUse { sortablePods1 := sortablePods(meta1.serviceAffinityMatchingPodList) @@ -464,24 +433,24 @@ func TestPredicateMetadata_ShallowCopy(t *testing.T) { }, }, topologyPairsAntiAffinityPodsMap: &topologyPairsMaps{ - topologyPairToPods: map[topologyPair][]*v1.Pod{ + topologyPairToPods: map[topologyPair]podSet{ {key: "name", value: "machine1"}: { &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p2", Labels: selector1}, Spec: v1.PodSpec{NodeName: "nodeC"}, - }, + }: struct{}{}, }, {key: "name", value: "machine2"}: { &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p1", Labels: selector1}, Spec: v1.PodSpec{NodeName: "nodeA"}, - }, + }: struct{}{}, }, }, - podToTopologyPairs: map[string][]topologyPair{ - "p2": { - topologyPair{key: "name", value: "machine1"}, + podToTopologyPairs: map[string]topologyPairSet{ + "p2_": { + topologyPair{key: "name", value: "machine1"}: struct{}{}, }, - "p1": { - topologyPair{key: "name", value: "machine2"}, + "p1_": { + topologyPair{key: "name", value: "machine2"}: struct{}{}, }, }, }, diff --git a/pkg/scheduler/algorithm/predicates/predicates.go b/pkg/scheduler/algorithm/predicates/predicates.go index 103d5ea7f5..c919282c83 100644 --- a/pkg/scheduler/algorithm/predicates/predicates.go +++ b/pkg/scheduler/algorithm/predicates/predicates.go @@ -1255,8 +1255,7 @@ func getMatchingTopologyPairs(pod *v1.Pod, nodeInfoMap map[string]*schedulercach var lock sync.Mutex var firstError error - topologyMaps := &topologyPairsMaps{topologyPairToPods: make(map[topologyPair][]*v1.Pod), - podToTopologyPairs: make(map[string][]topologyPair)} + topologyMaps := newTopologyPairsMaps() appendTopologyPairsMaps := func(toAppend *topologyPairsMaps) { lock.Lock() @@ -1278,8 +1277,7 @@ func getMatchingTopologyPairs(pod *v1.Pod, nodeInfoMap map[string]*schedulercach catchError(fmt.Errorf("node not found")) return } - nodeTopologyMaps := &topologyPairsMaps{topologyPairToPods: make(map[topologyPair][]*v1.Pod), - podToTopologyPairs: make(map[string][]topologyPair)} + nodeTopologyMaps := newTopologyPairsMaps() for _, existingPod := range nodeInfo.PodsWithAffinity() { affinity := existingPod.Spec.Affinity if affinity == nil { @@ -1295,7 +1293,7 @@ func getMatchingTopologyPairs(pod *v1.Pod, nodeInfoMap map[string]*schedulercach if priorityutil.PodMatchesTermsNamespaceAndSelector(pod, namespaces, selector) { if topologyValue, ok := node.Labels[term.TopologyKey]; ok { pair := topologyPair{key: term.TopologyKey, value: topologyValue} - nodeTopologyMaps.AddTopologyPair(pair, existingPod) + nodeTopologyMaps.addTopologyPair(pair, existingPod) } } } @@ -1309,8 +1307,7 @@ func getMatchingTopologyPairs(pod *v1.Pod, nodeInfoMap map[string]*schedulercach } func getMatchingTopologyPairsOfExistingPod(newPod *v1.Pod, existingPod *v1.Pod, node *v1.Node) (*topologyPairsMaps, error) { - topologyMaps := &topologyPairsMaps{topologyPairToPods: make(map[topologyPair][]*v1.Pod), - podToTopologyPairs: make(map[string][]topologyPair)} + topologyMaps := newTopologyPairsMaps() affinity := existingPod.Spec.Affinity if affinity != nil && affinity.PodAntiAffinity != nil { for _, term := range GetPodAntiAffinityTerms(affinity.PodAntiAffinity) { @@ -1322,7 +1319,7 @@ func getMatchingTopologyPairsOfExistingPod(newPod *v1.Pod, existingPod *v1.Pod, if priorityutil.PodMatchesTermsNamespaceAndSelector(newPod, namespaces, selector) { if topologyValue, ok := node.Labels[term.TopologyKey]; ok { pair := topologyPair{key: term.TopologyKey, value: topologyValue} - topologyMaps.AddTopologyPair(pair, existingPod) + topologyMaps.addTopologyPair(pair, existingPod) } } } @@ -1330,8 +1327,7 @@ func getMatchingTopologyPairsOfExistingPod(newPod *v1.Pod, existingPod *v1.Pod, return topologyMaps, nil } func (c *PodAffinityChecker) getMatchingAntiAffinityTopologyPairs(pod *v1.Pod, allPods []*v1.Pod) (*topologyPairsMaps, error) { - topologyMaps := &topologyPairsMaps{topologyPairToPods: make(map[topologyPair][]*v1.Pod), - podToTopologyPairs: make(map[string][]topologyPair)} + topologyMaps := newTopologyPairsMaps() for _, existingPod := range allPods { affinity := existingPod.Spec.Affinity @@ -1383,7 +1379,7 @@ func (c *PodAffinityChecker) satisfiesExistingPodsAntiAffinity(pod *v1.Pod, meta // Iterate over topology pairs to get any of the pods being affected by // the scheduled pod anti-affinity rules for topologyKey, topologyValue := range node.Labels { - if _, ok := topologyMaps.topologyPairToPods[topologyPair{key: topologyKey, value: topologyValue}]; ok { + if topologyMaps.topologyPairToPods[topologyPair{key: topologyKey, value: topologyValue}] != nil { glog.V(10).Infof("Cannot schedule pod %+v onto node %v", podName(pod), node.Name) return ErrExistingPodsAntiAffinityRulesNotMatch, nil }