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

Handle multiple environment templates #3529

Merged
merged 3 commits into from
Nov 10, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ BUG FIXES:
updates [GH-3445]
* core: Fixes an issue with jobs that have `auto_revert` set to true, where reverting
to a previously stable job that fails to start up causes an infinite cycle of reverts [GH-3496]
* template: Fix issue where multiple environment variable templates would be
parsed incorrectly when contents of one have changed after the initial
rendering [GH-3529]
* sentinel: (Nomad Enterprise) Fix an issue that could cause an import error
when multiple Sentinel policies are applied

Expand Down
4 changes: 2 additions & 2 deletions client/consul_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ func (tm *TaskTemplateManager) run() {
tm.config.Hooks.Kill(consulTemplateSourceName, err.Error(), true)
return
}
tm.config.EnvBuilder.SetTemplateEnv(envMap)
tm.config.EnvBuilder.MergeTemplateEnv(envMap)

// Unblock the task
tm.config.Hooks.UnblockStart(consulTemplateSourceName)
Expand Down Expand Up @@ -394,7 +394,7 @@ func (tm *TaskTemplateManager) handleTemplateRerenders(allRenderedTime time.Time
tm.config.Hooks.Kill(consulTemplateSourceName, err.Error(), true)
return
}
tm.config.EnvBuilder.SetTemplateEnv(envMap)
tm.config.EnvBuilder.MergeTemplateEnv(envMap)
Copy link
Member

Choose a reason for hiding this comment

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

This is only additive so env vars will never be removed.

I'm unclear why we don't reparse all env templates on any change and simply set the full map each time? That was my intended behavior, and from looking at the code I'm not sure why it doesn't work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah good point. At first I wanted to avoid reparsing everything but you are right, certain env's may be conditional. Will fix


for _, tmpl := range tmpls {
switch tmpl.ChangeMode {
Expand Down
80 changes: 80 additions & 0 deletions client/consul_template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1112,6 +1112,86 @@ func TestTaskTemplateManager_Env_Multi(t *testing.T) {
}
}

func TestTaskTemplateManager_Rerender_Env(t *testing.T) {
t.Parallel()
// Make a template that renders based on a key in Consul and sends restart
key1 := "bam"
key2 := "bar"
content1_1 := "cat"
content1_2 := "dog"
t1 := &structs.Template{
EmbeddedTmpl: `
FOO={{key "bam"}}
`,
DestPath: "test.env",
ChangeMode: structs.TemplateChangeModeRestart,
Envvars: true,
}
t2 := &structs.Template{
EmbeddedTmpl: `
BAR={{key "bar"}}
`,
DestPath: "test2.env",
ChangeMode: structs.TemplateChangeModeRestart,
Envvars: true,
}

harness := newTestHarness(t, []*structs.Template{t1, t2}, true, false)
harness.start(t)
defer harness.stop()

// Ensure no unblock
select {
case <-harness.mockHooks.UnblockCh:
t.Fatalf("Task unblock should have not have been called")
case <-time.After(time.Duration(1*testutil.TestMultiplier()) * time.Second):
}

// Write the key to Consul
harness.consul.SetKV(t, key1, []byte(content1_1))
harness.consul.SetKV(t, key2, []byte(content1_1))

// Wait for the unblock
select {
case <-harness.mockHooks.UnblockCh:
case <-time.After(time.Duration(5*testutil.TestMultiplier()) * time.Second):
t.Fatalf("Task unblock should have been called")
}

env := harness.envBuilder.Build().Map()
if v, ok := env["FOO"]; !ok || v != content1_1 {
t.Fatalf("Bad env for FOO: %v %v", v, ok)
}
if v, ok := env["BAR"]; !ok || v != content1_1 {
t.Fatalf("Bad env for BAR: %v %v", v, ok)
}

// Update the keys in Consul
harness.consul.SetKV(t, key1, []byte(content1_2))

// Wait for restart
timeout := time.After(time.Duration(1*testutil.TestMultiplier()) * time.Second)
OUTER:
for {
select {
case <-harness.mockHooks.RestartCh:
break OUTER
case <-harness.mockHooks.SignalCh:
t.Fatalf("Signal with restart policy: %+v", harness.mockHooks)
case <-timeout:
t.Fatalf("Should have received a restart: %+v", harness.mockHooks)
}
}

env = harness.envBuilder.Build().Map()
if v, ok := env["FOO"]; !ok || v != content1_2 {
t.Fatalf("Bad env for FOO: %v %v", v, ok)
}
if v, ok := env["BAR"]; !ok || v != content1_1 {
t.Fatalf("Bad env for BAR: %v %v", v, ok)
}
}

// TestTaskTemplateManager_Config_ServerName asserts the tls_server_name
// setting is propogated to consul-template's configuration. See #2776
func TestTaskTemplateManager_Config_ServerName(t *testing.T) {
Expand Down
12 changes: 10 additions & 2 deletions client/driver/env/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -532,9 +532,17 @@ func (b *Builder) SetHostEnvvars(filter []string) *Builder {
return b
}

func (b *Builder) SetTemplateEnv(m map[string]string) *Builder {
// MergeTemplateEnv is used to merge the passed environment variables with
// existing environment variables set from a template.
func (b *Builder) MergeTemplateEnv(m map[string]string) *Builder {
b.mu.Lock()
b.templateEnv = m
if b.templateEnv == nil {
b.templateEnv = m
} else {
for k, v := range m {
b.templateEnv[k] = v
}
}
b.mu.Unlock()
return b
}
Expand Down