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 <luiz@hashicorp.com>
hashicorp · Dec 20, 2023 · fdcd29b · fdcd29b
1 parent e4a2392
commit fdcd29b
Show file tree

Hide file tree

Showing 9 changed files with 124 additions and 70 deletions.
diff --git 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
@@ -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
@@ -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
@@ -149,6 +149,10 @@ Nomad Options:
   -nomad-skip-verify
     Do not verify TLS certificates. This is strongly discouraged.
 
+  -nomad-block-query-wait-time=<dur>
+    How long applicable Nomad API requests supporting blocking queries are held
+    open. Defaults to 5m.
+
 Policy Options:
 
   -policy-dir=<path>
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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",
 		},