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

Add envconsul-like support and refactor environment handling #2654

Merged
merged 30 commits into from
May 30, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
7dac668
Functional consul template env file support
schmichael May 13, 2017
ace0098
Refactor TaskEnvironment into Builder and TaskEnv
schmichael May 17, 2017
3b24980
Move path building to task dir initialization
schmichael May 18, 2017
96753dc
Improve PortMap handling and simplify Builder creation
schmichael May 18, 2017
37c1cbc
Move env template handling into consul_template.go
schmichael May 18, 2017
6db3501
Add PortMap to struct returned by Driver.Prestart
schmichael May 19, 2017
3743d34
Fix env var interpolation and env tests
schmichael May 19, 2017
a96fb5d
Move task env into execcontext
schmichael May 23, 2017
1295f88
Handle Driver.Prestart returning nil, nil
schmichael May 23, 2017
59b3cca
Fix test data
schmichael May 23, 2017
72a24ae
Add env.Builder.UpdateTask for alloc updates
schmichael May 23, 2017
1188924
Shrink chroot to avoid timing test failure
schmichael May 23, 2017
d7eccc1
Add env testing
schmichael May 23, 2017
9c66952
Add env file test
schmichael May 24, 2017
826aec4
Document env templates
schmichael May 24, 2017
0288582
Fail fast on env template failures
schmichael May 24, 2017
43cc0a0
Minor wording updates
schmichael May 24, 2017
6571fa5
Switch tests to mock_driver
schmichael May 25, 2017
8f2ea2f
Comment and correct formatting
schmichael May 25, 2017
572ca97
Fix formatting
schmichael May 25, 2017
be959a4
Fix and test multi-env-template loading
schmichael May 26, 2017
9ccb53d
Let's pretend I never committed this
schmichael May 26, 2017
fc5254d
Use custom TaskEnv to provide PATH for rkt
schmichael May 26, 2017
de4b008
Test env parsing
schmichael May 26, 2017
c4aa3c3
Always use PATH-only env for rkt commands
schmichael May 26, 2017
6d0bc98
Fix template canonicalize test
schmichael May 26, 2017
aa0e977
Fix executor tests
schmichael May 26, 2017
68db2aa
Fix getter tests
schmichael May 26, 2017
5933cac
Fix diff test
schmichael May 27, 2017
361db24
Fix Error -> Errorf
schmichael May 30, 2017
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
18 changes: 18 additions & 0 deletions api/jobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,11 @@ func TestJobs_Canonicalize(t *testing.T) {
EmbeddedTmpl: helper.StringToPtr("---"),
DestPath: helper.StringToPtr("local/file.yml"),
},
{
EmbeddedTmpl: helper.StringToPtr("FOO=bar\n"),
DestPath: helper.StringToPtr("local/file.env"),
Envvars: helper.BoolToPtr(true),
},
},
},
},
Expand Down Expand Up @@ -377,6 +382,19 @@ func TestJobs_Canonicalize(t *testing.T) {
Perms: helper.StringToPtr("0644"),
LeftDelim: helper.StringToPtr("{{"),
RightDelim: helper.StringToPtr("}}"),
Envvars: helper.BoolToPtr(false),
},
{
SourcePath: helper.StringToPtr(""),
DestPath: helper.StringToPtr("local/file.env"),
EmbeddedTmpl: helper.StringToPtr("FOO=bar\n"),
ChangeMode: helper.StringToPtr("restart"),
ChangeSignal: helper.StringToPtr(""),
Splay: helper.TimeToPtr(5 * time.Second),
Perms: helper.StringToPtr("0644"),
LeftDelim: helper.StringToPtr("{{"),
RightDelim: helper.StringToPtr("}}"),
Envvars: helper.BoolToPtr(true),
},
},
},
Expand Down
4 changes: 4 additions & 0 deletions api/tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ type Template struct {
Perms *string `mapstructure:"perms"`
LeftDelim *string `mapstructure:"left_delimiter"`
RightDelim *string `mapstructure:"right_delimiter"`
Envvars *bool `mapstructure:"env"`
}

func (tmpl *Template) Canonicalize() {
Expand Down Expand Up @@ -373,6 +374,9 @@ func (tmpl *Template) Canonicalize() {
if tmpl.RightDelim == nil {
tmpl.RightDelim = helper.StringToPtr("}}")
}
if tmpl.Envvars == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Believe there is a canonicalize test as well

tmpl.Envvars = helper.BoolToPtr(false)
}
}

type Vault struct {
Expand Down
18 changes: 8 additions & 10 deletions client/alloc_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,12 @@ func TestAllocRunner_RetryArtifact(t *testing.T) {
}

func TestAllocRunner_TerminalUpdate_Destroy(t *testing.T) {
ctestutil.ExecCompatible(t)
upd, ar := testAllocRunner(false)

// Ensure task takes some time
task := ar.alloc.Job.TaskGroups[0].Tasks[0]
task.Config["command"] = "/bin/sleep"
task.Config["args"] = []string{"10"}
task.Driver = "mock_driver"
task.Config["run_for"] = "10s"
go ar.Run()

testutil.WaitForResult(func() (bool, error) {
Expand Down Expand Up @@ -234,13 +233,12 @@ func TestAllocRunner_TerminalUpdate_Destroy(t *testing.T) {
}

func TestAllocRunner_Destroy(t *testing.T) {
ctestutil.ExecCompatible(t)
upd, ar := testAllocRunner(false)

// Ensure task takes some time
task := ar.alloc.Job.TaskGroups[0].Tasks[0]
task.Config["command"] = "/bin/sleep"
task.Config["args"] = []string{"10"}
task.Driver = "mock_driver"
task.Config["run_for"] = "10s"
go ar.Run()
start := time.Now()

Expand Down Expand Up @@ -269,7 +267,7 @@ func TestAllocRunner_Destroy(t *testing.T) {

return nil
}); err != nil {
return false, fmt.Errorf("state not destroyed")
return false, fmt.Errorf("state not destroyed: %v", err)
}

// Check the alloc directory was cleaned
Expand All @@ -290,13 +288,12 @@ func TestAllocRunner_Destroy(t *testing.T) {
}

func TestAllocRunner_Update(t *testing.T) {
ctestutil.ExecCompatible(t)
_, ar := testAllocRunner(false)

// Ensure task takes some time
task := ar.alloc.Job.TaskGroups[0].Tasks[0]
task.Config["command"] = "/bin/sleep"
task.Config["args"] = []string{"10"}
task.Driver = "mock_driver"
task.Config["run_for"] = "10s"
go ar.Run()
defer ar.Destroy()

Expand Down Expand Up @@ -612,6 +609,7 @@ func TestAllocRunner_RestoreOldState(t *testing.T) {

logger := testLogger()
conf := config.DefaultConfig()
conf.Node = mock.Node()
conf.StateDir = os.TempDir()
conf.AllocDir = os.TempDir()
tmp, err := ioutil.TempFile("", "state-db")
Expand Down
2 changes: 1 addition & 1 deletion client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@ func (c *Client) setupDrivers() error {

var avail []string
var skipped []string
driverCtx := driver.NewDriverContext("", "", c.config, c.config.Node, c.logger, nil, nil)
driverCtx := driver.NewDriverContext("", "", c.config, c.config.Node, c.logger, nil)
for name := range driver.BuiltinDrivers {
// Skip fingerprinting drivers that are not in the whitelist if it is
// enabled.
Expand Down
97 changes: 87 additions & 10 deletions client/consul_template.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package client

import (
"bufio"
"bytes"
"fmt"
"io"
"math/rand"
"os"
"path/filepath"
Expand Down Expand Up @@ -76,7 +79,7 @@ type TaskTemplateManager struct {

func NewTaskTemplateManager(hook TaskHooks, tmpls []*structs.Template,
config *config.Config, vaultToken, taskDir string,
taskEnv *env.TaskEnvironment) (*TaskTemplateManager, error) {
envBuilder *env.Builder) (*TaskTemplateManager, error) {

// Check pre-conditions
if hook == nil {
Expand All @@ -85,7 +88,7 @@ func NewTaskTemplateManager(hook TaskHooks, tmpls []*structs.Template,
return nil, fmt.Errorf("Invalid config given")
} else if taskDir == "" {
return nil, fmt.Errorf("Invalid task directory given")
} else if taskEnv == nil {
} else if envBuilder == nil {
return nil, fmt.Errorf("Invalid task environment given")
}

Expand Down Expand Up @@ -114,14 +117,14 @@ func NewTaskTemplateManager(hook TaskHooks, tmpls []*structs.Template,
}

// Build the consul-template runner
runner, lookup, err := templateRunner(tmpls, config, vaultToken, taskDir, taskEnv)
runner, lookup, err := templateRunner(tmpls, config, vaultToken, taskDir, envBuilder.Build())
if err != nil {
return nil, err
}
tm.runner = runner
tm.lookup = lookup

go tm.run()
go tm.run(envBuilder, taskDir)
return tm, nil
}

Expand All @@ -144,7 +147,7 @@ func (tm *TaskTemplateManager) Stop() {
}

// run is the long lived loop that handles errors and templates being rendered
func (tm *TaskTemplateManager) run() {
func (tm *TaskTemplateManager) run(envBuilder *env.Builder, taskDir string) {
// Runner is nil if there is no templates
if tm.runner == nil {
// Unblock the start if there is nothing to do
Expand Down Expand Up @@ -192,6 +195,14 @@ WAIT:
}
}

// Read environment variables from env templates
envMap, err := loadTemplateEnv(tm.templates, taskDir)
if err != nil {
tm.hook.Kill("consul-template", err.Error(), true)
return
}
envBuilder.SetTemplateEnv(envMap)

allRenderedTime = time.Now()
tm.hook.UnblockStart("consul-template")

Expand Down Expand Up @@ -242,6 +253,14 @@ WAIT:
return
}

// Read environment variables from templates
envMap, err := loadTemplateEnv(tmpls, taskDir)
if err != nil {
tm.hook.Kill("consul-template", err.Error(), true)
return
}
envBuilder.SetTemplateEnv(envMap)

for _, tmpl := range tmpls {
switch tmpl.ChangeMode {
case structs.TemplateChangeModeSignal:
Expand Down Expand Up @@ -317,7 +336,7 @@ func (tm *TaskTemplateManager) allTemplatesNoop() bool {
// lookup by destination to the template. If no templates are given, a nil
// template runner and lookup is returned.
func templateRunner(tmpls []*structs.Template, config *config.Config,
vaultToken, taskDir string, taskEnv *env.TaskEnvironment) (
vaultToken, taskDir string, taskEnv *env.TaskEnv) (
*manager.Runner, map[string][]*structs.Template, error) {

if len(tmpls) == 0 {
Expand Down Expand Up @@ -350,7 +369,7 @@ func templateRunner(tmpls []*structs.Template, config *config.Config,
}

// Set Nomad's environment variables
runner.Env = taskEnv.Build().EnvMapAll()
runner.Env = taskEnv.All()

// Build the lookup
idMap := runner.TemplateConfigMapping()
Expand All @@ -368,9 +387,7 @@ func templateRunner(tmpls []*structs.Template, config *config.Config,

// parseTemplateConfigs converts the tasks templates into consul-templates
func parseTemplateConfigs(tmpls []*structs.Template, taskDir string,
taskEnv *env.TaskEnvironment, allowAbs bool) (map[ctconf.TemplateConfig]*structs.Template, error) {
// Build the task environment
taskEnv.Build()
taskEnv *env.TaskEnv, allowAbs bool) (map[ctconf.TemplateConfig]*structs.Template, error) {

ctmpls := make(map[ctconf.TemplateConfig]*structs.Template, len(tmpls))
for _, tmpl := range tmpls {
Expand Down Expand Up @@ -492,3 +509,63 @@ func runnerConfig(config *config.Config, vaultToken string) (*ctconf.Config, err
conf.Finalize()
return conf, nil
}

// loadTemplateEnv loads task environment variables from all templates.
func loadTemplateEnv(tmpls []*structs.Template, taskDir string) (map[string]string, error) {
all := make(map[string]string, 50)
for _, t := range tmpls {
if !t.Envvars {
continue
}
f, err := os.Open(filepath.Join(taskDir, t.DestPath))
if err != nil {
return nil, fmt.Errorf("error opening env template: %v", err)
}
defer f.Close()

// Parse environment fil
vars, err := parseEnvFile(f)
if err != nil {
return nil, fmt.Errorf("error parsing env template %q: %v", t.DestPath, err)
}
for k, v := range vars {
all[k] = v
}
}
return all, nil
}

// parseEnvFile and return a map of the environment variables suitable for
// TaskEnvironment.AppendEnvvars or an error.
//
// See nomad/structs#Template.Envvars comment for format.
func parseEnvFile(r io.Reader) (map[string]string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It may be worth just using this: https://github.com/joho/godotenv

Copy link
Contributor

Choose a reason for hiding this comment

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

Lots of edge cases we don't need to handle

Copy link
Contributor

Choose a reason for hiding this comment

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

There is lots of prior art here, notably envdir and chpst's -e flag. If .env is a directory, not a file, it would be baller if this parsed the contents of the directory in an envdir-compatible way.

Copy link
Member Author

Choose a reason for hiding this comment

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

Since this is tied to a template it must be a single file. It's probably worth documenting how to use env files from artifacts by setting source path to where the artifact is downloaded, but env dirs (or autoloading .env files) is out of scope for this feature.

vars := make(map[string]string, 50)
lines := 0
scanner := bufio.NewScanner(r)
for scanner.Scan() {
lines++
buf := scanner.Bytes()
if len(buf) == 0 {
// Skip empty lines
continue
}
if buf[0] == '#' {
// Skip lines starting with a #
continue
}
n := bytes.IndexByte(buf, '=')
if n == -1 {
return nil, fmt.Errorf("line %d: no '=' sign: %q", lines, string(buf))
}
if len(buf) > n {
vars[string(buf[0:n])] = string(buf[n+1 : len(buf)])
} else {
vars[string(buf[0:n])] = ""
}
}
if err := scanner.Err(); err != nil {
return nil, err
}
return vars, nil
}
Loading