From fdcd29bce15934346bcb75c6d3630aa566148502 Mon Sep 17 00:00:00 2001 From: Jeffrey Hogan Date: Wed, 20 Dec 2023 16:23:16 -0600 Subject: [PATCH] Add `BlockQueryWaitTime` Nomad Configuration Option (#755) * add `block_query_wait_time` to Nomad client config block * Stop explicitly setting WaitTime Nomad API query option This value is now set on the Nomad api.Client config directly. :toot: * add changelog entry for `block_query_wait_time` / PR#755 * changelog: update entry for #755 --------- Co-authored-by: Luiz Aoqui --- CHANGELOG.md | 3 + agent/config/config.go | 26 ++++++- agent/config/config_test.go | 23 +++--- command/agent.go | 5 ++ command/agent_test.go | 24 ++++--- plugins/builtin/target/nomad/plugin/state.go | 1 - policy/nomad/source.go | 4 +- sdk/helper/nomad/config.go | 35 +++++++--- sdk/helper/nomad/config_test.go | 73 +++++++++++--------- 9 files changed, 124 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4dfc59de..d4407b80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ ## UNRELEASED +IMPROVEMENTS: + * agent: Add `BlockQueryWaitTime` config option for Nomad API connectivity [[GH-755](https://github.com/hashicorp/nomad-autoscaler/pull/755)] + ## 0.4.0 (December 20, 2023) FEATURES: diff --git a/agent/config/config.go b/agent/config/config.go index e7a5328c..da243c7e 100644 --- a/agent/config/config.go +++ b/agent/config/config.go @@ -160,6 +160,11 @@ type Nomad struct { // SkipVerify enables or disables SSL verification. SkipVerify bool `hcl:"skip_verify,optional"` + + // BlockQueryWaitTime controls how long Nomad API requests supporting blocking queries + // are held open. Defaults to 5m. + BlockQueryWaitTime time.Duration + BlockQueryWaitTimeHCL string `hcl:"block_query_wait_time,optional"` } // Telemetry holds the user specified configuration for metrics collection. @@ -395,6 +400,10 @@ const ( // defaultLockDelay is the default lockDelay used for the lock that syncs the leader // election. defaultLockDelay = 30 * time.Second + + // defaultBlockQueryWaitTime is the default duration Nomad API requests supporting + // blocking queries are held open. + defaultBlockQueryWaitTime = 5 * time.Minute ) // TODO: there's an unexpected import cycle that prevents us from using the @@ -431,7 +440,9 @@ func Default() (*Agent, error) { BindAddress: defaultHTTPBindAddress, BindPort: defaultHTTPBindPort, }, - Nomad: &Nomad{}, + Nomad: &Nomad{ + BlockQueryWaitTime: defaultBlockQueryWaitTime, + }, Telemetry: &Telemetry{ CollectionInterval: defaultTelemetryCollectionInterval, }, @@ -665,6 +676,9 @@ func (n *Nomad) merge(b *Nomad) *Nomad { if b.SkipVerify { result.SkipVerify = b.SkipVerify } + if b.BlockQueryWaitTime != 0 { + result.BlockQueryWaitTime = b.BlockQueryWaitTime + } return &result } @@ -1013,6 +1027,16 @@ func parseFile(file string, cfg *Agent) error { return err } + if cfg.Nomad != nil { + if cfg.Nomad.BlockQueryWaitTimeHCL != "" { + w, err := time.ParseDuration(cfg.Nomad.BlockQueryWaitTimeHCL) + if err != nil { + return err + } + cfg.Nomad.BlockQueryWaitTime = w + } + } + if cfg.Policy != nil { if cfg.Policy.DefaultCooldownHCL != "" { d, err := time.ParseDuration(cfg.Policy.DefaultCooldownHCL) diff --git a/agent/config/config_test.go b/agent/config/config_test.go index 7ca41ce9..bfaa7c39 100644 --- a/agent/config/config_test.go +++ b/agent/config/config_test.go @@ -209,17 +209,18 @@ func TestAgent_Merge(t *testing.T) { BindPort: 4646, }, Nomad: &Nomad{ - Address: "https://nomad-new.systems:4646", - Region: "moon-base-1", - Namespace: "fra-mauro", - Token: "super-secret-tokeny-thing", - HTTPAuth: "admin:admin", - CACert: "/etc/nomad.d/ca.crt", - CAPath: "/etc/nomad.d/ca/", - ClientCert: "/etc/nomad.d/client.crt", - ClientKey: "/etc/nomad.d/client-key.crt", - TLSServerName: "cows-or-pets", - SkipVerify: true, + Address: "https://nomad-new.systems:4646", + Region: "moon-base-1", + Namespace: "fra-mauro", + Token: "super-secret-tokeny-thing", + HTTPAuth: "admin:admin", + CACert: "/etc/nomad.d/ca.crt", + CAPath: "/etc/nomad.d/ca/", + ClientCert: "/etc/nomad.d/client.crt", + ClientKey: "/etc/nomad.d/client-key.crt", + TLSServerName: "cows-or-pets", + SkipVerify: true, + BlockQueryWaitTime: 5 * time.Minute, }, Policy: &Policy{ Dir: "/etc/scaling/policies", diff --git a/command/agent.go b/command/agent.go index def2c64a..124ac2e5 100644 --- a/command/agent.go +++ b/command/agent.go @@ -149,6 +149,10 @@ Nomad Options: -nomad-skip-verify Do not verify TLS certificates. This is strongly discouraged. + -nomad-block-query-wait-time= + How long applicable Nomad API requests supporting blocking queries are held + open. Defaults to 5m. + Policy Options: -policy-dir= @@ -507,6 +511,7 @@ func (c *AgentCommand) readConfig() (*config.Agent, []string) { flags.StringVar(&cmdConfig.Nomad.ClientKey, "nomad-client-key", "", "") flags.StringVar(&cmdConfig.Nomad.TLSServerName, "nomad-tls-server-name", "", "") flags.BoolVar(&cmdConfig.Nomad.SkipVerify, "nomad-skip-verify", false, "") + flags.DurationVar(&cmdConfig.Nomad.BlockQueryWaitTime, "nomad-block-query-wait-time", 0, "") // Specify our Policy CLI flags. flags.StringVar(&cmdConfig.Policy.Dir, "policy-dir", "", "") diff --git a/command/agent_test.go b/command/agent_test.go index 1c3a75cf..a2b2a293 100644 --- a/command/agent_test.go +++ b/command/agent_test.go @@ -70,20 +70,22 @@ func TestCommandAgent_readConfig(t *testing.T) { "-nomad-client-key", "./client-key.pem", "-nomad-tls-server-name", "server", "-nomad-skip-verify", + "-nomad-block-query-wait-time", "30s", }, want: defaultConfig.Merge(&config.Agent{ Nomad: &config.Nomad{ - Address: "http://nomad.example.com", - Region: "milky_way", - Namespace: "prod", - Token: "TOKEN", - HTTPAuth: "user:pass", - CACert: "./ca-cert.pem", - CAPath: "./ca-certs", - ClientCert: "./client-cert.pem", - ClientKey: "./client-key.pem", - TLSServerName: "server", - SkipVerify: true, + Address: "http://nomad.example.com", + Region: "milky_way", + Namespace: "prod", + Token: "TOKEN", + HTTPAuth: "user:pass", + CACert: "./ca-cert.pem", + CAPath: "./ca-certs", + ClientCert: "./client-cert.pem", + ClientKey: "./client-key.pem", + TLSServerName: "server", + SkipVerify: true, + BlockQueryWaitTime: 30 * time.Second, }, }), }, diff --git a/plugins/builtin/target/nomad/plugin/state.go b/plugins/builtin/target/nomad/plugin/state.go index 5d7c19dd..243582bb 100644 --- a/plugins/builtin/target/nomad/plugin/state.go +++ b/plugins/builtin/target/nomad/plugin/state.go @@ -168,7 +168,6 @@ func (jsh *jobScaleStatusHandler) start() { q := &api.QueryOptions{ Namespace: jsh.namespace, - WaitTime: 5 * time.Minute, WaitIndex: 1, } diff --git a/policy/nomad/source.go b/policy/nomad/source.go index 41e24041..71351475 100644 --- a/policy/nomad/source.go +++ b/policy/nomad/source.go @@ -89,7 +89,7 @@ func (s *Source) ReloadIDsMonitor() { func (s *Source) MonitorIDs(ctx context.Context, req policy.MonitorIDsReq) { s.log.Debug("starting policy blocking query watcher") - q := &api.QueryOptions{WaitTime: 5 * time.Minute, WaitIndex: 1} + q := &api.QueryOptions{WaitIndex: 1} for { var ( @@ -179,7 +179,7 @@ func (s *Source) MonitorPolicy(ctx context.Context, req policy.MonitorPolicyReq) log.Trace("starting policy blocking query watcher") - q := &api.QueryOptions{WaitTime: 5 * time.Minute, WaitIndex: 1} + q := &api.QueryOptions{WaitIndex: 1} for { var ( p *api.ScalingPolicy diff --git a/sdk/helper/nomad/config.go b/sdk/helper/nomad/config.go index f2334c3c..75578cc1 100644 --- a/sdk/helper/nomad/config.go +++ b/sdk/helper/nomad/config.go @@ -6,23 +6,25 @@ package nomad import ( "strconv" "strings" + "time" "github.com/hashicorp/nomad-autoscaler/agent/config" "github.com/hashicorp/nomad/api" ) const ( - configKeyNomadAddress = "nomad_address" - configKeyNomadRegion = "nomad_region" - configKeyNomadNamespace = "nomad_namespace" - configKeyNomadToken = "nomad_token" - configKeyNomadHTTPAuth = "nomad_http-auth" - configKeyNomadCACert = "nomad_ca-cert" - configKeyNomadCAPath = "nomad_ca-path" - configKeyNomadClientCert = "nomad_client-cert" - configKeyNomadClientKey = "nomad_client-key" - configKeyNomadTLSServerName = "nomad_tls-server-name" - configKeyNomadSkipVerify = "nomad_skip-verify" + configKeyNomadAddress = "nomad_address" + configKeyNomadRegion = "nomad_region" + configKeyNomadNamespace = "nomad_namespace" + configKeyNomadToken = "nomad_token" + configKeyNomadHTTPAuth = "nomad_http-auth" + configKeyNomadCACert = "nomad_ca-cert" + configKeyNomadCAPath = "nomad_ca-path" + configKeyNomadClientCert = "nomad_client-cert" + configKeyNomadClientKey = "nomad_client-key" + configKeyNomadTLSServerName = "nomad_tls-server-name" + configKeyNomadSkipVerify = "nomad_skip-verify" + configKeyNomadBlockQueryWaitTime = "nomad_block-query-wait-time" ) // ConfigFromNamespacedMap converts the map representation of a Nomad config to @@ -69,6 +71,11 @@ func ConfigFromNamespacedMap(cfg map[string]string) *api.Config { if httpAuth, ok := cfg[configKeyNomadHTTPAuth]; ok { c.HttpAuth = HTTPAuthFromString(httpAuth) } + // We may ignore errors from ParseDuration() here as the provided argument + // has already passed general validation as part of being a DurationVar flag. + if blockQueryWaitTime, ok := cfg[configKeyNomadBlockQueryWaitTime]; ok { + c.WaitTime, _ = time.ParseDuration(blockQueryWaitTime) + } return c } @@ -141,6 +148,9 @@ func MergeMapWithAgentConfig(m map[string]string, cfg *api.Config) { } m[configKeyNomadHTTPAuth] = auth } + if cfg.WaitTime != 0 && m[configKeyNomadBlockQueryWaitTime] == "" { + m[configKeyNomadBlockQueryWaitTime] = cfg.WaitTime.String() + } } // MergeDefaultWithAgentConfig merges the agent Nomad configuration with the @@ -193,6 +203,9 @@ func MergeDefaultWithAgentConfig(agentCfg *config.Nomad) *api.Config { if agentCfg.SkipVerify { cfg.TLSConfig.Insecure = agentCfg.SkipVerify } + if agentCfg.BlockQueryWaitTime != 0 { + cfg.WaitTime = agentCfg.BlockQueryWaitTime + } return cfg } diff --git a/sdk/helper/nomad/config_test.go b/sdk/helper/nomad/config_test.go index fbe471f2..c34aa936 100644 --- a/sdk/helper/nomad/config_test.go +++ b/sdk/helper/nomad/config_test.go @@ -5,6 +5,7 @@ package nomad import ( "testing" + "time" "github.com/hashicorp/nomad-autoscaler/agent/config" "github.com/hashicorp/nomad/api" @@ -18,17 +19,18 @@ func Test_ConfigFromNamespacedMap(t *testing.T) { }{ { inputCfg: map[string]string{ - "nomad_address": "vlc.nomad", - "nomad_region": "espana", - "nomad_namespace": "picassent", - "nomad_token": "my-precious", - "nomad_http-auth": "username:password", - "nomad_ca-cert": "/etc/nomad.d/ca.crt", - "nomad_ca-path": "/etc/nomad.d/", - "nomad_client-cert": "/etc/nomad.d/client.crt", - "nomad_client-key": "/etc/nomad.d/client.key", - "nomad_tls-server-name": "lord-of-the-servers", - "nomad_skip-verify": "true", + "nomad_address": "vlc.nomad", + "nomad_region": "espana", + "nomad_namespace": "picassent", + "nomad_token": "my-precious", + "nomad_http-auth": "username:password", + "nomad_ca-cert": "/etc/nomad.d/ca.crt", + "nomad_ca-path": "/etc/nomad.d/", + "nomad_client-cert": "/etc/nomad.d/client.crt", + "nomad_client-key": "/etc/nomad.d/client.key", + "nomad_tls-server-name": "lord-of-the-servers", + "nomad_skip-verify": "true", + "nomad_block-query-wait-time": "60000ms", }, expectedOutput: &api.Config{ Address: "vlc.nomad", @@ -47,6 +49,7 @@ func Test_ConfigFromNamespacedMap(t *testing.T) { TLSServerName: "lord-of-the-servers", Insecure: true, }, + WaitTime: 1 * time.Minute, }, }, } @@ -108,19 +111,21 @@ func Test_MergeMapWithAgentConfig(t *testing.T) { TLSServerName: "test", Insecure: true, }, + WaitTime: 2 * time.Minute, }, expectedOutputMap: map[string]string{ - "nomad_address": "test", - "nomad_region": "test", - "nomad_namespace": "test", - "nomad_token": "test", - "nomad_http-auth": "test:test", - "nomad_ca-cert": "test", - "nomad_ca-path": "test", - "nomad_client-cert": "test", - "nomad_client-key": "test", - "nomad_tls-server-name": "test", - "nomad_skip-verify": "true", + "nomad_address": "test", + "nomad_region": "test", + "nomad_namespace": "test", + "nomad_token": "test", + "nomad_http-auth": "test:test", + "nomad_ca-cert": "test", + "nomad_ca-path": "test", + "nomad_client-cert": "test", + "nomad_client-key": "test", + "nomad_tls-server-name": "test", + "nomad_skip-verify": "true", + "nomad_block-query-wait-time": "2m0s", }, name: "empty input map", }, @@ -147,17 +152,18 @@ func Test_MergeDefaultWithAgentConfig(t *testing.T) { }, { inputConfig: &config.Nomad{ - Address: "http://demo.nomad:4646", - Region: "vlc", - Namespace: "platform", - Token: "shhhhhhhh", - HTTPAuth: "admin:admin", - CACert: "/path/to/long-lived/ca-cert", - CAPath: "/path/to/long-lived/", - ClientCert: "/path/to/long-lived/client-cert", - ClientKey: "/path/to/long-lived/key-cert", - TLSServerName: "whatdoesthisdo", - SkipVerify: true, + Address: "http://demo.nomad:4646", + Region: "vlc", + Namespace: "platform", + Token: "shhhhhhhh", + HTTPAuth: "admin:admin", + CACert: "/path/to/long-lived/ca-cert", + CAPath: "/path/to/long-lived/", + ClientCert: "/path/to/long-lived/client-cert", + ClientKey: "/path/to/long-lived/key-cert", + TLSServerName: "whatdoesthisdo", + SkipVerify: true, + BlockQueryWaitTime: 5 * time.Second, }, expectedOutput: &api.Config{ Address: "http://demo.nomad:4646", @@ -176,6 +182,7 @@ func Test_MergeDefaultWithAgentConfig(t *testing.T) { TLSServerName: "whatdoesthisdo", Insecure: true, }, + WaitTime: 5 * time.Second, }, name: "full Autoscaler Nomad config override", },