From 2f1f7db9efbd19bcb2bc8bcaed229b1e423cd28c Mon Sep 17 00:00:00 2001 From: michaelawyu Date: Mon, 5 Jun 2023 14:06:22 +0800 Subject: [PATCH 1/3] Added status related structs --- pkg/scheduler/framework/status.go | 134 +++++++++++++++++++++++ pkg/scheduler/framework/status_test.go | 141 +++++++++++++++++++++++++ 2 files changed, 275 insertions(+) create mode 100644 pkg/scheduler/framework/status.go create mode 100644 pkg/scheduler/framework/status_test.go diff --git a/pkg/scheduler/framework/status.go b/pkg/scheduler/framework/status.go new file mode 100644 index 000000000..cdce0ac20 --- /dev/null +++ b/pkg/scheduler/framework/status.go @@ -0,0 +1,134 @@ +/* +Copyright (c) Microsoft Corporation. +Licensed under the MIT license. +*/ + +package framework + +import "strings" + +// StatusCode is the status code of a Status, returned by a plugin. +type StatusCode int + +// Pre-defined status codes. +const ( + // Success signals that a plugin has completed its run successfully. + // Note that a nil *Status is also considered as a Success Status. + Success StatusCode = iota + // internalError signals that a plugin has encountered an internal error. + // Note that this status code is NOT exported; to return an internalError status, use the + // FromError() call. + internalError + // ClusterUnschedulable signals that a plugin has found that a placement should not be bound + // to a specific cluster. + ClusterUnschedulable + // PreSkip signals that a plugin should be skipped in the following stage. This is returned + // by plugins at PreFilture or PreScore stages only, to save some overhead. + PreSkip +) + +var statusCodeNames = []string{"Success", "InternalError", "ClusterUnschedulable", "PreSkip"} + +// Name returns the name of a status code. +func (sc StatusCode) Name() string { + return statusCodeNames[sc] +} + +// Status is the result yielded by a plugin. +type Status struct { + // statusCode is the status code of a Status. + statusCode StatusCode + // The reasons behind a Status; this should be empty if the Status is of the status code + // Success. + reasons []string + // The error associated with a Status; this is only set when the Status is of the status code + // internalError. + err error + // The name of the plugin which returns the Status. + sourcePlugin string +} + +// code returns the status code of a Status. +func (s *Status) code() StatusCode { + if s == nil { + return Success + } + return s.statusCode +} + +// IsSuccess returns if a Status is of the status code Success. +func (s *Status) IsSuccess() bool { + return s.code() == Success +} + +// IsInternalError returns if a Status is of the status code interalError. +func (s *Status) IsInteralError() bool { + return s.code() == internalError +} + +// IsPreSkip returns if a Status is of the status code PreSkip. +func (s *Status) IsPreSkip() bool { + return s.code() == PreSkip +} + +// IsClusterUnschedulable returns if a Status is of the status code ClusterUnschedulable. +func (s *Status) IsClusterUnschedulable() bool { + return s.code() == ClusterUnschedulable +} + +// Reasons returns the reasons of a Status. +func (s *Status) Reasons() []string { + if s == nil { + return []string{} + } + return s.reasons +} + +// SourcePlugin returns the source plugin associated with a Status. +func (s *Status) SourcePlugin() string { + if s == nil { + return "" + } + return s.sourcePlugin +} + +// InternalError returns the error associated with a Status. +func (s *Status) InternalError() error { + if s == nil { + return nil + } + return s.err +} + +// String returns the description of a Status. +func (s *Status) String() string { + if s == nil { + return s.code().Name() + } + desc := []string{s.code().Name()} + if s.err != nil { + desc = append(desc, s.err.Error()) + } + desc = append(desc, s.reasons...) + return strings.Join(desc, ", ") +} + +// NewNonErrorStatus returns a Status with a non-error status code. +// To return a Status of the internalError status code, use FromError() instead. +func NewNonErrorStatus(code StatusCode, sourcePlugin string, reasons ...string) *Status { + return &Status{ + statusCode: code, + reasons: reasons, + sourcePlugin: sourcePlugin, + } +} + +// FromError returns a Status from an error. +func FromError(err error, sourcePlugin string, reasons ...string) *Status { + return &Status{ + statusCode: internalError, + reasons: reasons, + err: err, + sourcePlugin: sourcePlugin, + } +} diff --git a/pkg/scheduler/framework/status_test.go b/pkg/scheduler/framework/status_test.go new file mode 100644 index 000000000..6875f6c7f --- /dev/null +++ b/pkg/scheduler/framework/status_test.go @@ -0,0 +1,141 @@ +/* +Copyright (c) Microsoft Corporation. +Licensed under the MIT license. +*/ + +package framework + +import ( + "fmt" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" +) + +const ( + dummyPlugin = "dummyPlugin" +) + +var ( + dummyReasons = []string{"reason1", "reason2"} +) + +func TestNonNilStatusMethods(t *testing.T) { + testCases := []struct { + name string + statusCode StatusCode + reasons []string + err error + sourcePlugin string + desc string + }{ + { + name: "status success", + statusCode: Success, + reasons: []string{}, + sourcePlugin: dummyPlugin, + }, + { + name: "status error", + statusCode: internalError, + err: fmt.Errorf("an unexpected error has occurred"), + reasons: dummyReasons, + sourcePlugin: dummyPlugin, + }, + { + name: "status unschedulable", + statusCode: ClusterUnschedulable, + reasons: dummyReasons, + sourcePlugin: dummyPlugin, + }, + { + name: "status preskip", + statusCode: PreSkip, + reasons: dummyReasons, + sourcePlugin: dummyPlugin, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var status *Status + if tc.err != nil { + status = FromError(tc.err, tc.sourcePlugin, tc.reasons...) + } else { + status = NewNonErrorStatus(tc.statusCode, tc.sourcePlugin, tc.reasons...) + } + + wantCheckOutputs := make([]bool, len(statusCodeNames)) + wantCheckOutputs[tc.statusCode] = true + checkFuncs := []func() bool{ + status.IsSuccess, + status.IsInteralError, + status.IsClusterUnschedulable, + status.IsPreSkip, + } + for idx, checkFunc := range checkFuncs { + if wantCheckOutputs[idx] != checkFunc() { + t.Fatalf("check function for %s = %t, want %t", statusCodeNames[idx], checkFunc(), wantCheckOutputs[idx]) + } + } + + if !cmp.Equal(status.Reasons(), tc.reasons) { + t.Fatalf("Reasons() = %v, want %v", status.Reasons(), tc.reasons) + } + + if !cmp.Equal(status.SourcePlugin(), tc.sourcePlugin) { + t.Fatalf("SourcePlugin() = %s, want %s", status.SourcePlugin(), tc.sourcePlugin) + } + + if !cmp.Equal(status.InternalError(), tc.err, cmpopts.EquateErrors()) { + t.Fatalf("InternalError() = %v, want %v", status.InternalError(), tc.err) + } + + descElems := []string{statusCodeNames[tc.statusCode]} + if tc.err != nil { + descElems = append(descElems, tc.err.Error()) + } + descElems = append(descElems, tc.reasons...) + wantDesc := strings.Join(descElems, ", ") + if !cmp.Equal(status.String(), wantDesc) { + t.Fatalf("String() = %s, want %s", status.String(), wantDesc) + } + }) + } +} + +func TestNilStatusMethods(t *testing.T) { + var status *Status + wantCheckOutputs := make([]bool, len(statusCodeNames)) + wantCheckOutputs[Success] = true + checkFuncs := []func() bool{ + status.IsSuccess, + status.IsInteralError, + status.IsClusterUnschedulable, + status.IsPreSkip, + } + for idx, checkFunc := range checkFuncs { + if wantCheckOutputs[idx] != checkFunc() { + t.Fatalf("check function for %s = %t, want %t", statusCodeNames[idx], checkFunc(), wantCheckOutputs[idx]) + } + } + + if !cmp.Equal(status.Reasons(), []string{}) { + t.Fatalf("Reasons() = %v, want %v", status.Reasons(), []string{}) + } + + if !cmp.Equal(status.SourcePlugin(), "") { + t.Fatalf("SourcePlugin() = %s, want %s", status.SourcePlugin(), "") + } + + if !cmp.Equal(status.InternalError(), nil, cmpopts.EquateErrors()) { + t.Fatalf("InternalError() = %v, want %v", status.InternalError(), nil) + } + + wantDesc := statusCodeNames[Success] + if !cmp.Equal(status.String(), wantDesc) { + t.Fatalf("String() = %s, want %s", status.String(), wantDesc) + } +} From 80ffd04bb8874ad283ab09b2ab242c1295c347ad Mon Sep 17 00:00:00 2001 From: michaelawyu Date: Mon, 5 Jun 2023 16:03:05 +0800 Subject: [PATCH 2/3] Minor fixes --- pkg/scheduler/framework/status.go | 13 ++++++++----- pkg/scheduler/framework/status_test.go | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/scheduler/framework/status.go b/pkg/scheduler/framework/status.go index cdce0ac20..1efb0f887 100644 --- a/pkg/scheduler/framework/status.go +++ b/pkg/scheduler/framework/status.go @@ -22,9 +22,12 @@ const ( // ClusterUnschedulable signals that a plugin has found that a placement should not be bound // to a specific cluster. ClusterUnschedulable - // PreSkip signals that a plugin should be skipped in the following stage. This is returned - // by plugins at PreFilture or PreScore stages only, to save some overhead. - PreSkip + // Skip signals that no action is needed for the plugin to take at the stage. + // If this is returned by a plugin at the Pre- stages (PreFilter or PreScore), the associated + // plugin will be skipped at the following stages (Filter or Score) as well. This helps + // reduce the overhead of having to repeatedly call a plugin that is not needed for every + // cluster in the Filter or Score stage. + Skip ) var statusCodeNames = []string{"Success", "InternalError", "ClusterUnschedulable", "PreSkip"} @@ -66,9 +69,9 @@ func (s *Status) IsInteralError() bool { return s.code() == internalError } -// IsPreSkip returns if a Status is of the status code PreSkip. +// IsPreSkip returns if a Status is of the status code Skip. func (s *Status) IsPreSkip() bool { - return s.code() == PreSkip + return s.code() == Skip } // IsClusterUnschedulable returns if a Status is of the status code ClusterUnschedulable. diff --git a/pkg/scheduler/framework/status_test.go b/pkg/scheduler/framework/status_test.go index 6875f6c7f..8e50d6f20 100644 --- a/pkg/scheduler/framework/status_test.go +++ b/pkg/scheduler/framework/status_test.go @@ -52,7 +52,7 @@ func TestNonNilStatusMethods(t *testing.T) { }, { name: "status preskip", - statusCode: PreSkip, + statusCode: Skip, reasons: dummyReasons, sourcePlugin: dummyPlugin, }, From 789010091b28dbf5aab30990b2fbe1b21a117fc3 Mon Sep 17 00:00:00 2001 From: michaelawyu Date: Tue, 6 Jun 2023 13:20:28 +0800 Subject: [PATCH 3/3] Minor fixes --- pkg/scheduler/framework/status.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/scheduler/framework/status.go b/pkg/scheduler/framework/status.go index 1efb0f887..5e63d67a9 100644 --- a/pkg/scheduler/framework/status.go +++ b/pkg/scheduler/framework/status.go @@ -30,7 +30,7 @@ const ( Skip ) -var statusCodeNames = []string{"Success", "InternalError", "ClusterUnschedulable", "PreSkip"} +var statusCodeNames = []string{"Success", "InternalError", "ClusterUnschedulable", "Skip"} // Name returns the name of a status code. func (sc StatusCode) Name() string {