Skip to content

Commit 965137a

Browse files
authored
Merge pull request kubernetes#94692 from alculquicondor/wrap_errors_min
Wrap errors from VolumeBinding and DefaultBinder plugins
2 parents 3e8e0db + 7fb40fc commit 965137a

File tree

7 files changed

+60
-39
lines changed

7 files changed

+60
-39
lines changed

pkg/controller/volume/scheduling/scheduler_binder.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,7 @@ func (b *volumeBinder) BindPodVolumes(assumedPod *v1.Pod, podVolumes *PodVolumes
443443
return b, err
444444
})
445445
if err != nil {
446-
return fmt.Errorf("Failed to bind volumes: %v", err)
446+
return fmt.Errorf("binding volumes: %w", err)
447447
}
448448
return nil
449449
}
@@ -543,7 +543,7 @@ func (b *volumeBinder) checkBindings(pod *v1.Pod, bindings []*BindingInfo, claim
543543

544544
node, err := b.nodeLister.Get(pod.Spec.NodeName)
545545
if err != nil {
546-
return false, fmt.Errorf("failed to get node %q: %v", pod.Spec.NodeName, err)
546+
return false, fmt.Errorf("failed to get node %q: %w", pod.Spec.NodeName, err)
547547
}
548548

549549
csiNode, err := b.csiNodeLister.Get(node.Name)
@@ -559,20 +559,20 @@ func (b *volumeBinder) checkBindings(pod *v1.Pod, bindings []*BindingInfo, claim
559559
_, err = b.podLister.Pods(pod.Namespace).Get(pod.Name)
560560
if err != nil {
561561
if apierrors.IsNotFound(err) {
562-
return false, fmt.Errorf("pod %q does not exist any more", podName)
562+
return false, fmt.Errorf("pod does not exist any more: %w", err)
563563
}
564564
klog.Errorf("failed to get pod %s/%s from the lister: %v", pod.Namespace, pod.Name, err)
565565
}
566566

567567
for _, binding := range bindings {
568568
pv, err := b.pvCache.GetAPIPV(binding.pv.Name)
569569
if err != nil {
570-
return false, fmt.Errorf("failed to check binding: %v", err)
570+
return false, fmt.Errorf("failed to check binding: %w", err)
571571
}
572572

573573
pvc, err := b.pvcCache.GetAPIPVC(getPVCName(binding.pvc))
574574
if err != nil {
575-
return false, fmt.Errorf("failed to check binding: %v", err)
575+
return false, fmt.Errorf("failed to check binding: %w", err)
576576
}
577577

578578
// 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
583583

584584
pv, err = b.tryTranslatePVToCSI(pv, csiNode)
585585
if err != nil {
586-
return false, fmt.Errorf("failed to translate pv to csi: %v", err)
586+
return false, fmt.Errorf("failed to translate pv to csi: %w", err)
587587
}
588588

589589
// Check PV's node affinity (the node might not have the proper label)
590590
if err := volumeutil.CheckNodeAffinity(pv, node.Labels); err != nil {
591-
return false, fmt.Errorf("pv %q node affinity doesn't match node %q: %v", pv.Name, node.Name, err)
591+
return false, fmt.Errorf("pv %q node affinity doesn't match node %q: %w", pv.Name, node.Name, err)
592592
}
593593

594594
// Check if pv.ClaimRef got dropped by unbindVolume()
@@ -605,7 +605,7 @@ func (b *volumeBinder) checkBindings(pod *v1.Pod, bindings []*BindingInfo, claim
605605
for _, claim := range claimsToProvision {
606606
pvc, err := b.pvcCache.GetAPIPVC(getPVCName(claim))
607607
if err != nil {
608-
return false, fmt.Errorf("failed to check provisioning pvc: %v", err)
608+
return false, fmt.Errorf("failed to check provisioning pvc: %w", err)
609609
}
610610

611611
// 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
637637
// be unbound eventually.
638638
return false, nil
639639
}
640-
return false, fmt.Errorf("failed to get pv %q from cache: %v", pvc.Spec.VolumeName, err)
640+
return false, fmt.Errorf("failed to get pv %q from cache: %w", pvc.Spec.VolumeName, err)
641641
}
642642

643643
pv, err = b.tryTranslatePVToCSI(pv, csiNode)
@@ -646,7 +646,7 @@ func (b *volumeBinder) checkBindings(pod *v1.Pod, bindings []*BindingInfo, claim
646646
}
647647

648648
if err := volumeutil.CheckNodeAffinity(pv, node.Labels); err != nil {
649-
return false, fmt.Errorf("pv %q node affinity doesn't match node %q: %v", pv.Name, node.Name, err)
649+
return false, fmt.Errorf("pv %q node affinity doesn't match node %q: %w", pv.Name, node.Name, err)
650650
}
651651
}
652652

pkg/scheduler/framework/plugins/defaultbinder/default_binder.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func (b DefaultBinder) Bind(ctx context.Context, state *framework.CycleState, p
5555
}
5656
err := b.handle.ClientSet().CoreV1().Pods(binding.Namespace).Bind(ctx, binding, metav1.CreateOptions{})
5757
if err != nil {
58-
return framework.NewStatus(framework.Error, err.Error())
58+
return framework.AsStatus(err)
5959
}
6060
return nil
6161
}

pkg/scheduler/framework/plugins/volumebinding/volume_binding.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ func (pl *VolumeBinding) PreFilter(ctx context.Context, state *framework.CycleSt
130130
}
131131
boundClaims, claimsToBind, unboundClaimsImmediate, err := pl.Binder.GetPodVolumes(pod)
132132
if err != nil {
133-
return framework.NewStatus(framework.Error, err.Error())
133+
return framework.AsStatus(err)
134134
}
135135
if len(unboundClaimsImmediate) > 0 {
136136
// Return UnschedulableAndUnresolvable error if immediate claims are
@@ -184,7 +184,7 @@ func (pl *VolumeBinding) Filter(ctx context.Context, cs *framework.CycleState, p
184184

185185
state, err := getStateData(cs)
186186
if err != nil {
187-
return framework.NewStatus(framework.Error, err.Error())
187+
return framework.AsStatus(err)
188188
}
189189

190190
if state.skip {
@@ -215,14 +215,14 @@ func (pl *VolumeBinding) Filter(ctx context.Context, cs *framework.CycleState, p
215215
func (pl *VolumeBinding) Reserve(ctx context.Context, cs *framework.CycleState, pod *v1.Pod, nodeName string) *framework.Status {
216216
state, err := getStateData(cs)
217217
if err != nil {
218-
return framework.NewStatus(framework.Error, err.Error())
218+
return framework.AsStatus(err)
219219
}
220220
// we don't need to hold the lock as only one node will be reserved for the given pod
221221
podVolumes, ok := state.podVolumesByNode[nodeName]
222222
if ok {
223223
allBound, err := pl.Binder.AssumePodVolumes(pod, nodeName, podVolumes)
224224
if err != nil {
225-
return framework.NewStatus(framework.Error, err.Error())
225+
return framework.AsStatus(err)
226226
}
227227
state.allBound = allBound
228228
} else {
@@ -240,7 +240,7 @@ func (pl *VolumeBinding) Reserve(ctx context.Context, cs *framework.CycleState,
240240
func (pl *VolumeBinding) PreBind(ctx context.Context, cs *framework.CycleState, pod *v1.Pod, nodeName string) *framework.Status {
241241
s, err := getStateData(cs)
242242
if err != nil {
243-
return framework.NewStatus(framework.Error, err.Error())
243+
return framework.AsStatus(err)
244244
}
245245
if s.allBound {
246246
// no need to bind volumes
@@ -249,13 +249,13 @@ func (pl *VolumeBinding) PreBind(ctx context.Context, cs *framework.CycleState,
249249
// we don't need to hold the lock as only one node will be pre-bound for the given pod
250250
podVolumes, ok := s.podVolumesByNode[nodeName]
251251
if !ok {
252-
return framework.NewStatus(framework.Error, fmt.Sprintf("no pod volumes found for node %q", nodeName))
252+
return framework.AsStatus(fmt.Errorf("no pod volumes found for node %q", nodeName))
253253
}
254254
klog.V(5).Infof("Trying to bind volumes for pod \"%v/%v\"", pod.Namespace, pod.Name)
255255
err = pl.Binder.BindPodVolumes(pod, podVolumes)
256256
if err != nil {
257257
klog.V(1).Infof("Failed to bind volumes for pod \"%v/%v\": %v", pod.Namespace, pod.Name, err)
258-
return framework.NewStatus(framework.Error, err.Error())
258+
return framework.AsStatus(err)
259259
}
260260
klog.V(5).Infof("Success binding volumes for pod \"%v/%v\"", pod.Namespace, pod.Name)
261261
return nil

pkg/scheduler/framework/runtime/framework.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -710,9 +710,9 @@ func (f *frameworkImpl) RunPreBindPlugins(ctx context.Context, state *framework.
710710
for _, pl := range f.preBindPlugins {
711711
status = f.runPreBindPlugin(ctx, pl, state, pod, nodeName)
712712
if !status.IsSuccess() {
713-
msg := fmt.Sprintf("error while running %q prebind plugin for pod %q: %v", pl.Name(), pod.Name, status.Message())
714-
klog.Error(msg)
715-
return framework.NewStatus(framework.Error, msg)
713+
err := fmt.Errorf("error while running %q prebind plugin for pod %q: %w", pl.Name(), pod.Name, status.AsError())
714+
klog.Error(err)
715+
return framework.AsStatus(err)
716716
}
717717
}
718718
return nil
@@ -743,9 +743,9 @@ func (f *frameworkImpl) RunBindPlugins(ctx context.Context, state *framework.Cyc
743743
continue
744744
}
745745
if !status.IsSuccess() {
746-
msg := fmt.Sprintf("plugin %q failed to bind pod \"%v/%v\": %v", bp.Name(), pod.Namespace, pod.Name, status.Message())
747-
klog.Error(msg)
748-
return framework.NewStatus(framework.Error, msg)
746+
err := fmt.Errorf("plugin %q failed to bind pod \"%v/%v\": %w", bp.Name(), pod.Namespace, pod.Name, status.AsError())
747+
klog.Error(err)
748+
return framework.AsStatus(err)
749749
}
750750
return status
751751
}

pkg/scheduler/framework/runtime/framework_test.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package runtime
1818

1919
import (
2020
"context"
21+
"errors"
2122
"fmt"
2223
"reflect"
2324
"strings"
@@ -1138,6 +1139,7 @@ func TestPostFilterPlugins(t *testing.T) {
11381139
}
11391140

11401141
func TestPreBindPlugins(t *testing.T) {
1142+
injectedStatusErr := errors.New("injected status")
11411143
tests := []struct {
11421144
name string
11431145
plugins []*TestPlugin
@@ -1166,7 +1168,7 @@ func TestPreBindPlugins(t *testing.T) {
11661168
inj: injectedResult{PreBindStatus: int(v1alpha1.Unschedulable)},
11671169
},
11681170
},
1169-
wantStatus: v1alpha1.NewStatus(v1alpha1.Error, `error while running "TestPlugin" prebind plugin for pod "": injected status`),
1171+
wantStatus: v1alpha1.AsStatus(fmt.Errorf(`error while running "TestPlugin" prebind plugin for pod "": %w`, injectedStatusErr)),
11701172
},
11711173
{
11721174
name: "ErrorPreBindPlugin",
@@ -1176,7 +1178,7 @@ func TestPreBindPlugins(t *testing.T) {
11761178
inj: injectedResult{PreBindStatus: int(v1alpha1.Error)},
11771179
},
11781180
},
1179-
wantStatus: v1alpha1.NewStatus(v1alpha1.Error, `error while running "TestPlugin" prebind plugin for pod "": injected status`),
1181+
wantStatus: v1alpha1.AsStatus(fmt.Errorf(`error while running "TestPlugin" prebind plugin for pod "": %w`, injectedStatusErr)),
11801182
},
11811183
{
11821184
name: "UnschedulablePreBindPlugin",
@@ -1186,7 +1188,7 @@ func TestPreBindPlugins(t *testing.T) {
11861188
inj: injectedResult{PreBindStatus: int(v1alpha1.UnschedulableAndUnresolvable)},
11871189
},
11881190
},
1189-
wantStatus: v1alpha1.NewStatus(v1alpha1.Error, `error while running "TestPlugin" prebind plugin for pod "": injected status`),
1191+
wantStatus: v1alpha1.AsStatus(fmt.Errorf(`error while running "TestPlugin" prebind plugin for pod "": %w`, injectedStatusErr)),
11901192
},
11911193
{
11921194
name: "SuccessErrorPreBindPlugins",
@@ -1200,7 +1202,7 @@ func TestPreBindPlugins(t *testing.T) {
12001202
inj: injectedResult{PreBindStatus: int(v1alpha1.Error)},
12011203
},
12021204
},
1203-
wantStatus: v1alpha1.NewStatus(v1alpha1.Error, `error while running "TestPlugin 1" prebind plugin for pod "": injected status`),
1205+
wantStatus: v1alpha1.AsStatus(fmt.Errorf(`error while running "TestPlugin 1" prebind plugin for pod "": %w`, injectedStatusErr)),
12041206
},
12051207
{
12061208
name: "ErrorSuccessPreBindPlugin",
@@ -1214,7 +1216,7 @@ func TestPreBindPlugins(t *testing.T) {
12141216
inj: injectedResult{PreBindStatus: int(v1alpha1.Success)},
12151217
},
12161218
},
1217-
wantStatus: v1alpha1.NewStatus(v1alpha1.Error, `error while running "TestPlugin" prebind plugin for pod "": injected status`),
1219+
wantStatus: v1alpha1.AsStatus(fmt.Errorf(`error while running "TestPlugin" prebind plugin for pod "": %w`, injectedStatusErr)),
12181220
},
12191221
{
12201222
name: "SuccessSuccessPreBindPlugin",
@@ -1242,7 +1244,7 @@ func TestPreBindPlugins(t *testing.T) {
12421244
inj: injectedResult{PreBindStatus: int(v1alpha1.Error)},
12431245
},
12441246
},
1245-
wantStatus: v1alpha1.NewStatus(v1alpha1.Error, `error while running "TestPlugin" prebind plugin for pod "": injected status`),
1247+
wantStatus: v1alpha1.AsStatus(fmt.Errorf(`error while running "TestPlugin" prebind plugin for pod "": %w`, injectedStatusErr)),
12461248
},
12471249
{
12481250
name: "UnschedulableAndSuccessPreBindPlugin",
@@ -1256,7 +1258,7 @@ func TestPreBindPlugins(t *testing.T) {
12561258
inj: injectedResult{PreBindStatus: int(v1alpha1.Success)},
12571259
},
12581260
},
1259-
wantStatus: v1alpha1.NewStatus(v1alpha1.Error, `error while running "TestPlugin" prebind plugin for pod "": injected status`),
1261+
wantStatus: v1alpha1.AsStatus(fmt.Errorf(`error while running "TestPlugin" prebind plugin for pod "": %w`, injectedStatusErr)),
12601262
},
12611263
}
12621264

pkg/scheduler/framework/v1alpha1/interface.go

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,14 @@ const (
9292
MaxTotalScore int64 = math.MaxInt64
9393
)
9494

95-
// Status indicates the result of running a plugin. It consists of a code and a
96-
// message. When the status code is not `Success`, the reasons should explain why.
95+
// Status indicates the result of running a plugin. It consists of a code, a
96+
// message and (optionally) an error. When the status code is not `Success`,
97+
// the reasons should explain why.
9798
// NOTE: A nil Status is also considered as Success.
9899
type Status struct {
99100
code Code
100101
reasons []string
102+
err error
101103
}
102104

103105
// Code returns code of the Status.
@@ -143,15 +145,31 @@ func (s *Status) AsError() error {
143145
if s.IsSuccess() {
144146
return nil
145147
}
148+
if s.err != nil {
149+
return s.err
150+
}
146151
return errors.New(s.Message())
147152
}
148153

149154
// NewStatus makes a Status out of the given arguments and returns its pointer.
150155
func NewStatus(code Code, reasons ...string) *Status {
151-
return &Status{
156+
s := &Status{
152157
code: code,
153158
reasons: reasons,
154159
}
160+
if code == Error {
161+
s.err = errors.New(s.Message())
162+
}
163+
return s
164+
}
165+
166+
// AsStatus wraps an error in a Status.
167+
func AsStatus(err error) *Status {
168+
return &Status{
169+
code: Error,
170+
reasons: []string{err.Error()},
171+
err: err,
172+
}
155173
}
156174

157175
// PluginToStatus maps plugin name to status. Currently used to identify which Filter plugin
@@ -166,10 +184,10 @@ func (p PluginToStatus) Merge() *Status {
166184
}
167185

168186
finalStatus := NewStatus(Success)
169-
var hasError, hasUnschedulableAndUnresolvable, hasUnschedulable bool
187+
var hasUnschedulableAndUnresolvable, hasUnschedulable bool
170188
for _, s := range p {
171189
if s.Code() == Error {
172-
hasError = true
190+
finalStatus.err = s.AsError()
173191
} else if s.Code() == UnschedulableAndUnresolvable {
174192
hasUnschedulableAndUnresolvable = true
175193
} else if s.Code() == Unschedulable {
@@ -181,7 +199,7 @@ func (p PluginToStatus) Merge() *Status {
181199
}
182200
}
183201

184-
if hasError {
202+
if finalStatus.err != nil {
185203
finalStatus.code = Error
186204
} else if hasUnschedulableAndUnresolvable {
187205
finalStatus.code = UnschedulableAndUnresolvable

pkg/scheduler/scheduler_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@ func TestSchedulerScheduleOne(t *testing.T) {
210210
eventBroadcaster := events.NewBroadcaster(&events.EventSinkImpl{Interface: client.EventsV1()})
211211
errS := errors.New("scheduler")
212212
errB := errors.New("binder")
213+
preBindErr := errors.New("on PreBind")
213214

214215
table := []struct {
215216
name string
@@ -255,12 +256,12 @@ func TestSchedulerScheduleOne(t *testing.T) {
255256
sendPod: podWithID("foo", ""),
256257
algo: mockScheduler{core.ScheduleResult{SuggestedHost: testNode.Name, EvaluatedNodes: 1, FeasibleNodes: 1}, nil},
257258
registerPluginFuncs: []st.RegisterPluginFunc{
258-
st.RegisterPreBindPlugin("FakePreBind", st.NewFakePreBindPlugin(framework.NewStatus(framework.Error, "prebind error"))),
259+
st.RegisterPreBindPlugin("FakePreBind", st.NewFakePreBindPlugin(framework.AsStatus(preBindErr))),
259260
},
260261
expectErrorPod: podWithID("foo", testNode.Name),
261262
expectForgetPod: podWithID("foo", testNode.Name),
262263
expectAssumedPod: podWithID("foo", testNode.Name),
263-
expectError: errors.New(`error while running "FakePreBind" prebind plugin for pod "foo": prebind error`),
264+
expectError: fmt.Errorf(`error while running "FakePreBind" prebind plugin for pod "foo": %w`, preBindErr),
264265
eventReason: "FailedScheduling",
265266
},
266267
{

0 commit comments

Comments
 (0)