From e789beb213939b38993813210c95f1b570d0a601 Mon Sep 17 00:00:00 2001 From: kidddddddddddddddddddddd <1062602710@qq.com> Date: Tue, 13 Dec 2022 23:35:06 +0800 Subject: [PATCH] errMsg --- pkg/scheduler/framework/interface.go | 28 +++++++++---------- pkg/scheduler/framework/interface_test.go | 17 +++++++---- .../plugins/nodevolumelimits/csi_test.go | 5 ++-- .../plugins/nodevolumelimits/non_csi_test.go | 5 ++-- pkg/scheduler/schedule_one.go | 5 +--- 5 files changed, 33 insertions(+), 27 deletions(-) diff --git a/pkg/scheduler/framework/interface.go b/pkg/scheduler/framework/interface.go index 4b4c8a7aef2..2623f5b988a 100644 --- a/pkg/scheduler/framework/interface.go +++ b/pkg/scheduler/framework/interface.go @@ -177,7 +177,7 @@ func (s *Status) Message() string { if s == nil { return "" } - return strings.Join(s.reasons, ", ") + return strings.Join(s.Reasons(), ", ") } // SetFailedPlugin sets the given plugin name to s.failedPlugin. @@ -199,6 +199,9 @@ func (s *Status) FailedPlugin() string { // Reasons returns reasons of the Status. func (s *Status) Reasons() []string { + if s.err != nil { + return append([]string{s.err.Error()}, s.reasons...) + } return s.reasons } @@ -249,8 +252,8 @@ func (s *Status) Equal(x *Status) bool { if s.code != x.code { return false } - if s.code == Error { - return cmp.Equal(s.err, x.err, cmpopts.EquateErrors()) + if !cmp.Equal(s.err, x.err, cmpopts.EquateErrors()) { + return false } return cmp.Equal(s.reasons, x.reasons) } @@ -261,9 +264,6 @@ func NewStatus(code Code, reasons ...string) *Status { code: code, reasons: reasons, } - if code == Error { - s.err = errors.New(s.Message()) - } return s } @@ -273,9 +273,8 @@ func AsStatus(err error) *Status { return nil } return &Status{ - code: Error, - reasons: []string{err.Error()}, - err: err, + code: Error, + err: err, } } @@ -292,20 +291,21 @@ func (p PluginToStatus) Merge() *Status { finalStatus := NewStatus(Success) for _, s := range p { - if s.Code() == Error { - finalStatus.err = s.AsError() - } if statusPrecedence[s.Code()] > statusPrecedence[finalStatus.code] { finalStatus.code = s.Code() // Same as code, we keep the most relevant failedPlugin in the returned Status. finalStatus.failedPlugin = s.FailedPlugin() } - for _, r := range s.reasons { + reasons := s.Reasons() + if finalStatus.err == nil { + finalStatus.err = s.err + reasons = s.reasons + } + for _, r := range reasons { finalStatus.AppendReason(r) } } - return finalStatus } diff --git a/pkg/scheduler/framework/interface_test.go b/pkg/scheduler/framework/interface_test.go index dc4e1aa4d05..a8c5fbc0b4a 100644 --- a/pkg/scheduler/framework/interface_test.go +++ b/pkg/scheduler/framework/interface_test.go @@ -26,6 +26,7 @@ import ( ) var errorStatus = NewStatus(Error, "internal error") +var statusWithErr = AsStatus(errors.New("internal error")) func TestStatus(t *testing.T) { tests := []struct { @@ -273,10 +274,10 @@ func TestIsStatusEqual(t *testing.T) { want: true, }, { - name: "error statuses with same message should not be equal", + name: "error statuses with same message should be equal", x: NewStatus(Error, "error"), y: NewStatus(Error, "error"), - want: false, + want: true, }, { name: "statuses with different reasons should not be equal", @@ -292,14 +293,20 @@ func TestIsStatusEqual(t *testing.T) { }, { name: "wrap error status should be equal with original one", - x: errorStatus, - y: AsStatus(fmt.Errorf("error: %w", errorStatus.AsError())), + x: statusWithErr, + y: AsStatus(fmt.Errorf("error: %w", statusWithErr.AsError())), want: true, }, + { + name: "statues with different errors that have the same message shouldn't be equal", + x: AsStatus(errors.New("error")), + y: AsStatus(errors.New("error")), + want: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := cmp.Equal(tt.x, tt.y); got != tt.want { + if got := tt.x.Equal(tt.y); got != tt.want { t.Errorf("cmp.Equal() = %v, want %v", got, tt.want) } }) diff --git a/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go b/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go index f26c9b8cf00..4c3f4768feb 100644 --- a/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go +++ b/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go @@ -18,6 +18,7 @@ package nodevolumelimits import ( "context" + "errors" "fmt" "reflect" "strings" @@ -397,7 +398,7 @@ func TestCSILimits(t *testing.T) { ephemeralEnabled: true, driverNames: []string{ebsCSIDriverName}, test: "ephemeral volume missing", - wantStatus: framework.NewStatus(framework.Error, `looking up PVC test/abc-xyz: persistentvolumeclaim "abc-xyz" not found`), + wantStatus: framework.AsStatus(errors.New(`looking up PVC test/abc-xyz: persistentvolumeclaim "abc-xyz" not found`)), }, { newPod: ephemeralVolumePod, @@ -406,7 +407,7 @@ func TestCSILimits(t *testing.T) { extraClaims: []v1.PersistentVolumeClaim{*conflictingClaim}, driverNames: []string{ebsCSIDriverName}, test: "ephemeral volume not owned", - wantStatus: framework.NewStatus(framework.Error, "PVC test/abc-xyz was not created for pod test/abc (pod is not owner)"), + wantStatus: framework.AsStatus(errors.New("PVC test/abc-xyz was not created for pod test/abc (pod is not owner)")), }, { newPod: ephemeralVolumePod, diff --git a/pkg/scheduler/framework/plugins/nodevolumelimits/non_csi_test.go b/pkg/scheduler/framework/plugins/nodevolumelimits/non_csi_test.go index e74787edb98..cac58b82f29 100644 --- a/pkg/scheduler/framework/plugins/nodevolumelimits/non_csi_test.go +++ b/pkg/scheduler/framework/plugins/nodevolumelimits/non_csi_test.go @@ -18,6 +18,7 @@ package nodevolumelimits import ( "context" + "errors" "fmt" "os" "reflect" @@ -126,14 +127,14 @@ func TestEphemeralLimits(t *testing.T) { newPod: ephemeralVolumePod, ephemeralEnabled: true, test: "volume missing", - wantStatus: framework.NewStatus(framework.Error, `looking up PVC test/abc-xyz: persistentvolumeclaim "abc-xyz" not found`), + wantStatus: framework.AsStatus(errors.New(`looking up PVC test/abc-xyz: persistentvolumeclaim "abc-xyz" not found`)), }, { newPod: ephemeralVolumePod, ephemeralEnabled: true, extraClaims: []v1.PersistentVolumeClaim{*conflictingClaim}, test: "volume not owned", - wantStatus: framework.NewStatus(framework.Error, "PVC test/abc-xyz was not created for pod test/abc (pod is not owner)"), + wantStatus: framework.AsStatus(errors.New("PVC test/abc-xyz was not created for pod test/abc (pod is not owner)")), }, { newPod: ephemeralVolumePod, diff --git a/pkg/scheduler/schedule_one.go b/pkg/scheduler/schedule_one.go index 1135b7ab121..06703c509fb 100644 --- a/pkg/scheduler/schedule_one.go +++ b/pkg/scheduler/schedule_one.go @@ -859,10 +859,7 @@ func (sched *Scheduler) handleSchedulingFailure(ctx context.Context, fwk framewo pod := podInfo.Pod err := status.AsError() - var errMsg string - if err != nil { - errMsg = err.Error() - } + errMsg := status.Message() if err == ErrNoNodesAvailable { klog.V(2).InfoS("Unable to schedule pod; no nodes are registered to the cluster; waiting", "pod", klog.KObj(pod), "err", err)