From 66fa7b411e84a568d17ff4d5f1e1d78814658a47 Mon Sep 17 00:00:00 2001 From: Abdullah Gharaibeh Date: Tue, 25 May 2021 18:58:11 -0400 Subject: [PATCH 1/2] Return UnschedulableAndUnresolvable instead of Error when failing to lookup pvc or storageclass in VolumeZone plugin --- .../plugins/volumezone/volume_zone.go | 26 +++++++------------ .../plugins/volumezone/volume_zone_test.go | 8 +++--- 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/pkg/scheduler/framework/plugins/volumezone/volume_zone.go b/pkg/scheduler/framework/plugins/volumezone/volume_zone.go index c62b2ea3f73..023b52259f3 100644 --- a/pkg/scheduler/framework/plugins/volumezone/volume_zone.go +++ b/pkg/scheduler/framework/plugins/volumezone/volume_zone.go @@ -108,47 +108,39 @@ func (pl *VolumeZone) Filter(ctx context.Context, _ *framework.CycleState, pod * } pvcName := volume.PersistentVolumeClaim.ClaimName if pvcName == "" { - return framework.NewStatus(framework.Error, "PersistentVolumeClaim had no name") + return framework.NewStatus(framework.UnschedulableAndUnresolvable, "PersistentVolumeClaim had no name") } pvc, err := pl.pvcLister.PersistentVolumeClaims(pod.Namespace).Get(pvcName) if err != nil { - return framework.NewStatus(framework.Error, err.Error()) - } - - if pvc == nil { - return framework.NewStatus(framework.Error, fmt.Sprintf("PersistentVolumeClaim was not found: %q", pvcName)) + return framework.NewStatus(framework.UnschedulableAndUnresolvable, err.Error()) } pvName := pvc.Spec.VolumeName if pvName == "" { scName := v1helper.GetPersistentVolumeClaimClass(pvc) if len(scName) == 0 { - return framework.NewStatus(framework.Error, fmt.Sprint("PersistentVolumeClaim had no pv name and storageClass name")) + return framework.NewStatus(framework.UnschedulableAndUnresolvable, "PersistentVolumeClaim had no pv name and storageClass name") } - class, _ := pl.scLister.Get(scName) - if class == nil { - return framework.NewStatus(framework.Error, fmt.Sprintf("StorageClass %q claimed by PersistentVolumeClaim %q not found", scName, pvcName)) + class, err := pl.scLister.Get(scName) + if err != nil { + return framework.NewStatus(framework.UnschedulableAndUnresolvable, err.Error()) } if class.VolumeBindingMode == nil { - return framework.NewStatus(framework.Error, fmt.Sprintf("VolumeBindingMode not set for StorageClass %q", scName)) + return framework.NewStatus(framework.UnschedulableAndUnresolvable, fmt.Sprintf("VolumeBindingMode not set for StorageClass %q", scName)) } if *class.VolumeBindingMode == storage.VolumeBindingWaitForFirstConsumer { // Skip unbound volumes continue } - return framework.NewStatus(framework.Error, fmt.Sprint("PersistentVolume had no name")) + return framework.NewStatus(framework.UnschedulableAndUnresolvable, "PersistentVolume had no name") } pv, err := pl.pvLister.Get(pvName) if err != nil { - return framework.NewStatus(framework.Error, err.Error()) - } - - if pv == nil { - return framework.NewStatus(framework.Error, fmt.Sprintf("PersistentVolume was not found: %q", pvName)) + return framework.NewStatus(framework.UnschedulableAndUnresolvable, err.Error()) } for k, v := range pv.ObjectMeta.Labels { diff --git a/pkg/scheduler/framework/plugins/volumezone/volume_zone_test.go b/pkg/scheduler/framework/plugins/volumezone/volume_zone_test.go index 2c6deaa70e5..699b8846558 100644 --- a/pkg/scheduler/framework/plugins/volumezone/volume_zone_test.go +++ b/pkg/scheduler/framework/plugins/volumezone/volume_zone_test.go @@ -413,21 +413,21 @@ func TestWithBinding(t *testing.T) { name: "unbound volume empty storage class", Pod: createPodWithVolume("pod_1", "vol_1", "PVC_EmptySC"), Node: testNode, - wantStatus: framework.NewStatus(framework.Error, + wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, "PersistentVolumeClaim had no pv name and storageClass name"), }, { name: "unbound volume no storage class", Pod: createPodWithVolume("pod_1", "vol_1", "PVC_NoSC"), Node: testNode, - wantStatus: framework.NewStatus(framework.Error, - "StorageClass \"Class_0\" claimed by PersistentVolumeClaim \"PVC_NoSC\" not found"), + wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, + "Unable to find storage class: Class_0"), }, { name: "unbound volume immediate binding mode", Pod: createPodWithVolume("pod_1", "vol_1", "PVC_ImmediateSC"), Node: testNode, - wantStatus: framework.NewStatus(framework.Error, "VolumeBindingMode not set for StorageClass \"Class_Immediate\""), + wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, "VolumeBindingMode not set for StorageClass \"Class_Immediate\""), }, { name: "unbound volume wait binding mode", From 369eafa479a3d8510b33a0f47e73dc060e938ca3 Mon Sep 17 00:00:00 2001 From: Abdullah Gharaibeh Date: Mon, 31 May 2021 22:44:34 -0400 Subject: [PATCH 2/2] Return UnschedulableAndUnresolvable when looking up volume-related resources returns NotFound error --- .../framework/plugins/volumezone/BUILD | 1 + .../plugins/volumezone/volume_zone.go | 24 +++++++++++++------ .../plugins/volumezone/volume_zone_test.go | 2 +- pkg/scheduler/framework/v1alpha1/fake/BUILD | 1 + .../framework/v1alpha1/fake/listers.go | 8 ++++++- 5 files changed, 27 insertions(+), 9 deletions(-) diff --git a/pkg/scheduler/framework/plugins/volumezone/BUILD b/pkg/scheduler/framework/plugins/volumezone/BUILD index 762d6b72f28..9a8830717da 100644 --- a/pkg/scheduler/framework/plugins/volumezone/BUILD +++ b/pkg/scheduler/framework/plugins/volumezone/BUILD @@ -10,6 +10,7 @@ go_library( "//pkg/scheduler/framework/v1alpha1:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/api/storage/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/client-go/listers/core/v1:go_default_library", diff --git a/pkg/scheduler/framework/plugins/volumezone/volume_zone.go b/pkg/scheduler/framework/plugins/volumezone/volume_zone.go index 023b52259f3..b498ccc6c65 100644 --- a/pkg/scheduler/framework/plugins/volumezone/volume_zone.go +++ b/pkg/scheduler/framework/plugins/volumezone/volume_zone.go @@ -22,6 +22,7 @@ import ( v1 "k8s.io/api/core/v1" storage "k8s.io/api/storage/v1" + "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" corelisters "k8s.io/client-go/listers/core/v1" @@ -111,8 +112,8 @@ func (pl *VolumeZone) Filter(ctx context.Context, _ *framework.CycleState, pod * return framework.NewStatus(framework.UnschedulableAndUnresolvable, "PersistentVolumeClaim had no name") } pvc, err := pl.pvcLister.PersistentVolumeClaims(pod.Namespace).Get(pvcName) - if err != nil { - return framework.NewStatus(framework.UnschedulableAndUnresolvable, err.Error()) + if s := getErrorAsStatus(err); !s.IsSuccess() { + return s } pvName := pvc.Spec.VolumeName @@ -123,9 +124,8 @@ func (pl *VolumeZone) Filter(ctx context.Context, _ *framework.CycleState, pod * } class, err := pl.scLister.Get(scName) - if err != nil { - return framework.NewStatus(framework.UnschedulableAndUnresolvable, err.Error()) - + if s := getErrorAsStatus(err); !s.IsSuccess() { + return s } if class.VolumeBindingMode == nil { return framework.NewStatus(framework.UnschedulableAndUnresolvable, fmt.Sprintf("VolumeBindingMode not set for StorageClass %q", scName)) @@ -139,8 +139,8 @@ func (pl *VolumeZone) Filter(ctx context.Context, _ *framework.CycleState, pod * } pv, err := pl.pvLister.Get(pvName) - if err != nil { - return framework.NewStatus(framework.UnschedulableAndUnresolvable, err.Error()) + if s := getErrorAsStatus(err); !s.IsSuccess() { + return s } for k, v := range pv.ObjectMeta.Labels { @@ -163,6 +163,16 @@ func (pl *VolumeZone) Filter(ctx context.Context, _ *framework.CycleState, pod * return nil } +func getErrorAsStatus(err error) *framework.Status { + if err != nil { + if errors.IsNotFound(err) { + return framework.NewStatus(framework.UnschedulableAndUnresolvable, err.Error()) + } + return framework.NewStatus(framework.Error, err.Error()) + } + return nil +} + // New initializes a new plugin and returns it. func New(_ runtime.Object, handle framework.FrameworkHandle) (framework.Plugin, error) { informerFactory := handle.SharedInformerFactory() diff --git a/pkg/scheduler/framework/plugins/volumezone/volume_zone_test.go b/pkg/scheduler/framework/plugins/volumezone/volume_zone_test.go index 699b8846558..dda41495830 100644 --- a/pkg/scheduler/framework/plugins/volumezone/volume_zone_test.go +++ b/pkg/scheduler/framework/plugins/volumezone/volume_zone_test.go @@ -421,7 +421,7 @@ func TestWithBinding(t *testing.T) { Pod: createPodWithVolume("pod_1", "vol_1", "PVC_NoSC"), Node: testNode, wantStatus: framework.NewStatus(framework.UnschedulableAndUnresolvable, - "Unable to find storage class: Class_0"), + "unable to find storage class: Class_0"), }, { name: "unbound volume immediate binding mode", diff --git a/pkg/scheduler/framework/v1alpha1/fake/BUILD b/pkg/scheduler/framework/v1alpha1/fake/BUILD index c9167c7e5e0..231cc4a2c81 100644 --- a/pkg/scheduler/framework/v1alpha1/fake/BUILD +++ b/pkg/scheduler/framework/v1alpha1/fake/BUILD @@ -10,6 +10,7 @@ go_library( "//staging/src/k8s.io/api/apps/v1:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/api/storage/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", "//staging/src/k8s.io/client-go/listers/apps/v1:go_default_library", diff --git a/pkg/scheduler/framework/v1alpha1/fake/listers.go b/pkg/scheduler/framework/v1alpha1/fake/listers.go index 863b595cbe6..89aeb9b82c9 100644 --- a/pkg/scheduler/framework/v1alpha1/fake/listers.go +++ b/pkg/scheduler/framework/v1alpha1/fake/listers.go @@ -22,6 +22,7 @@ import ( appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" appslisters "k8s.io/client-go/listers/apps/v1" @@ -311,7 +312,12 @@ func (classes StorageClassLister) Get(name string) (*storagev1.StorageClass, err return &sc, nil } } - return nil, fmt.Errorf("Unable to find storage class: %s", name) + return nil, &errors.StatusError{ + ErrStatus: metav1.Status{ + Reason: metav1.StatusReasonNotFound, + Message: fmt.Sprintf("unable to find storage class: %s", name), + }, + } } // List lists all StorageClass in the indexer.