Add Nomad RetryConfig to agent template config (#13907)

* add Nomad RetryConfig to agent template config
hashicorp · Aug 3, 2022 · 696deb9 · 696deb9
1 parent dc0bac7
commit 696deb9
Show file tree

Hide file tree

Showing 10 changed files with 106 additions and 9 deletions.
diff --git a/.changelog/13907.txt b/.changelog/13907.txt
@@ -0,0 +1,3 @@
+```release-note:improvement
+template: Expose consul-template configuration options at the client level for `nomad_retry`.
+```
diff --git a/client/allocrunner/taskrunner/template/template.go b/client/allocrunner/taskrunner/template/template.go
@@ -823,6 +823,17 @@ func newRunnerConfig(config *TaskTemplateManagerConfig,
 	conf.Nomad.Namespace = &config.NomadNamespace
 	conf.Nomad.Transport.CustomDialer = cc.TemplateDialer
 	conf.Nomad.Token = &config.NomadToken
+	if cc.TemplateConfig != nil && cc.TemplateConfig.NomadRetry != nil {
+		// Set the user-specified Nomad RetryConfig
+		var err error
+		if err = cc.TemplateConfig.NomadRetry.Validate(); err != nil {
+			return nil, err
+		}
+		conf.Nomad.Retry, err = cc.TemplateConfig.NomadRetry.ToConsulTemplate()
+		if err != nil {
+			return nil, err
+		}
+	}
 
 	conf.Finalize()
 	return conf, nil

diff --git a/client/allocrunner/taskrunner/template/template_test.go b/client/allocrunner/taskrunner/template/template_test.go
@@ -1959,6 +1959,7 @@ func TestTaskTemplateManager_ClientTemplateConfig_Set(t *testing.T) {
 	clientConfig.TemplateConfig.Wait = waitConfig.Copy()
 	clientConfig.TemplateConfig.ConsulRetry = retryConfig.Copy()
 	clientConfig.TemplateConfig.VaultRetry = retryConfig.Copy()
+	clientConfig.TemplateConfig.NomadRetry = retryConfig.Copy()
 
 	alloc := mock.Alloc()
 	allocWithOverride := mock.Alloc()
@@ -1986,6 +1987,7 @@ func TestTaskTemplateManager_ClientTemplateConfig_Set(t *testing.T) {
 				Wait:               waitConfig.Copy(),
 				ConsulRetry:        retryConfig.Copy(),
 				VaultRetry:         retryConfig.Copy(),
+				NomadRetry:         retryConfig.Copy(),
 			},
 			&TaskTemplateManagerConfig{
 				ClientConfig: clientConfig,
@@ -1999,6 +2001,7 @@ func TestTaskTemplateManager_ClientTemplateConfig_Set(t *testing.T) {
 					Wait:               waitConfig.Copy(),
 					ConsulRetry:        retryConfig.Copy(),
 					VaultRetry:         retryConfig.Copy(),
+					NomadRetry:         retryConfig.Copy(),
 				},
 			},
 			&templateconfig.TemplateConfig{
@@ -2017,6 +2020,7 @@ func TestTaskTemplateManager_ClientTemplateConfig_Set(t *testing.T) {
 				Wait:               waitConfig.Copy(),
 				ConsulRetry:        retryConfig.Copy(),
 				VaultRetry:         retryConfig.Copy(),
+				NomadRetry:         retryConfig.Copy(),
 			},
 			&TaskTemplateManagerConfig{
 				ClientConfig: clientConfig,
@@ -2030,6 +2034,7 @@ func TestTaskTemplateManager_ClientTemplateConfig_Set(t *testing.T) {
 					Wait:               waitConfig.Copy(),
 					ConsulRetry:        retryConfig.Copy(),
 					VaultRetry:         retryConfig.Copy(),
+					NomadRetry:         retryConfig.Copy(),
 				},
 			},
 			&templateconfig.TemplateConfig{
@@ -2052,6 +2057,7 @@ func TestTaskTemplateManager_ClientTemplateConfig_Set(t *testing.T) {
 				},
 				ConsulRetry: retryConfig.Copy(),
 				VaultRetry:  retryConfig.Copy(),
+				NomadRetry:  retryConfig.Copy(),
 			},
 			&TaskTemplateManagerConfig{
 				ClientConfig: clientConfig,
@@ -2077,6 +2083,7 @@ func TestTaskTemplateManager_ClientTemplateConfig_Set(t *testing.T) {
 					},
 					ConsulRetry: retryConfig.Copy(),
 					VaultRetry:  retryConfig.Copy(),
+					NomadRetry:  retryConfig.Copy(),
 				},
 			},
 			&templateconfig.TemplateConfig{
@@ -2117,6 +2124,12 @@ func TestTaskTemplateManager_ClientTemplateConfig_Set(t *testing.T) {
 			require.Equal(t, *_case.ExpectedRunnerConfig.TemplateConfig.VaultRetry.Attempts, *runnerConfig.Vault.Retry.Attempts)
 			require.Equal(t, *_case.ExpectedRunnerConfig.TemplateConfig.VaultRetry.Backoff, *runnerConfig.Vault.Retry.Backoff)
 			require.Equal(t, *_case.ExpectedRunnerConfig.TemplateConfig.VaultRetry.MaxBackoff, *runnerConfig.Vault.Retry.MaxBackoff)
+			// Nomad Retry
+			require.NotNil(t, runnerConfig.Nomad)
+			require.NotNil(t, runnerConfig.Nomad.Retry)
+			require.Equal(t, *_case.ExpectedRunnerConfig.TemplateConfig.NomadRetry.Attempts, *runnerConfig.Nomad.Retry.Attempts)
+			require.Equal(t, *_case.ExpectedRunnerConfig.TemplateConfig.NomadRetry.Backoff, *runnerConfig.Nomad.Retry.Backoff)
+			require.Equal(t, *_case.ExpectedRunnerConfig.TemplateConfig.NomadRetry.MaxBackoff, *runnerConfig.Nomad.Retry.MaxBackoff)
 
 			// Test that wait_bounds are enforced
 			for _, tmpl := range *runnerConfig.Templates {

diff --git a/client/config/config.go b/client/config/config.go
@@ -19,6 +19,7 @@ import (
 	"github.com/hashicorp/nomad/helper"
 	"github.com/hashicorp/nomad/helper/bufconndialer"
 	"github.com/hashicorp/nomad/helper/pluginutils/loader"
+	"github.com/hashicorp/nomad/helper/pointer"
 	"github.com/hashicorp/nomad/nomad/structs"
 	structsc "github.com/hashicorp/nomad/nomad/structs/config"
 	"github.com/hashicorp/nomad/plugins/base"
@@ -358,6 +359,13 @@ type ClientTemplateConfig struct {
 	// to wait for the cluster to become available, as is customary in distributed
 	// systems.
 	VaultRetry *RetryConfig `hcl:"vault_retry,optional"`
+
+	// This controls the retry behavior when an error is returned from Nomad.
+	// Consul Template is highly fault tolerant, meaning it does not exit in the
+	// face of failure. Instead, it uses exponential back-off and retry functions
+	// to wait for the cluster to become available, as is customary in distributed
+	// systems.
+	NomadRetry *RetryConfig `hcl:"nomad_retry,optional"`
 }
 
 // Copy returns a deep copy of a ClientTemplateConfig
@@ -396,6 +404,10 @@ func (c *ClientTemplateConfig) Copy() *ClientTemplateConfig {
 		nc.VaultRetry = c.VaultRetry.Copy()
 	}
 
+	if c.NomadRetry != nil {
+		nc.NomadRetry = c.NomadRetry.Copy()
+	}
+
 	return nc
 }
 
@@ -413,7 +425,8 @@ func (c *ClientTemplateConfig) IsEmpty() bool {
 		c.MaxStaleHCL == "" &&
 		c.Wait.IsEmpty() &&
 		c.ConsulRetry.IsEmpty() &&
-		c.VaultRetry.IsEmpty()
+		c.VaultRetry.IsEmpty() &&
+		c.NomadRetry.IsEmpty()
 }
 
 // WaitConfig is mirrored from templateconfig.WaitConfig because we need to handle
@@ -730,10 +743,13 @@ func DefaultConfig() *Config {
 				Max: helper.TimeToPtr(4 * time.Minute),
 			},
 			ConsulRetry: &RetryConfig{
-				Attempts: helper.IntToPtr(0), // unlimited
+				Attempts: pointer.Of[int](0), // unlimited
 			},
 			VaultRetry: &RetryConfig{
-				Attempts: helper.IntToPtr(0), // unlimited
+				Attempts: pointer.Of[int](0), // unlimited
+			},
+			NomadRetry: &RetryConfig{
+				Attempts: pointer.Of[int](0), // unlimited
 			},
 		},
 		RPCHoldTimeout:     5 * time.Second,

diff --git a/command/agent/config_parse.go b/command/agent/config_parse.go
@@ -41,6 +41,7 @@ func ParseConfigFile(path string) (*Config, error) {
 				WaitBounds:  &client.WaitConfig{},
 				ConsulRetry: &client.RetryConfig{},
 				VaultRetry:  &client.RetryConfig{},
+				NomadRetry:  &client.RetryConfig{},
 			},
 		},
 		Server: &ServerConfig{
@@ -125,6 +126,16 @@ func ParseConfigFile(path string) (*Config, error) {
 				c.Client.TemplateConfig.VaultRetry.MaxBackoff = d
 			},
 		},
+		{"client.template.nomad_retry.backoff", nil, &c.Client.TemplateConfig.NomadRetry.BackoffHCL,
+			func(d *time.Duration) {
+				c.Client.TemplateConfig.NomadRetry.Backoff = d
+			},
+		},
+		{"client.template.nomad_retry.max_backoff", nil, &c.Client.TemplateConfig.NomadRetry.MaxBackoffHCL,
+			func(d *time.Duration) {
+				c.Client.TemplateConfig.NomadRetry.MaxBackoff = d
+			},
+		},
 	}
 
 	// Add enterprise audit sinks for time.Duration parsing
@@ -264,6 +275,10 @@ func finalizeClientTemplateConfig(config *Config) {
 		config.Client.TemplateConfig.VaultRetry = nil
 	}
 
+	if config.Client.TemplateConfig.NomadRetry.IsEmpty() {
+		config.Client.TemplateConfig.NomadRetry = nil
+	}
+
 	if config.Client.TemplateConfig.IsEmpty() {
 		config.Client.TemplateConfig = nil
 	}

diff --git a/command/agent/config_test.go b/command/agent/config_test.go
@@ -1432,6 +1432,7 @@ func TestConfig_LoadConsulTemplateConfig(t *testing.T) {
 	require.NotNil(t, templateConfig.WaitBounds)
 	require.NotNil(t, templateConfig.ConsulRetry)
 	require.NotNil(t, templateConfig.VaultRetry)
+	require.NotNil(t, templateConfig.NomadRetry)
 
 	// Direct properties
 	require.Equal(t, 300*time.Second, *templateConfig.MaxStale)
@@ -1452,6 +1453,11 @@ func TestConfig_LoadConsulTemplateConfig(t *testing.T) {
 	require.Equal(t, 10, *templateConfig.VaultRetry.Attempts)
 	require.Equal(t, 15*time.Second, *templateConfig.VaultRetry.Backoff)
 	require.Equal(t, 20*time.Second, *templateConfig.VaultRetry.MaxBackoff)
+	// Nomad Retry
+	require.NotNil(t, templateConfig.NomadRetry)
+	require.Equal(t, 15, *templateConfig.NomadRetry.Attempts)
+	require.Equal(t, 20*time.Second, *templateConfig.NomadRetry.Backoff)
+	require.Equal(t, 25*time.Second, *templateConfig.NomadRetry.MaxBackoff)
 }
 
 func TestConfig_LoadConsulTemplate_FunctionDenylist(t *testing.T) {

diff --git a/command/agent/test-resources/client_with_template.hcl b/command/agent/test-resources/client_with_template.hcl
@@ -26,6 +26,12 @@ client {
       backoff     = "15s"
       max_backoff = "20s"
     }
+
+    nomad_retry {
+      attempts    = 15
+      backoff     = "20s"
+      max_backoff = "25s"
+    }
   }
 
 }
diff --git a/go.mod b/go.mod
@@ -42,7 +42,7 @@ require (
 	github.com/gosuri/uilive v0.0.4
 	github.com/grpc-ecosystem/go-grpc-middleware v1.2.1-0.20200228141219-3ce3d519df39
 	github.com/hashicorp/consul v1.7.8
-	github.com/hashicorp/consul-template v0.29.1
+	github.com/hashicorp/consul-template v0.29.2-0.20220803104536-583050a85eea
 	github.com/hashicorp/consul/api v1.13.0
 	github.com/hashicorp/consul/sdk v0.9.0
 	github.com/hashicorp/cronexpr v1.1.1
@@ -76,7 +76,7 @@ require (
 	github.com/hashicorp/logutils v1.0.0
 	github.com/hashicorp/memberlist v0.3.1
 	github.com/hashicorp/net-rpc-msgpackrpc v0.0.0-20151116020338-a14192a58a69
-	github.com/hashicorp/nomad/api v0.0.0-20220407202126-2eba643965c4
+	github.com/hashicorp/nomad/api v0.0.0-20220707195938-75f4c2237b28
 	github.com/hashicorp/raft v1.3.9
 	github.com/hashicorp/raft-boltdb/v2 v2.2.0
 	github.com/hashicorp/serf v0.9.7
@@ -214,7 +214,7 @@ require (
 	github.com/hashicorp/vault/api/auth/kubernetes v0.1.0 // indirect
 	github.com/hashicorp/vic v1.5.1-0.20190403131502-bbfe86ec9443 // indirect
 	github.com/huandu/xstrings v1.3.2 // indirect
-	github.com/imdario/mergo v0.3.12 // indirect
+	github.com/imdario/mergo v0.3.13 // indirect
 	github.com/ishidawataru/sctp v0.0.0-20191218070446-00ab2ac2db07 // indirect
 	github.com/jefferai/isbadcipher v0.0.0-20190226160619-51d2077c035f // indirect
 	github.com/jmespath/go-jmespath v0.4.0 // indirect

diff --git a/go.sum b/go.sum
@@ -660,8 +660,8 @@ github.com/grpc-ecosystem/grpc-gateway v1.9.5/go.mod h1:vNeuVxBJEsws4ogUvrchl83t
 github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw=
 github.com/hashicorp/consul v1.7.8 h1:hp308KxAf3zWoGuwp2e+0UUhrm6qHjeBQk3jCZ+bjcY=
 github.com/hashicorp/consul v1.7.8/go.mod h1:urbfGaVZDmnXC6geg0LYPh/SRUk1E8nfmDHpz+Q0nLw=
-github.com/hashicorp/consul-template v0.29.1 h1:icm/H7klHYlxpUoWqSmTIWaSLEfGqUJJBsZA/2JhTLU=
-github.com/hashicorp/consul-template v0.29.1/go.mod h1:QIohwBuXlKXtsmGGQdWrISlUy4E6LFg5tLZyrw4MyoU=
+github.com/hashicorp/consul-template v0.29.2-0.20220803104536-583050a85eea h1:d9frD3+sqQOG/4hOXLEfcXnNz+au0owaRUmM2WuzCBk=
+github.com/hashicorp/consul-template v0.29.2-0.20220803104536-583050a85eea/go.mod h1:i2oqMe0jIyHAKuimz7Q3sJU3vnwVx3QzDdDmrRrz5RI=
 github.com/hashicorp/consul/api v1.4.0/go.mod h1:xc8u05kyMa3Wjr9eEAsIAo3dg8+LywT5E/Cl7cNS5nU=
 github.com/hashicorp/consul/api v1.13.0 h1:2hnLQ0GjQvw7f3O61jMO8gbasZviZTrt9R8WzgiirHc=
 github.com/hashicorp/consul/api v1.13.0/go.mod h1:ZlVrynguJKcYr54zGaDbaL3fOvKC9m72FhPvA8T35KQ=
@@ -838,8 +838,9 @@ github.com/imdario/mergo v0.3.6/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJ
 github.com/imdario/mergo v0.3.8/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
 github.com/imdario/mergo v0.3.10/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
 github.com/imdario/mergo v0.3.11/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
-github.com/imdario/mergo v0.3.12 h1:b6R2BslTbIEToALKP7LxUvijTsNI9TAe80pLWN2g/HU=
 github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
+github.com/imdario/mergo v0.3.13 h1:lFzP57bqS/wsqKssCGmtLAb8A0wKjLGrve2q3PPVcBk=
+github.com/imdario/mergo v0.3.13/go.mod h1:4lJ1jqUDcsbIECGy0RUJAXNIhg+6ocWgb1ALK2O4oXg=
 github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
 github.com/ishidawataru/sctp v0.0.0-20191218070446-00ab2ac2db07 h1:rw3IAne6CDuVFlZbPOkA7bhxlqawFh7RJJ+CejfMaxE=
 github.com/ishidawataru/sctp v0.0.0-20191218070446-00ab2ac2db07/go.mod h1:co9pwDoBCm1kGxawmb4sPq0cSIOOWNPT4KnHotMP1Zg=
@@ -1879,6 +1880,7 @@ gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY=
 gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
+gopkg.in/yaml.v3 v3.0.0/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo=

diff --git a/website/content/docs/configuration/client.mdx b/website/content/docs/configuration/client.mdx
@@ -347,6 +347,31 @@ chroot as doing so would cause infinite recursion.
   }
   ```
 
+- `nomad_retry` `(map: { attempts = 0 backoff = "250ms" max_backoff = "1m" })` -
+  This controls the retry behavior when an error is returned from Nomad. Consul
+  Template is highly fault tolerant, meaning it does not exit in the face of failure.
+  Instead, it uses exponential back-off and retry functions to wait for the cluster
+  to become available, as is customary in distributed systems.
+
+  ```hcl
+  nomad_retry {
+    # This specifies the number of attempts to make before giving up. Each
+    # attempt adds the exponential backoff sleep time. Setting this to
+    # zero will implement an unlimited number of retries.
+    attempts = 0
+    # This is the base amount of time to sleep between retry attempts. Each
+    # retry sleeps for an exponent of 2 longer than this base. For 5 retries,
+    # the sleep times would be: 250ms, 500ms, 1s, 2s, then 4s.
+    backoff = "250ms"
+    # This is the maximum amount of time to sleep between retry attempts.
+    # When max_backoff is set to zero, there is no upper limit to the
+    # exponential sleep between retry attempts.
+    # If max_backoff is set to 10s and backoff is set to 1s, sleep times
+    # would be: 1s, 2s, 4s, 8s, 10s, 10s, ...
+    max_backoff = "1m"
+  }
+  ```
+
 ### `host_volume` Stanza
 
 The `host_volume` stanza is used to make volumes available to jobs.