From 2d9bc0b7d87a6dac97c7db6bdb14a75bed651fea Mon Sep 17 00:00:00 2001 From: Jasmine Dahilig Date: Wed, 24 Mar 2021 12:46:21 -0700 Subject: [PATCH 1/6] add new None strategy plugin for pass-thru scaling action --- plugins/builtin/strategy/none/main.go | 16 + .../builtin/strategy/none/plugin/plugin.go | 97 +++++ .../strategy/none/plugin/plugin_test.go | 351 ++++++++++++++++++ plugins/manager/internal.go | 5 + plugins/plugin.go | 2 + 5 files changed, 471 insertions(+) create mode 100644 plugins/builtin/strategy/none/main.go create mode 100644 plugins/builtin/strategy/none/plugin/plugin.go create mode 100644 plugins/builtin/strategy/none/plugin/plugin_test.go diff --git a/plugins/builtin/strategy/none/main.go b/plugins/builtin/strategy/none/main.go new file mode 100644 index 00000000..1e2c1679 --- /dev/null +++ b/plugins/builtin/strategy/none/main.go @@ -0,0 +1,16 @@ +package main + +import ( + hclog "github.com/hashicorp/go-hclog" + "github.com/hashicorp/nomad-autoscaler/plugins" + targetvalue "github.com/hashicorp/nomad-autoscaler/plugins/builtin/strategy/target-value/plugin" +) + +func main() { + plugins.Serve(factory) +} + +// factory returns a new instance of the TargetValue Strategy plugin. +func factory(log hclog.Logger) interface{} { + return targetvalue.NewTargetValuePlugin(log) +} diff --git a/plugins/builtin/strategy/none/plugin/plugin.go b/plugins/builtin/strategy/none/plugin/plugin.go new file mode 100644 index 00000000..5c5e7dc8 --- /dev/null +++ b/plugins/builtin/strategy/none/plugin/plugin.go @@ -0,0 +1,97 @@ +package plugin + +import ( + "fmt" + + "github.com/hashicorp/go-hclog" + "github.com/hashicorp/nomad-autoscaler/plugins" + "github.com/hashicorp/nomad-autoscaler/plugins/base" + "github.com/hashicorp/nomad-autoscaler/plugins/strategy" + "github.com/hashicorp/nomad-autoscaler/sdk" +) + +const ( + // pluginName is the unique name of the this plugin amongst strategy + // plugins. + pluginName = "none" +) + +var ( + PluginID = plugins.PluginID{ + Name: pluginName, + PluginType: sdk.PluginTypeStrategy, + } + + PluginConfig = &plugins.InternalPluginConfig{ + Factory: func(l hclog.Logger) interface{} { return NewTargetValuePlugin(l) }, + } + + pluginInfo = &base.PluginInfo{ + Name: pluginName, + PluginType: sdk.PluginTypeStrategy, + } +) + +// Assert that StrategyPlugin meets the strategy.Strategy interface. +var _ strategy.Strategy = (*StrategyPlugin)(nil) + +// StrategyPlugin is the None implementation of the strategy.Strategy +// interface. +type StrategyPlugin struct { + logger hclog.Logger +} + +// NewTargetValuePlugin returns the None implementation of the +// strategy.Strategy interface. +func NewTargetValuePlugin(log hclog.Logger) strategy.Strategy { + return &StrategyPlugin{ + logger: log, + } +} + +// SetConfig satisfies the SetConfig function on the base.Base interface. +func (s *StrategyPlugin) SetConfig(config map[string]string) error { + return nil +} + +// PluginInfo satisfies the PluginInfo function on the base.Base interface. +func (s *StrategyPlugin) PluginInfo() (*base.PluginInfo, error) { + return pluginInfo, nil +} + +// Run satisfies the Run function on the strategy.Strategy interface. +func (s *StrategyPlugin) Run(eval *sdk.ScalingCheckEvaluation, count int64) (*sdk.ScalingCheckEvaluation, error) { + + // Use only the latest value for now. + metric := eval.Metrics[len(eval.Metrics)-1] + + + // TODO: compare count > metric --> direction + // Identify the direction of scaling, if any. + eval.Action.Direction = s.calculateDirection(count, metric.Value) + if eval.Action.Direction == sdk.ScaleDirectionNone { + return eval, nil + } + + + eval.Action.Count = int64(metric.Value) + eval.Action.Reason = fmt.Sprintf("scaling %s because metric is %d", eval.Action.Direction, eval.Action.Count) + + return eval, nil +} + +// calculateDirection is used to calculate the direction of scaling that should +// occur, if any at all. It takes into account the current task group count in +// order to correctly account for 0 counts. +// +// The input factor value is padded by e, such that no action will be taken if +// factor is within [1-e; 1+e]. +func (s *StrategyPlugin) calculateDirection(count int64, metric float64) sdk.ScaleDirection { + if metric > float64(count) { + return sdk.ScaleDirectionUp + } else if metric < float64(count) { + return sdk.ScaleDirectionDown + } else { + return sdk.ScaleDirectionNone + } +} diff --git a/plugins/builtin/strategy/none/plugin/plugin_test.go b/plugins/builtin/strategy/none/plugin/plugin_test.go new file mode 100644 index 00000000..5cbbdfe0 --- /dev/null +++ b/plugins/builtin/strategy/none/plugin/plugin_test.go @@ -0,0 +1,351 @@ +package plugin + +import ( + "fmt" + "testing" + "time" + + hclog "github.com/hashicorp/go-hclog" + "github.com/hashicorp/nomad-autoscaler/plugins/base" + "github.com/hashicorp/nomad-autoscaler/sdk" + "github.com/stretchr/testify/assert" +) + +func TestStrategyPlugin_SetConfig(t *testing.T) { + s := &StrategyPlugin{} + expectedOutput := map[string]string{"example-item": "example-value"} + err := s.SetConfig(expectedOutput) + assert.Nil(t, err) + assert.Equal(t, expectedOutput, s.config) +} + +func TestStrategyPlugin_PluginInfo(t *testing.T) { + s := &StrategyPlugin{} + expectedOutput := &base.PluginInfo{Name: "target-value", PluginType: "strategy"} + actualOutput, err := s.PluginInfo() + assert.Nil(t, err) + assert.Equal(t, expectedOutput, actualOutput) +} + +func TestStrategyPlugin_Run(t *testing.T) { + testCases := []struct { + inputEval *sdk.ScalingCheckEvaluation + inputCount int64 + expectedResp *sdk.ScalingCheckEvaluation + expectedError error + name string + }{ + { + inputEval: &sdk.ScalingCheckEvaluation{ + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{}, + }, + }, + expectedResp: nil, + expectedError: fmt.Errorf("missing required field `target`"), + name: "incorrect strategy input config", + }, + { + inputEval: &sdk.ScalingCheckEvaluation{ + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{"target": "not-the-float-you're-looking-for"}, + }, + }, + }, + expectedResp: nil, + expectedError: fmt.Errorf("invalid value for `target`: not-the-float-you're-looking-for (string)"), + name: "incorrect input strategy config target value", + }, + { + inputEval: &sdk.ScalingCheckEvaluation{ + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{"target": "0", "threshold": "not-the-float-you're-looking-for"}, + }, + }, + }, + expectedResp: nil, + expectedError: fmt.Errorf("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": "13"}, + }, + }, + Action: &sdk.ScalingAction{}, + }, + inputCount: 2, + expectedResp: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 13}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{"target": "13"}, + }, + }, + Action: &sdk.ScalingAction{ + Direction: sdk.ScaleDirectionNone, + }, + }, + expectedError: nil, + name: "factor equals 1 with non-zero count", + }, + { + inputEval: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 26}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{"target": "13"}, + }, + }, + Action: &sdk.ScalingAction{}, + }, + inputCount: 2, + expectedResp: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 26}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{"target": "13"}, + }, + }, + Action: &sdk.ScalingAction{ + Count: 4, + Reason: "scaling up because factor is 2.000000", + Direction: sdk.ScaleDirectionUp, + }, + }, + expectedError: nil, + name: "factor greater than 1 with non-zero count", + }, + { + inputEval: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 20}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{"target": "10"}, + }, + }, + Action: &sdk.ScalingAction{}, + }, + inputCount: 0, + expectedResp: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 20}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{"target": "10"}, + }, + }, + Action: &sdk.ScalingAction{ + Count: 2, + Reason: "scaling up because factor is 2.000000", + Direction: sdk.ScaleDirectionUp, + }, + }, + expectedError: nil, + name: "scale from 0 with non-zero target", + }, + { + inputEval: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 9}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{"target": "10"}, + }, + }, + Action: &sdk.ScalingAction{}, + }, + inputCount: 1, + expectedResp: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 9}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{"target": "10"}, + }, + }, + Action: &sdk.ScalingAction{ + Direction: sdk.ScaleDirectionNone, + }, + }, + expectedError: nil, + name: "no scaling based on small target/metric difference", + }, + { + inputEval: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 1}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{"target": "10"}, + }, + }, + Action: &sdk.ScalingAction{}, + }, + inputCount: 0, + expectedResp: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 1}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{"target": "10"}, + }, + }, + Action: &sdk.ScalingAction{ + Count: 1, + Reason: "scaling up because factor is 0.100000", + Direction: sdk.ScaleDirectionUp, + }, + }, + expectedError: nil, + name: "scale from 0 with 0 target eg. build queue with 0 running build agents", + }, + { + inputEval: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 0}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{"target": "0"}, + }, + }, + Action: &sdk.ScalingAction{}, + }, + inputCount: 5, + expectedResp: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 0}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{"target": "0"}, + }, + }, + Action: &sdk.ScalingAction{ + Count: 0, + Direction: sdk.ScaleDirectionDown, + Reason: "scaling down because factor is 0.000000", + }, + }, + expectedError: nil, + name: "scale to 0 with 0 target eg. idle build agents", + }, + { + inputEval: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 5.00001}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{"target": "5"}, + }, + }, + Action: &sdk.ScalingAction{}, + }, + inputCount: 8, + expectedResp: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 5.00001}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{"target": "5"}, + }, + }, + Action: &sdk.ScalingAction{ + Direction: sdk.ScaleDirectionNone, + }, + }, + expectedError: nil, + name: "don't scale when the factor change is small", + }, + { + inputEval: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 5.00001}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{"target": "5", "threshold": "0.000001"}, + }, + }, + Action: &sdk.ScalingAction{}, + }, + inputCount: 8, + expectedResp: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 5.00001}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{"target": "5", "threshold": "0.000001"}, + }, + }, + Action: &sdk.ScalingAction{ + Count: 9, + Reason: "scaling up because factor is 1.000002", + Direction: sdk.ScaleDirectionUp, + }, + }, + expectedError: nil, + name: "scale up on small changes if threshold is small", + }, + { + inputEval: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{ + sdk.TimestampedMetric{Value: 13, Timestamp: time.Unix(1600000000, 0)}, + sdk.TimestampedMetric{Value: 14, Timestamp: time.Unix(1600000001, 0)}, + sdk.TimestampedMetric{Value: 15, Timestamp: time.Unix(1600000002, 0)}, + sdk.TimestampedMetric{Value: 16, Timestamp: time.Unix(1600000003, 0)}, + }, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{"target": "16"}, + }, + }, + Action: &sdk.ScalingAction{}, + }, + inputCount: 2, + expectedResp: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{ + sdk.TimestampedMetric{Value: 13, Timestamp: time.Unix(1600000000, 0)}, + sdk.TimestampedMetric{Value: 14, Timestamp: time.Unix(1600000001, 0)}, + sdk.TimestampedMetric{Value: 15, Timestamp: time.Unix(1600000002, 0)}, + sdk.TimestampedMetric{Value: 16, Timestamp: time.Unix(1600000003, 0)}, + }, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{"target": "16"}, + }, + }, + Action: &sdk.ScalingAction{ + Direction: sdk.ScaleDirectionNone, + }, + }, + expectedError: nil, + name: "properly handle multiple input", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + s := &StrategyPlugin{logger: hclog.NewNullLogger()} + actualResp, actualError := s.Run(tc.inputEval, tc.inputCount) + assert.Equal(t, tc.expectedResp, actualResp, tc.name) + assert.Equal(t, tc.expectedError, actualError, tc.name) + }) + } +} + +func TestStrategyPlugin_calculateDirection(t *testing.T) { + testCases := []struct { + inputCount int64 + inputFactor float64 + threshold float64 + expectedOutput sdk.ScaleDirection + }{ + {inputCount: 0, inputFactor: 1, expectedOutput: sdk.ScaleDirectionUp}, + {inputCount: 5, inputFactor: 1, expectedOutput: sdk.ScaleDirectionNone}, + {inputCount: 4, inputFactor: 0.5, expectedOutput: sdk.ScaleDirectionDown}, + {inputCount: 5, inputFactor: 2, expectedOutput: sdk.ScaleDirectionUp}, + {inputCount: 5, inputFactor: 1.0001, threshold: 0.01, expectedOutput: sdk.ScaleDirectionNone}, + {inputCount: 5, inputFactor: 1.02, threshold: 0.01, expectedOutput: sdk.ScaleDirectionUp}, + {inputCount: 5, inputFactor: 0.99, threshold: 0.01, expectedOutput: sdk.ScaleDirectionNone}, + {inputCount: 5, inputFactor: 0.98, threshold: 0.01, expectedOutput: sdk.ScaleDirectionDown}, + } + + s := &StrategyPlugin{} + + for _, tc := range testCases { + assert.Equal(t, tc.expectedOutput, s.calculateDirection(tc.inputCount, tc.inputFactor, tc.threshold)) + } +} diff --git a/plugins/manager/internal.go b/plugins/manager/internal.go index 9d826de6..aabbfa65 100644 --- a/plugins/manager/internal.go +++ b/plugins/manager/internal.go @@ -14,6 +14,7 @@ import ( azureVMSS "github.com/hashicorp/nomad-autoscaler/plugins/builtin/target/azure-vmss/plugin" gceMIG "github.com/hashicorp/nomad-autoscaler/plugins/builtin/target/gce-mig/plugin" nomadTarget "github.com/hashicorp/nomad-autoscaler/plugins/builtin/target/nomad/plugin" + none "github.com/hashicorp/nomad-autoscaler/plugins/builtin/strategy/none/plugin" ) // loadInternalPlugin takes the plugin configuration and attempts to load it @@ -29,6 +30,9 @@ func (pm *PluginManager) loadInternalPlugin(cfg *config.Plugin, pluginType strin case plugins.InternalTargetNomad: info.factory = nomadTarget.PluginConfig.Factory info.driver = "nomad-target" + case plugins.InternalStrategyNone: + info.factory = none.PluginConfig.Factory + info.driver = "none" case plugins.InternalStrategyTargetValue: info.factory = targetValue.PluginConfig.Factory info.driver = "target-value" @@ -86,6 +90,7 @@ func (pm *PluginManager) useInternal(plugin string) bool { case plugins.InternalAPMNomad, plugins.InternalTargetNomad, plugins.InternalAPMPrometheus, + plugins.InternalStrategyNone, plugins.InternalStrategyTargetValue, plugins.InternalTargetAWSASG, plugins.InternalTargetAzureVMSS, diff --git a/plugins/plugin.go b/plugins/plugin.go index 6551f424..8c736307 100644 --- a/plugins/plugin.go +++ b/plugins/plugin.go @@ -23,6 +23,8 @@ const ( // InternalAPMPrometheus is the Prometheus APM internal plugin name. InternalAPMPrometheus = "prometheus" + InternalStrategyNone = "none" + // InternalStrategyTargetValue is the Target Value Strategy internal plugin // name. InternalStrategyTargetValue = "target-value" From 1eebd38ad34b9bc131afe9dd4bb3a865c89ab071 Mon Sep 17 00:00:00 2001 From: Jasmine Dahilig Date: Tue, 30 Mar 2021 08:17:42 -0700 Subject: [PATCH 2/6] rename plugin to pass-thru --- plugins/builtin/strategy/none/main.go | 16 ---------------- plugins/builtin/strategy/pass-thru/main.go | 16 ++++++++++++++++ .../{none => pass-thru}/plugin/plugin.go | 6 +++--- .../{none => pass-thru}/plugin/plugin_test.go | 0 plugins/manager/internal.go | 8 ++++---- plugins/plugin.go | 3 ++- 6 files changed, 25 insertions(+), 24 deletions(-) delete mode 100644 plugins/builtin/strategy/none/main.go create mode 100644 plugins/builtin/strategy/pass-thru/main.go rename plugins/builtin/strategy/{none => pass-thru}/plugin/plugin.go (94%) rename plugins/builtin/strategy/{none => pass-thru}/plugin/plugin_test.go (100%) diff --git a/plugins/builtin/strategy/none/main.go b/plugins/builtin/strategy/none/main.go deleted file mode 100644 index 1e2c1679..00000000 --- a/plugins/builtin/strategy/none/main.go +++ /dev/null @@ -1,16 +0,0 @@ -package main - -import ( - hclog "github.com/hashicorp/go-hclog" - "github.com/hashicorp/nomad-autoscaler/plugins" - targetvalue "github.com/hashicorp/nomad-autoscaler/plugins/builtin/strategy/target-value/plugin" -) - -func main() { - plugins.Serve(factory) -} - -// factory returns a new instance of the TargetValue Strategy plugin. -func factory(log hclog.Logger) interface{} { - return targetvalue.NewTargetValuePlugin(log) -} diff --git a/plugins/builtin/strategy/pass-thru/main.go b/plugins/builtin/strategy/pass-thru/main.go new file mode 100644 index 00000000..7cc677a1 --- /dev/null +++ b/plugins/builtin/strategy/pass-thru/main.go @@ -0,0 +1,16 @@ +package main + +import ( + hclog "github.com/hashicorp/go-hclog" + "github.com/hashicorp/nomad-autoscaler/plugins" + passthru "github.com/hashicorp/nomad-autoscaler/plugins/builtin/strategy/pass-thru/plugin" +) + +func main() { + plugins.Serve(factory) +} + +// factory returns a new instance of the PassThru Strategy plugin. +func factory(log hclog.Logger) interface{} { + return passthru.NewPassThruPlugin(log) +} diff --git a/plugins/builtin/strategy/none/plugin/plugin.go b/plugins/builtin/strategy/pass-thru/plugin/plugin.go similarity index 94% rename from plugins/builtin/strategy/none/plugin/plugin.go rename to plugins/builtin/strategy/pass-thru/plugin/plugin.go index 5c5e7dc8..1620918e 100644 --- a/plugins/builtin/strategy/none/plugin/plugin.go +++ b/plugins/builtin/strategy/pass-thru/plugin/plugin.go @@ -13,7 +13,7 @@ import ( const ( // pluginName is the unique name of the this plugin amongst strategy // plugins. - pluginName = "none" + pluginName = "pass-thru" ) var ( @@ -23,7 +23,7 @@ var ( } PluginConfig = &plugins.InternalPluginConfig{ - Factory: func(l hclog.Logger) interface{} { return NewTargetValuePlugin(l) }, + Factory: func(l hclog.Logger) interface{} { return NewPassThruPlugin(l) }, } pluginInfo = &base.PluginInfo{ @@ -43,7 +43,7 @@ type StrategyPlugin struct { // NewTargetValuePlugin returns the None implementation of the // strategy.Strategy interface. -func NewTargetValuePlugin(log hclog.Logger) strategy.Strategy { +func NewPassThruPlugin(log hclog.Logger) strategy.Strategy { return &StrategyPlugin{ logger: log, } diff --git a/plugins/builtin/strategy/none/plugin/plugin_test.go b/plugins/builtin/strategy/pass-thru/plugin/plugin_test.go similarity index 100% rename from plugins/builtin/strategy/none/plugin/plugin_test.go rename to plugins/builtin/strategy/pass-thru/plugin/plugin_test.go diff --git a/plugins/manager/internal.go b/plugins/manager/internal.go index aabbfa65..6c4fd793 100644 --- a/plugins/manager/internal.go +++ b/plugins/manager/internal.go @@ -14,7 +14,7 @@ import ( azureVMSS "github.com/hashicorp/nomad-autoscaler/plugins/builtin/target/azure-vmss/plugin" gceMIG "github.com/hashicorp/nomad-autoscaler/plugins/builtin/target/gce-mig/plugin" nomadTarget "github.com/hashicorp/nomad-autoscaler/plugins/builtin/target/nomad/plugin" - none "github.com/hashicorp/nomad-autoscaler/plugins/builtin/strategy/none/plugin" + passthru "github.com/hashicorp/nomad-autoscaler/plugins/builtin/strategy/passthru/plugin" ) // loadInternalPlugin takes the plugin configuration and attempts to load it @@ -30,9 +30,9 @@ func (pm *PluginManager) loadInternalPlugin(cfg *config.Plugin, pluginType strin case plugins.InternalTargetNomad: info.factory = nomadTarget.PluginConfig.Factory info.driver = "nomad-target" - case plugins.InternalStrategyNone: + case plugins.InternalStrategyPassThru: info.factory = none.PluginConfig.Factory - info.driver = "none" + info.driver = "pass-thru" case plugins.InternalStrategyTargetValue: info.factory = targetValue.PluginConfig.Factory info.driver = "target-value" @@ -90,7 +90,7 @@ func (pm *PluginManager) useInternal(plugin string) bool { case plugins.InternalAPMNomad, plugins.InternalTargetNomad, plugins.InternalAPMPrometheus, - plugins.InternalStrategyNone, + plugins.InternalStrategyPassThru, plugins.InternalStrategyTargetValue, plugins.InternalTargetAWSASG, plugins.InternalTargetAzureVMSS, diff --git a/plugins/plugin.go b/plugins/plugin.go index 8c736307..770a4288 100644 --- a/plugins/plugin.go +++ b/plugins/plugin.go @@ -23,7 +23,8 @@ const ( // InternalAPMPrometheus is the Prometheus APM internal plugin name. InternalAPMPrometheus = "prometheus" - InternalStrategyNone = "none" + // InternalStrategyPassThru is the Pass Thru strategy internal plugin name. + InternalStrategyPassThru = "pass-thru" // InternalStrategyTargetValue is the Target Value Strategy internal plugin // name. From 793e375961c9e5bee751689deb0a50ca7474ad6f Mon Sep 17 00:00:00 2001 From: Jasmine Dahilig Date: Tue, 30 Mar 2021 08:21:00 -0700 Subject: [PATCH 3/6] fix bug oops --- plugins/manager/internal.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/manager/internal.go b/plugins/manager/internal.go index 6c4fd793..80fcbfb1 100644 --- a/plugins/manager/internal.go +++ b/plugins/manager/internal.go @@ -14,7 +14,7 @@ import ( azureVMSS "github.com/hashicorp/nomad-autoscaler/plugins/builtin/target/azure-vmss/plugin" gceMIG "github.com/hashicorp/nomad-autoscaler/plugins/builtin/target/gce-mig/plugin" nomadTarget "github.com/hashicorp/nomad-autoscaler/plugins/builtin/target/nomad/plugin" - passthru "github.com/hashicorp/nomad-autoscaler/plugins/builtin/strategy/passthru/plugin" + passthru "github.com/hashicorp/nomad-autoscaler/plugins/builtin/strategy/pass-thru/plugin" ) // loadInternalPlugin takes the plugin configuration and attempts to load it @@ -31,7 +31,7 @@ func (pm *PluginManager) loadInternalPlugin(cfg *config.Plugin, pluginType strin info.factory = nomadTarget.PluginConfig.Factory info.driver = "nomad-target" case plugins.InternalStrategyPassThru: - info.factory = none.PluginConfig.Factory + info.factory = passthru.PluginConfig.Factory info.driver = "pass-thru" case plugins.InternalStrategyTargetValue: info.factory = targetValue.PluginConfig.Factory From ef99f4d14e46e87a67802d6db71dd6f19b103f2e Mon Sep 17 00:00:00 2001 From: Jasmine Dahilig Date: Wed, 31 Mar 2021 13:24:37 -0700 Subject: [PATCH 4/6] code review fixes: rename to pass-through, tests, lint --- plugins/builtin/strategy/pass-through/main.go | 16 + .../plugin/plugin.go | 15 +- .../pass-through/plugin/plugin_test.go | 143 +++++++ plugins/builtin/strategy/pass-thru/main.go | 16 - .../strategy/pass-thru/plugin/plugin_test.go | 351 ------------------ plugins/manager/internal.go | 10 +- plugins/plugin.go | 4 +- 7 files changed, 171 insertions(+), 384 deletions(-) create mode 100644 plugins/builtin/strategy/pass-through/main.go rename plugins/builtin/strategy/{pass-thru => pass-through}/plugin/plugin.go (81%) create mode 100644 plugins/builtin/strategy/pass-through/plugin/plugin_test.go delete mode 100644 plugins/builtin/strategy/pass-thru/main.go delete mode 100644 plugins/builtin/strategy/pass-thru/plugin/plugin_test.go diff --git a/plugins/builtin/strategy/pass-through/main.go b/plugins/builtin/strategy/pass-through/main.go new file mode 100644 index 00000000..96f83128 --- /dev/null +++ b/plugins/builtin/strategy/pass-through/main.go @@ -0,0 +1,16 @@ +package main + +import ( + hclog "github.com/hashicorp/go-hclog" + "github.com/hashicorp/nomad-autoscaler/plugins" + passthrough "github.com/hashicorp/nomad-autoscaler/plugins/builtin/strategy/pass-through/plugin" +) + +func main() { + plugins.Serve(factory) +} + +// factory returns a new instance of the PassThrough Strategy plugin. +func factory(log hclog.Logger) interface{} { + return passthrough.NewPassThroughPlugin(log) +} diff --git a/plugins/builtin/strategy/pass-thru/plugin/plugin.go b/plugins/builtin/strategy/pass-through/plugin/plugin.go similarity index 81% rename from plugins/builtin/strategy/pass-thru/plugin/plugin.go rename to plugins/builtin/strategy/pass-through/plugin/plugin.go index 1620918e..c010dd2d 100644 --- a/plugins/builtin/strategy/pass-thru/plugin/plugin.go +++ b/plugins/builtin/strategy/pass-through/plugin/plugin.go @@ -13,7 +13,7 @@ import ( const ( // pluginName is the unique name of the this plugin amongst strategy // plugins. - pluginName = "pass-thru" + pluginName = "pass-through" ) var ( @@ -23,7 +23,7 @@ var ( } PluginConfig = &plugins.InternalPluginConfig{ - Factory: func(l hclog.Logger) interface{} { return NewPassThruPlugin(l) }, + Factory: func(l hclog.Logger) interface{} { return NewPassThroughPlugin(l) }, } pluginInfo = &base.PluginInfo{ @@ -41,9 +41,9 @@ type StrategyPlugin struct { logger hclog.Logger } -// NewTargetValuePlugin returns the None implementation of the +// NewPassThroughPlugin returns the Pass Through implementation of the // strategy.Strategy interface. -func NewPassThruPlugin(log hclog.Logger) strategy.Strategy { +func NewPassThroughPlugin(log hclog.Logger) strategy.Strategy { return &StrategyPlugin{ logger: log, } @@ -66,7 +66,6 @@ func (s *StrategyPlugin) Run(eval *sdk.ScalingCheckEvaluation, count int64) (*sd metric := eval.Metrics[len(eval.Metrics)-1] - // TODO: compare count > metric --> direction // Identify the direction of scaling, if any. eval.Action.Direction = s.calculateDirection(count, metric.Value) if eval.Action.Direction == sdk.ScaleDirectionNone { @@ -81,11 +80,7 @@ func (s *StrategyPlugin) Run(eval *sdk.ScalingCheckEvaluation, count int64) (*sd } // calculateDirection is used to calculate the direction of scaling that should -// occur, if any at all. It takes into account the current task group count in -// order to correctly account for 0 counts. -// -// The input factor value is padded by e, such that no action will be taken if -// factor is within [1-e; 1+e]. +// occur, if any at all. func (s *StrategyPlugin) calculateDirection(count int64, metric float64) sdk.ScaleDirection { if metric > float64(count) { return sdk.ScaleDirectionUp diff --git a/plugins/builtin/strategy/pass-through/plugin/plugin_test.go b/plugins/builtin/strategy/pass-through/plugin/plugin_test.go new file mode 100644 index 00000000..39e6cd46 --- /dev/null +++ b/plugins/builtin/strategy/pass-through/plugin/plugin_test.go @@ -0,0 +1,143 @@ +package plugin + +import ( + "testing" + + hclog "github.com/hashicorp/go-hclog" + "github.com/hashicorp/nomad-autoscaler/plugins/base" + "github.com/hashicorp/nomad-autoscaler/sdk" + "github.com/stretchr/testify/assert" +) + +func TestStrategyPlugin_SetConfig(t *testing.T) { + s := &StrategyPlugin{} + expectedOutput := map[string]string{"example-item": "example-value"} + err := s.SetConfig(expectedOutput) + assert.Nil(t, err) +} + +func TestStrategyPlugin_PluginInfo(t *testing.T) { + s := &StrategyPlugin{} + expectedOutput := &base.PluginInfo{Name: "pass-through", PluginType: "strategy"} + actualOutput, err := s.PluginInfo() + assert.Nil(t, err) + assert.Equal(t, expectedOutput, actualOutput) +} + +func TestStrategyPlugin_Run(t *testing.T) { + testCases := []struct { + inputEval *sdk.ScalingCheckEvaluation + inputCount int64 + expectedResp *sdk.ScalingCheckEvaluation + expectedError error + name string + }{ + { + inputEval: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 13}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{}, + }, + }, + Action: &sdk.ScalingAction{}, + }, + inputCount: 2, + expectedResp: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 13}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{}, + }, + }, + Action: &sdk.ScalingAction{ + Count: 13, + Direction: sdk.ScaleDirectionUp, + Reason: "scaling up because metric is 13", + }, + }, + expectedError: nil, + name: "pass-through scale up", + }, + { + inputEval: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 0}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{}, + }, + }, + Action: &sdk.ScalingAction{}, + }, + inputCount: 2, + expectedResp: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 0}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{}, + }, + }, + Action: &sdk.ScalingAction{ + Count: 0, + Direction: sdk.ScaleDirectionDown, + Reason: "scaling down because metric is 0", + }, + }, + expectedError: nil, + name: "pass-through scale down to 0", + }, + { + inputEval: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 10}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{}, + }, + }, + Action: &sdk.ScalingAction{}, + }, + inputCount: 10, + expectedResp: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 10}}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{}, + }, + }, + Action: &sdk.ScalingAction{ + Direction: sdk.ScaleDirectionNone, + }, + }, + expectedError: nil, + name: "no scaling - current count same as metric", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + s := &StrategyPlugin{logger: hclog.NewNullLogger()} + actualResp, actualError := s.Run(tc.inputEval, tc.inputCount) + assert.Equal(t, tc.expectedResp, actualResp, tc.name) + assert.Equal(t, tc.expectedError, actualError, tc.name) + }) + } +} + +func TestStrategyPlugin_calculateDirection(t *testing.T) { + testCases := []struct { + inputCount int64 + inputMetric float64 + threshold float64 + expectedOutput sdk.ScaleDirection + }{ + {inputCount: 0, inputMetric: 1, expectedOutput: sdk.ScaleDirectionUp}, + {inputCount: 5, inputMetric: 5, expectedOutput: sdk.ScaleDirectionNone}, + {inputCount: 4, inputMetric: 0, expectedOutput: sdk.ScaleDirectionDown}, + } + + s := &StrategyPlugin{} + + for _, tc := range testCases { + assert.Equal(t, tc.expectedOutput, s.calculateDirection(tc.inputCount, tc.inputMetric)) + } +} diff --git a/plugins/builtin/strategy/pass-thru/main.go b/plugins/builtin/strategy/pass-thru/main.go deleted file mode 100644 index 7cc677a1..00000000 --- a/plugins/builtin/strategy/pass-thru/main.go +++ /dev/null @@ -1,16 +0,0 @@ -package main - -import ( - hclog "github.com/hashicorp/go-hclog" - "github.com/hashicorp/nomad-autoscaler/plugins" - passthru "github.com/hashicorp/nomad-autoscaler/plugins/builtin/strategy/pass-thru/plugin" -) - -func main() { - plugins.Serve(factory) -} - -// factory returns a new instance of the PassThru Strategy plugin. -func factory(log hclog.Logger) interface{} { - return passthru.NewPassThruPlugin(log) -} diff --git a/plugins/builtin/strategy/pass-thru/plugin/plugin_test.go b/plugins/builtin/strategy/pass-thru/plugin/plugin_test.go deleted file mode 100644 index 5cbbdfe0..00000000 --- a/plugins/builtin/strategy/pass-thru/plugin/plugin_test.go +++ /dev/null @@ -1,351 +0,0 @@ -package plugin - -import ( - "fmt" - "testing" - "time" - - hclog "github.com/hashicorp/go-hclog" - "github.com/hashicorp/nomad-autoscaler/plugins/base" - "github.com/hashicorp/nomad-autoscaler/sdk" - "github.com/stretchr/testify/assert" -) - -func TestStrategyPlugin_SetConfig(t *testing.T) { - s := &StrategyPlugin{} - expectedOutput := map[string]string{"example-item": "example-value"} - err := s.SetConfig(expectedOutput) - assert.Nil(t, err) - assert.Equal(t, expectedOutput, s.config) -} - -func TestStrategyPlugin_PluginInfo(t *testing.T) { - s := &StrategyPlugin{} - expectedOutput := &base.PluginInfo{Name: "target-value", PluginType: "strategy"} - actualOutput, err := s.PluginInfo() - assert.Nil(t, err) - assert.Equal(t, expectedOutput, actualOutput) -} - -func TestStrategyPlugin_Run(t *testing.T) { - testCases := []struct { - inputEval *sdk.ScalingCheckEvaluation - inputCount int64 - expectedResp *sdk.ScalingCheckEvaluation - expectedError error - name string - }{ - { - inputEval: &sdk.ScalingCheckEvaluation{ - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{}, - }, - }, - expectedResp: nil, - expectedError: fmt.Errorf("missing required field `target`"), - name: "incorrect strategy input config", - }, - { - inputEval: &sdk.ScalingCheckEvaluation{ - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{ - Config: map[string]string{"target": "not-the-float-you're-looking-for"}, - }, - }, - }, - expectedResp: nil, - expectedError: fmt.Errorf("invalid value for `target`: not-the-float-you're-looking-for (string)"), - name: "incorrect input strategy config target value", - }, - { - inputEval: &sdk.ScalingCheckEvaluation{ - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{ - Config: map[string]string{"target": "0", "threshold": "not-the-float-you're-looking-for"}, - }, - }, - }, - expectedResp: nil, - expectedError: fmt.Errorf("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": "13"}, - }, - }, - Action: &sdk.ScalingAction{}, - }, - inputCount: 2, - expectedResp: &sdk.ScalingCheckEvaluation{ - Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 13}}, - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{ - Config: map[string]string{"target": "13"}, - }, - }, - Action: &sdk.ScalingAction{ - Direction: sdk.ScaleDirectionNone, - }, - }, - expectedError: nil, - name: "factor equals 1 with non-zero count", - }, - { - inputEval: &sdk.ScalingCheckEvaluation{ - Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 26}}, - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{ - Config: map[string]string{"target": "13"}, - }, - }, - Action: &sdk.ScalingAction{}, - }, - inputCount: 2, - expectedResp: &sdk.ScalingCheckEvaluation{ - Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 26}}, - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{ - Config: map[string]string{"target": "13"}, - }, - }, - Action: &sdk.ScalingAction{ - Count: 4, - Reason: "scaling up because factor is 2.000000", - Direction: sdk.ScaleDirectionUp, - }, - }, - expectedError: nil, - name: "factor greater than 1 with non-zero count", - }, - { - inputEval: &sdk.ScalingCheckEvaluation{ - Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 20}}, - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{ - Config: map[string]string{"target": "10"}, - }, - }, - Action: &sdk.ScalingAction{}, - }, - inputCount: 0, - expectedResp: &sdk.ScalingCheckEvaluation{ - Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 20}}, - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{ - Config: map[string]string{"target": "10"}, - }, - }, - Action: &sdk.ScalingAction{ - Count: 2, - Reason: "scaling up because factor is 2.000000", - Direction: sdk.ScaleDirectionUp, - }, - }, - expectedError: nil, - name: "scale from 0 with non-zero target", - }, - { - inputEval: &sdk.ScalingCheckEvaluation{ - Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 9}}, - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{ - Config: map[string]string{"target": "10"}, - }, - }, - Action: &sdk.ScalingAction{}, - }, - inputCount: 1, - expectedResp: &sdk.ScalingCheckEvaluation{ - Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 9}}, - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{ - Config: map[string]string{"target": "10"}, - }, - }, - Action: &sdk.ScalingAction{ - Direction: sdk.ScaleDirectionNone, - }, - }, - expectedError: nil, - name: "no scaling based on small target/metric difference", - }, - { - inputEval: &sdk.ScalingCheckEvaluation{ - Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 1}}, - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{ - Config: map[string]string{"target": "10"}, - }, - }, - Action: &sdk.ScalingAction{}, - }, - inputCount: 0, - expectedResp: &sdk.ScalingCheckEvaluation{ - Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 1}}, - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{ - Config: map[string]string{"target": "10"}, - }, - }, - Action: &sdk.ScalingAction{ - Count: 1, - Reason: "scaling up because factor is 0.100000", - Direction: sdk.ScaleDirectionUp, - }, - }, - expectedError: nil, - name: "scale from 0 with 0 target eg. build queue with 0 running build agents", - }, - { - inputEval: &sdk.ScalingCheckEvaluation{ - Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 0}}, - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{ - Config: map[string]string{"target": "0"}, - }, - }, - Action: &sdk.ScalingAction{}, - }, - inputCount: 5, - expectedResp: &sdk.ScalingCheckEvaluation{ - Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 0}}, - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{ - Config: map[string]string{"target": "0"}, - }, - }, - Action: &sdk.ScalingAction{ - Count: 0, - Direction: sdk.ScaleDirectionDown, - Reason: "scaling down because factor is 0.000000", - }, - }, - expectedError: nil, - name: "scale to 0 with 0 target eg. idle build agents", - }, - { - inputEval: &sdk.ScalingCheckEvaluation{ - Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 5.00001}}, - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{ - Config: map[string]string{"target": "5"}, - }, - }, - Action: &sdk.ScalingAction{}, - }, - inputCount: 8, - expectedResp: &sdk.ScalingCheckEvaluation{ - Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 5.00001}}, - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{ - Config: map[string]string{"target": "5"}, - }, - }, - Action: &sdk.ScalingAction{ - Direction: sdk.ScaleDirectionNone, - }, - }, - expectedError: nil, - name: "don't scale when the factor change is small", - }, - { - inputEval: &sdk.ScalingCheckEvaluation{ - Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 5.00001}}, - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{ - Config: map[string]string{"target": "5", "threshold": "0.000001"}, - }, - }, - Action: &sdk.ScalingAction{}, - }, - inputCount: 8, - expectedResp: &sdk.ScalingCheckEvaluation{ - Metrics: sdk.TimestampedMetrics{sdk.TimestampedMetric{Value: 5.00001}}, - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{ - Config: map[string]string{"target": "5", "threshold": "0.000001"}, - }, - }, - Action: &sdk.ScalingAction{ - Count: 9, - Reason: "scaling up because factor is 1.000002", - Direction: sdk.ScaleDirectionUp, - }, - }, - expectedError: nil, - name: "scale up on small changes if threshold is small", - }, - { - inputEval: &sdk.ScalingCheckEvaluation{ - Metrics: sdk.TimestampedMetrics{ - sdk.TimestampedMetric{Value: 13, Timestamp: time.Unix(1600000000, 0)}, - sdk.TimestampedMetric{Value: 14, Timestamp: time.Unix(1600000001, 0)}, - sdk.TimestampedMetric{Value: 15, Timestamp: time.Unix(1600000002, 0)}, - sdk.TimestampedMetric{Value: 16, Timestamp: time.Unix(1600000003, 0)}, - }, - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{ - Config: map[string]string{"target": "16"}, - }, - }, - Action: &sdk.ScalingAction{}, - }, - inputCount: 2, - expectedResp: &sdk.ScalingCheckEvaluation{ - Metrics: sdk.TimestampedMetrics{ - sdk.TimestampedMetric{Value: 13, Timestamp: time.Unix(1600000000, 0)}, - sdk.TimestampedMetric{Value: 14, Timestamp: time.Unix(1600000001, 0)}, - sdk.TimestampedMetric{Value: 15, Timestamp: time.Unix(1600000002, 0)}, - sdk.TimestampedMetric{Value: 16, Timestamp: time.Unix(1600000003, 0)}, - }, - Check: &sdk.ScalingPolicyCheck{ - Strategy: &sdk.ScalingPolicyStrategy{ - Config: map[string]string{"target": "16"}, - }, - }, - Action: &sdk.ScalingAction{ - Direction: sdk.ScaleDirectionNone, - }, - }, - expectedError: nil, - name: "properly handle multiple input", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - s := &StrategyPlugin{logger: hclog.NewNullLogger()} - actualResp, actualError := s.Run(tc.inputEval, tc.inputCount) - assert.Equal(t, tc.expectedResp, actualResp, tc.name) - assert.Equal(t, tc.expectedError, actualError, tc.name) - }) - } -} - -func TestStrategyPlugin_calculateDirection(t *testing.T) { - testCases := []struct { - inputCount int64 - inputFactor float64 - threshold float64 - expectedOutput sdk.ScaleDirection - }{ - {inputCount: 0, inputFactor: 1, expectedOutput: sdk.ScaleDirectionUp}, - {inputCount: 5, inputFactor: 1, expectedOutput: sdk.ScaleDirectionNone}, - {inputCount: 4, inputFactor: 0.5, expectedOutput: sdk.ScaleDirectionDown}, - {inputCount: 5, inputFactor: 2, expectedOutput: sdk.ScaleDirectionUp}, - {inputCount: 5, inputFactor: 1.0001, threshold: 0.01, expectedOutput: sdk.ScaleDirectionNone}, - {inputCount: 5, inputFactor: 1.02, threshold: 0.01, expectedOutput: sdk.ScaleDirectionUp}, - {inputCount: 5, inputFactor: 0.99, threshold: 0.01, expectedOutput: sdk.ScaleDirectionNone}, - {inputCount: 5, inputFactor: 0.98, threshold: 0.01, expectedOutput: sdk.ScaleDirectionDown}, - } - - s := &StrategyPlugin{} - - for _, tc := range testCases { - assert.Equal(t, tc.expectedOutput, s.calculateDirection(tc.inputCount, tc.inputFactor, tc.threshold)) - } -} diff --git a/plugins/manager/internal.go b/plugins/manager/internal.go index 80fcbfb1..9bc67014 100644 --- a/plugins/manager/internal.go +++ b/plugins/manager/internal.go @@ -9,12 +9,12 @@ import ( datadog "github.com/hashicorp/nomad-autoscaler/plugins/builtin/apm/datadog/plugin" nomadAPM "github.com/hashicorp/nomad-autoscaler/plugins/builtin/apm/nomad/plugin" prometheus "github.com/hashicorp/nomad-autoscaler/plugins/builtin/apm/prometheus/plugin" + passthrough "github.com/hashicorp/nomad-autoscaler/plugins/builtin/strategy/pass-through/plugin" targetValue "github.com/hashicorp/nomad-autoscaler/plugins/builtin/strategy/target-value/plugin" awsASG "github.com/hashicorp/nomad-autoscaler/plugins/builtin/target/aws-asg/plugin" azureVMSS "github.com/hashicorp/nomad-autoscaler/plugins/builtin/target/azure-vmss/plugin" gceMIG "github.com/hashicorp/nomad-autoscaler/plugins/builtin/target/gce-mig/plugin" nomadTarget "github.com/hashicorp/nomad-autoscaler/plugins/builtin/target/nomad/plugin" - passthru "github.com/hashicorp/nomad-autoscaler/plugins/builtin/strategy/pass-thru/plugin" ) // loadInternalPlugin takes the plugin configuration and attempts to load it @@ -30,9 +30,9 @@ func (pm *PluginManager) loadInternalPlugin(cfg *config.Plugin, pluginType strin case plugins.InternalTargetNomad: info.factory = nomadTarget.PluginConfig.Factory info.driver = "nomad-target" - case plugins.InternalStrategyPassThru: - info.factory = passthru.PluginConfig.Factory - info.driver = "pass-thru" + case plugins.InternalStrategyPassThrough: + info.factory = passthrough.PluginConfig.Factory + info.driver = "pass-through" case plugins.InternalStrategyTargetValue: info.factory = targetValue.PluginConfig.Factory info.driver = "target-value" @@ -90,7 +90,7 @@ func (pm *PluginManager) useInternal(plugin string) bool { case plugins.InternalAPMNomad, plugins.InternalTargetNomad, plugins.InternalAPMPrometheus, - plugins.InternalStrategyPassThru, + plugins.InternalStrategyPassThrough, plugins.InternalStrategyTargetValue, plugins.InternalTargetAWSASG, plugins.InternalTargetAzureVMSS, diff --git a/plugins/plugin.go b/plugins/plugin.go index 770a4288..b97f86cd 100644 --- a/plugins/plugin.go +++ b/plugins/plugin.go @@ -23,8 +23,8 @@ const ( // InternalAPMPrometheus is the Prometheus APM internal plugin name. InternalAPMPrometheus = "prometheus" - // InternalStrategyPassThru is the Pass Thru strategy internal plugin name. - InternalStrategyPassThru = "pass-thru" + // InternalStrategyPassThrough is the Pass Through strategy internal plugin name. + InternalStrategyPassThrough = "pass-through" // InternalStrategyTargetValue is the Target Value Strategy internal plugin // name. From 3b5422d6f13bd089acf5143b567dbd98be6e49a8 Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Wed, 31 Mar 2021 17:45:15 -0400 Subject: [PATCH 5/6] run gofmt --- plugins/builtin/strategy/pass-through/plugin/plugin.go | 2 -- .../strategy/pass-through/plugin/plugin_test.go | 10 +++++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/plugins/builtin/strategy/pass-through/plugin/plugin.go b/plugins/builtin/strategy/pass-through/plugin/plugin.go index c010dd2d..b578bfd6 100644 --- a/plugins/builtin/strategy/pass-through/plugin/plugin.go +++ b/plugins/builtin/strategy/pass-through/plugin/plugin.go @@ -65,14 +65,12 @@ func (s *StrategyPlugin) Run(eval *sdk.ScalingCheckEvaluation, count int64) (*sd // Use only the latest value for now. metric := eval.Metrics[len(eval.Metrics)-1] - // Identify the direction of scaling, if any. eval.Action.Direction = s.calculateDirection(count, metric.Value) if eval.Action.Direction == sdk.ScaleDirectionNone { return eval, nil } - eval.Action.Count = int64(metric.Value) eval.Action.Reason = fmt.Sprintf("scaling %s because metric is %d", eval.Action.Direction, eval.Action.Count) diff --git a/plugins/builtin/strategy/pass-through/plugin/plugin_test.go b/plugins/builtin/strategy/pass-through/plugin/plugin_test.go index 39e6cd46..70f6321e 100644 --- a/plugins/builtin/strategy/pass-through/plugin/plugin_test.go +++ b/plugins/builtin/strategy/pass-through/plugin/plugin_test.go @@ -51,9 +51,9 @@ func TestStrategyPlugin_Run(t *testing.T) { }, }, Action: &sdk.ScalingAction{ - Count: 13, + Count: 13, Direction: sdk.ScaleDirectionUp, - Reason: "scaling up because metric is 13", + Reason: "scaling up because metric is 13", }, }, expectedError: nil, @@ -78,9 +78,9 @@ func TestStrategyPlugin_Run(t *testing.T) { }, }, Action: &sdk.ScalingAction{ - Count: 0, + Count: 0, Direction: sdk.ScaleDirectionDown, - Reason: "scaling down because metric is 0", + Reason: "scaling down because metric is 0", }, }, expectedError: nil, @@ -111,7 +111,7 @@ func TestStrategyPlugin_Run(t *testing.T) { expectedError: nil, name: "no scaling - current count same as metric", }, - } + } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { From 6a93e94e0ec2e48420281da47e825eab13229fae Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Wed, 31 Mar 2021 17:55:05 -0400 Subject: [PATCH 6/6] guard pass-through strategy from empty metric input --- .../strategy/pass-through/plugin/plugin.go | 6 +++++ .../pass-through/plugin/plugin_test.go | 25 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/plugins/builtin/strategy/pass-through/plugin/plugin.go b/plugins/builtin/strategy/pass-through/plugin/plugin.go index b578bfd6..a49d5d37 100644 --- a/plugins/builtin/strategy/pass-through/plugin/plugin.go +++ b/plugins/builtin/strategy/pass-through/plugin/plugin.go @@ -62,6 +62,12 @@ func (s *StrategyPlugin) PluginInfo() (*base.PluginInfo, error) { // Run satisfies the Run function on the strategy.Strategy interface. func (s *StrategyPlugin) Run(eval *sdk.ScalingCheckEvaluation, count int64) (*sdk.ScalingCheckEvaluation, error) { + // This shouldn't happen, but check it just in case. + if len(eval.Metrics) == 0 { + eval.Action.Direction = sdk.ScaleDirectionNone + return eval, nil + } + // Use only the latest value for now. metric := eval.Metrics[len(eval.Metrics)-1] diff --git a/plugins/builtin/strategy/pass-through/plugin/plugin_test.go b/plugins/builtin/strategy/pass-through/plugin/plugin_test.go index 70f6321e..c562d3a8 100644 --- a/plugins/builtin/strategy/pass-through/plugin/plugin_test.go +++ b/plugins/builtin/strategy/pass-through/plugin/plugin_test.go @@ -111,6 +111,31 @@ func TestStrategyPlugin_Run(t *testing.T) { expectedError: nil, name: "no scaling - current count same as metric", }, + { + inputEval: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{}, + }, + }, + Action: &sdk.ScalingAction{}, + }, + inputCount: 10, + expectedResp: &sdk.ScalingCheckEvaluation{ + Metrics: sdk.TimestampedMetrics{}, + Check: &sdk.ScalingPolicyCheck{ + Strategy: &sdk.ScalingPolicyStrategy{ + Config: map[string]string{}, + }, + }, + Action: &sdk.ScalingAction{ + Direction: sdk.ScaleDirectionNone, + }, + }, + expectedError: nil, + name: "no scaling - empty metric timeseries", + }, } for _, tc := range testCases {