From 9fb0df5096a949a69246f35b34140ef230af0378 Mon Sep 17 00:00:00 2001
From: draveness <i@draveness.me>
Date: Fri, 9 Aug 2019 17:03:55 +0800
Subject: [PATCH 1/2] feat(scheduler): return error when score is out of range

---
 pkg/scheduler/framework/v1alpha1/framework.go | 12 ++++-
 .../framework/v1alpha1/framework_test.go      | 54 +++++++++++++++++++
 pkg/scheduler/framework/v1alpha1/interface.go |  8 +++
 test/integration/scheduler/framework_test.go  |  4 +-
 4 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/pkg/scheduler/framework/v1alpha1/framework.go b/pkg/scheduler/framework/v1alpha1/framework.go
index c1b03688590..521e67b37c7 100644
--- a/pkg/scheduler/framework/v1alpha1/framework.go
+++ b/pkg/scheduler/framework/v1alpha1/framework.go
@@ -431,8 +431,16 @@ func (f *framework) ApplyScoreWeights(pc *PluginContext, pod *v1.Pod, scores Plu
 			errCh.SendErrorWithCancel(err, cancel)
 			return
 		}
-		for i := range nodeScoreList {
-			nodeScoreList[i].Score = nodeScoreList[i].Score * weight
+
+		for i, nodeScore := range nodeScoreList {
+			// return error if score plugin returns invalid score.
+			if nodeScore.Score > MaxNodeScore || nodeScore.Score < MinNodeScore {
+				err := fmt.Errorf("score plugin %q returns an invalid score %q, it should in the range of [MinNodeScore, MaxNodeScore] after normalizing", pl.Name(), nodeScore.Score)
+				errCh.SendErrorWithCancel(err, cancel)
+				return
+			}
+
+			nodeScoreList[i].Score = nodeScore.Score * weight
 		}
 	})
 
diff --git a/pkg/scheduler/framework/v1alpha1/framework_test.go b/pkg/scheduler/framework/v1alpha1/framework_test.go
index 682110e407f..e6478ae21eb 100644
--- a/pkg/scheduler/framework/v1alpha1/framework_test.go
+++ b/pkg/scheduler/framework/v1alpha1/framework_test.go
@@ -604,6 +604,60 @@ func TestApplyScoreWeights(t *testing.T) {
 			},
 			err: true,
 		},
+		{
+			name:    "Score plugin return score greater than MaxNodeScore",
+			plugins: plugin1And2,
+			input: PluginToNodeScores{
+				scorePlugin1: {
+					{
+						Name:  "node1",
+						Score: MaxNodeScore + 1,
+					},
+					{
+						Name:  "node2",
+						Score: 3,
+					},
+				},
+				scorePlugin2: {
+					{
+						Name:  "node1",
+						Score: MinNodeScore,
+					},
+					{
+						Name:  "node2",
+						Score: 5,
+					},
+				},
+			},
+			err: true,
+		},
+		{
+			name:    "Score plugin return score less than MinNodeScore",
+			plugins: plugin1And2,
+			input: PluginToNodeScores{
+				scorePlugin1: {
+					{
+						Name:  "node1",
+						Score: MaxNodeScore,
+					},
+					{
+						Name:  "node2",
+						Score: 3,
+					},
+				},
+				scorePlugin2: {
+					{
+						Name:  "node1",
+						Score: MinNodeScore - 1,
+					},
+					{
+						Name:  "node2",
+						Score: 5,
+					},
+				},
+			},
+			err: true,
+		},
 	}
 
 	for _, tt := range tests {
diff --git a/pkg/scheduler/framework/v1alpha1/interface.go b/pkg/scheduler/framework/v1alpha1/interface.go
index 1044631b143..a95161676e1 100644
--- a/pkg/scheduler/framework/v1alpha1/interface.go
+++ b/pkg/scheduler/framework/v1alpha1/interface.go
@@ -61,6 +61,14 @@ const (
 	Skip
 )
 
+const (
+	// MaxNodeScore is the maximum score a Score plugin is expected to return.
+	MaxNodeScore int = 10
+
+	// MinNodeScore is the minimum score a Score plugin is expected to return.
+	MinNodeScore int = 0
+)
+
 // Status indicates the result of running a plugin. It consists of a code and a
 // message. When the status code is not `Success`, the status message should
 // explain why.
diff --git a/test/integration/scheduler/framework_test.go b/test/integration/scheduler/framework_test.go
index 285d8adab6b..08c982750fa 100644
--- a/test/integration/scheduler/framework_test.go
+++ b/test/integration/scheduler/framework_test.go
@@ -152,11 +152,11 @@ func (sp *ScorePlugin) Score(pc *framework.PluginContext, p *v1.Pod, nodeName st
 		return 0, framework.NewStatus(framework.Error, fmt.Sprintf("injecting failure for pod %v", p.Name))
 	}
 
-	score := 10
+	score := 1
 	if sp.numScoreCalled == 1 {
 		// The first node is scored the highest, the rest is scored lower.
 		sp.highScoreNode = nodeName
-		score = 100
+		score = framework.MaxNodeScore
 	}
 	return score, nil
 }

From d3cc73965a0e1ef0d8042a81a5ac79db5589da05 Mon Sep 17 00:00:00 2001
From: draveness <i@draveness.me>
Date: Wed, 14 Aug 2019 15:58:08 +0800
Subject: [PATCH 2/2] feat: use schedulerapi.MaxPriority instead of hard-coded
 int

---
 pkg/scheduler/framework/v1alpha1/BUILD        | 1 +
 pkg/scheduler/framework/v1alpha1/interface.go | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/pkg/scheduler/framework/v1alpha1/BUILD b/pkg/scheduler/framework/v1alpha1/BUILD
index 3962dbade6f..481eb106fdc 100644
--- a/pkg/scheduler/framework/v1alpha1/BUILD
+++ b/pkg/scheduler/framework/v1alpha1/BUILD
@@ -12,6 +12,7 @@ go_library(
     importpath = "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1",
     visibility = ["//visibility:public"],
     deps = [
+        "//pkg/scheduler/api:go_default_library",
         "//pkg/scheduler/apis/config:go_default_library",
         "//pkg/scheduler/internal/cache:go_default_library",
         "//pkg/scheduler/util:go_default_library",
diff --git a/pkg/scheduler/framework/v1alpha1/interface.go b/pkg/scheduler/framework/v1alpha1/interface.go
index a95161676e1..cde7300889e 100644
--- a/pkg/scheduler/framework/v1alpha1/interface.go
+++ b/pkg/scheduler/framework/v1alpha1/interface.go
@@ -24,6 +24,7 @@ import (
 
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
+	schedulerapi "k8s.io/kubernetes/pkg/scheduler/api"
 	internalcache "k8s.io/kubernetes/pkg/scheduler/internal/cache"
 )
 
@@ -63,7 +64,7 @@ const (
 
 const (
 	// MaxNodeScore is the maximum score a Score plugin is expected to return.
-	MaxNodeScore int = 10
+	MaxNodeScore int = schedulerapi.MaxPriority
 
 	// MinNodeScore is the minimum score a Score plugin is expected to return.
 	MinNodeScore int = 0