From ed9c93a84dd06a6d336b7c30b464b187f5682b13 Mon Sep 17 00:00:00 2001 From: lowang-bh Date: Fri, 17 May 2024 22:50:37 +0800 Subject: [PATCH] add testcase about FitErrors Signed-off-by: lowang-bh --- pkg/scheduler/actions/allocate/allocate.go | 2 +- pkg/scheduler/actions/preempt/preempt.go | 2 +- pkg/scheduler/api/unschedule_info.go | 6 +- pkg/scheduler/api/unschedule_info_test.go | 117 +++++++++++++++++++++ 4 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 pkg/scheduler/api/unschedule_info_test.go diff --git a/pkg/scheduler/actions/allocate/allocate.go b/pkg/scheduler/actions/allocate/allocate.go index 171be7cfee1..e6f4965db41 100644 --- a/pkg/scheduler/actions/allocate/allocate.go +++ b/pkg/scheduler/actions/allocate/allocate.go @@ -301,7 +301,7 @@ func (alloc *Action) predicate(task *api.TaskInfo, node *api.NodeInfo) ([]*api.S if statusSets.ContainsUnschedulable() || statusSets.ContainsUnschedulableAndUnresolvable() || statusSets.ContainsErrorSkipOrWait() { - return nil, api.NewFitStatus(task, node, statusSets) + return nil, api.NewFitStatus(task, node, statusSets...) } return nil, nil } diff --git a/pkg/scheduler/actions/preempt/preempt.go b/pkg/scheduler/actions/preempt/preempt.go index 78509632d7e..3ee2ec562e0 100644 --- a/pkg/scheduler/actions/preempt/preempt.go +++ b/pkg/scheduler/actions/preempt/preempt.go @@ -216,7 +216,7 @@ func preempt( // When filtering candidate nodes, need to consider the node statusSets instead of the err information. // refer to kube-scheduler preemption code: https://github.com/kubernetes/kubernetes/blob/9d87fa215d9e8020abdc17132d1252536cd752d2/pkg/scheduler/framework/preemption/preemption.go#L422 if statusSets.ContainsUnschedulableAndUnresolvable() || statusSets.ContainsErrorSkipOrWait() { - return nil, api.NewFitStatus(task, node, statusSets) + return nil, api.NewFitStatus(task, node, statusSets...) } return nil, nil } diff --git a/pkg/scheduler/api/unschedule_info.go b/pkg/scheduler/api/unschedule_info.go index 07cd55f5fa9..f7149222a67 100644 --- a/pkg/scheduler/api/unschedule_info.go +++ b/pkg/scheduler/api/unschedule_info.go @@ -114,7 +114,8 @@ func NewFitError(task *TaskInfo, node *NodeInfo, message ...string) *FitError { return fe } -func NewFitStatus(task *TaskInfo, node *NodeInfo, sts StatusSets) *FitError { +// NewFitStatus returns a fit error with code and reason in it +func NewFitStatus(task *TaskInfo, node *NodeInfo, sts ...*Status) *FitError { fe := &FitError{ taskName: task.Name, taskNamespace: task.Namespace, @@ -126,6 +127,9 @@ func NewFitStatus(task *TaskInfo, node *NodeInfo, sts StatusSets) *FitError { // Reasons returns the reasons func (fe *FitError) Reasons() []string { + if fe == nil { + return []string{} + } return fe.Status.Reasons() } diff --git a/pkg/scheduler/api/unschedule_info_test.go b/pkg/scheduler/api/unschedule_info_test.go new file mode 100644 index 00000000000..37ab23a4e32 --- /dev/null +++ b/pkg/scheduler/api/unschedule_info_test.go @@ -0,0 +1,117 @@ +/* +Copyright 2024 The Volcano Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package api + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +const ( + affinityRulesNotMatch = "node(s) didn't match pod affinity rules" + existingAntiAffinityNotMatch = "node(s) didn't satisfy existing pods anti-affinity rules" + nodeAffinity = "node(s) didn't match Pod's node affinity/selector" +) + +func TestFitError(t *testing.T) { + tests := []struct { + task *TaskInfo + node *NodeInfo + reason []string + status []*Status + want *FitError + errStr string + }{ + { + task: &TaskInfo{Name: "pod1", Namespace: "ns1"}, + node: &NodeInfo{Name: "node1"}, + reason: []string{affinityRulesNotMatch, nodeAffinity}, + want: &FitError{ + NodeName: "node1", taskNamespace: "ns1", taskName: "pod1", + Status: []*Status{{Reason: affinityRulesNotMatch, Code: Error}, {Reason: nodeAffinity, Code: Error}}, + }, + errStr: "task ns1/pod1 on node node1 fit failed: " + affinityRulesNotMatch + ", " + nodeAffinity, + }, + { + task: &TaskInfo{Name: "pod2", Namespace: "ns2"}, + node: &NodeInfo{Name: "node2"}, + status: []*Status{{Reason: nodeAffinity, Code: UnschedulableAndUnresolvable}, {Reason: existingAntiAffinityNotMatch, Code: Error}}, + reason: []string{nodeAffinity, existingAntiAffinityNotMatch}, + want: &FitError{ + NodeName: "node2", taskNamespace: "ns2", taskName: "pod2", + Status: []*Status{{Reason: nodeAffinity, Code: UnschedulableAndUnresolvable}, {Reason: existingAntiAffinityNotMatch, Code: Error}}, + }, + errStr: "task ns2/pod2 on node node2 fit failed: " + nodeAffinity + ", " + existingAntiAffinityNotMatch, + }, + } + + var got *FitError + for _, test := range tests { + if len(test.status) != 0 { + got = NewFitStatus(test.task, test.node, test.status...) + } else if len(test.reason) != 0 { + got = NewFitError(test.task, test.node, test.reason...) + } + + assert.Equal(t, test.want, got) + assert.Equal(t, test.reason, got.Reasons()) + assert.Equal(t, test.errStr, got.Error()) + } +} + +func TestFitErrors(t *testing.T) { + tests := []struct { + node string + fitStr string + err error + fiterr *FitError + want string // expected error string + }{ + { + want: "0/0 nodes are unavailable", // base fit err string is empty, set as the default + }, + { + node: "node1", + fitStr: "fit failed", + err: fmt.Errorf(NodePodNumberExceeded), + want: "fit failed: 1 node(s) pod number exceeded.", + }, + { + node: "node1", + fitStr: "NodeResourceFitFailed", + err: fmt.Errorf(NodePodNumberExceeded), + fiterr: &FitError{ + taskNamespace: "ns1", taskName: "task1", NodeName: "node2", + Status: []*Status{{Reason: nodeAffinity, Code: UnschedulableAndUnresolvable}}, + }, + want: "NodeResourceFitFailed: 1 node(s) didn't match Pod's node affinity/selector, 1 node(s) pod number exceeded.", + }, + } + for _, test := range tests { + fitErrs := NewFitErrors() + fitErrs.SetError(test.fitStr) + if test.err != nil { + fitErrs.SetNodeError(test.node, test.err) + } + if test.fiterr != nil { + fitErrs.SetNodeError(test.fiterr.NodeName, test.fiterr) + } + got := fitErrs.Error() + assert.Equal(t, test.want, got) + } +}