Test PodTopologySpread.PreScore instead of internal pre-processing.

Signed-off-by: Aldo Culquicondor <acondor@google.com>
This commit is contained in:
Aldo Culquicondor 2020-02-26 16:17:34 -05:00
parent 0e46803e77
commit fd5b298c50
3 changed files with 112 additions and 68 deletions

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",
],
)

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
}

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
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()).