EvenPodsSpread: refactor topologyPairsPodSpreadMap

- update minMatchMap from []int32 to map[string]int32

pkg/scheduler/algorithm/predicates/metadata.go

@@ -19,6 +19,7 @@ package predicates
 import (
 	"context"
 	"fmt"
+	"math"
 	"sync"
 
 	"k8s.io/klog"
@@ -34,9 +35,6 @@ import (
 	schedutil "k8s.io/kubernetes/pkg/scheduler/util"
 )
 
-// MaxInt32 is the maximum value of int32
-const MaxInt32 = int32(^uint32(0) >> 1)
-
 // PredicateMetadata interface represents anything that can access a predicate metadata.
 type PredicateMetadata interface {
 	ShallowCopy() PredicateMetadata
@@ -69,21 +67,17 @@ type topologyPairsMaps struct {
 	podToTopologyPairs map[string]topologyPairSet
 }
 
-// topologyPairsPodSpreadMap combines []int32 and topologyPairsMaps to represent
-// (1) how existing pods match incoming pod on its spread constraints
-// (2) minimum match number of each hard spread constraint
+// topologyPairsPodSpreadMap combines topologyKeyToMinPodsMap and topologyPairsMaps
+// to represent:
+// (1) minimum number of pods matched on the spread constraints.
+// (2) how existing pods match incoming pod on its spread constraints.
 type topologyPairsPodSpreadMap struct {
-	minMatches []int32
+	// This map is keyed with a topology key, and valued with minimum number
+	// of pods matched on that topology domain.
+	topologyKeyToMinPodsMap map[string]int32
 	*topologyPairsMaps
 }
 
-func newTopologyPairsPodSpreadMap() *topologyPairsPodSpreadMap {
-	return &topologyPairsPodSpreadMap{
-		// minMatches will be initilized with proper size later
-		topologyPairsMaps: newTopologyPairsMaps(),
-	}
-}
-
 // 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 {
@@ -109,7 +103,7 @@ type predicateMetadata struct {
 	// which should be accounted only by the extenders. This set is synthesized
 	// from scheduler extender configuration and does not change per pod.
 	ignoredExtendedResources sets.String
-	// Similar like map for pod (anti-)affinity, but impose additional min matches info
+	// Similar to the map for pod (anti-)affinity, but imposes additional min matches info
 	// to describe mininum match number on each topology spread constraint
 	topologyPairsPodSpreadMap *topologyPairsPodSpreadMap
 }
@@ -158,7 +152,7 @@ func (pfactory *PredicateMetadataFactory) GetMetadata(pod *v1.Pod, nodeNameToInf
 	if pod == nil {
 		return nil
 	}
-	// existingPodSpreadConstraintsMap represents how existing pods matches "pod"
+	// existingPodSpreadConstraintsMap represents how existing pods match "pod"
 	// on its spread constraints
 	existingPodSpreadConstraintsMap, err := getTPMapMatchingSpreadConstraints(pod, nodeNameToInfoMap)
 	if err != nil {
@@ -211,7 +205,10 @@ func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*sche
 	var lock sync.Mutex
 	var firstError error
 
-	topologyPairsPodSpreadMap := newTopologyPairsPodSpreadMap()
+	topologyPairsPodSpreadMap := &topologyPairsPodSpreadMap{
+		// topologyKeyToMinPodsMap will be initilized with proper size later.
+		topologyPairsMaps: newTopologyPairsMaps(),
+	}
 
 	appendTopologyPairsMaps := func(toAppend *topologyPairsMaps) {
 		lock.Lock()
@@ -288,20 +285,15 @@ func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*sche
 	}
 
 	// calculate min match for each topology pair
-	topologyPairsPodSpreadMap.minMatches = make([]int32, len(constraints))
-	tpKeyIdx := make(map[string]int)
-	for i, constraint := range constraints {
-		tpKeyIdx[constraint.TopologyKey] = i
-		topologyPairsPodSpreadMap.minMatches[i] = MaxInt32
+	topologyPairsPodSpreadMap.topologyKeyToMinPodsMap = make(map[string]int32, len(constraints))
+	for _, constraint := range constraints {
+		topologyPairsPodSpreadMap.topologyKeyToMinPodsMap[constraint.TopologyKey] = math.MaxInt32
 	}
 	for pair, podSet := range topologyPairsPodSpreadMap.topologyPairToPods {
-		idx := tpKeyIdx[pair.key]
-		// short circuit if we see 0 as min match of the topologyKey
-		if topologyPairsPodSpreadMap.minMatches[idx] == 0 {
-			continue
-		}
-		if l := int32(len(podSet)); l < topologyPairsPodSpreadMap.minMatches[idx] {
-			topologyPairsPodSpreadMap.minMatches[idx] = l
+		// TODO(Huang-Wei): short circuit all portions of <topologyKey: any value>
+		// if we see 0 as min match of the topologyKey
+		if l := int32(len(podSet)); l < topologyPairsPodSpreadMap.topologyKeyToMinPodsMap[pair.key] {
+			topologyPairsPodSpreadMap.topologyKeyToMinPodsMap[pair.key] = l
 		}
 	}
 	return topologyPairsPodSpreadMap, nil
@@ -333,12 +325,13 @@ func podLabelsMatchesSpreadConstraints(podLabels map[string]string, constraints
 	if len(constraints) == 0 {
 		return false, nil
 	}
+	podLabelSet := labels.Set(podLabels)
 	for _, constraint := range constraints {
 		selector, err := metav1.LabelSelectorAsSelector(constraint.LabelSelector)
 		if err != nil {
 			return false, err
 		}
-		if !selector.Matches(labels.Set(podLabels)) {
+		if !selector.Matches(podLabelSet) {
 			return false, nil
 		}
 	}
@@ -407,8 +400,13 @@ func (podSpreadMap *topologyPairsPodSpreadMap) clone() *topologyPairsPodSpreadMa
 	if podSpreadMap == nil {
 		return nil
 	}
-	copy := newTopologyPairsPodSpreadMap()
-	copy.minMatches = append([]int32(nil), podSpreadMap.minMatches...)
+	copy := &topologyPairsPodSpreadMap{
+		topologyKeyToMinPodsMap: make(map[string]int32),
+		topologyPairsMaps:       newTopologyPairsMaps(),
+	}
+	for key, minMatched := range podSpreadMap.topologyKeyToMinPodsMap {
+		copy.topologyKeyToMinPodsMap[key] = minMatched
+	}
 	copy.topologyPairsMaps.appendMaps(podSpreadMap.topologyPairsMaps)
 	return copy
 }

pkg/scheduler/algorithm/predicates/metadata_test.go

@@ -512,7 +512,7 @@ func TestPredicateMetadata_ShallowCopy(t *testing.T) {
 			},
 		},
 		topologyPairsPodSpreadMap: &topologyPairsPodSpreadMap{
-			minMatches: []int32{1},
+			topologyKeyToMinPodsMap: map[string]int32{"name": 1},
 			topologyPairsMaps: &topologyPairsMaps{
 				topologyPairToPods: map[topologyPair]podSet{
 					{key: "name", value: "nodeA"}: {
@@ -948,11 +948,11 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) {
 			injectPodPointers: map[topologyPair][]int{
 				// denotes no existing pod is matched on this zone pair, but still needed to be
 				// calculated if incoming pod matches its own spread constraints
-				{key: "zone", value: "zone1"}: []int{},
-				{key: "zone", value: "zone2"}: []int{},
+				{key: "zone", value: "zone1"}: {},
+				{key: "zone", value: "zone2"}: {},
 			},
 			want: &topologyPairsPodSpreadMap{
-				minMatches: []int32{0},
+				topologyKeyToMinPodsMap: map[string]int32{"zone": 0},
 				topologyPairsMaps: &topologyPairsMaps{
 					podToTopologyPairs: make(map[string]topologyPairSet),
 				},
@@ -978,12 +978,12 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) {
 			},
 			injectPodPointers: map[topologyPair][]int{
 				// denotes existingPods[0,1,2]
-				{key: "zone", value: "zone1"}: []int{0, 1, 2},
+				{key: "zone", value: "zone1"}: {0, 1, 2},
 				// denotes existingPods[3,4]
-				{key: "zone", value: "zone2"}: []int{3, 4},
+				{key: "zone", value: "zone2"}: {3, 4},
 			},
 			want: &topologyPairsPodSpreadMap{
-				minMatches: []int32{2},
+				topologyKeyToMinPodsMap: map[string]int32{"zone": 2},
 				topologyPairsMaps: &topologyPairsMaps{
 					podToTopologyPairs: map[string]topologyPairSet{
 						"p-a1_": newPairSet("zone", "zone1"),
@@ -1014,11 +1014,11 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) {
 				makePod().name("p-y2").node("node-y").label("foo", "").obj(),
 			},
 			injectPodPointers: map[topologyPair][]int{
-				{key: "zone", value: "zone1"}: []int{0, 2},
-				{key: "zone", value: "zone2"}: []int{4},
+				{key: "zone", value: "zone1"}: {0, 2},
+				{key: "zone", value: "zone2"}: {4},
 			},
 			want: &topologyPairsPodSpreadMap{
-				minMatches: []int32{1},
+				topologyKeyToMinPodsMap: map[string]int32{"zone": 1},
 				topologyPairsMaps: &topologyPairsMaps{
 					podToTopologyPairs: map[string]topologyPairSet{
 						"p-a1_": newPairSet("zone", "zone1"),
@@ -1050,15 +1050,15 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) {
 				makePod().name("p-y4").node("node-y").label("foo", "").obj(),
 			},
 			injectPodPointers: map[topologyPair][]int{
-				{key: "zone", value: "zone1"}:  []int{0, 1, 2},
-				{key: "zone", value: "zone2"}:  []int{3, 4, 5, 6},
-				{key: "node", value: "node-a"}: []int{0, 1},
-				{key: "node", value: "node-b"}: []int{2},
-				{key: "node", value: "node-x"}: []int{},
-				{key: "node", value: "node-y"}: []int{3, 4, 5, 6},
+				{key: "zone", value: "zone1"}:  {0, 1, 2},
+				{key: "zone", value: "zone2"}:  {3, 4, 5, 6},
+				{key: "node", value: "node-a"}: {0, 1},
+				{key: "node", value: "node-b"}: {2},
+				{key: "node", value: "node-x"}: {},
+				{key: "node", value: "node-y"}: {3, 4, 5, 6},
 			},
 			want: &topologyPairsPodSpreadMap{
-				minMatches: []int32{3, 0},
+				topologyKeyToMinPodsMap: map[string]int32{"zone": 3, "node": 0},
 				topologyPairsMaps: &topologyPairsMaps{
 					podToTopologyPairs: map[string]topologyPairSet{
 						"p-a1_": newPairSet("zone", "zone1", "node", "node-a"),
@@ -1095,14 +1095,14 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) {
 				makePod().name("p-y4").node("node-y").label("foo", "").obj(),
 			},
 			injectPodPointers: map[topologyPair][]int{
-				{key: "zone", value: "zone1"}:  []int{0, 1, 2},
-				{key: "zone", value: "zone2"}:  []int{3, 4, 5, 6},
-				{key: "node", value: "node-a"}: []int{0, 1},
-				{key: "node", value: "node-b"}: []int{2},
-				{key: "node", value: "node-y"}: []int{3, 4, 5, 6},
+				{key: "zone", value: "zone1"}:  {0, 1, 2},
+				{key: "zone", value: "zone2"}:  {3, 4, 5, 6},
+				{key: "node", value: "node-a"}: {0, 1},
+				{key: "node", value: "node-b"}: {2},
+				{key: "node", value: "node-y"}: {3, 4, 5, 6},
 			},
 			want: &topologyPairsPodSpreadMap{
-				minMatches: []int32{3, 1},
+				topologyKeyToMinPodsMap: map[string]int32{"zone": 3, "node": 1},
 				topologyPairsMaps: &topologyPairsMaps{
 					podToTopologyPairs: map[string]topologyPairSet{
 						"p-a1_": newPairSet("zone", "zone1", "node", "node-a"),
@@ -1137,14 +1137,14 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) {
 				makePod().name("p-y4").node("node-y").label("foo", "").label("bar", "").obj(),
 			},
 			injectPodPointers: map[topologyPair][]int{
-				{key: "zone", value: "zone1"}:  []int{1},
-				{key: "zone", value: "zone2"}:  []int{4, 6},
-				{key: "node", value: "node-a"}: []int{1},
-				{key: "node", value: "node-b"}: []int{},
-				{key: "node", value: "node-y"}: []int{4, 6},
+				{key: "zone", value: "zone1"}:  {1},
+				{key: "zone", value: "zone2"}:  {4, 6},
+				{key: "node", value: "node-a"}: {1},
+				{key: "node", value: "node-b"}: {},
+				{key: "node", value: "node-y"}: {4, 6},
 			},
 			want: &topologyPairsPodSpreadMap{
-				minMatches: []int32{1, 0},
+				topologyKeyToMinPodsMap: map[string]int32{"zone": 1, "node": 0},
 				topologyPairsMaps: &topologyPairsMaps{
 					podToTopologyPairs: map[string]topologyPairSet{
 						"p-a2_": newPairSet("zone", "zone1", "node", "node-a"),
@@ -1157,7 +1157,7 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) {
 		{
 			name: "two spreadConstraints, and with podAffinity",
 			pod: makePod().name("p").label("foo", "").
-				nodeAffinityIn("node", []string{"node-a", "node-b", "node-y"}). // exclude node-x
+				nodeAffinityNotIn("node", []string{"node-x"}). // exclude node-x
 				spreadConstraint(1, "zone", hardSpread, makeLabelSelector().exists("foo").obj()).
 				spreadConstraint(1, "node", hardSpread, makeLabelSelector().exists("foo").obj()).
 				obj(),
@@ -1177,14 +1177,14 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) {
 				makePod().name("p-y4").node("node-y").label("foo", "").obj(),
 			},
 			injectPodPointers: map[topologyPair][]int{
-				{key: "zone", value: "zone1"}:  []int{0, 1, 2},
-				{key: "zone", value: "zone2"}:  []int{3, 4, 5, 6},
-				{key: "node", value: "node-a"}: []int{0, 1},
-				{key: "node", value: "node-b"}: []int{2},
-				{key: "node", value: "node-y"}: []int{3, 4, 5, 6},
+				{key: "zone", value: "zone1"}:  {0, 1, 2},
+				{key: "zone", value: "zone2"}:  {3, 4, 5, 6},
+				{key: "node", value: "node-a"}: {0, 1},
+				{key: "node", value: "node-b"}: {2},
+				{key: "node", value: "node-y"}: {3, 4, 5, 6},
 			},
 			want: &topologyPairsPodSpreadMap{
-				minMatches: []int32{3, 1},
+				topologyKeyToMinPodsMap: map[string]int32{"zone": 3, "node": 1},
 				topologyPairsMaps: &topologyPairsMaps{
 					podToTopologyPairs: map[string]topologyPairSet{
 						"p-a1_": newPairSet("zone", "zone1", "node", "node-a"),

pkg/scheduler/algorithm/predicates/utils.go

@@ -170,6 +170,18 @@ func (s *nodeSelectorWrapper) in(key string, vals []string) *nodeSelectorWrapper
 	return s
 }
 
+func (s *nodeSelectorWrapper) notIn(key string, vals []string) *nodeSelectorWrapper {
+	expression := v1.NodeSelectorRequirement{
+		Key:      key,
+		Operator: v1.NodeSelectorOpNotIn,
+		Values:   vals,
+	}
+	selectorTerm := v1.NodeSelectorTerm{}
+	selectorTerm.MatchExpressions = append(selectorTerm.MatchExpressions, expression)
+	s.NodeSelectorTerms = append(s.NodeSelectorTerms, selectorTerm)
+	return s
+}
+
 func (s *nodeSelectorWrapper) obj() *v1.NodeSelector {
 	return &s.NodeSelector
 }
@@ -260,7 +272,7 @@ func (p *podWrapper) nodeSelector(m map[string]string) *podWrapper {
 	return p
 }
 
-// particular represents HARD node affinity
+// represents HARD node affinity in particular
 func (p *podWrapper) nodeAffinityIn(key string, vals []string) *podWrapper {
 	if p.Spec.Affinity == nil {
 		p.Spec.Affinity = &v1.Affinity{}
@@ -273,7 +285,19 @@ func (p *podWrapper) nodeAffinityIn(key string, vals []string) *podWrapper {
 	return p
 }
 
-func (p *podWrapper) spreadConstraint(maxSkew int, tpKey string, mode v1.UnsatisfiableConstraintResponse, selector *metav1.LabelSelector) *podWrapper {
+func (p *podWrapper) nodeAffinityNotIn(key string, vals []string) *podWrapper {
+	if p.Spec.Affinity == nil {
+		p.Spec.Affinity = &v1.Affinity{}
+	}
+	if p.Spec.Affinity.NodeAffinity == nil {
+		p.Spec.Affinity.NodeAffinity = &v1.NodeAffinity{}
+	}
+	nodeSelector := makeNodeSelector().notIn(key, vals).obj()
+	p.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution = nodeSelector
+	return p
+}
+
+func (p *podWrapper) spreadConstraint(maxSkew int, tpKey string, mode v1.UnsatisfiableConstraintAction, selector *metav1.LabelSelector) *podWrapper {
 	c := v1.TopologySpreadConstraint{
 		MaxSkew:           int32(maxSkew),
 		TopologyKey:       tpKey,