accept max_scale_up and max_scale_down for target-value (#848)

The two optional parameters max_scale_up and max_scale_down determine
the maximum and minimum number of scaling differences between two
evaluations.

It is useful to avoid scaling down or scaling up a job too quickly.

CHANGELOG.md

@@ -1,5 +1,8 @@
 ## UNRELEASED
 
+IMPROVEMENTS:
+ * plugin/strategy/target-value: Add new configuration `max_scale_up` and `max_scale_down` to allow restricting how much change is applied on each scaling event [[GH-848](https://github.com/hashicorp/nomad-autoscaler/pull/848)]
+
 BUG FIXES:
  * agent: Fixed a bug that caused a target in dry-run mode to scale when outside of its min/max range [[GH-845](https://github.com/hashicorp/nomad-autoscaler/pull/845)]
 

plugins/builtin/strategy/target-value/plugin/plugin.go

@@ -22,8 +22,10 @@ const (
 	pluginName = "target-value"
 
 	// These are the keys read from the RunRequest.Config map.
-	runConfigKeyTarget    = "target"
-	runConfigKeyThreshold = "threshold"
+	runConfigKeyTarget       = "target"
+	runConfigKeyThreshold    = "threshold"
+	runConfigKeyMaxScaleUp   = "max_scale_up"
+	runConfigKeyMaxScaleDown = "max_scale_down"
 
 	// defaultThreshold controls how significant is a change in the input
 	// metric value.
@@ -103,6 +105,32 @@ func (s *StrategyPlugin) Run(eval *sdk.ScalingCheckEvaluation, count int64) (*sd
 		return nil, fmt.Errorf("invalid value for `threshold`: %v (%T)", th, th)
 	}
 
+	// Read and parse max_scale_up from req.Config.
+	var maxScaleUp *int64
+	maxScaleUpStr := eval.Check.Strategy.Config[runConfigKeyMaxScaleUp]
+	if maxScaleUpStr != "" {
+		msu, err := strconv.ParseInt(maxScaleUpStr, 10, 64)
+		if err != nil {
+			return nil, fmt.Errorf("invalid value for `max_scale_up`: %v (%T)", maxScaleUpStr, maxScaleUpStr)
+		}
+		maxScaleUp = &msu
+	} else {
+		maxScaleUpStr = "+Inf"
+	}
+
+	// Read and parse max_scale_down from req.Config.
+	var maxScaleDown *int64
+	maxScaleDownStr := eval.Check.Strategy.Config[runConfigKeyMaxScaleDown]
+	if maxScaleDownStr != "" {
+		msd, err := strconv.ParseInt(maxScaleDownStr, 10, 64)
+		if err != nil {
+			return nil, fmt.Errorf("invalid value for `max_scale_down`: %v (%T)", maxScaleDownStr, maxScaleDownStr)
+		}
+		maxScaleDown = &msd
+	} else {
+		maxScaleDownStr = "-Inf"
+	}
+
 	var factor float64
 
 	// Use only the latest value for now.
@@ -136,13 +164,21 @@ func (s *StrategyPlugin) Run(eval *sdk.ScalingCheckEvaluation, count int64) (*sd
 		newCount = int64(math.Ceil(float64(count) * factor))
 	}
 
+	// Limit the increase or decrease with the values specified in max_scale_up and max_scale_down.
+	if maxScaleDown != nil && newCount < count-(*maxScaleDown) {
+		newCount = count - *maxScaleDown
+	}
+	if maxScaleUp != nil && newCount > count+(*maxScaleUp) {
+		newCount = count + *maxScaleUp
+	}
+
 	// Log at trace level the details of the strategy calculation. This is
 	// helpful in ultra-debugging situations when there is a need to understand
 	// all the calculations made.
 	s.logger.Trace("calculated scaling strategy results",
 		"check_name", eval.Check.Name, "current_count", count, "new_count", newCount,
 		"metric_value", metric.Value, "metric_time", metric.Timestamp, "factor", factor,
-		"direction", eval.Action.Direction)
+		"direction", eval.Action.Direction, "max_scale_up", maxScaleUpStr, "max_scale_down", maxScaleDownStr)
 
 	// If the calculated newCount is the same as the current count, we do not
 	// need to scale so return an empty response.

plugins/builtin/strategy/target-value/plugin/plugin_test.go

@@ -77,6 +77,32 @@ func TestStrategyPlugin_Run(t *testing.T) {
 			expectedError: errors.New("invalid value for `threshold`: not-the-float-you're-looking-for (string)"),
 			name:          "incorrect input strategy config threshold value",
 		},
+		{
+			inputEval: &sdk.ScalingCheckEvaluation{
+				Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 13}},
+				Check: &sdk.ScalingPolicyCheck{
+					Strategy: &sdk.ScalingPolicyStrategy{
+						Config: map[string]string{"target": "0", "max_scale_up": "not-the-int-you're-looking-for"},
+					},
+				},
+			},
+			expectedResp:  nil,
+			expectedError: errors.New("invalid value for `max_scale_up`: not-the-int-you're-looking-for (string)"),
+			name:          "incorrect input strategy config max_scale_up value",
+		},
+		{
+			inputEval: &sdk.ScalingCheckEvaluation{
+				Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 13}},
+				Check: &sdk.ScalingPolicyCheck{
+					Strategy: &sdk.ScalingPolicyStrategy{
+						Config: map[string]string{"target": "0", "max_scale_down": "not-the-int-you're-looking-for"},
+					},
+				},
+			},
+			expectedResp:  nil,
+			expectedError: errors.New("invalid value for `max_scale_down`: not-the-int-you're-looking-for (string)"),
+			name:          "incorrect input strategy config max_scale_down value",
+		},
 		{
 			inputEval: &sdk.ScalingCheckEvaluation{
 				Metrics: sdk.TimestampedMetrics{},
@@ -337,6 +363,60 @@ func TestStrategyPlugin_Run(t *testing.T) {
 			expectedError: nil,
 			name:          "properly handle multiple input",
 		},
+		{
+			inputEval: &sdk.ScalingCheckEvaluation{
+				Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 210}},
+				Check: &sdk.ScalingPolicyCheck{
+					Strategy: &sdk.ScalingPolicyStrategy{
+						Config: map[string]string{"target": "50", "max_scale_up": "2"},
+					},
+				},
+				Action: &sdk.ScalingAction{},
+			},
+			inputCount: 1,
+			expectedResp: &sdk.ScalingCheckEvaluation{
+				Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 210}},
+				Check: &sdk.ScalingPolicyCheck{
+					Strategy: &sdk.ScalingPolicyStrategy{
+						Config: map[string]string{"target": "50", "max_scale_up": "2"},
+					},
+				},
+				Action: &sdk.ScalingAction{
+					Count:     3,
+					Reason:    "scaling up because factor is 4.200000",
+					Direction: sdk.ScaleDirectionUp,
+				},
+			},
+			expectedError: nil,
+			name:          "scale up limited to max_scale_up",
+		},
+		{
+			inputEval: &sdk.ScalingCheckEvaluation{
+				Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 10}},
+				Check: &sdk.ScalingPolicyCheck{
+					Strategy: &sdk.ScalingPolicyStrategy{
+						Config: map[string]string{"target": "210", "max_scale_down": "1"},
+					},
+				},
+				Action: &sdk.ScalingAction{},
+			},
+			inputCount: 5,
+			expectedResp: &sdk.ScalingCheckEvaluation{
+				Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 10}},
+				Check: &sdk.ScalingPolicyCheck{
+					Strategy: &sdk.ScalingPolicyStrategy{
+						Config: map[string]string{"target": "210", "max_scale_down": "1"},
+					},
+				},
+				Action: &sdk.ScalingAction{
+					Count:     4,
+					Reason:    "scaling down because factor is 0.047619",
+					Direction: sdk.ScaleDirectionDown,
+				},
+			},
+			expectedError: nil,
+			name:          "scale down limited to max_scale_down",
+		},
 	}
 
 	for _, tc := range testCases {