github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #114629 from kerthcet/feat/change-return-type-of-Filter

Browse Source 
Modify the return type of RunFilterPlugins to *Status
This commit is contained in:
Kubernetes Prow Robot
2023-01-03 15:31:31 -08:00
committed by GitHub
parent 88c952d256 49e7e80999
commit 3f752b5edf
4 changed files with 46 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
pkg/scheduler/framework/autoscaler_contract/framework_contract_test.go
View File

@@ -32,7 +32,7 @@ import (
type frameworkContract interface {
RunPreFilterPlugins(ctx context.Context, state *framework.CycleState, pod *v1.Pod) (*framework.PreFilterResult, *framework.Status)
RunFilterPlugins(context.Context, *framework.CycleState, *v1.Pod, *framework.NodeInfo) framework.PluginToStatus
RunFilterPlugins(context.Context, *framework.CycleState, *v1.Pod, *framework.NodeInfo) *framework.Status
}
func TestFrameworkContract(t *testing.T) {

2
pkg/scheduler/framework/interface.go
View File

@@ -757,7 +757,7 @@ type PluginsRunner interface {
// preemption, we may pass a copy of the original nodeInfo object that has some pods
// removed from it to evaluate the possibility of preempting them to
// schedule the target pod.
RunFilterPlugins(context.Context, *CycleState, *v1.Pod, *NodeInfo) PluginToStatus
RunFilterPlugins(context.Context, *CycleState, *v1.Pod, *NodeInfo) *Status
// RunPreFilterExtensionAddPod calls the AddPod interface for the set of configured
// PreFilter plugins. It returns directly if any of the plugins return any
// status other than Success.

22
pkg/scheduler/framework/runtime/framework.go
View File

@@ -722,22 +722,23 @@ func (f *frameworkImpl) RunFilterPlugins(
state *framework.CycleState,
pod *v1.Pod,
nodeInfo *framework.NodeInfo,
) framework.PluginToStatus {
statuses := make(framework.PluginToStatus)
) *framework.Status {
var status *framework.Status
for _, pl := range f.filterPlugins {
pluginStatus := f.runFilterPlugin(ctx, pl, state, pod, nodeInfo)
if !pluginStatus.IsSuccess() {
if !pluginStatus.IsUnschedulable() {
status = f.runFilterPlugin(ctx, pl, state, pod, nodeInfo)
if !status.IsSuccess() {
if !status.IsUnschedulable() {
// Filter plugins are not supposed to return any status other than
// Success or Unschedulable.
pluginStatus = framework.AsStatus(fmt.Errorf("running %q filter plugin: %w", pl.Name(), pluginStatus.AsError()))
status = framework.AsStatus(fmt.Errorf("running %q filter plugin: %w", pl.Name(), status.AsError()))
}
pluginStatus.SetFailedPlugin(pl.Name())
return map[string]*framework.Status{pl.Name(): pluginStatus}
status.SetFailedPlugin(pl.Name())
return status
}
}
return statuses
return status
}
func (f *frameworkImpl) runFilterPlugin(ctx context.Context, pl framework.FilterPlugin, state *framework.CycleState, pod *v1.Pod, nodeInfo *framework.NodeInfo) *framework.Status {
@@ -832,8 +833,7 @@ func (f *frameworkImpl) RunFilterPluginsWithNominatedPods(ctx context.Context, s
break
}
statusMap := f.RunFilterPlugins(ctx, stateToUse, pod, nodeInfoToUse)
status = statusMap.Merge()
status = f.RunFilterPlugins(ctx, stateToUse, pod, nodeInfoToUse)
if !status.IsSuccess() && !status.IsUnschedulable() {
return status
}

75
pkg/scheduler/framework/runtime/framework_test.go
View File

@@ -67,6 +67,15 @@ const (
var _ framework.ScorePlugin = &TestScoreWithNormalizePlugin{}
var _ framework.ScorePlugin = &TestScorePlugin{}
var cmpOpts = []cmp.Option{
cmp.Comparer(func(s1 *framework.Status, s2 *framework.Status) bool {
if s1 == nil || s2 == nil {
return s1.IsSuccess() && s2.IsSuccess()
}
return s1.Code() == s2.Code() && s1.FailedPlugin() == s2.FailedPlugin() && s1.Message() == s2.Message()
}),
}
func newScoreWithNormalizePlugin1(injArgs runtime.Object, f framework.Handle) (framework.Plugin, error) {
var inj injectedResult
if err := DecodeInto(injArgs, &inj); err != nil {
@@ -1442,10 +1451,9 @@ func TestRunPreFilterPluginsStatus(t *testing.T) {
func TestFilterPlugins(t *testing.T) {
tests := []struct {
name string
plugins []*TestPlugin
wantStatus *framework.Status
wantStatusMap framework.PluginToStatus
name string
plugins []*TestPlugin
wantStatus *framework.Status
}{
{
name: "SuccessFilter",
@@ -1455,8 +1463,7 @@ func TestFilterPlugins(t *testing.T) {
inj: injectedResult{FilterStatus: int(framework.Success)},
},
},
wantStatus: nil,
wantStatusMap: framework.PluginToStatus{},
wantStatus: nil,
},
{
name: "ErrorFilter",
@@ -1467,9 +1474,6 @@ func TestFilterPlugins(t *testing.T) {
},
},
wantStatus: framework.AsStatus(fmt.Errorf(`running "TestPlugin" filter plugin: %w`, errInjectedFilterStatus)).WithFailedPlugin("TestPlugin"),
wantStatusMap: framework.PluginToStatus{
"TestPlugin": framework.AsStatus(fmt.Errorf(`running "TestPlugin" filter plugin: %w`, errInjectedFilterStatus)).WithFailedPlugin("TestPlugin"),
},
},
{
name: "UnschedulableFilter",
@@ -1480,9 +1484,6 @@ func TestFilterPlugins(t *testing.T) {
},
},
wantStatus: framework.NewStatus(framework.Unschedulable, injectFilterReason).WithFailedPlugin("TestPlugin"),
wantStatusMap: framework.PluginToStatus{
"TestPlugin": framework.NewStatus(framework.Unschedulable, injectFilterReason).WithFailedPlugin("TestPlugin"),
},
},
{
name: "UnschedulableAndUnresolvableFilter",
@@ -1494,9 +1495,6 @@ func TestFilterPlugins(t *testing.T) {
},
},
wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, injectFilterReason).WithFailedPlugin("TestPlugin"),
wantStatusMap: framework.PluginToStatus{
"TestPlugin": framework.NewStatus(framework.UnschedulableAndUnresolvable, injectFilterReason).WithFailedPlugin("TestPlugin"),
},
},
// following tests cover multiple-plugins scenarios
{
@@ -1512,9 +1510,6 @@ func TestFilterPlugins(t *testing.T) {
},
},
wantStatus: framework.AsStatus(fmt.Errorf(`running "TestPlugin1" filter plugin: %w`, errInjectedFilterStatus)).WithFailedPlugin("TestPlugin1"),
wantStatusMap: framework.PluginToStatus{
"TestPlugin1": framework.AsStatus(fmt.Errorf(`running "TestPlugin1" filter plugin: %w`, errInjectedFilterStatus)).WithFailedPlugin("TestPlugin1"),
},
},
{
name: "UnschedulableAndUnschedulableFilters",
@@ -1529,9 +1524,20 @@ func TestFilterPlugins(t *testing.T) {
},
},
wantStatus: framework.NewStatus(framework.Unschedulable, injectFilterReason).WithFailedPlugin("TestPlugin1"),
wantStatusMap: framework.PluginToStatus{
"TestPlugin1": framework.NewStatus(framework.Unschedulable, injectFilterReason).WithFailedPlugin("TestPlugin1"),
},
{
name: "UnschedulableAndUnschedulableAndUnresolvableFilters",
plugins: []*TestPlugin{
{
name: "TestPlugin1",
inj: injectedResult{FilterStatus: int(framework.UnschedulableAndUnresolvable)},
},
{
name: "TestPlugin2",
inj: injectedResult{FilterStatus: int(framework.Unschedulable)},
},
},
wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, injectFilterReason).WithFailedPlugin("TestPlugin1"),
},
{
name: "SuccessAndSuccessFilters",
@@ -1540,14 +1546,12 @@ func TestFilterPlugins(t *testing.T) {
name: "TestPlugin1",
inj: injectedResult{FilterStatus: int(framework.Success)},
},
{
name: "TestPlugin2",
inj: injectedResult{FilterStatus: int(framework.Success)},
},
},
wantStatus: nil,
wantStatusMap: framework.PluginToStatus{},
wantStatus: nil,
},
{
name: "ErrorAndSuccessFilters",
@@ -1562,9 +1566,6 @@ func TestFilterPlugins(t *testing.T) {
},
},
wantStatus: framework.AsStatus(fmt.Errorf(`running "TestPlugin1" filter plugin: %w`, errInjectedFilterStatus)).WithFailedPlugin("TestPlugin1"),
wantStatusMap: framework.PluginToStatus{
"TestPlugin1": framework.AsStatus(fmt.Errorf(`running "TestPlugin1" filter plugin: %w`, errInjectedFilterStatus)).WithFailedPlugin("TestPlugin1"),
},
},
{
name: "SuccessAndErrorFilters",
@@ -1580,9 +1581,6 @@ func TestFilterPlugins(t *testing.T) {
},
},
wantStatus: framework.AsStatus(fmt.Errorf(`running "TestPlugin2" filter plugin: %w`, errInjectedFilterStatus)).WithFailedPlugin("TestPlugin2"),
wantStatusMap: framework.PluginToStatus{
"TestPlugin2": framework.AsStatus(fmt.Errorf(`running "TestPlugin2" filter plugin: %w`, errInjectedFilterStatus)).WithFailedPlugin("TestPlugin2"),
},
},
{
name: "SuccessAndUnschedulableFilters",
@@ -1591,16 +1589,12 @@ func TestFilterPlugins(t *testing.T) {
name: "TestPlugin1",
inj: injectedResult{FilterStatus: int(framework.Success)},
},
{
name: "TestPlugin2",
inj: injectedResult{FilterStatus: int(framework.Unschedulable)},
},
},
wantStatus: framework.NewStatus(framework.Unschedulable, injectFilterReason).WithFailedPlugin("TestPlugin2"),
wantStatusMap: framework.PluginToStatus{
"TestPlugin2": framework.NewStatus(framework.Unschedulable, injectFilterReason).WithFailedPlugin("TestPlugin2"),
},
},
}
@@ -1625,17 +1619,14 @@ func TestFilterPlugins(t *testing.T) {
profile := config.KubeSchedulerProfile{Plugins: cfgPls}
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
f, err := newFrameworkWithQueueSortAndBind(registry, profile, ctx.Done())
if err != nil {
t.Fatalf("fail to create framework: %s", err)
}
gotStatusMap := f.RunFilterPlugins(ctx, nil, pod, nil)
gotStatus := gotStatusMap.Merge()
if !reflect.DeepEqual(gotStatus, tt.wantStatus) {
t.Errorf("wrong status code. got: %v, want:%v", gotStatus, tt.wantStatus)
}
if !reflect.DeepEqual(gotStatusMap, tt.wantStatusMap) {
t.Errorf("wrong status map. got: %+v, want: %+v", gotStatusMap, tt.wantStatusMap)
gotStatus := f.RunFilterPlugins(ctx, nil, pod, nil)
if diff := cmp.Diff(gotStatus, tt.wantStatus, cmpOpts...); diff != "" {
t.Errorf("Unexpected status: (-got, +want):\n%s", diff)
}
})
}
@@ -1864,8 +1855,8 @@ func TestFilterPluginsWithNominatedPods(t *testing.T) {
}
tt.nodeInfo.SetNode(tt.node)
gotStatus := f.RunFilterPluginsWithNominatedPods(ctx, nil, tt.pod, tt.nodeInfo)
if !reflect.DeepEqual(gotStatus, tt.wantStatus) {
t.Errorf("Unexpected status. got: %v, want: %v", gotStatus, tt.wantStatus)
if diff := cmp.Diff(gotStatus, tt.wantStatus, cmpOpts...); diff != "" {
t.Errorf("Unexpected status: (-got, +want):\n%s", diff)
}
})
}