github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-23 11:50:44 +00:00
Code Issues Projects Releases Wiki Activity

Test PodTopologySpread.PreFilter instead of internal pre-processing.

Browse Source 
Signed-off-by: Aldo Culquicondor <acondor@google.com>
This commit is contained in:
Aldo Culquicondor 2020-02-26 14:25:28 -05:00
parent b342818361
commit 0e46803e77
3 changed files with 299 additions and 255 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
pkg/scheduler/framework/plugins/podtopologyspread/common.go
View File

@ -29,16 +29,17 @@ type topologyPair struct {
// topologySpreadConstraint is an internal version for v1.TopologySpreadConstraint // topologySpreadConstraint is an internal version for v1.TopologySpreadConstraint
// and where the selector is parsed. // and where the selector is parsed.
// Fields are exported for comparison during testing.
type topologySpreadConstraint struct { type topologySpreadConstraint struct {
maxSkew int32 MaxSkew int32
topologyKey string TopologyKey string
selector labels.Selector Selector labels.Selector
} }
// nodeLabelsMatchSpreadConstraints checks if ALL topology keys in spread constraints are present in node labels. // nodeLabelsMatchSpreadConstraints checks if ALL topology keys in spread Constraints are present in node labels.
func nodeLabelsMatchSpreadConstraints(nodeLabels map[string]string, constraints []topologySpreadConstraint) bool { func nodeLabelsMatchSpreadConstraints(nodeLabels map[string]string, constraints []topologySpreadConstraint) bool {
for _, c := range constraints { for _, c := range constraints {
if _, ok := nodeLabels[c.topologyKey]; !ok { if _, ok := nodeLabels[c.TopologyKey]; !ok {
return false return false
} }
} }
@ -54,9 +55,9 @@ func filterTopologySpreadConstraints(constraints []v1.TopologySpreadConstraint,
return nil, err return nil, err
} }
result = append(result, topologySpreadConstraint{ result = append(result, topologySpreadConstraint{
maxSkew: c.MaxSkew, MaxSkew: c.MaxSkew,
topologyKey: c.TopologyKey, TopologyKey: c.TopologyKey,
selector: selector, Selector: selector,
}) })
} }
} }

144
pkg/scheduler/framework/plugins/podtopologyspread/filtering.go
View File

@ -29,26 +29,26 @@ import (
pluginhelper "k8s.io/kubernetes/pkg/scheduler/framework/plugins/helper" pluginhelper "k8s.io/kubernetes/pkg/scheduler/framework/plugins/helper"
framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1"
"k8s.io/kubernetes/pkg/scheduler/nodeinfo" "k8s.io/kubernetes/pkg/scheduler/nodeinfo"
schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo"
) )
const preFilterStateKey = "PreFilter" + Name const preFilterStateKey = "PreFilter" + Name
// preFilterState computed at PreFilter and used at Filter. // preFilterState computed at PreFilter and used at Filter.
// It combines tpKeyToCriticalPaths and tpPairToMatchNum to represent: // It combines TpKeyToCriticalPaths and TpPairToMatchNum to represent:
// (1) critical paths where the least pods are matched on each spread constraint. // (1) critical paths where the least pods are matched on each spread constraint.
// (2) number of pods 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); // 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. // An empty preFilterState object denotes it's a legit state and is set in PreFilter phase.
// Fields are exported for comparison during testing.
type preFilterState struct { type preFilterState struct {
constraints []topologySpreadConstraint Constraints []topologySpreadConstraint
// We record 2 critical paths instead of all critical paths here. // We record 2 critical paths instead of all critical paths here.
// criticalPaths[0].matchNum always holds the minimum matching number. // criticalPaths[0].MatchNum always holds the minimum matching number.
// criticalPaths[1].matchNum is always greater or equal to criticalPaths[0].matchNum, but // criticalPaths[1].MatchNum is always greater or equal to criticalPaths[0].MatchNum, but
// it's not guaranteed to be the 2nd minimum match number. // it's not guaranteed to be the 2nd minimum match number.
tpKeyToCriticalPaths map[string]*criticalPaths TpKeyToCriticalPaths map[string]*criticalPaths
// tpPairToMatchNum is keyed with topologyPair, and valued with the number of matching pods. // TpPairToMatchNum is keyed with topologyPair, and valued with the number of matching pods.
tpPairToMatchNum map[topologyPair]int32 TpPairToMatchNum map[topologyPair]int32
} }
// Clone makes a copy of the given state. // Clone makes a copy of the given state.
@ -58,66 +58,65 @@ func (s *preFilterState) Clone() framework.StateData {
return nil return nil
} }
copy := preFilterState{ copy := preFilterState{
// constraints are shared because they don't change. // Constraints are shared because they don't change.
constraints: s.constraints, Constraints: s.Constraints,
tpKeyToCriticalPaths: make(map[string]*criticalPaths, len(s.tpKeyToCriticalPaths)), TpKeyToCriticalPaths: make(map[string]*criticalPaths, len(s.TpKeyToCriticalPaths)),
tpPairToMatchNum: make(map[topologyPair]int32, len(s.tpPairToMatchNum)), TpPairToMatchNum: make(map[topologyPair]int32, len(s.TpPairToMatchNum)),
} }
for tpKey, paths := range s.tpKeyToCriticalPaths { for tpKey, paths := range s.TpKeyToCriticalPaths {
copy.tpKeyToCriticalPaths[tpKey] = &criticalPaths{paths[0], paths[1]} copy.TpKeyToCriticalPaths[tpKey] = &criticalPaths{paths[0], paths[1]}
} }
for tpPair, matchNum := range s.tpPairToMatchNum { for tpPair, matchNum := range s.TpPairToMatchNum {
copyPair := topologyPair{key: tpPair.key, value: tpPair.value} copyPair := topologyPair{key: tpPair.key, value: tpPair.value}
copy.tpPairToMatchNum[copyPair] = matchNum copy.TpPairToMatchNum[copyPair] = matchNum
} }
return &copy return &copy
} }
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 // CAVEAT: the reason that `[2]criticalPath` can work is based on the implementation of current
// preemption algorithm, in particular the following 2 facts: // 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 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. // 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 // If we plan to turn to a more complex algorithm like "arbitrary pods on multiple nodes", this
// structure needs to be revisited. // structure needs to be revisited.
type criticalPaths [2]criticalPath // Fields are exported for comparison during testing.
type criticalPaths [2]struct {
// TopologyValue denotes the topology value mapping to topology key.
TopologyValue string
// MatchNum denotes the number of matching pods.
MatchNum int32
}
func newCriticalPaths() *criticalPaths { func newCriticalPaths() *criticalPaths {
return &criticalPaths{{matchNum: math.MaxInt32}, {matchNum: math.MaxInt32}} return &criticalPaths{{MatchNum: math.MaxInt32}, {MatchNum: math.MaxInt32}}
} }
func (paths *criticalPaths) update(tpVal string, num int32) { func (paths *criticalPaths) update(tpVal string, num int32) {
// first verify if `tpVal` exists or not // first verify if `tpVal` exists or not
i := -1 i := -1
if tpVal == paths[0].topologyValue { if tpVal == paths[0].TopologyValue {
i = 0 i = 0
} else if tpVal == paths[1].topologyValue { } else if tpVal == paths[1].TopologyValue {
i = 1 i = 1
} }
if i >= 0 { if i >= 0 {
// `tpVal` exists // `tpVal` exists
paths[i].matchNum = num paths[i].MatchNum = num
if paths[0].matchNum > paths[1].matchNum { if paths[0].MatchNum > paths[1].MatchNum {
// swap paths[0] and paths[1] // swap paths[0] and paths[1]
paths[0], paths[1] = paths[1], paths[0] paths[0], paths[1] = paths[1], paths[0]
} }
} else { } else {
// `tpVal` doesn't exist // `tpVal` doesn't exist
if num < paths[0].matchNum { if num < paths[0].MatchNum {
// update paths[1] with paths[0] // update paths[1] with paths[0]
paths[1] = paths[0] paths[1] = paths[0]
// update paths[0] // update paths[0]
paths[0].topologyValue, paths[0].matchNum = tpVal, num paths[0].TopologyValue, paths[0].MatchNum = tpVal, num
} else if num < paths[1].matchNum { } else if num < paths[1].MatchNum {
// update paths[1] // update paths[1]
paths[1].topologyValue, paths[1].matchNum = tpVal, num paths[1].TopologyValue, paths[1].MatchNum = tpVal, num
} }
} }
} }
@ -126,40 +125,31 @@ func (s *preFilterState) updateWithPod(updatedPod, preemptorPod *v1.Pod, node *v
if s == nil || updatedPod.Namespace != preemptorPod.Namespace || node == nil { if s == nil || updatedPod.Namespace != preemptorPod.Namespace || node == nil {
return return
} }
if !nodeLabelsMatchSpreadConstraints(node.Labels, s.constraints) { if !nodeLabelsMatchSpreadConstraints(node.Labels, s.Constraints) {
return return
} }
podLabelSet := labels.Set(updatedPod.Labels) podLabelSet := labels.Set(updatedPod.Labels)
for _, constraint := range s.constraints { for _, constraint := range s.Constraints {
if !constraint.selector.Matches(podLabelSet) { if !constraint.Selector.Matches(podLabelSet) {
continue continue
} }
k, v := constraint.topologyKey, node.Labels[constraint.topologyKey] k, v := constraint.TopologyKey, node.Labels[constraint.TopologyKey]
pair := topologyPair{key: k, value: v} pair := topologyPair{key: k, value: v}
s.tpPairToMatchNum[pair] = s.tpPairToMatchNum[pair] + delta s.TpPairToMatchNum[pair] = s.TpPairToMatchNum[pair] + delta
s.tpKeyToCriticalPaths[k].update(v, s.tpPairToMatchNum[pair]) s.TpKeyToCriticalPaths[k].update(v, s.TpPairToMatchNum[pair])
} }
} }
// PreFilter invoked at the prefilter extension point. // PreFilter invoked at the prefilter extension point.
func (pl *PodTopologySpread) PreFilter(ctx context.Context, cycleState *framework.CycleState, pod *v1.Pod) *framework.Status { func (pl *PodTopologySpread) PreFilter(ctx context.Context, cycleState *framework.CycleState, pod *v1.Pod) *framework.Status {
var s *preFilterState s, err := pl.calPreFilterState(pod)
var allNodes []*nodeinfo.NodeInfo if err != nil {
var err error return framework.NewStatus(framework.Error, 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) cycleState.Write(preFilterStateKey, s)
return nil return nil
} }
@ -206,9 +196,13 @@ func getPreFilterState(cycleState *framework.CycleState) (*preFilterState, error
} }
// calPreFilterState computes preFilterState describing how pods are spread on topologies. // calPreFilterState computes preFilterState describing how pods are spread on topologies.
func calPreFilterState(pod *v1.Pod, allNodes []*schedulernodeinfo.NodeInfo) (*preFilterState, error) { func (pl *PodTopologySpread) calPreFilterState(pod *v1.Pod) (*preFilterState, error) {
allNodes, err := pl.sharedLister.NodeInfos().List()
if err != nil {
return nil, fmt.Errorf("listing NodeInfos: %v", err)
}
// We have feature gating in APIServer to strip the spec // We have feature gating in APIServer to strip the spec
// so don't need to re-check feature gate, just check length of constraints. // so don't need to re-check feature gate, just check length of Constraints.
constraints, err := filterTopologySpreadConstraints(pod.Spec.TopologySpreadConstraints, v1.DoNotSchedule) constraints, err := filterTopologySpreadConstraints(pod.Spec.TopologySpreadConstraints, v1.DoNotSchedule)
if err != nil { if err != nil {
return nil, err return nil, err
@ -222,13 +216,13 @@ func calPreFilterState(pod *v1.Pod, allNodes []*schedulernodeinfo.NodeInfo) (*pr
// TODO(Huang-Wei): It might be possible to use "make(map[topologyPair]*int32)". // 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. // In that case, need to consider how to init each tpPairToCount[pair] in an atomic fashion.
s := preFilterState{ s := preFilterState{
constraints: constraints, Constraints: constraints,
tpKeyToCriticalPaths: make(map[string]*criticalPaths, len(constraints)), TpKeyToCriticalPaths: make(map[string]*criticalPaths, len(constraints)),
tpPairToMatchNum: make(map[topologyPair]int32), TpPairToMatchNum: make(map[topologyPair]int32),
} }
addTopologyPairMatchNum := func(pair topologyPair, num int32) { addTopologyPairMatchNum := func(pair topologyPair, num int32) {
lock.Lock() lock.Lock()
s.tpPairToMatchNum[pair] += num s.TpPairToMatchNum[pair] += num
lock.Unlock() lock.Unlock()
} }
@ -245,7 +239,7 @@ func calPreFilterState(pod *v1.Pod, allNodes []*schedulernodeinfo.NodeInfo) (*pr
return return
} }
// Ensure current node's labels contains all topologyKeys in 'constraints'. // Ensure current node's labels contains all topologyKeys in 'Constraints'.
if !nodeLabelsMatchSpreadConstraints(node.Labels, constraints) { if !nodeLabelsMatchSpreadConstraints(node.Labels, constraints) {
return return
} }
@ -257,11 +251,11 @@ func calPreFilterState(pod *v1.Pod, allNodes []*schedulernodeinfo.NodeInfo) (*pr
if existingPod.DeletionTimestamp != nil || existingPod.Namespace != pod.Namespace { if existingPod.DeletionTimestamp != nil || existingPod.Namespace != pod.Namespace {
continue continue
} }
if constraint.selector.Matches(labels.Set(existingPod.Labels)) { if constraint.Selector.Matches(labels.Set(existingPod.Labels)) {
matchTotal++ matchTotal++
} }
} }
pair := topologyPair{key: constraint.topologyKey, value: node.Labels[constraint.topologyKey]} pair := topologyPair{key: constraint.TopologyKey, value: node.Labels[constraint.TopologyKey]}
addTopologyPairMatchNum(pair, matchTotal) addTopologyPairMatchNum(pair, matchTotal)
} }
} }
@ -269,11 +263,11 @@ func calPreFilterState(pod *v1.Pod, allNodes []*schedulernodeinfo.NodeInfo) (*pr
// calculate min match for each topology pair // calculate min match for each topology pair
for i := 0; i < len(constraints); i++ { for i := 0; i < len(constraints); i++ {
key := constraints[i].topologyKey key := constraints[i].TopologyKey
s.tpKeyToCriticalPaths[key] = newCriticalPaths() s.TpKeyToCriticalPaths[key] = newCriticalPaths()
} }
for pair, num := range s.tpPairToMatchNum { for pair, num := range s.TpPairToMatchNum {
s.tpKeyToCriticalPaths[pair.key].update(pair.value, num) s.TpKeyToCriticalPaths[pair.key].update(pair.value, num)
} }
return &s, nil return &s, nil
@ -292,38 +286,38 @@ func (pl *PodTopologySpread) Filter(ctx context.Context, cycleState *framework.C
} }
// However, "empty" preFilterState is legit which tolerates every toSchedule Pod. // However, "empty" preFilterState is legit which tolerates every toSchedule Pod.
if len(s.tpPairToMatchNum) == 0 || len(s.constraints) == 0 { if len(s.TpPairToMatchNum) == 0 || len(s.Constraints) == 0 {
return nil return nil
} }
podLabelSet := labels.Set(pod.Labels) podLabelSet := labels.Set(pod.Labels)
for _, c := range s.constraints { for _, c := range s.Constraints {
tpKey := c.topologyKey tpKey := c.TopologyKey
tpVal, ok := node.Labels[c.topologyKey] tpVal, ok := node.Labels[c.TopologyKey]
if !ok { if !ok {
klog.V(5).Infof("node '%s' doesn't have required label '%s'", node.Name, tpKey) klog.V(5).Infof("node '%s' doesn't have required label '%s'", node.Name, tpKey)
return framework.NewStatus(framework.Unschedulable, ErrReasonConstraintsNotMatch) return framework.NewStatus(framework.Unschedulable, ErrReasonConstraintsNotMatch)
} }
selfMatchNum := int32(0) selfMatchNum := int32(0)
if c.selector.Matches(podLabelSet) { if c.Selector.Matches(podLabelSet) {
selfMatchNum = 1 selfMatchNum = 1
} }
pair := topologyPair{key: tpKey, value: tpVal} pair := topologyPair{key: tpKey, value: tpVal}
paths, ok := s.tpKeyToCriticalPaths[tpKey] paths, ok := s.TpKeyToCriticalPaths[tpKey]
if !ok { if !ok {
// error which should not happen // error which should not happen
klog.Errorf("internal error: get paths from key %q of %#v", tpKey, s.tpKeyToCriticalPaths) klog.Errorf("internal error: get paths from key %q of %#v", tpKey, s.TpKeyToCriticalPaths)
continue continue
} }
// judging criteria: // judging criteria:
// 'existing matching num' + 'if self-match (1 or 0)' - 'global min matching num' <= 'maxSkew' // 'existing matching num' + 'if self-match (1 or 0)' - 'global min matching num' <= 'maxSkew'
minMatchNum := paths[0].matchNum minMatchNum := paths[0].MatchNum
matchNum := s.tpPairToMatchNum[pair] matchNum := s.TpPairToMatchNum[pair]
skew := matchNum + selfMatchNum - minMatchNum skew := matchNum + selfMatchNum - minMatchNum
if skew > c.maxSkew { 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) 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 framework.NewStatus(framework.Unschedulable, ErrReasonConstraintsNotMatch) return framework.NewStatus(framework.Unschedulable, ErrReasonConstraintsNotMatch)
} }
} }

393
pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go
View File

@ -21,6 +21,7 @@ import (
"reflect" "reflect"
"testing" "testing"
"github.com/google/go-cmp/cmp"
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/labels"
@ -35,7 +36,24 @@ var (
softSpread = v1.ScheduleAnyway softSpread = v1.ScheduleAnyway
) )
func TestCalPreFilterState(t *testing.T) { func (s preFilterState) Equal(o preFilterState) bool {
type internal preFilterState
type internalCP criticalPaths
return cmp.Equal(internal(s), internal(o),
cmp.Comparer(func(s1 labels.Selector, s2 labels.Selector) bool {
return reflect.DeepEqual(s1, s2)
}),
cmp.Transformer("sort", func(p criticalPaths) internalCP {
if p[0].MatchNum == p[1].MatchNum && p[0].TopologyValue > p[1].TopologyValue {
// Swap TopologyValue to make them sorted alphabetically.
p[0].TopologyValue, p[1].TopologyValue = p[1].TopologyValue, p[0].TopologyValue
}
return internalCP(p)
}),
)
}
func TestPreFilterState(t *testing.T) {
fooSelector := st.MakeLabelSelector().Exists("foo").Obj() fooSelector := st.MakeLabelSelector().Exists("foo").Obj()
barSelector := st.MakeLabelSelector().Exists("bar").Obj() barSelector := st.MakeLabelSelector().Exists("bar").Obj()
tests := []struct { tests := []struct {
@ -57,17 +75,17 @@ func TestCalPreFilterState(t *testing.T) {
st.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(), st.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(),
}, },
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{ Constraints: []topologySpreadConstraint{
{ {
maxSkew: 5, MaxSkew: 5,
topologyKey: "zone", TopologyKey: "zone",
selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Label("foo", "bar").Obj()), Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Label("foo", "bar").Obj()),
}, },
}, },
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 0}, {"zone2", 0}}, "zone": {{"zone1", 0}, {"zone2", 0}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 0, {key: "zone", value: "zone1"}: 0,
{key: "zone", value: "zone2"}: 0, {key: "zone", value: "zone2"}: 0,
}, },
@ -92,17 +110,17 @@ func TestCalPreFilterState(t *testing.T) {
st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(),
}, },
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{ Constraints: []topologySpreadConstraint{
{ {
maxSkew: 1, MaxSkew: 1,
topologyKey: "zone", TopologyKey: "zone",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector), Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
}, },
}, },
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone2", 2}, {"zone1", 3}}, "zone": {{"zone2", 2}, {"zone1", 3}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 3, {key: "zone", value: "zone1"}: 3,
{key: "zone", value: "zone2"}: 2, {key: "zone", value: "zone2"}: 2,
}, },
@ -129,17 +147,17 @@ func TestCalPreFilterState(t *testing.T) {
st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(),
}, },
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{ Constraints: []topologySpreadConstraint{
{ {
maxSkew: 1, MaxSkew: 1,
topologyKey: "zone", TopologyKey: "zone",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector), Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
}, },
}, },
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone3", 0}, {"zone2", 2}}, "zone": {{"zone3", 0}, {"zone2", 2}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 3, {key: "zone", value: "zone1"}: 3,
{key: "zone", value: "zone2"}: 2, {key: "zone", value: "zone2"}: 2,
{key: "zone", value: "zone3"}: 0, {key: "zone", value: "zone3"}: 0,
@ -165,17 +183,17 @@ func TestCalPreFilterState(t *testing.T) {
st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(),
}, },
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{ Constraints: []topologySpreadConstraint{
{ {
maxSkew: 1, MaxSkew: 1,
topologyKey: "zone", TopologyKey: "zone",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector), Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
}, },
}, },
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone2", 1}, {"zone1", 2}}, "zone": {{"zone2", 1}, {"zone1", 2}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 2, {key: "zone", value: "zone1"}: 2,
{key: "zone", value: "zone2"}: 1, {key: "zone", value: "zone2"}: 1,
}, },
@ -203,23 +221,23 @@ func TestCalPreFilterState(t *testing.T) {
st.MakePod().Name("p-y4").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y4").Node("node-y").Label("foo", "").Obj(),
}, },
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{ Constraints: []topologySpreadConstraint{
{ {
maxSkew: 1, MaxSkew: 1,
topologyKey: "zone", TopologyKey: "zone",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector), Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
}, },
{ {
maxSkew: 1, MaxSkew: 1,
topologyKey: "node", TopologyKey: "node",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector), Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
}, },
}, },
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 3}, {"zone2", 4}}, "zone": {{"zone1", 3}, {"zone2", 4}},
"node": {{"node-x", 0}, {"node-b", 1}}, "node": {{"node-x", 0}, {"node-b", 1}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 3, {key: "zone", value: "zone1"}: 3,
{key: "zone", value: "zone2"}: 4, {key: "zone", value: "zone2"}: 4,
{key: "node", value: "node-a"}: 2, {key: "node", value: "node-a"}: 2,
@ -252,23 +270,23 @@ func TestCalPreFilterState(t *testing.T) {
st.MakePod().Name("p-y4").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y4").Node("node-y").Label("foo", "").Obj(),
}, },
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{ Constraints: []topologySpreadConstraint{
{ {
maxSkew: 1, MaxSkew: 1,
topologyKey: "zone", TopologyKey: "zone",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector), Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
}, },
{ {
maxSkew: 1, MaxSkew: 1,
topologyKey: "node", TopologyKey: "node",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector), Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
}, },
}, },
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 3}, {"zone2", 4}}, "zone": {{"zone1", 3}, {"zone2", 4}},
"node": {{"node-b", 1}, {"node-a", 2}}, "node": {{"node-b", 1}, {"node-a", 2}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 3, {key: "zone", value: "zone1"}: 3,
{key: "zone", value: "zone2"}: 4, {key: "zone", value: "zone2"}: 4,
{key: "node", value: "node-a"}: 2, {key: "node", value: "node-a"}: 2,
@ -293,23 +311,23 @@ func TestCalPreFilterState(t *testing.T) {
st.MakePod().Name("p-b").Node("node-b").Label("bar", "").Obj(), st.MakePod().Name("p-b").Node("node-b").Label("bar", "").Obj(),
}, },
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{ Constraints: []topologySpreadConstraint{
{ {
maxSkew: 1, MaxSkew: 1,
topologyKey: "zone", TopologyKey: "zone",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector), Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
}, },
{ {
maxSkew: 1, MaxSkew: 1,
topologyKey: "node", TopologyKey: "node",
selector: mustConvertLabelSelectorAsSelector(t, barSelector), Selector: mustConvertLabelSelectorAsSelector(t, barSelector),
}, },
}, },
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone2", 0}, {"zone1", 1}}, "zone": {{"zone2", 0}, {"zone1", 1}},
"node": {{"node-a", 0}, {"node-y", 0}}, "node": {{"node-a", 0}, {"node-y", 0}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 1, {key: "zone", value: "zone1"}: 1,
{key: "zone", value: "zone2"}: 0, {key: "zone", value: "zone2"}: 0,
{key: "node", value: "node-a"}: 0, {key: "node", value: "node-a"}: 0,
@ -339,23 +357,23 @@ func TestCalPreFilterState(t *testing.T) {
st.MakePod().Name("p-y4").Node("node-y").Label("foo", "").Label("bar", "").Obj(), st.MakePod().Name("p-y4").Node("node-y").Label("foo", "").Label("bar", "").Obj(),
}, },
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{ Constraints: []topologySpreadConstraint{
{ {
maxSkew: 1, MaxSkew: 1,
topologyKey: "zone", TopologyKey: "zone",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector), Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
}, },
{ {
maxSkew: 1, MaxSkew: 1,
topologyKey: "node", TopologyKey: "node",
selector: mustConvertLabelSelectorAsSelector(t, barSelector), Selector: mustConvertLabelSelectorAsSelector(t, barSelector),
}, },
}, },
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 3}, {"zone2", 4}}, "zone": {{"zone1", 3}, {"zone2", 4}},
"node": {{"node-b", 0}, {"node-a", 1}}, "node": {{"node-b", 0}, {"node-a", 1}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 3, {key: "zone", value: "zone1"}: 3,
{key: "zone", value: "zone2"}: 4, {key: "zone", value: "zone2"}: 4,
{key: "node", value: "node-a"}: 1, {key: "node", value: "node-a"}: 1,
@ -387,23 +405,23 @@ func TestCalPreFilterState(t *testing.T) {
st.MakePod().Name("p-y4").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y4").Node("node-y").Label("foo", "").Obj(),
}, },
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{ Constraints: []topologySpreadConstraint{
{ {
maxSkew: 1, MaxSkew: 1,
topologyKey: "zone", TopologyKey: "zone",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector), Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
}, },
{ {
maxSkew: 1, MaxSkew: 1,
topologyKey: "node", TopologyKey: "node",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector), Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
}, },
}, },
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 3}, {"zone2", 4}}, "zone": {{"zone1", 3}, {"zone2", 4}},
"node": {{"node-b", 1}, {"node-a", 2}}, "node": {{"node-b", 1}, {"node-a", 2}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 3, {key: "zone", value: "zone1"}: 3,
{key: "zone", value: "zone2"}: 4, {key: "zone", value: "zone2"}: 4,
{key: "node", value: "node-a"}: 2, {key: "node", value: "node-a"}: 2,
@ -415,12 +433,19 @@ func TestCalPreFilterState(t *testing.T) {
} }
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
s := cache.NewSnapshot(tt.existingPods, tt.nodes) pl := PodTopologySpread{
l, _ := s.NodeInfos().List() sharedLister: cache.NewSnapshot(tt.existingPods, tt.nodes),
got, _ := calPreFilterState(tt.pod, l) }
got.sortCriticalPaths() cs := framework.NewCycleState()
if !reflect.DeepEqual(got, tt.want) { if s := pl.PreFilter(context.Background(), cs, tt.pod); !s.IsSuccess() {
t.Errorf("calPreFilterState() = %#v, want %#v", *got, *tt.want) t.Fatal(s.AsError())
}
got, err := getPreFilterState(cs)
if err != nil {
t.Fatal(err)
}
if diff := cmp.Diff(got, tt.want); diff != "" {
t.Errorf("PodTopologySpread#PreFilter() returned diff (-want,+got):\n%s", diff)
} }
}) })
} }
@ -428,12 +453,12 @@ func TestCalPreFilterState(t *testing.T) {
func TestPreFilterStateAddPod(t *testing.T) { func TestPreFilterStateAddPod(t *testing.T) {
nodeConstraint := topologySpreadConstraint{ nodeConstraint := topologySpreadConstraint{
maxSkew: 1, MaxSkew: 1,
topologyKey: "node", TopologyKey: "node",
selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("foo").Obj()), Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("foo").Obj()),
} }
zoneConstraint := nodeConstraint zoneConstraint := nodeConstraint
zoneConstraint.topologyKey = "zone" zoneConstraint.TopologyKey = "zone"
tests := []struct { tests := []struct {
name string name string
preemptor *v1.Pod preemptor *v1.Pod
@ -456,11 +481,11 @@ func TestPreFilterStateAddPod(t *testing.T) {
st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(), st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(),
}, },
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{nodeConstraint}, Constraints: []topologySpreadConstraint{nodeConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"node": {{"node-b", 0}, {"node-a", 1}}, "node": {{"node-b", 0}, {"node-a", 1}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "node", value: "node-a"}: 1, {key: "node", value: "node-a"}: 1,
{key: "node", value: "node-b"}: 0, {key: "node", value: "node-b"}: 0,
}, },
@ -481,11 +506,11 @@ func TestPreFilterStateAddPod(t *testing.T) {
st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(), st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(),
}, },
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{nodeConstraint}, Constraints: []topologySpreadConstraint{nodeConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"node": {{"node-a", 1}, {"node-b", 1}}, "node": {{"node-a", 1}, {"node-b", 1}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "node", value: "node-a"}: 1, {key: "node", value: "node-a"}: 1,
{key: "node", value: "node-b"}: 1, {key: "node", value: "node-b"}: 1,
}, },
@ -506,11 +531,11 @@ func TestPreFilterStateAddPod(t *testing.T) {
st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(), st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(),
}, },
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{nodeConstraint}, Constraints: []topologySpreadConstraint{nodeConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"node": {{"node-a", 0}, {"node-b", 1}}, "node": {{"node-a", 0}, {"node-b", 1}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "node", value: "node-a"}: 0, {key: "node", value: "node-a"}: 0,
{key: "node", value: "node-b"}: 1, {key: "node", value: "node-b"}: 1,
}, },
@ -531,11 +556,11 @@ func TestPreFilterStateAddPod(t *testing.T) {
st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(), st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(),
}, },
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{nodeConstraint}, Constraints: []topologySpreadConstraint{nodeConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"node": {{"node-a", 0}, {"node-b", 2}}, "node": {{"node-a", 0}, {"node-b", 2}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "node", value: "node-a"}: 0, {key: "node", value: "node-a"}: 0,
{key: "node", value: "node-b"}: 2, {key: "node", value: "node-b"}: 2,
}, },
@ -555,12 +580,12 @@ func TestPreFilterStateAddPod(t *testing.T) {
st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(), st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(),
}, },
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint}, Constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone2", 0}, {"zone1", 1}}, "zone": {{"zone2", 0}, {"zone1", 1}},
"node": {{"node-x", 0}, {"node-a", 1}}, "node": {{"node-x", 0}, {"node-a", 1}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 1, {key: "zone", value: "zone1"}: 1,
{key: "zone", value: "zone2"}: 0, {key: "zone", value: "zone2"}: 0,
{key: "node", value: "node-a"}: 1, {key: "node", value: "node-a"}: 1,
@ -584,12 +609,12 @@ func TestPreFilterStateAddPod(t *testing.T) {
st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(), st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(),
}, },
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint}, Constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 1}, {"zone2", 1}}, "zone": {{"zone1", 1}, {"zone2", 1}},
"node": {{"node-a", 1}, {"node-x", 1}}, "node": {{"node-a", 1}, {"node-x", 1}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 1, {key: "zone", value: "zone1"}: 1,
{key: "zone", value: "zone2"}: 1, {key: "zone", value: "zone2"}: 1,
{key: "node", value: "node-a"}: 1, {key: "node", value: "node-a"}: 1,
@ -616,12 +641,12 @@ func TestPreFilterStateAddPod(t *testing.T) {
st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(), st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(),
}, },
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint}, Constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone2", 1}, {"zone1", 3}}, "zone": {{"zone2", 1}, {"zone1", 3}},
"node": {{"node-a", 1}, {"node-x", 1}}, "node": {{"node-a", 1}, {"node-x", 1}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 3, {key: "zone", value: "zone1"}: 3,
{key: "zone", value: "zone2"}: 1, {key: "zone", value: "zone2"}: 1,
{key: "node", value: "node-a"}: 1, {key: "node", value: "node-a"}: 1,
@ -631,7 +656,7 @@ func TestPreFilterStateAddPod(t *testing.T) {
}, },
}, },
{ {
name: "constraints hold different labelSelectors, node a impacts topologyKeyToMinPodsMap on zone", name: "Constraints hold different labelSelectors, node a impacts topologyKeyToMinPodsMap on zone",
preemptor: st.MakePod().Name("p").Label("foo", "").Label("bar", ""). preemptor: st.MakePod().Name("p").Label("foo", "").Label("bar", "").
SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()).
SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("bar").Obj()). SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("bar").Obj()).
@ -649,19 +674,19 @@ func TestPreFilterStateAddPod(t *testing.T) {
st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(), st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(),
}, },
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{ Constraints: []topologySpreadConstraint{
zoneConstraint, zoneConstraint,
{ {
maxSkew: 1, MaxSkew: 1,
topologyKey: "node", TopologyKey: "node",
selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("bar").Obj()), Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("bar").Obj()),
}, },
}, },
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone2", 1}, {"zone1", 2}}, "zone": {{"zone2", 1}, {"zone1", 2}},
"node": {{"node-a", 0}, {"node-b", 1}}, "node": {{"node-a", 0}, {"node-b", 1}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 2, {key: "zone", value: "zone1"}: 2,
{key: "zone", value: "zone2"}: 1, {key: "zone", value: "zone2"}: 1,
{key: "node", value: "node-a"}: 0, {key: "node", value: "node-a"}: 0,
@ -671,7 +696,7 @@ func TestPreFilterStateAddPod(t *testing.T) {
}, },
}, },
{ {
name: "constraints hold different labelSelectors, node a impacts topologyKeyToMinPodsMap on both zone and node", name: "Constraints hold different labelSelectors, node a impacts topologyKeyToMinPodsMap on both zone and node",
preemptor: st.MakePod().Name("p").Label("foo", "").Label("bar", ""). preemptor: st.MakePod().Name("p").Label("foo", "").Label("bar", "").
SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()).
SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("bar").Obj()). SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("bar").Obj()).
@ -689,19 +714,19 @@ func TestPreFilterStateAddPod(t *testing.T) {
st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(), st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(),
}, },
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{ Constraints: []topologySpreadConstraint{
zoneConstraint, zoneConstraint,
{ {
maxSkew: 1, MaxSkew: 1,
topologyKey: "node", TopologyKey: "node",
selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("bar").Obj()), Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("bar").Obj()),
}, },
}, },
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 1}, {"zone2", 1}}, "zone": {{"zone1", 1}, {"zone2", 1}},
"node": {{"node-a", 1}, {"node-b", 1}}, "node": {{"node-a", 1}, {"node-b", 1}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 1, {key: "zone", value: "zone1"}: 1,
{key: "zone", value: "zone2"}: 1, {key: "zone", value: "zone2"}: 1,
{key: "node", value: "node-a"}: 1, {key: "node", value: "node-a"}: 1,
@ -713,13 +738,28 @@ func TestPreFilterStateAddPod(t *testing.T) {
} }
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
s := cache.NewSnapshot(tt.existingPods, tt.nodes) snapshot := cache.NewSnapshot(tt.existingPods, tt.nodes)
l, _ := s.NodeInfos().List() pl := PodTopologySpread{
state, _ := calPreFilterState(tt.preemptor, l) sharedLister: snapshot,
state.updateWithPod(tt.addedPod, tt.preemptor, tt.nodes[tt.nodeIdx], 1) }
state.sortCriticalPaths() cs := framework.NewCycleState()
if !reflect.DeepEqual(state, tt.want) { ctx := context.Background()
t.Errorf("preFilterState#AddPod() = %v, want %v", state, tt.want) if s := pl.PreFilter(ctx, cs, tt.preemptor); !s.IsSuccess() {
t.Fatal(s.AsError())
}
nodeInfo, err := snapshot.Get(tt.nodes[tt.nodeIdx].Name)
if err != nil {
t.Fatal(err)
}
if s := pl.AddPod(ctx, cs, tt.preemptor, tt.addedPod, nodeInfo); !s.IsSuccess() {
t.Fatal(s.AsError())
}
state, err := getPreFilterState(cs)
if err != nil {
t.Fatal(err)
}
if diff := cmp.Diff(state, tt.want); diff != "" {
t.Errorf("PodTopologySpread.AddPod() returned diff (-want,+got):\n%s", diff)
} }
}) })
} }
@ -727,12 +767,12 @@ func TestPreFilterStateAddPod(t *testing.T) {
func TestPreFilterStateRemovePod(t *testing.T) { func TestPreFilterStateRemovePod(t *testing.T) {
nodeConstraint := topologySpreadConstraint{ nodeConstraint := topologySpreadConstraint{
maxSkew: 1, MaxSkew: 1,
topologyKey: "node", TopologyKey: "node",
selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("foo").Obj()), Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("foo").Obj()),
} }
zoneConstraint := nodeConstraint zoneConstraint := nodeConstraint
zoneConstraint.topologyKey = "zone" zoneConstraint.TopologyKey = "zone"
tests := []struct { tests := []struct {
name string name string
preemptor *v1.Pod // preemptor pod preemptor *v1.Pod // preemptor pod
@ -763,11 +803,11 @@ func TestPreFilterStateRemovePod(t *testing.T) {
deletedPodIdx: 0, // remove pod "p-a1" deletedPodIdx: 0, // remove pod "p-a1"
nodeIdx: 0, // node-a nodeIdx: 0, // node-a
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{zoneConstraint}, Constraints: []topologySpreadConstraint{zoneConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 1}, {"zone2", 1}}, "zone": {{"zone1", 1}, {"zone2", 1}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 1, {key: "zone", value: "zone1"}: 1,
{key: "zone", value: "zone2"}: 1, {key: "zone", value: "zone2"}: 1,
}, },
@ -793,11 +833,11 @@ func TestPreFilterStateRemovePod(t *testing.T) {
deletedPodIdx: 0, // remove pod "p-a1" deletedPodIdx: 0, // remove pod "p-a1"
nodeIdx: 0, // node-a nodeIdx: 0, // node-a
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{zoneConstraint}, Constraints: []topologySpreadConstraint{zoneConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 1}, {"zone2", 2}}, "zone": {{"zone1", 1}, {"zone2", 2}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 1, {key: "zone", value: "zone1"}: 1,
{key: "zone", value: "zone2"}: 2, {key: "zone", value: "zone2"}: 2,
}, },
@ -824,11 +864,11 @@ func TestPreFilterStateRemovePod(t *testing.T) {
deletedPodIdx: 0, // remove pod "p-a0" deletedPodIdx: 0, // remove pod "p-a0"
nodeIdx: 0, // node-a nodeIdx: 0, // node-a
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{zoneConstraint}, Constraints: []topologySpreadConstraint{zoneConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 2}, {"zone2", 2}}, "zone": {{"zone1", 2}, {"zone2", 2}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 2, {key: "zone", value: "zone1"}: 2,
{key: "zone", value: "zone2"}: 2, {key: "zone", value: "zone2"}: 2,
}, },
@ -855,11 +895,11 @@ func TestPreFilterStateRemovePod(t *testing.T) {
deletedPod: st.MakePod().Name("p-a0").Node("node-a").Label("bar", "").Obj(), deletedPod: st.MakePod().Name("p-a0").Node("node-a").Label("bar", "").Obj(),
nodeIdx: 0, // node-a nodeIdx: 0, // node-a
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{zoneConstraint}, Constraints: []topologySpreadConstraint{zoneConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 2}, {"zone2", 2}}, "zone": {{"zone1", 2}, {"zone2", 2}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 2, {key: "zone", value: "zone1"}: 2,
{key: "zone", value: "zone2"}: 2, {key: "zone", value: "zone2"}: 2,
}, },
@ -886,12 +926,12 @@ func TestPreFilterStateRemovePod(t *testing.T) {
deletedPodIdx: 3, // remove pod "p-x1" deletedPodIdx: 3, // remove pod "p-x1"
nodeIdx: 2, // node-x nodeIdx: 2, // node-x
want: &preFilterState{ want: &preFilterState{
constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint}, Constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{ TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone2", 1}, {"zone1", 3}}, "zone": {{"zone2", 1}, {"zone1", 3}},
"node": {{"node-b", 1}, {"node-x", 1}}, "node": {{"node-b", 1}, {"node-x", 1}},
}, },
tpPairToMatchNum: map[topologyPair]int32{ TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 3, {key: "zone", value: "zone1"}: 3,
{key: "zone", value: "zone2"}: 1, {key: "zone", value: "zone2"}: 1,
{key: "node", value: "node-a"}: 2, {key: "node", value: "node-a"}: 2,
@ -903,20 +943,36 @@ func TestPreFilterStateRemovePod(t *testing.T) {
} }
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
s := cache.NewSnapshot(tt.existingPods, tt.nodes) snapshot := cache.NewSnapshot(tt.existingPods, tt.nodes)
l, _ := s.NodeInfos().List() pl := PodTopologySpread{
state, _ := calPreFilterState(tt.preemptor, l) sharedLister: snapshot,
}
cs := framework.NewCycleState()
ctx := context.Background()
s := pl.PreFilter(ctx, cs, tt.preemptor)
if !s.IsSuccess() {
t.Fatal(s.AsError())
}
var deletedPod *v1.Pod deletedPod := tt.deletedPod
if tt.deletedPodIdx < len(tt.existingPods) && tt.deletedPodIdx >= 0 { if tt.deletedPodIdx < len(tt.existingPods) && tt.deletedPodIdx >= 0 {
deletedPod = tt.existingPods[tt.deletedPodIdx] deletedPod = tt.existingPods[tt.deletedPodIdx]
} else {
deletedPod = tt.deletedPod
} }
state.updateWithPod(deletedPod, tt.preemptor, tt.nodes[tt.nodeIdx], -1)
state.sortCriticalPaths() nodeInfo, err := snapshot.Get(tt.nodes[tt.nodeIdx].Name)
if !reflect.DeepEqual(state, tt.want) { if err != nil {
t.Errorf("preFilterState#RemovePod() = %v, want %v", state, tt.want) t.Fatal(err)
}
if s := pl.RemovePod(ctx, cs, tt.preemptor, deletedPod, nodeInfo); !s.IsSuccess() {
t.Fatal(s.AsError())
}
state, err := getPreFilterState(cs)
if err != nil {
t.Fatal(err)
}
if diff := cmp.Diff(state, tt.want); diff != "" {
t.Errorf("PodTopologySpread.RemovePod() returned diff (-want,+got):\n%s", diff)
} }
}) })
} }
@ -949,7 +1005,7 @@ func BenchmarkTestCalPreFilterState(b *testing.B) {
filteredNodesNum: 500, filteredNodesNum: 500,
}, },
{ {
name: "1000nodes/two-constraints-zone-node", name: "1000nodes/two-Constraints-zone-node",
pod: st.MakePod().Name("p").Label("foo", "").Label("bar", ""). pod: st.MakePod().Name("p").Label("foo", "").Label("bar", "").
SpreadConstraint(1, v1.LabelZoneFailureDomain, hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). SpreadConstraint(1, v1.LabelZoneFailureDomain, hardSpread, st.MakeLabelSelector().Exists("foo").Obj()).
SpreadConstraint(1, v1.LabelHostname, hardSpread, st.MakeLabelSelector().Exists("bar").Obj()). SpreadConstraint(1, v1.LabelHostname, hardSpread, st.MakeLabelSelector().Exists("bar").Obj()).
@ -962,27 +1018,20 @@ func BenchmarkTestCalPreFilterState(b *testing.B) {
for _, tt := range tests { for _, tt := range tests {
b.Run(tt.name, func(b *testing.B) { b.Run(tt.name, func(b *testing.B) {
existingPods, allNodes, _ := st.MakeNodesAndPodsForEvenPodsSpread(tt.pod.Labels, tt.existingPodsNum, tt.allNodesNum, tt.filteredNodesNum) existingPods, allNodes, _ := st.MakeNodesAndPodsForEvenPodsSpread(tt.pod.Labels, tt.existingPodsNum, tt.allNodesNum, tt.filteredNodesNum)
s := cache.NewSnapshot(existingPods, allNodes) pl := PodTopologySpread{
l, _ := s.NodeInfos().List() sharedLister: cache.NewSnapshot(existingPods, allNodes),
}
b.ResetTimer() b.ResetTimer()
for i := 0; i < b.N; i++ { for i := 0; i < b.N; i++ {
calPreFilterState(tt.pod, l) s := pl.PreFilter(context.Background(), framework.NewCycleState(), tt.pod)
if !s.IsSuccess() {
b.Fatal(s.AsError())
}
} }
}) })
} }
} }
// sortCriticalPaths is only served for testing purpose.
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.
paths[0].topologyValue, paths[1].topologyValue = paths[1].topologyValue, paths[0].topologyValue
}
}
}
func mustConvertLabelSelectorAsSelector(t *testing.T, ls *metav1.LabelSelector) labels.Selector { func mustConvertLabelSelectorAsSelector(t *testing.T, ls *metav1.LabelSelector) labels.Selector {
t.Helper() t.Helper()
s, err := metav1.LabelSelectorAsSelector(ls) s, err := metav1.LabelSelectorAsSelector(ls)
@ -1278,7 +1327,7 @@ func TestMultipleConstraints(t *testing.T) {
// 1. to fulfil "zone" constraint, incoming pod can be placed on any zone (hence any node) // 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 // 2. to fulfil "node" constraint, incoming pod can be placed on node-x
// intersection of (1) and (2) returns node-x // intersection of (1) and (2) returns node-x
name: "two constraints on zone and node, spreads = [3/3, 2/1/0/3]", name: "two Constraints on zone and node, spreads = [3/3, 2/1/0/3]",
pod: st.MakePod().Name("p").Label("foo", ""). pod: st.MakePod().Name("p").Label("foo", "").
SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()).
SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()).
@ -1308,7 +1357,7 @@ func TestMultipleConstraints(t *testing.T) {
// 1. to fulfil "zone" constraint, incoming pod can be placed on zone1 (node-a or node-b) // 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 // 2. to fulfil "node" constraint, incoming pod can be placed on node-x
// intersection of (1) and (2) returns no node // intersection of (1) and (2) returns no node
name: "two constraints on zone and node, spreads = [3/4, 2/1/0/4]", name: "two Constraints on zone and node, spreads = [3/4, 2/1/0/4]",
pod: st.MakePod().Name("p").Label("foo", ""). pod: st.MakePod().Name("p").Label("foo", "").
SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()).
SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()).
@ -1339,7 +1388,7 @@ func TestMultipleConstraints(t *testing.T) {
// 1. to fulfil "zone" constraint, incoming pod can be placed on zone2 (node-x or node-y) // 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 // 2. to fulfil "node" constraint, incoming pod can be placed on node-b or node-x
// intersection of (1) and (2) returns node-x // intersection of (1) and (2) returns node-x
name: "constraints hold different labelSelectors, spreads = [1/0, 1/0/0/1]", name: "Constraints hold different labelSelectors, spreads = [1/0, 1/0/0/1]",
pod: st.MakePod().Name("p").Label("foo", "").Label("bar", ""). pod: st.MakePod().Name("p").Label("foo", "").Label("bar", "").
SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()).
SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("bar").Obj()). SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("bar").Obj()).
@ -1365,7 +1414,7 @@ func TestMultipleConstraints(t *testing.T) {
// 1. to fulfil "zone" constraint, incoming pod can be placed on zone2 (node-x or node-y) // 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 // 2. to fulfil "node" constraint, incoming pod can be placed on node-a or node-b
// intersection of (1) and (2) returns no node // intersection of (1) and (2) returns no node
name: "constraints hold different labelSelectors, spreads = [1/0, 0/0/1/1]", name: "Constraints hold different labelSelectors, spreads = [1/0, 0/0/1/1]",
pod: st.MakePod().Name("p").Label("foo", "").Label("bar", ""). pod: st.MakePod().Name("p").Label("foo", "").Label("bar", "").
SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()).
SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("bar").Obj()). SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("bar").Obj()).
@ -1392,7 +1441,7 @@ func TestMultipleConstraints(t *testing.T) {
// 1. to fulfil "zone" constraint, incoming pod can be placed on zone1 (node-a or node-b) // 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 // 2. to fulfil "node" constraint, incoming pod can be placed on node-b or node-x
// intersection of (1) and (2) returns node-b // intersection of (1) and (2) returns node-b
name: "constraints hold different labelSelectors, spreads = [2/3, 1/0/0/1]", name: "Constraints hold different labelSelectors, spreads = [2/3, 1/0/0/1]",
pod: st.MakePod().Name("p").Label("foo", "").Label("bar", ""). pod: st.MakePod().Name("p").Label("foo", "").Label("bar", "").
SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()).
SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("bar").Obj()). SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("bar").Obj()).
@ -1421,7 +1470,7 @@ func TestMultipleConstraints(t *testing.T) {
// 1. pod doesn't match itself on "zone" constraint, so it can be put onto any zone // 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 // 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 // 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", name: "Constraints hold different labelSelectors but pod doesn't match itself on 'zone' constraint",
pod: st.MakePod().Name("p").Label("bar", ""). pod: st.MakePod().Name("p").Label("bar", "").
SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()).
SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("bar").Obj()). SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("bar").Obj()).