From b8a42786f3750442b4ea810e56aea25b0071e253 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Mon, 11 Jan 2016 09:58:26 -0800 Subject: [PATCH 1/4] merge --- client/client.go | 2 +- client/driver/docker.go | 20 +-- client/driver/driver.go | 32 ++--- client/driver/driver_test.go | 8 +- client/driver/environment/vars.go | 169 ----------------------- client/driver/environment/vars_test.go | 79 ----------- client/driver/exec.go | 11 +- client/driver/executor/exec.go | 25 +++- client/driver/executor/exec_basic.go | 16 +-- client/driver/executor/exec_linux.go | 19 +-- client/driver/executor/exec_universal.go | 8 +- client/driver/java.go | 11 +- client/driver/qemu.go | 9 +- client/driver/raw_exec.go | 11 +- client/driver/rkt.go | 12 +- client/task_runner.go | 12 +- helper/args/args.go | 22 +-- helper/args/args_test.go | 55 +++++--- 18 files changed, 145 insertions(+), 376 deletions(-) delete mode 100644 client/driver/environment/vars.go delete mode 100644 client/driver/environment/vars_test.go diff --git a/client/client.go b/client/client.go index b371b5df90d..61b31712702 100644 --- a/client/client.go +++ b/client/client.go @@ -533,7 +533,7 @@ func (c *Client) setupDrivers() error { var avail []string var skipped []string - driverCtx := driver.NewDriverContext("", c.config, c.config.Node, c.logger) + 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. diff --git a/client/driver/docker.go b/client/driver/docker.go index 82a1823c083..aa683126f40 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -17,7 +17,6 @@ import ( "github.com/hashicorp/nomad/client/config" cstructs "github.com/hashicorp/nomad/client/driver/structs" "github.com/hashicorp/nomad/client/fingerprint" - "github.com/hashicorp/nomad/helper/args" "github.com/hashicorp/nomad/nomad/structs" "github.com/mitchellh/mapstructure" ) @@ -188,9 +187,12 @@ func (d *DockerDriver) createContainer(ctx *ExecContext, task *structs.Task, dri } // Create environment variables. - env := TaskEnvironmentVariables(ctx, task) - env.SetAllocDir(filepath.Join("/", allocdir.SharedAllocName)) - env.SetTaskLocalDir(filepath.Join("/", allocdir.TaskLocal)) + taskEnv, err := GetTaskEnv(ctx.AllocDir, d.node, task) + if err != nil { + return c, err + } + taskEnv.SetAllocDir(filepath.Join("/", allocdir.SharedAllocName)) + taskEnv.SetTaskLocalDir(filepath.Join("/", allocdir.TaskLocal)) config := &docker.Config{ Image: driverConfig.ImageName, @@ -343,20 +345,20 @@ func (d *DockerDriver) createContainer(ctx *ExecContext, task *structs.Task, dri d.logger.Printf("[DEBUG] driver.docker: exposed port %s", containerPort) } - // This was set above in a call to TaskEnvironmentVariables but if we + // This was set above in a call to GetTaskEnv but if we // have mapped any ports we will need to override them. // - // TODO refactor the implementation in TaskEnvironmentVariables to match + // TODO refactor the implementation in GetTaskEnv to match // the 0.2 ports world view. Docker seems to be the only place where // this is actually needed, but this is kinda hacky. if len(driverConfig.PortMap) > 0 { - env.SetPorts(network.MapLabelToValues(driverConfig.PortMap)) + taskEnv.SetPorts(network.MapLabelToValues(driverConfig.PortMap)) } hostConfig.PortBindings = publishedPorts config.ExposedPorts = exposedPorts } - parsedArgs := args.ParseAndReplace(driverConfig.Args, env.Map()) + parsedArgs := taskEnv.ParseAndReplace(driverConfig.Args) // If the user specified a custom command to run as their entrypoint, we'll // inject it here. @@ -376,7 +378,7 @@ func (d *DockerDriver) createContainer(ctx *ExecContext, task *structs.Task, dri d.logger.Printf("[DEBUG] driver.docker: applied labels on the container: %+v", config.Labels) } - config.Env = env.List() + config.Env = taskEnv.EnvList() containerName := fmt.Sprintf("%s-%s", task.Name, ctx.AllocID) d.logger.Printf("[DEBUG] driver.docker: setting container name to: %s", containerName) diff --git a/client/driver/driver.go b/client/driver/driver.go index ba7e484f7a8..89a48f5690d 100644 --- a/client/driver/driver.go +++ b/client/driver/driver.go @@ -9,7 +9,7 @@ import ( "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/client/driver/environment" + "github.com/hashicorp/nomad/client/driver/env" "github.com/hashicorp/nomad/client/fingerprint" "github.com/hashicorp/nomad/nomad/structs" @@ -66,18 +66,21 @@ type DriverContext struct { config *config.Config logger *log.Logger node *structs.Node + taskEnv *env.TaskEnvironment } // NewDriverContext initializes a new DriverContext with the specified fields. // This enables other packages to create DriverContexts but keeps the fields // private to the driver. If we want to change this later we can gorename all of // the fields in DriverContext. -func NewDriverContext(taskName string, config *config.Config, node *structs.Node, logger *log.Logger) *DriverContext { +func NewDriverContext(taskName string, config *config.Config, node *structs.Node, + logger *log.Logger, taskEnv *env.TaskEnvironment) *DriverContext { return &DriverContext{ taskName: taskName, config: config, node: node, logger: logger, + taskEnv: taskEnv, } } @@ -125,17 +128,18 @@ func NewExecContext(alloc *allocdir.AllocDir, allocID string) *ExecContext { return &ExecContext{AllocDir: alloc, AllocID: allocID} } -// TaskEnvironmentVariables converts exec context and task configuration into a +// GetTaskEnv converts the alloc dir, the node and task configuration into a // TaskEnvironment. -func TaskEnvironmentVariables(ctx *ExecContext, task *structs.Task) environment.TaskEnvironment { - env := environment.NewTaskEnivornment() - env.SetMeta(task.Meta) - - if ctx.AllocDir != nil { - env.SetAllocDir(ctx.AllocDir.SharedDir) - taskdir, ok := ctx.AllocDir.TaskDirs[task.Name] +func GetTaskEnv(alloc *allocdir.AllocDir, node *structs.Node, task *structs.Task) (*env.TaskEnvironment, error) { + env := env.NewTaskEnvironment(node). + SetMeta(task.Meta). + SetEnvvars(task.Env) + + if alloc != nil { + env.SetAllocDir(alloc.SharedDir) + taskdir, ok := alloc.TaskDirs[task.Name] if !ok { - // TODO: Update this to return an error + return nil, fmt.Errorf("failed to get task directory for task %q", task.Name) } env.SetTaskLocalDir(filepath.Join(taskdir, allocdir.TaskLocal)) @@ -152,11 +156,7 @@ func TaskEnvironmentVariables(ctx *ExecContext, task *structs.Task) environment. } } - if task.Env != nil { - env.SetEnvvars(task.Env) - } - - return env + return env.Build(), nil } func mapMergeStrInt(maps ...map[string]int) map[string]int { diff --git a/client/driver/driver_test.go b/client/driver/driver_test.go index 92f72bf388f..b5df412ca69 100644 --- a/client/driver/driver_test.go +++ b/client/driver/driver_test.go @@ -81,7 +81,6 @@ func TestDriver_KillTimeout(t *testing.T) { func TestDriver_TaskEnvironmentVariables(t *testing.T) { t.Parallel() - ctx := &ExecContext{} task := &structs.Task{ Env: map[string]string{ "HELLO": "world", @@ -104,7 +103,10 @@ func TestDriver_TaskEnvironmentVariables(t *testing.T) { }, } - env := TaskEnvironmentVariables(ctx, task) + env, err := GetTaskEnv(nil, nil, task) + if err != nil { + t.Fatalf("GetTaskEnv() failed: %v", err) + } exp := map[string]string{ "NOMAD_CPU_LIMIT": "1000", "NOMAD_MEMORY_LIMIT": "500", @@ -123,7 +125,7 @@ func TestDriver_TaskEnvironmentVariables(t *testing.T) { act := env.Map() if !reflect.DeepEqual(act, exp) { - t.Fatalf("TaskEnvironmentVariables(%#v, %#v) returned %#v; want %#v", ctx, task, act, exp) + t.Fatalf("GetTaskEnv() returned %#v; want %#v", act, exp) } } diff --git a/client/driver/environment/vars.go b/client/driver/environment/vars.go deleted file mode 100644 index 3dc876c0131..00000000000 --- a/client/driver/environment/vars.go +++ /dev/null @@ -1,169 +0,0 @@ -package environment - -import ( - "fmt" - "strconv" - "strings" -) - -// A set of environment variables that are exported by each driver. -const ( - // The path to the alloc directory that is shared across tasks within a task - // group. - AllocDir = "NOMAD_ALLOC_DIR" - - // The path to the tasks local directory where it can store data that is - // persisted to the alloc is removed. - TaskLocalDir = "NOMAD_TASK_DIR" - - // The tasks memory limit in MBs. - MemLimit = "NOMAD_MEMORY_LIMIT" - - // The tasks limit in MHz. - CpuLimit = "NOMAD_CPU_LIMIT" - - // The IP address for the task. - TaskIP = "NOMAD_IP" - - // Prefix for passing both dynamic and static port allocations to - // tasks. - // E.g. $NOMAD_PORT_1 or $NOMAD_PORT_http - PortPrefix = "NOMAD_PORT_" - - // Prefix for passing task meta data. - MetaPrefix = "NOMAD_META_" -) - -var ( - nomadVars = []string{AllocDir, TaskLocalDir, MemLimit, CpuLimit, TaskIP, PortPrefix, MetaPrefix} -) - -type TaskEnvironment map[string]string - -func NewTaskEnivornment() TaskEnvironment { - return make(map[string]string) -} - -// ParseFromList parses a list of strings with NAME=value pairs and returns a -// TaskEnvironment. -func ParseFromList(envVars []string) (TaskEnvironment, error) { - t := NewTaskEnivornment() - - for _, pair := range envVars { - // Start the search from the second byte to skip a possible leading - // "=". Cmd.exe on Windows creates some special environment variables - // that start with an "=" and they can be properly retrieved by OS - // functions so we should handle them properly here. - idx := strings.Index(pair[1:], "=") - if idx == -1 { - return nil, fmt.Errorf("Couldn't parse environment variable: %v", pair) - } - idx++ // adjust for slice offset above - t[pair[:idx]] = pair[idx+1:] - } - - return t, nil -} - -// Returns a list of strings with NAME=value pairs. -func (t TaskEnvironment) List() []string { - env := []string{} - for k, v := range t { - env = append(env, fmt.Sprintf("%s=%s", k, v)) - } - - return env -} - -func (t TaskEnvironment) Map() map[string]string { - return t -} - -func (t TaskEnvironment) SetAllocDir(dir string) { - t[AllocDir] = dir -} - -func (t TaskEnvironment) ClearAllocDir() { - delete(t, AllocDir) -} - -func (t TaskEnvironment) SetTaskLocalDir(dir string) { - t[TaskLocalDir] = dir -} - -func (t TaskEnvironment) ClearTaskLocalDir() { - delete(t, TaskLocalDir) -} - -func (t TaskEnvironment) SetMemLimit(limit int) { - t[MemLimit] = strconv.Itoa(limit) -} - -func (t TaskEnvironment) ClearMemLimit() { - delete(t, MemLimit) -} - -func (t TaskEnvironment) SetCpuLimit(limit int) { - t[CpuLimit] = strconv.Itoa(limit) -} - -func (t TaskEnvironment) ClearCpuLimit() { - delete(t, CpuLimit) -} - -func (t TaskEnvironment) SetTaskIp(ip string) { - t[TaskIP] = ip -} - -func (t TaskEnvironment) ClearTaskIp() { - delete(t, TaskIP) -} - -// Takes a map of port labels to their port value. -func (t TaskEnvironment) SetPorts(ports map[string]int) { - for label, port := range ports { - t[fmt.Sprintf("%s%s", PortPrefix, label)] = strconv.Itoa(port) - } -} - -func (t TaskEnvironment) ClearPorts() { - for k, _ := range t { - if strings.HasPrefix(k, PortPrefix) { - delete(t, k) - } - } -} - -// Takes a map of meta values to be passed to the task. The keys are capatilized -// when the environent variable is set. -func (t TaskEnvironment) SetMeta(m map[string]string) { - for k, v := range m { - t[fmt.Sprintf("%s%s", MetaPrefix, strings.ToUpper(k))] = v - } -} - -func (t TaskEnvironment) ClearMeta() { - for k, _ := range t { - if strings.HasPrefix(k, MetaPrefix) { - delete(t, k) - } - } -} - -func (t TaskEnvironment) SetEnvvars(m map[string]string) { - for k, v := range m { - t[k] = v - } -} - -func (t TaskEnvironment) ClearEnvvars() { -OUTER: - for k, _ := range t { - for _, nomadPrefix := range nomadVars { - if strings.HasPrefix(k, nomadPrefix) { - continue OUTER - } - } - delete(t, k) - } -} diff --git a/client/driver/environment/vars_test.go b/client/driver/environment/vars_test.go deleted file mode 100644 index 1d28ffbaffb..00000000000 --- a/client/driver/environment/vars_test.go +++ /dev/null @@ -1,79 +0,0 @@ -package environment - -import ( - "reflect" - "sort" - "testing" -) - -func TestEnvironment_AsList(t *testing.T) { - env := NewTaskEnivornment() - env.SetTaskIp("127.0.0.1") - env.SetPorts(map[string]int{"http": 80}) - env.SetMeta(map[string]string{"foo": "baz"}) - - act := env.List() - exp := []string{"NOMAD_IP=127.0.0.1", "NOMAD_PORT_http=80", "NOMAD_META_FOO=baz"} - sort.Strings(act) - sort.Strings(exp) - if !reflect.DeepEqual(act, exp) { - t.Fatalf("env.List() returned %v; want %v", act, exp) - } -} - -func TestEnvironment_ParseFromList(t *testing.T) { - input := []string{ - "foo=bar", - "BAZ=baM", - "bar=emb=edded", // This can be done in multiple OSes. - "=ExitCode=00000000", // A Windows cmd.exe annoyance - } - env, err := ParseFromList(input) - if err != nil { - t.Fatalf("ParseFromList(%#v) failed: %v", input, err) - } - - exp := map[string]string{ - "foo": "bar", - "BAZ": "baM", - "bar": "emb=edded", - "=ExitCode": "00000000", - } - - if len(env) != len(exp) { - t.Errorf("ParseFromList(%#v) has length %v; want %v", input, len(env), len(exp)) - } - - for k, v := range exp { - if actV, ok := env[k]; !ok { - t.Errorf("ParseFromList(%#v) doesn't contain expected %v", input, k) - } else if actV != v { - t.Errorf("ParseFromList(%#v) has incorrect value for %v; got %v; want %v", input, k, actV, v) - } - } -} - -func TestEnvironment_ClearEnvvars(t *testing.T) { - env := NewTaskEnivornment() - env.SetTaskIp("127.0.0.1") - env.SetEnvvars(map[string]string{"foo": "baz", "bar": "bang"}) - - act := env.List() - exp := []string{"NOMAD_IP=127.0.0.1", "bar=bang", "foo=baz"} - sort.Strings(act) - sort.Strings(exp) - if !reflect.DeepEqual(act, exp) { - t.Fatalf("env.List() returned %v; want %v", act, exp) - } - - // Clear the environent variables. - env.ClearEnvvars() - - act = env.List() - exp = []string{"NOMAD_IP=127.0.0.1"} - sort.Strings(act) - sort.Strings(exp) - if !reflect.DeepEqual(act, exp) { - t.Fatalf("env.List() returned %v; want %v", act, exp) - } -} diff --git a/client/driver/exec.go b/client/driver/exec.go index ba4c02349cb..e1fdb1646c3 100644 --- a/client/driver/exec.go +++ b/client/driver/exec.go @@ -92,17 +92,15 @@ func (d *ExecDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, } } - // Get the environment variables. - envVars := TaskEnvironmentVariables(ctx, task) - // Setup the command - cmd := executor.Command(command, driverConfig.Args...) + execCtx := executor.NewExecutorContext(d.taskEnv) + cmd := executor.Command(execCtx, command, driverConfig.Args...) if err := cmd.Limit(task.Resources); err != nil { return nil, fmt.Errorf("failed to constrain resources: %s", err) } // Populate environment variables - cmd.Command().Env = envVars.List() + cmd.Command().Env = d.taskEnv.EnvList() if err := cmd.ConfigureTaskDir(d.taskName, ctx.AllocDir); err != nil { return nil, fmt.Errorf("failed to configure task directory: %v", err) @@ -136,7 +134,8 @@ func (d *ExecDriver) Open(ctx *ExecContext, handleID string) (DriverHandle, erro } // Find the process - cmd, err := executor.OpenId(id.ExecutorId) + execCtx := executor.NewExecutorContext(d.taskEnv) + cmd, err := executor.OpenId(execCtx, id.ExecutorId) if err != nil { return nil, fmt.Errorf("failed to open ID %v: %v", id.ExecutorId, err) } diff --git a/client/driver/executor/exec.go b/client/driver/executor/exec.go index c514890ef31..5acf8ecccc0 100644 --- a/client/driver/executor/exec.go +++ b/client/driver/executor/exec.go @@ -28,6 +28,7 @@ import ( "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/nomad/structs" + "github.com/hashicorp/nomad/client/driver/env" cstructs "github.com/hashicorp/nomad/client/driver/structs" ) @@ -75,9 +76,23 @@ type Executor interface { Command() *exec.Cmd } -// Command is a mirror of exec.Command that returns a platform-specific Executor -func Command(name string, args ...string) Executor { - executor := NewExecutor() +// ExecutorContext is a means to inject dependencies such as loggers, configs, and +// node attributes into a Driver without having to change the Driver interface +// each time we do it. Used in conjection with Factory, above. +type ExecutorContext struct { + taskEnv *env.TaskEnvironment +} + +// NewExecutorContext initializes a new DriverContext with the specified fields. +func NewExecutorContext(taskEnv *env.TaskEnvironment) *ExecutorContext { + return &ExecutorContext{ + taskEnv: taskEnv, + } +} + +// Command returns a platform-specific Executor +func Command(ctx *ExecutorContext, name string, args ...string) Executor { + executor := NewExecutor(ctx) SetCommand(executor, name, args) return executor } @@ -98,8 +113,8 @@ func SetCommand(e Executor, name string, args []string) { // OpenId is similar to executor.Command but will attempt to reopen with the // passed ID. -func OpenId(id string) (Executor, error) { - executor := NewExecutor() +func OpenId(ctx *ExecutorContext, id string) (Executor, error) { + executor := NewExecutor(ctx) err := executor.Open(id) if err != nil { return nil, err diff --git a/client/driver/executor/exec_basic.go b/client/driver/executor/exec_basic.go index 39ce3f36bc7..16745614beb 100644 --- a/client/driver/executor/exec_basic.go +++ b/client/driver/executor/exec_basic.go @@ -11,9 +11,7 @@ import ( "strings" "github.com/hashicorp/nomad/client/allocdir" - "github.com/hashicorp/nomad/client/driver/environment" "github.com/hashicorp/nomad/client/driver/spawn" - "github.com/hashicorp/nomad/helper/args" "github.com/hashicorp/nomad/nomad/structs" cstructs "github.com/hashicorp/nomad/client/driver/structs" @@ -22,6 +20,7 @@ import ( // BasicExecutor should work everywhere, and as a result does not include // any resource restrictions or runas capabilities. type BasicExecutor struct { + *ExecutorContext cmd exec.Cmd spawn *spawn.Spawner taskName string @@ -29,8 +28,8 @@ type BasicExecutor struct { allocDir string } -func NewBasicExecutor() Executor { - return &BasicExecutor{} +func NewBasicExecutor(ctx *ExecutorContext) Executor { + return &BasicExecutor{ExecutorContext: ctx} } func (e *BasicExecutor) Limit(resources *structs.Resources) error { @@ -56,13 +55,8 @@ func (e *BasicExecutor) ConfigureTaskDir(taskName string, alloc *allocdir.AllocD func (e *BasicExecutor) Start() error { // Parse the commands arguments and replace instances of Nomad environment // variables. - envVars, err := environment.ParseFromList(e.cmd.Env) - if err != nil { - return err - } - - e.cmd.Path = args.ReplaceEnv(e.cmd.Path, envVars.Map()) - e.cmd.Args = args.ParseAndReplace(e.cmd.Args, envVars.Map()) + e.cmd.Path = e.taskEnv.ReplaceEnv(e.cmd.Path) + e.cmd.Args = e.taskEnv.ParseAndReplace(e.cmd.Args) spawnState := filepath.Join(e.allocDir, fmt.Sprintf("%s_%s", e.taskName, "exit_status")) e.spawn = spawn.NewSpawner(spawnState) diff --git a/client/driver/executor/exec_linux.go b/client/driver/executor/exec_linux.go index 0648f3c2c5f..e9ceca710cd 100644 --- a/client/driver/executor/exec_linux.go +++ b/client/driver/executor/exec_linux.go @@ -23,7 +23,6 @@ import ( "github.com/hashicorp/nomad/client/driver/environment" "github.com/hashicorp/nomad/client/driver/spawn" cstructs "github.com/hashicorp/nomad/client/driver/structs" - "github.com/hashicorp/nomad/helper/args" "github.com/hashicorp/nomad/nomad/structs" ) @@ -42,16 +41,17 @@ var ( } ) -func NewExecutor() Executor { - return NewLinuxExecutor() +func NewExecutor(ctx *ExecutorContext) Executor { + return NewLinuxExecutor(ctx) } -func NewLinuxExecutor() Executor { - return &LinuxExecutor{} +func NewLinuxExecutor(ctx *ExecutorContext) Executor { + return &LinuxExecutor{ExecutorContext: ctx} } // Linux executor is designed to run on linux kernel 2.8+. type LinuxExecutor struct { + *ExecutorContext cmd exec.Cmd user *user.User @@ -161,13 +161,8 @@ func (e *LinuxExecutor) Start() error { // Parse the commands arguments and replace instances of Nomad environment // variables. - envVars, err := environment.ParseFromList(e.cmd.Env) - if err != nil { - return err - } - - e.cmd.Path = args.ReplaceEnv(e.cmd.Path, envVars.Map()) - e.cmd.Args = args.ParseAndReplace(e.cmd.Args, envVars.Map()) + e.cmd.Path = e.taskEnv.ReplaceEnv(e.cmd.Path, envVars.Map()) + e.cmd.Args = e.taskEnv.ParseAndReplace(e.cmd.Args, envVars.Map()) spawnState := filepath.Join(e.allocDir, fmt.Sprintf("%s_%s", e.taskName, "exit_status")) e.spawn = spawn.NewSpawner(spawnState) diff --git a/client/driver/executor/exec_universal.go b/client/driver/executor/exec_universal.go index 318faea4be8..5ce25ec8e2a 100644 --- a/client/driver/executor/exec_universal.go +++ b/client/driver/executor/exec_universal.go @@ -2,11 +2,13 @@ package executor -func NewExecutor() Executor { - return &UniversalExecutor{BasicExecutor{}} +func NewExecutor(ctx *ExecutorContext) Executor { + return &UniversalExecutor{ + BasicExecutor: NewBasicExecutor(ctx).(*BasicExecutor), + } } // UniversalExecutor wraps the BasicExecutor type UniversalExecutor struct { - BasicExecutor + *BasicExecutor } diff --git a/client/driver/java.go b/client/driver/java.go index 656b7ddd252..2a7cb5a3002 100644 --- a/client/driver/java.go +++ b/client/driver/java.go @@ -125,9 +125,6 @@ func (d *JavaDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, jarName := filepath.Base(path) - // Get the environment variables. - envVars := TaskEnvironmentVariables(ctx, task) - args := []string{} // Look for jvm options if len(driverConfig.JvmOpts) != 0 { @@ -143,10 +140,11 @@ func (d *JavaDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, // Setup the command // Assumes Java is in the $PATH, but could probably be detected - cmd := executor.Command("java", args...) + execCtx := executor.NewExecutorContext(d.taskEnv) + cmd := executor.Command(execCtx, "java", args...) // Populate environment variables - cmd.Command().Env = envVars.List() + cmd.Command().Env = d.taskEnv.EnvList() if err := cmd.Limit(task.Resources); err != nil { return nil, fmt.Errorf("failed to constrain resources: %s", err) @@ -185,7 +183,8 @@ func (d *JavaDriver) Open(ctx *ExecContext, handleID string) (DriverHandle, erro } // Find the process - cmd, err := executor.OpenId(id.ExecutorId) + execCtx := executor.NewExecutorContext(d.taskEnv) + cmd, err := executor.OpenId(execCtx, id.ExecutorId) if err != nil { return nil, fmt.Errorf("failed to open ID %v: %v", id.ExecutorId, err) } diff --git a/client/driver/qemu.go b/client/driver/qemu.go index 377d38e3e3c..76cd5aa42b2 100644 --- a/client/driver/qemu.go +++ b/client/driver/qemu.go @@ -184,8 +184,8 @@ func (d *QemuDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, } // Setup the command - cmd := executor.NewBasicExecutor() - executor.SetCommand(cmd, args[0], args[1:]) + execCtx := executor.NewExecutorContext(d.taskEnv) + cmd := executor.Command(execCtx, args[0], args[1:]...) if err := cmd.Limit(task.Resources); err != nil { return nil, fmt.Errorf("failed to constrain resources: %s", err) } @@ -225,8 +225,9 @@ func (d *QemuDriver) Open(ctx *ExecContext, handleID string) (DriverHandle, erro } // Find the process - cmd := executor.NewBasicExecutor() - if err := cmd.Open(id.ExecutorId); err != nil { + execCtx := executor.NewExecutorContext(d.taskEnv) + cmd, err := executor.OpenId(execCtx, id.ExecutorId) + if err != nil { return nil, fmt.Errorf("failed to open ID %v: %v", id.ExecutorId, err) } diff --git a/client/driver/raw_exec.go b/client/driver/raw_exec.go index dcfb72199e3..376d0b626fd 100644 --- a/client/driver/raw_exec.go +++ b/client/driver/raw_exec.go @@ -90,18 +90,16 @@ func (d *RawExecDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandl } } - // Get the environment variables. - envVars := TaskEnvironmentVariables(ctx, task) - // Setup the command - cmd := executor.NewBasicExecutor() + execCtx := executor.NewExecutorContext(d.taskEnv) + cmd := executor.NewBasicExecutor(execCtx) executor.SetCommand(cmd, command, driverConfig.Args) if err := cmd.Limit(task.Resources); err != nil { return nil, fmt.Errorf("failed to constrain resources: %s", err) } // Populate environment variables - cmd.Command().Env = envVars.List() + cmd.Command().Env = d.taskEnv.EnvList() if err := cmd.ConfigureTaskDir(d.taskName, ctx.AllocDir); err != nil { return nil, fmt.Errorf("failed to configure task directory: %v", err) @@ -135,7 +133,8 @@ func (d *RawExecDriver) Open(ctx *ExecContext, handleID string) (DriverHandle, e } // Find the process - cmd := executor.NewBasicExecutor() + execCtx := executor.NewExecutorContext(d.taskEnv) + cmd := executor.NewBasicExecutor(execCtx) if err := cmd.Open(id.ExecutorId); err != nil { return nil, fmt.Errorf("failed to open ID %v: %v", id.ExecutorId, err) } diff --git a/client/driver/rkt.go b/client/driver/rkt.go index df15a7d7816..abc1f9488f4 100644 --- a/client/driver/rkt.go +++ b/client/driver/rkt.go @@ -19,7 +19,6 @@ import ( "github.com/hashicorp/nomad/client/config" cstructs "github.com/hashicorp/nomad/client/driver/structs" "github.com/hashicorp/nomad/client/fingerprint" - "github.com/hashicorp/nomad/helper/args" "github.com/hashicorp/nomad/nomad/structs" "github.com/mitchellh/mapstructure" ) @@ -148,13 +147,10 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e cmdArgs = append(cmdArgs, "--insecure-options=all") } - // Inject the environment variables. - envVars := TaskEnvironmentVariables(ctx, task) + d.taskEnv.SetAllocDir(filepath.Join("/", allocdir.SharedAllocName)). + SetTaskLocalDir(filepath.Join("/", allocdir.TaskLocal)).Build() - envVars.SetAllocDir(filepath.Join("/", allocdir.SharedAllocName)) - envVars.SetTaskLocalDir(filepath.Join("/", allocdir.TaskLocal)) - - for k, v := range envVars.Map() { + for k, v := range d.taskEnv.EnvMap() { cmdArgs = append(cmdArgs, fmt.Sprintf("--set-env=%v=%v", k, v)) } @@ -188,7 +184,7 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e // Add user passed arguments. if len(driverConfig.Args) != 0 { - parsed := args.ParseAndReplace(driverConfig.Args, envVars.Map()) + parsed := d.taskEnv.ParseAndReplace(driverConfig.Args) // Need to start arguments with "--" if len(parsed) > 0 { diff --git a/client/task_runner.go b/client/task_runner.go index 55c920bd827..1a81eb1e3c6 100644 --- a/client/task_runner.go +++ b/client/task_runner.go @@ -172,12 +172,22 @@ func (r *TaskRunner) setState(state string, event *structs.TaskEvent) { // createDriver makes a driver for the task func (r *TaskRunner) createDriver() (driver.Driver, error) { - driverCtx := driver.NewDriverContext(r.task.Name, r.config, r.config.Node, r.logger) + taskEnv, err := driver.GetTaskEnv(r.ctx.AllocDir, r.config.Node, r.task) + if err != nil { + err = fmt.Errorf("failed to create driver '%s' for alloc %s: %v", + r.task.Driver, r.alloc.ID, err) + r.logger.Printf("[ERR] client: %s", err) + return nil, err + + } + + driverCtx := driver.NewDriverContext(r.task.Name, r.config, r.config.Node, r.logger, taskEnv) driver, err := driver.NewDriver(r.task.Driver, driverCtx) if err != nil { err = fmt.Errorf("failed to create driver '%s' for alloc %s: %v", r.task.Driver, r.alloc.ID, err) r.logger.Printf("[ERR] client: %s", err) + return nil, err } return driver, err } diff --git a/helper/args/args.go b/helper/args/args.go index 9e8a9980c75..d63fdd5221b 100644 --- a/helper/args/args.go +++ b/helper/args/args.go @@ -3,33 +3,23 @@ package args import "regexp" var ( - envRe = regexp.MustCompile(`\$({[a-zA-Z0-9_]+}|[a-zA-Z0-9_]+)`) + envRe = regexp.MustCompile(`\$({[a-zA-Z0-9_\.]+}|[a-zA-Z0-9_\.]+)`) ) -// ParseAndReplace takes the user supplied args and a map of environment -// variables. It replaces any instance of an environment variable in the args -// with the actual value. -func ParseAndReplace(args []string, env map[string]string) []string { - replaced := make([]string, len(args)) - for i, arg := range args { - replaced[i] = ReplaceEnv(arg, env) - } - - return replaced -} - // ReplaceEnv takes an arg and replaces all occurences of environment variables. // If the variable is found in the passed map it is replaced, otherwise the // original string is returned. -func ReplaceEnv(arg string, env map[string]string) string { +func ReplaceEnv(arg string, environents ...map[string]string) string { return envRe.ReplaceAllStringFunc(arg, func(arg string) string { stripped := arg[1:] if stripped[0] == '{' { stripped = stripped[1 : len(stripped)-1] } - if value, ok := env[stripped]; ok { - return value + for _, env := range environents { + if value, ok := env[stripped]; ok { + return value + } } return arg diff --git a/helper/args/args_test.go b/helper/args/args_test.go index 5e7cbca4a28..11b33103e60 100644 --- a/helper/args/args_test.go +++ b/helper/args/args_test.go @@ -7,45 +7,58 @@ import ( ) const ( - ipKey = "NOMAD_IP" - ipVal = "127.0.0.1" - portKey = "NOMAD_PORT_WEB" - portVal = ":80" + ipKey = "NOMAD_IP" + ipVal = "127.0.0.1" + portKey = "NOMAD_PORT_WEB" + portVal = ":80" + periodKey = "NOMAD.PERIOD" + periodVal = "period" ) var ( envVars = map[string]string{ - ipKey: ipVal, - portKey: portVal, + ipKey: ipVal, + portKey: portVal, + periodKey: periodVal, } ) -func TestDriverArgs_ParseAndReplaceInvalidEnv(t *testing.T) { - input := []string{"invalid", "$FOO"} - exp := []string{"invalid", "$FOO"} - act := ParseAndReplace(input, envVars) +func TestArgs_ReplaceEnv_Invalid(t *testing.T) { + input := "$FOO" + exp := "$FOO" + act := ReplaceEnv(input, envVars) if !reflect.DeepEqual(act, exp) { - t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) + t.Fatalf("ReplaceEnv(%v, %v) returned %#v; want %#v", input, envVars, act, exp) } } -func TestDriverArgs_ParseAndReplaceValidEnv(t *testing.T) { - input := []string{"nomad_ip", fmt.Sprintf(`"$%v"!`, ipKey)} - exp := []string{"nomad_ip", fmt.Sprintf("\"%s\"!", ipVal)} - act := ParseAndReplace(input, envVars) +func TestArgs_ReplaceEnv_Valid(t *testing.T) { + input := fmt.Sprintf(`"$%v"!`, ipKey) + exp := fmt.Sprintf("\"%s\"!", ipVal) + act := ReplaceEnv(input, envVars) if !reflect.DeepEqual(act, exp) { - t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) + t.Fatalf("ReplaceEnv(%v, %v) returned %#v; want %#v", input, envVars, act, exp) } } -func TestDriverArgs_ParseAndReplaceChainedEnv(t *testing.T) { - input := []string{"-foo", fmt.Sprintf("$%s$%s", ipKey, portKey)} - exp := []string{"-foo", fmt.Sprintf("%s%s", ipVal, portVal)} - act := ParseAndReplace(input, envVars) +func TestArgs_ReplaceEnv_Period(t *testing.T) { + input := fmt.Sprintf(`"$%v"!`, periodKey) + exp := fmt.Sprintf("\"%s\"!", periodVal) + act := ReplaceEnv(input, envVars) if !reflect.DeepEqual(act, exp) { - t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) + t.Fatalf("ReplaceEnv(%v, %v) returned %#v; want %#v", input, envVars, act, exp) + } +} + +func TestArgs_ReplaceEnv_Chained(t *testing.T) { + input := fmt.Sprintf("$%s$%s", ipKey, portKey) + exp := fmt.Sprintf("%s%s", ipVal, portVal) + act := ReplaceEnv(input, envVars) + + if !reflect.DeepEqual(act, exp) { + t.Fatalf("ReplaceEnv(%v, %v) returned %#v; want %#v", input, envVars, act, exp) } } From 8cc513d16c8dd95a2fd3b42a774399a811059d2a Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Tue, 5 Jan 2016 14:50:25 -0800 Subject: [PATCH 2/4] Add folder --- client/driver/env/env.go | 265 ++++++++++++++++++++++++++++++++++ client/driver/env/env_test.go | 174 ++++++++++++++++++++++ 2 files changed, 439 insertions(+) create mode 100644 client/driver/env/env.go create mode 100644 client/driver/env/env_test.go diff --git a/client/driver/env/env.go b/client/driver/env/env.go new file mode 100644 index 00000000000..d5ab3e9fcca --- /dev/null +++ b/client/driver/env/env.go @@ -0,0 +1,265 @@ +package env + +import ( + "fmt" + "strconv" + "strings" + + hargs "github.com/hashicorp/nomad/helper/args" + "github.com/hashicorp/nomad/nomad/structs" +) + +// A set of environment variables that are exported by each driver. +const ( + // The path to the alloc directory that is shared across tasks within a task + // group. + AllocDir = "NOMAD_ALLOC_DIR" + + // The path to the tasks local directory where it can store data that is + // persisted to the alloc is removed. + TaskLocalDir = "NOMAD_TASK_DIR" + + // The tasks memory limit in MBs. + MemLimit = "NOMAD_MEMORY_LIMIT" + + // The tasks limit in MHz. + CpuLimit = "NOMAD_CPU_LIMIT" + + // The IP address for the task. + TaskIP = "NOMAD_IP" + + // Prefix for passing both dynamic and static port allocations to + // tasks. + // E.g. $NOMAD_PORT_1 or $NOMAD_PORT_http + PortPrefix = "NOMAD_PORT_" + + // Prefix for passing task meta data. + MetaPrefix = "NOMAD_META_" +) + +// The node values that can be interpreted. +const ( + nodeIdKey = "node.id" + nodeDcKey = "node.datacenter" + nodeNameKey = "node.name" + nodeClassKey = "node.class" + + // Prefixes used for lookups. + nodeAttributePrefix = "attr." + nodeMetaPrefix = "meta." +) + +// TaskEnvironment is used to expose information to a task via environment +// variables and provide interpolation of Nomad variables. +type TaskEnvironment struct { + env map[string]string + meta map[string]string + ports map[string]int + allocDir string + taskDir string + cpuLimit int + memLimit int + ip string + node *structs.Node + + // taskEnv is the variables that will be set in the tasks environment + taskEnv map[string]string + + // nodeValues is the values that are allowed for interprolation from the + // node. + nodeValues map[string]string +} + +func NewTaskEnvironment(node *structs.Node) *TaskEnvironment { + return &TaskEnvironment{node: node} +} + +// ParseAndReplace takes the user supplied args replaces any instance of an +// environment variable or nomad variable in the args with the actual value. +func (t *TaskEnvironment) ParseAndReplace(args []string) []string { + replaced := make([]string, len(args)) + for i, arg := range args { + replaced[i] = hargs.ReplaceEnv(arg, t.taskEnv, t.nodeValues) + } + + return replaced +} + +// ReplaceEnv takes an arg and replaces all occurences of environment variables +// and nomad variables. If the variable is found in the passed map it is +// replaced, otherwise the original string is returned. +func (t *TaskEnvironment) ReplaceEnv(arg string) string { + return hargs.ReplaceEnv(arg, t.taskEnv, t.nodeValues) +} + +// Build must be called after all the tasks environment values have been set. +func (t *TaskEnvironment) Build() *TaskEnvironment { + t.nodeValues = make(map[string]string) + t.taskEnv = make(map[string]string) + + // Build the task metadata + for k, v := range t.meta { + t.taskEnv[fmt.Sprintf("%s%s", MetaPrefix, strings.ToUpper(k))] = v + } + + // Build the ports + for label, port := range t.ports { + t.taskEnv[fmt.Sprintf("%s%s", PortPrefix, label)] = strconv.Itoa(port) + } + + // Build the directories + if t.allocDir != "" { + t.taskEnv[AllocDir] = t.allocDir + } + if t.taskDir != "" { + t.taskEnv[TaskLocalDir] = t.taskDir + } + + // Build the resource limits + if t.memLimit != 0 { + t.taskEnv[MemLimit] = strconv.Itoa(t.memLimit) + } + if t.cpuLimit != 0 { + t.taskEnv[CpuLimit] = strconv.Itoa(t.cpuLimit) + } + + // Build the IP + if t.ip != "" { + t.taskEnv[TaskIP] = t.ip + } + + // Build the node + if t.node != nil { + // Set up the node values. + t.nodeValues[nodeIdKey] = t.node.ID + t.nodeValues[nodeDcKey] = t.node.Datacenter + t.nodeValues[nodeNameKey] = t.node.Name + t.nodeValues[nodeClassKey] = t.node.NodeClass + + // Set up the attributes. + for k, v := range t.node.Attributes { + t.nodeValues[fmt.Sprintf("%s%s", nodeAttributePrefix, k)] = v + } + + // Set up the meta. + for k, v := range t.node.Meta { + t.nodeValues[fmt.Sprintf("%s%s", nodeMetaPrefix, k)] = v + } + } + + // Interpret the environment variables + interpreted := make(map[string]string, len(t.env)) + for k, v := range t.env { + interpreted[k] = hargs.ReplaceEnv(v, t.nodeValues, t.taskEnv) + } + + for k, v := range interpreted { + t.taskEnv[k] = v + } + + return t +} + +// EnvList returns a list of strings with NAME=value pairs. +func (t *TaskEnvironment) EnvList() []string { + env := []string{} + for k, v := range t.taskEnv { + env = append(env, fmt.Sprintf("%s=%s", k, v)) + } + + return env +} + +// EnvMap returns a copy of the tasks environment variables. +func (t *TaskEnvironment) EnvMap() map[string]string { + m := make(map[string]string, len(t.taskEnv)) + for k, v := range t.taskEnv { + m[k] = v + } + + return m +} + +// Builder methods to build the TaskEnvironment +func (t *TaskEnvironment) SetAllocDir(dir string) *TaskEnvironment { + t.allocDir = dir + return t +} + +func (t *TaskEnvironment) ClearAllocDir() *TaskEnvironment { + t.allocDir = "" + return t +} + +func (t *TaskEnvironment) SetTaskLocalDir(dir string) *TaskEnvironment { + t.taskDir = dir + return t +} + +func (t *TaskEnvironment) ClearTaskLocalDir() *TaskEnvironment { + t.taskDir = "" + return t +} + +func (t *TaskEnvironment) SetMemLimit(limit int) *TaskEnvironment { + t.memLimit = limit + return t +} + +func (t *TaskEnvironment) ClearMemLimit() *TaskEnvironment { + t.memLimit = 0 + return t +} + +func (t *TaskEnvironment) SetCpuLimit(limit int) *TaskEnvironment { + t.cpuLimit = limit + return t +} + +func (t *TaskEnvironment) ClearCpuLimit() *TaskEnvironment { + t.cpuLimit = 0 + return t +} + +func (t *TaskEnvironment) SetTaskIp(ip string) *TaskEnvironment { + t.ip = ip + return t +} + +func (t *TaskEnvironment) ClearTaskIp() *TaskEnvironment { + t.ip = "" + return t +} + +// Takes a map of port labels to their port value. +func (t *TaskEnvironment) SetPorts(ports map[string]int) *TaskEnvironment { + t.ports = ports + return t +} + +func (t *TaskEnvironment) ClearPorts() *TaskEnvironment { + t.ports = nil + return t +} + +// Takes a map of meta values to be passed to the task. The keys are capatilized +// when the environent variable is set. +func (t *TaskEnvironment) SetMeta(m map[string]string) *TaskEnvironment { + t.meta = m + return t +} + +func (t *TaskEnvironment) ClearMeta() *TaskEnvironment { + t.meta = nil + return t +} + +func (t *TaskEnvironment) SetEnvvars(m map[string]string) *TaskEnvironment { + t.env = m + return t +} + +func (t *TaskEnvironment) ClearEnvvars() *TaskEnvironment { + t.env = nil + return t +} diff --git a/client/driver/env/env_test.go b/client/driver/env/env_test.go new file mode 100644 index 00000000000..43a8eb65dce --- /dev/null +++ b/client/driver/env/env_test.go @@ -0,0 +1,174 @@ +package env + +import ( + "fmt" + "reflect" + "sort" + "testing" + + "github.com/hashicorp/nomad/nomad/mock" +) + +const ( + // Node values that tests can rely on + metaKey = "instance" + metaVal = "t2-micro" + attrKey = "arch" + attrVal = "amd64" + nodeName = "test node" + nodeClass = "test class" + + // Environment variable values that tests can rely on + envOneKey = "NOMAD_IP" + envOneVal = "127.0.0.1" + envTwoKey = "NOMAD_PORT_WEB" + envTwoVal = ":80" +) + +func testTaskEnvironment() *TaskEnvironment { + n := mock.Node() + n.Attributes = map[string]string{ + attrKey: attrVal, + } + n.Meta = map[string]string{ + metaKey: metaVal, + } + n.Name = nodeName + n.NodeClass = nodeClass + + envVars := map[string]string{ + envOneKey: envOneVal, + envTwoKey: envTwoVal, + } + return NewTaskEnvironment(n).SetEnvvars(envVars).Build() +} + +func TestEnvironment_ParseAndReplace_Env(t *testing.T) { + env := testTaskEnvironment() + + input := []string{fmt.Sprintf(`"$%v"!`, envOneKey), fmt.Sprintf("$%s$%s", envOneKey, envTwoKey)} + act := env.ParseAndReplace(input) + exp := []string{fmt.Sprintf(`"%s"!`, envOneVal), fmt.Sprintf("%s%s", envOneVal, envTwoVal)} + + if !reflect.DeepEqual(act, exp) { + t.Fatalf("ParseAndReplace(%v) returned %#v; want %#v", input, act, exp) + } +} + +func TestEnvironment_ParseAndReplace_Meta(t *testing.T) { + input := []string{fmt.Sprintf("$%v%v", nodeMetaPrefix, metaKey)} + exp := []string{metaVal} + env := testTaskEnvironment() + act := env.ParseAndReplace(input) + + if !reflect.DeepEqual(act, exp) { + t.Fatalf("ParseAndReplace(%v) returned %#v; want %#v", input, act, exp) + } +} + +func TestEnvironment_ParseAndReplace_Attr(t *testing.T) { + input := []string{fmt.Sprintf("$%v%v", nodeAttributePrefix, attrKey)} + exp := []string{attrVal} + env := testTaskEnvironment() + act := env.ParseAndReplace(input) + + if !reflect.DeepEqual(act, exp) { + t.Fatalf("ParseAndReplace(%v) returned %#v; want %#v", input, act, exp) + } +} + +func TestEnvironment_ParseAndReplace_Node(t *testing.T) { + input := []string{fmt.Sprintf("$%v", nodeNameKey), fmt.Sprintf("$%v", nodeClassKey)} + exp := []string{nodeName, nodeClass} + env := testTaskEnvironment() + act := env.ParseAndReplace(input) + + if !reflect.DeepEqual(act, exp) { + t.Fatalf("ParseAndReplace(%v) returned %#v; want %#v", input, act, exp) + } +} + +func TestEnvironment_ParseAndReplace_Mixed(t *testing.T) { + input := []string{ + fmt.Sprintf("$%v$%v%v", nodeNameKey, nodeAttributePrefix, attrKey), + fmt.Sprintf("$%v$%v%v", nodeClassKey, nodeMetaPrefix, metaKey), + fmt.Sprintf("$%v$%v", envTwoKey, nodeClassKey), + } + exp := []string{ + fmt.Sprintf("%v%v", nodeName, attrVal), + fmt.Sprintf("%v%v", nodeClass, metaVal), + fmt.Sprintf("%v%v", envTwoVal, nodeClass), + } + env := testTaskEnvironment() + act := env.ParseAndReplace(input) + + if !reflect.DeepEqual(act, exp) { + t.Fatalf("ParseAndReplace(%v) returned %#v; want %#v", input, act, exp) + } +} + +func TestEnvironment_ReplaceEnv_Mixed(t *testing.T) { + input := fmt.Sprintf("$%v$%v%v", nodeNameKey, nodeAttributePrefix, attrKey) + exp := fmt.Sprintf("%v%v", nodeName, attrVal) + env := testTaskEnvironment() + act := env.ReplaceEnv(input) + + if act != exp { + t.Fatalf("ParseAndReplace(%v) returned %#v; want %#v", input, act, exp) + } +} + +func TestEnvironment_AsList(t *testing.T) { + n := mock.Node() + env := NewTaskEnvironment(n). + SetTaskIp("127.0.0.1").SetPorts(map[string]int{"http": 80}). + SetMeta(map[string]string{"foo": "baz"}).Build() + + act := env.EnvList() + exp := []string{"NOMAD_IP=127.0.0.1", "NOMAD_PORT_http=80", "NOMAD_META_FOO=baz"} + sort.Strings(act) + sort.Strings(exp) + if !reflect.DeepEqual(act, exp) { + t.Fatalf("env.List() returned %v; want %v", act, exp) + } +} + +func TestEnvironment_ClearEnvvars(t *testing.T) { + n := mock.Node() + env := NewTaskEnvironment(n). + SetTaskIp("127.0.0.1"). + SetEnvvars(map[string]string{"foo": "baz", "bar": "bang"}).Build() + + act := env.EnvList() + exp := []string{"NOMAD_IP=127.0.0.1", "bar=bang", "foo=baz"} + sort.Strings(act) + sort.Strings(exp) + if !reflect.DeepEqual(act, exp) { + t.Fatalf("env.List() returned %v; want %v", act, exp) + } + + // Clear the environent variables. + env.ClearEnvvars().Build() + + act = env.EnvList() + exp = []string{"NOMAD_IP=127.0.0.1"} + sort.Strings(act) + sort.Strings(exp) + if !reflect.DeepEqual(act, exp) { + t.Fatalf("env.List() returned %v; want %v", act, exp) + } +} + +func TestEnvironment_Interprolate(t *testing.T) { + env := testTaskEnvironment(). + SetEnvvars(map[string]string{"test": "$node.class", "test2": "$attr.arch"}). + Build() + + act := env.EnvList() + exp := []string{fmt.Sprintf("test=%s", nodeClass), fmt.Sprintf("test2=%s", attrVal)} + sort.Strings(act) + sort.Strings(exp) + if !reflect.DeepEqual(act, exp) { + t.Fatalf("env.List() returned %v; want %v", act, exp) + } +} From dccbc0320b290b1d0da8c83faea06c583a79497b Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Tue, 5 Jan 2016 18:02:11 -0800 Subject: [PATCH 3/4] Fix tests --- client/driver/docker_test.go | 56 +++++++---------- client/driver/driver_test.go | 29 +++++---- client/driver/exec_test.go | 53 +++++++--------- client/driver/executor/exec_linux.go | 15 ++--- client/driver/executor/test_harness_test.go | 18 ++++-- client/driver/java_test.go | 26 ++++---- client/driver/qemu_test.go | 19 +++--- client/driver/raw_exec_test.go | 70 ++++++++++----------- client/driver/rkt_test.go | 35 +++++------ 9 files changed, 148 insertions(+), 173 deletions(-) diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 8adcc60920b..7f58a349f84 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -11,17 +11,11 @@ import ( docker "github.com/fsouza/go-dockerclient" "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/client/driver/environment" + "github.com/hashicorp/nomad/client/driver/env" cstructs "github.com/hashicorp/nomad/client/driver/structs" "github.com/hashicorp/nomad/nomad/structs" ) -func testDockerDriverContext(task string) *DriverContext { - cfg := testConfig() - cfg.DevMode = true - return NewDriverContext(task, cfg, cfg.Node, testLogger()) -} - // dockerIsConnected checks to see if a docker daemon is available (local or remote) func dockerIsConnected(t *testing.T) bool { client, err := docker.NewClientFromEnv() @@ -96,23 +90,22 @@ func dockerSetup(t *testing.T, task *structs.Task) (*docker.Client, DriverHandle t.Fatalf("Failed to initialize client: %s\nStack\n%s", err, debug.Stack()) } - driverCtx := testDockerDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) + driverCtx, execCtx := testDriverContexts(task) driver := NewDockerDriver(driverCtx) - handle, err := driver.Start(ctx, task) + handle, err := driver.Start(execCtx, task) if err != nil { - ctx.AllocDir.Destroy() + execCtx.AllocDir.Destroy() t.Fatalf("Failed to start driver: %s\nStack\n%s", err, debug.Stack()) } if handle == nil { - ctx.AllocDir.Destroy() + execCtx.AllocDir.Destroy() t.Fatalf("handle is nil\nStack\n%s", debug.Stack()) } cleanup := func() { handle.Kill() - ctx.AllocDir.Destroy() + execCtx.AllocDir.Destroy() } return client, handle, cleanup @@ -138,7 +131,8 @@ func TestDockerDriver_Handle(t *testing.T) { // This test should always pass, even if docker daemon is not available func TestDockerDriver_Fingerprint(t *testing.T) { t.Parallel() - d := NewDockerDriver(testDockerDriverContext("")) + driverCtx, _ := testDriverContexts(&structs.Task{Name: "foo"}) + d := NewDockerDriver(driverCtx) node := &structs.Node{ Attributes: make(map[string]string), } @@ -169,12 +163,11 @@ func TestDockerDriver_StartOpen_Wait(t *testing.T) { Resources: basicResources, } - driverCtx := testDockerDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewDockerDriver(driverCtx) - handle, err := d.Start(ctx, task) + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } @@ -184,7 +177,7 @@ func TestDockerDriver_StartOpen_Wait(t *testing.T) { defer handle.Kill() // Attempt to open - handle2, err := d.Open(ctx, handle.ID()) + handle2, err := d.Open(execCtx, handle.ID()) if err != nil { t.Fatalf("err: %v", err) } @@ -246,7 +239,7 @@ func TestDockerDriver_Start_Wait_AllocDir(t *testing.T) { "args": []string{ "-c", fmt.Sprintf(`sleep 1; echo -n %s > $%s/%s`, - string(exp), environment.AllocDir, file), + string(exp), env.AllocDir, file), }, }, Resources: &structs.Resources{ @@ -255,12 +248,11 @@ func TestDockerDriver_Start_Wait_AllocDir(t *testing.T) { }, } - driverCtx := testDockerDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewDockerDriver(driverCtx) - handle, err := d.Start(ctx, task) + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } @@ -279,7 +271,7 @@ func TestDockerDriver_Start_Wait_AllocDir(t *testing.T) { } // Check that data was written to the shared alloc directory. - outputFile := filepath.Join(ctx.AllocDir.SharedDir, file) + outputFile := filepath.Join(execCtx.AllocDir.SharedDir, file) act, err := ioutil.ReadFile(outputFile) if err != nil { t.Fatalf("Couldn't read expected output: %v", err) @@ -350,12 +342,11 @@ func TestDocker_StartN(t *testing.T) { // Let's spin up a bunch of things var err error for idx, task := range taskList { - driverCtx := testDockerDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewDockerDriver(driverCtx) - handles[idx], err = d.Start(ctx, task) + handles[idx], err = d.Start(execCtx, task) if err != nil { t.Errorf("Failed starting task #%d: %s", idx+1, err) } @@ -408,12 +399,11 @@ func TestDocker_StartNVersions(t *testing.T) { // Let's spin up a bunch of things var err error for idx, task := range taskList { - driverCtx := testDockerDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewDockerDriver(driverCtx) - handles[idx], err = d.Start(ctx, task) + handles[idx], err = d.Start(execCtx, task) if err != nil { t.Errorf("Failed starting task #%d: %s", idx+1, err) } diff --git a/client/driver/driver_test.go b/client/driver/driver_test.go index b5df412ca69..02b74f83574 100644 --- a/client/driver/driver_test.go +++ b/client/driver/driver_test.go @@ -49,23 +49,26 @@ func testConfig() *config.Config { return conf } -func testDriverContext(task string) *DriverContext { +func testDriverContexts(task *structs.Task) (*DriverContext, *ExecContext) { cfg := testConfig() - return NewDriverContext(task, cfg, cfg.Node, testLogger()) -} - -func testDriverExecContext(task *structs.Task, driverCtx *DriverContext) *ExecContext { - allocDir := allocdir.NewAllocDir(filepath.Join(driverCtx.config.AllocDir, structs.GenerateUUID())) + allocDir := allocdir.NewAllocDir(filepath.Join(cfg.AllocDir, structs.GenerateUUID())) allocDir.Build([]*structs.Task{task}) - ctx := NewExecContext(allocDir, fmt.Sprintf("alloc-id-%d", int(rand.Int31()))) - return ctx + execCtx := NewExecContext(allocDir, fmt.Sprintf("alloc-id-%d", int(rand.Int31()))) + + taskEnv, err := GetTaskEnv(allocDir, cfg.Node, task) + if err != nil { + return nil, nil + } + + driverCtx := NewDriverContext(task.Name, cfg, cfg.Node, testLogger(), taskEnv) + return driverCtx, execCtx } func TestDriver_KillTimeout(t *testing.T) { - ctx := testDriverContext("foo") - ctx.config.MaxKillTimeout = 10 * time.Second expected := 1 * time.Second - task := &structs.Task{KillTimeout: expected} + task := &structs.Task{Name: "foo", KillTimeout: expected} + ctx, _ := testDriverContexts(task) + ctx.config.MaxKillTimeout = 10 * time.Second if actual := ctx.KillTimeout(task); expected != actual { t.Fatalf("KillTimeout(%v) returned %v; want %v", task, actual, expected) @@ -79,7 +82,7 @@ func TestDriver_KillTimeout(t *testing.T) { } } -func TestDriver_TaskEnvironmentVariables(t *testing.T) { +func TestDriver_GetTaskEnv(t *testing.T) { t.Parallel() task := &structs.Task{ Env: map[string]string{ @@ -123,7 +126,7 @@ func TestDriver_TaskEnvironmentVariables(t *testing.T) { "lorem": "ipsum", } - act := env.Map() + act := env.EnvMap() if !reflect.DeepEqual(act, exp) { t.Fatalf("GetTaskEnv() returned %#v; want %#v", act, exp) } diff --git a/client/driver/exec_test.go b/client/driver/exec_test.go index 66795781fee..5b5c1de2def 100644 --- a/client/driver/exec_test.go +++ b/client/driver/exec_test.go @@ -9,7 +9,7 @@ import ( "time" "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/client/driver/environment" + "github.com/hashicorp/nomad/client/driver/env" "github.com/hashicorp/nomad/nomad/structs" ctestutils "github.com/hashicorp/nomad/client/testutil" @@ -18,7 +18,8 @@ import ( func TestExecDriver_Fingerprint(t *testing.T) { t.Parallel() ctestutils.ExecCompatible(t) - d := NewExecDriver(testDriverContext("")) + driverCtx, _ := testDriverContexts(&structs.Task{Name: "foo"}) + d := NewExecDriver(driverCtx) node := &structs.Node{ Attributes: make(map[string]string), } @@ -46,12 +47,11 @@ func TestExecDriver_StartOpen_Wait(t *testing.T) { Resources: basicResources, } - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewExecDriver(driverCtx) - handle, err := d.Start(ctx, task) + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } @@ -60,7 +60,7 @@ func TestExecDriver_StartOpen_Wait(t *testing.T) { } // Attempt to open - handle2, err := d.Open(ctx, handle.ID()) + handle2, err := d.Open(execCtx, handle.ID()) if err != nil { t.Fatalf("err: %v", err) } @@ -81,12 +81,11 @@ func TestExecDriver_Start_Wait(t *testing.T) { Resources: basicResources, } - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewExecDriver(driverCtx) - handle, err := d.Start(ctx, task) + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } @@ -126,12 +125,11 @@ func TestExecDriver_Start_Artifact_basic(t *testing.T) { Resources: basicResources, } - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewExecDriver(driverCtx) - handle, err := d.Start(ctx, task) + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } @@ -174,12 +172,11 @@ func TestExecDriver_Start_Artifact_expanded(t *testing.T) { Resources: basicResources, } - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewExecDriver(driverCtx) - handle, err := d.Start(ctx, task) + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } @@ -215,18 +212,17 @@ func TestExecDriver_Start_Wait_AllocDir(t *testing.T) { "command": "/bin/bash", "args": []string{ "-c", - fmt.Sprintf(`sleep 1; echo -n %s > $%s/%s`, string(exp), environment.AllocDir, file), + fmt.Sprintf(`sleep 1; echo -n %s > $%s/%s`, string(exp), env.AllocDir, file), }, }, Resources: basicResources, } - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewExecDriver(driverCtx) - handle, err := d.Start(ctx, task) + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } @@ -245,7 +241,7 @@ func TestExecDriver_Start_Wait_AllocDir(t *testing.T) { } // Check that data was written to the shared alloc directory. - outputFile := filepath.Join(ctx.AllocDir.SharedDir, file) + outputFile := filepath.Join(execCtx.AllocDir.SharedDir, file) act, err := ioutil.ReadFile(outputFile) if err != nil { t.Fatalf("Couldn't read expected output: %v", err) @@ -268,12 +264,11 @@ func TestExecDriver_Start_Kill_Wait(t *testing.T) { Resources: basicResources, } - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewExecDriver(driverCtx) - handle, err := d.Start(ctx, task) + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } diff --git a/client/driver/executor/exec_linux.go b/client/driver/executor/exec_linux.go index e9ceca710cd..43a7730a4a1 100644 --- a/client/driver/executor/exec_linux.go +++ b/client/driver/executor/exec_linux.go @@ -20,7 +20,6 @@ import ( cgroupConfig "github.com/opencontainers/runc/libcontainer/configs" "github.com/hashicorp/nomad/client/allocdir" - "github.com/hashicorp/nomad/client/driver/environment" "github.com/hashicorp/nomad/client/driver/spawn" cstructs "github.com/hashicorp/nomad/client/driver/structs" "github.com/hashicorp/nomad/nomad/structs" @@ -161,8 +160,9 @@ func (e *LinuxExecutor) Start() error { // Parse the commands arguments and replace instances of Nomad environment // variables. - e.cmd.Path = e.taskEnv.ReplaceEnv(e.cmd.Path, envVars.Map()) - e.cmd.Args = e.taskEnv.ParseAndReplace(e.cmd.Args, envVars.Map()) + e.cmd.Path = e.taskEnv.ReplaceEnv(e.cmd.Path) + e.cmd.Args = e.taskEnv.ParseAndReplace(e.cmd.Args) + e.cmd.Env = e.taskEnv.EnvList() spawnState := filepath.Join(e.allocDir, fmt.Sprintf("%s_%s", e.taskName, "exit_status")) e.spawn = spawn.NewSpawner(spawnState) @@ -283,14 +283,7 @@ func (e *LinuxExecutor) ConfigureTaskDir(taskName string, alloc *allocdir.AllocD } // Set the tasks AllocDir environment variable. - env, err := environment.ParseFromList(e.cmd.Env) - if err != nil { - return err - } - env.SetAllocDir(filepath.Join("/", allocdir.SharedAllocName)) - env.SetTaskLocalDir(filepath.Join("/", allocdir.TaskLocal)) - e.cmd.Env = env.List() - + e.taskEnv.SetAllocDir(filepath.Join("/", allocdir.SharedAllocName)).SetTaskLocalDir(filepath.Join("/", allocdir.TaskLocal)).Build() return nil } diff --git a/client/driver/executor/test_harness_test.go b/client/driver/executor/test_harness_test.go index 8e156543ddc..e31cc422c33 100644 --- a/client/driver/executor/test_harness_test.go +++ b/client/driver/executor/test_harness_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/hashicorp/nomad/client/allocdir" + "github.com/hashicorp/nomad/client/driver/env" "github.com/hashicorp/nomad/helper/testtask" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" @@ -45,13 +46,18 @@ func mockAllocDir(t *testing.T) (string, *allocdir.AllocDir) { return task.Name, allocDir } -func testExecutor(t *testing.T, buildExecutor func() Executor, compatible func(*testing.T)) { +func testExecutorContext() *ExecutorContext { + taskEnv := env.NewTaskEnvironment(mock.Node()) + return &ExecutorContext{taskEnv: taskEnv} +} + +func testExecutor(t *testing.T, buildExecutor func(*ExecutorContext) Executor, compatible func(*testing.T)) { if compatible != nil { compatible(t) } command := func(name string, args ...string) Executor { - e := buildExecutor() + e := buildExecutor(testExecutorContext()) SetCommand(e, name, args) testtask.SetCmdEnv(e.Command()) return e @@ -185,7 +191,7 @@ func Executor_Start_Kill(t *testing.T, command buildExecCommand) { } } -func Executor_Open(t *testing.T, command buildExecCommand, newExecutor func() Executor) { +func Executor_Open(t *testing.T, command buildExecCommand, newExecutor func(*ExecutorContext) Executor) { task, alloc := mockAllocDir(t) defer alloc.Destroy() @@ -216,7 +222,7 @@ func Executor_Open(t *testing.T, command buildExecCommand, newExecutor func() Ex log.Panicf("ID() failed: %v", err) } - e2 := newExecutor() + e2 := newExecutor(testExecutorContext()) if err := e2.Open(id); err != nil { log.Panicf("Open(%v) failed: %v", id, err) } @@ -236,7 +242,7 @@ func Executor_Open(t *testing.T, command buildExecCommand, newExecutor func() Ex } } -func Executor_Open_Invalid(t *testing.T, command buildExecCommand, newExecutor func() Executor) { +func Executor_Open_Invalid(t *testing.T, command buildExecCommand, newExecutor func(*ExecutorContext) Executor) { task, alloc := mockAllocDir(t) e := command(testtask.Path(), "echo", "foo") @@ -271,7 +277,7 @@ func Executor_Open_Invalid(t *testing.T, command buildExecCommand, newExecutor f log.Panicf("alloc.Destroy() failed: %v", err) } - e2 := newExecutor() + e2 := newExecutor(testExecutorContext()) if err := e2.Open(id); err == nil { log.Panicf("Open(%v) should have failed", id) } diff --git a/client/driver/java_test.go b/client/driver/java_test.go index 5fa47320b4b..b06aaa56164 100644 --- a/client/driver/java_test.go +++ b/client/driver/java_test.go @@ -21,7 +21,8 @@ func javaLocated() bool { func TestJavaDriver_Fingerprint(t *testing.T) { t.Parallel() ctestutils.JavaCompatible(t) - d := NewJavaDriver(testDriverContext("")) + driverCtx, _ := testDriverContexts(&structs.Task{Name: "foo"}) + d := NewJavaDriver(driverCtx) node := &structs.Node{ Attributes: make(map[string]string), } @@ -59,12 +60,11 @@ func TestJavaDriver_StartOpen_Wait(t *testing.T) { Resources: basicResources, } - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewJavaDriver(driverCtx) - handle, err := d.Start(ctx, task) + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } @@ -73,7 +73,7 @@ func TestJavaDriver_StartOpen_Wait(t *testing.T) { } // Attempt to open - handle2, err := d.Open(ctx, handle.ID()) + handle2, err := d.Open(execCtx, handle.ID()) if err != nil { t.Fatalf("err: %v", err) } @@ -105,12 +105,11 @@ func TestJavaDriver_Start_Wait(t *testing.T) { Resources: basicResources, } - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewJavaDriver(driverCtx) - handle, err := d.Start(ctx, task) + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } @@ -151,12 +150,11 @@ func TestJavaDriver_Start_Kill_Wait(t *testing.T) { Resources: basicResources, } - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewJavaDriver(driverCtx) - handle, err := d.Start(ctx, task) + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } diff --git a/client/driver/qemu_test.go b/client/driver/qemu_test.go index 65846203bc6..151fcf7f955 100644 --- a/client/driver/qemu_test.go +++ b/client/driver/qemu_test.go @@ -14,7 +14,8 @@ import ( func TestQemuDriver_Fingerprint(t *testing.T) { t.Parallel() ctestutils.QemuCompatible(t) - d := NewQemuDriver(testDriverContext("")) + driverCtx, _ := testDriverContexts(&structs.Task{Name: "foo"}) + d := NewQemuDriver(driverCtx) node := &structs.Node{ Attributes: make(map[string]string), } @@ -59,12 +60,11 @@ func TestQemuDriver_StartOpen_Wait(t *testing.T) { }, } - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewQemuDriver(driverCtx) - handle, err := d.Start(ctx, task) + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } @@ -73,7 +73,7 @@ func TestQemuDriver_StartOpen_Wait(t *testing.T) { } // Attempt to open - handle2, err := d.Open(ctx, handle.ID()) + handle2, err := d.Open(execCtx, handle.ID()) if err != nil { t.Fatalf("err: %v", err) } @@ -103,12 +103,11 @@ func TestQemuDriver_RequiresMemory(t *testing.T) { }, } - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewQemuDriver(driverCtx) - _, err := d.Start(ctx, task) + _, err := d.Start(execCtx, task) if err == nil { t.Fatalf("Expected error when not specifying memory") } diff --git a/client/driver/raw_exec_test.go b/client/driver/raw_exec_test.go index 8d656c9b085..a2d3a86345d 100644 --- a/client/driver/raw_exec_test.go +++ b/client/driver/raw_exec_test.go @@ -11,14 +11,15 @@ import ( "time" "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/client/driver/environment" + "github.com/hashicorp/nomad/client/driver/env" "github.com/hashicorp/nomad/helper/testtask" "github.com/hashicorp/nomad/nomad/structs" ) func TestRawExecDriver_Fingerprint(t *testing.T) { t.Parallel() - d := NewRawExecDriver(testDriverContext("")) + driverCtx, _ := testDriverContexts(&structs.Task{Name: "foo"}) + d := NewRawExecDriver(driverCtx) node := &structs.Node{ Attributes: make(map[string]string), } @@ -62,12 +63,11 @@ func TestRawExecDriver_StartOpen_Wait(t *testing.T) { Resources: basicResources, } testtask.SetTaskEnv(task) - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() - + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewRawExecDriver(driverCtx) - handle, err := d.Start(ctx, task) + + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } @@ -76,7 +76,7 @@ func TestRawExecDriver_StartOpen_Wait(t *testing.T) { } // Attempt to open - handle2, err := d.Open(ctx, handle.ID()) + handle2, err := d.Open(execCtx, handle.ID()) if err != nil { t.Fatalf("err: %v", err) } @@ -110,12 +110,11 @@ func TestRawExecDriver_Start_Artifact_basic(t *testing.T) { } testtask.SetTaskEnv(task) - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() - + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewRawExecDriver(driverCtx) - handle, err := d.Start(ctx, task) + + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } @@ -124,7 +123,7 @@ func TestRawExecDriver_Start_Artifact_basic(t *testing.T) { } // Attempt to open - handle2, err := d.Open(ctx, handle.ID()) + handle2, err := d.Open(execCtx, handle.ID()) if err != nil { t.Fatalf("err: %v", err) } @@ -158,12 +157,11 @@ func TestRawExecDriver_Start_Artifact_expanded(t *testing.T) { } testtask.SetTaskEnv(task) - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() - + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewRawExecDriver(driverCtx) - handle, err := d.Start(ctx, task) + + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } @@ -172,7 +170,7 @@ func TestRawExecDriver_Start_Artifact_expanded(t *testing.T) { } // Attempt to open - handle2, err := d.Open(ctx, handle.ID()) + handle2, err := d.Open(execCtx, handle.ID()) if err != nil { t.Fatalf("err: %v", err) } @@ -199,13 +197,11 @@ func TestRawExecDriver_Start_Wait(t *testing.T) { Resources: basicResources, } testtask.SetTaskEnv(task) - - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() - + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewRawExecDriver(driverCtx) - handle, err := d.Start(ctx, task) + + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } @@ -234,7 +230,7 @@ func TestRawExecDriver_Start_Wait_AllocDir(t *testing.T) { t.Parallel() exp := []byte{'w', 'i', 'n'} file := "output.txt" - outPath := fmt.Sprintf(`$%s/%s`, environment.AllocDir, file) + outPath := fmt.Sprintf(`$%s/%s`, env.AllocDir, file) task := &structs.Task{ Name: "sleep", Config: map[string]interface{}{ @@ -248,12 +244,11 @@ func TestRawExecDriver_Start_Wait_AllocDir(t *testing.T) { } testtask.SetTaskEnv(task) - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() - + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewRawExecDriver(driverCtx) - handle, err := d.Start(ctx, task) + + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } @@ -272,7 +267,7 @@ func TestRawExecDriver_Start_Wait_AllocDir(t *testing.T) { } // Check that data was written to the shared alloc directory. - outputFile := filepath.Join(ctx.AllocDir.SharedDir, file) + outputFile := filepath.Join(execCtx.AllocDir.SharedDir, file) act, err := ioutil.ReadFile(outputFile) if err != nil { t.Fatalf("Couldn't read expected output: %v", err) @@ -295,12 +290,11 @@ func TestRawExecDriver_Start_Kill_Wait(t *testing.T) { } testtask.SetTaskEnv(task) - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) - defer ctx.AllocDir.Destroy() - + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewRawExecDriver(driverCtx) - handle, err := d.Start(ctx, task) + + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } diff --git a/client/driver/rkt_test.go b/client/driver/rkt_test.go index 13429c3cbca..95a8e5f7132 100644 --- a/client/driver/rkt_test.go +++ b/client/driver/rkt_test.go @@ -50,7 +50,8 @@ func TestRktDriver_Handle(t *testing.T) { // The fingerprinter test should always pass, even if rkt is not installed. func TestRktDriver_Fingerprint(t *testing.T) { ctestutils.RktCompatible(t) - d := NewRktDriver(testDriverContext("")) + driverCtx, _ := testDriverContexts(&structs.Task{Name: "foo"}) + d := NewRktDriver(driverCtx) node := &structs.Node{ Attributes: make(map[string]string), } @@ -88,12 +89,11 @@ func TestRktDriver_Start(t *testing.T) { }, } - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewRktDriver(driverCtx) - defer ctx.AllocDir.Destroy() - handle, err := d.Start(ctx, task) + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } @@ -102,7 +102,7 @@ func TestRktDriver_Start(t *testing.T) { } // Attempt to open - handle2, err := d.Open(ctx, handle.ID()) + handle2, err := d.Open(execCtx, handle.ID()) if err != nil { t.Fatalf("err: %v", err) } @@ -132,12 +132,11 @@ func TestRktDriver_Start_Wait(t *testing.T) { }, } - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewRktDriver(driverCtx) - defer ctx.AllocDir.Destroy() - handle, err := d.Start(ctx, task) + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } @@ -177,12 +176,11 @@ func TestRktDriver_Start_Wait_Skip_Trust(t *testing.T) { }, } - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewRktDriver(driverCtx) - defer ctx.AllocDir.Destroy() - handle, err := d.Start(ctx, task) + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } @@ -223,12 +221,11 @@ func TestRktDriver_Start_Wait_Logs(t *testing.T) { }, } - driverCtx := testDriverContext(task.Name) - ctx := testDriverExecContext(task, driverCtx) + driverCtx, execCtx := testDriverContexts(task) + defer execCtx.AllocDir.Destroy() d := NewRktDriver(driverCtx) - defer ctx.AllocDir.Destroy() - handle, err := d.Start(ctx, task) + handle, err := d.Start(execCtx, task) if err != nil { t.Fatalf("err: %v", err) } @@ -246,7 +243,7 @@ func TestRktDriver_Start_Wait_Logs(t *testing.T) { t.Fatalf("timeout") } - taskDir, ok := ctx.AllocDir.TaskDirs[task.Name] + taskDir, ok := execCtx.AllocDir.TaskDirs[task.Name] if !ok { t.Fatalf("Could not find task directory for task: %v", task) } From 9c6fcdc1d72888514c52d662f980b1718a645cd0 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Mon, 11 Jan 2016 11:12:09 -0800 Subject: [PATCH 4/4] Documentation --- website/source/docs/drivers/docker.html.md | 11 +++++++-- website/source/docs/drivers/exec.html.md | 9 +++++++- website/source/docs/drivers/java.html.md | 9 +++++++- website/source/docs/drivers/raw_exec.html.md | 9 +++++++- website/source/docs/drivers/rkt.html.md | 9 +++++++- website/source/docs/jobspec/index.html.md | 24 ++++++++++++++++---- 6 files changed, 61 insertions(+), 10 deletions(-) diff --git a/website/source/docs/drivers/docker.html.md b/website/source/docs/drivers/docker.html.md index cb2304b6408..5cf65d79c3f 100644 --- a/website/source/docs/drivers/docker.html.md +++ b/website/source/docs/drivers/docker.html.md @@ -37,8 +37,15 @@ The following options are available for use in the job specification. * `command` - (Optional) The command to run when starting the container. -* `args` - (Optional) A list of arguments to the optional `command`. If no - `command` is present, `args` are ignored. +* `args` - (Optional) A list of arguments to the optional `command`. If no + `command` is present, `args` are ignored. References to environment variables + or any [intepretable Nomad + variables](/docs/jobspec/index.html#interpreted_vars) will be interpreted + before launching the task. For example: + + ``` + args = ["$nomad.ip", "$MY_ENV", $meta.foo"] + ``` * `labels` - (Optional) A key/value map of labels to set to the containers on start. diff --git a/website/source/docs/drivers/exec.html.md b/website/source/docs/drivers/exec.html.md index 3d921aa6d73..ce141346150 100644 --- a/website/source/docs/drivers/exec.html.md +++ b/website/source/docs/drivers/exec.html.md @@ -32,7 +32,14 @@ The `exec` driver supports the following configuration in the job spec: is supplied and does not match the downloaded artifact, the driver will fail to start -* `args` - (Optional) A list of arguments to the `command`. +* `args` - (Optional) A list of arguments to the optional `command`. + References to environment variables or any [intepretable Nomad + variables](/docs/jobspec/index.html#interpreted_vars) will be interpreted + before launching the task. For example: + + ``` + args = ["$nomad.ip", "$MY_ENV", $meta.foo"] + ``` ## Client Requirements diff --git a/website/source/docs/drivers/java.html.md b/website/source/docs/drivers/java.html.md index 45baacb72ac..57d67a0bf08 100644 --- a/website/source/docs/drivers/java.html.md +++ b/website/source/docs/drivers/java.html.md @@ -27,7 +27,14 @@ The `java` driver supports the following configuration in the job spec: is supplied and does not match the downloaded artifact, the driver will fail to start -* `args` - (Optional) A list of arguments to the `java` command. +* `args` - (Optional) A list of arguments to the optional `command`. + References to environment variables or any [intepretable Nomad + variables](/docs/jobspec/index.html#interpreted_vars) will be interpreted + before launching the task. For example: + + ``` + args = ["$nomad.ip", "$MY_ENV", $meta.foo"] + ``` * `jvm_options` - (Optional) A list of JVM options to be passed while invoking java. These options are passed not validated in any way in Nomad. diff --git a/website/source/docs/drivers/raw_exec.html.md b/website/source/docs/drivers/raw_exec.html.md index cfca5e4f060..fc0fb48dc79 100644 --- a/website/source/docs/drivers/raw_exec.html.md +++ b/website/source/docs/drivers/raw_exec.html.md @@ -30,7 +30,14 @@ The `raw_exec` driver supports the following configuration in the job spec: is supplied and does not match the downloaded artifact, the driver will fail to start -* `args` - (Optional) A list of arguments to the `command`. +* `args` - (Optional) A list of arguments to the optional `command`. + References to environment variables or any [intepretable Nomad + variables](/docs/jobspec/index.html#interpreted_vars) will be interpreted + before launching the task. For example: + + ``` + args = ["$nomad.ip", "$MY_ENV", $meta.foo"] + ``` ## Client Requirements diff --git a/website/source/docs/drivers/rkt.html.md b/website/source/docs/drivers/rkt.html.md index 5efab860b50..6225faf8706 100644 --- a/website/source/docs/drivers/rkt.html.md +++ b/website/source/docs/drivers/rkt.html.md @@ -24,7 +24,14 @@ The `rkt` driver supports the following configuration in the job spec: * `command` - (Optional) A command to execute on the ACI. -* `args` - (Optional) A list of arguments to the image. +* `args` - (Optional) A list of arguments to the optional `command`. + References to environment variables or any [intepretable Nomad + variables](/docs/jobspec/index.html#interpreted_vars) will be interpreted + before launching the task. For example: + + ``` + args = ["$nomad.ip", "$MY_ENV", $meta.foo"] + ``` * `trust_prefix` - (Optional) The trust prefix to be passed to rkt. Must be reachable from the box running the nomad agent. If not specified, the image is diff --git a/website/source/docs/jobspec/index.html.md b/website/source/docs/jobspec/index.html.md index 9e3a0c59907..0923d325530 100644 --- a/website/source/docs/jobspec/index.html.md +++ b/website/source/docs/jobspec/index.html.md @@ -240,8 +240,20 @@ The `task` object supports the following keys: task transitons to the dead state. [Click here](/docs/jobspec/servicediscovery.html) to learn more about services. -* `env` - A map of key/value representing environment variables that - will be passed along to the running process. +* `env` - A map of key/value representing environment variables that + will be passed along to the running process. Nomad variables are + interpreted when set in the environment variable values. See the table of + interpreted variables [here](#interpreted_vars). + + For example the below environment map will be reinterpreted: + + ``` + env { + // The value will be interpreted by the client and set to the + // correct value. + NODE_CLASS = "$nomad.class" + } + ``` * `resources` - Provides the resource requirements of the task. See the resources reference for more details. @@ -336,7 +348,7 @@ restart { The `constraint` object supports the following keys: * `attribute` - Specifies the attribute to examine for the - constraint. See the table of attributes below. + constraint. See the table of attributes [below](#interpreted_vars). * `operator` - Specifies the comparison operator. Defaults to equality, and can be `=`, `==`, `is`, `!=`, `not`, `>`, `>=`, `<`, `<=`. The @@ -368,7 +380,11 @@ The `constraint` object supports the following keys: Tasks within a task group are always co-scheduled. -Below is a table documenting the variables that can be interpreted: +### Interpreted Variables + +Certain Nomad variables are interpretable for use in constraints, task +environment variables and task arguments. Below is a table documenting the +variables that can be interpreted: