diff --git a/client/alloc_runner_test.go b/client/alloc_runner_test.go index 15077c76c..bc4a7aa4f 100644 --- a/client/alloc_runner_test.go +++ b/client/alloc_runner_test.go @@ -65,7 +65,7 @@ func TestAllocRunner_Destroy(t *testing.T) { // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] task.Config["command"] = "/bin/sleep" - task.Config["args"] = "10" + task.Config["args"] = []string{"10"} go ar.Run() start := time.Now() @@ -97,7 +97,7 @@ func TestAllocRunner_Update(t *testing.T) { // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] task.Config["command"] = "/bin/sleep" - task.Config["args"] = "10" + task.Config["args"] = []string{"10"} go ar.Run() defer ar.Destroy() start := time.Now() @@ -130,7 +130,7 @@ func TestAllocRunner_SaveRestoreState(t *testing.T) { // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] task.Config["command"] = "/bin/sleep" - task.Config["args"] = "10" + task.Config["args"] = []string{"10"} go ar.Run() defer ar.Destroy() diff --git a/client/client_test.go b/client/client_test.go index 90fecad6b..935c63dea 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -336,7 +336,7 @@ func TestClient_SaveRestoreState(t *testing.T) { alloc1.NodeID = c1.Node().ID task := alloc1.Job.TaskGroups[0].Tasks[0] task.Config["command"] = "/bin/sleep" - task.Config["args"] = "10" + task.Config["args"] = []string{"10"} state := s1.State() err := state.UpsertAllocs(100, diff --git a/client/driver/args/args.go b/client/driver/args/args.go index 51793bd8b..9e8a9980c 100644 --- a/client/driver/args/args.go +++ b/client/driver/args/args.go @@ -1,11 +1,6 @@ package args -import ( - "fmt" - "regexp" - - "github.com/mattn/go-shellwords" -) +import "regexp" var ( envRe = regexp.MustCompile(`\$({[a-zA-Z0-9_]+}|[a-zA-Z0-9_]+)`) @@ -13,27 +8,17 @@ var ( // 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 and does correct splitting of the arg list. -func ParseAndReplace(args string, env map[string]string) ([]string, error) { - // Set up parser. - p := shellwords.NewParser() - p.ParseEnv = false - p.ParseBacktick = false - - parsed, err := p.Parse(args) - if err != nil { - return nil, fmt.Errorf("Couldn't parse args %v: %v", args, err) - } - - replaced := make([]string, len(parsed)) - for i, arg := range parsed { +// 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, nil + return replaced } -// replaceEnv takes an arg and replaces all occurences of environment variables. +// 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 { diff --git a/client/driver/args/args_test.go b/client/driver/args/args_test.go index 4e1d88b59..5e7cbca4a 100644 --- a/client/driver/args/args_test.go +++ b/client/driver/args/args_test.go @@ -21,12 +21,9 @@ var ( ) func TestDriverArgs_ParseAndReplaceInvalidEnv(t *testing.T) { - input := "invalid $FOO" + input := []string{"invalid", "$FOO"} exp := []string{"invalid", "$FOO"} - act, err := ParseAndReplace(input, envVars) - if err != nil { - t.Fatalf("Failed to parse valid input args %v: %v", input, err) - } + act := ParseAndReplace(input, envVars) if !reflect.DeepEqual(act, exp) { t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) @@ -34,12 +31,9 @@ func TestDriverArgs_ParseAndReplaceInvalidEnv(t *testing.T) { } func TestDriverArgs_ParseAndReplaceValidEnv(t *testing.T) { - input := fmt.Sprintf("nomad_ip \\\"$%v\\\"!", ipKey) + input := []string{"nomad_ip", fmt.Sprintf(`"$%v"!`, ipKey)} exp := []string{"nomad_ip", fmt.Sprintf("\"%s\"!", ipVal)} - act, err := ParseAndReplace(input, envVars) - if err != nil { - t.Fatalf("Failed to parse valid input args %v: %v", input, err) - } + act := ParseAndReplace(input, envVars) if !reflect.DeepEqual(act, exp) { t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) @@ -47,32 +41,9 @@ func TestDriverArgs_ParseAndReplaceValidEnv(t *testing.T) { } func TestDriverArgs_ParseAndReplaceChainedEnv(t *testing.T) { - input := fmt.Sprintf("-foo $%s$%s", ipKey, portKey) + input := []string{"-foo", fmt.Sprintf("$%s$%s", ipKey, portKey)} exp := []string{"-foo", fmt.Sprintf("%s%s", ipVal, portVal)} - act, err := ParseAndReplace(input, envVars) - if err != nil { - t.Fatalf("Failed to parse valid input args %v: %v", input, err) - } - - if !reflect.DeepEqual(act, exp) { - t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) - } -} - -func TestDriverArgs_ParseAndReplaceInvalidArgEscape(t *testing.T) { - input := "-c \"echo \"foo\\\" > bar.txt\"" - if _, err := ParseAndReplace(input, envVars); err == nil { - t.Fatalf("ParseAndReplace(%v, %v) should have failed", input, envVars) - } -} - -func TestDriverArgs_ParseAndReplaceValidArgEscape(t *testing.T) { - input := "-c \"echo \\\"foo\\\" > bar.txt\"" - exp := []string{"-c", "echo \"foo\" > bar.txt"} - act, err := ParseAndReplace(input, envVars) - if err != nil { - t.Fatalf("Failed to parse valid input args %v: %v", input, err) - } + act := ParseAndReplace(input, envVars) if !reflect.DeepEqual(act, exp) { t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) diff --git a/client/driver/docker.go b/client/driver/docker.go index 6fa1af600..4aa97e13b 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -35,7 +35,7 @@ type DockerDriverAuth struct { type DockerDriverConfig struct { ImageName string `mapstructure:"image"` // Container's Image Name Command string `mapstructure:"command"` // The Command/Entrypoint to run when the container starts up - Args string `mapstructure:"args"` // The arguments to the Command/Entrypoint + Args []string `mapstructure:"args"` // The arguments to the Command/Entrypoint NetworkMode string `mapstructure:"network_mode"` // The network mode of the container - host, net and none PortMap []map[string]int `mapstructure:"port_map"` // A map of host port labels and the ports exposed on the container Privileged bool `mapstructure:"privileged"` // Flag to run the container in priviledged mode @@ -293,21 +293,18 @@ func (d *DockerDriver) createContainer(ctx *ExecContext, task *structs.Task, dri config.ExposedPorts = exposedPorts } - parsedArgs, err := args.ParseAndReplace(driverConfig.Args, env.Map()) - if err != nil { - return c, err - } + parsedArgs := args.ParseAndReplace(driverConfig.Args, env.Map()) // If the user specified a custom command to run as their entrypoint, we'll // inject it here. if driverConfig.Command != "" { cmd := []string{driverConfig.Command} - if driverConfig.Args != "" { + if len(driverConfig.Args) != 0 { cmd = append(cmd, parsedArgs...) } d.logger.Printf("[DEBUG] driver.docker: setting container startup command to: %s", strings.Join(cmd, " ")) config.Cmd = cmd - } else if driverConfig.Args != "" { + } else if len(driverConfig.Args) != 0 { d.logger.Println("[DEBUG] driver.docker: ignoring command arguments because command is not specified") } @@ -431,7 +428,7 @@ func (d *DockerDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle if len(containers) != 1 { log.Printf("[ERR] driver.docker: failed to get id for container %s", config.Name) - return nil, fmt.Errorf("Failed to get id for container %s: %s", config.Name, err) + return nil, fmt.Errorf("Failed to get id for container %s", config.Name) } log.Printf("[INFO] driver.docker: a container with the name %s already exists; will attempt to purge and re-create", config.Name) diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 4c9300579..a61de381c 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -137,7 +137,7 @@ func TestDockerDriver_Start_Wait(t *testing.T) { Config: map[string]interface{}{ "image": "redis", "command": "redis-server", - "args": "-v", + "args": []string{"-v"}, }, Resources: &structs.Resources{ MemoryMB: 256, @@ -190,7 +190,11 @@ func TestDockerDriver_Start_Wait_AllocDir(t *testing.T) { Config: map[string]interface{}{ "image": "redis", "command": "/bin/bash", - "args": fmt.Sprintf(`-c "sleep 1; echo -n %s > $%s/%s"`, string(exp), environment.AllocDir, file), + "args": []string{ + "-c", + fmt.Sprintf(`sleep 1; echo -n %s > $%s/%s`, + string(exp), environment.AllocDir, file), + }, }, Resources: &structs.Resources{ MemoryMB: 256, @@ -243,7 +247,7 @@ func TestDockerDriver_Start_Kill_Wait(t *testing.T) { Config: map[string]interface{}{ "image": "redis", "command": "/bin/sleep", - "args": "10", + "args": []string{"10"}, }, Resources: basicResources, } diff --git a/client/driver/exec.go b/client/driver/exec.go index 1f9e5b7d9..a0a136523 100644 --- a/client/driver/exec.go +++ b/client/driver/exec.go @@ -24,10 +24,10 @@ type ExecDriver struct { fingerprint.StaticFingerprinter } type ExecDriverConfig struct { - ArtifactSource string `mapstructure:"artifact_source"` - Checksum string `mapstructure:"checksum"` - Command string `mapstructure:"command"` - Args string `mapstructure:"args"` + ArtifactSource string `mapstructure:"artifact_source"` + Checksum string `mapstructure:"checksum"` + Command string `mapstructure:"command"` + Args []string `mapstructure:"args"` } // execHandle is returned from Start/Open as a handle to the PID @@ -91,14 +91,8 @@ func (d *ExecDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, // Get the environment variables. envVars := TaskEnvironmentVariables(ctx, task) - // Look for arguments - var args []string - if driverConfig.Args != "" { - args = append(args, driverConfig.Args) - } - // Setup the command - cmd := executor.Command(command, args...) + cmd := executor.Command(command, driverConfig.Args...) if err := cmd.Limit(task.Resources); err != nil { return nil, fmt.Errorf("failed to constrain resources: %s", err) } diff --git a/client/driver/exec_test.go b/client/driver/exec_test.go index 7e6554e1d..f5470074a 100644 --- a/client/driver/exec_test.go +++ b/client/driver/exec_test.go @@ -39,7 +39,7 @@ func TestExecDriver_StartOpen_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": "5", + "args": []string{"5"}, }, Resources: basicResources, } @@ -73,7 +73,7 @@ func TestExecDriver_Start_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": "2", + "args": []string{"2"}, }, Resources: basicResources, } @@ -161,7 +161,10 @@ func TestExecDriver_Start_Artifact_expanded(t *testing.T) { Config: map[string]interface{}{ "artifact_source": fmt.Sprintf("https://dl.dropboxusercontent.com/u/47675/jar_thing/%s", file), "command": "/bin/bash", - "args": fmt.Sprintf("-c '/bin/sleep 1 && %s'", filepath.Join("$NOMAD_TASK_DIR", file)), + "args": []string{ + "-c", + fmt.Sprintf(`/bin/sleep 1 && %s`, filepath.Join("$NOMAD_TASK_DIR", file)), + }, }, Resources: basicResources, } @@ -204,7 +207,10 @@ func TestExecDriver_Start_Wait_AllocDir(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/bash", - "args": fmt.Sprintf("-c \"sleep 1; echo -n %s > $%s/%s\"", string(exp), environment.AllocDir, file), + "args": []string{ + "-c", + fmt.Sprintf(`sleep 1; echo -n %s > $%s/%s`, string(exp), environment.AllocDir, file), + }, }, Resources: basicResources, } @@ -250,7 +256,7 @@ func TestExecDriver_Start_Kill_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": "1", + "args": []string{"1"}, }, Resources: basicResources, } diff --git a/client/driver/executor/exec_basic.go b/client/driver/executor/exec_basic.go index 438ae6b92..2ef4fbedc 100644 --- a/client/driver/executor/exec_basic.go +++ b/client/driver/executor/exec_basic.go @@ -29,7 +29,6 @@ type BasicExecutor struct { allocDir string } -// TODO: Have raw_exec use this as well. func NewBasicExecutor() Executor { return &BasicExecutor{} } @@ -63,12 +62,7 @@ func (e *BasicExecutor) Start() error { } e.cmd.Path = args.ReplaceEnv(e.cmd.Path, envVars.Map()) - combined := strings.Join(e.cmd.Args, " ") - parsed, err := args.ParseAndReplace(combined, envVars.Map()) - if err != nil { - return err - } - e.cmd.Args = parsed + e.cmd.Args = args.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_linux.go b/client/driver/executor/exec_linux.go index 14f4f809c..43c9270e0 100644 --- a/client/driver/executor/exec_linux.go +++ b/client/driver/executor/exec_linux.go @@ -167,12 +167,7 @@ func (e *LinuxExecutor) Start() error { } e.cmd.Path = args.ReplaceEnv(e.cmd.Path, envVars.Map()) - combined := strings.Join(e.cmd.Args, " ") - parsed, err := args.ParseAndReplace(combined, envVars.Map()) - if err != nil { - return err - } - e.cmd.Args = parsed + e.cmd.Args = args.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/test_harness_test.go b/client/driver/executor/test_harness_test.go index 10cbac371..b5413700c 100644 --- a/client/driver/executor/test_harness_test.go +++ b/client/driver/executor/test_harness_test.go @@ -112,7 +112,7 @@ func Executor_Start_Wait(t *testing.T, command buildExecCommand) { expected := "hello world" file := filepath.Join(allocdir.TaskLocal, "output.txt") absFilePath := filepath.Join(taskDir, file) - cmd := fmt.Sprintf(`"%v \"%v\" > %v"`, "/bin/sleep 1 ; echo -n", expected, file) + cmd := fmt.Sprintf(`/bin/sleep 1 ; echo -n %v > %v`, expected, file) e := command("/bin/bash", "-c", cmd) if err := e.Limit(constraint); err != nil { @@ -190,7 +190,7 @@ func Executor_Open(t *testing.T, command buildExecCommand, newExecutor func() Ex expected := "hello world" file := filepath.Join(allocdir.TaskLocal, "output.txt") absFilePath := filepath.Join(taskDir, file) - cmd := fmt.Sprintf(`"%v \"%v\" > %v"`, "/bin/sleep 1 ; echo -n", expected, file) + cmd := fmt.Sprintf(`/bin/sleep 1 ; echo -n %v > %v`, expected, file) e := command("/bin/bash", "-c", cmd) if err := e.Limit(constraint); err != nil { diff --git a/client/driver/java.go b/client/driver/java.go index eb2930a28..eb475db32 100644 --- a/client/driver/java.go +++ b/client/driver/java.go @@ -28,10 +28,10 @@ type JavaDriver struct { } type JavaDriverConfig struct { - JvmOpts string `mapstructure:"jvm_options"` - ArtifactSource string `mapstructure:"artifact_source"` - Checksum string `mapstructure:"checksum"` - Args string `mapstructure:"args"` + JvmOpts []string `mapstructure:"jvm_options"` + ArtifactSource string `mapstructure:"artifact_source"` + Checksum string `mapstructure:"checksum"` + Args []string `mapstructure:"args"` } // javaHandle is returned from Start/Open as a handle to the PID @@ -126,15 +126,15 @@ func (d *JavaDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, args := []string{} // Look for jvm options - if driverConfig.JvmOpts != "" { + if len(driverConfig.JvmOpts) != 0 { d.logger.Printf("[DEBUG] driver.java: found JVM options: %s", driverConfig.JvmOpts) - args = append(args, driverConfig.JvmOpts) + args = append(args, driverConfig.JvmOpts...) } // Build the argument list. args = append(args, "-jar", filepath.Join(allocdir.TaskLocal, jarName)) - if driverConfig.Args != "" { - args = append(args, driverConfig.Args) + if len(driverConfig.Args) != 0 { + args = append(args, driverConfig.Args...) } // Setup the command diff --git a/client/driver/java_test.go b/client/driver/java_test.go index a0c6d3b80..af6e44d6d 100644 --- a/client/driver/java_test.go +++ b/client/driver/java_test.go @@ -51,7 +51,7 @@ func TestJavaDriver_StartOpen_Wait(t *testing.T) { Name: "demo-app", Config: map[string]interface{}{ "artifact_source": "https://dl.dropboxusercontent.com/u/47675/jar_thing/demoapp.jar", - "jvm_options": "-Xmx2048m -Xms256m", + "jvm_options": []string{"-Xmx64m", "-Xms32m"}, "checksum": "sha256:58d6e8130308d32e197c5108edd4f56ddf1417408f743097c2e662df0f0b17c8", }, Resources: basicResources, @@ -97,7 +97,6 @@ func TestJavaDriver_Start_Wait(t *testing.T) { Name: "demo-app", Config: map[string]interface{}{ "artifact_source": "https://dl.dropboxusercontent.com/u/47675/jar_thing/demoapp.jar", - "jvm_options": "-Xmx2048m -Xms256m", "checksum": "sha256:58d6e8130308d32e197c5108edd4f56ddf1417408f743097c2e662df0f0b17c8", }, Resources: basicResources, diff --git a/client/driver/raw_exec.go b/client/driver/raw_exec.go index d5202fc39..5b1d98269 100644 --- a/client/driver/raw_exec.go +++ b/client/driver/raw_exec.go @@ -93,15 +93,9 @@ func (d *RawExecDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandl // Get the environment variables. envVars := TaskEnvironmentVariables(ctx, task) - // Look for arguments - var args []string - if driverConfig.Args != "" { - args = append(args, driverConfig.Args) - } - // Setup the command cmd := executor.NewBasicExecutor() - executor.SetCommand(cmd, command, args) + executor.SetCommand(cmd, command, driverConfig.Args) if err := cmd.Limit(task.Resources); err != nil { return nil, fmt.Errorf("failed to constrain resources: %s", err) } diff --git a/client/driver/raw_exec_test.go b/client/driver/raw_exec_test.go index 1b7a0c8db..8b6e0c649 100644 --- a/client/driver/raw_exec_test.go +++ b/client/driver/raw_exec_test.go @@ -53,7 +53,7 @@ func TestRawExecDriver_StartOpen_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": "1", + "args": []string{"1"}, }, Resources: basicResources, } @@ -151,7 +151,10 @@ func TestRawExecDriver_Start_Artifact_expanded(t *testing.T) { Config: map[string]interface{}{ "artifact_source": fmt.Sprintf("https://dl.dropboxusercontent.com/u/47675/jar_thing/%s", file), "command": "/bin/bash", - "args": fmt.Sprintf("-c '/bin/sleep 1 && %s'", filepath.Join("$NOMAD_TASK_DIR", file)), + "args": []string{ + "-c", + fmt.Sprintf(`'/bin/sleep 1 && %s'`, filepath.Join("$NOMAD_TASK_DIR", file)), + }, }, Resources: basicResources, } @@ -190,7 +193,7 @@ func TestRawExecDriver_Start_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": "1", + "args": []string{"1"}, }, Resources: basicResources, } @@ -232,7 +235,10 @@ func TestRawExecDriver_Start_Wait_AllocDir(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/bash", - "args": fmt.Sprintf(`-c "sleep 1; echo -n %s > $%s/%s"`, string(exp), environment.AllocDir, file), + "args": []string{ + "-c", + fmt.Sprintf(`sleep 1; echo -n %s > $%s/%s`, string(exp), environment.AllocDir, file), + }, }, Resources: basicResources, } @@ -277,7 +283,7 @@ func TestRawExecDriver_Start_Kill_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": "1", + "args": []string{"1"}, }, Resources: basicResources, } diff --git a/client/driver/rkt.go b/client/driver/rkt.go index d09eac1db..1fb6e9e06 100644 --- a/client/driver/rkt.go +++ b/client/driver/rkt.go @@ -37,8 +37,8 @@ type RktDriver struct { } type RktDriverConfig struct { - ImageName string `mapstructure:"image"` - Args string `mapstructure:"args"` + ImageName string `mapstructure:"image"` + Args []string `mapstructure:"args"` } // rktHandle is returned from Start/Open as a handle to the PID @@ -150,11 +150,8 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e } // Add user passed arguments. - if driverConfig.Args != "" { - parsed, err := args.ParseAndReplace(driverConfig.Args, envVars.Map()) - if err != nil { - return nil, err - } + if len(driverConfig.Args) != 0 { + parsed := args.ParseAndReplace(driverConfig.Args, envVars.Map()) // Need to start arguments with "--" if len(parsed) > 0 { diff --git a/client/driver/rkt_test.go b/client/driver/rkt_test.go index a6b3dfb78..15db27b27 100644 --- a/client/driver/rkt_test.go +++ b/client/driver/rkt_test.go @@ -119,7 +119,7 @@ func TestRktDriver_Start_Wait(t *testing.T) { "trust_prefix": "coreos.com/etcd", "image": "coreos.com/etcd:v2.0.4", "command": "/etcd", - "args": "--version", + "args": []string{"--version"}, }, } @@ -160,7 +160,7 @@ func TestRktDriver_Start_Wait_Skip_Trust(t *testing.T) { Config: map[string]interface{}{ "image": "coreos.com/etcd:v2.0.4", "command": "/etcd", - "args": "--version", + "args": []string{"--version"}, }, } @@ -202,7 +202,7 @@ func TestRktDriver_Start_Wait_Logs(t *testing.T) { "trust_prefix": "coreos.com/etcd", "image": "coreos.com/etcd:v2.0.4", "command": "/etcd", - "args": "--version", + "args": []string{"--version"}, }, } diff --git a/client/task_runner_test.go b/client/task_runner_test.go index f8bc9e466..1ada5060b 100644 --- a/client/task_runner_test.go +++ b/client/task_runner_test.go @@ -89,7 +89,7 @@ func TestTaskRunner_Destroy(t *testing.T) { // Change command to ensure we run for a bit tr.task.Config["command"] = "/bin/sleep" - tr.task.Config["args"] = "10" + tr.task.Config["args"] = []string{"10"} go tr.Run() // Begin the tear down @@ -128,7 +128,7 @@ func TestTaskRunner_Update(t *testing.T) { // Change command to ensure we run for a bit tr.task.Config["command"] = "/bin/sleep" - tr.task.Config["args"] = "10" + tr.task.Config["args"] = []string{"10"} go tr.Run() defer tr.Destroy() defer tr.ctx.AllocDir.Destroy() @@ -153,7 +153,7 @@ func TestTaskRunner_SaveRestoreState(t *testing.T) { // Change command to ensure we run for a bit tr.task.Config["command"] = "/bin/sleep" - tr.task.Config["args"] = "10" + tr.task.Config["args"] = []string{"10"} go tr.Run() defer tr.Destroy() diff --git a/nomad/fsm.go b/nomad/fsm.go index 21b3538e4..71c40d68f 100644 --- a/nomad/fsm.go +++ b/nomad/fsm.go @@ -13,13 +13,6 @@ import ( "github.com/hashicorp/raft" ) -var ( - msgpackHandle = &codec.MsgpackHandle{ - RawToString: true, - WriteExt: true, - } -) - const ( // timeTableGranularity is the granularity of index to time tracking timeTableGranularity = 5 * time.Minute @@ -328,7 +321,7 @@ func (n *nomadFSM) Restore(old io.ReadCloser) error { defer restore.Abort() // Create a decoder - dec := codec.NewDecoder(old, msgpackHandle) + dec := codec.NewDecoder(old, structs.MsgpackHandle) // Read in the header var header snapshotHeader @@ -412,7 +405,7 @@ func (n *nomadFSM) Restore(old io.ReadCloser) error { func (s *nomadSnapshot) Persist(sink raft.SnapshotSink) error { defer metrics.MeasureSince([]string{"nomad", "fsm", "persist"}, time.Now()) // Register the nodes - encoder := codec.NewEncoder(sink, msgpackHandle) + encoder := codec.NewEncoder(sink, structs.MsgpackHandle) // Write the header header := snapshotHeader{} diff --git a/nomad/mock/mock.go b/nomad/mock/mock.go index 12a8484c0..25ca60ef2 100644 --- a/nomad/mock/mock.go +++ b/nomad/mock/mock.go @@ -86,7 +86,7 @@ func Job() *structs.Job { Driver: "exec", Config: map[string]interface{}{ "command": "/bin/date", - "args": "+%s", + "args": []string{"+%s"}, }, Env: map[string]string{ "FOO": "bar", @@ -151,7 +151,7 @@ func SystemJob() *structs.Job { Driver: "exec", Config: map[string]interface{}{ "command": "/bin/date", - "args": "+%s", + "args": []string{"+%s"}, }, Resources: &structs.Resources{ CPU: 500, diff --git a/nomad/rpc.go b/nomad/rpc.go index 123c35028..e52e258f0 100644 --- a/nomad/rpc.go +++ b/nomad/rpc.go @@ -11,7 +11,6 @@ import ( "time" "github.com/armon/go-metrics" - "github.com/hashicorp/go-msgpack/codec" "github.com/hashicorp/net-rpc-msgpackrpc" "github.com/hashicorp/nomad/nomad/state" "github.com/hashicorp/nomad/nomad/structs" @@ -53,24 +52,16 @@ const ( enqueueLimit = 30 * time.Second ) -var ( - // rpcHandle is the MsgpackHandle to be used by both Client and Server codecs. - rpcHandle = &codec.MsgpackHandle{ - // Enables proper encoding of strings within nil interfaces. - RawToString: true, - } -) - // NewClientCodec returns a new rpc.ClientCodec to be used to make RPC calls to // the Nomad Server. func NewClientCodec(conn io.ReadWriteCloser) rpc.ClientCodec { - return msgpackrpc.NewCodecFromHandle(true, true, conn, rpcHandle) + return msgpackrpc.NewCodecFromHandle(true, true, conn, structs.MsgpackHandle) } // NewServerCodec returns a new rpc.ServerCodec to be used by the Nomad Server // to handle rpcs. func NewServerCodec(conn io.ReadWriteCloser) rpc.ServerCodec { - return msgpackrpc.NewCodecFromHandle(true, true, conn, rpcHandle) + return msgpackrpc.NewCodecFromHandle(true, true, conn, structs.MsgpackHandle) } // listen is used to listen for incoming RPC connections diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index b7453b512..876bb0c39 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1704,25 +1704,26 @@ func (p *PlanResult) FullCommit(plan *Plan) (bool, int, int) { } // msgpackHandle is a shared handle for encoding/decoding of structs -var msgpackHandle = func() *codec.MsgpackHandle { +var MsgpackHandle = func() *codec.MsgpackHandle { h := &codec.MsgpackHandle{RawToString: true} // Sets the default type for decoding a map into a nil interface{}. // This is necessary in particular because we store the driver configs as a // nil interface{}. h.MapType = reflect.TypeOf(map[string]interface{}(nil)) + h.SliceType = reflect.TypeOf([]string{}) return h }() // Decode is used to decode a MsgPack encoded object func Decode(buf []byte, out interface{}) error { - return codec.NewDecoder(bytes.NewReader(buf), msgpackHandle).Decode(out) + return codec.NewDecoder(bytes.NewReader(buf), MsgpackHandle).Decode(out) } // Encode is used to encode a MsgPack object with type prefix func Encode(t MessageType, msg interface{}) ([]byte, error) { var buf bytes.Buffer buf.WriteByte(uint8(t)) - err := codec.NewEncoder(&buf, msgpackHandle).Encode(msg) + err := codec.NewEncoder(&buf, MsgpackHandle).Encode(msg) return buf.Bytes(), err } diff --git a/nomad/timetable_test.go b/nomad/timetable_test.go index 3a771d6fa..a76446a13 100644 --- a/nomad/timetable_test.go +++ b/nomad/timetable_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/hashicorp/go-msgpack/codec" + "github.com/hashicorp/nomad/nomad/structs" ) func TestTimeTable(t *testing.T) { @@ -104,14 +105,14 @@ func TestTimeTable_SerializeDeserialize(t *testing.T) { tt.Witness(50, plusHour) var buf bytes.Buffer - enc := codec.NewEncoder(&buf, msgpackHandle) + enc := codec.NewEncoder(&buf, structs.MsgpackHandle) err := tt.Serialize(enc) if err != nil { t.Fatalf("err: %v", err) } - dec := codec.NewDecoder(&buf, msgpackHandle) + dec := codec.NewDecoder(&buf, structs.MsgpackHandle) tt2 := NewTimeTable(time.Second, time.Minute) err = tt2.Deserialize(dec) diff --git a/website/source/docs/drivers/docker.html.md b/website/source/docs/drivers/docker.html.md index d1c6bafbe..a50bc1cec 100644 --- a/website/source/docs/drivers/docker.html.md +++ b/website/source/docs/drivers/docker.html.md @@ -19,13 +19,13 @@ and cleaning up after containers. The `docker` driver supports the following configuration in the job specification: -* `image` - (Required) The Docker image to run. The image may include a tag or +* `image` - The Docker image to run. The image may include a tag or custom URL. By default it will be fetched from Docker Hub. * `command` - (Optional) The command to run when starting the container. -* `args` - (Optional) 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. * `network_mode` - (Optional) The network mode to be used for the container. In order to support userspace networking plugins in Docker 1.9 this accepts any diff --git a/website/source/docs/drivers/exec.html.md b/website/source/docs/drivers/exec.html.md index f897b1ea4..3d921aa6d 100644 --- a/website/source/docs/drivers/exec.html.md +++ b/website/source/docs/drivers/exec.html.md @@ -20,15 +20,19 @@ scripts or other wrappers which provide higher level features. The `exec` driver supports the following configuration in the job spec: -* `command` - (Required) The command to execute. Must be provided. -* `artifact_source` – (Optional) Source location of an executable artifact. Must be accessible -from the Nomad client. If you specify an `artifact_source` to be executed, you -must reference it in the `command` as show in the examples below -* `checksum` - **(Optional)** The checksum type and value for the `artifact_source` image. -The format is `type:value`, where type is any of `md5`, `sha1`, `sha256`, or `sha512`, -and the value is the computed checksum. If a checksum is supplied and does not -match the downloaded artifact, the driver will fail to start -* `args` - The argument list to the command, space seperated. Optional. +* `command` - The command to execute. Must be provided. + +* `artifact_source` – (Optional) Source location of an executable artifact. Must + be accessible from the Nomad client. If you specify an `artifact_source` to be + executed, you must reference it in the `command` as show in the examples below + +* `checksum` - (Optional) The checksum type and value for the `artifact_source` + image. The format is `type:value`, where type is any of `md5`, `sha1`, + `sha256`, or `sha512`, and the value is the computed checksum. If a checksum + is supplied and does not match the downloaded artifact, the driver will fail + to start + +* `args` - (Optional) A list of arguments to the `command`. ## Client Requirements diff --git a/website/source/docs/drivers/java.html.md b/website/source/docs/drivers/java.html.md index f2bbd2b76..45baacb72 100644 --- a/website/source/docs/drivers/java.html.md +++ b/website/source/docs/drivers/java.html.md @@ -18,17 +18,19 @@ HTTP from the Nomad client. The `java` driver supports the following configuration in the job spec: -* `artifact_source` - **(Required)** The hosted location of the source Jar file. Must be accessible -from the Nomad client -* `checksum` - **(Optional)** The checksum type and value for the `artifact_source` image. -The format is `type:value`, where type is any of `md5`, `sha1`, `sha256`, or `sha512`, -and the value is the computed checksum. If a checksum is supplied and does not -match the downloaded artifact, the driver will fail to start +* `artifact_source` - The hosted location of the source Jar file. Must be + accessible from the Nomad client -* `args` - **(Optional)** The argument list for the `java` command, space separated. +* `checksum` - (Optional) The checksum type and value for the `artifact_source` + image. The format is `type:value`, where type is any of `md5`, `sha1`, + `sha256`, or `sha512`, and the value is the computed checksum. If a checksum + is supplied and does not match the downloaded artifact, the driver will fail + to start -* `jvm_options` - **(Optional)** JVM options to be passed while invoking java. These options - are passed not validated in any way in Nomad. +* `args` - (Optional) A list of arguments to the `java` command. + +* `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. ## Client Requirements diff --git a/website/source/docs/drivers/qemu.html.md b/website/source/docs/drivers/qemu.html.md index 84909e331..d329c7a8e 100644 --- a/website/source/docs/drivers/qemu.html.md +++ b/website/source/docs/drivers/qemu.html.md @@ -23,16 +23,19 @@ The `Qemu` driver can execute any regular `qemu` image (e.g. `qcow`, `img`, The `Qemu` driver supports the following configuration in the job spec: -* `artifact_source` - **(Required)** The hosted location of the source Qemu image. Must be accessible +* `artifact_source` - The hosted location of the source Qemu image. Must be accessible from the Nomad client, via HTTP. -* `checksum` - **(Optional)** The checksum type and value for the `artifact_source` image. + +* `checksum` - (Optional) The checksum type and value for the `artifact_source` image. The format is `type:value`, where type is any of `md5`, `sha1`, `sha256`, or `sha512`, and the value is the computed checksum. If a checksum is supplied and does not match the downloaded artifact, the driver will fail to start + * `accelerator` - (Optional) The type of accelerator to use in the invocation. If the host machine has `Qemu` installed with KVM support, users can specify `kvm` for the `accelerator`. Default is `tcg` -* `port_map` - **(Optional)** A `map[string]int` that maps port labels to ports + +* `port_map` - (Optional) A `map[string]int` that maps port labels to ports on the guest. This forwards the host port to the guest vm. For example, `port_map { db = 6539 }` would forward the host port with label `db` to the guest vm's port 6539. diff --git a/website/source/docs/drivers/raw_exec.html.md b/website/source/docs/drivers/raw_exec.html.md index 2dc741887..c76825f25 100644 --- a/website/source/docs/drivers/raw_exec.html.md +++ b/website/source/docs/drivers/raw_exec.html.md @@ -18,15 +18,19 @@ As such, it should be used with extreme care and is disabled by default. The `raw_exec` driver supports the following configuration in the job spec: -* `command` - (Required) The command to execute. Must be provided. -* `artifact_source` – (Optional) Source location of an executable artifact. Must be accessible -from the Nomad client. If you specify an `artifact_source` to be executed, you -must reference it in the `command` as show in the examples below -* `checksum` - **(Optional)** The checksum type and value for the `artifact_source` image. -The format is `type:value`, where type is any of `md5`, `sha1`, `sha256`, or `sha512`, -and the value is the computed checksum. If a checksum is supplied and does not -match the downloaded artifact, the driver will fail to start -* `args` - The argument list to the command, space seperated. Optional. +* `command` - The command to execute. Must be provided. + +* `artifact_source` – (Optional) Source location of an executable artifact. Must + be accessible from the Nomad client. If you specify an `artifact_source` to be + executed, you must reference it in the `command` as show in the examples below + +* `checksum` - (Optional) The checksum type and value for the `artifact_source` + image. The format is `type:value`, where type is any of `md5`, `sha1`, + `sha256`, or `sha512`, and the value is the computed checksum. If a checksum + is supplied and does not match the downloaded artifact, the driver will fail + to start + +* `args` - (Optional) A list of arguments to the `command`. ## Client Requirements diff --git a/website/source/docs/drivers/rkt.html.md b/website/source/docs/drivers/rkt.html.md index ddbb76601..7ce3e1a49 100644 --- a/website/source/docs/drivers/rkt.html.md +++ b/website/source/docs/drivers/rkt.html.md @@ -20,13 +20,16 @@ being marked as experimental and should be used with care. The `rkt` driver supports the following configuration in the job spec: -* `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 run without -verifying the image signature. -* `image` - **(Required)** The image to run which may be specified by name, -hash, ACI address or docker registry. -* `command` - **(Optional**) A command to execute on the ACI. -* `args` - **(Optional**) A string of args to pass into the image. +* `image` - The image to run which may be specified by name, hash, ACI address + or docker registry. + +* `command` - (Optional) A command to execute on the ACI. + +* `args` - (Optional) A list of arguments to the image. + +* `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 + run without verifying the image signature. ## Task Directories