diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go index bfd8d8c034a..f765581fbb1 100644 --- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go +++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go @@ -958,7 +958,7 @@ func TestNodesWherePreemptionMightHelp(t *testing.T) { expected: map[string]bool{"node4": true}, }, { - name: "ErrTopologySpreadConstraintsNotMatch should be tried as it indicates that the pod is unschedulable due to topology spread constraints", + name: "ErrReasonConstraintsNotMatch should be tried as it indicates that the pod is unschedulable due to topology spread constraints", nodesStatuses: framework.NodeToStatusMap{ "node1": framework.NewStatus(framework.Unschedulable, podtopologyspread.ErrReasonConstraintsNotMatch), "node2": framework.NewStatus(framework.UnschedulableAndUnresolvable, nodename.ErrReason), @@ -975,6 +975,15 @@ func TestNodesWherePreemptionMightHelp(t *testing.T) { }, expected: map[string]bool{"node1": true, "node3": true}, }, + { + name: "ErrReasonNodeLabelNotMatch should not be tried as it indicates that the pod is unschedulable due to node doesn't have the required label", + nodesStatuses: framework.NodeToStatusMap{ + "node2": framework.NewStatus(framework.UnschedulableAndUnresolvable, podtopologyspread.ErrReasonNodeLabelNotMatch), + "node3": framework.NewStatus(framework.Unschedulable, ""), + "node4": framework.NewStatus(framework.UnschedulableAndUnresolvable, ""), + }, + expected: map[string]bool{"node1": true, "node3": true}, + }, } for _, tt := range tests { diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/filtering.go b/pkg/scheduler/framework/plugins/podtopologyspread/filtering.go index 98705c5b380..c6aa6414970 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/filtering.go +++ b/pkg/scheduler/framework/plugins/podtopologyspread/filtering.go @@ -295,7 +295,7 @@ func (pl *PodTopologySpread) Filter(ctx context.Context, cycleState *framework.C 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) + return framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrReasonNodeLabelNotMatch) } selfMatchNum := int32(0) diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go b/pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go index dab540152f2..ea9d5bcfeaa 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go +++ b/pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go @@ -1145,11 +1145,11 @@ func mustConvertLabelSelectorAsSelector(t *testing.T, ls *metav1.LabelSelector) func TestSingleConstraint(t *testing.T) { tests := []struct { - name string - pod *v1.Pod - nodes []*v1.Node - existingPods []*v1.Pod - fits map[string]bool + name string + pod *v1.Pod + nodes []*v1.Node + existingPods []*v1.Pod + wantStatusCode map[string]framework.Code }{ { name: "no existing pods", @@ -1162,11 +1162,11 @@ func TestSingleConstraint(t *testing.T) { st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(), st.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(), }, - fits: map[string]bool{ - "node-a": true, - "node-b": true, - "node-x": true, - "node-y": true, + wantStatusCode: map[string]framework.Code{ + "node-a": framework.Success, + "node-b": framework.Success, + "node-x": framework.Success, + "node-y": framework.Success, }, }, { @@ -1180,11 +1180,11 @@ func TestSingleConstraint(t *testing.T) { st.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(), st.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(), }, - fits: map[string]bool{ - "node-a": true, - "node-b": true, - "node-x": true, - "node-y": true, + wantStatusCode: map[string]framework.Code{ + "node-a": framework.Success, + "node-b": framework.Success, + "node-x": framework.Success, + "node-y": framework.Success, }, }, { @@ -1204,11 +1204,11 @@ func TestSingleConstraint(t *testing.T) { st.MakePod().Name("p-x1").Node("node-x").Label("foo", "").Obj(), st.MakePod().Name("p-y1").Node("node-y").Label("foo", "").Obj(), }, - fits: map[string]bool{ - "node-a": true, - "node-b": true, - "node-x": false, - "node-y": false, + wantStatusCode: map[string]framework.Code{ + "node-a": framework.Success, + "node-b": framework.Success, + "node-x": framework.Unschedulable, + "node-y": framework.Unschedulable, }, }, { @@ -1230,11 +1230,11 @@ func TestSingleConstraint(t *testing.T) { st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y3").Node("node-y").Label("foo", "").Obj(), }, - fits: map[string]bool{ - "node-a": true, - "node-b": true, - "node-x": true, - "node-y": true, + wantStatusCode: map[string]framework.Code{ + "node-a": framework.Success, + "node-b": framework.Success, + "node-x": framework.Success, + "node-y": framework.Success, }, }, { @@ -1256,11 +1256,11 @@ func TestSingleConstraint(t *testing.T) { st.MakePod().Name("p-x1").Node("node-x").Label("foo", "").Obj(), st.MakePod().Name("p-y1").Node("node-y").Label("foo", "").Obj(), }, - fits: map[string]bool{ - "node-a": true, - "node-b": false, - "node-x": false, - "node-y": false, + wantStatusCode: map[string]framework.Code{ + "node-a": framework.Success, + "node-b": framework.UnschedulableAndUnresolvable, + "node-x": framework.Unschedulable, + "node-y": framework.Unschedulable, }, }, { @@ -1282,11 +1282,11 @@ func TestSingleConstraint(t *testing.T) { st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y3").Node("node-y").Label("foo", "").Obj(), }, - fits: map[string]bool{ - "node-a": false, - "node-b": false, - "node-x": true, - "node-y": false, + wantStatusCode: map[string]framework.Code{ + "node-a": framework.Unschedulable, + "node-b": framework.Unschedulable, + "node-x": framework.Success, + "node-y": framework.Unschedulable, }, }, { @@ -1308,11 +1308,11 @@ func TestSingleConstraint(t *testing.T) { st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y3").Node("node-y").Label("foo", "").Obj(), }, - fits: map[string]bool{ - "node-a": false, - "node-b": true, - "node-x": true, - "node-y": false, + wantStatusCode: map[string]framework.Code{ + "node-a": framework.Unschedulable, + "node-b": framework.Success, + "node-x": framework.Success, + "node-y": framework.Unschedulable, }, }, { @@ -1338,11 +1338,11 @@ func TestSingleConstraint(t *testing.T) { st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y3").Node("node-y").Label("foo", "").Obj(), }, - fits: map[string]bool{ - "node-a": false, - "node-b": true, - "node-x": true, - "node-y": false, + wantStatusCode: map[string]framework.Code{ + "node-a": framework.Unschedulable, + "node-b": framework.Success, + "node-x": framework.Success, + "node-y": framework.Unschedulable, }, }, { @@ -1370,11 +1370,11 @@ func TestSingleConstraint(t *testing.T) { st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y3").Node("node-y").Label("foo", "").Obj(), }, - fits: map[string]bool{ - "node-a": true, - "node-b": true, // in real case, it's false - "node-x": true, // in real case, it's false - "node-y": false, + wantStatusCode: map[string]framework.Code{ + "node-a": framework.Success, + "node-b": framework.Success, // in real case, it's false + "node-x": framework.Success, // in real case, it's false + "node-y": framework.Unschedulable, }, }, { @@ -1390,9 +1390,9 @@ func TestSingleConstraint(t *testing.T) { st.MakePod().Name("p-a").Node("node-a").Label("foo", "").Terminating().Obj(), st.MakePod().Name("p-b").Node("node-b").Label("foo", "").Obj(), }, - fits: map[string]bool{ - "node-a": true, - "node-b": false, + wantStatusCode: map[string]framework.Code{ + "node-a": framework.Success, + "node-b": framework.Unschedulable, }, }, } @@ -1409,8 +1409,8 @@ func TestSingleConstraint(t *testing.T) { for _, node := range tt.nodes { nodeInfo, _ := snapshot.NodeInfos().Get(node.Name) status := p.Filter(context.Background(), state, tt.pod, nodeInfo) - if status.IsSuccess() != tt.fits[node.Name] { - t.Errorf("[%s]: expected %v got %v", node.Name, tt.fits[node.Name], status.IsSuccess()) + if len(tt.wantStatusCode) != 0 && status.Code() != tt.wantStatusCode[node.Name] { + t.Errorf("[%s]: expected status code %v got %v", node.Name, tt.wantStatusCode[node.Name], status.Code()) } } }) @@ -1419,11 +1419,11 @@ func TestSingleConstraint(t *testing.T) { func TestMultipleConstraints(t *testing.T) { tests := []struct { - name string - pod *v1.Pod - nodes []*v1.Node - existingPods []*v1.Pod - fits map[string]bool + name string + pod *v1.Pod + nodes []*v1.Node + existingPods []*v1.Pod + wantStatusCode map[string]framework.Code }{ { // 1. to fulfil "zone" constraint, incoming pod can be placed on any zone (hence any node) @@ -1448,11 +1448,11 @@ func TestMultipleConstraints(t *testing.T) { st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y3").Node("node-y").Label("foo", "").Obj(), }, - fits: map[string]bool{ - "node-a": false, - "node-b": false, - "node-x": true, - "node-y": false, + wantStatusCode: map[string]framework.Code{ + "node-a": framework.Unschedulable, + "node-b": framework.Unschedulable, + "node-x": framework.Success, + "node-y": framework.Unschedulable, }, }, { @@ -1479,11 +1479,11 @@ func TestMultipleConstraints(t *testing.T) { st.MakePod().Name("p-y3").Node("node-y").Label("foo", "").Obj(), st.MakePod().Name("p-y4").Node("node-y").Label("foo", "").Obj(), }, - fits: map[string]bool{ - "node-a": false, - "node-b": false, - "node-x": false, - "node-y": false, + wantStatusCode: map[string]framework.Code{ + "node-a": framework.Unschedulable, + "node-b": framework.Unschedulable, + "node-x": framework.Unschedulable, + "node-y": framework.Unschedulable, }, }, { @@ -1505,11 +1505,11 @@ func TestMultipleConstraints(t *testing.T) { st.MakePod().Name("p-a1").Node("node-a").Label("foo", "").Obj(), st.MakePod().Name("p-y1").Node("node-y").Label("bar", "").Obj(), }, - fits: map[string]bool{ - "node-a": false, - "node-b": false, - "node-x": true, - "node-y": false, + wantStatusCode: map[string]framework.Code{ + "node-a": framework.Unschedulable, + "node-b": framework.Unschedulable, + "node-x": framework.Success, + "node-y": framework.Unschedulable, }, }, { @@ -1532,11 +1532,11 @@ func TestMultipleConstraints(t *testing.T) { st.MakePod().Name("p-x1").Node("node-x").Label("bar", "").Obj(), st.MakePod().Name("p-y1").Node("node-y").Label("bar", "").Obj(), }, - fits: map[string]bool{ - "node-a": false, - "node-b": false, - "node-x": false, - "node-y": false, + wantStatusCode: map[string]framework.Code{ + "node-a": framework.Unschedulable, + "node-b": framework.Unschedulable, + "node-x": framework.Unschedulable, + "node-y": framework.Unschedulable, }, }, { @@ -1561,11 +1561,11 @@ func TestMultipleConstraints(t *testing.T) { st.MakePod().Name("p-y2").Node("node-y").Label("foo", "").Label("bar", "").Obj(), st.MakePod().Name("p-y3").Node("node-y").Label("foo", "").Obj(), }, - fits: map[string]bool{ - "node-a": false, - "node-b": true, - "node-x": false, - "node-y": false, + wantStatusCode: map[string]framework.Code{ + "node-a": framework.Unschedulable, + "node-b": framework.Success, + "node-x": framework.Unschedulable, + "node-y": framework.Unschedulable, }, }, { @@ -1588,11 +1588,37 @@ func TestMultipleConstraints(t *testing.T) { st.MakePod().Name("p-x1").Node("node-x").Label("bar", "").Obj(), st.MakePod().Name("p-y1").Node("node-y").Label("bar", "").Obj(), }, - fits: map[string]bool{ - "node-a": true, - "node-b": true, - "node-x": false, - "node-y": false, + wantStatusCode: map[string]framework.Code{ + "node-a": framework.Success, + "node-b": framework.Success, + "node-x": framework.Unschedulable, + "node-y": framework.Unschedulable, + }, + }, + { + // 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-b (node-x doesn't have the required label) + // intersection of (1) and (2) returns node-b + name: "two Constraints on zone and node, absence of label 'node' on node-x, spreads = [1/1, 1/0/0/1]", + pod: st.MakePod().Name("p").Label("foo", ""). + SpreadConstraint(1, "zone", v1.DoNotSchedule, st.MakeLabelSelector().Exists("foo").Obj()). + SpreadConstraint(1, "node", v1.DoNotSchedule, st.MakeLabelSelector().Exists("foo").Obj()). + Obj(), + nodes: []*v1.Node{ + st.MakeNode().Name("node-a").Label("zone", "zone1").Label("node", "node-a").Obj(), + st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(), + st.MakeNode().Name("node-x").Label("zone", "zone2").Obj(), + st.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(), + }, + existingPods: []*v1.Pod{ + st.MakePod().Name("p-a1").Node("node-a").Label("foo", "").Obj(), + st.MakePod().Name("p-y3").Node("node-y").Label("foo", "").Obj(), + }, + wantStatusCode: map[string]framework.Code{ + "node-a": framework.Unschedulable, + "node-b": framework.Success, + "node-x": framework.UnschedulableAndUnresolvable, + "node-y": framework.Unschedulable, }, }, } @@ -1609,8 +1635,8 @@ func TestMultipleConstraints(t *testing.T) { for _, node := range tt.nodes { nodeInfo, _ := snapshot.NodeInfos().Get(node.Name) status := p.Filter(context.Background(), state, tt.pod, nodeInfo) - if status.IsSuccess() != tt.fits[node.Name] { - t.Errorf("[%s]: expected %v got %v", node.Name, tt.fits[node.Name], status.IsSuccess()) + if len(tt.wantStatusCode) != 0 && status.Code() != tt.wantStatusCode[node.Name] { + t.Errorf("[%s]: expected error code %v got %v", node.Name, tt.wantStatusCode[node.Name], status.Code()) } } }) diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go b/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go index 3d70276a03e..581eb98a774 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go +++ b/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go @@ -31,6 +31,8 @@ import ( const ( // ErrReasonConstraintsNotMatch is used for PodTopologySpread filter error. ErrReasonConstraintsNotMatch = "node(s) didn't match pod topology spread constraints" + // ErrReasonNodeLabelNotMatch is used when the node doesn't hold the required label. + ErrReasonNodeLabelNotMatch = ErrReasonConstraintsNotMatch + " (missing required label)" ) // PodTopologySpread is a plugin that ensures pod's topologySpreadConstraints is satisfied.