From 08e7b3bacb1956c3010672d5c8fcf8483909fc9f Mon Sep 17 00:00:00 2001 From: Wei Huang Date: Tue, 30 Apr 2019 16:13:45 -0700 Subject: [PATCH 1/5] EvenPodsSpread: Define a new Predicate --- pkg/scheduler/algorithm/predicates/error.go | 2 ++ .../algorithm/predicates/predicates.go | 18 +++++++++++++++++- .../algorithmprovider/defaults/defaults.go | 6 ++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/pkg/scheduler/algorithm/predicates/error.go b/pkg/scheduler/algorithm/predicates/error.go index 7f35c84f93e..372cfc8ddf9 100644 --- a/pkg/scheduler/algorithm/predicates/error.go +++ b/pkg/scheduler/algorithm/predicates/error.go @@ -75,6 +75,8 @@ var ( ErrVolumeNodeConflict = newPredicateFailureError("VolumeNodeAffinityConflict", "node(s) had volume node affinity conflict") // ErrVolumeBindConflict is used for VolumeBindingNoMatch predicate error. ErrVolumeBindConflict = newPredicateFailureError("VolumeBindingNoMatch", "node(s) didn't find available persistent volumes to bind") + // ErrSpreadConstraintsNotMatch is used for EvenPodsSpread predicate error. + ErrSpreadConstraintsNotMatch = newPredicateFailureError("EvenPodsSpread", "node(s) didn't match pod topology spread constraints") // ErrFakePredicate is used for test only. The fake predicates returning false also returns error // as ErrFakePredicate. ErrFakePredicate = newPredicateFailureError("FakePredicateError", "Nodes failed the fake predicate") diff --git a/pkg/scheduler/algorithm/predicates/predicates.go b/pkg/scheduler/algorithm/predicates/predicates.go index 9a5e43c4734..6487a6bcdec 100644 --- a/pkg/scheduler/algorithm/predicates/predicates.go +++ b/pkg/scheduler/algorithm/predicates/predicates.go @@ -106,6 +106,8 @@ const ( CheckNodeDiskPressurePred = "CheckNodeDiskPressure" // CheckNodePIDPressurePred defines the name of predicate CheckNodePIDPressure. CheckNodePIDPressurePred = "CheckNodePIDPressure" + // EvenPodsSpreadPred defines the name of predicate EvenPodsSpread + EvenPodsSpreadPred = "EvenPodsSpread" // DefaultMaxGCEPDVolumes defines the maximum number of PD Volumes for GCE // GCE instances can have up to 16 PD volumes attached. @@ -148,7 +150,7 @@ var ( PodToleratesNodeTaintsPred, PodToleratesNodeNoExecuteTaintsPred, CheckNodeLabelPresencePred, CheckServiceAffinityPred, MaxEBSVolumeCountPred, MaxGCEPDVolumeCountPred, MaxCSIVolumeCountPred, MaxAzureDiskVolumeCountPred, MaxCinderVolumeCountPred, CheckVolumeBindingPred, NoVolumeZoneConflictPred, - CheckNodeMemoryPressurePred, CheckNodePIDPressurePred, CheckNodeDiskPressurePred, MatchInterPodAffinityPred} + CheckNodeMemoryPressurePred, CheckNodePIDPressurePred, CheckNodeDiskPressurePred, EvenPodsSpreadPred, MatchInterPodAffinityPred} ) // FitPredicate is a function that indicates if a pod fits into an existing node. @@ -1712,3 +1714,17 @@ func (c *VolumeBindingChecker) predicate(pod *v1.Pod, meta PredicateMetadata, no klog.V(5).Infof("All PVCs found matches for pod %v/%v, node %q", pod.Namespace, pod.Name, node.Name) return true, nil, nil } + +// EvenPodsSpreadPredicate checks if a pod can be scheduled on a node which satisfies +// its topologySpreadConstraints. +func EvenPodsSpreadPredicate(pod *v1.Pod, meta PredicateMetadata, nodeInfo *schedulernodeinfo.NodeInfo) (bool, []PredicateFailureReason, error) { + node := nodeInfo.Node() + if node == nil { + return false, nil, fmt.Errorf("node not found") + } + constraints := getHardTopologySpreadConstraints(pod) + if len(constraints) == 0 { + return true, nil, nil + } + return true, nil, nil +} diff --git a/pkg/scheduler/algorithmprovider/defaults/defaults.go b/pkg/scheduler/algorithmprovider/defaults/defaults.go index 3a290c6b110..a77ee69e069 100644 --- a/pkg/scheduler/algorithmprovider/defaults/defaults.go +++ b/pkg/scheduler/algorithmprovider/defaults/defaults.go @@ -87,6 +87,12 @@ func ApplyFeatureGates() { klog.Infof("TaintNodesByCondition is enabled, PodToleratesNodeTaints predicate is mandatory") } + // Only register EvenPodsSpreadPredicate if the feature is enabled + if utilfeature.DefaultFeatureGate.Enabled(features.EvenPodsSpread) { + factory.InsertPredicateKeyToAlgorithmProviderMap(predicates.EvenPodsSpreadPred) + factory.RegisterFitPredicate(predicates.EvenPodsSpreadPred, predicates.EvenPodsSpreadPredicate) + } + // Prioritizes nodes that satisfy pod's resource limits if utilfeature.DefaultFeatureGate.Enabled(features.ResourceLimitsPriorityFunction) { klog.Infof("Registering resourcelimits priority function") From e0e3889d745039b73c9a2bb981c6a5201e64cf1b Mon Sep 17 00:00:00 2001 From: Wei Huang Date: Wed, 1 May 2019 10:56:06 -0700 Subject: [PATCH 2/5] EvenPodsSpread: Core Predicate logic --- pkg/scheduler/algorithm/predicates/BUILD | 1 + pkg/scheduler/algorithm/predicates/error.go | 4 +- .../algorithm/predicates/metadata.go | 3 + .../algorithm/predicates/predicates.go | 51 +++ .../algorithm/predicates/predicates_test.go | 370 +++++++++++++++++- 5 files changed, 422 insertions(+), 7 deletions(-) diff --git a/pkg/scheduler/algorithm/predicates/BUILD b/pkg/scheduler/algorithm/predicates/BUILD index 5e6c799e5c4..dd31d198949 100644 --- a/pkg/scheduler/algorithm/predicates/BUILD +++ b/pkg/scheduler/algorithm/predicates/BUILD @@ -64,6 +64,7 @@ go_test( "//pkg/scheduler/api:go_default_library", "//pkg/scheduler/nodeinfo:go_default_library", "//pkg/scheduler/testing:go_default_library", + "//pkg/scheduler/util:go_default_library", "//pkg/volume/util:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/api/storage/v1:go_default_library", diff --git a/pkg/scheduler/algorithm/predicates/error.go b/pkg/scheduler/algorithm/predicates/error.go index 372cfc8ddf9..d5325af3f2e 100644 --- a/pkg/scheduler/algorithm/predicates/error.go +++ b/pkg/scheduler/algorithm/predicates/error.go @@ -75,8 +75,8 @@ var ( ErrVolumeNodeConflict = newPredicateFailureError("VolumeNodeAffinityConflict", "node(s) had volume node affinity conflict") // ErrVolumeBindConflict is used for VolumeBindingNoMatch predicate error. ErrVolumeBindConflict = newPredicateFailureError("VolumeBindingNoMatch", "node(s) didn't find available persistent volumes to bind") - // ErrSpreadConstraintsNotMatch is used for EvenPodsSpread predicate error. - ErrSpreadConstraintsNotMatch = newPredicateFailureError("EvenPodsSpread", "node(s) didn't match pod topology spread constraints") + // ErrTopologySpreadConstraintsNotMatch is used for EvenPodsSpread predicate error. + ErrTopologySpreadConstraintsNotMatch = newPredicateFailureError("EvenPodsSpreadNotMatch", "node(s) didn't match pod topology spread constraints") // ErrFakePredicate is used for test only. The fake predicates returning false also returns error // as ErrFakePredicate. ErrFakePredicate = newPredicateFailureError("FakePredicateError", "Nodes failed the fake predicate") diff --git a/pkg/scheduler/algorithm/predicates/metadata.go b/pkg/scheduler/algorithm/predicates/metadata.go index cd5d0367d1a..147f2ef5d8a 100644 --- a/pkg/scheduler/algorithm/predicates/metadata.go +++ b/pkg/scheduler/algorithm/predicates/metadata.go @@ -35,6 +35,9 @@ import ( schedutil "k8s.io/kubernetes/pkg/scheduler/util" ) +// MaxInt32 is the maximum value of int32 +const MaxInt32 = math.MaxInt32 + // PredicateMetadata interface represents anything that can access a predicate metadata. type PredicateMetadata interface { ShallowCopy() PredicateMetadata diff --git a/pkg/scheduler/algorithm/predicates/predicates.go b/pkg/scheduler/algorithm/predicates/predicates.go index 6487a6bcdec..4fefdbbe577 100644 --- a/pkg/scheduler/algorithm/predicates/predicates.go +++ b/pkg/scheduler/algorithm/predicates/predicates.go @@ -1722,9 +1722,60 @@ func EvenPodsSpreadPredicate(pod *v1.Pod, meta PredicateMetadata, nodeInfo *sche if node == nil { return false, nil, fmt.Errorf("node not found") } + constraints := getHardTopologySpreadConstraints(pod) if len(constraints) == 0 { return true, nil, nil } + + var topologyPairsPodSpreadMap *topologyPairsPodSpreadMap + if predicateMeta, ok := meta.(*predicateMetadata); ok { + topologyPairsPodSpreadMap = predicateMeta.topologyPairsPodSpreadMap + } else { // We don't have precomputed metadata. We have to follow a slow path to check spread constraints. + // TODO(Huang-Wei): get it implemented + return false, nil, errors.New("metadata not pre-computed for EvenPodsSpreadPredicate") + } + + if topologyPairsPodSpreadMap == nil || len(topologyPairsPodSpreadMap.topologyKeyToMinPodsMap) == 0 { + return true, nil, nil + } + + selfMatch, err := podLabelsMatchesSpreadConstraints(pod.Labels, constraints) + if err != nil { + return false, nil, err + } + selfMatchNum := 0 + if selfMatch { + selfMatchNum = 1 + } + for _, constraint := range constraints { + tpKey := constraint.TopologyKey + tpVal, ok := node.Labels[constraint.TopologyKey] + if !ok { + klog.V(5).Infof("node '%s' doesn't have required label '%s'", node.Name, tpKey) + return false, []PredicateFailureReason{ErrTopologySpreadConstraintsNotMatch}, nil + } + + pair := topologyPair{key: tpKey, value: tpVal} + minMatchNum, ok := topologyPairsPodSpreadMap.topologyKeyToMinPodsMap[tpKey] + if !ok { + // error which should not happen + klog.Errorf("internal error: get minMatchNum from key %q of %#v", tpKey, topologyPairsPodSpreadMap.topologyKeyToMinPodsMap) + continue + } + // judging criteria: + // 'existing matching num' + 'if self-match (1 or 0)' - 'global min matching num' <= 'maxSkew' + matchNum := len(topologyPairsPodSpreadMap.topologyPairToPods[pair]) + + // TODO(Huang-Wei): remove this after thorough testing + // fmt.Printf("node '%s' => spreadConstraint[%s]: matchNum(%d) + selfMatchNum(%d) - minMatchNum(%d) ?<= maxSkew(%d)\n", node.Name, tpKey, matchNum, selfMatchNum, minMatchNum, constraint.MaxSkew) + + // TODO(Huang-Wei): check if it can overflow? + if int32(matchNum+selfMatchNum)-minMatchNum > constraint.MaxSkew { + klog.V(5).Infof("node '%s' failed spreadConstraint[%s]: matchNum(%d) + selfMatchNum(%d) - minMatchNum(%d) > maxSkew(%d)", node.Name, tpKey, matchNum, selfMatchNum, minMatchNum, constraint.MaxSkew) + return false, []PredicateFailureReason{ErrTopologySpreadConstraintsNotMatch}, nil + } + } + return true, nil, nil } diff --git a/pkg/scheduler/algorithm/predicates/predicates_test.go b/pkg/scheduler/algorithm/predicates/predicates_test.go index f45f30d5c7f..b091c02dac6 100644 --- a/pkg/scheduler/algorithm/predicates/predicates_test.go +++ b/pkg/scheduler/algorithm/predicates/predicates_test.go @@ -35,7 +35,7 @@ import ( "k8s.io/kubernetes/pkg/features" schedulerapi "k8s.io/kubernetes/pkg/scheduler/api" schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo" - schedulertesting "k8s.io/kubernetes/pkg/scheduler/testing" + st "k8s.io/kubernetes/pkg/scheduler/testing" ) var ( @@ -95,7 +95,7 @@ func newResourceOverheadPod(pod *v1.Pod, overhead v1.ResourceList) *v1.Pod { } func GetPredicateMetadata(p *v1.Pod, nodeInfo map[string]*schedulernodeinfo.NodeInfo) PredicateMetadata { - pm := PredicateMetadataFactory{schedulertesting.FakePodLister{p}} + pm := PredicateMetadataFactory{st.FakePodLister{p}} return pm.GetMetadata(p, nodeInfo) } @@ -1863,7 +1863,7 @@ func TestServiceAffinity(t *testing.T) { nodeInfo.SetNode(test.node) nodeInfoMap := map[string]*schedulernodeinfo.NodeInfo{test.node.Name: nodeInfo} // Reimplementing the logic that the scheduler implements: Any time it makes a predicate, it registers any precomputations. - predicate, precompute := NewServiceAffinityPredicate(schedulertesting.FakePodLister(test.pods), schedulertesting.FakeServiceLister(test.services), FakeNodeListInfo(nodes), test.labels) + predicate, precompute := NewServiceAffinityPredicate(st.FakePodLister(test.pods), st.FakeServiceLister(test.services), FakeNodeListInfo(nodes), test.labels) // Register a precomputation or Rewrite the precomputation to a no-op, depending on the state we want to test. RegisterPredicateMetadataProducer("ServiceAffinityMetaProducer", func(pm *predicateMetadata) { if !skipPrecompute { @@ -2937,7 +2937,7 @@ func TestInterPodAffinity(t *testing.T) { fit := PodAffinityChecker{ info: FakeNodeInfo(*node), - podLister: schedulertesting.FakePodLister(test.pods), + podLister: st.FakePodLister(test.pods), } nodeInfo := schedulernodeinfo.NewNodeInfo(podsOnNode...) nodeInfo.SetNode(test.node) @@ -4051,7 +4051,7 @@ func TestInterPodAffinityWithMultipleNodes(t *testing.T) { for indexNode, node := range test.nodes { testFit := PodAffinityChecker{ info: nodeListInfo, - podLister: schedulertesting.FakePodLister(test.pods), + podLister: st.FakePodLister(test.pods), } var meta PredicateMetadata @@ -5039,3 +5039,363 @@ func TestCheckNodeUnschedulablePredicate(t *testing.T) { } } } + +func TestEvenPodsSpreadPredicate_SingleConstraint(t *testing.T) { + tests := []struct { + name string + pod *v1.Pod + nodes []*v1.Node + existingPods []*v1.Pod + fits map[string]bool + }{ + { + name: "no existing pods", + pod: st.MakePod().Name("p").Label("foo", "").SpreadConstraint( + 1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").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-x").Label("zone", "zone2").Label("node", "node-x").Obj(), + st.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(), + }, + fits: map[string]bool{ + "node-a": true, + "node-b": true, + "node-x": true, + "node-y": true, + }, + }, + { + name: "no existing pods, incoming pod doesn't match itself", + pod: st.MakePod().Name("p").Label("foo", "").SpreadConstraint( + 1, "zone", 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-x").Label("zone", "zone2").Label("node", "node-x").Obj(), + st.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(), + }, + fits: map[string]bool{ + "node-a": true, + "node-b": true, + "node-x": true, + "node-y": true, + }, + }, + { + name: "existing pods with mis-matched namespace doens't count", + pod: st.MakePod().Name("p").Label("foo", "").SpreadConstraint( + 1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").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-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{ + st.MakePod().Name("p-a1").Namespace("ns1").Node("node-a").Label("foo", "").Obj(), + st.MakePod().Name("p-b1").Namespace("ns2").Node("node-a").Label("foo", "").Obj(), + st.MakePod().Name("p-x1").Node("node-x").Label("foo", "").Obj(), + st.MakePod().Name("p-y1").Node("node-y").Label("foo", "").Obj(), + }, + fits: map[string]bool{ + "node-a": true, + "node-b": true, + "node-x": false, + "node-y": false, + }, + }, + { + name: "pods spread across zones as 3/3, all nodes fit", + pod: st.MakePod().Name("p").Label("foo", "").SpreadConstraint( + 1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").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-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{ + 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(), + }, + fits: map[string]bool{ + "node-a": true, + "node-b": true, + "node-x": true, + "node-y": true, + }, + }, + { + // TODO(Huang-Wei): maybe document this to remind users that typos on node labels + // can cause unexpected behavior + name: "pods spread across zones as 1/2 due to absence of label 'zone' on node-b", + pod: st.MakePod().Name("p").Label("foo", "").SpreadConstraint( + 1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj(), + ).Obj(), + nodes: []*v1.Node{ + st.MakeNode().Name("node-a").Label("zone", "zone1").Label("node", "node-a").Obj(), + st.MakeNode().Name("node-b").Label("zon", "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{ + st.MakePod().Name("p-a1").Node("node-a").Label("foo", "").Obj(), + st.MakePod().Name("p-b1").Node("node-b").Label("foo", "").Obj(), + st.MakePod().Name("p-x1").Node("node-x").Label("foo", "").Obj(), + st.MakePod().Name("p-y1").Node("node-y").Label("foo", "").Obj(), + }, + fits: map[string]bool{ + "node-a": true, + "node-b": false, + "node-x": false, + "node-y": false, + }, + }, + { + name: "pods spread across nodes as 2/1/0/3, only node-x fits", + pod: st.MakePod().Name("p").Label("foo", "").SpreadConstraint( + 1, "node", hardSpread, st.MakeLabelSelector().Exists("foo").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-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{ + 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(), + }, + fits: map[string]bool{ + "node-a": false, + "node-b": false, + "node-x": true, + "node-y": false, + }, + }, + { + name: "pods spread across nodes as 2/1/0/3, maxSkew is 2, node-b and node-x fit", + pod: st.MakePod().Name("p").Label("foo", "").SpreadConstraint( + 2, "node", hardSpread, st.MakeLabelSelector().Exists("foo").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-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{ + 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(), + }, + fits: map[string]bool{ + "node-a": false, + "node-b": true, + "node-x": true, + "node-y": false, + }, + }, + { + // not a desired case, but it can happen + // TODO(Huang-Wei): document this "pod-not-match-itself" case + // in this case, placement of the new pod doesn't change pod distribution of the cluster + // as the incoming pod doesn't have label "foo" + name: "pods spread across nodes as 2/1/0/3, but pod doesn't match itself", + pod: st.MakePod().Name("p").Label("bar", "").SpreadConstraint( + 1, "node", hardSpread, st.MakeLabelSelector().Exists("foo").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-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{ + 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(), + }, + fits: map[string]bool{ + "node-a": false, + "node-b": true, + "node-x": true, + "node-y": false, + }, + }, + { + // only node-a and node-y are considered, so pods spread as 2/~1~/~0~/3 + // ps: '~num~' is a markdown symbol to denote a crossline through 'num' + // but in this unit test, we don't run NodeAffinityPredicate, so node-b and node-x are + // still expected to be fits; + // the fact that node-a fits can prove the underlying logic works + name: "incoming pod has nodeAffinity, pods spread as 2/~1~/~0~/3, hence node-a fits", + pod: st.MakePod().Name("p").Label("foo", ""). + NodeAffinityIn("node", []string{"node-a", "node-y"}). + SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("foo").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-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{ + 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(), + }, + fits: map[string]bool{ + "node-a": true, + "node-b": true, // in real case, it's false + "node-x": true, // in real case, it's false + "node-y": false, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + nodeInfoMap := schedulernodeinfo.CreateNodeNameToInfoMap(tt.existingPods, tt.nodes) + meta := GetPredicateMetadata(tt.pod, nodeInfoMap) + for _, node := range tt.nodes { + fits, _, _ := EvenPodsSpreadPredicate(tt.pod, meta, nodeInfoMap[node.Name]) + if fits != tt.fits[node.Name] { + t.Errorf("[%s]: expected %v got %v", node.Name, tt.fits[node.Name], fits) + } + } + }) + } +} + +func TestEvenPodsSpreadPredicate_MultipleConstraints(t *testing.T) { + tests := []struct { + name string + pod *v1.Pod + nodes []*v1.Node + existingPods []*v1.Pod + fits map[string]bool + }{ + { + // 1. to fulfil "zone" constraint, incoming pod can be placed on any zone (hence any node) + // 2. to fulfil "node" constraint, incoming pod can be placed on node-x + // intersaction of (1) and (2) returns node-x + name: "two constraints on zone and node, spreads = [3/3, 2/1/0/3]", + 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{ + 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{ + 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(), + }, + fits: map[string]bool{ + "node-a": false, + "node-b": false, + "node-x": true, + "node-y": false, + }, + }, + { + // 1. to fulfil "zone" constraint, incoming pod can be placed on zone1 (node-a or node-b) + // 2. to fulfil "node" constraint, incoming pod can be placed on node-x + // intersaction of (1) and (2) returns no node + name: "two constraints on zone and node, spreads = [3/4, 2/1/0/4]", + 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{ + 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{ + 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(), + }, + fits: map[string]bool{ + "node-a": false, + "node-b": false, + "node-x": false, + "node-y": false, + }, + }, + { + name: "constraints hold different labelSelectors, spreads = [1/1, 1/0/0/1]", + 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-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{ + 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(), + }, + fits: map[string]bool{ + "node-a": false, + "node-b": true, + "node-x": true, + "node-y": false, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + nodeInfoMap := schedulernodeinfo.CreateNodeNameToInfoMap(tt.existingPods, tt.nodes) + meta := GetPredicateMetadata(tt.pod, nodeInfoMap) + for _, node := range tt.nodes { + fits, _, _ := EvenPodsSpreadPredicate(tt.pod, meta, nodeInfoMap[node.Name]) + if fits != tt.fits[node.Name] { + t.Errorf("[%s]: expected %v got %v", node.Name, tt.fits, fits) + } + } + }) + } +} From b99fb9187be1b3408926cd735e128dcbd9add16d Mon Sep 17 00:00:00 2001 From: Wei Huang Date: Tue, 7 May 2019 15:01:30 -0700 Subject: [PATCH 3/5] EvenPodsSpread: UT on genericScheduler.Schedule() --- pkg/scheduler/core/generic_scheduler_test.go | 113 ++++++++++++++++++- 1 file changed, 111 insertions(+), 2 deletions(-) diff --git a/pkg/scheduler/core/generic_scheduler_test.go b/pkg/scheduler/core/generic_scheduler_test.go index 213a52acac9..cd5bf50493a 100644 --- a/pkg/scheduler/core/generic_scheduler_test.go +++ b/pkg/scheduler/core/generic_scheduler_test.go @@ -228,6 +228,7 @@ func TestGenericScheduler(t *testing.T) { pvcs []*v1.PersistentVolumeClaim pod *v1.Pod pods []*v1.Pod + buildPredMeta bool // build predicates metadata or not expectedHosts sets.String expectsErr bool wErr error @@ -435,6 +436,108 @@ func TestGenericScheduler(t *testing.T) { name: "test error with priority map", wErr: errors.NewAggregate([]error{errPrioritize, errPrioritize}), }, + { + name: "test even pods spread predicate - 2 nodes with maxskew=1", + predicates: map[string]algorithmpredicates.FitPredicate{ + "matches": algorithmpredicates.EvenPodsSpreadPredicate, + }, + // prioritizers: []priorities.PriorityConfig{{Map: EqualPriorityMap, Weight: 1}}, + nodes: []string{"machine1", "machine2"}, + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "p", UID: types.UID("p"), Labels: map[string]string{"foo": ""}}, + Spec: v1.PodSpec{ + TopologySpreadConstraints: []v1.TopologySpreadConstraint{ + { + MaxSkew: 1, + TopologyKey: "hostname", + WhenUnsatisfiable: v1.DoNotSchedule, + LabelSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "foo", + Operator: metav1.LabelSelectorOpExists, + }, + }, + }, + }, + }, + }, + }, + pods: []*v1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{Name: "pod1", UID: types.UID("pod1"), Labels: map[string]string{"foo": ""}}, + Spec: v1.PodSpec{ + NodeName: "machine1", + }, + Status: v1.PodStatus{ + Phase: v1.PodRunning, + }, + }, + }, + buildPredMeta: true, + expectedHosts: sets.NewString("machine2"), + wErr: nil, + }, + { + name: "test even pods spread predicate - 3 nodes with maxskew=2", + predicates: map[string]algorithmpredicates.FitPredicate{ + "matches": algorithmpredicates.EvenPodsSpreadPredicate, + }, + // prioritizers: []priorities.PriorityConfig{{Map: EqualPriorityMap, Weight: 1}}, + nodes: []string{"machine1", "machine2", "machine3"}, + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "p", UID: types.UID("p"), Labels: map[string]string{"foo": ""}}, + Spec: v1.PodSpec{ + TopologySpreadConstraints: []v1.TopologySpreadConstraint{ + { + MaxSkew: 2, + TopologyKey: "hostname", + WhenUnsatisfiable: v1.DoNotSchedule, + LabelSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "foo", + Operator: metav1.LabelSelectorOpExists, + }, + }, + }, + }, + }, + }, + }, + pods: []*v1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{Name: "pod1a", UID: types.UID("pod1a"), Labels: map[string]string{"foo": ""}}, + Spec: v1.PodSpec{ + NodeName: "machine1", + }, + Status: v1.PodStatus{ + Phase: v1.PodRunning, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "pod1b", UID: types.UID("pod1b"), Labels: map[string]string{"foo": ""}}, + Spec: v1.PodSpec{ + NodeName: "machine1", + }, + Status: v1.PodStatus{ + Phase: v1.PodRunning, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "pod2", UID: types.UID("pod2"), Labels: map[string]string{"foo": ""}}, + Spec: v1.PodSpec{ + NodeName: "machine2", + }, + Status: v1.PodStatus{ + Phase: v1.PodRunning, + }, + }, + }, + buildPredMeta: true, + expectedHosts: sets.NewString("machine2", "machine3"), + wErr: nil, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -443,18 +546,24 @@ func TestGenericScheduler(t *testing.T) { cache.AddPod(pod) } for _, name := range test.nodes { - cache.AddNode(&v1.Node{ObjectMeta: metav1.ObjectMeta{Name: name}}) + cache.AddNode(&v1.Node{ObjectMeta: metav1.ObjectMeta{Name: name, Labels: map[string]string{"hostname": name}}}) } pvcs := []*v1.PersistentVolumeClaim{} pvcs = append(pvcs, test.pvcs...) pvcLister := schedulertesting.FakePersistentVolumeClaimLister(pvcs) + var predMetaProducer algorithmpredicates.PredicateMetadataProducer + if test.buildPredMeta { + predMetaProducer = algorithmpredicates.NewPredicateMetadataFactory(schedulertesting.FakePodLister(test.pods)) + } else { + predMetaProducer = algorithmpredicates.EmptyPredicateMetadataProducer + } scheduler := NewGenericScheduler( cache, internalqueue.NewSchedulingQueue(nil, nil), test.predicates, - algorithmpredicates.EmptyPredicateMetadataProducer, + predMetaProducer, test.prioritizers, priorities.EmptyPriorityMetadataProducer, emptyFramework, From 39e459ae9a6c951a2d0ca4a94a935df5b723d5d9 Mon Sep 17 00:00:00 2001 From: Wei Huang Date: Fri, 19 Jul 2019 22:52:49 -0700 Subject: [PATCH 4/5] fixup: address comments --- pkg/scheduler/algorithm/predicates/BUILD | 1 - pkg/scheduler/algorithm/predicates/predicates.go | 8 +++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/pkg/scheduler/algorithm/predicates/BUILD b/pkg/scheduler/algorithm/predicates/BUILD index dd31d198949..5e6c799e5c4 100644 --- a/pkg/scheduler/algorithm/predicates/BUILD +++ b/pkg/scheduler/algorithm/predicates/BUILD @@ -64,7 +64,6 @@ go_test( "//pkg/scheduler/api:go_default_library", "//pkg/scheduler/nodeinfo:go_default_library", "//pkg/scheduler/testing:go_default_library", - "//pkg/scheduler/util:go_default_library", "//pkg/volume/util:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/api/storage/v1:go_default_library", diff --git a/pkg/scheduler/algorithm/predicates/predicates.go b/pkg/scheduler/algorithm/predicates/predicates.go index 4fefdbbe577..5ca0853e180 100644 --- a/pkg/scheduler/algorithm/predicates/predicates.go +++ b/pkg/scheduler/algorithm/predicates/predicates.go @@ -1767,11 +1767,9 @@ func EvenPodsSpreadPredicate(pod *v1.Pod, meta PredicateMetadata, nodeInfo *sche // 'existing matching num' + 'if self-match (1 or 0)' - 'global min matching num' <= 'maxSkew' matchNum := len(topologyPairsPodSpreadMap.topologyPairToPods[pair]) - // TODO(Huang-Wei): remove this after thorough testing - // fmt.Printf("node '%s' => spreadConstraint[%s]: matchNum(%d) + selfMatchNum(%d) - minMatchNum(%d) ?<= maxSkew(%d)\n", node.Name, tpKey, matchNum, selfMatchNum, minMatchNum, constraint.MaxSkew) - - // TODO(Huang-Wei): check if it can overflow? - if int32(matchNum+selfMatchNum)-minMatchNum > constraint.MaxSkew { + // cast to int to avoid potential overflow. + skew := matchNum + selfMatchNum - int(minMatchNum) + if skew > int(constraint.MaxSkew) { klog.V(5).Infof("node '%s' failed spreadConstraint[%s]: matchNum(%d) + selfMatchNum(%d) - minMatchNum(%d) > maxSkew(%d)", node.Name, tpKey, matchNum, selfMatchNum, minMatchNum, constraint.MaxSkew) return false, []PredicateFailureReason{ErrTopologySpreadConstraintsNotMatch}, nil } From 1822085088dcf43712049e3fa58c46f456505d03 Mon Sep 17 00:00:00 2001 From: Wei Huang Date: Mon, 22 Jul 2019 18:48:26 -0700 Subject: [PATCH 5/5] EvenPodsSpread: update 'selfMatch' logic --- .../algorithm/predicates/metadata.go | 3 - .../algorithm/predicates/predicates.go | 18 ++-- .../algorithm/predicates/predicates_test.go | 94 +++++++++++++++++-- 3 files changed, 98 insertions(+), 17 deletions(-) diff --git a/pkg/scheduler/algorithm/predicates/metadata.go b/pkg/scheduler/algorithm/predicates/metadata.go index 147f2ef5d8a..cd5d0367d1a 100644 --- a/pkg/scheduler/algorithm/predicates/metadata.go +++ b/pkg/scheduler/algorithm/predicates/metadata.go @@ -35,9 +35,6 @@ import ( schedutil "k8s.io/kubernetes/pkg/scheduler/util" ) -// MaxInt32 is the maximum value of int32 -const MaxInt32 = math.MaxInt32 - // PredicateMetadata interface represents anything that can access a predicate metadata. type PredicateMetadata interface { ShallowCopy() PredicateMetadata diff --git a/pkg/scheduler/algorithm/predicates/predicates.go b/pkg/scheduler/algorithm/predicates/predicates.go index 5ca0853e180..4a47662c2ca 100644 --- a/pkg/scheduler/algorithm/predicates/predicates.go +++ b/pkg/scheduler/algorithm/predicates/predicates.go @@ -1740,14 +1740,7 @@ func EvenPodsSpreadPredicate(pod *v1.Pod, meta PredicateMetadata, nodeInfo *sche return true, nil, nil } - selfMatch, err := podLabelsMatchesSpreadConstraints(pod.Labels, constraints) - if err != nil { - return false, nil, err - } - selfMatchNum := 0 - if selfMatch { - selfMatchNum = 1 - } + podLabelSet := labels.Set(pod.Labels) for _, constraint := range constraints { tpKey := constraint.TopologyKey tpVal, ok := node.Labels[constraint.TopologyKey] @@ -1756,6 +1749,15 @@ func EvenPodsSpreadPredicate(pod *v1.Pod, meta PredicateMetadata, nodeInfo *sche return false, []PredicateFailureReason{ErrTopologySpreadConstraintsNotMatch}, nil } + selfMatch, err := podMatchesSpreadConstraint(podLabelSet, constraint) + if err != nil { + return false, nil, err + } + selfMatchNum := 0 + if selfMatch { + selfMatchNum = 1 + } + pair := topologyPair{key: tpKey, value: tpVal} minMatchNum, ok := topologyPairsPodSpreadMap.topologyKeyToMinPodsMap[tpKey] if !ok { diff --git a/pkg/scheduler/algorithm/predicates/predicates_test.go b/pkg/scheduler/algorithm/predicates/predicates_test.go index b091c02dac6..a48484ead18 100644 --- a/pkg/scheduler/algorithm/predicates/predicates_test.go +++ b/pkg/scheduler/algorithm/predicates/predicates_test.go @@ -5300,7 +5300,7 @@ func TestEvenPodsSpreadPredicate_MultipleConstraints(t *testing.T) { { // 1. to fulfil "zone" constraint, incoming pod can be placed on any zone (hence any node) // 2. to fulfil "node" constraint, incoming pod can be placed on node-x - // intersaction of (1) and (2) returns node-x + // intersection of (1) and (2) returns node-x name: "two constraints on zone and node, spreads = [3/3, 2/1/0/3]", pod: st.MakePod().Name("p").Label("foo", ""). SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). @@ -5330,7 +5330,7 @@ func TestEvenPodsSpreadPredicate_MultipleConstraints(t *testing.T) { { // 1. to fulfil "zone" constraint, incoming pod can be placed on zone1 (node-a or node-b) // 2. to fulfil "node" constraint, incoming pod can be placed on node-x - // intersaction of (1) and (2) returns no node + // intersection of (1) and (2) returns no node name: "two constraints on zone and node, spreads = [3/4, 2/1/0/4]", pod: st.MakePod().Name("p").Label("foo", ""). SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). @@ -5359,7 +5359,63 @@ func TestEvenPodsSpreadPredicate_MultipleConstraints(t *testing.T) { }, }, { - name: "constraints hold different labelSelectors, spreads = [1/1, 1/0/0/1]", + // 1. to fulfil "zone" constraint, incoming pod can be placed on zone2 (node-x or node-y) + // 2. to fulfil "node" constraint, incoming pod can be placed on node-b or node-x + // intersection of (1) and (2) returns node-x + name: "constraints hold different labelSelectors, spreads = [1/0, 1/0/0/1]", + 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-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{ + st.MakePod().Name("p-a1").Node("node-a").Label("foo", "").Obj(), + st.MakePod().Name("p-y1").Node("node-y").Label("bar", "").Obj(), + }, + fits: map[string]bool{ + "node-a": false, + "node-b": false, + "node-x": true, + "node-y": false, + }, + }, + { + // 1. to fulfil "zone" constraint, incoming pod can be placed on zone2 (node-x or node-y) + // 2. to fulfil "node" constraint, incoming pod can be placed on node-a or node-b + // intersection of (1) and (2) returns no node + name: "constraints hold different labelSelectors, spreads = [1/0, 0/0/1/1]", + 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-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{ + st.MakePod().Name("p-a1").Node("node-a").Label("foo", "").Obj(), + st.MakePod().Name("p-x1").Node("node-x").Label("bar", "").Obj(), + st.MakePod().Name("p-y1").Node("node-y").Label("bar", "").Obj(), + }, + fits: map[string]bool{ + "node-a": false, + "node-b": false, + "node-x": false, + "node-y": false, + }, + }, + { + // 1. to fulfil "zone" constraint, incoming pod can be placed on zone1 (node-a or node-b) + // 2. to fulfil "node" constraint, incoming pod can be placed on node-b or node-x + // intersection of (1) and (2) returns node-b + name: "constraints hold different labelSelectors, spreads = [2/3, 1/0/0/1]", 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()). @@ -5373,7 +5429,6 @@ func TestEvenPodsSpreadPredicate_MultipleConstraints(t *testing.T) { existingPods: []*v1.Pod{ 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(), @@ -5381,7 +5436,34 @@ func TestEvenPodsSpreadPredicate_MultipleConstraints(t *testing.T) { fits: map[string]bool{ "node-a": false, "node-b": true, - "node-x": true, + "node-x": false, + "node-y": false, + }, + }, + { + // 1. pod doesn't match itself on "zone" constraint, so it can be put onto any zone + // 2. to fulfil "node" constraint, incoming pod can be placed on node-a or node-b + // intersection of (1) and (2) returns node-a and node-b + name: "constraints hold different labelSelectors but pod doesn't match itself on 'zone' constraint", + pod: st.MakePod().Name("p").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-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{ + st.MakePod().Name("p-a1").Node("node-a").Label("foo", "").Obj(), + st.MakePod().Name("p-x1").Node("node-x").Label("bar", "").Obj(), + st.MakePod().Name("p-y1").Node("node-y").Label("bar", "").Obj(), + }, + fits: map[string]bool{ + "node-a": true, + "node-b": true, + "node-x": false, "node-y": false, }, }, @@ -5393,7 +5475,7 @@ func TestEvenPodsSpreadPredicate_MultipleConstraints(t *testing.T) { for _, node := range tt.nodes { fits, _, _ := EvenPodsSpreadPredicate(tt.pod, meta, nodeInfoMap[node.Name]) if fits != tt.fits[node.Name] { - t.Errorf("[%s]: expected %v got %v", node.Name, tt.fits, fits) + t.Errorf("[%s]: expected %v got %v", node.Name, tt.fits[node.Name], fits) } } })