Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

agent: restart template runner on retry for unlimited retries #11775

Merged
merged 26 commits into from Jun 21, 2021
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
2609f7c
agent: restart template runner on retry for unlimited retries
calvn Jun 4, 2021
a521f1d
template: log error message early
calvn Jun 4, 2021
83e2d94
template: delegate retries back to template if param is set to true
calvn Jun 4, 2021
20af485
agent: add and use the new template config stanza
calvn Jun 9, 2021
4c23bad
agent: fix panic, fix existing tests
calvn Jun 10, 2021
b346dd8
changelog: add changelog entry
calvn Jun 10, 2021
89fa90d
agent: add tests for exit_on_retry_failure
calvn Jun 11, 2021
0b834db
Merge remote-tracking branch 'origin/master' into agent-template-retr…
calvn Jun 11, 2021
518ed86
agent: properly check on agent exit cases, add separate tests for mis…
calvn Jun 11, 2021
27df22b
agent: add note on difference between missing key vs missing secret
calvn Jun 11, 2021
300dc8c
docs: add docs for template_config
calvn Jun 12, 2021
3cf2e48
Update website/content/docs/agent/template-config.mdx
calvn Jun 14, 2021
310f564
Update website/content/docs/agent/template-config.mdx
calvn Jun 14, 2021
d7b42ac
Update website/content/docs/agent/template-config.mdx
calvn Jun 16, 2021
4f673d8
Update website/content/docs/agent/template-config.mdx
calvn Jun 16, 2021
ddccb43
Update website/content/docs/agent/template-config.mdx
calvn Jun 16, 2021
b7ecf45
docs: fix exit_on_retry_failure, fix Functionality section
calvn Jun 16, 2021
3657ea6
docs: update interaction title
calvn Jun 17, 2021
ffe15f5
template: add internal note on behavior for persist case
calvn Jun 17, 2021
1440475
docs: update agent, template, and template-config docs
calvn Jun 18, 2021
9a73bb5
docs: update agent docs on retry stanza
calvn Jun 18, 2021
69b3204
Apply suggestions from code review
calvn Jun 18, 2021
5d41bee
Update changelog/11775.txt
calvn Jun 18, 2021
8e6be32
agent/test: rename expectExit to expectExitFromError
calvn Jun 18, 2021
b3dea46
agent/test: add check on early exits on the happy path
calvn Jun 19, 2021
08be22d
Update website/content/docs/agent/template-config.mdx
calvn Jun 21, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions changelog/11775.txt
@@ -0,0 +1,9 @@
```release-note:change
agent: The template engine errors will no longer cause agent to exit unless
calvn marked this conversation as resolved.
Show resolved Hide resolved
explicitly defined to do so. A new configuration parameter,
`exit_on_retry_failure`, within the new top-level stanza, `template_config`, can
be set to `true` in order to cause agent to exit. Note that for agent to exit if
`template.error_on_missing_key` is set to `true`, `exit_on_retry_failure` must
be also set to `true`. Otherwise, the template engine will log an error but then
restart its internal runner.
```
4 changes: 4 additions & 0 deletions command/agent/cache/api_proxy.go
Expand Up @@ -119,6 +119,10 @@ func (ap *APIProxy) Send(ctx context.Context, req *SendRequest) (*SendResponse,
}
client.SetToken(req.Token)

// Derive and set a logger for the client
clientLogger := ap.logger.Named("client")
client.SetLogger(clientLogger)

// http.Transport will transparently request gzip and decompress the response, but only if
// the client doesn't manually set the header. Removing any Accept-Encoding header allows the
// transparent compression to occur.
Expand Down
45 changes: 40 additions & 5 deletions command/agent/config/config.go
Expand Up @@ -22,11 +22,12 @@ import (
type Config struct {
*configutil.SharedConfig `hcl:"-"`

AutoAuth *AutoAuth `hcl:"auto_auth"`
ExitAfterAuth bool `hcl:"exit_after_auth"`
Cache *Cache `hcl:"cache"`
Vault *Vault `hcl:"vault"`
Templates []*ctconfig.TemplateConfig `hcl:"templates"`
AutoAuth *AutoAuth `hcl:"auto_auth"`
ExitAfterAuth bool `hcl:"exit_after_auth"`
Cache *Cache `hcl:"cache"`
Vault *Vault `hcl:"vault"`
TemplateConfig *TemplateConfig `hcl:"template_config"`
Templates []*ctconfig.TemplateConfig `hcl:"templates"`
}

func (c *Config) Prune() {
Expand Down Expand Up @@ -114,6 +115,11 @@ type Sink struct {
Config map[string]interface{}
}

// TemplateConfig defines global behaviors around template
type TemplateConfig struct {
ExitOnRetryFailure bool `hcl:"exit_on_retry_failure"`
}

func NewConfig() *Config {
return &Config{
SharedConfig: new(configutil.SharedConfig),
Expand Down Expand Up @@ -177,6 +183,10 @@ func LoadConfig(path string) (*Config, error) {
return nil, fmt.Errorf("error parsing 'cache':%w", err)
}

if err := parseTemplateConfig(result, list); err != nil {
return nil, fmt.Errorf("error parsing 'template_config': %w", err)
}

if err := parseTemplates(result, list); err != nil {
return nil, fmt.Errorf("error parsing 'template': %w", err)
}
Expand Down Expand Up @@ -551,6 +561,31 @@ func parseSinks(result *Config, list *ast.ObjectList) error {
return nil
}

func parseTemplateConfig(result *Config, list *ast.ObjectList) error {
name := "template_config"

templateConfigList := list.Filter(name)
if len(templateConfigList.Items) == 0 {
return nil
}

if len(templateConfigList.Items) > 1 {
return fmt.Errorf("at most one %q block is allowed", name)
}

// Get our item
item := templateConfigList.Items[0]

var cfg TemplateConfig
if err := hcl.DecodeObject(&cfg, item.Val); err != nil {
return err
}

result.TemplateConfig = &cfg

return nil
}

func parseTemplates(result *Config, list *ast.ObjectList) error {
name := "template"

Expand Down
53 changes: 53 additions & 0 deletions command/agent/config/config_test.go
Expand Up @@ -535,6 +535,59 @@ func TestLoadConfigFile_AgentCache_PersistMissingType(t *testing.T) {
}
}

func TestLoadConfigFile_TemplateConfig(t *testing.T) {

testCases := map[string]struct {
fixturePath string
expectedTemplateConfig TemplateConfig
}{
"set-true": {
"./test-fixtures/config-template_config.hcl",
TemplateConfig{
ExitOnRetryFailure: true,
},
},
"empty": {
"./test-fixtures/config-template_config-empty.hcl",
TemplateConfig{
ExitOnRetryFailure: false,
},
},
}

for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
config, err := LoadConfig(tc.fixturePath)
if err != nil {
t.Fatal(err)
}

expected := &Config{
SharedConfig: &configutil.SharedConfig{},
Vault: &Vault{
Address: "http://127.0.0.1:1111",
Retry: &Retry{
NumRetries: 5,
},
},
TemplateConfig: &tc.expectedTemplateConfig,
Templates: []*ctconfig.TemplateConfig{
{
Source: pointerutil.StringPtr("/path/on/disk/to/template.ctmpl"),
Destination: pointerutil.StringPtr("/path/on/disk/where/template/will/render.txt"),
},
},
}

config.Prune()
if diff := deep.Equal(config, expected); diff != nil {
t.Fatal(diff)
}
})
}

}

// TestLoadConfigFile_Template tests template definitions in Vault Agent
func TestLoadConfigFile_Template(t *testing.T) {
testCases := map[string]struct {
Expand Down
@@ -0,0 +1,13 @@
vault {
address = "http://127.0.0.1:1111"
retry {
num_retries = 5
}
}

template_config {}
jasonodonnell marked this conversation as resolved.
Show resolved Hide resolved

template {
source = "/path/on/disk/to/template.ctmpl"
destination = "/path/on/disk/where/template/will/render.txt"
}
15 changes: 15 additions & 0 deletions command/agent/config/test-fixtures/config-template_config.hcl
@@ -0,0 +1,15 @@
vault {
address = "http://127.0.0.1:1111"
retry {
num_retries = 5
}
}

template_config {
exit_on_retry_failure = true
}

template {
source = "/path/on/disk/to/template.ctmpl"
destination = "/path/on/disk/where/template/will/render.txt"
}
28 changes: 27 additions & 1 deletion command/agent/template/template.go
Expand Up @@ -172,8 +172,20 @@ func (ts *Server) Run(ctx context.Context, incoming chan string, templates []*ct
}

case err := <-ts.runner.ErrCh:
ts.logger.Error("template server error", "error", err.Error())
ts.runner.StopImmediately()
return fmt.Errorf("template server: %w", err)

// Return after stopping the runner if exit on retry failure was
// specified
if ts.config.AgentConfig.TemplateConfig != nil && ts.config.AgentConfig.TemplateConfig.ExitOnRetryFailure {
return fmt.Errorf("template server: %w", err)
}

ts.runner, err = manager.NewRunner(runnerConfig, false)
if err != nil {
return fmt.Errorf("template server failed to create: %w", err)
}
go ts.runner.Start()

case <-ts.runner.TemplateRenderedCh():
// A template has been rendered, figure out what to do
Expand Down Expand Up @@ -253,6 +265,20 @@ func newRunnerConfig(sc *ServerConfig, templates ctconfig.TemplateConfigs) (*ctc
// that need to be fixed for 1.7x/1.8
if sc.AgentConfig.Cache != nil && sc.AgentConfig.Cache.Persist != nil && len(sc.AgentConfig.Listeners) != 0 {
attempts = 0

// If we don't want exit on template retry failure (i.e. unlimited
// retries), let consul-template handle retry and backoff logic.
//
// Note: This is a fixed value (12) that ends up being a multiplier to
// retry.num_retires (i.e. 12 * N total retries per runner restart).
// Since we are performing retries indefinitely this base number helps
// prevent agent from spamming Vault if retry.num_retries is set to a
// low value by forcing exponential backoff to be high towards the end
// of retries during the process.
if sc.AgentConfig.TemplateConfig != nil && !sc.AgentConfig.TemplateConfig.ExitOnRetryFailure {
attempts = ctconfig.DefaultRetryAttempts
}
Comment on lines +278 to +280
Copy link
Member Author

@calvn calvn Jun 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic over here is what allows agent to perform a reasonable number of retries with enough backoff pause towards the tail end of the runner's current run before restarting. I've chosen ctconfig.DefaultRetryAttempts as the value since that was always the case on Agent 1.5.x and earlier. We can't completely hand off retries to the cache's proxy because the API client only performs retries on 5xx errors whereas the builtin retry logic from consul-template performs retries on any errors. A non-existent secret (i.e. a 404) without retry attempts from consul-template itself will spam apiproxy, and in turn the server, with requests indefinitely which can easily DDoS Vault.

The alternative here could be to set attempts = sc.AgentConfig.Vault.Retry.NumRetries but it has the danger of spamming the server with requests if the value is too low or zero due to low initial backoff values.


scheme := "unix://"
if sc.AgentConfig.Listeners[0].Type == "tcp" {
scheme = "https://"
Expand Down
23 changes: 16 additions & 7 deletions command/agent/template/template_test.go
Expand Up @@ -352,8 +352,9 @@ func TestServerRun(t *testing.T) {
}

testCases := map[string]struct {
templateMap map[string]*templateTest
expectError bool
templateMap map[string]*templateTest
expectError bool
exitOnRetryFailure bool
}{
"simple": {
templateMap: map[string]*templateTest{
Expand All @@ -363,7 +364,8 @@ func TestServerRun(t *testing.T) {
},
},
},
expectError: false,
expectError: false,
exitOnRetryFailure: false,
},
"multiple": {
templateMap: map[string]*templateTest{
Expand Down Expand Up @@ -403,7 +405,8 @@ func TestServerRun(t *testing.T) {
},
},
},
expectError: false,
expectError: false,
exitOnRetryFailure: false,
},
"bad secret": {
templateMap: map[string]*templateTest{
Expand All @@ -413,7 +416,8 @@ func TestServerRun(t *testing.T) {
},
},
},
expectError: true,
expectError: true,
exitOnRetryFailure: true,
},
"missing key": {
templateMap: map[string]*templateTest{
Expand All @@ -424,7 +428,8 @@ func TestServerRun(t *testing.T) {
},
},
},
expectError: true,
expectError: true,
exitOnRetryFailure: true,
},
"permission denied": {
templateMap: map[string]*templateTest{
Expand All @@ -434,7 +439,8 @@ func TestServerRun(t *testing.T) {
},
},
},
expectError: true,
expectError: true,
exitOnRetryFailure: true,
},
}

Expand All @@ -458,6 +464,9 @@ func TestServerRun(t *testing.T) {
NumRetries: 3,
},
},
TemplateConfig: &config.TemplateConfig{
ExitOnRetryFailure: tc.exitOnRetryFailure,
},
},
LogLevel: hclog.Trace,
LogWriter: hclog.DefaultOutput,
Expand Down