diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f93bb90..a31996ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ ## UNRELEASED +__BACKWARDS INCOMPATIBILITIES:__ + * agent: configuration `scan_interval` renamed to `default_evaluation_interval` [[GH-114](https://github.com/hashicorp/nomad-autoscaler/pull/114)] + FEATURES: * agent: allow policies to specify `evaluation_interval` [[GH-30](https://github.com/hashicorp/nomad-autoscaler/pull/30)] diff --git a/agent/agent.go b/agent/agent.go index 06419478..bbe6b563 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -54,7 +54,10 @@ func (a *Agent) Run(ctx context.Context) error { a.healthServer = healthServer go a.healthServer.run() - source := nomadpolicy.NewNomadSource(a.logger, a.nomadClient) + sourceConfig := &nomadpolicy.SourceConfig{ + DefaultEvaluationInterval: a.config.DefaultEvaluationInterval, + } + source := nomadpolicy.NewNomadSource(a.logger, a.nomadClient, sourceConfig) manager := policy.NewManager(a.logger, source, a.pluginManager) policyEvalCh := make(chan *policy.Evaluation, 10) diff --git a/agent/config/config.go b/agent/config/config.go index 21dfe0d5..3b79b1bb 100644 --- a/agent/config/config.go +++ b/agent/config/config.go @@ -35,10 +35,10 @@ type Agent struct { // PluginDir is the directory that holds the autoscaler plugin binaries. PluginDir string `hcl:"plugin_dir,optional"` - // ScanInterval is the time duration interval at which the autoscaler runs - // evaluations. - ScanInterval time.Duration - ScanIntervalHCL string `hcl:"scan_interval,optional" json:"-"` + // DefaultEvaluationInterval is the time duration interval used when + // `evaluation_interval` is not defined in a policy. + DefaultEvaluationInterval time.Duration + DefaultEvaluationIntervalHCL string `hcl:"default_evaluation_interval,optional" json:"-"` // HTTP is the configuration used to setup the HTTP health server. HTTP *HTTP `hcl:"http,block"` @@ -124,9 +124,9 @@ const ( // defaultHTTPBindPort is the default port used for the HTTP health server. defaultHTTPBindPort = 8080 - // defaultScanInterval is the default value for the ScaInterval in nano - // seconds. - defaultScanInterval = time.Duration(10000000000) + // defaultEvaluationInterval is the default value for the + // DefaultEvaluationInterval in nano seconds. + defaultEvaluationInterval = time.Duration(10000000000) // defaultPluginDirSuffix is the suffix appended to the PWD when building // the PluginDir default value. @@ -152,9 +152,9 @@ func Default() (*Agent, error) { } return &Agent{ - LogLevel: defaultLogLevel, - PluginDir: pwd + defaultPluginDirSuffix, - ScanInterval: defaultScanInterval, + LogLevel: defaultLogLevel, + PluginDir: pwd + defaultPluginDirSuffix, + DefaultEvaluationInterval: defaultEvaluationInterval, HTTP: &HTTP{ BindAddress: defaultHTTPBindAddress, BindPort: defaultHTTPBindPort, @@ -181,8 +181,8 @@ func (a *Agent) Merge(b *Agent) *Agent { if b.PluginDir != "" { result.PluginDir = b.PluginDir } - if b.ScanInterval != 0 { - result.ScanInterval = b.ScanInterval + if b.DefaultEvaluationInterval != 0 { + result.DefaultEvaluationInterval = b.DefaultEvaluationInterval } if b.HTTP != nil { @@ -349,12 +349,12 @@ func parseFile(file string, cfg *Agent) error { return err } - if cfg.ScanIntervalHCL != "" { - d, err := time.ParseDuration(cfg.ScanIntervalHCL) + if cfg.DefaultEvaluationIntervalHCL != "" { + d, err := time.ParseDuration(cfg.DefaultEvaluationIntervalHCL) if err != nil { return err } - cfg.ScanInterval = d + cfg.DefaultEvaluationInterval = d } return nil } diff --git a/agent/config/config_test.go b/agent/config/config_test.go index 0f1c92b5..18b46267 100644 --- a/agent/config/config_test.go +++ b/agent/config/config_test.go @@ -18,7 +18,7 @@ func Test_Default(t *testing.T) { assert.False(t, def.LogJson) assert.Equal(t, def.LogLevel, "info") assert.True(t, strings.HasSuffix(def.PluginDir, "/plugins")) - assert.Equal(t, def.ScanInterval, time.Duration(10000000000)) + assert.Equal(t, def.DefaultEvaluationInterval, time.Duration(10000000000)) assert.Equal(t, def.Nomad.Address, "http://127.0.0.1:4646") assert.Equal(t, "127.0.0.1", def.HTTP.BindAddress) assert.Equal(t, 8080, def.HTTP.BindPort) @@ -31,8 +31,8 @@ func TestAgent_Merge(t *testing.T) { assert.Nil(t, err) cfg1 := &Agent{ - PluginDir: "/opt/nomad-autoscaler/plugins", - ScanInterval: 5000000000, + PluginDir: "/opt/nomad-autoscaler/plugins", + DefaultEvaluationInterval: 5000000000, HTTP: &HTTP{ BindAddress: "scaler.nomad", }, @@ -49,10 +49,10 @@ func TestAgent_Merge(t *testing.T) { } cfg2 := &Agent{ - LogLevel: "trace", - LogJson: true, - PluginDir: "/var/lib/nomad-autoscaler/plugins", - ScanInterval: 10000000000, + LogLevel: "trace", + LogJson: true, + PluginDir: "/var/lib/nomad-autoscaler/plugins", + DefaultEvaluationInterval: 10000000000, HTTP: &HTTP{ BindPort: 4646, }, @@ -90,10 +90,10 @@ func TestAgent_Merge(t *testing.T) { } expectedResult := &Agent{ - LogLevel: "trace", - LogJson: true, - PluginDir: "/var/lib/nomad-autoscaler/plugins", - ScanInterval: 10000000000, + LogLevel: "trace", + LogJson: true, + PluginDir: "/var/lib/nomad-autoscaler/plugins", + DefaultEvaluationInterval: 10000000000, HTTP: &HTTP{ BindAddress: "scaler.nomad", BindPort: 4646, @@ -149,7 +149,7 @@ func TestAgent_Merge(t *testing.T) { assert.Equal(t, expectedResult.LogLevel, actualResult.LogLevel) assert.Equal(t, expectedResult.Nomad, actualResult.Nomad) assert.Equal(t, expectedResult.PluginDir, actualResult.PluginDir) - assert.Equal(t, expectedResult.ScanInterval, actualResult.ScanInterval) + assert.Equal(t, expectedResult.DefaultEvaluationInterval, actualResult.DefaultEvaluationInterval) assert.ElementsMatch(t, expectedResult.APMs, actualResult.APMs) assert.ElementsMatch(t, expectedResult.Targets, actualResult.Targets) assert.ElementsMatch(t, expectedResult.Strategies, actualResult.Strategies) @@ -181,11 +181,11 @@ func TestAgent_parseFile(t *testing.T) { // Write some valid content, and ensure this is read and parsed. cfg := &Agent{} - if _, err := fh.WriteString("scan_interval = \"10s\"\nplugin_dir = \"/opt/nomad-autoscaler/plugins\""); err != nil { + if _, err := fh.WriteString("default_evaluation_interval = \"10s\"\nplugin_dir = \"/opt/nomad-autoscaler/plugins\""); err != nil { t.Fatalf("err: %s", err) } assert.Nil(t, parseFile(fh.Name(), cfg)) - assert.Equal(t, time.Duration(10000000000), cfg.ScanInterval) + assert.Equal(t, time.Duration(10000000000), cfg.DefaultEvaluationInterval) assert.Equal(t, "/opt/nomad-autoscaler/plugins", cfg.PluginDir) } @@ -198,13 +198,13 @@ func TestConfig_Load(t *testing.T) { assert.Nil(t, err) defer os.Remove(fh.Name()) - _, err = fh.WriteString("scan_interval = \"10s\"") + _, err = fh.WriteString("default_evaluation_interval = \"10s\"") assert.Nil(t, err) // Works on a config file cfg, err := Load(fh.Name()) assert.Nil(t, err) - assert.Equal(t, time.Duration(10000000000), cfg.ScanInterval) + assert.Equal(t, time.Duration(10000000000), cfg.DefaultEvaluationInterval) dir, err := ioutil.TempDir("", "nomad-autoscaler") assert.Nil(t, err) @@ -234,7 +234,7 @@ func TestAgent_loadDir(t *testing.T) { assert.Equal(t, config, &Agent{}) file1 := filepath.Join(dir, "config1.hcl") - assert.Nil(t, ioutil.WriteFile(file1, []byte("scan_interval = \"10s\""), 0600)) + assert.Nil(t, ioutil.WriteFile(file1, []byte("default_evaluation_interval = \"10s\""), 0600)) file2 := filepath.Join(dir, "config2.hcl") assert.Nil(t, ioutil.WriteFile(file2, []byte("plugin_dir = \"/opt/nomad-autoscaler/plugins\""), 0600)) @@ -252,7 +252,7 @@ func TestAgent_loadDir(t *testing.T) { // We should now be able to load as all the configs are valid. cfg, err := loadDir(dir) assert.Nil(t, err) - assert.Equal(t, time.Duration(10000000000), cfg.ScanInterval) + assert.Equal(t, time.Duration(10000000000), cfg.DefaultEvaluationInterval) assert.Equal(t, "/opt/nomad-autoscaler/plugins", cfg.PluginDir) } diff --git a/command/agent.go b/command/agent.go index d4bf86ac..18493765 100644 --- a/command/agent.go +++ b/command/agent.go @@ -104,7 +104,7 @@ Nomad Options: -nomad-skip-verify Do not verify TLS certificates. This is strongly discouraged. - + ` return strings.TrimSpace(helpText) } @@ -165,7 +165,7 @@ func (c *AgentCommand) readConfig() *config.Agent { flags.BoolVar(&cmdConfig.LogJson, "log-json", false, "") flags.StringVar(&cmdConfig.PluginDir, "plugin-dir", "", "") flags.Var((flaghelper.FuncDurationVar)(func(d time.Duration) error { - cmdConfig.ScanInterval = d + cmdConfig.DefaultEvaluationInterval = d return nil }), "scan-interval", "") diff --git a/docs/agent/README.md b/docs/agent/README.md index a6f56854..4a877db2 100644 --- a/docs/agent/README.md +++ b/docs/agent/README.md @@ -39,7 +39,7 @@ As each file is processed, its contents are merged into the existing configurati * `log_level` `(string: "INFO")` - Specify the verbosity level of Nomad Autoscaler's logs. Valid values include `DEBUG`, `INFO`, and `WARN`, in decreasing order of verbosity. * `log_json` `(bool: false)` - Output logs in a JSON format. * `plugin_dir` `(string: "./plugins")` - The plugin directory used to discover Nomad Autoscaler plugins. - * `scan_interval` `(duration: "10s")` - The time to wait between Nomad Autoscaler evaluations. + * `default_evaluation_interval` `(duration: "10s")` - The default time to use when a policy doesn't specify an `evaluation_interval`. ## `http` Block The `http` block configures the Nomad Autoscaler's HTTP health endpoint. diff --git a/policy/nomad/parser.go b/policy/nomad/parser.go index 669faca0..a2c17dff 100644 --- a/policy/nomad/parser.go +++ b/policy/nomad/parser.go @@ -4,107 +4,159 @@ import ( "fmt" "time" - "github.com/hashicorp/nomad-autoscaler/plugins" "github.com/hashicorp/nomad-autoscaler/policy" "github.com/hashicorp/nomad/api" ) -func parsePolicy(p *api.ScalingPolicy) (policy.Policy, error) { - var to policy.Policy +// parsePolicy parses the values on an api.ScalingPolicy into a policy.Policy. +// +// It provides best-effort parsing, with any invalid values being skipped from +// the end result. To avoid missing values use validateScalingPolicy() to +// detect errors first. +func parsePolicy(p *api.ScalingPolicy) policy.Policy { + to := policy.Policy{ + ID: p.ID, + Max: p.Max, + Enabled: true, + Target: parseTarget(p.Policy[keyTarget], p.Target), + Strategy: parseStrategy(p.Policy[keyStrategy]), + } + + // Add non-typed values. + if p.Min != nil { + to.Min = *p.Min + } - if err := validateScalingPolicy(p); err != nil { - return to, err + if query, ok := p.Policy[keyQuery].(string); ok { + to.Query = query } - source := p.Policy[keySource] - if source == nil { - source = "" + if source, ok := p.Policy[keySource].(string); ok { + to.Source = source } - to = policy.Policy{ - ID: p.ID, - Min: *p.Min, - Max: p.Max, - Enabled: *p.Enabled, - Source: source.(string), - Query: p.Policy[keyQuery].(string), - EvaluationInterval: defaultEvaluationInterval, //TODO(luiz): use agent scan interval as default - Target: parseTarget(p.Policy[keyTarget]), - Strategy: parseStrategy(p.Policy[keyStrategy]), + if p.Enabled != nil { + to.Enabled = *p.Enabled } - canonicalizePolicy(p, &to) + // Parse evaluation_interval as time.Duration. + // Ignore error since we assume policy has been validated. + if eval, ok := p.Policy[keyEvaluationInterval].(string); ok { + to.EvaluationInterval, _ = time.ParseDuration(eval) + } - return to, nil + return to } +// parseStrategy parses the content of the strategy block from a policy. +// +// It provides best-effort parsing and will return `nil` in case of errors. +// +// scaling { +// policy { +// strategy = { +// +-------------------+ +// | name = "strategy" | +// | config = { | +// | key = "value" | +// | } | +// +-------------------+ +// } +// } +// } func parseStrategy(s interface{}) *policy.Strategy { - strategyMap := s.([]interface{})[0].(map[string]interface{}) - configMap := strategyMap["config"].([]interface{})[0].(map[string]interface{}) - configMapString := make(map[string]string) - for k, v := range configMap { - configMapString[k] = fmt.Sprintf("%v", v) + if s == nil { + return nil } + strategyMap := parseBlock(s) + if strategyMap == nil { + return nil + } + + var configMapString map[string]string + configMap := parseBlock(strategyMap["config"]) + + if configMap != nil { + configMapString = make(map[string]string) + for k, v := range configMap { + configMapString[k] = fmt.Sprintf("%v", v) + } + } + + // Ignore ok, but we need _ to avoid panics. + name, _ := strategyMap["name"].(string) + return &policy.Strategy{ - Name: strategyMap["name"].(string), + Name: name, Config: configMapString, } } -func parseTarget(t interface{}) *policy.Target { - if t == nil { - return &policy.Target{} +// parseTarget parses the content of the target block from a policy and +// enhances it with values defined in Target as well. Values in target.config +// takes precedence over values in Target. +// +// It provides best-effort parsing and will return `nil` in case of errors. +// +// scaling { +// policy { +// target = { +// +-----------------+ +// | name = "target" | +// | config = { | +// | key = "value" | +// | } | +// +-----------------+ +// } +// } +// } +func parseTarget(targetBlock interface{}, targetAttr map[string]string) *policy.Target { + if targetBlock == nil && targetAttr == nil { + return nil } - targetMap := t.([]interface{})[0].(map[string]interface{}) + targetMap := parseBlock(targetBlock) if targetMap == nil { - return &policy.Target{} + return nil } var configMapString map[string]string - if targetMap["config"] != nil { - configMap := targetMap["config"].([]interface{})[0].(map[string]interface{}) + configMap := parseBlock(targetMap["config"]) + + if configMap != nil { configMapString = make(map[string]string) + + for k, v := range targetAttr { + configMapString[k] = v + } + for k, v := range configMap { configMapString[k] = fmt.Sprintf("%v", v) } } + + // Ignore ok, but we need _ to avoid panics. + name, _ := targetMap["name"].(string) + return &policy.Target{ - Name: targetMap["name"].(string), + Name: name, Config: configMapString, } } -// canonicalizePolicy sets standarized values for missing fields. -// It must be called after Validate. -func canonicalizePolicy(from *api.ScalingPolicy, to *policy.Policy) { - - if from.Enabled == nil { - to.Enabled = true +// parseBlock parses the specific structure of a block into a more usable +// value of map[string]interface{}. +func parseBlock(block interface{}) map[string]interface{} { + blockInterfaceList, ok := block.([]interface{}) + if !ok || len(blockInterfaceList) != 1 { + return nil } - if evalInterval, ok := from.Policy[keyEvaluationInterval].(string); ok { - // Ignore parse error since we assume Canonicalize is called after Validate - to.EvaluationInterval, _ = time.ParseDuration(evalInterval) + blockMap, ok := blockInterfaceList[0].(map[string]interface{}) + if !ok { + return nil } - if to.Target.Name == "" { - to.Target.Name = plugins.InternalTargetNomad - } - - if to.Target.Config == nil { - to.Target.Config = make(map[string]string) - } - - to.Target.Config["job_id"] = from.Target["Job"] - to.Target.Config["group"] = from.Target["Group"] - - if to.Source == "" { - to.Source = plugins.InternalAPMNomad - } - - if to.Source == plugins.InternalAPMNomad { - to.Query = fmt.Sprintf("%s/%s/%s", to.Query, from.Target["Group"], from.Target["Job"]) - } + return blockMap } diff --git a/policy/nomad/parser_test.go b/policy/nomad/parser_test.go index c3028e4f..c9a556fe 100644 --- a/policy/nomad/parser_test.go +++ b/policy/nomad/parser_test.go @@ -2,53 +2,168 @@ package nomad import ( "testing" + "time" + "github.com/hashicorp/nomad-autoscaler/helper/ptr" "github.com/hashicorp/nomad-autoscaler/policy" + "github.com/hashicorp/nomad/api" "github.com/stretchr/testify/assert" ) -func Test_parseStrategy(t *testing.T) { +func Test_parsePolicy(t *testing.T) { testCases := []struct { - inputStrategy interface{} - expectedOutput *policy.Strategy + name string + input *api.ScalingPolicy + expected policy.Policy }{ { - inputStrategy: []interface{}{ - map[string]interface{}{ - "name": "target-value", - "config": []interface{}{ - map[string]interface{}{"target": float64(20)}, + name: "full policy", + input: &api.ScalingPolicy{ + ID: "id", + Namespace: "default", + Target: map[string]string{ + "Namespace": "namespace", + "Job": "example", + "Group": "cache", + }, + Min: ptr.Int64ToPtr(1), + Max: 5, + Policy: map[string]interface{}{ + keySource: "source", + keyQuery: "query", + keyEvaluationInterval: "5s", + keyTarget: []interface{}{ + map[string]interface{}{ + "name": "target", + "config": []interface{}{ + map[string]interface{}{ + "int_config": 2, + }, + }, + }, + }, + keyStrategy: []interface{}{ + map[string]interface{}{ + "name": "strategy", + "config": []interface{}{ + map[string]interface{}{ + "bool_config": true, + }, + }, + }, + }, + }, + Enabled: ptr.BoolToPtr(true), + }, + expected: policy.Policy{ + ID: "id", + Min: 1, + Max: 5, + Source: "source", + Query: "query", + Enabled: true, + EvaluationInterval: 5 * time.Second, + Target: &policy.Target{ + Name: "target", + Config: map[string]string{ + "Namespace": "namespace", + "Job": "example", + "Group": "cache", + "int_config": "2", + }, + }, + Strategy: &policy.Strategy{ + Name: "strategy", + Config: map[string]string{ + "bool_config": "true", }, }, }, - expectedOutput: &policy.Strategy{ - Name: "target-value", - Config: map[string]string{"target": "20"}, + }, + { + name: "empty policy", + input: &api.ScalingPolicy{}, + expected: policy.Policy{ + Enabled: true, + }, + }, + { + name: "invalid strategy", + input: &api.ScalingPolicy{ + Policy: map[string]interface{}{ + keyStrategy: true, + }, + }, + expected: policy.Policy{ + Enabled: true, + }, + }, + { + name: "invalid target", + input: &api.ScalingPolicy{ + Policy: map[string]interface{}{ + keyTarget: true, + }, + }, + expected: policy.Policy{ + Enabled: true, }, }, } for _, tc := range testCases { - actualOutput := parseStrategy(tc.inputStrategy) - assert.Equal(t, tc.expectedOutput, actualOutput) + t.Run(tc.name, func(t *testing.T) { + actual := parsePolicy(tc.input) + assert.Equal(t, tc.expected, actual) + }) } } -func Test_parseTarget(t *testing.T) { +func Test_parseBlock(t *testing.T) { testCases := []struct { - inputTarget interface{} - expectedOutput *policy.Target - name string + name string + input interface{} + expected map[string]interface{} }{ { - inputTarget: nil, - expectedOutput: &policy.Target{}, - name: "nil passed target interface", + name: "valid block", + input: []interface{}{map[string]interface{}{}}, + expected: map[string]interface{}{}, + }, + { + name: "nil block", + input: nil, + expected: nil, + }, + { + name: "invalid root", + input: 1, + expected: nil, + }, + { + name: "no element", + input: []interface{}{}, + expected: nil, + }, + { + name: "more than one element", + input: []interface{}{ + map[string]interface{}{}, + map[string]interface{}{}, + }, + expected: nil, + }, + { + name: "invalid element type", + input: []interface{}{1}, + expected: nil, }, } for _, tc := range testCases { - actualOutput := parseTarget(tc.inputTarget) - assert.Equal(t, tc.expectedOutput, actualOutput, tc.name) + t.Run(tc.name, func(t *testing.T) { + actual := parseBlock(tc.input) + assert.Equal(t, tc.expected, actual) + }) } } diff --git a/policy/nomad/source.go b/policy/nomad/source.go index 43f48163..23ea7bba 100644 --- a/policy/nomad/source.go +++ b/policy/nomad/source.go @@ -3,10 +3,12 @@ package nomad import ( "context" "fmt" + "strings" "time" hclog "github.com/hashicorp/go-hclog" "github.com/hashicorp/nomad-autoscaler/helper/blocking" + "github.com/hashicorp/nomad-autoscaler/plugins" "github.com/hashicorp/nomad-autoscaler/policy" "github.com/hashicorp/nomad/api" ) @@ -21,25 +23,36 @@ const ( keyStrategy = "strategy" ) -const ( - defaultEvaluationInterval = 10 * time.Second -) - // Ensure NomadSource satisfies the Source interface. var _ policy.Source = (*Source)(nil) +// SourceConfig holds configuration values for the Nomad source. +type SourceConfig struct { + DefaultEvaluationInterval time.Duration +} + +func (c *SourceConfig) canonicalize() { + if c.DefaultEvaluationInterval == 0 { + c.DefaultEvaluationInterval = policy.DefaultEvaluationInterval + } +} + // Source is an implementation of the Source interface that retrieves // policies from a Nomad cluster. type Source struct { - log hclog.Logger - nomad *api.Client + log hclog.Logger + nomad *api.Client + config *SourceConfig } // NewNomadSource returns a new Nomad policy source. -func NewNomadSource(log hclog.Logger, nomad *api.Client) *Source { +func NewNomadSource(log hclog.Logger, nomad *api.Client, config *SourceConfig) *Source { + config.canonicalize() + return &Source{ - log: log.Named("nomad_policy_source"), - nomad: nomad, + log: log.Named("nomad_policy_source"), + nomad: nomad, + config: config, } } @@ -147,16 +160,86 @@ func (s *Source) MonitorPolicy(ctx context.Context, ID policy.PolicyID, resultCh continue } - var autoPolicy policy.Policy - // TODO(jrasell) once we have a better method for surfacing errors to the - // user, this error should be presented. - if autoPolicy, err = parsePolicy(p); err != nil { - errCh <- fmt.Errorf("failed to parse policy: %v", err) - return + if err := validateScalingPolicy(p); err != nil { + errCh <- fmt.Errorf("invalid policy: %v", err) + continue } + autoPolicy := parsePolicy(p) + s.canonicalizePolicy(&autoPolicy) + resultCh <- autoPolicy q.WaitIndex = meta.LastIndex } } } + +// canonicalizePolicy sets standarized values for missing fields. +func (s *Source) canonicalizePolicy(p *policy.Policy) { + if p == nil { + return + } + + // Default EvaluationInterval to the agent's DefaultEvaluationInterval. + if p.EvaluationInterval == 0 { + p.EvaluationInterval = s.config.DefaultEvaluationInterval + } + + // Set default values for Strategy. + if p.Strategy == nil { + p.Strategy = &policy.Strategy{} + } + + if p.Strategy.Config == nil { + p.Strategy.Config = make(map[string]string) + } + + // Set default values for Target. + if p.Target == nil { + p.Target = &policy.Target{} + } + + if p.Target.Name == "" { + p.Target.Name = plugins.InternalTargetNomad + } + + if p.Target.Config == nil { + p.Target.Config = make(map[string]string) + } + + // Map values from the original api.ScalingPolicy.Target into + // policy.Policy.Target.Config + // TODO(luiz): is this really necessay? We should probably just use what's returned in Target + targetConfigKeyMapping := map[string]string{ + "Job": "job_id", + "Group": "group", + } + + for k1, k2 := range targetConfigKeyMapping { + v, hasK1 := p.Target.Config[k1] + _, hasK2 := p.Target.Config[k2] + + if hasK1 && !hasK2 { + p.Target.Config[k2] = v + } + } + + // Default source to the Nomad APM. + if p.Source == "" { + p.Source = plugins.InternalAPMNomad + } + + // Expand short Nomad APM query from _ into _//. + // must be the last element so we can parse the query correctly + // since Nomad allows "/" in job IDs. + if p.Source == plugins.InternalAPMNomad && isShortQuery(p.Query) { + p.Query = fmt.Sprintf("%s/%s/%s", p.Query, p.Target.Config["group"], p.Target.Config["job_id"]) + } +} + +// isShortQuery detects if a query is in the _ format. +func isShortQuery(q string) bool { + opMetric := strings.SplitN(q, "_", 2) + hasSlash := strings.Contains(q, "/") + return len(opMetric) == 2 && !hasSlash +} diff --git a/policy/nomad/source_test.go b/policy/nomad/source_test.go new file mode 100644 index 00000000..3cb59b68 --- /dev/null +++ b/policy/nomad/source_test.go @@ -0,0 +1,303 @@ +package nomad + +import ( + "testing" + "time" + + "github.com/hashicorp/nomad-autoscaler/plugins" + "github.com/hashicorp/nomad-autoscaler/policy" + "github.com/hashicorp/nomad/api" + "github.com/stretchr/testify/assert" +) + +func TestSourceConfig_canonicalize(t *testing.T) { + testCases := []struct { + name string + input *SourceConfig + expected *SourceConfig + }{ + { + name: "empty config", + input: &SourceConfig{}, + expected: &SourceConfig{ + DefaultEvaluationInterval: policy.DefaultEvaluationInterval, + }, + }, + { + name: "don't overwrite values", + input: &SourceConfig{ + DefaultEvaluationInterval: time.Second, + }, + expected: &SourceConfig{ + DefaultEvaluationInterval: time.Second, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tc.input.canonicalize() + assert.Equal(t, tc.expected, tc.input) + }) + } +} + +func TestSource_canonicalizePolicy(t *testing.T) { + testCases := []struct { + name string + input *policy.Policy + expected *policy.Policy + cb func(*api.Config, *SourceConfig) + }{ + { + name: "full policy", + input: &policy.Policy{ + ID: "string", + Min: 1, + Max: 5, + Source: "source", + Query: "query", + Enabled: true, + EvaluationInterval: time.Second, + Target: &policy.Target{ + Name: "target", + Config: map[string]string{ + "target_config": "yes", + "target_config2": "no", + "Job": "job", + "Group": "group", + }, + }, + Strategy: &policy.Strategy{ + Name: "strategy", + Config: map[string]string{ + "strategy_config1": "yes", + "strategy_config2": "no", + }, + }, + }, + expected: &policy.Policy{ + ID: "string", + Min: 1, + Max: 5, + Source: "source", + Query: "query", + Enabled: true, + EvaluationInterval: time.Second, + Target: &policy.Target{ + Name: "target", + Config: map[string]string{ + "target_config": "yes", + "target_config2": "no", + "Job": "job", + "Group": "group", + "job_id": "job", + "group": "group", + }, + }, + Strategy: &policy.Strategy{ + Name: "strategy", + Config: map[string]string{ + "strategy_config1": "yes", + "strategy_config2": "no", + }, + }, + }, + }, + { + name: "set all defaults", + input: &policy.Policy{}, + expected: &policy.Policy{ + Source: plugins.InternalAPMNomad, + EvaluationInterval: 10 * time.Second, + Target: &policy.Target{ + Name: plugins.InternalTargetNomad, + Config: map[string]string{}, + }, + Strategy: &policy.Strategy{ + Config: map[string]string{}, + }, + }, + }, + { + name: "nil input", + input: nil, + expected: nil, + }, + { + name: "expand query when source is empty", + input: &policy.Policy{ + Query: "avg_cpu", + Target: &policy.Target{ + Config: map[string]string{ + "Job": "job", + "Group": "group", + }, + }, + }, + expected: &policy.Policy{ + Source: plugins.InternalAPMNomad, + Query: "avg_cpu/group/job", + EvaluationInterval: 10 * time.Second, + Target: &policy.Target{ + Name: plugins.InternalTargetNomad, + Config: map[string]string{ + "Job": "job", + "Group": "group", + "job_id": "job", + "group": "group", + }, + }, + Strategy: &policy.Strategy{ + Config: map[string]string{}, + }, + }, + }, + { + name: "expand query when source is nomad apm", + input: &policy.Policy{ + Source: plugins.InternalAPMNomad, + Query: "avg_cpu", + Target: &policy.Target{ + Config: map[string]string{ + "Job": "job", + "Group": "group", + }, + }, + }, + expected: &policy.Policy{ + Source: plugins.InternalAPMNomad, + Query: "avg_cpu/group/job", + EvaluationInterval: 10 * time.Second, + Target: &policy.Target{ + Name: plugins.InternalTargetNomad, + Config: map[string]string{ + "Job": "job", + "Group": "group", + "job_id": "job", + "group": "group", + }, + }, + Strategy: &policy.Strategy{ + Config: map[string]string{}, + }, + }, + }, + { + name: "expand query from user-defined values", + input: &policy.Policy{ + Query: "avg_cpu", + Target: &policy.Target{ + Config: map[string]string{ + "Job": "job", + "Group": "group", + "job_id": "my_job", + "group": "my_group", + }, + }, + }, + expected: &policy.Policy{ + Source: plugins.InternalAPMNomad, + Query: "avg_cpu/my_group/my_job", + EvaluationInterval: 10 * time.Second, + Target: &policy.Target{ + Name: plugins.InternalTargetNomad, + Config: map[string]string{ + "Job": "job", + "Group": "group", + "job_id": "my_job", + "group": "my_group", + }, + }, + Strategy: &policy.Strategy{ + Config: map[string]string{}, + }, + }, + }, + { + name: "don't expand query if not nomad apm", + input: &policy.Policy{ + Source: "not_nomad", + Query: "avg_cpu", + Target: &policy.Target{ + Config: map[string]string{ + "Job": "job", + "Group": "group", + }, + }, + }, + expected: &policy.Policy{ + Source: "not_nomad", + Query: "avg_cpu", + EvaluationInterval: 10 * time.Second, + Target: &policy.Target{ + Name: plugins.InternalTargetNomad, + Config: map[string]string{ + "Job": "job", + "Group": "group", + "job_id": "job", + "group": "group", + }, + }, + Strategy: &policy.Strategy{ + Config: map[string]string{}, + }, + }, + }, + { + name: "don't expand query if not in short format", + input: &policy.Policy{ + Query: "avg_cpu/my_group/my_job", + Target: &policy.Target{ + Config: map[string]string{ + "Job": "job", + "Group": "group", + }, + }, + }, + expected: &policy.Policy{ + Source: plugins.InternalAPMNomad, + Query: "avg_cpu/my_group/my_job", + EvaluationInterval: 10 * time.Second, + Target: &policy.Target{ + Name: plugins.InternalTargetNomad, + Config: map[string]string{ + "Job": "job", + "Group": "group", + "job_id": "job", + "group": "group", + }, + }, + Strategy: &policy.Strategy{ + Config: map[string]string{}, + }, + }, + }, + { + name: "sets eval interval from agent", + input: &policy.Policy{}, + expected: &policy.Policy{ + Source: plugins.InternalAPMNomad, + EvaluationInterval: 5 * time.Second, + Target: &policy.Target{ + Name: plugins.InternalTargetNomad, + Config: map[string]string{}, + }, + Strategy: &policy.Strategy{ + Config: map[string]string{}, + }, + }, + cb: func(_ *api.Config, sourceConfig *SourceConfig) { + sourceConfig.DefaultEvaluationInterval = 5 * time.Second + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + s := TestNomadSource(t, tc.cb) + s.canonicalizePolicy(tc.input) + assert.Equal(t, tc.expected, tc.input) + }) + } +} diff --git a/policy/nomad/testing.go b/policy/nomad/testing.go new file mode 100644 index 00000000..ad6b536d --- /dev/null +++ b/policy/nomad/testing.go @@ -0,0 +1,31 @@ +package nomad + +import ( + "testing" + + hclog "github.com/hashicorp/go-hclog" + "github.com/hashicorp/nomad/api" +) + +// TestNomadSource returns a default policy.Source that retrieves policies +// from Nomad. +// +// The Nomad client and the agent can be configured by passing a cb function. +func TestNomadSource(t *testing.T, cb func(*api.Config, *SourceConfig)) *Source { + nomadConfig := api.DefaultConfig() + sourceConfig := &SourceConfig{} + + if cb != nil { + cb(nomadConfig, sourceConfig) + } + + nomad, err := api.NewClient(nomadConfig) + if err != nil { + t.Fatal(err) + } + + log := hclog.New(&hclog.LoggerOptions{ + Level: hclog.Trace, + }) + return NewNomadSource(log, nomad, sourceConfig) +} diff --git a/policy/source.go b/policy/source.go index 023b279d..a7695609 100644 --- a/policy/source.go +++ b/policy/source.go @@ -1,6 +1,13 @@ package policy -import "context" +import ( + "context" + "time" +) + +const ( + DefaultEvaluationInterval = 10 * time.Second +) // Source is the interface that must be implemented by backends which // provide the canonical source for scaling policies.