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

Merge pull request #88589 from alculquicondor/cleanup/spread

Browse Source 
Test PodTopologySpread.{PreFilter,PreScore} instead of internal pre-processing
This commit is contained in:
Kubernetes Prow Robot 2020-02-26 19:40:11 -08:00 committed by GitHub
commit 48a4da8a19
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 411 additions and 323 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
pkg/scheduler/framework/plugins/podtopologyspread/BUILD
View File

@ -41,6 +41,8 @@ go_test(
"//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/util/sets:go_default_library",
"//vendor/github.com/google/go-cmp/cmp:go_default_library",
"//vendor/k8s.io/utils/pointer:go_default_library",
],
)

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
// and where the selector is parsed.
// Fields are exported for comparison during testing.
type topologySpreadConstraint struct {
maxSkew int32
topologyKey string
selector labels.Selector
MaxSkew int32
TopologyKey string
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 {
for _, c := range constraints {
if _, ok := nodeLabels[c.topologyKey]; !ok {
if _, ok := nodeLabels[c.TopologyKey]; !ok {
return false
}
}
@ -54,9 +55,9 @@ func filterTopologySpreadConstraints(constraints []v1.TopologySpreadConstraint,
return nil, err
}
result = append(result, topologySpreadConstraint{
maxSkew: c.MaxSkew,
topologyKey: c.TopologyKey,
selector: selector,
MaxSkew: c.MaxSkew,
TopologyKey: c.TopologyKey,
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"
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:
// 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.
// Fields are exported for comparison during testing.
type preFilterState struct {
constraints []topologySpreadConstraint
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
// 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
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.
@ -58,66 +58,65 @@ func (s *preFilterState) Clone() framework.StateData {
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)),
// 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 tpKey, paths := range s.TpKeyToCriticalPaths {
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}
copy.tpPairToMatchNum[copyPair] = matchNum
copy.TpPairToMatchNum[copyPair] = matchNum
}
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
// 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
// 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 {
return &criticalPaths{{matchNum: math.MaxInt32}, {matchNum: math.MaxInt32}}
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 {
if tpVal == paths[0].TopologyValue {
i = 0
} else if tpVal == paths[1].topologyValue {
} else if tpVal == paths[1].TopologyValue {
i = 1
}
if i >= 0 {
// `tpVal` exists
paths[i].matchNum = num
if paths[0].matchNum > paths[1].matchNum {
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 {
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 {
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
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 {
return
}
if !nodeLabelsMatchSpreadConstraints(node.Labels, s.constraints) {
if !nodeLabelsMatchSpreadConstraints(node.Labels, s.Constraints) {
return
}
podLabelSet := labels.Set(updatedPod.Labels)
for _, constraint := range s.constraints {
if !constraint.selector.Matches(podLabelSet) {
for _, constraint := range s.Constraints {
if !constraint.Selector.Matches(podLabelSet) {
continue
}
k, v := constraint.topologyKey, node.Labels[constraint.topologyKey]
k, v := constraint.TopologyKey, node.Labels[constraint.TopologyKey]
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.
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))
s, err := pl.calPreFilterState(pod)
if err != nil {
return framework.NewStatus(framework.Error, err.Error())
}
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
}
@ -206,9 +196,13 @@ func getPreFilterState(cycleState *framework.CycleState) (*preFilterState, error
}
// 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
// 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)
if err != nil {
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)".
// 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),
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
s.TpPairToMatchNum[pair] += num
lock.Unlock()
}
@ -245,7 +239,7 @@ func calPreFilterState(pod *v1.Pod, allNodes []*schedulernodeinfo.NodeInfo) (*pr
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) {
return
}
@ -257,11 +251,11 @@ func calPreFilterState(pod *v1.Pod, allNodes []*schedulernodeinfo.NodeInfo) (*pr
if existingPod.DeletionTimestamp != nil || existingPod.Namespace != pod.Namespace {
continue
}
if constraint.selector.Matches(labels.Set(existingPod.Labels)) {
if constraint.Selector.Matches(labels.Set(existingPod.Labels)) {
matchTotal++
}
}
pair := topologyPair{key: constraint.topologyKey, value: node.Labels[constraint.topologyKey]}
pair := topologyPair{key: constraint.TopologyKey, value: node.Labels[constraint.TopologyKey]}
addTopologyPairMatchNum(pair, matchTotal)
}
}
@ -269,11 +263,11 @@ func calPreFilterState(pod *v1.Pod, allNodes []*schedulernodeinfo.NodeInfo) (*pr
// calculate min match for each topology pair
for i := 0; i < len(constraints); i++ {
key := constraints[i].topologyKey
s.tpKeyToCriticalPaths[key] = newCriticalPaths()
key := constraints[i].TopologyKey
s.TpKeyToCriticalPaths[key] = newCriticalPaths()
}
for pair, num := range s.tpPairToMatchNum {
s.tpKeyToCriticalPaths[pair.key].update(pair.value, num)
for pair, num := range s.TpPairToMatchNum {
s.TpKeyToCriticalPaths[pair.key].update(pair.value, num)
}
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.
if len(s.tpPairToMatchNum) == 0 || len(s.constraints) == 0 {
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]
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 framework.NewStatus(framework.Unschedulable, ErrReasonConstraintsNotMatch)
}
selfMatchNum := int32(0)
if c.selector.Matches(podLabelSet) {
if c.Selector.Matches(podLabelSet) {
selfMatchNum = 1
}
pair := topologyPair{key: tpKey, value: tpVal}
paths, ok := s.tpKeyToCriticalPaths[tpKey]
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)
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]
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)
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 framework.NewStatus(framework.Unschedulable, ErrReasonConstraintsNotMatch)
}
}

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

@ -21,6 +21,7 @@ import (
"reflect"
"testing"
"github.com/google/go-cmp/cmp"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
@ -35,7 +36,24 @@ var (
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()
barSelector := st.MakeLabelSelector().Exists("bar").Obj()
tests := []struct {
@ -57,17 +75,17 @@ func TestCalPreFilterState(t *testing.T) {
st.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(),
},
want: &preFilterState{
constraints: []topologySpreadConstraint{
Constraints: []topologySpreadConstraint{
{
maxSkew: 5,
topologyKey: "zone",
selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Label("foo", "bar").Obj()),
MaxSkew: 5,
TopologyKey: "zone",
Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Label("foo", "bar").Obj()),
},
},
tpKeyToCriticalPaths: map[string]*criticalPaths{
TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 0}, {"zone2", 0}},
},
tpPairToMatchNum: map[topologyPair]int32{
TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 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(),
},
want: &preFilterState{
constraints: []topologySpreadConstraint{
Constraints: []topologySpreadConstraint{
{
maxSkew: 1,
topologyKey: "zone",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
MaxSkew: 1,
TopologyKey: "zone",
Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
},
},
tpKeyToCriticalPaths: map[string]*criticalPaths{
TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone2", 2}, {"zone1", 3}},
},
tpPairToMatchNum: map[topologyPair]int32{
TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 3,
{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(),
},
want: &preFilterState{
constraints: []topologySpreadConstraint{
Constraints: []topologySpreadConstraint{
{
maxSkew: 1,
topologyKey: "zone",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
MaxSkew: 1,
TopologyKey: "zone",
Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
},
},
tpKeyToCriticalPaths: map[string]*criticalPaths{
TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone3", 0}, {"zone2", 2}},
},
tpPairToMatchNum: map[topologyPair]int32{
TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 3,
{key: "zone", value: "zone2"}: 2,
{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(),
},
want: &preFilterState{
constraints: []topologySpreadConstraint{
Constraints: []topologySpreadConstraint{
{
maxSkew: 1,
topologyKey: "zone",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
MaxSkew: 1,
TopologyKey: "zone",
Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
},
},
tpKeyToCriticalPaths: map[string]*criticalPaths{
TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone2", 1}, {"zone1", 2}},
},
tpPairToMatchNum: map[topologyPair]int32{
TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 2,
{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(),
},
want: &preFilterState{
constraints: []topologySpreadConstraint{
Constraints: []topologySpreadConstraint{
{
maxSkew: 1,
topologyKey: "zone",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
MaxSkew: 1,
TopologyKey: "zone",
Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
},
{
maxSkew: 1,
topologyKey: "node",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
MaxSkew: 1,
TopologyKey: "node",
Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
},
},
tpKeyToCriticalPaths: map[string]*criticalPaths{
TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 3}, {"zone2", 4}},
"node": {{"node-x", 0}, {"node-b", 1}},
},
tpPairToMatchNum: map[topologyPair]int32{
TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 3,
{key: "zone", value: "zone2"}: 4,
{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(),
},
want: &preFilterState{
constraints: []topologySpreadConstraint{
Constraints: []topologySpreadConstraint{
{
maxSkew: 1,
topologyKey: "zone",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
MaxSkew: 1,
TopologyKey: "zone",
Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
},
{
maxSkew: 1,
topologyKey: "node",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
MaxSkew: 1,
TopologyKey: "node",
Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
},
},
tpKeyToCriticalPaths: map[string]*criticalPaths{
TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 3}, {"zone2", 4}},
"node": {{"node-b", 1}, {"node-a", 2}},
},
tpPairToMatchNum: map[topologyPair]int32{
TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 3,
{key: "zone", value: "zone2"}: 4,
{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(),
},
want: &preFilterState{
constraints: []topologySpreadConstraint{
Constraints: []topologySpreadConstraint{
{
maxSkew: 1,
topologyKey: "zone",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
MaxSkew: 1,
TopologyKey: "zone",
Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
},
{
maxSkew: 1,
topologyKey: "node",
selector: mustConvertLabelSelectorAsSelector(t, barSelector),
MaxSkew: 1,
TopologyKey: "node",
Selector: mustConvertLabelSelectorAsSelector(t, barSelector),
},
},
tpKeyToCriticalPaths: map[string]*criticalPaths{
TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone2", 0}, {"zone1", 1}},
"node": {{"node-a", 0}, {"node-y", 0}},
},
tpPairToMatchNum: map[topologyPair]int32{
TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 1,
{key: "zone", value: "zone2"}: 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(),
},
want: &preFilterState{
constraints: []topologySpreadConstraint{
Constraints: []topologySpreadConstraint{
{
maxSkew: 1,
topologyKey: "zone",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
MaxSkew: 1,
TopologyKey: "zone",
Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
},
{
maxSkew: 1,
topologyKey: "node",
selector: mustConvertLabelSelectorAsSelector(t, barSelector),
MaxSkew: 1,
TopologyKey: "node",
Selector: mustConvertLabelSelectorAsSelector(t, barSelector),
},
},
tpKeyToCriticalPaths: map[string]*criticalPaths{
TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 3}, {"zone2", 4}},
"node": {{"node-b", 0}, {"node-a", 1}},
},
tpPairToMatchNum: map[topologyPair]int32{
TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 3,
{key: "zone", value: "zone2"}: 4,
{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(),
},
want: &preFilterState{
constraints: []topologySpreadConstraint{
Constraints: []topologySpreadConstraint{
{
maxSkew: 1,
topologyKey: "zone",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
MaxSkew: 1,
TopologyKey: "zone",
Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
},
{
maxSkew: 1,
topologyKey: "node",
selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
MaxSkew: 1,
TopologyKey: "node",
Selector: mustConvertLabelSelectorAsSelector(t, fooSelector),
},
},
tpKeyToCriticalPaths: map[string]*criticalPaths{
TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 3}, {"zone2", 4}},
"node": {{"node-b", 1}, {"node-a", 2}},
},
tpPairToMatchNum: map[topologyPair]int32{
TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 3,
{key: "zone", value: "zone2"}: 4,
{key: "node", value: "node-a"}: 2,
@ -415,12 +433,19 @@ func TestCalPreFilterState(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := cache.NewSnapshot(tt.existingPods, tt.nodes)
l, _ := s.NodeInfos().List()
got, _ := calPreFilterState(tt.pod, l)
got.sortCriticalPaths()
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("calPreFilterState() = %#v, want %#v", *got, *tt.want)
pl := PodTopologySpread{
sharedLister: cache.NewSnapshot(tt.existingPods, tt.nodes),
}
cs := framework.NewCycleState()
if s := pl.PreFilter(context.Background(), cs, tt.pod); !s.IsSuccess() {
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) {
nodeConstraint := topologySpreadConstraint{
maxSkew: 1,
topologyKey: "node",
selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("foo").Obj()),
MaxSkew: 1,
TopologyKey: "node",
Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("foo").Obj()),
}
zoneConstraint := nodeConstraint
zoneConstraint.topologyKey = "zone"
zoneConstraint.TopologyKey = "zone"
tests := []struct {
name string
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(),
},
want: &preFilterState{
constraints: []topologySpreadConstraint{nodeConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{
Constraints: []topologySpreadConstraint{nodeConstraint},
TpKeyToCriticalPaths: map[string]*criticalPaths{
"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-b"}: 0,
},
@ -481,11 +506,11 @@ func TestPreFilterStateAddPod(t *testing.T) {
st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(),
},
want: &preFilterState{
constraints: []topologySpreadConstraint{nodeConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{
Constraints: []topologySpreadConstraint{nodeConstraint},
TpKeyToCriticalPaths: map[string]*criticalPaths{
"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-b"}: 1,
},
@ -506,11 +531,11 @@ func TestPreFilterStateAddPod(t *testing.T) {
st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(),
},
want: &preFilterState{
constraints: []topologySpreadConstraint{nodeConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{
Constraints: []topologySpreadConstraint{nodeConstraint},
TpKeyToCriticalPaths: map[string]*criticalPaths{
"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-b"}: 1,
},
@ -531,11 +556,11 @@ func TestPreFilterStateAddPod(t *testing.T) {
st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(),
},
want: &preFilterState{
constraints: []topologySpreadConstraint{nodeConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{
Constraints: []topologySpreadConstraint{nodeConstraint},
TpKeyToCriticalPaths: map[string]*criticalPaths{
"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-b"}: 2,
},
@ -555,12 +580,12 @@ func TestPreFilterStateAddPod(t *testing.T) {
st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(),
},
want: &preFilterState{
constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{
Constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint},
TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone2", 0}, {"zone1", 1}},
"node": {{"node-x", 0}, {"node-a", 1}},
},
tpPairToMatchNum: map[topologyPair]int32{
TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 1,
{key: "zone", value: "zone2"}: 0,
{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(),
},
want: &preFilterState{
constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{
Constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint},
TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 1}, {"zone2", 1}},
"node": {{"node-a", 1}, {"node-x", 1}},
},
tpPairToMatchNum: map[topologyPair]int32{
TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 1,
{key: "zone", value: "zone2"}: 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(),
},
want: &preFilterState{
constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{
Constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint},
TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone2", 1}, {"zone1", 3}},
"node": {{"node-a", 1}, {"node-x", 1}},
},
tpPairToMatchNum: map[topologyPair]int32{
TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 3,
{key: "zone", value: "zone2"}: 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", "").
SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").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(),
},
want: &preFilterState{
constraints: []topologySpreadConstraint{
Constraints: []topologySpreadConstraint{
zoneConstraint,
{
maxSkew: 1,
topologyKey: "node",
selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("bar").Obj()),
MaxSkew: 1,
TopologyKey: "node",
Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("bar").Obj()),
},
},
tpKeyToCriticalPaths: map[string]*criticalPaths{
TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone2", 1}, {"zone1", 2}},
"node": {{"node-a", 0}, {"node-b", 1}},
},
tpPairToMatchNum: map[topologyPair]int32{
TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 2,
{key: "zone", value: "zone2"}: 1,
{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", "").
SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").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(),
},
want: &preFilterState{
constraints: []topologySpreadConstraint{
Constraints: []topologySpreadConstraint{
zoneConstraint,
{
maxSkew: 1,
topologyKey: "node",
selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("bar").Obj()),
MaxSkew: 1,
TopologyKey: "node",
Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("bar").Obj()),
},
},
tpKeyToCriticalPaths: map[string]*criticalPaths{
TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 1}, {"zone2", 1}},
"node": {{"node-a", 1}, {"node-b", 1}},
},
tpPairToMatchNum: map[topologyPair]int32{
TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 1,
{key: "zone", value: "zone2"}: 1,
{key: "node", value: "node-a"}: 1,
@ -713,13 +738,28 @@ func TestPreFilterStateAddPod(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := cache.NewSnapshot(tt.existingPods, tt.nodes)
l, _ := s.NodeInfos().List()
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)
snapshot := cache.NewSnapshot(tt.existingPods, tt.nodes)
pl := PodTopologySpread{
sharedLister: snapshot,
}
cs := framework.NewCycleState()
ctx := context.Background()
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) {
nodeConstraint := topologySpreadConstraint{
maxSkew: 1,
topologyKey: "node",
selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("foo").Obj()),
MaxSkew: 1,
TopologyKey: "node",
Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("foo").Obj()),
}
zoneConstraint := nodeConstraint
zoneConstraint.topologyKey = "zone"
zoneConstraint.TopologyKey = "zone"
tests := []struct {
name string
preemptor *v1.Pod // preemptor pod
@ -763,11 +803,11 @@ func TestPreFilterStateRemovePod(t *testing.T) {
deletedPodIdx: 0, // remove pod "p-a1"
nodeIdx: 0, // node-a
want: &preFilterState{
constraints: []topologySpreadConstraint{zoneConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{
Constraints: []topologySpreadConstraint{zoneConstraint},
TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 1}, {"zone2", 1}},
},
tpPairToMatchNum: map[topologyPair]int32{
TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 1,
{key: "zone", value: "zone2"}: 1,
},
@ -793,11 +833,11 @@ func TestPreFilterStateRemovePod(t *testing.T) {
deletedPodIdx: 0, // remove pod "p-a1"
nodeIdx: 0, // node-a
want: &preFilterState{
constraints: []topologySpreadConstraint{zoneConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{
Constraints: []topologySpreadConstraint{zoneConstraint},
TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 1}, {"zone2", 2}},
},
tpPairToMatchNum: map[topologyPair]int32{
TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 1,
{key: "zone", value: "zone2"}: 2,
},
@ -824,11 +864,11 @@ func TestPreFilterStateRemovePod(t *testing.T) {
deletedPodIdx: 0, // remove pod "p-a0"
nodeIdx: 0, // node-a
want: &preFilterState{
constraints: []topologySpreadConstraint{zoneConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{
Constraints: []topologySpreadConstraint{zoneConstraint},
TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 2}, {"zone2", 2}},
},
tpPairToMatchNum: map[topologyPair]int32{
TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 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(),
nodeIdx: 0, // node-a
want: &preFilterState{
constraints: []topologySpreadConstraint{zoneConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{
Constraints: []topologySpreadConstraint{zoneConstraint},
TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone1", 2}, {"zone2", 2}},
},
tpPairToMatchNum: map[topologyPair]int32{
TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 2,
{key: "zone", value: "zone2"}: 2,
},
@ -886,12 +926,12 @@ func TestPreFilterStateRemovePod(t *testing.T) {
deletedPodIdx: 3, // remove pod "p-x1"
nodeIdx: 2, // node-x
want: &preFilterState{
constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint},
tpKeyToCriticalPaths: map[string]*criticalPaths{
Constraints: []topologySpreadConstraint{zoneConstraint, nodeConstraint},
TpKeyToCriticalPaths: map[string]*criticalPaths{
"zone": {{"zone2", 1}, {"zone1", 3}},
"node": {{"node-b", 1}, {"node-x", 1}},
},
tpPairToMatchNum: map[topologyPair]int32{
TpPairToMatchNum: map[topologyPair]int32{
{key: "zone", value: "zone1"}: 3,
{key: "zone", value: "zone2"}: 1,
{key: "node", value: "node-a"}: 2,
@ -903,20 +943,36 @@ func TestPreFilterStateRemovePod(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := cache.NewSnapshot(tt.existingPods, tt.nodes)
l, _ := s.NodeInfos().List()
state, _ := calPreFilterState(tt.preemptor, l)
snapshot := cache.NewSnapshot(tt.existingPods, tt.nodes)
pl := PodTopologySpread{
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 {
deletedPod = tt.existingPods[tt.deletedPodIdx]
} else {
deletedPod = tt.deletedPod
}
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)
nodeInfo, err := snapshot.Get(tt.nodes[tt.nodeIdx].Name)
if err != nil {
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,
},
{
name: "1000nodes/two-constraints-zone-node",
name: "1000nodes/two-Constraints-zone-node",
pod: st.MakePod().Name("p").Label("foo", "").Label("bar", "").
SpreadConstraint(1, v1.LabelZoneFailureDomain, hardSpread, st.MakeLabelSelector().Exists("foo").Obj()).
SpreadConstraint(1, v1.LabelHostname, hardSpread, st.MakeLabelSelector().Exists("bar").Obj()).
@ -962,27 +1018,20 @@ func BenchmarkTestCalPreFilterState(b *testing.B) {
for _, tt := range tests {
b.Run(tt.name, func(b *testing.B) {
existingPods, allNodes, _ := st.MakeNodesAndPodsForEvenPodsSpread(tt.pod.Labels, tt.existingPodsNum, tt.allNodesNum, tt.filteredNodesNum)
s := cache.NewSnapshot(existingPods, allNodes)
l, _ := s.NodeInfos().List()
pl := PodTopologySpread{
sharedLister: cache.NewSnapshot(existingPods, allNodes),
}
b.ResetTimer()
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 {
t.Helper()
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)
// 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]",
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()).
@ -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)
// 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]",
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()).
@ -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)
// 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]",
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()).
@ -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)
// 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]",
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()).
@ -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)
// 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]",
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()).
@ -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
// 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",
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()).

67
pkg/scheduler/framework/plugins/podtopologyspread/scoring.go
View File

@ -34,12 +34,13 @@ import (
const preScoreStateKey = "PreScore" + Name
// preScoreState computed at PreScore and used at Score.
// Fields are exported for comparison during testing.
type preScoreState struct {
constraints []topologySpreadConstraint
// nodeNameSet is a string set holding all node names which have all constraints[*].topologyKey present.
nodeNameSet sets.String
// topologyPairToPodCounts is keyed with topologyPair, and valued with the number of matching pods.
topologyPairToPodCounts map[topologyPair]*int64
Constraints []topologySpreadConstraint
// NodeNameSet is a string set holding all node names which have all Constraints[*].topologyKey present.
NodeNameSet sets.String
// TopologyPairToPodCounts is keyed with topologyPair, and valued with the number of matching pods.
TopologyPairToPodCounts map[topologyPair]*int64
}
// Clone implements the mandatory Clone interface. We don't really copy the data since
@ -50,8 +51,8 @@ func (s *preScoreState) Clone() framework.StateData {
// initialize iterates "filteredNodes" to filter out the nodes which don't have required topologyKey(s),
// and initialize two maps:
// 1) s.topologyPairToPodCounts: keyed with both eligible topology pair and node names.
// 2) s.nodeNameSet: keyed with node name, and valued with a *int64 pointer for eligible node only.
// 1) s.TopologyPairToPodCounts: keyed with both eligible topology pair and node names.
// 2) s.NodeNameSet: keyed with node name, and valued with a *int64 pointer for eligible node only.
func (s *preScoreState) initialize(pod *v1.Pod, filteredNodes []*v1.Node) error {
constraints, err := filterTopologySpreadConstraints(pod.Spec.TopologySpreadConstraints, v1.ScheduleAnyway)
if err != nil {
@ -60,20 +61,20 @@ func (s *preScoreState) initialize(pod *v1.Pod, filteredNodes []*v1.Node) error
if constraints == nil {
return nil
}
s.constraints = constraints
s.Constraints = constraints
for _, node := range filteredNodes {
if !nodeLabelsMatchSpreadConstraints(node.Labels, s.constraints) {
if !nodeLabelsMatchSpreadConstraints(node.Labels, s.Constraints) {
continue
}
for _, constraint := range s.constraints {
pair := topologyPair{key: constraint.topologyKey, value: node.Labels[constraint.topologyKey]}
if s.topologyPairToPodCounts[pair] == nil {
s.topologyPairToPodCounts[pair] = new(int64)
for _, constraint := range s.Constraints {
pair := topologyPair{key: constraint.TopologyKey, value: node.Labels[constraint.TopologyKey]}
if s.TopologyPairToPodCounts[pair] == nil {
s.TopologyPairToPodCounts[pair] = new(int64)
}
}
s.nodeNameSet.Insert(node.Name)
s.NodeNameSet.Insert(node.Name)
// For those nodes which don't have all required topologyKeys present, it's intentional to leave
// their entries absent in nodeNameSet, so that we're able to score them to 0 afterwards.
// their entries absent in NodeNameSet, so that we're able to score them to 0 afterwards.
}
return nil
}
@ -96,16 +97,16 @@ func (pl *PodTopologySpread) PreScore(
}
state := &preScoreState{
nodeNameSet: sets.String{},
topologyPairToPodCounts: make(map[topologyPair]*int64),
NodeNameSet: sets.String{},
TopologyPairToPodCounts: make(map[topologyPair]*int64),
}
err = state.initialize(pod, filteredNodes)
if err != nil {
return framework.NewStatus(framework.Error, fmt.Sprintf("error when calculating preScoreState: %v", err))
}
// return if incoming pod doesn't have soft topology spread constraints.
if state.constraints == nil {
// return if incoming pod doesn't have soft topology spread Constraints.
if state.Constraints == nil {
cycleState.Write(preScoreStateKey, state)
return nil
}
@ -119,15 +120,15 @@ func (pl *PodTopologySpread) PreScore(
// (1) `node` should satisfy incoming pod's NodeSelector/NodeAffinity
// (2) All topologyKeys need to be present in `node`
if !pluginhelper.PodMatchesNodeSelectorAndAffinityTerms(pod, node) ||
!nodeLabelsMatchSpreadConstraints(node.Labels, state.constraints) {
!nodeLabelsMatchSpreadConstraints(node.Labels, state.Constraints) {
return
}
for _, c := range state.constraints {
pair := topologyPair{key: c.topologyKey, value: node.Labels[c.topologyKey]}
for _, c := range state.Constraints {
pair := topologyPair{key: c.TopologyKey, value: node.Labels[c.TopologyKey]}
// If current topology pair is not associated with any candidate node,
// continue to avoid unnecessary calculation.
if state.topologyPairToPodCounts[pair] == nil {
if state.TopologyPairToPodCounts[pair] == nil {
continue
}
@ -138,11 +139,11 @@ func (pl *PodTopologySpread) PreScore(
if existingPod.DeletionTimestamp != nil || existingPod.Namespace != pod.Namespace {
continue
}
if c.selector.Matches(labels.Set(existingPod.Labels)) {
if c.Selector.Matches(labels.Set(existingPod.Labels)) {
matchSum++
}
}
atomic.AddInt64(state.topologyPairToPodCounts[pair], matchSum)
atomic.AddInt64(state.TopologyPairToPodCounts[pair], matchSum)
}
}
workqueue.ParallelizeUntil(ctx, 16, len(allNodes), processAllNode)
@ -167,17 +168,17 @@ func (pl *PodTopologySpread) Score(ctx context.Context, cycleState *framework.Cy
}
// Return if the node is not qualified.
if _, ok := s.nodeNameSet[node.Name]; !ok {
if _, ok := s.NodeNameSet[node.Name]; !ok {
return 0, nil
}
// For each present <pair>, current node gets a credit of <matchSum>.
// And we sum up <matchSum> and return it as this node's score.
var score int64
for _, c := range s.constraints {
if tpVal, ok := node.Labels[c.topologyKey]; ok {
pair := topologyPair{key: c.topologyKey, value: tpVal}
matchSum := *s.topologyPairToPodCounts[pair]
for _, c := range s.Constraints {
if tpVal, ok := node.Labels[c.TopologyKey]; ok {
pair := topologyPair{key: c.TopologyKey, value: tpVal}
matchSum := *s.TopologyPairToPodCounts[pair]
score += matchSum
}
}
@ -198,8 +199,8 @@ func (pl *PodTopologySpread) NormalizeScore(ctx context.Context, cycleState *fra
var minScore int64 = math.MaxInt64
var total int64
for _, score := range scores {
// it's mandatory to check if <score.Name> is present in m.nodeNameSet
if _, ok := s.nodeNameSet[score.Name]; !ok {
// it's mandatory to check if <score.Name> is present in m.NodeNameSet
if _, ok := s.NodeNameSet[score.Name]; !ok {
continue
}
total += score.Score
@ -228,7 +229,7 @@ func (pl *PodTopologySpread) NormalizeScore(ctx context.Context, cycleState *fra
continue
}
if _, ok := s.nodeNameSet[node.Name]; !ok {
if _, ok := s.NodeNameSet[node.Name]; !ok {
scores[i].Score = 0
continue
}

111
pkg/scheduler/framework/plugins/podtopologyspread/scoring_test.go
View File

@ -21,20 +21,31 @@ import (
"reflect"
"testing"
"github.com/google/go-cmp/cmp"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/util/sets"
framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1"
"k8s.io/kubernetes/pkg/scheduler/internal/cache"
st "k8s.io/kubernetes/pkg/scheduler/testing"
"k8s.io/utils/pointer"
)
func TestPreScoreStateInitialize(t *testing.T) {
func (s preScoreState) Equal(o preScoreState) bool {
type internal preScoreState
return cmp.Equal(internal(s), internal(o),
cmp.Comparer(func(s1 labels.Selector, s2 labels.Selector) bool {
return reflect.DeepEqual(s1, s2)
}),
)
}
func TestPreScoreStateEmptyNodes(t *testing.T) {
tests := []struct {
name string
pod *v1.Pod
nodes []*v1.Node
wantNodeNameSet sets.String
wantTopologyPairMap map[topologyPair]*int64
name string
pod *v1.Pod
nodes []*v1.Node
want *preScoreState
}{
{
name: "normal case",
@ -47,13 +58,27 @@ func TestPreScoreStateInitialize(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(),
},
wantNodeNameSet: sets.NewString("node-a", "node-b", "node-x"),
wantTopologyPairMap: map[topologyPair]*int64{
{key: "zone", value: "zone1"}: new(int64),
{key: "zone", value: "zone2"}: new(int64),
{key: "node", value: "node-a"}: new(int64),
{key: "node", value: "node-b"}: new(int64),
{key: "node", value: "node-x"}: new(int64),
want: &preScoreState{
Constraints: []topologySpreadConstraint{
{
MaxSkew: 1,
TopologyKey: "zone",
Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("foo").Obj()),
},
{
MaxSkew: 1,
TopologyKey: "node",
Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("foo").Obj()),
},
},
NodeNameSet: sets.NewString("node-a", "node-b", "node-x"),
TopologyPairToPodCounts: map[topologyPair]*int64{
{key: "zone", value: "zone1"}: pointer.Int64Ptr(0),
{key: "zone", value: "zone2"}: pointer.Int64Ptr(0),
{key: "node", value: "node-a"}: pointer.Int64Ptr(0),
{key: "node", value: "node-b"}: pointer.Int64Ptr(0),
{key: "node", value: "node-x"}: pointer.Int64Ptr(0),
},
},
},
{
@ -67,28 +92,44 @@ func TestPreScoreStateInitialize(t *testing.T) {
st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(),
st.MakeNode().Name("node-x").Label("node", "node-x").Obj(),
},
wantNodeNameSet: sets.NewString("node-a", "node-b"),
wantTopologyPairMap: map[topologyPair]*int64{
{key: "zone", value: "zone1"}: new(int64),
{key: "node", value: "node-a"}: new(int64),
{key: "node", value: "node-b"}: new(int64),
want: &preScoreState{
Constraints: []topologySpreadConstraint{
{
MaxSkew: 1,
TopologyKey: "zone",
Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("foo").Obj()),
},
{
MaxSkew: 1,
TopologyKey: "node",
Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("bar").Obj()),
},
},
NodeNameSet: sets.NewString("node-a", "node-b"),
TopologyPairToPodCounts: map[topologyPair]*int64{
{key: "zone", value: "zone1"}: pointer.Int64Ptr(0),
{key: "node", value: "node-a"}: pointer.Int64Ptr(0),
{key: "node", value: "node-b"}: pointer.Int64Ptr(0),
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := &preScoreState{
nodeNameSet: sets.String{},
topologyPairToPodCounts: make(map[topologyPair]*int64),
pl := PodTopologySpread{
sharedLister: cache.NewSnapshot(nil, tt.nodes),
}
if err := s.initialize(tt.pod, tt.nodes); err != nil {
cs := framework.NewCycleState()
if s := pl.PreScore(context.Background(), cs, tt.pod, tt.nodes); !s.IsSuccess() {
t.Fatal(s.AsError())
}
got, err := getPreScoreState(cs)
if err != nil {
t.Fatal(err)
}
if !reflect.DeepEqual(s.nodeNameSet, tt.wantNodeNameSet) {
t.Errorf("initilize().nodeNameSet = %#v, want %#v", s.nodeNameSet, tt.wantNodeNameSet)
}
if !reflect.DeepEqual(s.topologyPairToPodCounts, tt.wantTopologyPairMap) {
t.Errorf("initilize().topologyPairToPodCounts = %#v, want %#v", s.topologyPairToPodCounts, tt.wantTopologyPairMap)
if diff := cmp.Diff(tt.want, got); diff != "" {
t.Errorf("PodTopologySpread#PreScore() returned (-want, +got):\n%s", diff)
}
})
}
@ -300,7 +341,7 @@ func TestPodTopologySpreadScore(t *testing.T) {
// matching pods spread as 2/~1~/2/~4~, total = 2+3 + 2+6 = 13 (zone and node should be both summed up)
// after reversing, it's 8/5
// so scores = 800/8, 500/8
name: "two constraints on zone and node, 2 out of 4 nodes are candidates",
name: "two Constraints on zone and node, 2 out of 4 nodes are candidates",
pod: st.MakePod().Name("p").Label("foo", "").
SpreadConstraint(1, "zone", softSpread, st.MakeLabelSelector().Exists("foo").Obj()).
SpreadConstraint(1, "node", softSpread, st.MakeLabelSelector().Exists("foo").Obj()).
@ -330,7 +371,7 @@ func TestPodTopologySpreadScore(t *testing.T) {
},
},
{
// If constraints hold different labelSelectors, it's a little complex.
// If Constraints hold different labelSelectors, it's a little complex.
// +----------------------+------------------------+
// | zone1 | zone2 |
// +----------------------+------------------------+
@ -343,7 +384,7 @@ func TestPodTopologySpreadScore(t *testing.T) {
// sum them up gets: 2/3/1/2, and total number is 8.
// after reversing, it's 6/5/7/6
// so scores = 600/7, 500/7, 700/7, 600/7
name: "two constraints on zone and node, with different labelSelectors",
name: "two Constraints on zone and node, with different labelSelectors",
pod: st.MakePod().Name("p").Label("foo", "").Label("bar", "").
SpreadConstraint(1, "zone", softSpread, st.MakeLabelSelector().Exists("foo").Obj()).
SpreadConstraint(1, "node", softSpread, st.MakeLabelSelector().Exists("bar").Obj()).
@ -374,7 +415,7 @@ func TestPodTopologySpreadScore(t *testing.T) {
// sum them up gets: 0/1/2/3, and total number is 6.
// after reversing, it's 6/5/4/3.
// so scores = 600/6, 500/6, 400/6, 300/6
name: "two constraints on zone and node, with different labelSelectors, some nodes have 0 pods",
name: "two Constraints on zone and node, with different labelSelectors, some nodes have 0 pods",
pod: st.MakePod().Name("p").Label("foo", "").Label("bar", "").
SpreadConstraint(1, "zone", softSpread, st.MakeLabelSelector().Exists("foo").Obj()).
SpreadConstraint(1, "node", softSpread, st.MakeLabelSelector().Exists("bar").Obj()).
@ -404,7 +445,7 @@ func TestPodTopologySpreadScore(t *testing.T) {
// sum them up gets: 2/3/1, and total number is 6.
// after reversing, it's 4/3/5
// so scores = 400/5, 300/5, 500/5
name: "two constraints on zone and node, with different labelSelectors, 3 out of 4 nodes are candidates",
name: "two Constraints on zone and node, with different labelSelectors, 3 out of 4 nodes are candidates",
pod: st.MakePod().Name("p").Label("foo", "").Label("bar", "").
SpreadConstraint(1, "zone", softSpread, st.MakeLabelSelector().Exists("foo").Obj()).
SpreadConstraint(1, "node", softSpread, st.MakeLabelSelector().Exists("bar").Obj()).
@ -495,8 +536,8 @@ func TestPodTopologySpreadScore(t *testing.T) {
if !status.IsSuccess() {
t.Errorf("unexpected error: %v", status)
}
if !reflect.DeepEqual(tt.want, gotList) {
t.Errorf("expected:\n\t%+v,\ngot:\n\t%+v", tt.want, gotList)
if diff := cmp.Diff(tt.want, gotList); diff != "" {
t.Errorf("unexpected scores (-want,+got):\n%s", diff)
}
})
}
@ -529,7 +570,7 @@ func BenchmarkTestPodTopologySpreadScore(b *testing.B) {
filteredNodesNum: 500,
},
{
name: "1000nodes/two-constraints-zone-node",
name: "1000nodes/two-Constraints-zone-node",
pod: st.MakePod().Name("p").Label("foo", "").Label("bar", "").
SpreadConstraint(1, v1.LabelZoneFailureDomain, softSpread, st.MakeLabelSelector().Exists("foo").Obj()).
SpreadConstraint(1, v1.LabelHostname, softSpread, st.MakeLabelSelector().Exists("bar").Obj()).