diff --git a/.changelog/11606.txt b/.changelog/11606.txt new file mode 100644 index 000000000000..ee1e7cedb50b --- /dev/null +++ b/.changelog/11606.txt @@ -0,0 +1,6 @@ +```release-note:improvement +template: Expose consul-template configuration options at the client level for `consul_retry`, +`vault_retry`, `max_stale`, `block_query_wait` and `wait`. Expose per-template configuration +for wait that will override the client level configuration. Add `wait_bounds` to +allow operators to constrain per-template overrides at the client level. +``` diff --git a/api/tasks.go b/api/tasks.go index efc1b87af9de..10629aa2d2bb 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -772,6 +772,24 @@ func (a *TaskArtifact) Canonicalize() { } } +// WaitConfig is the Min/Max duration to wait for the Consul cluster to reach a +// consistent state before attempting to render Templates. +type WaitConfig struct { + Min *time.Duration `mapstructure:"min" hcl:"min"` + Max *time.Duration `mapstructure:"max" hcl:"max"` +} + +func (wc *WaitConfig) Copy() *WaitConfig { + if wc == nil { + return nil + } + + nwc := new(WaitConfig) + *nwc = *wc + + return nwc +} + type Template struct { SourcePath *string `mapstructure:"source" hcl:"source,optional"` DestPath *string `mapstructure:"destination" hcl:"destination,optional"` @@ -784,6 +802,7 @@ type Template struct { RightDelim *string `mapstructure:"right_delimiter" hcl:"right_delimiter,optional"` Envvars *bool `mapstructure:"env" hcl:"env,optional"` VaultGrace *time.Duration `mapstructure:"vault_grace" hcl:"vault_grace,optional"` + Wait *WaitConfig `mapstructure:"wait" hcl:"wait,block"` } func (tmpl *Template) Canonicalize() { diff --git a/api/tasks_test.go b/api/tasks_test.go index b922123f7a18..14ccf837e024 100644 --- a/api/tasks_test.go +++ b/api/tasks_test.go @@ -406,6 +406,91 @@ func TestTask_Canonicalize_TaskLifecycle(t *testing.T) { } } +func TestTask_Template_WaitConfig_Canonicalize_and_Copy(t *testing.T) { + taskWithWait := func(wc *WaitConfig) *Task { + return &Task{ + Templates: []*Template{ + { + Wait: wc, + }, + }, + } + } + + testCases := []struct { + name string + canonicalized *WaitConfig + copied *WaitConfig + task *Task + }{ + { + name: "all-fields", + task: taskWithWait(&WaitConfig{ + Min: timeToPtr(5), + Max: timeToPtr(10), + }), + canonicalized: &WaitConfig{ + Min: timeToPtr(5), + Max: timeToPtr(10), + }, + copied: &WaitConfig{ + Min: timeToPtr(5), + Max: timeToPtr(10), + }, + }, + { + name: "no-fields", + task: taskWithWait(&WaitConfig{}), + canonicalized: &WaitConfig{ + Min: nil, + Max: nil, + }, + copied: &WaitConfig{ + Min: nil, + Max: nil, + }, + }, + { + name: "min-only", + task: taskWithWait(&WaitConfig{ + Min: timeToPtr(5), + }), + canonicalized: &WaitConfig{ + Min: timeToPtr(5), + }, + copied: &WaitConfig{ + Min: timeToPtr(5), + }, + }, + { + name: "max-only", + task: taskWithWait(&WaitConfig{ + Max: timeToPtr(10), + }), + canonicalized: &WaitConfig{ + Max: timeToPtr(10), + }, + copied: &WaitConfig{ + Max: timeToPtr(10), + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tg := &TaskGroup{ + Name: stringToPtr("foo"), + } + j := &Job{ + ID: stringToPtr("test"), + } + require.Equal(t, tc.copied, tc.task.Templates[0].Wait.Copy()) + tc.task.Canonicalize(tg, j) + require.Equal(t, tc.canonicalized, tc.task.Templates[0].Wait) + }) + } +} + // Ensures no regression on https://github.com/hashicorp/nomad/issues/3132 func TestTaskGroup_Canonicalize_Update(t *testing.T) { // Job with an Empty() Update diff --git a/client/allocrunner/taskrunner/template/template.go b/client/allocrunner/taskrunner/template/template.go index 5c6202941c50..f37d76f7be82 100644 --- a/client/allocrunner/taskrunner/template/template.go +++ b/client/allocrunner/taskrunner/template/template.go @@ -102,9 +102,6 @@ type TaskTemplateManagerConfig struct { // MaxTemplateEventRate is the maximum rate at which we should emit events. MaxTemplateEventRate time.Duration - - // retryRate is only used for testing and is used to increase the retry rate - retryRate time.Duration } // Validate validates the configuration. @@ -191,7 +188,7 @@ func (tm *TaskTemplateManager) Stop() { // run is the long lived loop that handles errors and templates being rendered func (tm *TaskTemplateManager) run() { - // Runner is nil if there is no templates + // Runner is nil if there are no templates if tm.runner == nil { // Unblock the start if there is nothing to do close(tm.config.UnblockCh) @@ -602,6 +599,18 @@ func parseTemplateConfigs(config *TaskTemplateManagerConfig) (map[*ctconf.Templa ct.SandboxPath = &config.TaskDir } + if tmpl.Wait != nil { + if err := tmpl.Wait.Validate(); err != nil { + return nil, err + } + + ct.Wait = &ctconf.WaitConfig{ + Enabled: helper.BoolToPtr(true), + Min: tmpl.Wait.Min, + Max: tmpl.Wait.Max, + } + } + // Set the permissions if tmpl.Perms != "" { v, err := strconv.ParseUint(tmpl.Perms, 8, 12) @@ -635,13 +644,60 @@ func newRunnerConfig(config *TaskTemplateManagerConfig, } conf.Templates = &flat - // Force faster retries - if config.retryRate != 0 { - rate := config.retryRate - conf.Consul.Retry.Backoff = &rate + // Set the amount of time to do a blocking query for. + if cc.TemplateConfig.BlockQueryWaitTime != nil { + conf.BlockQueryWaitTime = cc.TemplateConfig.BlockQueryWaitTime + } + + // Set the stale-read threshold to allow queries to be served by followers + // if the last replicated data is within this bound. + if cc.TemplateConfig.MaxStale != nil { + conf.MaxStale = cc.TemplateConfig.MaxStale + } + + // Set the minimum and maximum amount of time to wait for the cluster to reach + // a consistent state before rendering a template. + if cc.TemplateConfig.Wait != nil { + // If somehow the WaitConfig wasn't set correctly upstream, return an error. + var err error + err = cc.TemplateConfig.Wait.Validate() + if err != nil { + return nil, err + } + conf.Wait, err = cc.TemplateConfig.Wait.ToConsulTemplate() + if err != nil { + return nil, err + } + } + + // Make sure any template specific configuration set by the job author is within + // the bounds set by the operator. + if cc.TemplateConfig.WaitBounds != nil { + // If somehow the WaitBounds weren't set correctly upstream, return an error. + err := cc.TemplateConfig.WaitBounds.Validate() + if err != nil { + return nil, err + } + + // Check and override with bounds + for _, tmpl := range *conf.Templates { + if tmpl.Wait == nil || !*tmpl.Wait.Enabled { + continue + } + if cc.TemplateConfig.WaitBounds.Min != nil { + if tmpl.Wait.Min != nil && *tmpl.Wait.Min < *cc.TemplateConfig.WaitBounds.Min { + tmpl.Wait.Min = &*cc.TemplateConfig.WaitBounds.Min + } + } + if cc.TemplateConfig.WaitBounds.Max != nil { + if tmpl.Wait.Max != nil && *tmpl.Wait.Max > *cc.TemplateConfig.WaitBounds.Max { + tmpl.Wait.Max = &*cc.TemplateConfig.WaitBounds.Max + } + } + } } - // Setup the Consul config + // Set up the Consul config if cc.ConsulConfig != nil { conf.Consul.Address = &cc.ConsulConfig.Addr conf.Consul.Token = &cc.ConsulConfig.Token @@ -675,6 +731,19 @@ func newRunnerConfig(config *TaskTemplateManagerConfig, Password: &parts[1], } } + + // Set the user-specified Consul RetryConfig + if cc.TemplateConfig.ConsulRetry != nil { + var err error + err = cc.TemplateConfig.ConsulRetry.Validate() + if err != nil { + return nil, err + } + conf.Consul.Retry, err = cc.TemplateConfig.ConsulRetry.ToConsulTemplate() + if err != nil { + return nil, err + } + } } // Get the Consul namespace from job/group config. This is the higher level @@ -683,7 +752,7 @@ func newRunnerConfig(config *TaskTemplateManagerConfig, conf.Consul.Namespace = &config.ConsulNamespace } - // Setup the Vault config + // Set up the Vault config // Always set these to ensure nothing is picked up from the environment emptyStr := "" conf.Vault.RenewToken = helper.BoolToPtr(false) @@ -724,6 +793,18 @@ func newRunnerConfig(config *TaskTemplateManagerConfig, ServerName: &emptyStr, } } + + // Set the user-specified Vault RetryConfig + if cc.TemplateConfig.VaultRetry != nil { + var err error + if err = cc.TemplateConfig.VaultRetry.Validate(); err != nil { + return nil, err + } + conf.Vault.Retry, err = cc.TemplateConfig.VaultRetry.ToConsulTemplate() + if err != nil { + return nil, err + } + } } conf.Finalize() diff --git a/client/allocrunner/taskrunner/template/template_test.go b/client/allocrunner/taskrunner/template/template_test.go index 32fa862a6ccf..dcd9a8eb0daf 100644 --- a/client/allocrunner/taskrunner/template/template_test.go +++ b/client/allocrunner/taskrunner/template/template_test.go @@ -16,6 +16,7 @@ import ( "testing" "time" + templateconfig "github.com/hashicorp/consul-template/config" ctestutil "github.com/hashicorp/consul/sdk/testutil" "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/config" @@ -145,6 +146,7 @@ func newTestHarness(t *testing.T, templates []*structs.Template, consul, vault b TemplateConfig: &config.ClientTemplateConfig{ FunctionDenylist: []string{"plugin"}, DisableSandbox: false, + ConsulRetry: &config.RetryConfig{Backoff: helper.TimeToPtr(10 * time.Millisecond)}, }}, emitRate: DefaultMaxTemplateEventRate, } @@ -202,7 +204,6 @@ func (h *testHarness) startWithErr() error { TaskDir: h.taskDir, EnvBuilder: h.envBuilder, MaxTemplateEventRate: h.emitRate, - retryRate: 10 * time.Millisecond, }) return err @@ -1914,3 +1915,244 @@ WAIT_LOOP: t.Fatalf("bad event, expected only 3 and 5 blocked got: %q", event.DisplayMessage) } } + +// TestTaskTemplateManager_ClientTemplateConfig_Set asserts that all client level +// configuration is accurately mapped from the client to the TaskTemplateManager +// and that any operator defined boundaries are enforced. +func TestTaskTemplateManager_ClientTemplateConfig_Set(t *testing.T) { + t.Parallel() + + testNS := "test-namespace" + + clientConfig := config.DefaultConfig() + clientConfig.Node = mock.Node() + + clientConfig.VaultConfig = &sconfig.VaultConfig{ + Enabled: helper.BoolToPtr(true), + Namespace: testNS, + } + + clientConfig.ConsulConfig = &sconfig.ConsulConfig{ + Namespace: testNS, + } + + // helper to reduce boilerplate + waitConfig := &config.WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(10 * time.Second), + } + // helper to reduce boilerplate + retryConfig := &config.RetryConfig{ + Attempts: helper.IntToPtr(5), + Backoff: helper.TimeToPtr(5 * time.Second), + MaxBackoff: helper.TimeToPtr(20 * time.Second), + } + + clientConfig.TemplateConfig.MaxStale = helper.TimeToPtr(5 * time.Second) + clientConfig.TemplateConfig.BlockQueryWaitTime = helper.TimeToPtr(60 * time.Second) + clientConfig.TemplateConfig.Wait = waitConfig.Copy() + clientConfig.TemplateConfig.ConsulRetry = retryConfig.Copy() + clientConfig.TemplateConfig.VaultRetry = retryConfig.Copy() + + alloc := mock.Alloc() + allocWithOverride := mock.Alloc() + allocWithOverride.Job.TaskGroups[0].Tasks[0].Templates = []*structs.Template{ + { + Wait: &structs.WaitConfig{ + Min: helper.TimeToPtr(2 * time.Second), + Max: helper.TimeToPtr(12 * time.Second), + }, + }, + } + + cases := []struct { + Name string + ClientTemplateConfig *config.ClientTemplateConfig + TTMConfig *TaskTemplateManagerConfig + ExpectedRunnerConfig *config.Config + ExpectedTemplateConfig *templateconfig.TemplateConfig + }{ + { + "basic-wait-config", + &config.ClientTemplateConfig{ + MaxStale: helper.TimeToPtr(5 * time.Second), + BlockQueryWaitTime: helper.TimeToPtr(60 * time.Second), + Wait: waitConfig.Copy(), + ConsulRetry: retryConfig.Copy(), + VaultRetry: retryConfig.Copy(), + }, + &TaskTemplateManagerConfig{ + ClientConfig: clientConfig, + VaultToken: "token", + EnvBuilder: taskenv.NewBuilder(clientConfig.Node, alloc, alloc.Job.TaskGroups[0].Tasks[0], clientConfig.Region), + }, + &config.Config{ + TemplateConfig: &config.ClientTemplateConfig{ + MaxStale: helper.TimeToPtr(5 * time.Second), + BlockQueryWaitTime: helper.TimeToPtr(60 * time.Second), + Wait: waitConfig.Copy(), + ConsulRetry: retryConfig.Copy(), + VaultRetry: retryConfig.Copy(), + }, + }, + &templateconfig.TemplateConfig{ + Wait: &templateconfig.WaitConfig{ + Enabled: helper.BoolToPtr(true), + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(10 * time.Second), + }, + }, + }, + { + "template-override", + &config.ClientTemplateConfig{ + MaxStale: helper.TimeToPtr(5 * time.Second), + BlockQueryWaitTime: helper.TimeToPtr(60 * time.Second), + Wait: waitConfig.Copy(), + ConsulRetry: retryConfig.Copy(), + VaultRetry: retryConfig.Copy(), + }, + &TaskTemplateManagerConfig{ + ClientConfig: clientConfig, + VaultToken: "token", + EnvBuilder: taskenv.NewBuilder(clientConfig.Node, allocWithOverride, allocWithOverride.Job.TaskGroups[0].Tasks[0], clientConfig.Region), + }, + &config.Config{ + TemplateConfig: &config.ClientTemplateConfig{ + MaxStale: helper.TimeToPtr(5 * time.Second), + BlockQueryWaitTime: helper.TimeToPtr(60 * time.Second), + Wait: waitConfig.Copy(), + ConsulRetry: retryConfig.Copy(), + VaultRetry: retryConfig.Copy(), + }, + }, + &templateconfig.TemplateConfig{ + Wait: &templateconfig.WaitConfig{ + Enabled: helper.BoolToPtr(true), + Min: helper.TimeToPtr(2 * time.Second), + Max: helper.TimeToPtr(12 * time.Second), + }, + }, + }, + { + "bounds-override", + &config.ClientTemplateConfig{ + MaxStale: helper.TimeToPtr(5 * time.Second), + BlockQueryWaitTime: helper.TimeToPtr(60 * time.Second), + Wait: waitConfig.Copy(), + WaitBounds: &config.WaitConfig{ + Min: helper.TimeToPtr(3 * time.Second), + Max: helper.TimeToPtr(11 * time.Second), + }, + ConsulRetry: retryConfig.Copy(), + VaultRetry: retryConfig.Copy(), + }, + &TaskTemplateManagerConfig{ + ClientConfig: clientConfig, + VaultToken: "token", + EnvBuilder: taskenv.NewBuilder(clientConfig.Node, allocWithOverride, allocWithOverride.Job.TaskGroups[0].Tasks[0], clientConfig.Region), + Templates: []*structs.Template{ + { + Wait: &structs.WaitConfig{ + Min: helper.TimeToPtr(2 * time.Second), + Max: helper.TimeToPtr(12 * time.Second), + }, + }, + }, + }, + &config.Config{ + TemplateConfig: &config.ClientTemplateConfig{ + MaxStale: helper.TimeToPtr(5 * time.Second), + BlockQueryWaitTime: helper.TimeToPtr(60 * time.Second), + Wait: waitConfig.Copy(), + WaitBounds: &config.WaitConfig{ + Min: helper.TimeToPtr(3 * time.Second), + Max: helper.TimeToPtr(11 * time.Second), + }, + ConsulRetry: retryConfig.Copy(), + VaultRetry: retryConfig.Copy(), + }, + }, + &templateconfig.TemplateConfig{ + Wait: &templateconfig.WaitConfig{ + Enabled: helper.BoolToPtr(true), + Min: helper.TimeToPtr(3 * time.Second), + Max: helper.TimeToPtr(11 * time.Second), + }, + }, + }, + } + + for _, _case := range cases { + t.Run(_case.Name, func(t *testing.T) { + // monkey patch the client config with the version of the ClientTemplateConfig we want to test. + _case.TTMConfig.ClientConfig.TemplateConfig = _case.ClientTemplateConfig + templateMapping, err := parseTemplateConfigs(_case.TTMConfig) + require.NoError(t, err) + + runnerConfig, err := newRunnerConfig(_case.TTMConfig, templateMapping) + require.NoError(t, err) + + // Direct properties + require.Equal(t, *_case.ExpectedRunnerConfig.TemplateConfig.MaxStale, *runnerConfig.MaxStale) + require.Equal(t, *_case.ExpectedRunnerConfig.TemplateConfig.BlockQueryWaitTime, *runnerConfig.BlockQueryWaitTime) + // WaitConfig + require.Equal(t, *_case.ExpectedRunnerConfig.TemplateConfig.Wait.Min, *runnerConfig.Wait.Min) + require.Equal(t, *_case.ExpectedRunnerConfig.TemplateConfig.Wait.Max, *runnerConfig.Wait.Max) + // Consul Retry + require.NotNil(t, runnerConfig.Consul) + require.NotNil(t, runnerConfig.Consul.Retry) + require.Equal(t, *_case.ExpectedRunnerConfig.TemplateConfig.ConsulRetry.Attempts, *runnerConfig.Consul.Retry.Attempts) + require.Equal(t, *_case.ExpectedRunnerConfig.TemplateConfig.ConsulRetry.Backoff, *runnerConfig.Consul.Retry.Backoff) + require.Equal(t, *_case.ExpectedRunnerConfig.TemplateConfig.ConsulRetry.MaxBackoff, *runnerConfig.Consul.Retry.MaxBackoff) + // Vault Retry + require.NotNil(t, runnerConfig.Vault) + require.NotNil(t, runnerConfig.Vault.Retry) + 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) + + // Test that wait_bounds are enforced + for _, tmpl := range *runnerConfig.Templates { + require.Equal(t, *_case.ExpectedTemplateConfig.Wait.Enabled, *tmpl.Wait.Enabled) + require.Equal(t, *_case.ExpectedTemplateConfig.Wait.Min, *tmpl.Wait.Min) + require.Equal(t, *_case.ExpectedTemplateConfig.Wait.Max, *tmpl.Wait.Max) + } + }) + } +} + +// TestTaskTemplateManager_Template_Wait_Set asserts that all template level +// configuration is accurately mapped from the template to the TaskTemplateManager's +// template config. +func TestTaskTemplateManager_Template_Wait_Set(t *testing.T) { + t.Parallel() + + c := config.DefaultConfig() + c.Node = mock.Node() + + alloc := mock.Alloc() + + ttmConfig := &TaskTemplateManagerConfig{ + ClientConfig: c, + VaultToken: "token", + EnvBuilder: taskenv.NewBuilder(c.Node, alloc, alloc.Job.TaskGroups[0].Tasks[0], c.Region), + Templates: []*structs.Template{ + { + Wait: &structs.WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(10 * time.Second), + }, + }, + }, + } + + templateMapping, err := parseTemplateConfigs(ttmConfig) + require.NoError(t, err) + + for k, _ := range templateMapping { + require.True(t, *k.Wait.Enabled) + require.Equal(t, 5*time.Second, *k.Wait.Min) + require.Equal(t, 10*time.Second, *k.Wait.Max) + } +} diff --git a/client/config/config.go b/client/config/config.go index fb9c7d1fc3cc..45400fcaa96c 100644 --- a/client/config/config.go +++ b/client/config/config.go @@ -1,13 +1,16 @@ package config import ( + "errors" "fmt" "io" "os" + "reflect" "strconv" "strings" "time" + "github.com/hashicorp/consul-template/config" "github.com/hashicorp/nomad/client/lib/cgutil" "github.com/hashicorp/nomad/command/agent/host" @@ -59,6 +62,8 @@ var ( // In non-systemd systems, this mount is a no-op and the path is ignored if not present. "/run/systemd/resolve": "/run/systemd/resolve", } + + DefaultTemplateMaxStale = 5 * time.Second ) // RPCHandler can be provided to the Client if there is a local server @@ -276,11 +281,72 @@ type Config struct { ReservableCores []uint16 } +// ClientTemplateConfig is configuration on the client specific to template +// rendering type ClientTemplateConfig struct { - FunctionDenylist []string - DisableSandbox bool + // FunctionDenylist disables functions in consul-template that + // are unsafe because they expose information from the client host. + FunctionDenylist []string `hcl:"function_denylist"` + + // Deprecated: COMPAT(1.0) consul-template uses inclusive language from + // v0.25.0 - function_blacklist is kept for compatibility + FunctionBlacklist []string `hcl:"function_blacklist"` + + // DisableSandbox allows templates to access arbitrary files on the + // client host. By default templates can access files only within + // the task directory. + DisableSandbox bool `hcl:"disable_file_sandbox"` + + // This is the maximum interval to allow "stale" data. By default, only the + // Consul leader will respond to queries; any requests to a follower will + // forward to the leader. In large clusters with many requests, this is not as + // scalable, so this option allows any follower to respond to a query, so long + // as the last-replicated data is within these bounds. Higher values result in + // less cluster load, but are more likely to have outdated data. + // NOTE: Since Consul Template uses a pointer, this field uses a pointer which + // is inconsistent with how Nomad typically works. This decision was made to + // maintain parity with the external subsystem, not to establish a new standard. + MaxStale *time.Duration `hcl:"-"` + MaxStaleHCL string `hcl:"max_stale,optional"` + + // BlockQueryWaitTime is amount of time in seconds to do a blocking query for. + // Many endpoints in Consul support a feature known as "blocking queries". + // A blocking query is used to wait for a potential change using long polling. + // NOTE: Since Consul Template uses a pointer, this field uses a pointer which + // is inconsistent with how Nomad typically works. This decision was made to + // maintain parity with the external subsystem, not to establish a new standard. + BlockQueryWaitTime *time.Duration `hcl:"-"` + BlockQueryWaitTimeHCL string `hcl:"block_query_wait,optional"` + + // Wait is the quiescence timers; it defines the minimum and maximum amount of + // time to wait for the Consul cluster to reach a consistent state before rendering a + // template. This is useful to enable in systems where Consul is experiencing + // a lot of flapping because it will reduce the number of times a template is rendered. + Wait *WaitConfig `hcl:"wait,optional" json:"-"` + + // WaitBounds allows operators to define boundaries on individual template wait + // configuration overrides. If set, this ensures that if a job author specifies + // a wait configuration with values the cluster operator does not allow, the + // cluster operator's boundary will be applied rather than the job author's + // out of bounds configuration. + WaitBounds *WaitConfig `hcl:"wait_bounds,optional" json:"-"` + + // This controls the retry behavior when an error is returned from Consul. + // 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. + ConsulRetry *RetryConfig `hcl:"consul_retry,optional"` + + // This controls the retry behavior when an error is returned from Vault. + // 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. + VaultRetry *RetryConfig `hcl:"vault_retry,optional"` } +// Copy returns a deep copy of a ClientTemplateConfig func (c *ClientTemplateConfig) Copy() *ClientTemplateConfig { if c == nil { return nil @@ -289,9 +355,378 @@ func (c *ClientTemplateConfig) Copy() *ClientTemplateConfig { nc := new(ClientTemplateConfig) *nc = *c nc.FunctionDenylist = helper.CopySliceString(nc.FunctionDenylist) + + if c.BlockQueryWaitTime != nil { + nc.BlockQueryWaitTime = &*c.BlockQueryWaitTime + } + + if c.MaxStale != nil { + nc.MaxStale = &*c.MaxStale + } + + if c.Wait != nil { + nc.Wait = c.Wait.Copy() + } + + if c.ConsulRetry != nil { + nc.ConsulRetry = c.ConsulRetry.Copy() + } + + if c.VaultRetry != nil { + nc.VaultRetry = c.VaultRetry.Copy() + } + return nc } +// Merge merges the values of two ClientTemplateConfigs. If first copies the receiver +// instance, and then overrides those values with the instance to merge with. +func (c *ClientTemplateConfig) Merge(b *ClientTemplateConfig) *ClientTemplateConfig { + if c == nil { + return b + } + + result := *c + + if b == nil { + return &result + } + + if b.BlockQueryWaitTime != nil { + result.BlockQueryWaitTime = b.BlockQueryWaitTime + } + if b.BlockQueryWaitTimeHCL != "" { + result.BlockQueryWaitTimeHCL = b.BlockQueryWaitTimeHCL + } + + if b.ConsulRetry != nil { + result.ConsulRetry = result.ConsulRetry.Merge(b.ConsulRetry) + } + + result.DisableSandbox = b.DisableSandbox + + // Maintain backward compatibility for older clients + if len(b.FunctionBlacklist) > 0 { + for _, fn := range b.FunctionBlacklist { + if !helper.SliceStringContains(result.FunctionBlacklist, fn) { + result.FunctionBlacklist = append(result.FunctionBlacklist, fn) + } + } + } + + if len(b.FunctionDenylist) > 0 { + for _, fn := range b.FunctionDenylist { + if !helper.SliceStringContains(result.FunctionDenylist, fn) { + result.FunctionDenylist = append(result.FunctionDenylist, fn) + } + } + } + + if b.MaxStale != nil { + result.MaxStale = b.MaxStale + } + + if b.MaxStaleHCL != "" { + result.MaxStaleHCL = b.MaxStaleHCL + } + + if b.Wait != nil { + result.Wait = result.Wait.Merge(b.Wait) + } + + if b.WaitBounds != nil { + result.WaitBounds = result.WaitBounds.Merge(b.WaitBounds) + } + + if b.VaultRetry != nil { + result.VaultRetry = result.VaultRetry.Merge(b.VaultRetry) + } + + return &result +} + +func (c *ClientTemplateConfig) IsEmpty() bool { + if c == nil { + return true + } + + return c.BlockQueryWaitTime == nil && + c.BlockQueryWaitTimeHCL == "" && + c.MaxStale == nil && + c.MaxStaleHCL == "" && + c.Wait.IsEmpty() && + c.ConsulRetry.IsEmpty() && + c.VaultRetry.IsEmpty() +} + +// WaitConfig is mirrored from templateconfig.WaitConfig because we need to handle +// the HCL conversion which happens in agent.ParseConfigFile +// NOTE: Since Consul Template requires pointers, this type uses pointers to fields +// which is inconsistent with how Nomad typically works. This decision was made +// to maintain parity with the external subsystem, not to establish a new standard. +type WaitConfig struct { + Min *time.Duration `hcl:"-"` + MinHCL string `hcl:"min,optional" json:"-"` + Max *time.Duration `hcl:"-"` + MaxHCL string `hcl:"max,optional" json:"-"` +} + +// Copy returns a deep copy of the receiver. +func (wc *WaitConfig) Copy() *WaitConfig { + if wc == nil { + return nil + } + + nwc := new(WaitConfig) + + if wc.Min != nil { + nwc.Min = &*wc.Min + } + + if wc.Max != nil { + nwc.Max = &*wc.Max + } + + return wc +} + +// Equals returns the result of reflect.DeepEqual +func (wc *WaitConfig) Equals(other *WaitConfig) bool { + return reflect.DeepEqual(wc, other) +} + +// IsEmpty returns true if the receiver only contains an instance with no fields set. +func (wc *WaitConfig) IsEmpty() bool { + if wc == nil { + return true + } + return wc.Equals(&WaitConfig{}) +} + +// Validate returns an error if the receiver is nil or empty or if Min is greater +// than Max the user specified Max. +func (wc *WaitConfig) Validate() error { + // If the config is nil or empty return false so that it is never assigned. + if wc == nil || wc.IsEmpty() { + return errors.New("wait config is nil or empty") + } + + // If min is nil, return + if wc.Min == nil { + return nil + } + + // If min isn't nil, make sure Max is less than Min. + if wc.Max != nil { + if *wc.Min > *wc.Max { + return fmt.Errorf("wait config min %d is greater than max %d", *wc.Min, *wc.Max) + } + } + + // Otherwise, return nil. Consul Template will set a Max based off of Min. + return nil +} + +// Merge merges two WaitConfigs. The passed instance always takes precedence. +func (wc *WaitConfig) Merge(b *WaitConfig) *WaitConfig { + if wc == nil { + return b + } + + result := *wc + if b == nil { + return &result + } + + if b.Min != nil { + result.Min = &*b.Min + } + + if b.MinHCL != "" { + result.MinHCL = b.MinHCL + } + + if b.Max != nil { + result.Max = &*b.Max + } + + if b.MaxHCL != "" { + result.MaxHCL = b.MaxHCL + } + + return &result +} + +// ToConsulTemplate converts a client WaitConfig instance to a consul-template WaitConfig +func (wc *WaitConfig) ToConsulTemplate() (*config.WaitConfig, error) { + if wc.IsEmpty() { + return nil, errors.New("wait config is empty") + } + + if err := wc.Validate(); err != nil { + return nil, err + } + + result := &config.WaitConfig{Enabled: helper.BoolToPtr(true)} + + if wc.Min != nil { + result.Min = wc.Min + } + + if wc.Max != nil { + result.Max = wc.Max + } + + return result, nil +} + +// RetryConfig is mirrored from templateconfig.WaitConfig because we need to handle +// the HCL indirection to support mapping in agent.ParseConfigFile. +// NOTE: Since Consul Template requires pointers, this type uses pointers to fields +// which is inconsistent with how Nomad typically works. However, since zero in +// Attempts and MaxBackoff have special meaning, it is necessary to know if the +// value was actually set rather than if it defaulted to 0. The rest of the fields +// use pointers to maintain parity with the external subystem, not to establish +// a new standard. +type RetryConfig struct { + // Attempts is the total number of maximum attempts to retry before letting + // the error fall through. + // 0 means unlimited. + Attempts *int `hcl:"attempts,optional"` + // Backoff is the base of the exponential backoff. This number will be + // multiplied by the next power of 2 on each iteration. + Backoff *time.Duration `hcl:"-"` + BackoffHCL string `hcl:"backoff,optional" json:"-"` + // MaxBackoff is an upper limit to the sleep time between retries + // A MaxBackoff of 0 means there is no limit to the exponential growth of the backoff. + MaxBackoff *time.Duration `hcl:"-"` + MaxBackoffHCL string `hcl:"max_backoff,optional" json:"-"` +} + +func (rc *RetryConfig) Copy() *RetryConfig { + if rc == nil { + return nil + } + + nrc := new(RetryConfig) + *nrc = *rc + + // Now copy pointer values + if rc.Attempts != nil { + nrc.Attempts = &*rc.Attempts + } + if rc.Backoff != nil { + nrc.Backoff = &*rc.Backoff + } + if rc.MaxBackoff != nil { + nrc.MaxBackoff = &*rc.MaxBackoff + } + + return nrc +} + +// Equals returns the result of reflect.DeepEqual +func (rc *RetryConfig) Equals(other *RetryConfig) bool { + return reflect.DeepEqual(rc, other) +} + +// IsEmpty returns true if the receiver only contains an instance with no fields set. +func (rc *RetryConfig) IsEmpty() bool { + if rc == nil { + return true + } + + return rc.Equals(&RetryConfig{}) +} + +// Validate returns an error if the receiver is nil or empty, or if Backoff +// is greater than MaxBackoff. +func (rc *RetryConfig) Validate() error { + // If the config is nil or empty return false so that it is never assigned. + if rc == nil || rc.IsEmpty() { + return errors.New("retry config is nil or empty") + } + + // If Backoff not set, no need to validate + if rc.Backoff == nil { + return nil + } + + // MaxBackoff nil will end up defaulted to 1 minutes. We should validate that + // the user supplied backoff does not exceed that. + if rc.MaxBackoff == nil && *rc.Backoff > config.DefaultRetryMaxBackoff { + return fmt.Errorf("retry config backoff %d is greater than default max_backoff %d", *rc.Backoff, config.DefaultRetryMaxBackoff) + } + + // MaxBackoff == 0 means backoff is unbounded. No need to validate. + if rc.MaxBackoff != nil && *rc.MaxBackoff == 0 { + return nil + } + + if rc.MaxBackoff != nil && *rc.Backoff > *rc.MaxBackoff { + return fmt.Errorf("retry config backoff %d is greater than max_backoff %d", *rc.Backoff, *rc.MaxBackoff) + } + + return nil +} + +// Merge merges two RetryConfigs. The passed instance always takes precedence. +func (rc *RetryConfig) Merge(b *RetryConfig) *RetryConfig { + if rc == nil { + return b + } + + result := *rc + if b == nil { + return &result + } + + if b.Attempts != nil { + result.Attempts = &*b.Attempts + } + + if b.Backoff != nil { + result.Backoff = &*b.Backoff + } + + if b.BackoffHCL != "" { + result.BackoffHCL = b.BackoffHCL + } + + if b.MaxBackoff != nil { + result.MaxBackoff = &*b.MaxBackoff + } + + if b.MaxBackoffHCL != "" { + result.MaxBackoffHCL = b.MaxBackoffHCL + } + + return &result +} + +// ToConsulTemplate converts a client RetryConfig instance to a consul-template RetryConfig +func (rc *RetryConfig) ToConsulTemplate() (*config.RetryConfig, error) { + if err := rc.Validate(); err != nil { + return nil, err + } + + result := &config.RetryConfig{Enabled: helper.BoolToPtr(true)} + + if rc.Attempts != nil { + result.Attempts = rc.Attempts + } + + if rc.Backoff != nil { + result.Backoff = rc.Backoff + } + + if rc.MaxBackoff != nil { + result.MaxBackoff = &*rc.MaxBackoff + } + + return result, nil +} + func (c *Config) Copy() *Config { nc := new(Config) *nc = *c diff --git a/client/config/config_test.go b/client/config/config_test.go index 9fa5e310e5c7..bef9995c6a1e 100644 --- a/client/config/config_test.go +++ b/client/config/config_test.go @@ -1,6 +1,13 @@ package config -import "testing" +import ( + "testing" + "time" + + "github.com/hashicorp/consul-template/config" + "github.com/hashicorp/nomad/helper" + "github.com/stretchr/testify/require" +) func TestConfigRead(t *testing.T) { config := Config{} @@ -34,3 +41,620 @@ func TestConfigReadDefault(t *testing.T) { t.Errorf("Expected %s, found %s", expected, actual) } } + +func mockWaitConfig() *WaitConfig { + return &WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(10 * time.Second), + } +} + +func TestWaitConfig_Copy(t *testing.T) { + cases := []struct { + Name string + Wait *WaitConfig + Expected *WaitConfig + }{ + { + "fully-populated", + mockWaitConfig(), + &WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(10 * time.Second), + }, + }, + { + "min-only", + &WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + }, + &WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + }, + }, + { + "max-only", + &WaitConfig{ + Max: helper.TimeToPtr(5 * time.Second), + }, + &WaitConfig{ + Max: helper.TimeToPtr(5 * time.Second), + }, + }, + } + + for _, _case := range cases { + t.Run(_case.Name, func(t *testing.T) { + result := _case.Expected.Equals(_case.Wait.Copy()) + if !result { + t.Logf("\nExpected %v\n Found %v", _case.Expected, result) + } + require.True(t, result) + }) + } +} + +func TestWaitConfig_IsEmpty(t *testing.T) { + cases := []struct { + Name string + Wait *WaitConfig + Expected bool + }{ + { + "is-nil", + nil, + true, + }, + { + "is-empty", + &WaitConfig{}, + true, + }, + { + "is-not-empty", + &WaitConfig{ + Min: helper.TimeToPtr(10 * time.Second), + }, + false, + }, + } + + for _, _case := range cases { + t.Run(_case.Name, func(t *testing.T) { + require.Equal(t, _case.Expected, _case.Wait.IsEmpty()) + }) + } +} + +func TestWaitConfig_IsEqual(t *testing.T) { + cases := []struct { + Name string + Wait *WaitConfig + Other *WaitConfig + Expected bool + }{ + { + "are-equal", + mockWaitConfig(), + &WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(10 * time.Second), + }, + true, + }, + { + "min-different", + mockWaitConfig(), + &WaitConfig{ + Min: helper.TimeToPtr(4 * time.Second), + Max: helper.TimeToPtr(10 * time.Second), + }, + false, + }, + { + "max-different", + mockWaitConfig(), + &WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(9 * time.Second), + }, + false, + }, + } + + for _, _case := range cases { + t.Run(_case.Name, func(t *testing.T) { + require.Equal(t, _case.Expected, _case.Wait.Equals(_case.Other)) + }) + } +} + +func TestWaitConfig_IsValid(t *testing.T) { + cases := []struct { + Name string + Retry *WaitConfig + Expected string + }{ + { + "is-valid", + &WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(10 * time.Second), + }, + "", + }, + { + "is-nil", + nil, + "is nil", + }, + { + "is-empty", + &WaitConfig{}, + "or empty", + }, + { + "min-greater-than-max", + &WaitConfig{ + Min: helper.TimeToPtr(10 * time.Second), + Max: helper.TimeToPtr(5 * time.Second), + }, + "greater than", + }, + { + "max-not-set", + &WaitConfig{ + Min: helper.TimeToPtr(10 * time.Second), + }, + "", + }, + } + + for _, _case := range cases { + t.Run(_case.Name, func(t *testing.T) { + if _case.Expected == "" { + require.Nil(t, _case.Retry.Validate()) + } else { + err := _case.Retry.Validate() + require.Contains(t, err.Error(), _case.Expected) + } + }) + } +} + +func TestWaitConfig_Merge(t *testing.T) { + cases := []struct { + Name string + Target *WaitConfig + Other *WaitConfig + Expected *WaitConfig + }{ + { + "all-fields", + mockWaitConfig(), + &WaitConfig{ + Min: helper.TimeToPtr(4 * time.Second), + Max: helper.TimeToPtr(9 * time.Second), + }, + &WaitConfig{ + Min: helper.TimeToPtr(4 * time.Second), + Max: helper.TimeToPtr(9 * time.Second), + }, + }, + { + "min-only", + mockWaitConfig(), + &WaitConfig{ + Min: helper.TimeToPtr(4 * time.Second), + Max: helper.TimeToPtr(10 * time.Second), + }, + &WaitConfig{ + Min: helper.TimeToPtr(4 * time.Second), + Max: helper.TimeToPtr(10 * time.Second), + }, + }, + { + "max-only", + mockWaitConfig(), + &WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(9 * time.Second), + }, + &WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(9 * time.Second), + }, + }, + } + + for _, _case := range cases { + t.Run(_case.Name, func(t *testing.T) { + merged := _case.Target.Merge(_case.Other) + result := _case.Expected.Equals(merged) + if !result { + t.Logf("\nExpected %v\n Found %v", _case.Expected, merged) + } + require.True(t, result) + }) + } +} + +func TestWaitConfig_ToConsulTemplate(t *testing.T) { + expected := config.WaitConfig{ + Enabled: helper.BoolToPtr(true), + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(10 * time.Second), + } + + clientWaitConfig := &WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(10 * time.Second), + } + + actual, err := clientWaitConfig.ToConsulTemplate() + require.NoError(t, err) + require.Equal(t, *expected.Min, *actual.Min) + require.Equal(t, *expected.Max, *actual.Max) +} + +func mockRetryConfig() *RetryConfig { + return &RetryConfig{ + Attempts: helper.IntToPtr(5), + Backoff: helper.TimeToPtr(5 * time.Second), + BackoffHCL: "5s", + MaxBackoff: helper.TimeToPtr(10 * time.Second), + MaxBackoffHCL: "10s", + } +} +func TestRetryConfig_Copy(t *testing.T) { + cases := []struct { + Name string + Retry *RetryConfig + Expected *RetryConfig + }{ + { + "fully-populated", + mockRetryConfig(), + &RetryConfig{ + Attempts: helper.IntToPtr(5), + Backoff: helper.TimeToPtr(5 * time.Second), + BackoffHCL: "5s", + MaxBackoff: helper.TimeToPtr(10 * time.Second), + MaxBackoffHCL: "10s", + }, + }, + { + "attempts-only", + &RetryConfig{ + Attempts: helper.IntToPtr(5), + }, + &RetryConfig{ + Attempts: helper.IntToPtr(5), + }, + }, + { + "backoff-only", + &RetryConfig{ + Backoff: helper.TimeToPtr(5 * time.Second), + }, + &RetryConfig{ + Backoff: helper.TimeToPtr(5 * time.Second), + }, + }, + { + "backoff-hcl-only", + &RetryConfig{ + BackoffHCL: "5s", + }, + &RetryConfig{ + BackoffHCL: "5s", + }, + }, + { + "max-backoff-only", + &RetryConfig{ + MaxBackoff: helper.TimeToPtr(10 * time.Second), + }, + &RetryConfig{ + MaxBackoff: helper.TimeToPtr(10 * time.Second), + }, + }, + { + "max-backoff-hcl-only", + &RetryConfig{ + MaxBackoffHCL: "10s", + }, + &RetryConfig{ + MaxBackoffHCL: "10s", + }, + }, + } + + for _, _case := range cases { + t.Run(_case.Name, func(t *testing.T) { + result := _case.Expected.Equals(_case.Retry.Copy()) + if !result { + t.Logf("\nExpected %v\n Found %v", _case.Expected, result) + } + require.True(t, result) + }) + } +} + +func TestRetryConfig_IsEmpty(t *testing.T) { + cases := []struct { + Name string + Retry *RetryConfig + Expected bool + }{ + { + "is-nil", + nil, + true, + }, + { + "is-empty", + &RetryConfig{}, + true, + }, + { + "is-not-empty", + &RetryConfig{ + Attempts: helper.IntToPtr(12), + }, + false, + }, + } + + for _, _case := range cases { + t.Run(_case.Name, func(t *testing.T) { + require.Equal(t, _case.Expected, _case.Retry.IsEmpty()) + }) + } +} + +func TestRetryConfig_IsEqual(t *testing.T) { + cases := []struct { + Name string + Retry *RetryConfig + Other *RetryConfig + Expected bool + }{ + { + "are-equal", + mockRetryConfig(), + &RetryConfig{ + Attempts: helper.IntToPtr(5), + Backoff: helper.TimeToPtr(5 * time.Second), + BackoffHCL: "5s", + MaxBackoff: helper.TimeToPtr(10 * time.Second), + MaxBackoffHCL: "10s", + }, + true, + }, + { + "attempts-different", + mockRetryConfig(), + &RetryConfig{ + Attempts: helper.IntToPtr(4), + Backoff: helper.TimeToPtr(5 * time.Second), + BackoffHCL: "5s", + MaxBackoff: helper.TimeToPtr(10 * time.Second), + MaxBackoffHCL: "10s", + }, + false, + }, + { + "backoff-different", + mockRetryConfig(), + &RetryConfig{ + Attempts: helper.IntToPtr(5), + Backoff: helper.TimeToPtr(4 * time.Second), + BackoffHCL: "5s", + MaxBackoff: helper.TimeToPtr(10 * time.Second), + MaxBackoffHCL: "10s", + }, + false, + }, + { + "backoff-hcl-different", + mockRetryConfig(), + &RetryConfig{ + Attempts: helper.IntToPtr(5), + Backoff: helper.TimeToPtr(5 * time.Second), + BackoffHCL: "4s", + MaxBackoff: helper.TimeToPtr(10 * time.Second), + MaxBackoffHCL: "10s", + }, + false, + }, + { + "max-backoff-different", + mockRetryConfig(), + &RetryConfig{ + Attempts: helper.IntToPtr(5), + Backoff: helper.TimeToPtr(5 * time.Second), + BackoffHCL: "5s", + MaxBackoff: helper.TimeToPtr(9 * time.Second), + MaxBackoffHCL: "10s", + }, + false, + }, + { + "max-backoff-hcl-different", + mockRetryConfig(), + &RetryConfig{ + Attempts: helper.IntToPtr(5), + Backoff: helper.TimeToPtr(5 * time.Second), + BackoffHCL: "5s", + MaxBackoff: helper.TimeToPtr(10 * time.Second), + MaxBackoffHCL: "9s", + }, + false, + }, + } + + for _, _case := range cases { + t.Run(_case.Name, func(t *testing.T) { + require.Equal(t, _case.Expected, _case.Retry.Equals(_case.Other)) + }) + } +} + +func TestRetryConfig_IsValid(t *testing.T) { + cases := []struct { + Name string + Retry *RetryConfig + Expected string + }{ + { + "is-valid", + &RetryConfig{ + Backoff: helper.TimeToPtr(5 * time.Second), + MaxBackoff: helper.TimeToPtr(10 * time.Second), + }, + "", + }, + { + "is-nil", + nil, + "is nil", + }, + { + "is-empty", + &RetryConfig{}, + "or empty", + }, + { + "backoff-greater-than-max-backoff", + &RetryConfig{ + Backoff: helper.TimeToPtr(10 * time.Second), + MaxBackoff: helper.TimeToPtr(5 * time.Second), + }, + "greater than max_backoff", + }, + { + "backoff-not-set", + &RetryConfig{ + MaxBackoff: helper.TimeToPtr(10 * time.Second), + }, + "", + }, + { + "max-backoff-not-set", + &RetryConfig{ + Backoff: helper.TimeToPtr(2 * time.Minute), + }, + "greater than default", + }, + { + "max-backoff-unbounded", + &RetryConfig{ + Backoff: helper.TimeToPtr(10 * time.Second), + MaxBackoff: helper.TimeToPtr(0 * time.Second), + }, + "", + }, + } + + for _, _case := range cases { + t.Run(_case.Name, func(t *testing.T) { + if _case.Expected == "" { + require.Nil(t, _case.Retry.Validate()) + } else { + err := _case.Retry.Validate() + require.Contains(t, err.Error(), _case.Expected) + } + }) + } +} + +func TestRetryConfig_Merge(t *testing.T) { + cases := []struct { + Name string + Target *RetryConfig + Other *RetryConfig + Expected *RetryConfig + }{ + { + "all-fields", + mockRetryConfig(), + &RetryConfig{ + Attempts: helper.IntToPtr(4), + Backoff: helper.TimeToPtr(4 * time.Second), + BackoffHCL: "4s", + MaxBackoff: helper.TimeToPtr(9 * time.Second), + MaxBackoffHCL: "9s", + }, + &RetryConfig{ + Attempts: helper.IntToPtr(4), + Backoff: helper.TimeToPtr(4 * time.Second), + BackoffHCL: "4s", + MaxBackoff: helper.TimeToPtr(9 * time.Second), + MaxBackoffHCL: "9s", + }, + }, + { + "attempts-only", + mockRetryConfig(), + &RetryConfig{ + Attempts: helper.IntToPtr(4), + Backoff: helper.TimeToPtr(5 * time.Second), + BackoffHCL: "5s", + MaxBackoff: helper.TimeToPtr(10 * time.Second), + MaxBackoffHCL: "10s", + }, + &RetryConfig{ + Attempts: helper.IntToPtr(4), + Backoff: helper.TimeToPtr(5 * time.Second), + BackoffHCL: "5s", + MaxBackoff: helper.TimeToPtr(10 * time.Second), + MaxBackoffHCL: "10s", + }, + }, + { + "multi-field", + mockRetryConfig(), + &RetryConfig{ + Attempts: helper.IntToPtr(5), + Backoff: helper.TimeToPtr(4 * time.Second), + BackoffHCL: "4s", + MaxBackoff: helper.TimeToPtr(9 * time.Second), + MaxBackoffHCL: "9s", + }, + &RetryConfig{ + Attempts: helper.IntToPtr(5), + Backoff: helper.TimeToPtr(4 * time.Second), + BackoffHCL: "4s", + MaxBackoff: helper.TimeToPtr(9 * time.Second), + MaxBackoffHCL: "9s", + }, + }, + } + + for _, _case := range cases { + t.Run(_case.Name, func(t *testing.T) { + merged := _case.Target.Merge(_case.Other) + result := _case.Expected.Equals(merged) + if !result { + t.Logf("\nExpected %v\n Found %v", _case.Expected, merged) + } + require.True(t, result) + }) + } +} + +func TestRetryConfig_ToConsulTemplate(t *testing.T) { + expected := config.RetryConfig{ + Enabled: helper.BoolToPtr(true), + Attempts: helper.IntToPtr(5), + Backoff: helper.TimeToPtr(5 * time.Second), + MaxBackoff: helper.TimeToPtr(10 * time.Second), + } + + actual := mockRetryConfig() + + require.Equal(t, *expected.Attempts, *actual.Attempts) + require.Equal(t, *expected.Backoff, *actual.Backoff) + require.Equal(t, *expected.MaxBackoff, *actual.MaxBackoff) +} diff --git a/command/agent/agent.go b/command/agent/agent.go index c608e1d8ba20..b89d2d5a324b 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -404,7 +404,6 @@ func convertServerConfig(agentConfig *Config) (*nomad.Config, error) { conf.StatsCollectionInterval = agentConfig.Telemetry.collectionInterval conf.DisableDispatchedJobSummaryMetrics = agentConfig.Telemetry.DisableDispatchedJobSummaryMetrics - // Parse Limits timeout from a string into durations if d, err := time.ParseDuration(agentConfig.Limits.RPCHandshakeTimeout); err != nil { return nil, fmt.Errorf("error parsing rpc_handshake_timeout: %v", err) } else if d < 0 { @@ -545,7 +544,7 @@ func (a *Agent) finalizeClientConfig(c *clientconfig.Config) error { // Config. There may be missing fields that must be set by the agent. To do this // call finalizeServerConfig func convertClientConfig(agentConfig *Config) (*clientconfig.Config, error) { - // Setup the configuration + // Set up the configuration conf := agentConfig.ClientConfig if conf == nil { conf = clientconfig.DefaultConfig() @@ -595,12 +594,10 @@ func convertClientConfig(agentConfig *Config) (*clientconfig.Config, error) { conf.MaxDynamicPort = agentConfig.Client.MaxDynamicPort conf.MinDynamicPort = agentConfig.Client.MinDynamicPort conf.DisableRemoteExec = agentConfig.Client.DisableRemoteExec - if agentConfig.Client.TemplateConfig.FunctionBlacklist != nil { - conf.TemplateConfig.FunctionDenylist = agentConfig.Client.TemplateConfig.FunctionBlacklist - } else { - conf.TemplateConfig.FunctionDenylist = agentConfig.Client.TemplateConfig.FunctionDenylist + + if agentConfig.Client.TemplateConfig != nil { + conf.TemplateConfig = agentConfig.Client.TemplateConfig.Copy() } - conf.TemplateConfig.DisableSandbox = agentConfig.Client.TemplateConfig.DisableSandbox hvMap := make(map[string]*structs.ClientHostVolumeConfig, len(agentConfig.Client.HostVolumes)) for _, v := range agentConfig.Client.HostVolumes { diff --git a/command/agent/config.go b/command/agent/config.go index 48573396c1f2..4040355d113c 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -278,7 +278,7 @@ type ClientConfig struct { DisableRemoteExec bool `hcl:"disable_remote_exec"` // TemplateConfig includes configuration for template rendering - TemplateConfig *ClientTemplateConfig `hcl:"template"` + TemplateConfig *client.ClientTemplateConfig `hcl:"template"` // ServerJoin contains information that is used to attempt to join servers ServerJoin *ServerJoin `hcl:"server_join"` @@ -321,24 +321,6 @@ type ClientConfig struct { ExtraKeysHCL []string `hcl:",unusedKeys" json:"-"` } -// ClientTemplateConfig is configuration on the client specific to template -// rendering -type ClientTemplateConfig struct { - - // FunctionDenylist disables functions in consul-template that - // are unsafe because they expose information from the client host. - FunctionDenylist []string `hcl:"function_denylist"` - - // Deprecated: COMPAT(1.0) consul-template uses inclusive language from - // v0.25.0 - function_blacklist is kept for compatibility - FunctionBlacklist []string `hcl:"function_blacklist"` - - // DisableSandbox allows templates to access arbitrary files on the - // client host. By default templates can access files only within - // the task directory. - DisableSandbox bool `hcl:"disable_file_sandbox"` -} - // ACLConfig is configuration specific to the ACL system type ACLConfig struct { // Enabled controls if we are enforce and manage ACLs @@ -910,7 +892,7 @@ func DevConfig(mode *devModeConfig) *Config { conf.Client.GCDiskUsageThreshold = 99 conf.Client.GCInodeUsageThreshold = 99 conf.Client.GCMaxAllocs = 50 - conf.Client.TemplateConfig = &ClientTemplateConfig{ + conf.Client.TemplateConfig = &client.ClientTemplateConfig{ FunctionDenylist: []string{"plugin"}, DisableSandbox: false, } @@ -959,7 +941,7 @@ func DefaultConfig() *Config { RetryInterval: 30 * time.Second, RetryMaxAttempts: 0, }, - TemplateConfig: &ClientTemplateConfig{ + TemplateConfig: &client.ClientTemplateConfig{ FunctionDenylist: []string{"plugin"}, DisableSandbox: false, }, @@ -1706,8 +1688,11 @@ func (a *ClientConfig) Merge(b *ClientConfig) *ClientConfig { result.DisableRemoteExec = b.DisableRemoteExec } - if b.TemplateConfig != nil { - result.TemplateConfig = b.TemplateConfig + if result.TemplateConfig == nil && b.TemplateConfig != nil { + templateConfig := *b.TemplateConfig + result.TemplateConfig = &templateConfig + } else if b.TemplateConfig != nil { + result.TemplateConfig = result.TemplateConfig.Merge(b.TemplateConfig) } // Add the servers diff --git a/command/agent/config_parse.go b/command/agent/config_parse.go index b745835711df..363db9e1e331 100644 --- a/command/agent/config_parse.go +++ b/command/agent/config_parse.go @@ -9,10 +9,12 @@ import ( "time" "github.com/hashicorp/hcl" + client "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/nomad/structs/config" ) +// ParseConfigFile returns an agent.Config from parsed from a file. func ParseConfigFile(path string) (*Config, error) { // slurp var buf bytes.Buffer @@ -32,7 +34,15 @@ func ParseConfigFile(path string) (*Config, error) { // parse c := &Config{ - Client: &ClientConfig{ServerJoin: &ServerJoin{}}, + Client: &ClientConfig{ + ServerJoin: &ServerJoin{}, + TemplateConfig: &client.ClientTemplateConfig{ + Wait: &client.WaitConfig{}, + WaitBounds: &client.WaitConfig{}, + ConsulRetry: &client.RetryConfig{}, + VaultRetry: &client.RetryConfig{}, + }, + }, ACL: &ACLConfig{}, Audit: &config.AuditConfig{}, Server: &ServerConfig{ServerJoin: &ServerJoin{}}, @@ -48,31 +58,79 @@ func ParseConfigFile(path string) (*Config, error) { } // convert strings to time.Durations - tds := []td{ - {"gc_interval", &c.Client.GCInterval, &c.Client.GCIntervalHCL}, - {"acl.token_ttl", &c.ACL.TokenTTL, &c.ACL.TokenTTLHCL}, - {"acl.policy_ttl", &c.ACL.PolicyTTL, &c.ACL.PolicyTTLHCL}, - {"client.server_join.retry_interval", &c.Client.ServerJoin.RetryInterval, &c.Client.ServerJoin.RetryIntervalHCL}, - {"server.heartbeat_grace", &c.Server.HeartbeatGrace, &c.Server.HeartbeatGraceHCL}, - {"server.min_heartbeat_ttl", &c.Server.MinHeartbeatTTL, &c.Server.MinHeartbeatTTLHCL}, - {"server.failover_heartbeat_ttl", &c.Server.FailoverHeartbeatTTL, &c.Server.FailoverHeartbeatTTLHCL}, - {"server.retry_interval", &c.Server.RetryInterval, &c.Server.RetryIntervalHCL}, - {"server.server_join.retry_interval", &c.Server.ServerJoin.RetryInterval, &c.Server.ServerJoin.RetryIntervalHCL}, - {"consul.timeout", &c.Consul.Timeout, &c.Consul.TimeoutHCL}, - {"autopilot.server_stabilization_time", &c.Autopilot.ServerStabilizationTime, &c.Autopilot.ServerStabilizationTimeHCL}, - {"autopilot.last_contact_threshold", &c.Autopilot.LastContactThreshold, &c.Autopilot.LastContactThresholdHCL}, - {"telemetry.collection_interval", &c.Telemetry.collectionInterval, &c.Telemetry.CollectionInterval}, + tds := []durationConversionMap{ + {"gc_interval", &c.Client.GCInterval, &c.Client.GCIntervalHCL, nil}, + {"acl.token_ttl", &c.ACL.TokenTTL, &c.ACL.TokenTTLHCL, nil}, + {"acl.policy_ttl", &c.ACL.PolicyTTL, &c.ACL.PolicyTTLHCL, nil}, + {"client.server_join.retry_interval", &c.Client.ServerJoin.RetryInterval, &c.Client.ServerJoin.RetryIntervalHCL, nil}, + {"server.heartbeat_grace", &c.Server.HeartbeatGrace, &c.Server.HeartbeatGraceHCL, nil}, + {"server.min_heartbeat_ttl", &c.Server.MinHeartbeatTTL, &c.Server.MinHeartbeatTTLHCL, nil}, + {"server.failover_heartbeat_ttl", &c.Server.FailoverHeartbeatTTL, &c.Server.FailoverHeartbeatTTLHCL, nil}, + {"server.retry_interval", &c.Server.RetryInterval, &c.Server.RetryIntervalHCL, nil}, + {"server.server_join.retry_interval", &c.Server.ServerJoin.RetryInterval, &c.Server.ServerJoin.RetryIntervalHCL, nil}, + {"consul.timeout", &c.Consul.Timeout, &c.Consul.TimeoutHCL, nil}, + {"autopilot.server_stabilization_time", &c.Autopilot.ServerStabilizationTime, &c.Autopilot.ServerStabilizationTimeHCL, nil}, + {"autopilot.last_contact_threshold", &c.Autopilot.LastContactThreshold, &c.Autopilot.LastContactThresholdHCL, nil}, + {"telemetry.collection_interval", &c.Telemetry.collectionInterval, &c.Telemetry.CollectionInterval, nil}, + {"client.template.block_query_wait", nil, &c.Client.TemplateConfig.BlockQueryWaitTimeHCL, + func(d *time.Duration) { + c.Client.TemplateConfig.BlockQueryWaitTime = d + }, + }, + {"client.template.max_stale", nil, &c.Client.TemplateConfig.MaxStaleHCL, + func(d *time.Duration) { + c.Client.TemplateConfig.MaxStale = d + }}, + {"client.template.wait.min", nil, &c.Client.TemplateConfig.Wait.MinHCL, + func(d *time.Duration) { + c.Client.TemplateConfig.Wait.Min = d + }, + }, + {"client.template.wait.max", nil, &c.Client.TemplateConfig.Wait.MaxHCL, + func(d *time.Duration) { + c.Client.TemplateConfig.Wait.Max = d + }, + }, + {"client.template.wait_bounds.min", nil, &c.Client.TemplateConfig.WaitBounds.MinHCL, + func(d *time.Duration) { + c.Client.TemplateConfig.WaitBounds.Min = d + }, + }, + {"client.template.wait_bounds.max", nil, &c.Client.TemplateConfig.WaitBounds.MaxHCL, + func(d *time.Duration) { + c.Client.TemplateConfig.WaitBounds.Max = d + }, + }, + {"client.template.consul_retry.backoff", nil, &c.Client.TemplateConfig.ConsulRetry.BackoffHCL, + func(d *time.Duration) { + c.Client.TemplateConfig.ConsulRetry.Backoff = d + }, + }, + {"client.template.consul_retry.max_backoff", nil, &c.Client.TemplateConfig.ConsulRetry.MaxBackoffHCL, + func(d *time.Duration) { + c.Client.TemplateConfig.ConsulRetry.MaxBackoff = d + }, + }, + {"client.template.vault_retry.backoff", nil, &c.Client.TemplateConfig.VaultRetry.BackoffHCL, + func(d *time.Duration) { + c.Client.TemplateConfig.VaultRetry.Backoff = d + }, + }, + {"client.template.vault_retry.max_backoff", nil, &c.Client.TemplateConfig.VaultRetry.MaxBackoffHCL, + func(d *time.Duration) { + c.Client.TemplateConfig.VaultRetry.MaxBackoff = d + }, + }, } // Add enterprise audit sinks for time.Duration parsing for i, sink := range c.Audit.Sinks { - tds = append(tds, td{ - fmt.Sprintf("audit.sink.%d", i), &sink.RotateDuration, &sink.RotateDurationHCL, - }) + tds = append(tds, durationConversionMap{ + fmt.Sprintf("audit.sink.%d", i), &sink.RotateDuration, &sink.RotateDurationHCL, nil}) } // convert strings to time.Durations - err = durations(tds) + err = convertDurations(tds) if err != nil { return nil, err } @@ -83,27 +141,39 @@ func ParseConfigFile(path string) (*Config, error) { return nil, err } + // Set client template config or its members to nil if not set. + finalizeClientTemplateConfig(c) + return c, nil } -// td holds args for one duration conversion -type td struct { - path string - td *time.Duration - str *string +// durationConversionMap holds args for one duration conversion +type durationConversionMap struct { + targetFieldPath string + targetField *time.Duration + sourceField *string + setFunc func(*time.Duration) } -// durations parses the duration strings specified in the config files +// convertDurations parses the duration strings specified in the config files // into time.Durations -func durations(xs []td) error { +func convertDurations(xs []durationConversionMap) error { for _, x := range xs { - if x.td != nil && x.str != nil && "" != *x.str { - d, err := time.ParseDuration(*x.str) + // if targetField is not a pointer itself, use the field map. + if x.targetField != nil && x.sourceField != nil && "" != *x.sourceField { + d, err := time.ParseDuration(*x.sourceField) if err != nil { - return fmt.Errorf("%s can't parse time duration %s", x.path, *x.str) + return fmt.Errorf("%s can't parse time duration %s", x.targetFieldPath, *x.sourceField) } - *x.td = d + *x.targetField = d + } else if x.setFunc != nil && x.sourceField != nil && "" != *x.sourceField { + // if targetField is a pointer itself, use the setFunc closure. + d, err := time.ParseDuration(*x.sourceField) + if err != nil { + return fmt.Errorf("%s can't parse time duration %s", x.targetFieldPath, *x.sourceField) + } + x.setFunc(&d) } } @@ -168,3 +238,29 @@ func extraKeys(c *Config) error { return helper.UnusedKeys(c) } + +// hcl.Decode will error if the ClientTemplateConfig isn't initialized with empty +// structs, however downstream code expect nils if the struct only contains fields +// with the zero value for its type. This function nils out type members that are +// structs where all the member fields are just the zero value for its type. +func finalizeClientTemplateConfig(config *Config) { + if config.Client.TemplateConfig.Wait.IsEmpty() { + config.Client.TemplateConfig.Wait = nil + } + + if config.Client.TemplateConfig.WaitBounds.IsEmpty() { + config.Client.TemplateConfig.WaitBounds = nil + } + + if config.Client.TemplateConfig.ConsulRetry.IsEmpty() { + config.Client.TemplateConfig.ConsulRetry = nil + } + + if config.Client.TemplateConfig.VaultRetry.IsEmpty() { + config.Client.TemplateConfig.VaultRetry = nil + } + + if config.Client.TemplateConfig.IsEmpty() { + config.Client.TemplateConfig = nil + } +} diff --git a/command/agent/config_test.go b/command/agent/config_test.go index b226b430207a..72aa864b319b 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -13,6 +13,7 @@ import ( "time" sockaddr "github.com/hashicorp/go-sockaddr" + client "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/testutil" "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/helper/freeport" @@ -115,7 +116,7 @@ func TestConfig_Merge(t *testing.T) { MaxKillTimeout: "20s", ClientMaxPort: 19996, DisableRemoteExec: false, - TemplateConfig: &ClientTemplateConfig{ + TemplateConfig: &client.ClientTemplateConfig{ FunctionDenylist: []string{"plugin"}, DisableSandbox: false, }, @@ -299,7 +300,7 @@ func TestConfig_Merge(t *testing.T) { MemoryMB: 105, MaxKillTimeout: "50s", DisableRemoteExec: false, - TemplateConfig: &ClientTemplateConfig{ + TemplateConfig: &client.ClientTemplateConfig{ FunctionDenylist: []string{"plugin"}, DisableSandbox: false, }, @@ -1365,6 +1366,53 @@ func TestEventBroker_Parse(t *testing.T) { } } +func TestConfig_LoadConsulTemplateConfig(t *testing.T) { + defaultConfig := DefaultConfig() + // Test that loading without template config didn't create load errors + agentConfig, err := LoadConfig("test-resources/minimal_client.hcl") + require.NoError(t, err) + + // Test loading with this config didn't create load errors + agentConfig, err = LoadConfig("test-resources/client_with_template.hcl") + require.NoError(t, err) + + agentConfig = defaultConfig.Merge(agentConfig) + + clientAgent := Agent{config: agentConfig} + clientConfig, err := clientAgent.clientConfig() + require.NoError(t, err) + + templateConfig := clientConfig.TemplateConfig + + // Make sure all fields to test are set + require.NotNil(t, templateConfig.BlockQueryWaitTime) + require.NotNil(t, templateConfig.MaxStale) + require.NotNil(t, templateConfig.Wait) + require.NotNil(t, templateConfig.WaitBounds) + require.NotNil(t, templateConfig.ConsulRetry) + require.NotNil(t, templateConfig.VaultRetry) + + // Direct properties + require.Equal(t, 300*time.Second, *templateConfig.MaxStale) + require.Equal(t, 90*time.Second, *templateConfig.BlockQueryWaitTime) + // Wait + require.Equal(t, 2*time.Second, *templateConfig.Wait.Min) + require.Equal(t, 60*time.Second, *templateConfig.Wait.Max) + // WaitBounds + require.Equal(t, 2*time.Second, *templateConfig.WaitBounds.Min) + require.Equal(t, 60*time.Second, *templateConfig.WaitBounds.Max) + // Consul Retry + require.NotNil(t, templateConfig.ConsulRetry) + require.Equal(t, 5, *templateConfig.ConsulRetry.Attempts) + require.Equal(t, 5*time.Second, *templateConfig.ConsulRetry.Backoff) + require.Equal(t, 10*time.Second, *templateConfig.ConsulRetry.MaxBackoff) + // Vault Retry + require.NotNil(t, templateConfig.VaultRetry) + 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) +} + func TestParseMultipleIPTemplates(t *testing.T) { testCases := []struct { name string diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 1ff8a7bde07c..b3a4b6bd225a 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1159,6 +1159,7 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup, RightDelim: *template.RightDelim, Envvars: *template.Envvars, VaultGrace: *template.VaultGrace, + Wait: ApiWaitConfigToStructsWaitConfig(template.Wait), }) } } @@ -1177,6 +1178,19 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup, } } +// ApiWaitConfigToStructsWaitConfig is a copy and type conversion between the API +// representation of a WaitConfig from a struct representation of a WaitConfig. +func ApiWaitConfigToStructsWaitConfig(waitConfig *api.WaitConfig) *structs.WaitConfig { + if waitConfig == nil { + return nil + } + + return &structs.WaitConfig{ + Min: &*waitConfig.Min, + Max: &*waitConfig.Max, + } +} + func ApiCSIPluginConfigToStructsCSIPluginConfig(apiConfig *api.TaskCSIPluginConfig) *structs.TaskCSIPluginConfig { if apiConfig == nil { return nil diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 841090014b28..76b4bb9152b1 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -2498,6 +2498,10 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { LeftDelim: helper.StringToPtr("abc"), RightDelim: helper.StringToPtr("def"), Envvars: helper.BoolToPtr(true), + Wait: &api.WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(10 * time.Second), + }, }, }, DispatchPayload: &api.DispatchPayloadConfig{ @@ -2891,6 +2895,10 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { LeftDelim: "abc", RightDelim: "def", Envvars: true, + Wait: &structs.WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(10 * time.Second), + }, }, }, DispatchPayload: &structs.DispatchPayloadConfig{ diff --git a/command/agent/test-resources/client_with_template.hcl b/command/agent/test-resources/client_with_template.hcl new file mode 100644 index 000000000000..f094cda437e7 --- /dev/null +++ b/command/agent/test-resources/client_with_template.hcl @@ -0,0 +1,31 @@ +client { + enabled = true + + template { + max_stale = "300s" + block_query_wait = "90s" + + wait { + min = "2s" + max = "60s" + } + + wait_bounds { + min = "2s" + max = "60s" + } + + consul_retry { + attempts = 5 + backoff = "5s" + max_backoff = "10s" + } + + vault_retry { + attempts = 10 + backoff = "15s" + max_backoff = "20s" + } + } + +} diff --git a/command/agent/test-resources/minimal_client.hcl b/command/agent/test-resources/minimal_client.hcl new file mode 100644 index 000000000000..cae89867b8f7 --- /dev/null +++ b/command/agent/test-resources/minimal_client.hcl @@ -0,0 +1,3 @@ +client { + enabled = true +} diff --git a/jobspec2/parse_test.go b/jobspec2/parse_test.go index 13e877325cad..7457b3b027bc 100644 --- a/jobspec2/parse_test.go +++ b/jobspec2/parse_test.go @@ -5,6 +5,7 @@ import ( "os" "strings" "testing" + "time" "github.com/hashicorp/nomad/api" "github.com/hashicorp/nomad/jobspec" @@ -950,3 +951,22 @@ func TestParseServiceCheck(t *testing.T) { require.Equal(t, expectedJob, parsedJob) } + +func TestWaitConfig(t *testing.T) { + hclBytes, err := os.ReadFile("test-fixtures/template-wait-config.hcl") + require.NoError(t, err) + + job, err := ParseWithConfig(&ParseConfig{ + Path: "test-fixtures/template-wait-config.hcl", + Body: hclBytes, + AllowFS: false, + }) + + require.NoError(t, err) + + tmpl := job.TaskGroups[0].Tasks[0].Templates[0] + require.NotNil(t, tmpl) + require.NotNil(t, tmpl.Wait) + require.Equal(t, 5*time.Second, *tmpl.Wait.Min) + require.Equal(t, 60*time.Second, *tmpl.Wait.Max) +} diff --git a/jobspec2/test-fixtures/template-wait-config.hcl b/jobspec2/test-fixtures/template-wait-config.hcl new file mode 100644 index 000000000000..e410248fbee7 --- /dev/null +++ b/jobspec2/test-fixtures/template-wait-config.hcl @@ -0,0 +1,12 @@ +job "example" { + group "group" { + task "task" { + template { + wait { + min = "5s" + max = "60s" + } + } + } + } +} diff --git a/nomad/structs/diff.go b/nomad/structs/diff.go index fe8a0013540d..91985a2372d7 100644 --- a/nomad/structs/diff.go +++ b/nomad/structs/diff.go @@ -525,12 +525,7 @@ func (t *Task) Diff(other *Task, contextual bool) (*TaskDiff, error) { } // Template diff - tmplDiffs := primitiveObjectSetDiff( - interfaceSlice(t.Templates), - interfaceSlice(other.Templates), - nil, - "Template", - contextual) + tmplDiffs := templateDiffs(t.Templates, other.Templates, contextual) if tmplDiffs != nil { diff.Objects = append(diff.Objects, tmplDiffs...) } @@ -1607,6 +1602,145 @@ func vaultDiff(old, new *Vault, contextual bool) *ObjectDiff { return diff } +// waitConfigDiff returns the diff of two WaitConfig objects. If contextual diff is +// enabled, all fields will be returned, even if no diff occurred. +func waitConfigDiff(old, new *WaitConfig, contextual bool) *ObjectDiff { + diff := &ObjectDiff{Type: DiffTypeNone, Name: "Template"} + var oldPrimitiveFlat, newPrimitiveFlat map[string]string + + if reflect.DeepEqual(old, new) { + return nil + } else if old == nil { + diff.Type = DiffTypeAdded + newPrimitiveFlat = flatmap.Flatten(new, nil, false) + } else if new == nil { + diff.Type = DiffTypeDeleted + oldPrimitiveFlat = flatmap.Flatten(old, nil, false) + } else { + diff.Type = DiffTypeEdited + oldPrimitiveFlat = flatmap.Flatten(old, nil, false) + newPrimitiveFlat = flatmap.Flatten(new, nil, false) + } + + // Diff the primitive fields. + diff.Fields = fieldDiffs(oldPrimitiveFlat, newPrimitiveFlat, contextual) + + return diff +} + +// templateDiff returns the diff of two Consul Template objects. If contextual diff is +// enabled, all fields will be returned, even if no diff occurred. +func templateDiff(old, new *Template, contextual bool) *ObjectDiff { + diff := &ObjectDiff{Type: DiffTypeNone, Name: "Template"} + var oldPrimitiveFlat, newPrimitiveFlat map[string]string + + if reflect.DeepEqual(old, new) { + return nil + } else if old == nil { + old = &Template{} + diff.Type = DiffTypeAdded + newPrimitiveFlat = flatmap.Flatten(new, nil, true) + } else if new == nil { + new = &Template{} + diff.Type = DiffTypeDeleted + oldPrimitiveFlat = flatmap.Flatten(old, nil, true) + } else { + diff.Type = DiffTypeEdited + oldPrimitiveFlat = flatmap.Flatten(old, nil, true) + newPrimitiveFlat = flatmap.Flatten(new, nil, true) + } + + // Diff the primitive fields. + diff.Fields = fieldDiffs(oldPrimitiveFlat, newPrimitiveFlat, contextual) + + // WaitConfig diffs + if waitDiffs := waitConfigDiff(old.Wait, new.Wait, contextual); waitDiffs != nil { + diff.Objects = append(diff.Objects, waitDiffs) + } + + return diff +} + +// templateDiffs returns the diff of two Consul Template slices. If contextual diff is +// enabled, all fields will be returned, even if no diff occurred. +// serviceDiffs diffs a set of services. If contextual diff is enabled, unchanged +// fields within objects nested in the tasks will be returned. +func templateDiffs(old, new []*Template, contextual bool) []*ObjectDiff { + // Handle trivial case. + if len(old) == 1 && len(new) == 1 { + if diff := templateDiff(old[0], new[0], contextual); diff != nil { + return []*ObjectDiff{diff} + } + return nil + } + + // For each template we will try to find a corresponding match in the other list. + // The following lists store the index of the matching template for each + // position of the inputs. + oldMatches := make([]int, len(old)) + newMatches := make([]int, len(new)) + + // Initialize all templates as unmatched. + for i := range oldMatches { + oldMatches[i] = -1 + } + for i := range newMatches { + newMatches[i] = -1 + } + + // Find a match in the new templates list for each old template and compute + // their diffs. + var diffs []*ObjectDiff + for oldIndex, oldTemplate := range old { + newIndex := findTemplateMatch(oldTemplate, new, newMatches) + + // Old templates that don't have a match were deleted. + if newIndex < 0 { + diff := templateDiff(oldTemplate, nil, contextual) + diffs = append(diffs, diff) + continue + } + + // If A matches B then B matches A. + oldMatches[oldIndex] = newIndex + newMatches[newIndex] = oldIndex + + newTemplate := new[newIndex] + if diff := templateDiff(oldTemplate, newTemplate, contextual); diff != nil { + diffs = append(diffs, diff) + } + } + + // New templates without match were added. + for i, m := range newMatches { + if m == -1 { + diff := templateDiff(nil, new[i], contextual) + diffs = append(diffs, diff) + } + } + + sort.Sort(ObjectDiffs(diffs)) + return diffs +} + +func findTemplateMatch(template *Template, newTemplates []*Template, newTemplateMatches []int) int { + indexMatch := -1 + + for i, newTemplate := range newTemplates { + // Skip template if it's already matched. + if newTemplateMatches[i] >= 0 { + continue + } + + if template.DiffID() == newTemplate.DiffID() { + indexMatch = i + break + } + } + + return indexMatch +} + // parameterizedJobDiff returns the diff of two parameterized job objects. If // contextual diff is enabled, all fields will be returned, even if no diff // occurred. diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index bfb2f960b22f..ef950b90d421 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -6908,6 +6908,10 @@ func TestTaskDiff(t *testing.T) { ChangeSignal: "SIGHUP", Splay: 1, Perms: "0644", + Wait: &WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(5 * time.Second), + }, }, { SourcePath: "foo2", @@ -6931,6 +6935,10 @@ func TestTaskDiff(t *testing.T) { ChangeSignal: "SIGHUP", Splay: 1, Perms: "0644", + Wait: &WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(10 * time.Second), + }, }, { SourcePath: "foo3", @@ -6940,6 +6948,10 @@ func TestTaskDiff(t *testing.T) { ChangeSignal: "SIGHUP3", Splay: 3, Perms: "0776", + Wait: &WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(10 * time.Second), + }, }, }, }, @@ -6957,6 +6969,20 @@ func TestTaskDiff(t *testing.T) { New: "baz new", }, }, + Objects: []*ObjectDiff{ + { + Type: DiffTypeEdited, + Name: "Template", + Fields: []*FieldDiff{ + { + Type: DiffTypeEdited, + Name: "Max", + Old: "5000000000", + New: "10000000000", + }, + }, + }, + }, }, { Type: DiffTypeAdded, @@ -7017,6 +7043,26 @@ func TestTaskDiff(t *testing.T) { New: "0", }, }, + Objects: []*ObjectDiff{ + { + Type: DiffTypeAdded, + Name: "Template", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "Max", + Old: "", + New: "10000000000", + }, + { + Type: DiffTypeAdded, + Name: "Min", + Old: "", + New: "5000000000", + }, + }, + }, + }, }, { Type: DiffTypeDeleted, @@ -7197,10 +7243,8 @@ func TestTaskDiff(t *testing.T) { }, } - for i, c := range cases { + for _, c := range cases { t.Run(c.Name, func(t *testing.T) { - t.Logf("running case: %d %v", i, c.Name) - actual, err := c.Old.Diff(c.New, c.Contextual) if c.Error { require.Error(t, err) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index f19656fce5d0..da87aacb75e5 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -25,6 +25,8 @@ import ( "strings" "time" + "golang.org/x/crypto/blake2b" + "github.com/hashicorp/cronexpr" "github.com/hashicorp/go-msgpack/codec" "github.com/hashicorp/go-multierror" @@ -41,7 +43,6 @@ import ( psstructs "github.com/hashicorp/nomad/plugins/shared/structs" "github.com/miekg/dns" "github.com/mitchellh/copystructure" - "golang.org/x/crypto/blake2b" ) var ( @@ -7463,6 +7464,9 @@ type Template struct { // acquired. // COMPAT(0.12) VaultGrace has been ignored by Vault since Vault v0.5. VaultGrace time.Duration + + // WaitConfig is used to override the global WaitConfig on a per-template basis + Wait *WaitConfig } // DefaultTemplate returns a default template. @@ -7478,9 +7482,14 @@ func (t *Template) Copy() *Template { if t == nil { return nil } - copy := new(Template) - *copy = *t - return copy + nt := new(Template) + *nt = *t + + if t.Wait != nil { + nt.Wait = t.Wait.Copy() + } + + return nt } func (t *Template) Canonicalize() { @@ -7536,6 +7545,10 @@ func (t *Template) Validate() error { } } + if err = t.Wait.Validate(); err != nil { + _ = multierror.Append(&mErr, err) + } + return mErr.ErrorOrNil() } @@ -7555,6 +7568,71 @@ func (t *Template) DiffID() string { return t.DestPath } +// WaitConfig is the Min/Max duration used by the Consul Template Watcher. Consul +// Template relies on pointer based business logic. This struct uses pointers so +// that we tell the different between zero values and unset values. +type WaitConfig struct { + Min *time.Duration + Max *time.Duration +} + +// Copy returns a deep copy of this configuration. +func (wc *WaitConfig) Copy() *WaitConfig { + if wc == nil { + return nil + } + + nwc := new(WaitConfig) + + if wc.Min != nil { + nwc.Min = &*wc.Min + } + + if wc.Max != nil { + nwc.Max = &*wc.Max + } + + return nwc +} + +func (wc *WaitConfig) Equals(o *WaitConfig) bool { + if wc.Min == nil && o.Min != nil { + return false + } + + if wc.Max == nil && o.Max != nil { + return false + } + + if wc.Min != nil && (o.Min == nil || *wc.Min != *o.Min) { + return false + } + + if wc.Max != nil && (o.Max == nil || *wc.Max != *o.Max) { + return false + } + + return true +} + +// Validate that the min is not greater than the max +func (wc *WaitConfig) Validate() error { + if wc == nil { + return nil + } + + // If either one is nil, they aren't comparable, so they can't be invalid. + if wc.Min == nil || wc.Max == nil { + return nil + } + + if *wc.Min > *wc.Max { + return fmt.Errorf("wait min %s is greater than max %s", wc.Min, wc.Max) + } + + return nil +} + // AllocState records a single event that changes the state of the whole allocation type AllocStateField uint8 diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index bc57916333a6..aa3e735dd663 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/consul/api" "github.com/hashicorp/go-multierror" + "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/helper/uuid" "github.com/kr/pretty" @@ -2551,6 +2552,45 @@ func TestTemplate_Validate(t *testing.T) { "as octal", }, }, + { + Tmpl: &Template{ + SourcePath: "foo", + DestPath: "local/foo", + ChangeMode: "noop", + Wait: &WaitConfig{ + Min: helper.TimeToPtr(10 * time.Second), + Max: helper.TimeToPtr(5 * time.Second), + }, + }, + Fail: true, + ContainsErrs: []string{ + "greater than", + }, + }, + { + Tmpl: &Template{ + SourcePath: "foo", + DestPath: "local/foo", + ChangeMode: "noop", + Wait: &WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(5 * time.Second), + }, + }, + Fail: false, + }, + { + Tmpl: &Template{ + SourcePath: "foo", + DestPath: "local/foo", + ChangeMode: "noop", + Wait: &WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(10 * time.Second), + }, + }, + Fail: false, + }, } for i, c := range cases { @@ -2572,6 +2612,55 @@ func TestTemplate_Validate(t *testing.T) { } } +func TestTaskWaitConfig_Equals(t *testing.T) { + testCases := []struct { + name string + config *WaitConfig + expected *WaitConfig + }{ + { + name: "all-fields", + config: &WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(10 * time.Second), + }, + expected: &WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(10 * time.Second), + }, + }, + { + name: "no-fields", + config: &WaitConfig{}, + expected: &WaitConfig{}, + }, + { + name: "min-only", + config: &WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + }, + expected: &WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + }, + }, + { + name: "max-only", + config: &WaitConfig{ + Max: helper.TimeToPtr(10 * time.Second), + }, + expected: &WaitConfig{ + Max: helper.TimeToPtr(10 * time.Second), + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + require.True(t, tc.config.Equals(tc.expected)) + }) + } +} + func TestConstraint_Validate(t *testing.T) { c := &Constraint{} err := c.Validate() diff --git a/scheduler/util_test.go b/scheduler/util_test.go index 7064c50ae671..1b5ec8d9a30a 100644 --- a/scheduler/util_test.go +++ b/scheduler/util_test.go @@ -4,6 +4,7 @@ import ( "fmt" "reflect" "testing" + "time" "github.com/stretchr/testify/require" @@ -753,6 +754,29 @@ func TestTasksUpdated(t *testing.T) { j21.TaskGroups[0].Tasks[0].Resources.Cores = 4 require.True(t, tasksUpdated(j20, j21, name)) + // Compare identical Template wait configs + j22 := mock.Job() + j22.TaskGroups[0].Tasks[0].Templates = []*structs.Template{ + { + Wait: &structs.WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(5 * time.Second), + }, + }, + } + j23 := mock.Job() + j23.TaskGroups[0].Tasks[0].Templates = []*structs.Template{ + { + Wait: &structs.WaitConfig{ + Min: helper.TimeToPtr(5 * time.Second), + Max: helper.TimeToPtr(5 * time.Second), + }, + }, + } + require.False(t, tasksUpdated(j22, j23, name)) + // Compare changed Template wait configs + j23.TaskGroups[0].Tasks[0].Templates[0].Wait.Max = helper.TimeToPtr(10 * time.Second) + require.True(t, tasksUpdated(j22, j23, name)) } func TestTasksUpdated_connectServiceUpdated(t *testing.T) { diff --git a/website/content/docs/configuration/client.mdx b/website/content/docs/configuration/client.mdx index f1a9ecc43cfb..22e9c2261447 100644 --- a/website/content/docs/configuration/client.mdx +++ b/website/content/docs/configuration/client.mdx @@ -352,9 +352,97 @@ see the [drivers documentation](/docs/drivers). the host as root (unless Nomad is configured to run as a non-root user). - `disable_file_sandbox` `(bool: false)` - Allows templates access to arbitrary - files on the client host via the `file` function. By default templates can + files on the client host via the `file` function. By default, templates can access files only within the [task working directory]. +- `max_stale` `(string: "")` - # This is the maximum interval to allow "stale" + data. By default, only the Consul leader will respond to queries. Requests to + a follower will forward to the leader. In large clusters with many requests, + this is not as scalable. This option allows any follower to respond to a query, + so long as the last-replicated data is within this bound. Higher values result + in less cluster load, but are more likely to have outdated data. + +- `wait` `(Code: nil)` - Defines the minimum and maximum amount of time to wait + for the Consul cluster to reach a consistent state before rendering a template. + This is useful to enable in systems where network connectivity to Consul is degraded, + because it will reduce the number of times a template is rendered. This configuration is + also exposed in the _task template stanza_ to allow overrides per task. + + ```hcl + wait { + min = "5s" + max = "10s" + } + ``` + +- `wait_bounds` `(Code: nil)` - Defines client level lower and upper bounds for + per-template `wait` configuration. If the individual template configuration has + a `min` lower than `wait_bounds.min` or a `max` greater than the `wait_bounds.max`, + the bounds will be enforced, and the template `wait` will be adjusted before being + sent to `consul-template`. + + ```hcl + wait_bounds { + min = "5s" + max = "10s" + } + ``` + +- `block_query_wait` `(string: "60s")` - This is amount of time in seconds to wait + for the results of a blocking query. Many endpoints in Consul support a feature known as + "blocking queries". A blocking query is used to wait for a potential change + using long polling. + +- `consul_retry` `(Code: nil)` - This controls the retry behavior when an error is + returned from Consul. 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 + consul_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 = 12 + # 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" + } + ``` + +- `vault_retry` `(Code: nil)` - This controls the retry behavior when an error is + returned from Vault. 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 + vault_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 = 12 + # 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. diff --git a/website/content/docs/job-specification/template.mdx b/website/content/docs/job-specification/template.mdx index d47e6bbf52bb..7176a4786039 100644 --- a/website/content/docs/job-specification/template.mdx +++ b/website/content/docs/job-specification/template.mdx @@ -92,7 +92,7 @@ refer to the [Learn Go Template Syntax][gt_learn] Learn guide. One of `source` or `data` must be specified, but not both. This source can optionally be fetched using an [`artifact`][artifact] resource. This template must exist on the machine prior to starting the task; it is not possible to - reference a template inside of a Docker container, for example. + reference a template inside a Docker container, for example. - `splay` `(string: "5s")` - Specifies a random amount of time to wait between 0 ms and the given splay value before invoking the change mode. This is @@ -100,6 +100,22 @@ refer to the [Learn Go Template Syntax][gt_learn] Learn guide. prevent a thundering herd problem where all task instances restart at the same time. +- `wait` `(Code: nil)` - Defines the minimum and maximum amount of time to wait + for the Consul cluster to reach a consistent state before rendering a template. + This is useful to enable in systems where network connectivity to Consul is degraded, + because it will reduce the number of times a template is rendered. This setting + can be overridden by the [`client.template.wait_bounds`]. If the template + configuration has a `min` lower than `client.template.wait_bounds.min` or a `max` + greater than `client.template.wait_bounds.max`, the client's bounds will be enforced, + and the template `wait` will be adjusted before being sent to `consul-template`. + + ```hcl + wait { + min = "5s" + max = "10s" + } + ``` + - `vault_grace` `(string: "15s")` - [Deprecated](https://github.com/hashicorp/consul-template/issues/1268) ## `template` Examples @@ -483,3 +499,4 @@ options](/docs/configuration/client#options): [go-envparse]: https://github.com/hashicorp/go-envparse#readme 'The go-envparse Readme' [task working directory]: /docs/runtime/environment#task-directories 'Task Directories' [filesystem internals]: /docs/internals/filesystem#templates-artifacts-and-dispatch-payloads +[`client.template.wait_bounds`]: /doc/configuration/client#wait_bounds