Merge pull request kubernetes#86875 from ahg-g/ahg1-cleanup

Move RequestedToCapacityRatio argument processing to its plugin

Author: k8s-ci-robot

pkg/scheduler/framework/plugins/BUILD

@@ -32,7 +32,6 @@ go_library(
         "//pkg/scheduler/framework/plugins/volumezone:go_default_library",
         "//pkg/scheduler/framework/v1alpha1:go_default_library",
         "//pkg/scheduler/volumebinder:go_default_library",
-        "//staging/src/k8s.io/api/core/v1: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/apiserver/pkg/util/feature:go_default_library",
@@ -82,9 +81,6 @@ go_test(
     embed = [":go_default_library"],
     deps = [
         "//pkg/scheduler/apis/config:go_default_library",
-        "//pkg/scheduler/framework/plugins/noderesources:go_default_library",
-        "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//vendor/github.com/google/go-cmp/cmp:go_default_library",
-        "//vendor/github.com/stretchr/testify/assert:go_default_library",
     ],
 )

pkg/scheduler/framework/plugins/legacy_registry.go

@@ -20,7 +20,6 @@ import (
 	"encoding/json"
 	"fmt"
 
-	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/sets"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
@@ -46,7 +45,6 @@ import (
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/volumebinding"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/volumerestrictions"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/volumezone"
-	framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1"
 )
 
 // LegacyRegistry is used to store current state of registered predicates and priorities.
@@ -504,10 +502,8 @@ func (lr *LegacyRegistry) ProcessPriorityPolicy(policy config.PriorityPolicy, co
 	}
 
 	if policy.Argument.RequestedToCapacityRatioArguments != nil {
-		scoringFunctionShape, resources := buildScoringFunctionShapeFromRequestedToCapacityRatioArguments(policy.Argument.RequestedToCapacityRatioArguments)
 		configProducerArgs.RequestedToCapacityRatioArgs = &noderesources.RequestedToCapacityRatioArgs{
-			FunctionShape:       scoringFunctionShape,
-			ResourceToWeightMap: resources,
+			RequestedToCapacityRatioArguments: *policy.Argument.RequestedToCapacityRatioArguments,
 		}
 		// We do not allow specifying the name for custom plugins, see #83472
 		priorityName = noderesources.RequestedToCapacityRatioName
@@ -516,37 +512,6 @@ func (lr *LegacyRegistry) ProcessPriorityPolicy(policy config.PriorityPolicy, co
 	return priorityName
 }
 
-// TODO(ahg-g): move to RequestedToCapacityRatio plugin.
-func buildScoringFunctionShapeFromRequestedToCapacityRatioArguments(arguments *config.RequestedToCapacityRatioArguments) (noderesources.FunctionShape, noderesources.ResourceToWeightMap) {
-	n := len(arguments.Shape)
-	points := make([]noderesources.FunctionShapePoint, 0, n)
-	for _, point := range arguments.Shape {
-		points = append(points, noderesources.FunctionShapePoint{
-			Utilization: int64(point.Utilization),
-			// MaxCustomPriorityScore may diverge from the max score used in the scheduler and defined by MaxNodeScore,
-			// therefore we need to scale the score returned by requested to capacity ratio to the score range
-			// used by the scheduler.
-			Score: int64(point.Score) * (framework.MaxNodeScore / config.MaxCustomPriorityScore),
-		})
-	}
-	shape, err := noderesources.NewFunctionShape(points)
-	if err != nil {
-		klog.Fatalf("invalid RequestedToCapacityRatioPriority arguments: %s", err.Error())
-	}
-	resourceToWeightMap := make(noderesources.ResourceToWeightMap)
-	if len(arguments.Resources) == 0 {
-		resourceToWeightMap = noderesources.DefaultRequestedRatioResources
-		return shape, resourceToWeightMap
-	}
-	for _, resource := range arguments.Resources {
-		resourceToWeightMap[v1.ResourceName(resource.Name)] = resource.Weight
-		if resource.Weight == 0 {
-			resourceToWeightMap[v1.ResourceName(resource.Name)] = 1
-		}
-	}
-	return shape, resourceToWeightMap
-}
-
 func validatePredicateOrDie(predicate config.PredicatePolicy) {
 	if predicate.Argument != nil {
 		numArgs := 0

pkg/scheduler/framework/plugins/legacy_registry_test.go

@@ -22,61 +22,9 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 
-	"github.com/stretchr/testify/assert"
-	v1 "k8s.io/api/core/v1"
 	"k8s.io/kubernetes/pkg/scheduler/apis/config"
-	schedulerapi "k8s.io/kubernetes/pkg/scheduler/apis/config"
-	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/noderesources"
 )
 
-func TestBuildScoringFunctionShapeFromRequestedToCapacityRatioArguments(t *testing.T) {
-	arguments := schedulerapi.RequestedToCapacityRatioArguments{
-		Shape: []schedulerapi.UtilizationShapePoint{
-			{Utilization: 10, Score: 1},
-			{Utilization: 30, Score: 5},
-			{Utilization: 70, Score: 2},
-		},
-		Resources: []schedulerapi.ResourceSpec{
-			{Name: string(v1.ResourceCPU)},
-			{Name: string(v1.ResourceMemory)},
-		},
-	}
-	builtShape, resources := buildScoringFunctionShapeFromRequestedToCapacityRatioArguments(&arguments)
-	expectedShape, _ := noderesources.NewFunctionShape([]noderesources.FunctionShapePoint{
-		{Utilization: 10, Score: 10},
-		{Utilization: 30, Score: 50},
-		{Utilization: 70, Score: 20},
-	})
-	expectedResources := noderesources.ResourceToWeightMap{
-		v1.ResourceCPU:    1,
-		v1.ResourceMemory: 1,
-	}
-	assert.Equal(t, expectedShape, builtShape)
-	assert.Equal(t, expectedResources, resources)
-}
-
-func TestBuildScoringFunctionShapeFromRequestedToCapacityRatioArgumentsNilResourceToWeightMap(t *testing.T) {
-	arguments := schedulerapi.RequestedToCapacityRatioArguments{
-		Shape: []schedulerapi.UtilizationShapePoint{
-			{Utilization: 10, Score: 1},
-			{Utilization: 30, Score: 5},
-			{Utilization: 70, Score: 2},
-		},
-	}
-	builtShape, resources := buildScoringFunctionShapeFromRequestedToCapacityRatioArguments(&arguments)
-	expectedShape, _ := noderesources.NewFunctionShape([]noderesources.FunctionShapePoint{
-		{Utilization: 10, Score: 10},
-		{Utilization: 30, Score: 50},
-		{Utilization: 70, Score: 20},
-	})
-	expectedResources := noderesources.ResourceToWeightMap{
-		v1.ResourceCPU:    1,
-		v1.ResourceMemory: 1,
-	}
-	assert.Equal(t, expectedShape, builtShape)
-	assert.Equal(t, expectedResources, resources)
-}
-
 func produceConfig(keys []string, producersMap map[string]ConfigProducer, args ConfigProducerArgs) (*config.Plugins, []config.PluginConfig, error) {
 	var plugins config.Plugins
 	var pluginConfig []config.PluginConfig

pkg/scheduler/framework/plugins/noderesources/BUILD

@@ -19,6 +19,7 @@ go_library(
         "//pkg/features:go_default_library",
         "//pkg/scheduler/algorithm/predicates:go_default_library",
         "//pkg/scheduler/algorithm/priorities/util:go_default_library",
+        "//pkg/scheduler/apis/config:go_default_library",
         "//pkg/scheduler/framework/plugins/migration:go_default_library",
         "//pkg/scheduler/framework/v1alpha1:go_default_library",
         "//pkg/scheduler/nodeinfo:go_default_library",

pkg/scheduler/framework/plugins/noderesources/balanced_allocation.go

@@ -74,7 +74,7 @@ func NewBalancedAllocation(_ *runtime.Unknown, h framework.FrameworkHandle) (fra
 		resourceAllocationScorer: resourceAllocationScorer{
 			BalancedAllocationName,
 			balancedResourceScorer,
-			DefaultRequestedRatioResources,
+			defaultRequestedRatioResources,
 		},
 	}, nil
 }

pkg/scheduler/framework/plugins/noderesources/least_allocated.go

@@ -69,14 +69,14 @@ func NewLeastAllocated(_ *runtime.Unknown, h framework.FrameworkHandle) (framewo
 		resourceAllocationScorer: resourceAllocationScorer{
 			LeastAllocatedName,
 			leastResourceScorer,
-			DefaultRequestedRatioResources,
+			defaultRequestedRatioResources,
 		},
 	}, nil
 }
 
 func leastResourceScorer(requested, allocable resourceToValueMap, includeVolumes bool, requestedVolumes int, allocatableVolumes int) int64 {
 	var nodeScore, weightSum int64
-	for resource, weight := range DefaultRequestedRatioResources {
+	for resource, weight := range defaultRequestedRatioResources {
 		resourceScore := leastRequestedScore(requested[resource], allocable[resource])
 		nodeScore += resourceScore * weight
 		weightSum += weight

pkg/scheduler/framework/plugins/noderesources/most_allocated.go

@@ -67,14 +67,14 @@ func NewMostAllocated(_ *runtime.Unknown, h framework.FrameworkHandle) (framewor
 		resourceAllocationScorer: resourceAllocationScorer{
 			MostAllocatedName,
 			mostResourceScorer,
-			DefaultRequestedRatioResources,
+			defaultRequestedRatioResources,
 		},
 	}, nil
 }
 
 func mostResourceScorer(requested, allocable resourceToValueMap, includeVolumes bool, requestedVolumes int, allocatableVolumes int) int64 {
 	var nodeScore, weightSum int64
-	for resource, weight := range DefaultRequestedRatioResources {
+	for resource, weight := range defaultRequestedRatioResources {
 		resourceScore := mostRequestedScore(requested[resource], allocable[resource])
 		nodeScore += resourceScore * weight
 		weightSum += weight

pkg/scheduler/framework/plugins/noderesources/requested_to_capacity_ratio.go

@@ -24,6 +24,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/klog"
+	"k8s.io/kubernetes/pkg/scheduler/apis/config"
 	framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1"
 )
 
@@ -36,37 +37,61 @@ const (
 	maxScore                     = framework.MaxNodeScore
 )
 
-// FunctionShape represents shape of scoring function.
-// For safety use NewFunctionShape which performs precondition checks for struct creation.
-type FunctionShape []FunctionShapePoint
+// RequestedToCapacityRatioArgs holds the args that are used to configure the plugin.
+type RequestedToCapacityRatioArgs struct {
+	config.RequestedToCapacityRatioArguments
+}
 
-// FunctionShapePoint represents single point in scoring function shape.
-type FunctionShapePoint struct {
+type functionShape []functionShapePoint
+
+type functionShapePoint struct {
 	// Utilization is function argument.
-	Utilization int64
+	utilization int64
 	// Score is function value.
-	Score int64
-}
-
-// RequestedToCapacityRatioArgs holds the args that are used to configure the plugin.
-type RequestedToCapacityRatioArgs struct {
-	FunctionShape       FunctionShape
-	ResourceToWeightMap ResourceToWeightMap
+	score int64
 }
 
 // NewRequestedToCapacityRatio initializes a new plugin and returns it.
 func NewRequestedToCapacityRatio(plArgs *runtime.Unknown, handle framework.FrameworkHandle) (framework.Plugin, error) {
-	args := &RequestedToCapacityRatioArgs{}
+	args := &config.RequestedToCapacityRatioArguments{}
 	if err := framework.DecodeInto(plArgs, args); err != nil {
 		return nil, err
 	}
 
+	shape := make([]functionShapePoint, 0, len(args.Shape))
+	for _, point := range args.Shape {
+		shape = append(shape, functionShapePoint{
+			utilization: int64(point.Utilization),
+			// MaxCustomPriorityScore may diverge from the max score used in the scheduler and defined by MaxNodeScore,
+			// therefore we need to scale the score returned by requested to capacity ratio to the score range
+			// used by the scheduler.
+			score: int64(point.Score) * (framework.MaxNodeScore / config.MaxCustomPriorityScore),
+		})
+	}
+
+	if err := validateFunctionShape(shape); err != nil {
+		return nil, err
+	}
+
+	resourceToWeightMap := make(resourceToWeightMap)
+	for _, resource := range args.Resources {
+		resourceToWeightMap[v1.ResourceName(resource.Name)] = resource.Weight
+		if resource.Weight == 0 {
+			// Apply the default weight.
+			resourceToWeightMap[v1.ResourceName(resource.Name)] = 1
+		}
+	}
+	if len(args.Resources) == 0 {
+		// If no resources specified, used the default set.
+		resourceToWeightMap = defaultRequestedRatioResources
+	}
+
 	return &RequestedToCapacityRatio{
 		handle: handle,
 		resourceAllocationScorer: resourceAllocationScorer{
 			RequestedToCapacityRatioName,
-			buildRequestedToCapacityRatioScorerFunction(args.FunctionShape, args.ResourceToWeightMap),
-			args.ResourceToWeightMap,
+			buildRequestedToCapacityRatioScorerFunction(shape, resourceToWeightMap),
+			resourceToWeightMap,
 		},
 	}, nil
 }
@@ -99,44 +124,36 @@ func (pl *RequestedToCapacityRatio) ScoreExtensions() framework.ScoreExtensions
 	return nil
 }
 
-// NewFunctionShape creates instance of FunctionShape in a safe way performing all
-// necessary sanity checks.
-func NewFunctionShape(points []FunctionShapePoint) (FunctionShape, error) {
-
-	n := len(points)
-
-	if n == 0 {
-		return nil, fmt.Errorf("at least one point must be specified")
+func validateFunctionShape(shape functionShape) error {
+	if len(shape) == 0 {
+		return fmt.Errorf("at least one point must be specified")
 	}
 
-	for i := 1; i < n; i++ {
-		if points[i-1].Utilization >= points[i].Utilization {
-			return nil, fmt.Errorf("utilization values must be sorted. Utilization[%d]==%d >= Utilization[%d]==%d", i-1, points[i-1].Utilization, i, points[i].Utilization)
+	for i := 1; i < len(shape); i++ {
+		if shape[i-1].utilization >= shape[i].utilization {
+			return fmt.Errorf("utilization values must be sorted. Utilization[%d]==%d >= Utilization[%d]==%d", i-1, shape[i-1].utilization, i, shape[i].utilization)
 		}
 	}
 
-	for i, point := range points {
-		if point.Utilization < minUtilization {
-			return nil, fmt.Errorf("utilization values must not be less than %d. Utilization[%d]==%d", minUtilization, i, point.Utilization)
+	for i, point := range shape {
+		if point.utilization < minUtilization {
+			return fmt.Errorf("utilization values must not be less than %d. Utilization[%d]==%d", minUtilization, i, point.utilization)
 		}
-		if point.Utilization > maxUtilization {
-			return nil, fmt.Errorf("utilization values must not be greater than %d. Utilization[%d]==%d", maxUtilization, i, point.Utilization)
+		if point.utilization > maxUtilization {
+			return fmt.Errorf("utilization values must not be greater than %d. Utilization[%d]==%d", maxUtilization, i, point.utilization)
 		}
-		if point.Score < minScore {
-			return nil, fmt.Errorf("score values must not be less than %d. Score[%d]==%d", minScore, i, point.Score)
+		if point.score < minScore {
+			return fmt.Errorf("score values must not be less than %d. Score[%d]==%d", minScore, i, point.score)
 		}
-		if point.Score > maxScore {
-			return nil, fmt.Errorf("score valuses not be greater than %d. Score[%d]==%d", maxScore, i, point.Score)
+		if int64(point.score) > maxScore {
+			return fmt.Errorf("score values not be greater than %d. Score[%d]==%d", maxScore, i, point.score)
 		}
 	}
 
-	// We make defensive copy so we make no assumption if array passed as argument is not changed afterwards
-	pointsCopy := make(FunctionShape, n)
-	copy(pointsCopy, points)
-	return pointsCopy, nil
+	return nil
 }
 
-func validateResourceWeightMap(resourceToWeightMap ResourceToWeightMap) error {
+func validateResourceWeightMap(resourceToWeightMap resourceToWeightMap) error {
 	if len(resourceToWeightMap) == 0 {
 		return fmt.Errorf("resourceToWeightMap cannot be nil")
 	}
@@ -149,7 +166,7 @@ func validateResourceWeightMap(resourceToWeightMap ResourceToWeightMap) error {
 	return nil
 }
 
-func buildRequestedToCapacityRatioScorerFunction(scoringFunctionShape FunctionShape, resourceToWeightMap ResourceToWeightMap) func(resourceToValueMap, resourceToValueMap, bool, int, int) int64 {
+func buildRequestedToCapacityRatioScorerFunction(scoringFunctionShape functionShape, resourceToWeightMap resourceToWeightMap) func(resourceToValueMap, resourceToValueMap, bool, int, int) int64 {
 	rawScoringFunction := buildBrokenLinearFunction(scoringFunctionShape)
 	err := validateResourceWeightMap(resourceToWeightMap)
 	if err != nil {
@@ -179,25 +196,24 @@ func buildRequestedToCapacityRatioScorerFunction(scoringFunctionShape FunctionSh
 }
 
 // Creates a function which is built using linear segments. Segments are defined via shape array.
-// Shape[i].Utilization slice represents points on "utilization" axis where different segments meet.
-// Shape[i].Score represents function values at meeting points.
+// Shape[i].utilization slice represents points on "utilization" axis where different segments meet.
+// Shape[i].score represents function values at meeting points.
 //
 // function f(p) is defined as:
-//   shape[0].Score for p < f[0].Utilization
-//   shape[i].Score for p == shape[i].Utilization
-//   shape[n-1].Score for p > shape[n-1].Utilization
-// and linear between points (p < shape[i].Utilization)
-func buildBrokenLinearFunction(shape FunctionShape) func(int64) int64 {
-	n := len(shape)
+//   shape[0].score for p < f[0].utilization
+//   shape[i].score for p == shape[i].utilization
+//   shape[n-1].score for p > shape[n-1].utilization
+// and linear between points (p < shape[i].utilization)
+func buildBrokenLinearFunction(shape functionShape) func(int64) int64 {
 	return func(p int64) int64 {
-		for i := 0; i < n; i++ {
-			if p <= shape[i].Utilization {
+		for i := 0; i < len(shape); i++ {
+			if p <= int64(shape[i].utilization) {
 				if i == 0 {
-					return shape[0].Score
+					return shape[0].score
 				}
-				return shape[i-1].Score + (shape[i].Score-shape[i-1].Score)*(p-shape[i-1].Utilization)/(shape[i].Utilization-shape[i-1].Utilization)
+				return shape[i-1].score + (shape[i].score-shape[i-1].score)*(p-shape[i-1].utilization)/(shape[i].utilization-shape[i-1].utilization)
 			}
 		}
-		return shape[n-1].Score
+		return shape[len(shape)-1].score
 	}
 }