From a242e376aca8867954dad191a473fa190faba895 Mon Sep 17 00:00:00 2001 From: Wei Huang Date: Tue, 30 Apr 2019 16:13:45 -0700 Subject: [PATCH 1/6] EvenPodsSpread: PredicateMetadata initilization - build a new `topologyPairsPodSpreadMap` into PredicateMetadata - update ShallowCopy() - unit tests --- .../algorithm/predicates/metadata.go | 253 +++++++++- .../algorithm/predicates/metadata_test.go | 439 ++++++++++++++++++ pkg/scheduler/algorithm/predicates/utils.go | 169 ++++++- 3 files changed, 836 insertions(+), 25 deletions(-) diff --git a/pkg/scheduler/algorithm/predicates/metadata.go b/pkg/scheduler/algorithm/predicates/metadata.go index 095d80f29d5..48fa06bb090 100644 --- a/pkg/scheduler/algorithm/predicates/metadata.go +++ b/pkg/scheduler/algorithm/predicates/metadata.go @@ -34,6 +34,9 @@ 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 @@ -66,6 +69,21 @@ 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 +type topologyPairsPodSpreadMap struct { + minMatches []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 { @@ -91,6 +109,9 @@ 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 + // to describe mininum match number on each topology spread constraint + topologyPairsPodSpreadMap *topologyPairsPodSpreadMap } // Ensure that predicateMetadata implements algorithm.PredicateMetadata. @@ -137,17 +158,24 @@ func (pfactory *PredicateMetadataFactory) GetMetadata(pod *v1.Pod, nodeNameToInf if pod == nil { return nil } + // existingPodSpreadConstraintsMap represents how existing pods matches "pod" + // on its spread constraints + existingPodSpreadConstraintsMap, err := getTPMapMatchingSpreadConstraints(pod, nodeNameToInfoMap) + if err != nil { + klog.Errorf("Error calculating spreadConstraintsMap: %v", err) + return nil + } // existingPodAntiAffinityMap will be used later for efficient check on existing pods' anti-affinity existingPodAntiAffinityMap, err := getTPMapMatchingExistingAntiAffinity(pod, nodeNameToInfoMap) if err != nil { - klog.Errorf("[predicate meta data generation] error finding pods whose affinity terms are matched: %v", err) + klog.Errorf("Error calculating existingPodAntiAffinityMap: %v", err) return nil } // incomingPodAffinityMap will be used later for efficient check on incoming pod's affinity // incomingPodAntiAffinityMap will be used later for efficient check on incoming pod's anti-affinity incomingPodAffinityMap, incomingPodAntiAffinityMap, err := getTPMapMatchingIncomingAffinityAntiAffinity(pod, nodeNameToInfoMap) if err != nil { - klog.Errorf("[predicate meta data generation] error finding pods that match affinity terms: %v", err) + klog.Errorf("Error calculating incomingPod(Anti)AffinityMap: %v", err) return nil } predicateMetadata := &predicateMetadata{ @@ -158,6 +186,7 @@ func (pfactory *PredicateMetadataFactory) GetMetadata(pod *v1.Pod, nodeNameToInf topologyPairsPotentialAffinityPods: incomingPodAffinityMap, topologyPairsPotentialAntiAffinityPods: incomingPodAntiAffinityMap, topologyPairsAntiAffinityPodsMap: existingPodAntiAffinityMap, + topologyPairsPodSpreadMap: existingPodSpreadConstraintsMap, } for predicateName, precomputeFunc := range predicateMetadataProducers { klog.V(10).Infof("Precompute: %v", predicateName) @@ -166,46 +195,224 @@ func (pfactory *PredicateMetadataFactory) GetMetadata(pod *v1.Pod, nodeNameToInf return predicateMetadata } +func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*schedulernodeinfo.NodeInfo) (*topologyPairsPodSpreadMap, error) { + // we have feature gating in APIserver to strip the spec + // so don't need to re-check feature gate, just check length of constraints + constraints := getHardTopologySpreadConstraints(pod) + if len(constraints) == 0 { + return nil, nil + } + + allNodeNames := make([]string, 0, len(nodeInfoMap)) + for name := range nodeInfoMap { + allNodeNames = append(allNodeNames, name) + } + + var lock sync.Mutex + var firstError error + + topologyPairsPodSpreadMap := newTopologyPairsPodSpreadMap() + + appendTopologyPairsMaps := func(toAppend *topologyPairsMaps) { + lock.Lock() + topologyPairsPodSpreadMap.appendMaps(toAppend) + lock.Unlock() + } + catchError := func(err error) { + lock.Lock() + if firstError == nil { + firstError = err + } + lock.Unlock() + } + + ctx, cancel := context.WithCancel(context.Background()) + + processNode := func(i int) { + nodeInfo := nodeInfoMap[allNodeNames[i]] + node := nodeInfo.Node() + if node == nil { + catchError(fmt.Errorf("node %q not found", allNodeNames[i])) + cancel() + return + } + // Be design if NodeAffinity or NodeSelector is defined, spreading is + // applied to nodes that pass those filters. + if !podMatchesNodeSelectorAndAffinityTerms(pod, node) { + return + } + // ensure current node's labels contains all topologyKeys in 'constraints' + for _, constraint := range constraints { + if _, ok := node.Labels[constraint.TopologyKey]; !ok { + return + } + } + + nodeTopologyMaps := newTopologyPairsMaps() + // nodeInfo.Pods() can be empty; or all pods don't fit + for _, existingPod := range nodeInfo.Pods() { + ok, err := podMatchesAllSpreadConstraints(existingPod, pod.Namespace, constraints) + if err != nil { + catchError(err) + cancel() + return + } + if ok { + for _, constraint := range constraints { + // constraint.TopologyKey is already guaranteed to be present + pair := topologyPair{key: constraint.TopologyKey, value: node.Labels[constraint.TopologyKey]} + nodeTopologyMaps.addTopologyPair(pair, existingPod) + } + } + } + // If needed, append topology pair without entry of pods. + // For example, on node-x, there is no pod matching spread constraints + // but node-x should be also considered as a match (with match number 0) + // i.e. : {} + for _, constraint := range constraints { + // constraint.TopologyKey is already guaranteed to be present + pair := topologyPair{ + key: constraint.TopologyKey, + value: node.Labels[constraint.TopologyKey], + } + // addTopologyPairWithoutPods is a non-op if other pods match this pair + nodeTopologyMaps.addTopologyPairWithoutPods(pair) + } + + appendTopologyPairsMaps(nodeTopologyMaps) + } + workqueue.ParallelizeUntil(ctx, 16, len(allNodeNames), processNode) + + if firstError != nil { + return nil, firstError + } + + // 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 + } + 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 + } + } + return topologyPairsPodSpreadMap, nil +} + +func getHardTopologySpreadConstraints(pod *v1.Pod) (constraints []v1.TopologySpreadConstraint) { + if pod != nil { + for _, constraint := range pod.Spec.TopologySpreadConstraints { + if constraint.WhenUnsatisfiable == v1.DoNotSchedule { + constraints = append(constraints, constraint) + } + } + } + return +} + +func podMatchesAllSpreadConstraints(pod *v1.Pod, ns string, constraints []v1.TopologySpreadConstraint) (bool, error) { + if len(constraints) == 0 || pod.Namespace != ns { + return false, nil + } + return podLabelsMatchesSpreadConstraints(pod.Labels, constraints) +} + +// some corner cases: +// 1. podLabels = nil, constraint.LabelSelector = nil => returns false +// 2. podLabels = nil => returns false +// 3. constraint.LabelSelector = nil => returns false +func podLabelsMatchesSpreadConstraints(podLabels map[string]string, constraints []v1.TopologySpreadConstraint) (bool, error) { + if len(constraints) == 0 { + return false, nil + } + for _, constraint := range constraints { + selector, err := metav1.LabelSelectorAsSelector(constraint.LabelSelector) + if err != nil { + return false, err + } + if !selector.Matches(labels.Set(podLabels)) { + return false, nil + } + } + return true, nil +} + // 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) addTopologyPair(pair topologyPair, pod *v1.Pod) { +func (m *topologyPairsMaps) addTopologyPair(pair topologyPair, pod *v1.Pod) { podFullName := schedutil.GetPodFullName(pod) - if topologyPairsMaps.topologyPairToPods[pair] == nil { - topologyPairsMaps.topologyPairToPods[pair] = make(map[*v1.Pod]struct{}) + if m.topologyPairToPods[pair] == nil { + m.topologyPairToPods[pair] = make(map[*v1.Pod]struct{}) } - topologyPairsMaps.topologyPairToPods[pair][pod] = struct{}{} - if topologyPairsMaps.podToTopologyPairs[podFullName] == nil { - topologyPairsMaps.podToTopologyPairs[podFullName] = make(map[topologyPair]struct{}) + m.topologyPairToPods[pair][pod] = struct{}{} + if m.podToTopologyPairs[podFullName] == nil { + m.podToTopologyPairs[podFullName] = make(map[topologyPair]struct{}) } - topologyPairsMaps.podToTopologyPairs[podFullName][pair] = struct{}{} + m.podToTopologyPairs[podFullName][pair] = struct{}{} } -func (topologyPairsMaps *topologyPairsMaps) removePod(deletedPod *v1.Pod) { +// add a topology pair holder if needed +func (m *topologyPairsMaps) addTopologyPairWithoutPods(pair topologyPair) { + if m.topologyPairToPods[pair] == nil { + m.topologyPairToPods[pair] = make(map[*v1.Pod]struct{}) + } +} + +func (m *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) + for pair := range m.podToTopologyPairs[deletedPodFullName] { + delete(m.topologyPairToPods[pair], deletedPod) + if len(m.topologyPairToPods[pair]) == 0 { + delete(m.topologyPairToPods, pair) } } - delete(topologyPairsMaps.podToTopologyPairs, deletedPodFullName) + delete(m.podToTopologyPairs, deletedPodFullName) } -func (topologyPairsMaps *topologyPairsMaps) appendMaps(toAppend *topologyPairsMaps) { +func (m *topologyPairsMaps) appendMaps(toAppend *topologyPairsMaps) { if toAppend == nil { return } for pair := range toAppend.topologyPairToPods { - for pod := range toAppend.topologyPairToPods[pair] { - topologyPairsMaps.addTopologyPair(pair, pod) + if podSet := toAppend.topologyPairToPods[pair]; len(podSet) == 0 { + m.addTopologyPairWithoutPods(pair) + } else { + for pod := range podSet { + m.addTopologyPair(pair, pod) + } } } } +func (m *topologyPairsMaps) clone() *topologyPairsMaps { + copy := newTopologyPairsMaps() + copy.appendMaps(m) + return copy +} + +func (podSpreadMap *topologyPairsPodSpreadMap) clone() *topologyPairsPodSpreadMap { + // podSpreadMap could be nil when EvenPodsSpread feature is disabled + if podSpreadMap == nil { + return nil + } + copy := newTopologyPairsPodSpreadMap() + copy.minMatches = append([]int32(nil), podSpreadMap.minMatches...) + copy.topologyPairsMaps.appendMaps(podSpreadMap.topologyPairsMaps) + return copy +} + // RemovePod changes predicateMetadata assuming that the given `deletedPod` is // deleted from the system. func (meta *predicateMetadata) RemovePod(deletedPod *v1.Pod) error { @@ -301,12 +508,10 @@ func (meta *predicateMetadata) ShallowCopy() PredicateMetadata { ignoredExtendedResources: meta.ignoredExtendedResources, } newPredMeta.podPorts = append([]*v1.ContainerPort(nil), meta.podPorts...) - newPredMeta.topologyPairsPotentialAffinityPods = newTopologyPairsMaps() - newPredMeta.topologyPairsPotentialAffinityPods.appendMaps(meta.topologyPairsPotentialAffinityPods) - newPredMeta.topologyPairsPotentialAntiAffinityPods = newTopologyPairsMaps() - newPredMeta.topologyPairsPotentialAntiAffinityPods.appendMaps(meta.topologyPairsPotentialAntiAffinityPods) - newPredMeta.topologyPairsAntiAffinityPodsMap = newTopologyPairsMaps() - newPredMeta.topologyPairsAntiAffinityPodsMap.appendMaps(meta.topologyPairsAntiAffinityPodsMap) + newPredMeta.topologyPairsPotentialAffinityPods = meta.topologyPairsPotentialAffinityPods.clone() + newPredMeta.topologyPairsPotentialAntiAffinityPods = meta.topologyPairsPotentialAntiAffinityPods.clone() + newPredMeta.topologyPairsAntiAffinityPodsMap = meta.topologyPairsAntiAffinityPodsMap.clone() + newPredMeta.topologyPairsPodSpreadMap = meta.topologyPairsPodSpreadMap.clone() 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 3ab656dc30a..c29f8b10670 100644 --- a/pkg/scheduler/algorithm/predicates/metadata_test.go +++ b/pkg/scheduler/algorithm/predicates/metadata_test.go @@ -511,6 +511,39 @@ func TestPredicateMetadata_ShallowCopy(t *testing.T) { }, }, }, + topologyPairsPodSpreadMap: &topologyPairsPodSpreadMap{ + minMatches: []int32{1}, + topologyPairsMaps: &topologyPairsMaps{ + topologyPairToPods: map[topologyPair]podSet{ + {key: "name", value: "nodeA"}: { + &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p1", Labels: selector1}, + Spec: v1.PodSpec{NodeName: "nodeA"}, + }: struct{}{}, + }, + {key: "name", value: "nodeC"}: { + &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p2"}, + Spec: v1.PodSpec{ + NodeName: "nodeC", + }, + }: struct{}{}, + &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p6", Labels: selector1}, + Spec: v1.PodSpec{NodeName: "nodeC"}, + }: struct{}{}, + }, + }, + podToTopologyPairs: map[string]topologyPairSet{ + "p1_": { + topologyPair{key: "name", value: "nodeA"}: struct{}{}, + }, + "p2_": { + topologyPair{key: "name", value: "nodeC"}: struct{}{}, + }, + "p6_": { + topologyPair{key: "name", value: "nodeC"}: struct{}{}, + }, + }, + }, + }, serviceAffinityInUse: true, serviceAffinityMatchingPodList: []*v1.Pod{ {ObjectMeta: metav1.ObjectMeta{Name: "pod1"}}, @@ -791,3 +824,409 @@ func TestGetTPMapMatchingIncomingAffinityAntiAffinity(t *testing.T) { }) } } + +func TestPodLabelsMatchesSpreadConstraints(t *testing.T) { + tests := []struct { + name string + podLabels map[string]string + constraints []v1.TopologySpreadConstraint + want bool + wantErr bool + }{ + { + name: "normal match", + podLabels: map[string]string{"foo": "", "bar": ""}, + constraints: []v1.TopologySpreadConstraint{ + { + LabelSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "foo", + Operator: metav1.LabelSelectorOpExists, + }, + }, + }, + }, + }, + want: true, + }, + { + name: "normal mismatch", + podLabels: map[string]string{"foo": "", "baz": ""}, + constraints: []v1.TopologySpreadConstraint{ + { + LabelSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "foo", + Operator: metav1.LabelSelectorOpExists, + }, + { + Key: "bar", + Operator: metav1.LabelSelectorOpExists, + }, + }, + }, + }, + }, + want: false, + }, + { + name: "podLabels is nil", + constraints: []v1.TopologySpreadConstraint{ + { + LabelSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "foo", + Operator: metav1.LabelSelectorOpExists, + }, + }, + }, + }, + }, + want: false, + }, + { + name: "constraint.LabelSelector is nil", + podLabels: map[string]string{ + "foo": "", + "bar": "", + }, + constraints: []v1.TopologySpreadConstraint{ + { + MaxSkew: 1, + }, + }, + want: false, + }, + { + name: "both podLabels and constraint.LabelSelector are nil", + constraints: []v1.TopologySpreadConstraint{ + { + MaxSkew: 1, + }, + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := podLabelsMatchesSpreadConstraints(tt.podLabels, tt.constraints) + if (err != nil) != tt.wantErr { + t.Errorf("podLabelsMatchesSpreadConstraints() error = %v, wantErr %v", err, tt.wantErr) + } + if got != tt.want { + t.Errorf("podLabelsMatchesSpreadConstraints() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { + // we need to inject the exact pod pointers to want.topologyPairsMaps.topologyPairToPods + // otherwise, *pod (as key of a map) will always fail in reflect.DeepEqual() + tests := []struct { + name string + pod *v1.Pod + nodes []*v1.Node + existingPods []*v1.Pod + injectPodPointers map[topologyPair][]int + want *topologyPairsPodSpreadMap + }{ + { + name: "clean cluster with one spreadConstraint", + pod: makePod().name("p").label("foo", "").spreadConstraint( + 1, "zone", hardSpread, makeLabelSelector().exists("foo").obj(), + ).obj(), + nodes: []*v1.Node{ + makeNode().name("node-a").label("zone", "zone1").label("node", "node-a").obj(), + makeNode().name("node-b").label("zone", "zone1").label("node", "node-b").obj(), + makeNode().name("node-x").label("zone", "zone2").label("node", "node-x").obj(), + makeNode().name("node-y").label("zone", "zone2").label("node", "node-y").obj(), + }, + 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{}, + }, + want: &topologyPairsPodSpreadMap{ + minMatches: []int32{0}, + topologyPairsMaps: &topologyPairsMaps{ + podToTopologyPairs: make(map[string]topologyPairSet), + }, + }, + }, + { + name: "normal case with one spreadConstraint", + pod: makePod().name("p").label("foo", "").spreadConstraint( + 1, "zone", hardSpread, makeLabelSelector().exists("foo").obj(), + ).obj(), + nodes: []*v1.Node{ + makeNode().name("node-a").label("zone", "zone1").label("node", "node-a").obj(), + makeNode().name("node-b").label("zone", "zone1").label("node", "node-b").obj(), + makeNode().name("node-x").label("zone", "zone2").label("node", "node-x").obj(), + makeNode().name("node-y").label("zone", "zone2").label("node", "node-y").obj(), + }, + existingPods: []*v1.Pod{ + makePod().name("p-a1").node("node-a").label("foo", "").obj(), + makePod().name("p-a2").node("node-a").label("foo", "").obj(), + makePod().name("p-b1").node("node-b").label("foo", "").obj(), + makePod().name("p-y1").node("node-y").label("foo", "").obj(), + makePod().name("p-y2").node("node-y").label("foo", "").obj(), + }, + injectPodPointers: map[topologyPair][]int{ + // denotes existingPods[0,1,2] + {key: "zone", value: "zone1"}: []int{0, 1, 2}, + // denotes existingPods[3,4] + {key: "zone", value: "zone2"}: []int{3, 4}, + }, + want: &topologyPairsPodSpreadMap{ + minMatches: []int32{2}, + topologyPairsMaps: &topologyPairsMaps{ + podToTopologyPairs: map[string]topologyPairSet{ + "p-a1_": newPairSet("zone", "zone1"), + "p-a2_": newPairSet("zone", "zone1"), + "p-b1_": newPairSet("zone", "zone1"), + "p-y1_": newPairSet("zone", "zone2"), + "p-y2_": newPairSet("zone", "zone2"), + }, + }, + }, + }, + { + name: "namespace mis-match doesn't count", + pod: makePod().name("p").label("foo", "").spreadConstraint( + 1, "zone", hardSpread, makeLabelSelector().exists("foo").obj(), + ).obj(), + nodes: []*v1.Node{ + makeNode().name("node-a").label("zone", "zone1").label("node", "node-a").obj(), + makeNode().name("node-b").label("zone", "zone1").label("node", "node-b").obj(), + makeNode().name("node-x").label("zone", "zone2").label("node", "node-x").obj(), + makeNode().name("node-y").label("zone", "zone2").label("node", "node-y").obj(), + }, + existingPods: []*v1.Pod{ + makePod().name("p-a1").node("node-a").label("foo", "").obj(), + makePod().name("p-a2").namespace("ns1").node("node-a").label("foo", "").obj(), + makePod().name("p-b1").node("node-b").label("foo", "").obj(), + makePod().name("p-y1").namespace("ns2").node("node-y").label("foo", "").obj(), + 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}, + }, + want: &topologyPairsPodSpreadMap{ + minMatches: []int32{1}, + topologyPairsMaps: &topologyPairsMaps{ + podToTopologyPairs: map[string]topologyPairSet{ + "p-a1_": newPairSet("zone", "zone1"), + "p-b1_": newPairSet("zone", "zone1"), + "p-y2_": newPairSet("zone", "zone2"), + }, + }, + }, + }, + { + name: "normal case with two spreadConstraints", + pod: makePod().name("p").label("foo", ""). + spreadConstraint(1, "zone", hardSpread, makeLabelSelector().exists("foo").obj()). + spreadConstraint(1, "node", hardSpread, makeLabelSelector().exists("foo").obj()). + obj(), + nodes: []*v1.Node{ + makeNode().name("node-a").label("zone", "zone1").label("node", "node-a").obj(), + makeNode().name("node-b").label("zone", "zone1").label("node", "node-b").obj(), + makeNode().name("node-x").label("zone", "zone2").label("node", "node-x").obj(), + makeNode().name("node-y").label("zone", "zone2").label("node", "node-y").obj(), + }, + existingPods: []*v1.Pod{ + makePod().name("p-a1").node("node-a").label("foo", "").obj(), + makePod().name("p-a2").node("node-a").label("foo", "").obj(), + makePod().name("p-b1").node("node-b").label("foo", "").obj(), + makePod().name("p-y1").node("node-y").label("foo", "").obj(), + makePod().name("p-y2").node("node-y").label("foo", "").obj(), + makePod().name("p-y3").node("node-y").label("foo", "").obj(), + 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}, + }, + want: &topologyPairsPodSpreadMap{ + minMatches: []int32{3, 0}, + topologyPairsMaps: &topologyPairsMaps{ + podToTopologyPairs: map[string]topologyPairSet{ + "p-a1_": newPairSet("zone", "zone1", "node", "node-a"), + "p-a2_": newPairSet("zone", "zone1", "node", "node-a"), + "p-b1_": newPairSet("zone", "zone1", "node", "node-b"), + "p-y1_": newPairSet("zone", "zone2", "node", "node-y"), + "p-y2_": newPairSet("zone", "zone2", "node", "node-y"), + "p-y3_": newPairSet("zone", "zone2", "node", "node-y"), + "p-y4_": newPairSet("zone", "zone2", "node", "node-y"), + }, + }, + }, + }, + { + name: "soft spreadConstraints should be bypassed", + pod: makePod().name("p").label("foo", ""). + spreadConstraint(1, "zone", softSpread, makeLabelSelector().exists("foo").obj()). + spreadConstraint(1, "zone", hardSpread, makeLabelSelector().exists("foo").obj()). + spreadConstraint(1, "zone", softSpread, makeLabelSelector().exists("foo").obj()). + spreadConstraint(1, "node", hardSpread, makeLabelSelector().exists("foo").obj()). + obj(), + nodes: []*v1.Node{ + makeNode().name("node-a").label("zone", "zone1").label("node", "node-a").obj(), + makeNode().name("node-b").label("zone", "zone1").label("node", "node-b").obj(), + makeNode().name("node-y").label("zone", "zone2").label("node", "node-y").obj(), + }, + existingPods: []*v1.Pod{ + makePod().name("p-a1").node("node-a").label("foo", "").obj(), + makePod().name("p-a2").node("node-a").label("foo", "").obj(), + makePod().name("p-b1").node("node-b").label("foo", "").obj(), + makePod().name("p-y1").node("node-y").label("foo", "").obj(), + makePod().name("p-y2").node("node-y").label("foo", "").obj(), + makePod().name("p-y3").node("node-y").label("foo", "").obj(), + 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}, + }, + want: &topologyPairsPodSpreadMap{ + minMatches: []int32{3, 1}, + topologyPairsMaps: &topologyPairsMaps{ + podToTopologyPairs: map[string]topologyPairSet{ + "p-a1_": newPairSet("zone", "zone1", "node", "node-a"), + "p-a2_": newPairSet("zone", "zone1", "node", "node-a"), + "p-b1_": newPairSet("zone", "zone1", "node", "node-b"), + "p-y1_": newPairSet("zone", "zone2", "node", "node-y"), + "p-y2_": newPairSet("zone", "zone2", "node", "node-y"), + "p-y3_": newPairSet("zone", "zone2", "node", "node-y"), + "p-y4_": newPairSet("zone", "zone2", "node", "node-y"), + }, + }, + }, + }, + { + name: "different labelSelectors", + pod: makePod().name("p").label("foo", "").label("bar", ""). + spreadConstraint(1, "zone", hardSpread, makeLabelSelector().exists("foo").obj()). + spreadConstraint(1, "node", hardSpread, makeLabelSelector().exists("bar").obj()). + obj(), + nodes: []*v1.Node{ + makeNode().name("node-a").label("zone", "zone1").label("node", "node-a").obj(), + makeNode().name("node-b").label("zone", "zone1").label("node", "node-b").obj(), + makeNode().name("node-y").label("zone", "zone2").label("node", "node-y").obj(), + }, + existingPods: []*v1.Pod{ + makePod().name("p-a1").node("node-a").label("foo", "").obj(), + makePod().name("p-a2").node("node-a").label("foo", "").label("bar", "").obj(), + makePod().name("p-b1").node("node-b").label("foo", "").obj(), + makePod().name("p-y1").node("node-y").label("foo", "").obj(), + makePod().name("p-y2").node("node-y").label("foo", "").label("bar", "").obj(), + makePod().name("p-y3").node("node-y").label("foo", "").obj(), + 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}, + }, + want: &topologyPairsPodSpreadMap{ + minMatches: []int32{1, 0}, + topologyPairsMaps: &topologyPairsMaps{ + podToTopologyPairs: map[string]topologyPairSet{ + "p-a2_": newPairSet("zone", "zone1", "node", "node-a"), + "p-y2_": newPairSet("zone", "zone2", "node", "node-y"), + "p-y4_": newPairSet("zone", "zone2", "node", "node-y"), + }, + }, + }, + }, + { + name: "two spreadConstraints, and with podAffinity", + pod: makePod().name("p").label("foo", ""). + nodeAffinityIn("node", []string{"node-a", "node-b", "node-y"}). // exclude node-x + spreadConstraint(1, "zone", hardSpread, makeLabelSelector().exists("foo").obj()). + spreadConstraint(1, "node", hardSpread, makeLabelSelector().exists("foo").obj()). + obj(), + nodes: []*v1.Node{ + makeNode().name("node-a").label("zone", "zone1").label("node", "node-a").obj(), + makeNode().name("node-b").label("zone", "zone1").label("node", "node-b").obj(), + makeNode().name("node-x").label("zone", "zone2").label("node", "node-x").obj(), + makeNode().name("node-y").label("zone", "zone2").label("node", "node-y").obj(), + }, + existingPods: []*v1.Pod{ + makePod().name("p-a1").node("node-a").label("foo", "").obj(), + makePod().name("p-a2").node("node-a").label("foo", "").obj(), + makePod().name("p-b1").node("node-b").label("foo", "").obj(), + makePod().name("p-y1").node("node-y").label("foo", "").obj(), + makePod().name("p-y2").node("node-y").label("foo", "").obj(), + makePod().name("p-y3").node("node-y").label("foo", "").obj(), + 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}, + }, + want: &topologyPairsPodSpreadMap{ + minMatches: []int32{3, 1}, + topologyPairsMaps: &topologyPairsMaps{ + podToTopologyPairs: map[string]topologyPairSet{ + "p-a1_": newPairSet("zone", "zone1", "node", "node-a"), + "p-a2_": newPairSet("zone", "zone1", "node", "node-a"), + "p-b1_": newPairSet("zone", "zone1", "node", "node-b"), + "p-y1_": newPairSet("zone", "zone2", "node", "node-y"), + "p-y2_": newPairSet("zone", "zone2", "node", "node-y"), + "p-y3_": newPairSet("zone", "zone2", "node", "node-y"), + "p-y4_": newPairSet("zone", "zone2", "node", "node-y"), + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.want.topologyPairToPods = make(map[topologyPair]podSet) + for pair, indexes := range tt.injectPodPointers { + pSet := make(podSet) + for _, i := range indexes { + pSet[tt.existingPods[i]] = struct{}{} + } + tt.want.topologyPairToPods[pair] = pSet + } + nodeInfoMap := schedulernodeinfo.CreateNodeNameToInfoMap(tt.existingPods, tt.nodes) + if got, _ := getTPMapMatchingSpreadConstraints(tt.pod, nodeInfoMap); !reflect.DeepEqual(got, tt.want) { + t.Errorf("getTPMapMatchingSpreadConstraints() = %v, want %v", got, tt.want) + } + }) + } +} + +var ( + hardSpread = v1.DoNotSchedule + softSpread = v1.ScheduleAnyway +) + +func newPairSet(kv ...string) topologyPairSet { + result := make(topologyPairSet) + for i := 0; i < len(kv); i += 2 { + pair := topologyPair{key: kv[i], value: kv[i+1]} + result[pair] = struct{}{} + } + return result +} diff --git a/pkg/scheduler/algorithm/predicates/utils.go b/pkg/scheduler/algorithm/predicates/utils.go index dc833b2d6fb..2e7a2281fea 100644 --- a/pkg/scheduler/algorithm/predicates/utils.go +++ b/pkg/scheduler/algorithm/predicates/utils.go @@ -19,8 +19,9 @@ package predicates import ( "strings" - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" storagev1beta1 "k8s.io/api/storage/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/sets" utilfeature "k8s.io/apiserver/pkg/util/feature" @@ -147,3 +148,169 @@ func isCSIMigrationOn(csiNode *storagev1beta1.CSINode, pluginName string) bool { return mpaSet.Has(pluginName) } + +// utilities for building pod/node objects using a "chained" manner +type nodeSelectorWrapper struct{ v1.NodeSelector } + +func makeNodeSelector() *nodeSelectorWrapper { + return &nodeSelectorWrapper{v1.NodeSelector{}} +} + +// NOTE: each time we append a selectorTerm into `s` +// and overall all selecterTerms are ORed +func (s *nodeSelectorWrapper) in(key string, vals []string) *nodeSelectorWrapper { + expression := v1.NodeSelectorRequirement{ + Key: key, + Operator: v1.NodeSelectorOpIn, + 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 +} + +type labelSelectorWrapper struct{ metav1.LabelSelector } + +func makeLabelSelector() *labelSelectorWrapper { + return &labelSelectorWrapper{metav1.LabelSelector{}} +} + +func (s *labelSelectorWrapper) label(k, v string) *labelSelectorWrapper { + if s.MatchLabels == nil { + s.MatchLabels = make(map[string]string) + } + s.MatchLabels[k] = v + return s +} + +func (s *labelSelectorWrapper) in(key string, vals []string) *labelSelectorWrapper { + expression := metav1.LabelSelectorRequirement{ + Key: key, + Operator: metav1.LabelSelectorOpIn, + Values: vals, + } + s.MatchExpressions = append(s.MatchExpressions, expression) + return s +} + +func (s *labelSelectorWrapper) notIn(key string, vals []string) *labelSelectorWrapper { + expression := metav1.LabelSelectorRequirement{ + Key: key, + Operator: metav1.LabelSelectorOpNotIn, + Values: vals, + } + s.MatchExpressions = append(s.MatchExpressions, expression) + return s +} + +func (s *labelSelectorWrapper) exists(k string) *labelSelectorWrapper { + expression := metav1.LabelSelectorRequirement{ + Key: k, + Operator: metav1.LabelSelectorOpExists, + } + s.MatchExpressions = append(s.MatchExpressions, expression) + return s +} + +func (s *labelSelectorWrapper) notExist(k string) *labelSelectorWrapper { + expression := metav1.LabelSelectorRequirement{ + Key: k, + Operator: metav1.LabelSelectorOpDoesNotExist, + } + s.MatchExpressions = append(s.MatchExpressions, expression) + return s +} + +func (s *labelSelectorWrapper) obj() *metav1.LabelSelector { + return &s.LabelSelector +} + +type podWrapper struct{ v1.Pod } + +func makePod() *podWrapper { + return &podWrapper{v1.Pod{}} +} + +func (p *podWrapper) obj() *v1.Pod { + return &p.Pod +} + +func (p *podWrapper) name(s string) *podWrapper { + p.Name = s + return p +} + +func (p *podWrapper) namespace(s string) *podWrapper { + p.Namespace = s + return p +} + +func (p *podWrapper) node(s string) *podWrapper { + p.Spec.NodeName = s + return p +} + +func (p *podWrapper) nodeSelector(m map[string]string) *podWrapper { + p.Spec.NodeSelector = m + return p +} + +// particular represents HARD node affinity +func (p *podWrapper) nodeAffinityIn(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().in(key, vals).obj() + p.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution = nodeSelector + return p +} + +func (p *podWrapper) spreadConstraint(maxSkew int, tpKey string, mode v1.UnsatisfiableConstraintResponse, selector *metav1.LabelSelector) *podWrapper { + c := v1.TopologySpreadConstraint{ + MaxSkew: int32(maxSkew), + TopologyKey: tpKey, + WhenUnsatisfiable: mode, + LabelSelector: selector, + } + p.Spec.TopologySpreadConstraints = append(p.Spec.TopologySpreadConstraints, c) + return p +} + +func (p *podWrapper) label(k, v string) *podWrapper { + if p.Labels == nil { + p.Labels = make(map[string]string) + } + p.Labels[k] = v + return p +} + +type nodeWrapper struct{ v1.Node } + +func makeNode() *nodeWrapper { + return &nodeWrapper{v1.Node{}} +} + +func (n *nodeWrapper) obj() *v1.Node { + return &n.Node +} + +func (n *nodeWrapper) name(s string) *nodeWrapper { + n.Name = s + return n +} + +func (n *nodeWrapper) label(k, v string) *nodeWrapper { + if n.Labels == nil { + n.Labels = make(map[string]string) + } + n.Labels[k] = v + return n +} From 3dbef991a33eb105b00a48c939c955cca767c27f Mon Sep 17 00:00:00 2001 From: Wei Huang Date: Sat, 4 May 2019 00:16:27 -0700 Subject: [PATCH 2/6] EvenPodsSpread: refactor topologyPairsPodSpreadMap - update minMatchMap from []int32 to map[string]int32 --- .../algorithm/predicates/metadata.go | 62 ++++++++-------- .../algorithm/predicates/metadata_test.go | 72 +++++++++---------- pkg/scheduler/algorithm/predicates/utils.go | 28 +++++++- 3 files changed, 92 insertions(+), 70 deletions(-) diff --git a/pkg/scheduler/algorithm/predicates/metadata.go b/pkg/scheduler/algorithm/predicates/metadata.go index 48fa06bb090..5fd5b7b83fa 100644 --- a/pkg/scheduler/algorithm/predicates/metadata.go +++ b/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 + // 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 } diff --git a/pkg/scheduler/algorithm/predicates/metadata_test.go b/pkg/scheduler/algorithm/predicates/metadata_test.go index c29f8b10670..f76d9b4a36e 100644 --- a/pkg/scheduler/algorithm/predicates/metadata_test.go +++ b/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"), diff --git a/pkg/scheduler/algorithm/predicates/utils.go b/pkg/scheduler/algorithm/predicates/utils.go index 2e7a2281fea..3cf266a516d 100644 --- a/pkg/scheduler/algorithm/predicates/utils.go +++ b/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, From dce6686c9a308f4d8622a5e3a4f0bdb29295d432 Mon Sep 17 00:00:00 2001 From: Wei Huang Date: Fri, 10 May 2019 12:24:48 -0700 Subject: [PATCH 3/6] EvenPodsSpread: refactor "chained" utils - move "chanined" utils to pkg/scheduler/testing/utils.go so as to be re-used by all scheduler tests --- .../algorithm/predicates/metadata_test.go | 196 +++++++------- pkg/scheduler/algorithm/predicates/utils.go | 191 -------------- pkg/scheduler/testing/utils.go | 243 ++++++++++++++++++ pkg/scheduler/util/utils.go | 2 +- 4 files changed, 342 insertions(+), 290 deletions(-) create mode 100644 pkg/scheduler/testing/utils.go diff --git a/pkg/scheduler/algorithm/predicates/metadata_test.go b/pkg/scheduler/algorithm/predicates/metadata_test.go index f76d9b4a36e..1ca96f948e9 100644 --- a/pkg/scheduler/algorithm/predicates/metadata_test.go +++ b/pkg/scheduler/algorithm/predicates/metadata_test.go @@ -25,7 +25,7 @@ import ( "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo" - schedulertesting "k8s.io/kubernetes/pkg/scheduler/testing" + st "k8s.io/kubernetes/pkg/scheduler/testing" ) // sortablePods lets us to sort pods. @@ -352,16 +352,16 @@ func TestPredicateMetadata_AddRemovePod(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - allPodLister := schedulertesting.FakePodLister(append(test.existingPods, test.addedPod)) + allPodLister := st.FakePodLister(append(test.existingPods, test.addedPod)) // getMeta creates predicate meta data given the list of pods. - getMeta := func(lister schedulertesting.FakePodLister) (*predicateMetadata, map[string]*schedulernodeinfo.NodeInfo) { + getMeta := func(lister st.FakePodLister) (*predicateMetadata, map[string]*schedulernodeinfo.NodeInfo) { nodeInfoMap := schedulernodeinfo.CreateNodeNameToInfoMap(lister, test.nodes) // nodeList is a list of non-pointer nodes to feed to FakeNodeListInfo. nodeList := []v1.Node{} for _, n := range test.nodes { nodeList = append(nodeList, *n) } - _, precompute := NewServiceAffinityPredicate(lister, schedulertesting.FakeServiceLister(test.services), FakeNodeListInfo(nodeList), nil) + _, precompute := NewServiceAffinityPredicate(lister, st.FakeServiceLister(test.services), FakeNodeListInfo(nodeList), nil) RegisterPredicateMetadataProducer("ServiceAffinityMetaProducer", precompute) pmf := PredicateMetadataFactory{lister} meta := pmf.GetMetadata(test.pendingPod, nodeInfoMap) @@ -372,7 +372,7 @@ func TestPredicateMetadata_AddRemovePod(t *testing.T) { // are given to the metadata producer. allPodsMeta, _ := getMeta(allPodLister) // existingPodsMeta1 is meta data produced for test.existingPods (without test.addedPod). - existingPodsMeta1, nodeInfoMap := getMeta(schedulertesting.FakePodLister(test.existingPods)) + existingPodsMeta1, nodeInfoMap := getMeta(st.FakePodLister(test.existingPods)) // Add test.addedPod to existingPodsMeta1 and make sure meta is equal to allPodsMeta nodeInfo := nodeInfoMap[test.addedPod.Spec.NodeName] if err := existingPodsMeta1.AddPod(test.addedPod, nodeInfo); err != nil { @@ -383,7 +383,7 @@ func TestPredicateMetadata_AddRemovePod(t *testing.T) { } // Remove the added pod and from existingPodsMeta1 an make sure it is equal // to meta generated for existing pods. - existingPodsMeta2, _ := getMeta(schedulertesting.FakePodLister(test.existingPods)) + existingPodsMeta2, _ := getMeta(st.FakePodLister(test.existingPods)) if err := existingPodsMeta1.RemovePod(test.addedPod); err != nil { t.Errorf("error removing pod from meta: %v", err) } @@ -936,14 +936,14 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { }{ { name: "clean cluster with one spreadConstraint", - pod: makePod().name("p").label("foo", "").spreadConstraint( - 1, "zone", hardSpread, makeLabelSelector().exists("foo").obj(), - ).obj(), + pod: st.MakePod().Name("p").Label("foo", "").SpreadConstraint( + 1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj(), + ).Obj(), nodes: []*v1.Node{ - makeNode().name("node-a").label("zone", "zone1").label("node", "node-a").obj(), - makeNode().name("node-b").label("zone", "zone1").label("node", "node-b").obj(), - makeNode().name("node-x").label("zone", "zone2").label("node", "node-x").obj(), - makeNode().name("node-y").label("zone", "zone2").label("node", "node-y").obj(), + st.MakeNode().Name("node-a").Label("zone", "zone1").Label("node", "node-a").Obj(), + st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(), + st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(), + st.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(), }, injectPodPointers: map[topologyPair][]int{ // denotes no existing pod is matched on this zone pair, but still needed to be @@ -960,21 +960,21 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { }, { name: "normal case with one spreadConstraint", - pod: makePod().name("p").label("foo", "").spreadConstraint( - 1, "zone", hardSpread, makeLabelSelector().exists("foo").obj(), - ).obj(), + pod: st.MakePod().Name("p").Label("foo", "").SpreadConstraint( + 1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj(), + ).Obj(), nodes: []*v1.Node{ - makeNode().name("node-a").label("zone", "zone1").label("node", "node-a").obj(), - makeNode().name("node-b").label("zone", "zone1").label("node", "node-b").obj(), - makeNode().name("node-x").label("zone", "zone2").label("node", "node-x").obj(), - makeNode().name("node-y").label("zone", "zone2").label("node", "node-y").obj(), + st.MakeNode().Name("node-a").Label("zone", "zone1").Label("node", "node-a").Obj(), + st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(), + st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(), + st.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(), }, existingPods: []*v1.Pod{ - makePod().name("p-a1").node("node-a").label("foo", "").obj(), - makePod().name("p-a2").node("node-a").label("foo", "").obj(), - makePod().name("p-b1").node("node-b").label("foo", "").obj(), - makePod().name("p-y1").node("node-y").label("foo", "").obj(), - makePod().name("p-y2").node("node-y").label("foo", "").obj(), + st.MakePod().Name("p-a1").Node("node-a").Label("foo", "").Obj(), + st.MakePod().Name("p-a2").Node("node-a").Label("foo", "").Obj(), + st.MakePod().Name("p-b1").Node("node-b").Label("foo", "").Obj(), + st.MakePod().Name("p-y1").Node("node-y").Label("foo", "").Obj(), + st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(), }, injectPodPointers: map[topologyPair][]int{ // denotes existingPods[0,1,2] @@ -997,21 +997,21 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { }, { name: "namespace mis-match doesn't count", - pod: makePod().name("p").label("foo", "").spreadConstraint( - 1, "zone", hardSpread, makeLabelSelector().exists("foo").obj(), - ).obj(), + pod: st.MakePod().Name("p").Label("foo", "").SpreadConstraint( + 1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj(), + ).Obj(), nodes: []*v1.Node{ - makeNode().name("node-a").label("zone", "zone1").label("node", "node-a").obj(), - makeNode().name("node-b").label("zone", "zone1").label("node", "node-b").obj(), - makeNode().name("node-x").label("zone", "zone2").label("node", "node-x").obj(), - makeNode().name("node-y").label("zone", "zone2").label("node", "node-y").obj(), + st.MakeNode().Name("node-a").Label("zone", "zone1").Label("node", "node-a").Obj(), + st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(), + st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(), + st.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(), }, existingPods: []*v1.Pod{ - makePod().name("p-a1").node("node-a").label("foo", "").obj(), - makePod().name("p-a2").namespace("ns1").node("node-a").label("foo", "").obj(), - makePod().name("p-b1").node("node-b").label("foo", "").obj(), - makePod().name("p-y1").namespace("ns2").node("node-y").label("foo", "").obj(), - makePod().name("p-y2").node("node-y").label("foo", "").obj(), + st.MakePod().Name("p-a1").Node("node-a").Label("foo", "").Obj(), + st.MakePod().Name("p-a2").Namespace("ns1").Node("node-a").Label("foo", "").Obj(), + st.MakePod().Name("p-b1").Node("node-b").Label("foo", "").Obj(), + st.MakePod().Name("p-y1").Namespace("ns2").Node("node-y").Label("foo", "").Obj(), + st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(), }, injectPodPointers: map[topologyPair][]int{ {key: "zone", value: "zone1"}: {0, 2}, @@ -1030,24 +1030,24 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { }, { name: "normal case with two spreadConstraints", - pod: makePod().name("p").label("foo", ""). - spreadConstraint(1, "zone", hardSpread, makeLabelSelector().exists("foo").obj()). - spreadConstraint(1, "node", hardSpread, makeLabelSelector().exists("foo").obj()). - obj(), + pod: st.MakePod().Name("p").Label("foo", ""). + SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). + SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). + Obj(), nodes: []*v1.Node{ - makeNode().name("node-a").label("zone", "zone1").label("node", "node-a").obj(), - makeNode().name("node-b").label("zone", "zone1").label("node", "node-b").obj(), - makeNode().name("node-x").label("zone", "zone2").label("node", "node-x").obj(), - makeNode().name("node-y").label("zone", "zone2").label("node", "node-y").obj(), + st.MakeNode().Name("node-a").Label("zone", "zone1").Label("node", "node-a").Obj(), + st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(), + st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(), + st.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(), }, existingPods: []*v1.Pod{ - makePod().name("p-a1").node("node-a").label("foo", "").obj(), - makePod().name("p-a2").node("node-a").label("foo", "").obj(), - makePod().name("p-b1").node("node-b").label("foo", "").obj(), - makePod().name("p-y1").node("node-y").label("foo", "").obj(), - makePod().name("p-y2").node("node-y").label("foo", "").obj(), - makePod().name("p-y3").node("node-y").label("foo", "").obj(), - makePod().name("p-y4").node("node-y").label("foo", "").obj(), + st.MakePod().Name("p-a1").Node("node-a").Label("foo", "").Obj(), + st.MakePod().Name("p-a2").Node("node-a").Label("foo", "").Obj(), + st.MakePod().Name("p-b1").Node("node-b").Label("foo", "").Obj(), + st.MakePod().Name("p-y1").Node("node-y").Label("foo", "").Obj(), + st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(), + st.MakePod().Name("p-y3").Node("node-y").Label("foo", "").Obj(), + st.MakePod().Name("p-y4").Node("node-y").Label("foo", "").Obj(), }, injectPodPointers: map[topologyPair][]int{ {key: "zone", value: "zone1"}: {0, 1, 2}, @@ -1074,25 +1074,25 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { }, { name: "soft spreadConstraints should be bypassed", - pod: makePod().name("p").label("foo", ""). - spreadConstraint(1, "zone", softSpread, makeLabelSelector().exists("foo").obj()). - spreadConstraint(1, "zone", hardSpread, makeLabelSelector().exists("foo").obj()). - spreadConstraint(1, "zone", softSpread, makeLabelSelector().exists("foo").obj()). - spreadConstraint(1, "node", hardSpread, makeLabelSelector().exists("foo").obj()). - obj(), + pod: st.MakePod().Name("p").Label("foo", ""). + SpreadConstraint(1, "zone", softSpread, st.MakeLabelSelector().Exists("foo").Obj()). + SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). + SpreadConstraint(1, "zone", softSpread, st.MakeLabelSelector().Exists("foo").Obj()). + SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). + Obj(), nodes: []*v1.Node{ - makeNode().name("node-a").label("zone", "zone1").label("node", "node-a").obj(), - makeNode().name("node-b").label("zone", "zone1").label("node", "node-b").obj(), - makeNode().name("node-y").label("zone", "zone2").label("node", "node-y").obj(), + st.MakeNode().Name("node-a").Label("zone", "zone1").Label("node", "node-a").Obj(), + st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(), + st.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(), }, existingPods: []*v1.Pod{ - makePod().name("p-a1").node("node-a").label("foo", "").obj(), - makePod().name("p-a2").node("node-a").label("foo", "").obj(), - makePod().name("p-b1").node("node-b").label("foo", "").obj(), - makePod().name("p-y1").node("node-y").label("foo", "").obj(), - makePod().name("p-y2").node("node-y").label("foo", "").obj(), - makePod().name("p-y3").node("node-y").label("foo", "").obj(), - makePod().name("p-y4").node("node-y").label("foo", "").obj(), + st.MakePod().Name("p-a1").Node("node-a").Label("foo", "").Obj(), + st.MakePod().Name("p-a2").Node("node-a").Label("foo", "").Obj(), + st.MakePod().Name("p-b1").Node("node-b").Label("foo", "").Obj(), + st.MakePod().Name("p-y1").Node("node-y").Label("foo", "").Obj(), + st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(), + st.MakePod().Name("p-y3").Node("node-y").Label("foo", "").Obj(), + st.MakePod().Name("p-y4").Node("node-y").Label("foo", "").Obj(), }, injectPodPointers: map[topologyPair][]int{ {key: "zone", value: "zone1"}: {0, 1, 2}, @@ -1118,23 +1118,23 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { }, { name: "different labelSelectors", - pod: makePod().name("p").label("foo", "").label("bar", ""). - spreadConstraint(1, "zone", hardSpread, makeLabelSelector().exists("foo").obj()). - spreadConstraint(1, "node", hardSpread, makeLabelSelector().exists("bar").obj()). - obj(), + pod: st.MakePod().Name("p").Label("foo", "").Label("bar", ""). + SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). + SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("bar").Obj()). + Obj(), nodes: []*v1.Node{ - makeNode().name("node-a").label("zone", "zone1").label("node", "node-a").obj(), - makeNode().name("node-b").label("zone", "zone1").label("node", "node-b").obj(), - makeNode().name("node-y").label("zone", "zone2").label("node", "node-y").obj(), + st.MakeNode().Name("node-a").Label("zone", "zone1").Label("node", "node-a").Obj(), + st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(), + st.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(), }, existingPods: []*v1.Pod{ - makePod().name("p-a1").node("node-a").label("foo", "").obj(), - makePod().name("p-a2").node("node-a").label("foo", "").label("bar", "").obj(), - makePod().name("p-b1").node("node-b").label("foo", "").obj(), - makePod().name("p-y1").node("node-y").label("foo", "").obj(), - makePod().name("p-y2").node("node-y").label("foo", "").label("bar", "").obj(), - makePod().name("p-y3").node("node-y").label("foo", "").obj(), - makePod().name("p-y4").node("node-y").label("foo", "").label("bar", "").obj(), + st.MakePod().Name("p-a1").Node("node-a").Label("foo", "").Obj(), + st.MakePod().Name("p-a2").Node("node-a").Label("foo", "").Label("bar", "").Obj(), + st.MakePod().Name("p-b1").Node("node-b").Label("foo", "").Obj(), + st.MakePod().Name("p-y1").Node("node-y").Label("foo", "").Obj(), + st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Label("bar", "").Obj(), + st.MakePod().Name("p-y3").Node("node-y").Label("foo", "").Obj(), + st.MakePod().Name("p-y4").Node("node-y").Label("foo", "").Label("bar", "").Obj(), }, injectPodPointers: map[topologyPair][]int{ {key: "zone", value: "zone1"}: {1}, @@ -1156,25 +1156,25 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { }, { name: "two spreadConstraints, and with podAffinity", - pod: makePod().name("p").label("foo", ""). - 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(), + pod: st.MakePod().Name("p").Label("foo", ""). + NodeAffinityNotIn("node", []string{"node-x"}). // exclude node-x + SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). + SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). + Obj(), nodes: []*v1.Node{ - makeNode().name("node-a").label("zone", "zone1").label("node", "node-a").obj(), - makeNode().name("node-b").label("zone", "zone1").label("node", "node-b").obj(), - makeNode().name("node-x").label("zone", "zone2").label("node", "node-x").obj(), - makeNode().name("node-y").label("zone", "zone2").label("node", "node-y").obj(), + st.MakeNode().Name("node-a").Label("zone", "zone1").Label("node", "node-a").Obj(), + st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(), + st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(), + st.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(), }, existingPods: []*v1.Pod{ - makePod().name("p-a1").node("node-a").label("foo", "").obj(), - makePod().name("p-a2").node("node-a").label("foo", "").obj(), - makePod().name("p-b1").node("node-b").label("foo", "").obj(), - makePod().name("p-y1").node("node-y").label("foo", "").obj(), - makePod().name("p-y2").node("node-y").label("foo", "").obj(), - makePod().name("p-y3").node("node-y").label("foo", "").obj(), - makePod().name("p-y4").node("node-y").label("foo", "").obj(), + st.MakePod().Name("p-a1").Node("node-a").Label("foo", "").Obj(), + st.MakePod().Name("p-a2").Node("node-a").Label("foo", "").Obj(), + st.MakePod().Name("p-b1").Node("node-b").Label("foo", "").Obj(), + st.MakePod().Name("p-y1").Node("node-y").Label("foo", "").Obj(), + st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(), + st.MakePod().Name("p-y3").Node("node-y").Label("foo", "").Obj(), + st.MakePod().Name("p-y4").Node("node-y").Label("foo", "").Obj(), }, injectPodPointers: map[topologyPair][]int{ {key: "zone", value: "zone1"}: {0, 1, 2}, diff --git a/pkg/scheduler/algorithm/predicates/utils.go b/pkg/scheduler/algorithm/predicates/utils.go index 3cf266a516d..3115b4e8391 100644 --- a/pkg/scheduler/algorithm/predicates/utils.go +++ b/pkg/scheduler/algorithm/predicates/utils.go @@ -21,7 +21,6 @@ import ( "k8s.io/api/core/v1" storagev1beta1 "k8s.io/api/storage/v1beta1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/sets" utilfeature "k8s.io/apiserver/pkg/util/feature" @@ -148,193 +147,3 @@ func isCSIMigrationOn(csiNode *storagev1beta1.CSINode, pluginName string) bool { return mpaSet.Has(pluginName) } - -// utilities for building pod/node objects using a "chained" manner -type nodeSelectorWrapper struct{ v1.NodeSelector } - -func makeNodeSelector() *nodeSelectorWrapper { - return &nodeSelectorWrapper{v1.NodeSelector{}} -} - -// NOTE: each time we append a selectorTerm into `s` -// and overall all selecterTerms are ORed -func (s *nodeSelectorWrapper) in(key string, vals []string) *nodeSelectorWrapper { - expression := v1.NodeSelectorRequirement{ - Key: key, - Operator: v1.NodeSelectorOpIn, - Values: vals, - } - selectorTerm := v1.NodeSelectorTerm{} - selectorTerm.MatchExpressions = append(selectorTerm.MatchExpressions, expression) - s.NodeSelectorTerms = append(s.NodeSelectorTerms, selectorTerm) - 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 -} - -type labelSelectorWrapper struct{ metav1.LabelSelector } - -func makeLabelSelector() *labelSelectorWrapper { - return &labelSelectorWrapper{metav1.LabelSelector{}} -} - -func (s *labelSelectorWrapper) label(k, v string) *labelSelectorWrapper { - if s.MatchLabels == nil { - s.MatchLabels = make(map[string]string) - } - s.MatchLabels[k] = v - return s -} - -func (s *labelSelectorWrapper) in(key string, vals []string) *labelSelectorWrapper { - expression := metav1.LabelSelectorRequirement{ - Key: key, - Operator: metav1.LabelSelectorOpIn, - Values: vals, - } - s.MatchExpressions = append(s.MatchExpressions, expression) - return s -} - -func (s *labelSelectorWrapper) notIn(key string, vals []string) *labelSelectorWrapper { - expression := metav1.LabelSelectorRequirement{ - Key: key, - Operator: metav1.LabelSelectorOpNotIn, - Values: vals, - } - s.MatchExpressions = append(s.MatchExpressions, expression) - return s -} - -func (s *labelSelectorWrapper) exists(k string) *labelSelectorWrapper { - expression := metav1.LabelSelectorRequirement{ - Key: k, - Operator: metav1.LabelSelectorOpExists, - } - s.MatchExpressions = append(s.MatchExpressions, expression) - return s -} - -func (s *labelSelectorWrapper) notExist(k string) *labelSelectorWrapper { - expression := metav1.LabelSelectorRequirement{ - Key: k, - Operator: metav1.LabelSelectorOpDoesNotExist, - } - s.MatchExpressions = append(s.MatchExpressions, expression) - return s -} - -func (s *labelSelectorWrapper) obj() *metav1.LabelSelector { - return &s.LabelSelector -} - -type podWrapper struct{ v1.Pod } - -func makePod() *podWrapper { - return &podWrapper{v1.Pod{}} -} - -func (p *podWrapper) obj() *v1.Pod { - return &p.Pod -} - -func (p *podWrapper) name(s string) *podWrapper { - p.Name = s - return p -} - -func (p *podWrapper) namespace(s string) *podWrapper { - p.Namespace = s - return p -} - -func (p *podWrapper) node(s string) *podWrapper { - p.Spec.NodeName = s - return p -} - -func (p *podWrapper) nodeSelector(m map[string]string) *podWrapper { - p.Spec.NodeSelector = m - return p -} - -// 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{} - } - if p.Spec.Affinity.NodeAffinity == nil { - p.Spec.Affinity.NodeAffinity = &v1.NodeAffinity{} - } - nodeSelector := makeNodeSelector().in(key, vals).obj() - p.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution = nodeSelector - return p -} - -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, - WhenUnsatisfiable: mode, - LabelSelector: selector, - } - p.Spec.TopologySpreadConstraints = append(p.Spec.TopologySpreadConstraints, c) - return p -} - -func (p *podWrapper) label(k, v string) *podWrapper { - if p.Labels == nil { - p.Labels = make(map[string]string) - } - p.Labels[k] = v - return p -} - -type nodeWrapper struct{ v1.Node } - -func makeNode() *nodeWrapper { - return &nodeWrapper{v1.Node{}} -} - -func (n *nodeWrapper) obj() *v1.Node { - return &n.Node -} - -func (n *nodeWrapper) name(s string) *nodeWrapper { - n.Name = s - return n -} - -func (n *nodeWrapper) label(k, v string) *nodeWrapper { - if n.Labels == nil { - n.Labels = make(map[string]string) - } - n.Labels[k] = v - return n -} diff --git a/pkg/scheduler/testing/utils.go b/pkg/scheduler/testing/utils.go new file mode 100644 index 00000000000..e72e9fd2855 --- /dev/null +++ b/pkg/scheduler/testing/utils.go @@ -0,0 +1,243 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package testing + +import ( + "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// NodeSelectorWrapper wraps a NodeSelector inside. +type NodeSelectorWrapper struct{ v1.NodeSelector } + +// MakeNodeSelector creates a NodeSelector wrapper. +func MakeNodeSelector() *NodeSelectorWrapper { + return &NodeSelectorWrapper{v1.NodeSelector{}} +} + +// In injects a matchExpression (with an operator IN) as a selectorTerm +// to the inner nodeSelector. +// NOTE: appended selecterTerms are ORed. +func (s *NodeSelectorWrapper) In(key string, vals []string) *NodeSelectorWrapper { + expression := v1.NodeSelectorRequirement{ + Key: key, + Operator: v1.NodeSelectorOpIn, + Values: vals, + } + selectorTerm := v1.NodeSelectorTerm{} + selectorTerm.MatchExpressions = append(selectorTerm.MatchExpressions, expression) + s.NodeSelectorTerms = append(s.NodeSelectorTerms, selectorTerm) + return s +} + +// NotIn injects a matchExpression (with an operator NotIn) as a selectorTerm +// to the inner nodeSelector. +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 +} + +// Obj returns the inner NodeSelector. +func (s *NodeSelectorWrapper) Obj() *v1.NodeSelector { + return &s.NodeSelector +} + +// LabelSelectorWrapper wraps a LabelSelector inside. +type LabelSelectorWrapper struct{ metav1.LabelSelector } + +// MakeLabelSelector creates a LabelSelector wrapper. +func MakeLabelSelector() *LabelSelectorWrapper { + return &LabelSelectorWrapper{metav1.LabelSelector{}} +} + +// Label applies a {k,v} pair to the inner LabelSelector. +func (s *LabelSelectorWrapper) Label(k, v string) *LabelSelectorWrapper { + if s.MatchLabels == nil { + s.MatchLabels = make(map[string]string) + } + s.MatchLabels[k] = v + return s +} + +// In injects a matchExpression (with an operator In) to the inner labelSelector. +func (s *LabelSelectorWrapper) In(key string, vals []string) *LabelSelectorWrapper { + expression := metav1.LabelSelectorRequirement{ + Key: key, + Operator: metav1.LabelSelectorOpIn, + Values: vals, + } + s.MatchExpressions = append(s.MatchExpressions, expression) + return s +} + +// NotIn injects a matchExpression (with an operator NotIn) to the inner labelSelector. +func (s *LabelSelectorWrapper) NotIn(key string, vals []string) *LabelSelectorWrapper { + expression := metav1.LabelSelectorRequirement{ + Key: key, + Operator: metav1.LabelSelectorOpNotIn, + Values: vals, + } + s.MatchExpressions = append(s.MatchExpressions, expression) + return s +} + +// Exists injects a matchExpression (with an operator Exists) to the inner labelSelector. +func (s *LabelSelectorWrapper) Exists(k string) *LabelSelectorWrapper { + expression := metav1.LabelSelectorRequirement{ + Key: k, + Operator: metav1.LabelSelectorOpExists, + } + s.MatchExpressions = append(s.MatchExpressions, expression) + return s +} + +// NotExist injects a matchExpression (with an operator NotExist) to the inner labelSelector. +func (s *LabelSelectorWrapper) NotExist(k string) *LabelSelectorWrapper { + expression := metav1.LabelSelectorRequirement{ + Key: k, + Operator: metav1.LabelSelectorOpDoesNotExist, + } + s.MatchExpressions = append(s.MatchExpressions, expression) + return s +} + +// Obj returns the inner LabelSelector. +func (s *LabelSelectorWrapper) Obj() *metav1.LabelSelector { + return &s.LabelSelector +} + +// PodWrapper wraps a Pod inside. +type PodWrapper struct{ v1.Pod } + +// MakePod creates a Pod wrapper. +func MakePod() *PodWrapper { + return &PodWrapper{v1.Pod{}} +} + +// Obj returns the inner Pod. +func (p *PodWrapper) Obj() *v1.Pod { + return &p.Pod +} + +// Name sets `s` as the name of the inner pod. +func (p *PodWrapper) Name(s string) *PodWrapper { + p.SetName(s) + return p +} + +// Namespace sets `s` as the namespace of the inner pod. +func (p *PodWrapper) Namespace(s string) *PodWrapper { + p.SetNamespace(s) + return p +} + +// Node sets `s` as the nodeName of the inner pod. +func (p *PodWrapper) Node(s string) *PodWrapper { + p.Spec.NodeName = s + return p +} + +// NodeSelector sets `m` as the nodeSelector of the inner pod. +func (p *PodWrapper) NodeSelector(m map[string]string) *PodWrapper { + p.Spec.NodeSelector = m + return p +} + +// NodeAffinityIn creates a HARD node affinity (with the operator In) +// and injects into the innner pod. +func (p *PodWrapper) NodeAffinityIn(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().In(key, vals).Obj() + p.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution = nodeSelector + return p +} + +// NodeAffinityNotIn creates a HARD node affinity (with the operator NotIn) +// and injects into the innner pod. +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 +} + +// SpreadConstraint constructs a TopologySpreadConstraint object and injects +// into the inner pod. +func (p *PodWrapper) SpreadConstraint(maxSkew int, tpKey string, mode v1.UnsatisfiableConstraintAction, selector *metav1.LabelSelector) *PodWrapper { + c := v1.TopologySpreadConstraint{ + MaxSkew: int32(maxSkew), + TopologyKey: tpKey, + WhenUnsatisfiable: mode, + LabelSelector: selector, + } + p.Spec.TopologySpreadConstraints = append(p.Spec.TopologySpreadConstraints, c) + return p +} + +// Label sets a {k,v} pair to the inner pod. +func (p *PodWrapper) Label(k, v string) *PodWrapper { + if p.Labels == nil { + p.Labels = make(map[string]string) + } + p.Labels[k] = v + return p +} + +// NodeWrapper wraps a LabelSelector inside. +type NodeWrapper struct{ v1.Node } + +// MakeNode creates a Node wrapper. +func MakeNode() *NodeWrapper { + return &NodeWrapper{v1.Node{}} +} + +// Obj returns the inner Node. +func (n *NodeWrapper) Obj() *v1.Node { + return &n.Node +} + +// Name sets `s` as the name of the inner pod. +func (n *NodeWrapper) Name(s string) *NodeWrapper { + n.SetName(s) + return n +} + +// Label applies a {k,v} label pair to the inner node. +func (n *NodeWrapper) Label(k, v string) *NodeWrapper { + if n.Labels == nil { + n.Labels = make(map[string]string) + } + n.Labels[k] = v + return n +} diff --git a/pkg/scheduler/util/utils.go b/pkg/scheduler/util/utils.go index 79b8c333e34..eb61f13a085 100644 --- a/pkg/scheduler/util/utils.go +++ b/pkg/scheduler/util/utils.go @@ -21,7 +21,7 @@ import ( "time" - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/klog" "k8s.io/kubernetes/pkg/apis/scheduling" From 249752cc1fc259eaf755f8ef6b62568e6b7e6298 Mon Sep 17 00:00:00 2001 From: Wei Huang Date: Fri, 19 Jul 2019 00:20:37 -0700 Subject: [PATCH 4/6] fixup: fix comments and use a channel to pass err --- .../algorithm/predicates/metadata.go | 34 +++++++------------ .../testing/{utils.go => wrappers.go} | 2 +- 2 files changed, 13 insertions(+), 23 deletions(-) rename pkg/scheduler/testing/{utils.go => wrappers.go} (99%) diff --git a/pkg/scheduler/algorithm/predicates/metadata.go b/pkg/scheduler/algorithm/predicates/metadata.go index 5fd5b7b83fa..1e422849e07 100644 --- a/pkg/scheduler/algorithm/predicates/metadata.go +++ b/pkg/scheduler/algorithm/predicates/metadata.go @@ -202,8 +202,8 @@ func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*sche allNodeNames = append(allNodeNames, name) } + errCh := schedutil.NewErrorChannel() var lock sync.Mutex - var firstError error topologyPairsPodSpreadMap := &topologyPairsPodSpreadMap{ // topologyKeyToMinPodsMap will be initilized with proper size later. @@ -215,13 +215,6 @@ func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*sche topologyPairsPodSpreadMap.appendMaps(toAppend) lock.Unlock() } - catchError := func(err error) { - lock.Lock() - if firstError == nil { - firstError = err - } - lock.Unlock() - } ctx, cancel := context.WithCancel(context.Background()) @@ -229,12 +222,11 @@ func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*sche nodeInfo := nodeInfoMap[allNodeNames[i]] node := nodeInfo.Node() if node == nil { - catchError(fmt.Errorf("node %q not found", allNodeNames[i])) - cancel() + klog.Errorf("node %q not found", allNodeNames[i]) return } - // Be design if NodeAffinity or NodeSelector is defined, spreading is - // applied to nodes that pass those filters. + // In accordance to design, if NodeAffinity or NodeSelector is defined, + // spreading is applied to nodes that pass those filters. if !podMatchesNodeSelectorAndAffinityTerms(pod, node) { return } @@ -250,8 +242,7 @@ func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*sche for _, existingPod := range nodeInfo.Pods() { ok, err := podMatchesAllSpreadConstraints(existingPod, pod.Namespace, constraints) if err != nil { - catchError(err) - cancel() + errCh.SendErrorWithCancel(err, cancel) return } if ok { @@ -280,8 +271,8 @@ func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*sche } workqueue.ParallelizeUntil(ctx, 16, len(allNodeNames), processNode) - if firstError != nil { - return nil, firstError + if err := errCh.ReceiveError(); err != nil { + return nil, err } // calculate min match for each topology pair @@ -395,19 +386,18 @@ func (m *topologyPairsMaps) clone() *topologyPairsMaps { return copy } -func (podSpreadMap *topologyPairsPodSpreadMap) clone() *topologyPairsPodSpreadMap { - // podSpreadMap could be nil when EvenPodsSpread feature is disabled - if podSpreadMap == nil { +func (m *topologyPairsPodSpreadMap) clone() *topologyPairsPodSpreadMap { + // m could be nil when EvenPodsSpread feature is disabled + if m == nil { return nil } copy := &topologyPairsPodSpreadMap{ topologyKeyToMinPodsMap: make(map[string]int32), - topologyPairsMaps: newTopologyPairsMaps(), + topologyPairsMaps: m.topologyPairsMaps.clone(), } - for key, minMatched := range podSpreadMap.topologyKeyToMinPodsMap { + for key, minMatched := range m.topologyKeyToMinPodsMap { copy.topologyKeyToMinPodsMap[key] = minMatched } - copy.topologyPairsMaps.appendMaps(podSpreadMap.topologyPairsMaps) return copy } diff --git a/pkg/scheduler/testing/utils.go b/pkg/scheduler/testing/wrappers.go similarity index 99% rename from pkg/scheduler/testing/utils.go rename to pkg/scheduler/testing/wrappers.go index e72e9fd2855..242e8c0da2c 100644 --- a/pkg/scheduler/testing/utils.go +++ b/pkg/scheduler/testing/wrappers.go @@ -214,7 +214,7 @@ func (p *PodWrapper) Label(k, v string) *PodWrapper { return p } -// NodeWrapper wraps a LabelSelector inside. +// NodeWrapper wraps a Node inside. type NodeWrapper struct{ v1.Node } // MakeNode creates a Node wrapper. From c23aef40f2f62a95e252dfebd24e572cf53f1b3d Mon Sep 17 00:00:00 2001 From: Wei Huang Date: Fri, 10 May 2019 13:52:52 -0700 Subject: [PATCH 5/6] [eps-pred-meta] auto-gen files --- pkg/scheduler/testing/BUILD | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/scheduler/testing/BUILD b/pkg/scheduler/testing/BUILD index 8b9a1e8e885..3f900db5ff6 100644 --- a/pkg/scheduler/testing/BUILD +++ b/pkg/scheduler/testing/BUILD @@ -4,7 +4,10 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( name = "go_default_library", - srcs = ["fake_lister.go"], + srcs = [ + "fake_lister.go", + "wrappers.go", + ], importpath = "k8s.io/kubernetes/pkg/scheduler/testing", deps = [ "//pkg/scheduler/algorithm:go_default_library", From f822487f05313290140e08e508249e9970b5b5c3 Mon Sep 17 00:00:00 2001 From: Wei Huang Date: Mon, 22 Jul 2019 17:31:36 -0700 Subject: [PATCH 6/6] EvenPodsSpread: match selector of each constraint independently - see more discussion at https://github.com/kubernetes/kubernetes/pull/77760#discussion_r305107973 --- .../algorithm/predicates/metadata.go | 66 ++++----- .../algorithm/predicates/metadata_test.go | 126 +++++++++++------- 2 files changed, 103 insertions(+), 89 deletions(-) diff --git a/pkg/scheduler/algorithm/predicates/metadata.go b/pkg/scheduler/algorithm/predicates/metadata.go index 1e422849e07..cd5d0367d1a 100644 --- a/pkg/scheduler/algorithm/predicates/metadata.go +++ b/pkg/scheduler/algorithm/predicates/metadata.go @@ -104,7 +104,7 @@ type predicateMetadata struct { // from scheduler extender configuration and does not change per pod. ignoredExtendedResources sets.String // Similar to the map for pod (anti-)affinity, but imposes additional min matches info - // to describe mininum match number on each topology spread constraint + // to describe minimum match number on each topology spread constraint. topologyPairsPodSpreadMap *topologyPairsPodSpreadMap } @@ -190,8 +190,8 @@ func (pfactory *PredicateMetadataFactory) GetMetadata(pod *v1.Pod, nodeNameToInf } func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*schedulernodeinfo.NodeInfo) (*topologyPairsPodSpreadMap, error) { - // we have feature gating in APIserver to strip the spec - // so don't need to re-check feature gate, just check length of constraints + // We have feature gating in APIServer to strip the spec + // so don't need to re-check feature gate, just check length of constraints. constraints := getHardTopologySpreadConstraints(pod) if len(constraints) == 0 { return nil, nil @@ -206,7 +206,7 @@ func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*sche var lock sync.Mutex topologyPairsPodSpreadMap := &topologyPairsPodSpreadMap{ - // topologyKeyToMinPodsMap will be initilized with proper size later. + // topologyKeyToMinPodsMap will be initialized with proper size later. topologyPairsMaps: newTopologyPairsMaps(), } @@ -230,7 +230,7 @@ func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*sche if !podMatchesNodeSelectorAndAffinityTerms(pod, node) { return } - // ensure current node's labels contains all topologyKeys in 'constraints' + // Ensure current node's labels contains all topologyKeys in 'constraints'. for _, constraint := range constraints { if _, ok := node.Labels[constraint.TopologyKey]; !ok { return @@ -240,13 +240,17 @@ func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*sche nodeTopologyMaps := newTopologyPairsMaps() // nodeInfo.Pods() can be empty; or all pods don't fit for _, existingPod := range nodeInfo.Pods() { - ok, err := podMatchesAllSpreadConstraints(existingPod, pod.Namespace, constraints) - if err != nil { - errCh.SendErrorWithCancel(err, cancel) - return + if existingPod.Namespace != pod.Namespace { + continue } - if ok { - for _, constraint := range constraints { + podLabelSet := labels.Set(existingPod.Labels) + for _, constraint := range constraints { + ok, err := podMatchesSpreadConstraint(podLabelSet, constraint) + if err != nil { + errCh.SendErrorWithCancel(err, cancel) + return + } + if ok { // constraint.TopologyKey is already guaranteed to be present pair := topologyPair{key: constraint.TopologyKey, value: node.Labels[constraint.TopologyKey]} nodeTopologyMaps.addTopologyPair(pair, existingPod) @@ -254,7 +258,7 @@ func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*sche } } // If needed, append topology pair without entry of pods. - // For example, on node-x, there is no pod matching spread constraints + // For example, on node-x, there is no pod matching spread constraints, // but node-x should be also considered as a match (with match number 0) // i.e. : {} for _, constraint := range constraints { @@ -281,8 +285,8 @@ func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*sche topologyPairsPodSpreadMap.topologyKeyToMinPodsMap[constraint.TopologyKey] = math.MaxInt32 } for pair, podSet := range topologyPairsPodSpreadMap.topologyPairToPods { - // TODO(Huang-Wei): short circuit all portions of - // if we see 0 as min match of the topologyKey + // TODO(Huang-Wei): short circuit unvisited portions of + // if we already see 0 as min match of that topologyKey. if l := int32(len(podSet)); l < topologyPairsPodSpreadMap.topologyKeyToMinPodsMap[pair.key] { topologyPairsPodSpreadMap.topologyKeyToMinPodsMap[pair.key] = l } @@ -301,30 +305,16 @@ func getHardTopologySpreadConstraints(pod *v1.Pod) (constraints []v1.TopologySpr return } -func podMatchesAllSpreadConstraints(pod *v1.Pod, ns string, constraints []v1.TopologySpreadConstraint) (bool, error) { - if len(constraints) == 0 || pod.Namespace != ns { - return false, nil - } - return podLabelsMatchesSpreadConstraints(pod.Labels, constraints) -} - // some corner cases: -// 1. podLabels = nil, constraint.LabelSelector = nil => returns false -// 2. podLabels = nil => returns false -// 3. constraint.LabelSelector = nil => returns false -func podLabelsMatchesSpreadConstraints(podLabels map[string]string, constraints []v1.TopologySpreadConstraint) (bool, error) { - if len(constraints) == 0 { - return false, nil +// 1. podLabelSet = nil => returns (false, nil) +// 2. constraint.LabelSelector = nil => returns (false, nil) +func podMatchesSpreadConstraint(podLabelSet labels.Set, constraint v1.TopologySpreadConstraint) (bool, error) { + selector, err := metav1.LabelSelectorAsSelector(constraint.LabelSelector) + if err != nil { + return false, err } - podLabelSet := labels.Set(podLabels) - for _, constraint := range constraints { - selector, err := metav1.LabelSelectorAsSelector(constraint.LabelSelector) - if err != nil { - return false, err - } - if !selector.Matches(podLabelSet) { - return false, nil - } + if !selector.Matches(podLabelSet) { + return false, nil } return true, nil } @@ -337,9 +327,7 @@ func newTopologyPairsMaps() *topologyPairsMaps { func (m *topologyPairsMaps) addTopologyPair(pair topologyPair, pod *v1.Pod) { podFullName := schedutil.GetPodFullName(pod) - if m.topologyPairToPods[pair] == nil { - m.topologyPairToPods[pair] = make(map[*v1.Pod]struct{}) - } + m.addTopologyPairWithoutPods(pair) m.topologyPairToPods[pair][pod] = struct{}{} if m.podToTopologyPairs[podFullName] == nil { m.podToTopologyPairs[podFullName] = make(map[topologyPair]struct{}) diff --git a/pkg/scheduler/algorithm/predicates/metadata_test.go b/pkg/scheduler/algorithm/predicates/metadata_test.go index 1ca96f948e9..4a284667f60 100644 --- a/pkg/scheduler/algorithm/predicates/metadata_test.go +++ b/pkg/scheduler/algorithm/predicates/metadata_test.go @@ -24,6 +24,7 @@ import ( "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo" st "k8s.io/kubernetes/pkg/scheduler/testing" ) @@ -825,25 +826,23 @@ func TestGetTPMapMatchingIncomingAffinityAntiAffinity(t *testing.T) { } } -func TestPodLabelsMatchesSpreadConstraints(t *testing.T) { +func TestPodMatchesSpreadConstraint(t *testing.T) { tests := []struct { - name string - podLabels map[string]string - constraints []v1.TopologySpreadConstraint - want bool - wantErr bool + name string + podLabels map[string]string + constraint v1.TopologySpreadConstraint + want bool + wantErr bool }{ { name: "normal match", podLabels: map[string]string{"foo": "", "bar": ""}, - constraints: []v1.TopologySpreadConstraint{ - { - LabelSelector: &metav1.LabelSelector{ - MatchExpressions: []metav1.LabelSelectorRequirement{ - { - Key: "foo", - Operator: metav1.LabelSelectorOpExists, - }, + constraint: v1.TopologySpreadConstraint{ + LabelSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "foo", + Operator: metav1.LabelSelectorOpExists, }, }, }, @@ -853,18 +852,16 @@ func TestPodLabelsMatchesSpreadConstraints(t *testing.T) { { name: "normal mismatch", podLabels: map[string]string{"foo": "", "baz": ""}, - constraints: []v1.TopologySpreadConstraint{ - { - LabelSelector: &metav1.LabelSelector{ - MatchExpressions: []metav1.LabelSelectorRequirement{ - { - Key: "foo", - Operator: metav1.LabelSelectorOpExists, - }, - { - Key: "bar", - Operator: metav1.LabelSelectorOpExists, - }, + constraint: v1.TopologySpreadConstraint{ + LabelSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "foo", + Operator: metav1.LabelSelectorOpExists, + }, + { + Key: "bar", + Operator: metav1.LabelSelectorOpExists, }, }, }, @@ -873,14 +870,12 @@ func TestPodLabelsMatchesSpreadConstraints(t *testing.T) { }, { name: "podLabels is nil", - constraints: []v1.TopologySpreadConstraint{ - { - LabelSelector: &metav1.LabelSelector{ - MatchExpressions: []metav1.LabelSelectorRequirement{ - { - Key: "foo", - Operator: metav1.LabelSelectorOpExists, - }, + constraint: v1.TopologySpreadConstraint{ + LabelSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "foo", + Operator: metav1.LabelSelectorOpExists, }, }, }, @@ -893,31 +888,28 @@ func TestPodLabelsMatchesSpreadConstraints(t *testing.T) { "foo": "", "bar": "", }, - constraints: []v1.TopologySpreadConstraint{ - { - MaxSkew: 1, - }, + constraint: v1.TopologySpreadConstraint{ + MaxSkew: 1, }, want: false, }, { name: "both podLabels and constraint.LabelSelector are nil", - constraints: []v1.TopologySpreadConstraint{ - { - MaxSkew: 1, - }, + constraint: v1.TopologySpreadConstraint{ + MaxSkew: 1, }, want: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := podLabelsMatchesSpreadConstraints(tt.podLabels, tt.constraints) + podLabelSet := labels.Set(tt.podLabels) + got, err := podMatchesSpreadConstraint(podLabelSet, tt.constraint) if (err != nil) != tt.wantErr { - t.Errorf("podLabelsMatchesSpreadConstraints() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("podMatchesSpreadConstraint() error = %v, wantErr %v", err, tt.wantErr) } if got != tt.want { - t.Errorf("podLabelsMatchesSpreadConstraints() = %v, want %v", got, tt.want) + t.Errorf("podMatchesSpreadConstraint() = %v, want %v", got, tt.want) } }) } @@ -996,7 +988,7 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { }, }, { - name: "namespace mis-match doesn't count", + name: "namespace mismatch doesn't count", pod: st.MakePod().Name("p").Label("foo", "").SpreadConstraint( 1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj(), ).Obj(), @@ -1117,7 +1109,37 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { }, }, { - name: "different labelSelectors", + name: "different labelSelectors - simple version", + pod: st.MakePod().Name("p").Label("foo", "").Label("bar", ""). + SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). + SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("bar").Obj()). + Obj(), + nodes: []*v1.Node{ + st.MakeNode().Name("node-a").Label("zone", "zone1").Label("node", "node-a").Obj(), + st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(), + st.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(), + }, + existingPods: []*v1.Pod{ + st.MakePod().Name("p-a").Node("node-a").Label("foo", "").Obj(), + }, + injectPodPointers: map[topologyPair][]int{ + {key: "zone", value: "zone1"}: {0}, + {key: "zone", value: "zone2"}: {}, + {key: "node", value: "node-a"}: {}, + {key: "node", value: "node-b"}: {}, + {key: "node", value: "node-y"}: {}, + }, + want: &topologyPairsPodSpreadMap{ + topologyKeyToMinPodsMap: map[string]int32{"zone": 0, "node": 0}, + topologyPairsMaps: &topologyPairsMaps{ + podToTopologyPairs: map[string]topologyPairSet{ + "p-a_": newPairSet("zone", "zone1"), + }, + }, + }, + }, + { + name: "different labelSelectors - complex version", pod: st.MakePod().Name("p").Label("foo", "").Label("bar", ""). SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("bar").Obj()). @@ -1137,18 +1159,22 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { st.MakePod().Name("p-y4").Node("node-y").Label("foo", "").Label("bar", "").Obj(), }, injectPodPointers: map[topologyPair][]int{ - {key: "zone", value: "zone1"}: {1}, - {key: "zone", value: "zone2"}: {4, 6}, + {key: "zone", value: "zone1"}: {0, 1, 2}, + {key: "zone", value: "zone2"}: {3, 4, 5, 6}, {key: "node", value: "node-a"}: {1}, {key: "node", value: "node-b"}: {}, {key: "node", value: "node-y"}: {4, 6}, }, want: &topologyPairsPodSpreadMap{ - topologyKeyToMinPodsMap: map[string]int32{"zone": 1, "node": 0}, + topologyKeyToMinPodsMap: map[string]int32{"zone": 3, "node": 0}, topologyPairsMaps: &topologyPairsMaps{ podToTopologyPairs: map[string]topologyPairSet{ + "p-a1_": newPairSet("zone", "zone1"), "p-a2_": newPairSet("zone", "zone1", "node", "node-a"), + "p-b1_": newPairSet("zone", "zone1"), + "p-y1_": newPairSet("zone", "zone2"), "p-y2_": newPairSet("zone", "zone2", "node", "node-y"), + "p-y3_": newPairSet("zone", "zone2"), "p-y4_": newPairSet("zone", "zone2", "node", "node-y"), }, },