diff --git a/pkg/controller/volume/scheduling/scheduler_binder.go b/pkg/controller/volume/scheduling/scheduler_binder.go index 10c4caccc13..ed3805099bd 100644 --- a/pkg/controller/volume/scheduling/scheduler_binder.go +++ b/pkg/controller/volume/scheduling/scheduler_binder.go @@ -443,7 +443,7 @@ func (b *volumeBinder) BindPodVolumes(assumedPod *v1.Pod, podVolumes *PodVolumes return b, err }) if err != nil { - return fmt.Errorf("Failed to bind volumes: %v", err) + return fmt.Errorf("binding volumes: %w", err) } return nil } @@ -543,7 +543,7 @@ func (b *volumeBinder) checkBindings(pod *v1.Pod, bindings []*BindingInfo, claim node, err := b.nodeLister.Get(pod.Spec.NodeName) if err != nil { - return false, fmt.Errorf("failed to get node %q: %v", pod.Spec.NodeName, err) + return false, fmt.Errorf("failed to get node %q: %w", pod.Spec.NodeName, err) } csiNode, err := b.csiNodeLister.Get(node.Name) @@ -559,7 +559,7 @@ func (b *volumeBinder) checkBindings(pod *v1.Pod, bindings []*BindingInfo, claim _, err = b.podLister.Pods(pod.Namespace).Get(pod.Name) if err != nil { if apierrors.IsNotFound(err) { - return false, fmt.Errorf("pod %q does not exist any more", podName) + return false, fmt.Errorf("pod does not exist any more: %w", err) } klog.Errorf("failed to get pod %s/%s from the lister: %v", pod.Namespace, pod.Name, err) } @@ -567,12 +567,12 @@ func (b *volumeBinder) checkBindings(pod *v1.Pod, bindings []*BindingInfo, claim for _, binding := range bindings { pv, err := b.pvCache.GetAPIPV(binding.pv.Name) if err != nil { - return false, fmt.Errorf("failed to check binding: %v", err) + return false, fmt.Errorf("failed to check binding: %w", err) } pvc, err := b.pvcCache.GetAPIPVC(getPVCName(binding.pvc)) if err != nil { - return false, fmt.Errorf("failed to check binding: %v", err) + return false, fmt.Errorf("failed to check binding: %w", err) } // Because we updated PV in apiserver, skip if API object is older @@ -583,12 +583,12 @@ func (b *volumeBinder) checkBindings(pod *v1.Pod, bindings []*BindingInfo, claim pv, err = b.tryTranslatePVToCSI(pv, csiNode) if err != nil { - return false, fmt.Errorf("failed to translate pv to csi: %v", err) + return false, fmt.Errorf("failed to translate pv to csi: %w", err) } // Check PV's node affinity (the node might not have the proper label) if err := volumeutil.CheckNodeAffinity(pv, node.Labels); err != nil { - return false, fmt.Errorf("pv %q node affinity doesn't match node %q: %v", pv.Name, node.Name, err) + return false, fmt.Errorf("pv %q node affinity doesn't match node %q: %w", pv.Name, node.Name, err) } // Check if pv.ClaimRef got dropped by unbindVolume() @@ -605,7 +605,7 @@ func (b *volumeBinder) checkBindings(pod *v1.Pod, bindings []*BindingInfo, claim for _, claim := range claimsToProvision { pvc, err := b.pvcCache.GetAPIPVC(getPVCName(claim)) if err != nil { - return false, fmt.Errorf("failed to check provisioning pvc: %v", err) + return false, fmt.Errorf("failed to check provisioning pvc: %w", err) } // Because we updated PVC in apiserver, skip if API object is older @@ -637,7 +637,7 @@ func (b *volumeBinder) checkBindings(pod *v1.Pod, bindings []*BindingInfo, claim // be unbound eventually. return false, nil } - return false, fmt.Errorf("failed to get pv %q from cache: %v", pvc.Spec.VolumeName, err) + return false, fmt.Errorf("failed to get pv %q from cache: %w", pvc.Spec.VolumeName, err) } pv, err = b.tryTranslatePVToCSI(pv, csiNode) @@ -646,7 +646,7 @@ func (b *volumeBinder) checkBindings(pod *v1.Pod, bindings []*BindingInfo, claim } if err := volumeutil.CheckNodeAffinity(pv, node.Labels); err != nil { - return false, fmt.Errorf("pv %q node affinity doesn't match node %q: %v", pv.Name, node.Name, err) + return false, fmt.Errorf("pv %q node affinity doesn't match node %q: %w", pv.Name, node.Name, err) } } diff --git a/pkg/scheduler/framework/plugins/defaultbinder/default_binder.go b/pkg/scheduler/framework/plugins/defaultbinder/default_binder.go index f3319dba412..2b2c7f9bb47 100644 --- a/pkg/scheduler/framework/plugins/defaultbinder/default_binder.go +++ b/pkg/scheduler/framework/plugins/defaultbinder/default_binder.go @@ -55,7 +55,7 @@ func (b DefaultBinder) Bind(ctx context.Context, state *framework.CycleState, p } err := b.handle.ClientSet().CoreV1().Pods(binding.Namespace).Bind(ctx, binding, metav1.CreateOptions{}) if err != nil { - return framework.NewStatus(framework.Error, err.Error()) + return framework.AsStatus(err) } return nil } diff --git a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go index a8fa0256e80..ff0005074a7 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go +++ b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go @@ -130,7 +130,7 @@ func (pl *VolumeBinding) PreFilter(ctx context.Context, state *framework.CycleSt } boundClaims, claimsToBind, unboundClaimsImmediate, err := pl.Binder.GetPodVolumes(pod) if err != nil { - return framework.NewStatus(framework.Error, err.Error()) + return framework.AsStatus(err) } if len(unboundClaimsImmediate) > 0 { // Return UnschedulableAndUnresolvable error if immediate claims are @@ -184,7 +184,7 @@ func (pl *VolumeBinding) Filter(ctx context.Context, cs *framework.CycleState, p state, err := getStateData(cs) if err != nil { - return framework.NewStatus(framework.Error, err.Error()) + return framework.AsStatus(err) } if state.skip { @@ -215,14 +215,14 @@ func (pl *VolumeBinding) Filter(ctx context.Context, cs *framework.CycleState, p func (pl *VolumeBinding) Reserve(ctx context.Context, cs *framework.CycleState, pod *v1.Pod, nodeName string) *framework.Status { state, err := getStateData(cs) if err != nil { - return framework.NewStatus(framework.Error, err.Error()) + return framework.AsStatus(err) } // we don't need to hold the lock as only one node will be reserved for the given pod podVolumes, ok := state.podVolumesByNode[nodeName] if ok { allBound, err := pl.Binder.AssumePodVolumes(pod, nodeName, podVolumes) if err != nil { - return framework.NewStatus(framework.Error, err.Error()) + return framework.AsStatus(err) } state.allBound = allBound } else { @@ -240,7 +240,7 @@ func (pl *VolumeBinding) Reserve(ctx context.Context, cs *framework.CycleState, func (pl *VolumeBinding) PreBind(ctx context.Context, cs *framework.CycleState, pod *v1.Pod, nodeName string) *framework.Status { s, err := getStateData(cs) if err != nil { - return framework.NewStatus(framework.Error, err.Error()) + return framework.AsStatus(err) } if s.allBound { // no need to bind volumes @@ -249,13 +249,13 @@ func (pl *VolumeBinding) PreBind(ctx context.Context, cs *framework.CycleState, // we don't need to hold the lock as only one node will be pre-bound for the given pod podVolumes, ok := s.podVolumesByNode[nodeName] if !ok { - return framework.NewStatus(framework.Error, fmt.Sprintf("no pod volumes found for node %q", nodeName)) + return framework.AsStatus(fmt.Errorf("no pod volumes found for node %q", nodeName)) } klog.V(5).Infof("Trying to bind volumes for pod \"%v/%v\"", pod.Namespace, pod.Name) err = pl.Binder.BindPodVolumes(pod, podVolumes) if err != nil { klog.V(1).Infof("Failed to bind volumes for pod \"%v/%v\": %v", pod.Namespace, pod.Name, err) - return framework.NewStatus(framework.Error, err.Error()) + return framework.AsStatus(err) } klog.V(5).Infof("Success binding volumes for pod \"%v/%v\"", pod.Namespace, pod.Name) return nil diff --git a/pkg/scheduler/framework/runtime/framework.go b/pkg/scheduler/framework/runtime/framework.go index 2ae78a1555c..d4708b82192 100644 --- a/pkg/scheduler/framework/runtime/framework.go +++ b/pkg/scheduler/framework/runtime/framework.go @@ -710,9 +710,9 @@ func (f *frameworkImpl) RunPreBindPlugins(ctx context.Context, state *framework. for _, pl := range f.preBindPlugins { status = f.runPreBindPlugin(ctx, pl, state, pod, nodeName) if !status.IsSuccess() { - msg := fmt.Sprintf("error while running %q prebind plugin for pod %q: %v", pl.Name(), pod.Name, status.Message()) - klog.Error(msg) - return framework.NewStatus(framework.Error, msg) + err := fmt.Errorf("error while running %q prebind plugin for pod %q: %w", pl.Name(), pod.Name, status.AsError()) + klog.Error(err) + return framework.AsStatus(err) } } return nil @@ -743,9 +743,9 @@ func (f *frameworkImpl) RunBindPlugins(ctx context.Context, state *framework.Cyc continue } if !status.IsSuccess() { - msg := fmt.Sprintf("plugin %q failed to bind pod \"%v/%v\": %v", bp.Name(), pod.Namespace, pod.Name, status.Message()) - klog.Error(msg) - return framework.NewStatus(framework.Error, msg) + err := fmt.Errorf("plugin %q failed to bind pod \"%v/%v\": %w", bp.Name(), pod.Namespace, pod.Name, status.AsError()) + klog.Error(err) + return framework.AsStatus(err) } return status } diff --git a/pkg/scheduler/framework/runtime/framework_test.go b/pkg/scheduler/framework/runtime/framework_test.go index aad04425c5c..d066e05bde8 100644 --- a/pkg/scheduler/framework/runtime/framework_test.go +++ b/pkg/scheduler/framework/runtime/framework_test.go @@ -18,6 +18,7 @@ package runtime import ( "context" + "errors" "fmt" "reflect" "strings" @@ -1138,6 +1139,7 @@ func TestPostFilterPlugins(t *testing.T) { } func TestPreBindPlugins(t *testing.T) { + injectedStatusErr := errors.New("injected status") tests := []struct { name string plugins []*TestPlugin @@ -1166,7 +1168,7 @@ func TestPreBindPlugins(t *testing.T) { inj: injectedResult{PreBindStatus: int(v1alpha1.Unschedulable)}, }, }, - wantStatus: v1alpha1.NewStatus(v1alpha1.Error, `error while running "TestPlugin" prebind plugin for pod "": injected status`), + wantStatus: v1alpha1.AsStatus(fmt.Errorf(`error while running "TestPlugin" prebind plugin for pod "": %w`, injectedStatusErr)), }, { name: "ErrorPreBindPlugin", @@ -1176,7 +1178,7 @@ func TestPreBindPlugins(t *testing.T) { inj: injectedResult{PreBindStatus: int(v1alpha1.Error)}, }, }, - wantStatus: v1alpha1.NewStatus(v1alpha1.Error, `error while running "TestPlugin" prebind plugin for pod "": injected status`), + wantStatus: v1alpha1.AsStatus(fmt.Errorf(`error while running "TestPlugin" prebind plugin for pod "": %w`, injectedStatusErr)), }, { name: "UnschedulablePreBindPlugin", @@ -1186,7 +1188,7 @@ func TestPreBindPlugins(t *testing.T) { inj: injectedResult{PreBindStatus: int(v1alpha1.UnschedulableAndUnresolvable)}, }, }, - wantStatus: v1alpha1.NewStatus(v1alpha1.Error, `error while running "TestPlugin" prebind plugin for pod "": injected status`), + wantStatus: v1alpha1.AsStatus(fmt.Errorf(`error while running "TestPlugin" prebind plugin for pod "": %w`, injectedStatusErr)), }, { name: "SuccessErrorPreBindPlugins", @@ -1200,7 +1202,7 @@ func TestPreBindPlugins(t *testing.T) { inj: injectedResult{PreBindStatus: int(v1alpha1.Error)}, }, }, - wantStatus: v1alpha1.NewStatus(v1alpha1.Error, `error while running "TestPlugin 1" prebind plugin for pod "": injected status`), + wantStatus: v1alpha1.AsStatus(fmt.Errorf(`error while running "TestPlugin 1" prebind plugin for pod "": %w`, injectedStatusErr)), }, { name: "ErrorSuccessPreBindPlugin", @@ -1214,7 +1216,7 @@ func TestPreBindPlugins(t *testing.T) { inj: injectedResult{PreBindStatus: int(v1alpha1.Success)}, }, }, - wantStatus: v1alpha1.NewStatus(v1alpha1.Error, `error while running "TestPlugin" prebind plugin for pod "": injected status`), + wantStatus: v1alpha1.AsStatus(fmt.Errorf(`error while running "TestPlugin" prebind plugin for pod "": %w`, injectedStatusErr)), }, { name: "SuccessSuccessPreBindPlugin", @@ -1242,7 +1244,7 @@ func TestPreBindPlugins(t *testing.T) { inj: injectedResult{PreBindStatus: int(v1alpha1.Error)}, }, }, - wantStatus: v1alpha1.NewStatus(v1alpha1.Error, `error while running "TestPlugin" prebind plugin for pod "": injected status`), + wantStatus: v1alpha1.AsStatus(fmt.Errorf(`error while running "TestPlugin" prebind plugin for pod "": %w`, injectedStatusErr)), }, { name: "UnschedulableAndSuccessPreBindPlugin", @@ -1256,7 +1258,7 @@ func TestPreBindPlugins(t *testing.T) { inj: injectedResult{PreBindStatus: int(v1alpha1.Success)}, }, }, - wantStatus: v1alpha1.NewStatus(v1alpha1.Error, `error while running "TestPlugin" prebind plugin for pod "": injected status`), + wantStatus: v1alpha1.AsStatus(fmt.Errorf(`error while running "TestPlugin" prebind plugin for pod "": %w`, injectedStatusErr)), }, } diff --git a/pkg/scheduler/framework/v1alpha1/interface.go b/pkg/scheduler/framework/v1alpha1/interface.go index 8c49bf0d60f..f7a82e1ca0f 100644 --- a/pkg/scheduler/framework/v1alpha1/interface.go +++ b/pkg/scheduler/framework/v1alpha1/interface.go @@ -92,12 +92,14 @@ const ( MaxTotalScore int64 = math.MaxInt64 ) -// Status indicates the result of running a plugin. It consists of a code and a -// message. When the status code is not `Success`, the reasons should explain why. +// Status indicates the result of running a plugin. It consists of a code, a +// message and (optionally) an error. When the status code is not `Success`, +// the reasons should explain why. // NOTE: A nil Status is also considered as Success. type Status struct { code Code reasons []string + err error } // Code returns code of the Status. @@ -143,15 +145,31 @@ func (s *Status) AsError() error { if s.IsSuccess() { return nil } + if s.err != nil { + return s.err + } return errors.New(s.Message()) } // NewStatus makes a Status out of the given arguments and returns its pointer. func NewStatus(code Code, reasons ...string) *Status { - return &Status{ + s := &Status{ code: code, reasons: reasons, } + if code == Error { + s.err = errors.New(s.Message()) + } + return s +} + +// AsStatus wraps an error in a Status. +func AsStatus(err error) *Status { + return &Status{ + code: Error, + reasons: []string{err.Error()}, + err: err, + } } // PluginToStatus maps plugin name to status. Currently used to identify which Filter plugin @@ -166,10 +184,10 @@ func (p PluginToStatus) Merge() *Status { } finalStatus := NewStatus(Success) - var hasError, hasUnschedulableAndUnresolvable, hasUnschedulable bool + var hasUnschedulableAndUnresolvable, hasUnschedulable bool for _, s := range p { if s.Code() == Error { - hasError = true + finalStatus.err = s.AsError() } else if s.Code() == UnschedulableAndUnresolvable { hasUnschedulableAndUnresolvable = true } else if s.Code() == Unschedulable { @@ -181,7 +199,7 @@ func (p PluginToStatus) Merge() *Status { } } - if hasError { + if finalStatus.err != nil { finalStatus.code = Error } else if hasUnschedulableAndUnresolvable { finalStatus.code = UnschedulableAndUnresolvable diff --git a/pkg/scheduler/scheduler_test.go b/pkg/scheduler/scheduler_test.go index 72b2312e8ac..24952f71f6c 100644 --- a/pkg/scheduler/scheduler_test.go +++ b/pkg/scheduler/scheduler_test.go @@ -210,6 +210,7 @@ func TestSchedulerScheduleOne(t *testing.T) { eventBroadcaster := events.NewBroadcaster(&events.EventSinkImpl{Interface: client.EventsV1()}) errS := errors.New("scheduler") errB := errors.New("binder") + preBindErr := errors.New("on PreBind") table := []struct { name string @@ -255,12 +256,12 @@ func TestSchedulerScheduleOne(t *testing.T) { sendPod: podWithID("foo", ""), algo: mockScheduler{core.ScheduleResult{SuggestedHost: testNode.Name, EvaluatedNodes: 1, FeasibleNodes: 1}, nil}, registerPluginFuncs: []st.RegisterPluginFunc{ - st.RegisterPreBindPlugin("FakePreBind", st.NewFakePreBindPlugin(framework.NewStatus(framework.Error, "prebind error"))), + st.RegisterPreBindPlugin("FakePreBind", st.NewFakePreBindPlugin(framework.AsStatus(preBindErr))), }, expectErrorPod: podWithID("foo", testNode.Name), expectForgetPod: podWithID("foo", testNode.Name), expectAssumedPod: podWithID("foo", testNode.Name), - expectError: errors.New(`error while running "FakePreBind" prebind plugin for pod "foo": prebind error`), + expectError: fmt.Errorf(`error while running "FakePreBind" prebind plugin for pod "foo": %w`, preBindErr), eventReason: "FailedScheduling", }, {