From 8a4dce5431f7b55e0c71440734cf7a84fd238cad Mon Sep 17 00:00:00 2001 From: Wei Huang Date: Thu, 26 Dec 2019 23:53:57 -0800 Subject: [PATCH] Move pod topology spread predicate logic to its filter plugin --- pkg/scheduler/algorithm/predicates/BUILD | 5 - .../algorithm/predicates/metadata.go | 246 -------- .../algorithm/predicates/predicates.go | 55 -- .../framework/plugins/podtopologyspread/BUILD | 12 +- .../plugins/podtopologyspread/filtering.go | 382 ++++++++++++ .../podtopologyspread/filtering_test.go} | 562 ++++++++++++++++-- .../plugins/podtopologyspread/plugin.go | 82 +++ .../podtopologyspread/pod_topology_spread.go | 179 ------ .../pod_topology_spread_test.go | 487 --------------- 9 files changed, 984 insertions(+), 1026 deletions(-) create mode 100644 pkg/scheduler/framework/plugins/podtopologyspread/filtering.go rename pkg/scheduler/{algorithm/predicates/metadata_test.go => framework/plugins/podtopologyspread/filtering_test.go} (62%) create mode 100644 pkg/scheduler/framework/plugins/podtopologyspread/plugin.go delete mode 100644 pkg/scheduler/framework/plugins/podtopologyspread/pod_topology_spread.go delete mode 100644 pkg/scheduler/framework/plugins/podtopologyspread/pod_topology_spread_test.go diff --git a/pkg/scheduler/algorithm/predicates/BUILD b/pkg/scheduler/algorithm/predicates/BUILD index d7753914058..7311e96a56d 100644 --- a/pkg/scheduler/algorithm/predicates/BUILD +++ b/pkg/scheduler/algorithm/predicates/BUILD @@ -24,7 +24,6 @@ go_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", - "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/rand:go_default_library", @@ -32,7 +31,6 @@ go_library( "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/listers/core/v1:go_default_library", "//staging/src/k8s.io/client-go/listers/storage/v1:go_default_library", - "//staging/src/k8s.io/client-go/util/workqueue:go_default_library", "//staging/src/k8s.io/csi-translation-lib/plugins:go_default_library", "//vendor/k8s.io/klog:go_default_library", ], @@ -42,7 +40,6 @@ go_test( name = "go_default_test", srcs = [ "max_attachable_volume_predicate_test.go", - "metadata_test.go", "predicates_test.go", "utils_test.go", ], @@ -53,8 +50,6 @@ go_test( "//pkg/features:go_default_library", "//pkg/scheduler/listers/fake:go_default_library", "//pkg/scheduler/nodeinfo:go_default_library", - "//pkg/scheduler/nodeinfo/snapshot:go_default_library", - "//pkg/scheduler/testing: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/metadata.go b/pkg/scheduler/algorithm/predicates/metadata.go index 89a07bb0e7f..513eb9e5364 100644 --- a/pkg/scheduler/algorithm/predicates/metadata.go +++ b/pkg/scheduler/algorithm/predicates/metadata.go @@ -16,252 +16,6 @@ limitations under the License. package predicates -import ( - "context" - "math" - "sync" - - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/client-go/util/workqueue" - "k8s.io/klog" - schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo" -) - // Metadata interface represents anything that can access a predicate metadata. // DEPRECATED. type Metadata interface{} - -type criticalPath struct { - // topologyValue denotes the topology value mapping to topology key. - topologyValue string - // matchNum denotes the number of matching pods. - matchNum int32 -} - -// CAVEAT: the reason that `[2]criticalPath` can work is based on the implementation of current -// preemption algorithm, in particular the following 2 facts: -// Fact 1: we only preempt pods on the same node, instead of pods on multiple nodes. -// Fact 2: each node is evaluated on a separate copy of the metadata during its preemption cycle. -// If we plan to turn to a more complex algorithm like "arbitrary pods on multiple nodes", this -// structure needs to be revisited. -type criticalPaths [2]criticalPath - -func newCriticalPaths() *criticalPaths { - return &criticalPaths{{matchNum: math.MaxInt32}, {matchNum: math.MaxInt32}} -} - -func (paths *criticalPaths) update(tpVal string, num int32) { - // first verify if `tpVal` exists or not - i := -1 - if tpVal == paths[0].topologyValue { - i = 0 - } else if tpVal == paths[1].topologyValue { - i = 1 - } - - if i >= 0 { - // `tpVal` exists - paths[i].matchNum = num - if paths[0].matchNum > paths[1].matchNum { - // swap paths[0] and paths[1] - paths[0], paths[1] = paths[1], paths[0] - } - } else { - // `tpVal` doesn't exist - if num < paths[0].matchNum { - // update paths[1] with paths[0] - paths[1] = paths[0] - // update paths[0] - paths[0].topologyValue, paths[0].matchNum = tpVal, num - } else if num < paths[1].matchNum { - // update paths[1] - paths[1].topologyValue, paths[1].matchNum = tpVal, num - } - } -} - -type topologyPair struct { - key string - value string -} - -// PodTopologySpreadMetadata combines tpKeyToCriticalPaths and tpPairToMatchNum -// to represent: -// (1) critical paths where the least pods are matched on each spread constraint. -// (2) number of pods matched on each spread constraint. -type PodTopologySpreadMetadata struct { - constraints []topologySpreadConstraint - // We record 2 critical paths instead of all critical paths here. - // criticalPaths[0].matchNum always holds the minimum matching number. - // criticalPaths[1].matchNum is always greater or equal to criticalPaths[0].matchNum, but - // it's not guaranteed to be the 2nd minimum match number. - tpKeyToCriticalPaths map[string]*criticalPaths - // tpPairToMatchNum is keyed with topologyPair, and valued with the number of matching pods. - tpPairToMatchNum map[topologyPair]int32 -} - -// topologySpreadConstraint is an internal version for a hard (DoNotSchedule -// unsatisfiable constraint action) v1.TopologySpreadConstraint and where the -// selector is parsed. -type topologySpreadConstraint struct { - maxSkew int32 - topologyKey string - selector labels.Selector -} - -// GetPodTopologySpreadMetadata computes pod topology spread metadata. -func GetPodTopologySpreadMetadata(pod *v1.Pod, allNodes []*schedulernodeinfo.NodeInfo) (*PodTopologySpreadMetadata, 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, err := filterHardTopologySpreadConstraints(pod.Spec.TopologySpreadConstraints) - if err != nil { - return nil, err - } - if len(constraints) == 0 { - return &PodTopologySpreadMetadata{}, nil - } - - var lock sync.Mutex - - // TODO(Huang-Wei): It might be possible to use "make(map[topologyPair]*int32)". - // In that case, need to consider how to init each tpPairToCount[pair] in an atomic fashion. - m := PodTopologySpreadMetadata{ - constraints: constraints, - tpKeyToCriticalPaths: make(map[string]*criticalPaths, len(constraints)), - tpPairToMatchNum: make(map[topologyPair]int32), - } - addTopologyPairMatchNum := func(pair topologyPair, num int32) { - lock.Lock() - m.tpPairToMatchNum[pair] += num - lock.Unlock() - } - - processNode := func(i int) { - nodeInfo := allNodes[i] - node := nodeInfo.Node() - if node == nil { - klog.Error("node not found") - return - } - // In accordance to 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'. - if !NodeLabelsMatchSpreadConstraints(node.Labels, constraints) { - return - } - for _, constraint := range constraints { - matchTotal := int32(0) - // nodeInfo.Pods() can be empty; or all pods don't fit - for _, existingPod := range nodeInfo.Pods() { - if existingPod.Namespace != pod.Namespace { - continue - } - if constraint.selector.Matches(labels.Set(existingPod.Labels)) { - matchTotal++ - } - } - pair := topologyPair{key: constraint.topologyKey, value: node.Labels[constraint.topologyKey]} - addTopologyPairMatchNum(pair, matchTotal) - } - } - workqueue.ParallelizeUntil(context.Background(), 16, len(allNodes), processNode) - - // calculate min match for each topology pair - for i := 0; i < len(constraints); i++ { - key := constraints[i].topologyKey - m.tpKeyToCriticalPaths[key] = newCriticalPaths() - } - for pair, num := range m.tpPairToMatchNum { - m.tpKeyToCriticalPaths[pair.key].update(pair.value, num) - } - - return &m, nil -} - -func filterHardTopologySpreadConstraints(constraints []v1.TopologySpreadConstraint) ([]topologySpreadConstraint, error) { - var result []topologySpreadConstraint - for _, c := range constraints { - if c.WhenUnsatisfiable == v1.DoNotSchedule { - selector, err := metav1.LabelSelectorAsSelector(c.LabelSelector) - if err != nil { - return nil, err - } - result = append(result, topologySpreadConstraint{ - maxSkew: c.MaxSkew, - topologyKey: c.TopologyKey, - selector: selector, - }) - } - } - return result, nil -} - -// NodeLabelsMatchSpreadConstraints checks if ALL topology keys in spread constraints are present in node labels. -func NodeLabelsMatchSpreadConstraints(nodeLabels map[string]string, constraints []topologySpreadConstraint) bool { - for _, c := range constraints { - if _, ok := nodeLabels[c.topologyKey]; !ok { - return false - } - } - return true -} - -// AddPod updates the metadata with addedPod. -func (m *PodTopologySpreadMetadata) AddPod(addedPod, preemptorPod *v1.Pod, node *v1.Node) { - m.updateWithPod(addedPod, preemptorPod, node, 1) -} - -// RemovePod updates the metadata with deletedPod. -func (m *PodTopologySpreadMetadata) RemovePod(deletedPod, preemptorPod *v1.Pod, node *v1.Node) { - m.updateWithPod(deletedPod, preemptorPod, node, -1) -} - -func (m *PodTopologySpreadMetadata) updateWithPod(updatedPod, preemptorPod *v1.Pod, node *v1.Node, delta int32) { - if m == nil || updatedPod.Namespace != preemptorPod.Namespace || node == nil { - return - } - if !NodeLabelsMatchSpreadConstraints(node.Labels, m.constraints) { - return - } - - podLabelSet := labels.Set(updatedPod.Labels) - for _, constraint := range m.constraints { - if !constraint.selector.Matches(podLabelSet) { - continue - } - - k, v := constraint.topologyKey, node.Labels[constraint.topologyKey] - pair := topologyPair{key: k, value: v} - m.tpPairToMatchNum[pair] = m.tpPairToMatchNum[pair] + delta - - m.tpKeyToCriticalPaths[k].update(v, m.tpPairToMatchNum[pair]) - } -} - -// Clone makes a deep copy of PodTopologySpreadMetadata. -func (m *PodTopologySpreadMetadata) Clone() *PodTopologySpreadMetadata { - // m could be nil when EvenPodsSpread feature is disabled - if m == nil { - return nil - } - cp := PodTopologySpreadMetadata{ - // constraints are shared because they don't change. - constraints: m.constraints, - tpKeyToCriticalPaths: make(map[string]*criticalPaths, len(m.tpKeyToCriticalPaths)), - tpPairToMatchNum: make(map[topologyPair]int32, len(m.tpPairToMatchNum)), - } - for tpKey, paths := range m.tpKeyToCriticalPaths { - cp.tpKeyToCriticalPaths[tpKey] = &criticalPaths{paths[0], paths[1]} - } - for tpPair, matchNum := range m.tpPairToMatchNum { - copyPair := topologyPair{key: tpPair.key, value: tpPair.value} - cp.tpPairToMatchNum[copyPair] = matchNum - } - return &cp -} diff --git a/pkg/scheduler/algorithm/predicates/predicates.go b/pkg/scheduler/algorithm/predicates/predicates.go index ee0cce5ea36..de483c9a223 100644 --- a/pkg/scheduler/algorithm/predicates/predicates.go +++ b/pkg/scheduler/algorithm/predicates/predicates.go @@ -17,7 +17,6 @@ limitations under the License. package predicates import ( - "errors" "fmt" "os" "regexp" @@ -837,57 +836,3 @@ func podToleratesNodeTaints(pod *v1.Pod, nodeInfo *schedulernodeinfo.NodeInfo, f func EvenPodsSpreadPredicate(pod *v1.Pod, meta Metadata, nodeInfo *schedulernodeinfo.NodeInfo) (bool, []PredicateFailureReason, error) { return false, nil, fmt.Errorf("this function should never be called") } - -// PodTopologySpreadPredicate checks if a pod can be scheduled on a node which satisfies -// its topologySpreadConstraints. -func PodTopologySpreadPredicate(pod *v1.Pod, meta *PodTopologySpreadMetadata, nodeInfo *schedulernodeinfo.NodeInfo) (bool, []PredicateFailureReason, error) { - node := nodeInfo.Node() - if node == nil { - return false, nil, fmt.Errorf("node not found") - } - - // nil meta is illegal. - if meta == nil { - // TODO(autoscaler): get it implemented. - return false, nil, errors.New("metadata not pre-computed for PodTopologySpreadPredicate") - } - - // However, "empty" meta is legit which tolerates every toSchedule Pod. - if len(meta.tpPairToMatchNum) == 0 || len(meta.constraints) == 0 { - return true, nil, nil - } - - podLabelSet := labels.Set(pod.Labels) - for _, c := range meta.constraints { - tpKey := c.topologyKey - tpVal, ok := node.Labels[c.topologyKey] - if !ok { - klog.V(5).Infof("node '%s' doesn't have required label '%s'", node.Name, tpKey) - return false, []PredicateFailureReason{ErrTopologySpreadConstraintsNotMatch}, nil - } - - selfMatchNum := int32(0) - if c.selector.Matches(podLabelSet) { - selfMatchNum = 1 - } - - pair := topologyPair{key: tpKey, value: tpVal} - paths, ok := meta.tpKeyToCriticalPaths[tpKey] - if !ok { - // error which should not happen - klog.Errorf("internal error: get paths from key %q of %#v", tpKey, meta.tpKeyToCriticalPaths) - continue - } - // judging criteria: - // 'existing matching num' + 'if self-match (1 or 0)' - 'global min matching num' <= 'maxSkew' - minMatchNum := paths[0].matchNum - matchNum := meta.tpPairToMatchNum[pair] - skew := matchNum + selfMatchNum - minMatchNum - if skew > c.maxSkew { - klog.V(5).Infof("node '%s' failed spreadConstraint[%s]: matchNum(%d) + selfMatchNum(%d) - minMatchNum(%d) > maxSkew(%d)", node.Name, tpKey, matchNum, selfMatchNum, minMatchNum, c.maxSkew) - return false, []PredicateFailureReason{ErrTopologySpreadConstraintsNotMatch}, nil - } - } - - return true, nil, nil -} diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/BUILD b/pkg/scheduler/framework/plugins/podtopologyspread/BUILD index 362b21b493a..84adc861eb8 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/BUILD +++ b/pkg/scheduler/framework/plugins/podtopologyspread/BUILD @@ -2,7 +2,10 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", - srcs = ["pod_topology_spread.go"], + srcs = [ + "filtering.go", + "plugin.go", + ], importpath = "k8s.io/kubernetes/pkg/scheduler/framework/plugins/podtopologyspread", visibility = ["//visibility:public"], deps = [ @@ -13,20 +16,25 @@ go_library( "//pkg/scheduler/listers:go_default_library", "//pkg/scheduler/nodeinfo:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//staging/src/k8s.io/client-go/util/workqueue:go_default_library", "//vendor/k8s.io/klog:go_default_library", ], ) go_test( name = "go_default_test", - srcs = ["pod_topology_spread_test.go"], + srcs = ["filtering_test.go"], embed = [":go_default_library"], deps = [ "//pkg/scheduler/framework/v1alpha1:go_default_library", "//pkg/scheduler/nodeinfo/snapshot:go_default_library", "//pkg/scheduler/testing:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", ], ) diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/filtering.go b/pkg/scheduler/framework/plugins/podtopologyspread/filtering.go new file mode 100644 index 00000000000..bb84f4955bd --- /dev/null +++ b/pkg/scheduler/framework/plugins/podtopologyspread/filtering.go @@ -0,0 +1,382 @@ +/* +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 podtopologyspread + +import ( + "context" + "fmt" + "math" + "sync" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/client-go/util/workqueue" + "k8s.io/klog" + "k8s.io/kubernetes/pkg/scheduler/algorithm/predicates" + "k8s.io/kubernetes/pkg/scheduler/framework/plugins/migration" + framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" + "k8s.io/kubernetes/pkg/scheduler/nodeinfo" + schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo" +) + +const preFilterStateKey = "PreFilter" + Name + +// preFilterState computed at PreFilter and used at Filter. +// It combines tpKeyToCriticalPaths and tpPairToMatchNum to represent: +// (1) critical paths where the least pods are matched on each spread constraint. +// (2) number of pods matched on each spread constraint. +// A nil preFilterState denotes it's not set at all (in PreFilter phase); +// An empty preFilterState object denotes it's a legit state and is set in PreFilter phase. +type preFilterState struct { + constraints []topologySpreadConstraint + // We record 2 critical paths instead of all critical paths here. + // criticalPaths[0].matchNum always holds the minimum matching number. + // criticalPaths[1].matchNum is always greater or equal to criticalPaths[0].matchNum, but + // it's not guaranteed to be the 2nd minimum match number. + tpKeyToCriticalPaths map[string]*criticalPaths + // tpPairToMatchNum is keyed with topologyPair, and valued with the number of matching pods. + tpPairToMatchNum map[topologyPair]int32 +} + +// Clone makes a copy of the given state. +func (s *preFilterState) Clone() framework.StateData { + // s could be nil when EvenPodsSpread feature is disabled + if s == nil { + return nil + } + copy := preFilterState{ + // constraints are shared because they don't change. + constraints: s.constraints, + tpKeyToCriticalPaths: make(map[string]*criticalPaths, len(s.tpKeyToCriticalPaths)), + tpPairToMatchNum: make(map[topologyPair]int32, len(s.tpPairToMatchNum)), + } + for tpKey, paths := range s.tpKeyToCriticalPaths { + copy.tpKeyToCriticalPaths[tpKey] = &criticalPaths{paths[0], paths[1]} + } + for tpPair, matchNum := range s.tpPairToMatchNum { + copyPair := topologyPair{key: tpPair.key, value: tpPair.value} + copy.tpPairToMatchNum[copyPair] = matchNum + } + return © +} + +type criticalPath struct { + // topologyValue denotes the topology value mapping to topology key. + topologyValue string + // matchNum denotes the number of matching pods. + matchNum int32 +} + +// CAVEAT: the reason that `[2]criticalPath` can work is based on the implementation of current +// preemption algorithm, in particular the following 2 facts: +// Fact 1: we only preempt pods on the same node, instead of pods on multiple nodes. +// Fact 2: each node is evaluated on a separate copy of the preFilterState during its preemption cycle. +// If we plan to turn to a more complex algorithm like "arbitrary pods on multiple nodes", this +// structure needs to be revisited. +type criticalPaths [2]criticalPath + +func newCriticalPaths() *criticalPaths { + return &criticalPaths{{matchNum: math.MaxInt32}, {matchNum: math.MaxInt32}} +} + +func (paths *criticalPaths) update(tpVal string, num int32) { + // first verify if `tpVal` exists or not + i := -1 + if tpVal == paths[0].topologyValue { + i = 0 + } else if tpVal == paths[1].topologyValue { + i = 1 + } + + if i >= 0 { + // `tpVal` exists + paths[i].matchNum = num + if paths[0].matchNum > paths[1].matchNum { + // swap paths[0] and paths[1] + paths[0], paths[1] = paths[1], paths[0] + } + } else { + // `tpVal` doesn't exist + if num < paths[0].matchNum { + // update paths[1] with paths[0] + paths[1] = paths[0] + // update paths[0] + paths[0].topologyValue, paths[0].matchNum = tpVal, num + } else if num < paths[1].matchNum { + // update paths[1] + paths[1].topologyValue, paths[1].matchNum = tpVal, num + } + } +} + +type topologyPair struct { + key string + value string +} + +// topologySpreadConstraint is an internal version for a hard (DoNotSchedule +// unsatisfiable constraint action) v1.TopologySpreadConstraint and where the +// selector is parsed. +type topologySpreadConstraint struct { + maxSkew int32 + topologyKey string + selector labels.Selector +} + +func (s *preFilterState) updateWithPod(updatedPod, preemptorPod *v1.Pod, node *v1.Node, delta int32) { + if s == nil || updatedPod.Namespace != preemptorPod.Namespace || node == nil { + return + } + if !nodeLabelsMatchSpreadConstraints(node.Labels, s.constraints) { + return + } + + podLabelSet := labels.Set(updatedPod.Labels) + for _, constraint := range s.constraints { + if !constraint.selector.Matches(podLabelSet) { + continue + } + + k, v := constraint.topologyKey, node.Labels[constraint.topologyKey] + pair := topologyPair{key: k, value: v} + s.tpPairToMatchNum[pair] = s.tpPairToMatchNum[pair] + delta + + s.tpKeyToCriticalPaths[k].update(v, s.tpPairToMatchNum[pair]) + } +} + +// nodeLabelsMatchSpreadConstraints checks if ALL topology keys in spread constraints are present in node labels. +func nodeLabelsMatchSpreadConstraints(nodeLabels map[string]string, constraints []topologySpreadConstraint) bool { + for _, c := range constraints { + if _, ok := nodeLabels[c.topologyKey]; !ok { + return false + } + } + return true +} + +// PreFilter invoked at the prefilter extension point. +func (pl *PodTopologySpread) PreFilter(ctx context.Context, cycleState *framework.CycleState, pod *v1.Pod) *framework.Status { + var s *preFilterState + var allNodes []*nodeinfo.NodeInfo + var err error + + if allNodes, err = pl.sharedLister.NodeInfos().List(); err != nil { + return framework.NewStatus(framework.Error, fmt.Sprintf("failed to list NodeInfos: %v", err)) + } + + if s, err = calPreFilterState(pod, allNodes); err != nil { + return framework.NewStatus(framework.Error, fmt.Sprintf("Error calculating preFilterState of PodTopologySpread Plugin: %v", err)) + } + + cycleState.Write(preFilterStateKey, s) + + return nil +} + +// PreFilterExtensions returns prefilter extensions, pod add and remove. +func (pl *PodTopologySpread) PreFilterExtensions() framework.PreFilterExtensions { + return pl +} + +// AddPod from pre-computed data in cycleState. +func (pl *PodTopologySpread) AddPod(ctx context.Context, cycleState *framework.CycleState, podToSchedule *v1.Pod, podToAdd *v1.Pod, nodeInfo *nodeinfo.NodeInfo) *framework.Status { + s, err := getPreFilterState(cycleState) + if err != nil { + return framework.NewStatus(framework.Error, err.Error()) + } + + s.updateWithPod(podToAdd, podToSchedule, nodeInfo.Node(), 1) + return nil +} + +// RemovePod from pre-computed data in cycleState. +func (pl *PodTopologySpread) RemovePod(ctx context.Context, cycleState *framework.CycleState, podToSchedule *v1.Pod, podToRemove *v1.Pod, nodeInfo *nodeinfo.NodeInfo) *framework.Status { + s, err := getPreFilterState(cycleState) + if err != nil { + return framework.NewStatus(framework.Error, err.Error()) + } + + s.updateWithPod(podToRemove, podToSchedule, nodeInfo.Node(), -1) + return nil +} + +// getPreFilterState fetches a pre-computed preFilterState +func getPreFilterState(cycleState *framework.CycleState) (*preFilterState, error) { + c, err := cycleState.Read(preFilterStateKey) + if err != nil { + // The metadata wasn't pre-computed in prefilter. We ignore the error for now since + // we are able to handle that by computing it again (e.g. in Filter()). + klog.V(5).Infof("Error reading %q from cycleState: %v", preFilterStateKey, err) + return nil, nil + } + + s, ok := c.(*preFilterState) + if !ok { + return nil, fmt.Errorf("%+v convert to podtopologyspread.state error", c) + } + return s, nil +} + +// calPreFilterState computes preFilterState describing how pods are spread on topologies. +func calPreFilterState(pod *v1.Pod, allNodes []*schedulernodeinfo.NodeInfo) (*preFilterState, 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, err := filterHardTopologySpreadConstraints(pod.Spec.TopologySpreadConstraints) + if err != nil { + return nil, err + } + if len(constraints) == 0 { + return &preFilterState{}, nil + } + + var lock sync.Mutex + + // TODO(Huang-Wei): It might be possible to use "make(map[topologyPair]*int32)". + // In that case, need to consider how to init each tpPairToCount[pair] in an atomic fashion. + s := preFilterState{ + constraints: constraints, + tpKeyToCriticalPaths: make(map[string]*criticalPaths, len(constraints)), + tpPairToMatchNum: make(map[topologyPair]int32), + } + addTopologyPairMatchNum := func(pair topologyPair, num int32) { + lock.Lock() + s.tpPairToMatchNum[pair] += num + lock.Unlock() + } + + processNode := func(i int) { + nodeInfo := allNodes[i] + node := nodeInfo.Node() + if node == nil { + klog.Error("node not found") + return + } + // In accordance to design, if NodeAffinity or NodeSelector is defined, + // spreading is applied to nodes that pass those filters. + if !predicates.PodMatchesNodeSelectorAndAffinityTerms(pod, node) { + return + } + + // Ensure current node's labels contains all topologyKeys in 'constraints'. + if !nodeLabelsMatchSpreadConstraints(node.Labels, constraints) { + return + } + for _, constraint := range constraints { + matchTotal := int32(0) + // nodeInfo.Pods() can be empty; or all pods don't fit + for _, existingPod := range nodeInfo.Pods() { + if existingPod.Namespace != pod.Namespace { + continue + } + if constraint.selector.Matches(labels.Set(existingPod.Labels)) { + matchTotal++ + } + } + pair := topologyPair{key: constraint.topologyKey, value: node.Labels[constraint.topologyKey]} + addTopologyPairMatchNum(pair, matchTotal) + } + } + workqueue.ParallelizeUntil(context.Background(), 16, len(allNodes), processNode) + + // calculate min match for each topology pair + for i := 0; i < len(constraints); i++ { + key := constraints[i].topologyKey + s.tpKeyToCriticalPaths[key] = newCriticalPaths() + } + for pair, num := range s.tpPairToMatchNum { + s.tpKeyToCriticalPaths[pair.key].update(pair.value, num) + } + + return &s, nil +} + +func filterHardTopologySpreadConstraints(constraints []v1.TopologySpreadConstraint) ([]topologySpreadConstraint, error) { + var result []topologySpreadConstraint + for _, c := range constraints { + if c.WhenUnsatisfiable == v1.DoNotSchedule { + selector, err := metav1.LabelSelectorAsSelector(c.LabelSelector) + if err != nil { + return nil, err + } + result = append(result, topologySpreadConstraint{ + maxSkew: c.MaxSkew, + topologyKey: c.TopologyKey, + selector: selector, + }) + } + } + return result, nil +} + +// Filter invoked at the filter extension point. +func (pl *PodTopologySpread) Filter(ctx context.Context, cycleState *framework.CycleState, pod *v1.Pod, nodeInfo *nodeinfo.NodeInfo) *framework.Status { + node := nodeInfo.Node() + if node == nil { + return framework.NewStatus(framework.Error, "node not found") + } + + s, err := getPreFilterState(cycleState) + if err != nil { + return framework.NewStatus(framework.Error, err.Error()) + } + // nil preFilterState is illegal. + if s == nil { + // TODO(autoscaler): get it implemented. + return framework.NewStatus(framework.Error, "preFilterState not pre-computed for PodTopologySpread Plugin") + } + + // However, "empty" meta is legit which tolerates every toSchedule Pod. + if len(s.tpPairToMatchNum) == 0 || len(s.constraints) == 0 { + return nil + } + + podLabelSet := labels.Set(pod.Labels) + for _, c := range s.constraints { + tpKey := c.topologyKey + tpVal, ok := node.Labels[c.topologyKey] + if !ok { + klog.V(5).Infof("node '%s' doesn't have required label '%s'", node.Name, tpKey) + return migration.PredicateResultToFrameworkStatus([]predicates.PredicateFailureReason{predicates.ErrTopologySpreadConstraintsNotMatch}, nil) + } + + selfMatchNum := int32(0) + if c.selector.Matches(podLabelSet) { + selfMatchNum = 1 + } + + pair := topologyPair{key: tpKey, value: tpVal} + paths, ok := s.tpKeyToCriticalPaths[tpKey] + if !ok { + // error which should not happen + klog.Errorf("internal error: get paths from key %q of %#v", tpKey, s.tpKeyToCriticalPaths) + continue + } + // judging criteria: + // 'existing matching num' + 'if self-match (1 or 0)' - 'global min matching num' <= 'maxSkew' + minMatchNum := paths[0].matchNum + matchNum := s.tpPairToMatchNum[pair] + skew := matchNum + selfMatchNum - minMatchNum + if skew > c.maxSkew { + klog.V(5).Infof("node '%s' failed spreadConstraint[%s]: matchNum(%d) + selfMatchNum(%d) - minMatchNum(%d) > maxSkew(%d)", node.Name, tpKey, matchNum, selfMatchNum, minMatchNum, c.maxSkew) + return migration.PredicateResultToFrameworkStatus([]predicates.PredicateFailureReason{predicates.ErrTopologySpreadConstraintsNotMatch}, nil) + } + } + + return nil +} diff --git a/pkg/scheduler/algorithm/predicates/metadata_test.go b/pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go similarity index 62% rename from pkg/scheduler/algorithm/predicates/metadata_test.go rename to pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go index a3a4887ad1f..9ca27409f7e 100644 --- a/pkg/scheduler/algorithm/predicates/metadata_test.go +++ b/pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go @@ -1,5 +1,5 @@ /* -Copyright 2017 The Kubernetes Authors. +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. @@ -14,20 +14,27 @@ See the License for the specific language governing permissions and limitations under the License. */ -package predicates +package podtopologyspread import ( + "context" "reflect" "testing" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" nodeinfosnapshot "k8s.io/kubernetes/pkg/scheduler/nodeinfo/snapshot" st "k8s.io/kubernetes/pkg/scheduler/testing" ) -func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { +var ( + hardSpread = v1.DoNotSchedule + softSpread = v1.ScheduleAnyway +) + +func TestCalPreFilterState(t *testing.T) { fooSelector := st.MakeLabelSelector().Exists("foo").Obj() barSelector := st.MakeLabelSelector().Exists("bar").Obj() tests := []struct { @@ -35,7 +42,7 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { pod *v1.Pod nodes []*v1.Node existingPods []*v1.Pod - want *PodTopologySpreadMetadata + want *preFilterState }{ { name: "clean cluster with one spreadConstraint", @@ -48,7 +55,7 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { 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(), }, - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{ { maxSkew: 5, @@ -83,7 +90,7 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { st.MakePod().Name("p-y1").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(), }, - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{ { maxSkew: 1, @@ -120,7 +127,7 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { st.MakePod().Name("p-y1").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(), }, - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{ { maxSkew: 1, @@ -156,7 +163,7 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { st.MakePod().Name("p-y1").Namespace("ns2").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(), }, - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{ { maxSkew: 1, @@ -194,7 +201,7 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { st.MakePod().Name("p-y3").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y4").Node("node-y").Label("foo", "").Obj(), }, - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{ { maxSkew: 1, @@ -243,7 +250,7 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { st.MakePod().Name("p-y3").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y4").Node("node-y").Label("foo", "").Obj(), }, - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{ { maxSkew: 1, @@ -284,7 +291,7 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { st.MakePod().Name("p-a").Node("node-a").Label("foo", "").Obj(), st.MakePod().Name("p-b").Node("node-b").Label("bar", "").Obj(), }, - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{ { maxSkew: 1, @@ -330,7 +337,7 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { st.MakePod().Name("p-y3").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y4").Node("node-y").Label("foo", "").Label("bar", "").Obj(), }, - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{ { maxSkew: 1, @@ -378,7 +385,7 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { st.MakePod().Name("p-y3").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y4").Node("node-y").Label("foo", "").Obj(), }, - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{ { maxSkew: 1, @@ -409,16 +416,16 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { t.Run(tt.name, func(t *testing.T) { s := nodeinfosnapshot.NewSnapshot(nodeinfosnapshot.CreateNodeInfoMap(tt.existingPods, tt.nodes)) l, _ := s.NodeInfos().List() - got, _ := GetPodTopologySpreadMetadata(tt.pod, l) + got, _ := calPreFilterState(tt.pod, l) got.sortCriticalPaths() if !reflect.DeepEqual(got, tt.want) { - t.Errorf("getEvenPodsSpreadMetadata() = %#v, want %#v", *got, *tt.want) + t.Errorf("calPreFilterState() = %#v, want %#v", *got, *tt.want) } }) } } -func TestPodSpreadCache_addPod(t *testing.T) { +func TestPreFilterStateAddPod(t *testing.T) { nodeConstraint := topologySpreadConstraint{ maxSkew: 1, topologyKey: "node", @@ -433,7 +440,7 @@ func TestPodSpreadCache_addPod(t *testing.T) { existingPods []*v1.Pod nodeIdx int // denotes which node 'addedPod' belongs to nodes []*v1.Node - want *PodTopologySpreadMetadata + want *preFilterState }{ { name: "node a and b both impact current min match", @@ -447,7 +454,7 @@ func TestPodSpreadCache_addPod(t *testing.T) { 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(), }, - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{nodeConstraint}, tpKeyToCriticalPaths: map[string]*criticalPaths{ "node": {{"node-b", 0}, {"node-a", 1}}, @@ -472,7 +479,7 @@ func TestPodSpreadCache_addPod(t *testing.T) { 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(), }, - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{nodeConstraint}, tpKeyToCriticalPaths: map[string]*criticalPaths{ "node": {{"node-a", 1}, {"node-b", 1}}, @@ -497,7 +504,7 @@ func TestPodSpreadCache_addPod(t *testing.T) { 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(), }, - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{nodeConstraint}, tpKeyToCriticalPaths: map[string]*criticalPaths{ "node": {{"node-a", 0}, {"node-b", 1}}, @@ -522,7 +529,7 @@ func TestPodSpreadCache_addPod(t *testing.T) { 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(), }, - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{nodeConstraint}, tpKeyToCriticalPaths: map[string]*criticalPaths{ "node": {{"node-a", 0}, {"node-b", 2}}, @@ -546,7 +553,7 @@ func TestPodSpreadCache_addPod(t *testing.T) { st.MakeNode().Name("node-a").Label("zone", "zone1").Label("node", "node-a").Obj(), st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(), }, - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint}, tpKeyToCriticalPaths: map[string]*criticalPaths{ "zone": {{"zone2", 0}, {"zone1", 1}}, @@ -575,7 +582,7 @@ func TestPodSpreadCache_addPod(t *testing.T) { st.MakeNode().Name("node-a").Label("zone", "zone1").Label("node", "node-a").Obj(), st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(), }, - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint}, tpKeyToCriticalPaths: map[string]*criticalPaths{ "zone": {{"zone1", 1}, {"zone2", 1}}, @@ -607,7 +614,7 @@ func TestPodSpreadCache_addPod(t *testing.T) { 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(), }, - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint}, tpKeyToCriticalPaths: map[string]*criticalPaths{ "zone": {{"zone2", 1}, {"zone1", 3}}, @@ -640,7 +647,7 @@ func TestPodSpreadCache_addPod(t *testing.T) { 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(), }, - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{ zoneConstraint, { @@ -680,7 +687,7 @@ func TestPodSpreadCache_addPod(t *testing.T) { 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(), }, - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{ zoneConstraint, { @@ -707,17 +714,17 @@ func TestPodSpreadCache_addPod(t *testing.T) { t.Run(tt.name, func(t *testing.T) { s := nodeinfosnapshot.NewSnapshot(nodeinfosnapshot.CreateNodeInfoMap(tt.existingPods, tt.nodes)) l, _ := s.NodeInfos().List() - podTopologySpreadMeta, _ := GetPodTopologySpreadMetadata(tt.preemptor, l) - podTopologySpreadMeta.AddPod(tt.addedPod, tt.preemptor, tt.nodes[tt.nodeIdx]) - podTopologySpreadMeta.sortCriticalPaths() - if !reflect.DeepEqual(podTopologySpreadMeta, tt.want) { - t.Errorf("podTopologySpreadMeta#addPod() = %v, want %v", podTopologySpreadMeta, tt.want) + state, _ := calPreFilterState(tt.preemptor, l) + state.updateWithPod(tt.addedPod, tt.preemptor, tt.nodes[tt.nodeIdx], 1) + state.sortCriticalPaths() + if !reflect.DeepEqual(state, tt.want) { + t.Errorf("preFilterState#AddPod() = %v, want %v", state, tt.want) } }) } } -func TestPodSpreadCache_removePod(t *testing.T) { +func TestPreFilterStateRemovePod(t *testing.T) { nodeConstraint := topologySpreadConstraint{ maxSkew: 1, topologyKey: "node", @@ -733,7 +740,7 @@ func TestPodSpreadCache_removePod(t *testing.T) { deletedPodIdx int // need to reuse *Pod of existingPods[i] deletedPod *v1.Pod // this field is used only when deletedPodIdx is -1 nodeIdx int // denotes which node "deletedPod" belongs to - want *PodTopologySpreadMetadata + want *preFilterState }{ { // A high priority pod may not be scheduled due to node taints or resource shortage. @@ -754,7 +761,7 @@ func TestPodSpreadCache_removePod(t *testing.T) { }, deletedPodIdx: 0, // remove pod "p-a1" nodeIdx: 0, // node-a - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{zoneConstraint}, tpKeyToCriticalPaths: map[string]*criticalPaths{ "zone": {{"zone1", 1}, {"zone2", 1}}, @@ -784,7 +791,7 @@ func TestPodSpreadCache_removePod(t *testing.T) { }, deletedPodIdx: 0, // remove pod "p-a1" nodeIdx: 0, // node-a - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{zoneConstraint}, tpKeyToCriticalPaths: map[string]*criticalPaths{ "zone": {{"zone1", 1}, {"zone2", 2}}, @@ -815,7 +822,7 @@ func TestPodSpreadCache_removePod(t *testing.T) { }, deletedPodIdx: 0, // remove pod "p-a0" nodeIdx: 0, // node-a - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{zoneConstraint}, tpKeyToCriticalPaths: map[string]*criticalPaths{ "zone": {{"zone1", 2}, {"zone2", 2}}, @@ -846,7 +853,7 @@ func TestPodSpreadCache_removePod(t *testing.T) { deletedPodIdx: -1, deletedPod: st.MakePod().Name("p-a0").Node("node-a").Label("bar", "").Obj(), nodeIdx: 0, // node-a - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{zoneConstraint}, tpKeyToCriticalPaths: map[string]*criticalPaths{ "zone": {{"zone1", 2}, {"zone2", 2}}, @@ -877,7 +884,7 @@ func TestPodSpreadCache_removePod(t *testing.T) { }, deletedPodIdx: 3, // remove pod "p-x1" nodeIdx: 2, // node-x - want: &PodTopologySpreadMetadata{ + want: &preFilterState{ constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint}, tpKeyToCriticalPaths: map[string]*criticalPaths{ "zone": {{"zone2", 1}, {"zone1", 3}}, @@ -897,7 +904,7 @@ func TestPodSpreadCache_removePod(t *testing.T) { t.Run(tt.name, func(t *testing.T) { s := nodeinfosnapshot.NewSnapshot(nodeinfosnapshot.CreateNodeInfoMap(tt.existingPods, tt.nodes)) l, _ := s.NodeInfos().List() - podTopologySpreadMeta, _ := GetPodTopologySpreadMetadata(tt.preemptor, l) + state, _ := calPreFilterState(tt.preemptor, l) var deletedPod *v1.Pod if tt.deletedPodIdx < len(tt.existingPods) && tt.deletedPodIdx >= 0 { @@ -905,16 +912,16 @@ func TestPodSpreadCache_removePod(t *testing.T) { } else { deletedPod = tt.deletedPod } - podTopologySpreadMeta.RemovePod(deletedPod, tt.preemptor, tt.nodes[tt.nodeIdx]) - podTopologySpreadMeta.sortCriticalPaths() - if !reflect.DeepEqual(podTopologySpreadMeta, tt.want) { - t.Errorf("podTopologySpreadMeta#removePod() = %v, want %v", podTopologySpreadMeta, tt.want) + state.updateWithPod(deletedPod, tt.preemptor, tt.nodes[tt.nodeIdx], -1) + state.sortCriticalPaths() + if !reflect.DeepEqual(state, tt.want) { + t.Errorf("preFilterState#RemovePod() = %v, want %v", state, tt.want) } }) } } -func BenchmarkTestGetTPMapMatchingSpreadConstraints(b *testing.B) { +func BenchmarkTestCalPreFilterState(b *testing.B) { tests := []struct { name string pod *v1.Pod @@ -958,20 +965,15 @@ func BenchmarkTestGetTPMapMatchingSpreadConstraints(b *testing.B) { l, _ := s.NodeInfos().List() b.ResetTimer() for i := 0; i < b.N; i++ { - GetPodTopologySpreadMetadata(tt.pod, l) + calPreFilterState(tt.pod, l) } }) } } -var ( - hardSpread = v1.DoNotSchedule - softSpread = v1.ScheduleAnyway -) - // sortCriticalPaths is only served for testing purpose. -func (m *PodTopologySpreadMetadata) sortCriticalPaths() { - for _, paths := range m.tpKeyToCriticalPaths { +func (s *preFilterState) sortCriticalPaths() { + for _, paths := range s.tpKeyToCriticalPaths { // If two paths both hold minimum matching number, and topologyValue is unordered. if paths[0].matchNum == paths[1].matchNum && paths[0].topologyValue > paths[1].topologyValue { // Swap topologyValue to make them sorted alphabetically. @@ -988,3 +990,459 @@ func mustConvertLabelSelectorAsSelector(t *testing.T, ls *metav1.LabelSelector) } return s } + +func TestSingleConstraint(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 doesn'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) { + snapshot := nodeinfosnapshot.NewSnapshot(nodeinfosnapshot.CreateNodeInfoMap(tt.existingPods, tt.nodes)) + p := &PodTopologySpread{sharedLister: snapshot} + state := framework.NewCycleState() + preFilterStatus := p.PreFilter(context.Background(), state, tt.pod) + if !preFilterStatus.IsSuccess() { + t.Errorf("preFilter failed with status: %v", preFilterStatus) + } + + for _, node := range tt.nodes { + nodeInfo, _ := snapshot.NodeInfos().Get(node.Name) + status := p.Filter(context.Background(), state, tt.pod, nodeInfo) + if status.IsSuccess() != tt.fits[node.Name] { + t.Errorf("[%s]: expected %v got %v", node.Name, tt.fits[node.Name], status.IsSuccess()) + } + } + }) + } +} + +func TestMultipleConstraints(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 + // 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()). + 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 + // 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()). + 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, + }, + }, + { + // 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()). + 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-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": 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, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + snapshot := nodeinfosnapshot.NewSnapshot(nodeinfosnapshot.CreateNodeInfoMap(tt.existingPods, tt.nodes)) + p := &PodTopologySpread{sharedLister: snapshot} + state := framework.NewCycleState() + preFilterStatus := p.PreFilter(context.Background(), state, tt.pod) + if !preFilterStatus.IsSuccess() { + t.Errorf("preFilter failed with status: %v", preFilterStatus) + } + + for _, node := range tt.nodes { + nodeInfo, _ := snapshot.NodeInfos().Get(node.Name) + status := p.Filter(context.Background(), state, tt.pod, nodeInfo) + if status.IsSuccess() != tt.fits[node.Name] { + t.Errorf("[%s]: expected %v got %v", node.Name, tt.fits[node.Name], status.IsSuccess()) + } + } + }) + } +} diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go b/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go new file mode 100644 index 00000000000..05f566d6131 --- /dev/null +++ b/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go @@ -0,0 +1,82 @@ +/* +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 podtopologyspread + +import ( + "context" + "fmt" + + "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/kubernetes/pkg/scheduler/algorithm/priorities" + "k8s.io/kubernetes/pkg/scheduler/framework/plugins/migration" + framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" + schedulerlisters "k8s.io/kubernetes/pkg/scheduler/listers" +) + +// PodTopologySpread is a plugin that ensures pod's topologySpreadConstraints is satisfied. +type PodTopologySpread struct { + sharedLister schedulerlisters.SharedLister +} + +var _ framework.PreFilterPlugin = &PodTopologySpread{} +var _ framework.FilterPlugin = &PodTopologySpread{} +var _ framework.ScorePlugin = &PodTopologySpread{} + +const ( + // Name is the name of the plugin used in the plugin registry and configurations. + Name = "PodTopologySpread" +) + +// Name returns name of the plugin. It is used in logs, etc. +func (pl *PodTopologySpread) Name() string { + return Name +} + +// Score invoked at the Score extension point. +// The "score" returned in this function is the matching number of pods on the `nodeName`, +// it is normalized later. +func (pl *PodTopologySpread) Score(ctx context.Context, state *framework.CycleState, pod *v1.Pod, nodeName string) (int64, *framework.Status) { + nodeInfo, err := pl.sharedLister.NodeInfos().Get(nodeName) + if err != nil { + return 0, framework.NewStatus(framework.Error, fmt.Sprintf("getting node %q from Snapshot: %v", nodeName, err)) + } + + meta := migration.PriorityMetadata(state) + s, err := priorities.CalculateEvenPodsSpreadPriorityMap(pod, meta, nodeInfo) + return s.Score, migration.ErrorToFrameworkStatus(err) +} + +// NormalizeScore invoked after scoring all nodes. +func (pl *PodTopologySpread) NormalizeScore(ctx context.Context, state *framework.CycleState, pod *v1.Pod, scores framework.NodeScoreList) *framework.Status { + meta := migration.PriorityMetadata(state) + err := priorities.CalculateEvenPodsSpreadPriorityReduce(pod, meta, pl.sharedLister, scores) + return migration.ErrorToFrameworkStatus(err) +} + +// ScoreExtensions of the Score plugin. +func (pl *PodTopologySpread) ScoreExtensions() framework.ScoreExtensions { + return pl +} + +// New initializes a new plugin and returns it. +func New(_ *runtime.Unknown, h framework.FrameworkHandle) (framework.Plugin, error) { + if h.SnapshotSharedLister() == nil { + return nil, fmt.Errorf("SnapshotSharedlister is nil") + } + return &PodTopologySpread{sharedLister: h.SnapshotSharedLister()}, nil +} diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/pod_topology_spread.go b/pkg/scheduler/framework/plugins/podtopologyspread/pod_topology_spread.go deleted file mode 100644 index c5f3fcf6bf2..00000000000 --- a/pkg/scheduler/framework/plugins/podtopologyspread/pod_topology_spread.go +++ /dev/null @@ -1,179 +0,0 @@ -/* -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 podtopologyspread - -import ( - "context" - "fmt" - - "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/klog" - "k8s.io/kubernetes/pkg/scheduler/algorithm/predicates" - "k8s.io/kubernetes/pkg/scheduler/algorithm/priorities" - "k8s.io/kubernetes/pkg/scheduler/framework/plugins/migration" - framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" - schedulerlisters "k8s.io/kubernetes/pkg/scheduler/listers" - "k8s.io/kubernetes/pkg/scheduler/nodeinfo" -) - -// PodTopologySpread is a plugin that ensures pod's topologySpreadConstraints is satisfied. -type PodTopologySpread struct { - snapshotSharedLister schedulerlisters.SharedLister -} - -var _ framework.PreFilterPlugin = &PodTopologySpread{} -var _ framework.FilterPlugin = &PodTopologySpread{} -var _ framework.ScorePlugin = &PodTopologySpread{} - -const ( - // Name is the name of the plugin used in the plugin registry and configurations. - Name = "PodTopologySpread" - - // preFilterStateKey is the key in CycleState to PodTopologySpread pre-computed data. - // Using the name of the plugin will likely help us avoid collisions with other plugins. - preFilterStateKey = "PreFilter" + Name -) - -// Name returns name of the plugin. It is used in logs, etc. -func (pl *PodTopologySpread) Name() string { - return Name -} - -// preFilterState computed at PreFilter and used at Filter. -type preFilterState struct { - // `nil` represents the meta is not set at all (in PreFilter phase) - // An empty `PodTopologySpreadMetadata` object denotes it's a legit meta and is set in PreFilter phase. - meta *predicates.PodTopologySpreadMetadata -} - -// Clone makes a copy of the given state. -func (s *preFilterState) Clone() framework.StateData { - copy := &preFilterState{ - meta: s.meta.Clone(), - } - return copy -} - -// PreFilter invoked at the prefilter extension point. -func (pl *PodTopologySpread) PreFilter(ctx context.Context, cycleState *framework.CycleState, pod *v1.Pod) *framework.Status { - var meta *predicates.PodTopologySpreadMetadata - var allNodes []*nodeinfo.NodeInfo - var err error - - if allNodes, err = pl.snapshotSharedLister.NodeInfos().List(); err != nil { - return framework.NewStatus(framework.Error, fmt.Sprintf("failed to list NodeInfos: %v", err)) - } - - if meta, err = predicates.GetPodTopologySpreadMetadata(pod, allNodes); err != nil { - return framework.NewStatus(framework.Error, fmt.Sprintf("Error calculating podTopologySpreadMetadata: %v", err)) - } - - s := &preFilterState{ - meta: meta, - } - cycleState.Write(preFilterStateKey, s) - - return nil -} - -// PreFilterExtensions returns prefilter extensions, pod add and remove. -func (pl *PodTopologySpread) PreFilterExtensions() framework.PreFilterExtensions { - return pl -} - -// AddPod from pre-computed data in cycleState. -func (pl *PodTopologySpread) AddPod(ctx context.Context, cycleState *framework.CycleState, podToSchedule *v1.Pod, podToAdd *v1.Pod, nodeInfo *nodeinfo.NodeInfo) *framework.Status { - meta, err := getPodTopologySpreadMetadata(cycleState) - if err != nil { - return framework.NewStatus(framework.Error, err.Error()) - } - - meta.AddPod(podToAdd, podToSchedule, nodeInfo.Node()) - return nil -} - -// RemovePod from pre-computed data in cycleState. -func (pl *PodTopologySpread) RemovePod(ctx context.Context, cycleState *framework.CycleState, podToSchedule *v1.Pod, podToRemove *v1.Pod, nodeInfo *nodeinfo.NodeInfo) *framework.Status { - meta, err := getPodTopologySpreadMetadata(cycleState) - if err != nil { - return framework.NewStatus(framework.Error, err.Error()) - } - - meta.RemovePod(podToRemove, podToSchedule, nodeInfo.Node()) - return nil -} - -func getPodTopologySpreadMetadata(cycleState *framework.CycleState) (*predicates.PodTopologySpreadMetadata, error) { - c, err := cycleState.Read(preFilterStateKey) - if err != nil { - // The metadata wasn't pre-computed in prefilter. We ignore the error for now since - // we are able to handle that by computing it again (e.g. in Filter()). - klog.V(5).Infof("Error reading %q from cycleState: %v", preFilterStateKey, err) - return nil, nil - } - - s, ok := c.(*preFilterState) - if !ok { - return nil, fmt.Errorf("%+v convert to podtopologyspread.state error", c) - } - return s.meta, nil -} - -// Filter invoked at the filter extension point. -func (pl *PodTopologySpread) Filter(ctx context.Context, cycleState *framework.CycleState, pod *v1.Pod, nodeInfo *nodeinfo.NodeInfo) *framework.Status { - meta, err := getPodTopologySpreadMetadata(cycleState) - if err != nil { - return framework.NewStatus(framework.Error, err.Error()) - } - _, reasons, err := predicates.PodTopologySpreadPredicate(pod, meta, nodeInfo) - return migration.PredicateResultToFrameworkStatus(reasons, err) -} - -// Score invoked at the Score extension point. -// The "score" returned in this function is the matching number of pods on the `nodeName`, -// it is normalized later. -func (pl *PodTopologySpread) Score(ctx context.Context, state *framework.CycleState, pod *v1.Pod, nodeName string) (int64, *framework.Status) { - nodeInfo, err := pl.snapshotSharedLister.NodeInfos().Get(nodeName) - if err != nil { - return 0, framework.NewStatus(framework.Error, fmt.Sprintf("getting node %q from Snapshot: %v", nodeName, err)) - } - - meta := migration.PriorityMetadata(state) - s, err := priorities.CalculateEvenPodsSpreadPriorityMap(pod, meta, nodeInfo) - return s.Score, migration.ErrorToFrameworkStatus(err) -} - -// NormalizeScore invoked after scoring all nodes. -func (pl *PodTopologySpread) NormalizeScore(ctx context.Context, state *framework.CycleState, pod *v1.Pod, scores framework.NodeScoreList) *framework.Status { - meta := migration.PriorityMetadata(state) - err := priorities.CalculateEvenPodsSpreadPriorityReduce(pod, meta, pl.snapshotSharedLister, scores) - return migration.ErrorToFrameworkStatus(err) -} - -// ScoreExtensions of the Score plugin. -func (pl *PodTopologySpread) ScoreExtensions() framework.ScoreExtensions { - return pl -} - -// New initializes a new plugin and returns it. -func New(_ *runtime.Unknown, h framework.FrameworkHandle) (framework.Plugin, error) { - if h.SnapshotSharedLister() == nil { - return nil, fmt.Errorf("SnapshotSharedlister is nil") - } - return &PodTopologySpread{snapshotSharedLister: h.SnapshotSharedLister()}, nil -} diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/pod_topology_spread_test.go b/pkg/scheduler/framework/plugins/podtopologyspread/pod_topology_spread_test.go deleted file mode 100644 index d3aab10c149..00000000000 --- a/pkg/scheduler/framework/plugins/podtopologyspread/pod_topology_spread_test.go +++ /dev/null @@ -1,487 +0,0 @@ -/* -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 podtopologyspread - -import ( - "context" - "testing" - - "k8s.io/api/core/v1" - framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" - nodeinfosnapshot "k8s.io/kubernetes/pkg/scheduler/nodeinfo/snapshot" - st "k8s.io/kubernetes/pkg/scheduler/testing" -) - -var ( - hardSpread = v1.DoNotSchedule -) - -func TestSingleConstraint(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 doesn'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) { - snapshot := nodeinfosnapshot.NewSnapshot(nodeinfosnapshot.CreateNodeInfoMap(tt.existingPods, tt.nodes)) - p := &PodTopologySpread{snapshotSharedLister: snapshot} - state := framework.NewCycleState() - preFilterStatus := p.PreFilter(context.Background(), state, tt.pod) - if !preFilterStatus.IsSuccess() { - t.Errorf("preFilter failed with status: %v", preFilterStatus) - } - - for _, node := range tt.nodes { - nodeInfo, _ := snapshot.NodeInfos().Get(node.Name) - status := p.Filter(context.Background(), state, tt.pod, nodeInfo) - if status.IsSuccess() != tt.fits[node.Name] { - t.Errorf("[%s]: expected %v got %v", node.Name, tt.fits[node.Name], status.IsSuccess()) - } - } - }) - } -} - -func TestMultipleConstraints(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 - // 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()). - 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 - // 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()). - 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, - }, - }, - { - // 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()). - 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-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": 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, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - snapshot := nodeinfosnapshot.NewSnapshot(nodeinfosnapshot.CreateNodeInfoMap(tt.existingPods, tt.nodes)) - p := &PodTopologySpread{snapshotSharedLister: snapshot} - state := framework.NewCycleState() - preFilterStatus := p.PreFilter(context.Background(), state, tt.pod) - if !preFilterStatus.IsSuccess() { - t.Errorf("preFilter failed with status: %v", preFilterStatus) - } - - for _, node := range tt.nodes { - nodeInfo, _ := snapshot.NodeInfos().Get(node.Name) - status := p.Filter(context.Background(), state, tt.pod, nodeInfo) - if status.IsSuccess() != tt.fits[node.Name] { - t.Errorf("[%s]: expected %v got %v", node.Name, tt.fits[node.Name], status.IsSuccess()) - } - } - }) - } -}