Cleanup FailureReason in scheduler - part 1

- Plugin returns 'framework.Status' instead of '[]FailureReason'
kubernetes · Dec 20, 2019 · 3cd6276 · 3cd6276
1 parent 449fe0b
commit 3cd6276
Show file tree

Hide file tree

Showing 40 changed files with 426 additions and 426 deletions.
diff --git a/pkg/scheduler/algorithm/predicates/BUILD b/pkg/scheduler/algorithm/predicates/BUILD
@@ -21,6 +21,8 @@ go_library(
         "//pkg/features:go_default_library",
         "//pkg/scheduler/algorithm:go_default_library",
         "//pkg/scheduler/algorithm/priorities/util:go_default_library",
+        "//pkg/scheduler/framework/plugins/migration:go_default_library",
+        "//pkg/scheduler/framework/v1alpha1:go_default_library",
         "//pkg/scheduler/listers:go_default_library",
         "//pkg/scheduler/nodeinfo:go_default_library",
         "//pkg/scheduler/util:go_default_library",
@@ -58,6 +60,7 @@ go_test(
         "//pkg/apis/core:go_default_library",
         "//pkg/apis/core/v1/helper:go_default_library",
         "//pkg/features:go_default_library",
+        "//pkg/scheduler/framework/v1alpha1:go_default_library",
         "//pkg/scheduler/listers/fake:go_default_library",
         "//pkg/scheduler/nodeinfo:go_default_library",
         "//pkg/scheduler/nodeinfo/snapshot:go_default_library",

diff --git a/pkg/scheduler/algorithm/predicates/csi_volume_predicate.go b/pkg/scheduler/algorithm/predicates/csi_volume_predicate.go
@@ -27,6 +27,7 @@ import (
 	csitrans "k8s.io/csi-translation-lib"
 	"k8s.io/klog"
 	v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper"
+	framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1"
 	schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo"
 	volumeutil "k8s.io/kubernetes/pkg/volume/util"
 )
@@ -55,7 +56,7 @@ type CSIMaxVolumeLimitChecker struct {
 
 // NewCSIMaxVolumeLimitPredicate returns a predicate for counting CSI volumes
 func NewCSIMaxVolumeLimitPredicate(
-	csiNodeLister storagelisters.CSINodeLister, pvLister corelisters.PersistentVolumeLister, pvcLister corelisters.PersistentVolumeClaimLister, scLister storagelisters.StorageClassLister) FitPredicate {
+	csiNodeLister storagelisters.CSINodeLister, pvLister corelisters.PersistentVolumeLister, pvcLister corelisters.PersistentVolumeClaimLister, scLister storagelisters.StorageClassLister) FilterFn {
 	c := &CSIMaxVolumeLimitChecker{
 		csiNodeLister:        csiNodeLister,
 		pvLister:             pvLister,
@@ -84,7 +85,7 @@ func getVolumeLimits(nodeInfo *schedulernodeinfo.NodeInfo, csiNode *storagev1.CS
 }
 
 func (c *CSIMaxVolumeLimitChecker) attachableLimitPredicate(
-	pod *v1.Pod, meta Metadata, nodeInfo *schedulernodeinfo.NodeInfo) (bool, []PredicateFailureReason, error) {
+	pod *v1.Pod, nodeInfo *schedulernodeinfo.NodeInfo) (bool, *framework.Status, error) {
 	// If the new pod doesn't have any volume attached to it, the predicate will always be true
 	if len(pod.Spec.Volumes) == 0 {
 		return true, nil, nil
@@ -144,7 +145,7 @@ func (c *CSIMaxVolumeLimitChecker) attachableLimitPredicate(
 		if ok {
 			currentVolumeCount := attachedVolumeCount[volumeLimitKey]
 			if currentVolumeCount+count > int(maxVolumeLimit) {
-				return false, []PredicateFailureReason{ErrMaxVolumeCountExceeded}, nil
+				return false, framework.NewStatus(framework.Unschedulable, ErrMaxVolumeCountExceeded.GetReason()), nil
 			}
 		}
 	}

diff --git a/pkg/scheduler/algorithm/predicates/csi_volume_predicate_test.go b/pkg/scheduler/algorithm/predicates/csi_volume_predicate_test.go
@@ -30,6 +30,7 @@ import (
 	featuregatetesting "k8s.io/component-base/featuregate/testing"
 	csilibplugins "k8s.io/csi-translation-lib/plugins"
 	"k8s.io/kubernetes/pkg/features"
+	framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1"
 	fakelisters "k8s.io/kubernetes/pkg/scheduler/listers/fake"
 )
 
@@ -214,16 +215,15 @@ func TestCSIVolumeCountPredicate(t *testing.T) {
 	}
 
 	tests := []struct {
-		newPod                *v1.Pod
-		existingPods          []*v1.Pod
-		filterName            string
-		maxVols               int
-		driverNames           []string
-		fits                  bool
-		test                  string
-		migrationEnabled      bool
-		limitSource           string
-		expectedFailureReason *PredicateFailureError
+		newPod           *v1.Pod
+		existingPods     []*v1.Pod
+		filterName       string
+		maxVols          int
+		driverNames      []string
+		fits             bool
+		test             string
+		migrationEnabled bool
+		limitSource      string
 	}{
 		{
 			newPod:       csiEBSOneVolPod,
@@ -447,22 +447,19 @@ func TestCSIVolumeCountPredicate(t *testing.T) {
 				defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationAWS, false)()
 			}
 
-			expectedFailureReasons := []PredicateFailureReason{ErrMaxVolumeCountExceeded}
-			if test.expectedFailureReason != nil {
-				expectedFailureReasons = []PredicateFailureReason{test.expectedFailureReason}
-			}
+			expectedStatus := framework.NewStatus(framework.Unschedulable, ErrMaxVolumeCountExceeded.GetReason())
 
 			pred := NewCSIMaxVolumeLimitPredicate(getFakeCSINodeLister(csiNode),
 				getFakeCSIPVLister(test.filterName, test.driverNames...),
 				getFakeCSIPVCLister(test.filterName, "csi-sc", test.driverNames...),
 				getFakeCSIStorageClassLister("csi-sc", test.driverNames[0]))
 
-			fits, reasons, err := pred(test.newPod, nil, node)
+			fits, gotStatus, err := pred(test.newPod, node)
 			if err != nil {
 				t.Errorf("Using allocatable [%s]%s: unexpected error: %v", test.filterName, test.test, err)
 			}
-			if !fits && !reflect.DeepEqual(expectedFailureReasons, reasons) {
-				t.Errorf("Using allocatable [%s]%s: unexpected failure reasons: %v, want: %v", test.filterName, test.test, reasons, expectedFailureReasons)
+			if !fits && !reflect.DeepEqual(expectedStatus, gotStatus) {
+				t.Errorf("Using allocatable [%s]%s: unexpected status: %v, want: %v", test.filterName, test.test, gotStatus, expectedStatus)
 			}
 			if fits != test.fits {
 				t.Errorf("Using allocatable [%s]%s: expected %v, got %v", test.filterName, test.test, test.fits, fits)

diff --git a/pkg/scheduler/algorithm/predicates/error.go b/pkg/scheduler/algorithm/predicates/error.go
@@ -82,6 +82,7 @@ var (
 	ErrFakePredicate = NewPredicateFailureError("FakePredicateError", "Nodes failed the fake predicate")
 )
 
+// TODO(Huang-Wei): remove this.
 var unresolvablePredicateFailureErrors = map[PredicateFailureReason]struct{}{
 	ErrNodeSelectorNotMatch:      {},
 	ErrPodAffinityRulesNotMatch:  {},
@@ -104,6 +105,7 @@ var unresolvablePredicateFailureErrors = map[PredicateFailureReason]struct{}{
 }
 
 // UnresolvablePredicateExists checks if there is at least one unresolvable predicate failure reason.
+// DEPRECATED
 func UnresolvablePredicateExists(reasons []PredicateFailureReason) bool {
 	for _, r := range reasons {
 		if _, ok := unresolvablePredicateFailureErrors[r]; ok {

diff --git a/pkg/scheduler/algorithm/predicates/max_attachable_volume_predicate_test.go b/pkg/scheduler/algorithm/predicates/max_attachable_volume_predicate_test.go
@@ -27,6 +27,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	csilibplugins "k8s.io/csi-translation-lib/plugins"
+	framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1"
 	fakelisters "k8s.io/kubernetes/pkg/scheduler/listers/fake"
 	schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo"
 	volumeutil "k8s.io/kubernetes/pkg/volume/util"
@@ -842,7 +843,7 @@ func TestVolumeCountConflicts(t *testing.T) {
 		},
 	}
 
-	expectedFailureReasons := []PredicateFailureReason{ErrMaxVolumeCountExceeded}
+	expectedStatus := framework.NewStatus(framework.Unschedulable, ErrMaxVolumeCountExceeded.GetReason())
 
 	// running attachable predicate tests with feature gate and limit present on nodes
 	for _, test := range tests {
@@ -852,12 +853,12 @@ func TestVolumeCountConflicts(t *testing.T) {
 			getFakeStorageClassLister(test.filterName),
 			getFakePVLister(test.filterName),
 			getFakePVCLister(test.filterName))
-		fits, reasons, err := pred(test.newPod, nil, node)
+		fits, gotStatus, err := pred(test.newPod, node)
 		if err != nil {
 			t.Errorf("Using allocatable [%s]%s: unexpected error: %v", test.filterName, test.test, err)
 		}
-		if !fits && !reflect.DeepEqual(reasons, expectedFailureReasons) {
-			t.Errorf("Using allocatable [%s]%s: unexpected failure reasons: %v, want: %v", test.filterName, test.test, reasons, expectedFailureReasons)
+		if !fits && !reflect.DeepEqual(gotStatus, expectedStatus) {
+			t.Errorf("Using allocatable [%s]%s: unexpected status: %v, want: %v", test.filterName, test.test, gotStatus, expectedStatus)
 		}
 		if fits != test.fits {
 			t.Errorf("Using allocatable [%s]%s: expected %v, got %v", test.filterName, test.test, test.fits, fits)