From d20be7b58cbc4f86008683bff0c7ae7823a90cbb Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 09:41:05 -0800 Subject: [PATCH 1/8] Update ParseAndReplace to take a list of args and remove shell style parsing --- client/driver/args/args.go | 27 ++++++--------------------- client/driver/args/args_test.go | 26 +++----------------------- 2 files changed, 9 insertions(+), 44 deletions(-) diff --git a/client/driver/args/args.go b/client/driver/args/args.go index 51793bd8b..e40be3f29 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, error) { + replaced := make([]string, len(args)) + for i, arg := range args { replaced[i] = ReplaceEnv(arg, env) } return replaced, nil } -// 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..e11b2cc8b 100644 --- a/client/driver/args/args_test.go +++ b/client/driver/args/args_test.go @@ -21,7 +21,7 @@ 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 { @@ -34,7 +34,7 @@ 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 { @@ -47,7 +47,7 @@ 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 { @@ -58,23 +58,3 @@ func TestDriverArgs_ParseAndReplaceChainedEnv(t *testing.T) { 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) - } - - if !reflect.DeepEqual(act, exp) { - t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) - } -} From ee1887e609e9360c2129b61d0288f77e2ca5ff1f Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 11:27:59 -0800 Subject: [PATCH 2/8] Rebase --- client/driver/docker.go | 6 +++--- client/driver/docker_test.go | 10 +++++++--- client/driver/exec.go | 16 +++++----------- client/driver/exec_test.go | 16 +++++++++++----- client/driver/executor/exec_basic.go | 3 +-- client/driver/executor/exec_linux.go | 3 +-- client/driver/java.go | 12 ++++++------ client/driver/raw_exec.go | 8 +------- client/driver/raw_exec_test.go | 16 +++++++++++----- client/driver/rkt.go | 6 +++--- client/driver/rkt_test.go | 6 +++--- 11 files changed, 52 insertions(+), 50 deletions(-) diff --git a/client/driver/docker.go b/client/driver/docker.go index 6fa1af600..a27da8fc4 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 @@ -302,12 +302,12 @@ func (d *DockerDriver) createContainer(ctx *ExecContext, task *structs.Task, dri // 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") } diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 4c9300579..00ffbe61b 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..34b71095b 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..7c97760ea 100644 --- a/client/driver/executor/exec_basic.go +++ b/client/driver/executor/exec_basic.go @@ -63,8 +63,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()) + parsed, err := args.ParseAndReplace(e.cmd.Args, envVars.Map()) if err != nil { return err } diff --git a/client/driver/executor/exec_linux.go b/client/driver/executor/exec_linux.go index 14f4f809c..c2f2931c0 100644 --- a/client/driver/executor/exec_linux.go +++ b/client/driver/executor/exec_linux.go @@ -167,8 +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()) + parsed, err := args.ParseAndReplace(e.cmd.Args, envVars.Map()) if err != nil { return err } diff --git a/client/driver/java.go b/client/driver/java.go index eb2930a28..f408c6087 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 @@ -133,8 +133,8 @@ func (d *JavaDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, // 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/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..9bf38ad6d 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..d86129656 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,7 +150,7 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e } // Add user passed arguments. - if driverConfig.Args != "" { + if len(driverConfig.Args) != 0 { parsed, err := args.ParseAndReplace(driverConfig.Args, envVars.Map()) if err != nil { return nil, err 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"}, }, } From b7e84bc124023b3913456f114c9278c4fb5a0fed Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 11:04:27 -0800 Subject: [PATCH 3/8] Docs --- website/source/docs/drivers/docker.html.md | 6 +++--- website/source/docs/drivers/exec.html.md | 22 ++++++++++++-------- website/source/docs/drivers/java.html.md | 18 +++++++++------- website/source/docs/drivers/qemu.html.md | 9 +++++--- website/source/docs/drivers/raw_exec.html.md | 22 ++++++++++++-------- website/source/docs/drivers/rkt.html.md | 17 ++++++++------- 6 files changed, 55 insertions(+), 39 deletions(-) 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..3d74fc751 100644 --- a/website/source/docs/drivers/java.html.md +++ b/website/source/docs/drivers/java.html.md @@ -18,16 +18,18 @@ 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 +* `args` - (Optional) A list of arguments to the `java` command. + +* `jvm_options` - (Optional) 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 From c6a895aff77a3fd5295ec1de7d4ca804e48bb411 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 11:22:08 -0800 Subject: [PATCH 4/8] Fix executor tests --- client/driver/executor/exec_basic.go | 1 - client/driver/executor/test_harness_test.go | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/client/driver/executor/exec_basic.go b/client/driver/executor/exec_basic.go index 7c97760ea..8bf75c2ee 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{} } 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 { From 62a0c5d14c80c912056921430978a9a8a617c0fb Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 12:30:35 -0800 Subject: [PATCH 5/8] Rebase --- client/driver/docker.go | 2 +- client/driver/docker_test.go | 2 +- client/driver/exec_test.go | 4 ++-- client/driver/java.go | 6 +++--- client/driver/java_test.go | 3 +-- client/driver/raw_exec_test.go | 2 +- website/source/docs/drivers/java.html.md | 4 ++-- 7 files changed, 11 insertions(+), 12 deletions(-) diff --git a/client/driver/docker.go b/client/driver/docker.go index a27da8fc4..69aea2b73 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -431,7 +431,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 00ffbe61b..a61de381c 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -192,7 +192,7 @@ func TestDockerDriver_Start_Wait_AllocDir(t *testing.T) { "command": "/bin/bash", "args": []string{ "-c", - fmt.Sprintf(`"sleep 1; echo -n %s > $%s/%s"`, + fmt.Sprintf(`sleep 1; echo -n %s > $%s/%s`, string(exp), environment.AllocDir, file), }, }, diff --git a/client/driver/exec_test.go b/client/driver/exec_test.go index 34b71095b..f5470074a 100644 --- a/client/driver/exec_test.go +++ b/client/driver/exec_test.go @@ -163,7 +163,7 @@ func TestExecDriver_Start_Artifact_expanded(t *testing.T) { "command": "/bin/bash", "args": []string{ "-c", - fmt.Sprintf(`'/bin/sleep 1 && %s'`, filepath.Join("$NOMAD_TASK_DIR", file)), + fmt.Sprintf(`/bin/sleep 1 && %s`, filepath.Join("$NOMAD_TASK_DIR", file)), }, }, Resources: basicResources, @@ -209,7 +209,7 @@ 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), environment.AllocDir, file), }, }, Resources: basicResources, diff --git a/client/driver/java.go b/client/driver/java.go index f408c6087..eb475db32 100644 --- a/client/driver/java.go +++ b/client/driver/java.go @@ -28,7 +28,7 @@ type JavaDriver struct { } type JavaDriverConfig struct { - JvmOpts string `mapstructure:"jvm_options"` + JvmOpts []string `mapstructure:"jvm_options"` ArtifactSource string `mapstructure:"artifact_source"` Checksum string `mapstructure:"checksum"` Args []string `mapstructure:"args"` @@ -126,9 +126,9 @@ 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. 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_test.go b/client/driver/raw_exec_test.go index 9bf38ad6d..8b6e0c649 100644 --- a/client/driver/raw_exec_test.go +++ b/client/driver/raw_exec_test.go @@ -237,7 +237,7 @@ func TestRawExecDriver_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), environment.AllocDir, file), }, }, Resources: basicResources, diff --git a/website/source/docs/drivers/java.html.md b/website/source/docs/drivers/java.html.md index 3d74fc751..45baacb72 100644 --- a/website/source/docs/drivers/java.html.md +++ b/website/source/docs/drivers/java.html.md @@ -29,8 +29,8 @@ The `java` driver supports the following configuration in the job spec: * `args` - (Optional) A list of arguments to the `java` command. -* `jvm_options` - (Optional) JVM options to be passed while invoking java. These options - are passed not validated in any way in Nomad. +* `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 From d0ebac151345040203b5ff9cc45bf47d76478f7a Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 12:28:34 -0800 Subject: [PATCH 6/8] More test fixes --- client/alloc_runner_test.go | 6 +++--- client/client_test.go | 2 +- client/task_runner_test.go | 6 +++--- nomad/mock/mock.go | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) 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/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/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, From 5aaa0553cf65a8cdccc0544aa1368665d76974b5 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 12:40:34 -0800 Subject: [PATCH 7/8] Remove returning the error --- client/driver/args/args.go | 4 ++-- client/driver/args/args_test.go | 15 +++------------ client/driver/docker.go | 5 +---- client/driver/executor/exec_basic.go | 6 +----- client/driver/executor/exec_linux.go | 6 +----- client/driver/rkt.go | 5 +---- 6 files changed, 9 insertions(+), 32 deletions(-) diff --git a/client/driver/args/args.go b/client/driver/args/args.go index e40be3f29..9e8a9980c 100644 --- a/client/driver/args/args.go +++ b/client/driver/args/args.go @@ -9,13 +9,13 @@ 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. -func ParseAndReplace(args []string, env map[string]string) ([]string, error) { +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. diff --git a/client/driver/args/args_test.go b/client/driver/args/args_test.go index e11b2cc8b..5e7cbca4a 100644 --- a/client/driver/args/args_test.go +++ b/client/driver/args/args_test.go @@ -23,10 +23,7 @@ var ( func TestDriverArgs_ParseAndReplaceInvalidEnv(t *testing.T) { 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) @@ -36,10 +33,7 @@ func TestDriverArgs_ParseAndReplaceInvalidEnv(t *testing.T) { func TestDriverArgs_ParseAndReplaceValidEnv(t *testing.T) { 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) @@ -49,10 +43,7 @@ func TestDriverArgs_ParseAndReplaceValidEnv(t *testing.T) { 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, 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 69aea2b73..4aa97e13b 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -293,10 +293,7 @@ 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. diff --git a/client/driver/executor/exec_basic.go b/client/driver/executor/exec_basic.go index 8bf75c2ee..2ef4fbedc 100644 --- a/client/driver/executor/exec_basic.go +++ b/client/driver/executor/exec_basic.go @@ -62,11 +62,7 @@ func (e *BasicExecutor) Start() error { } e.cmd.Path = args.ReplaceEnv(e.cmd.Path, envVars.Map()) - parsed, err := args.ParseAndReplace(e.cmd.Args, 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 c2f2931c0..43c9270e0 100644 --- a/client/driver/executor/exec_linux.go +++ b/client/driver/executor/exec_linux.go @@ -167,11 +167,7 @@ func (e *LinuxExecutor) Start() error { } e.cmd.Path = args.ReplaceEnv(e.cmd.Path, envVars.Map()) - parsed, err := args.ParseAndReplace(e.cmd.Args, 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/rkt.go b/client/driver/rkt.go index d86129656..1fb6e9e06 100644 --- a/client/driver/rkt.go +++ b/client/driver/rkt.go @@ -151,10 +151,7 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e // Add user passed arguments. if len(driverConfig.Args) != 0 { - parsed, err := args.ParseAndReplace(driverConfig.Args, envVars.Map()) - if err != nil { - return nil, err - } + parsed := args.ParseAndReplace(driverConfig.Args, envVars.Map()) // Need to start arguments with "--" if len(parsed) > 0 { From 1309a849151daa6f936098e66506af14dbc1c893 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 13:15:13 -0800 Subject: [PATCH 8/8] Use one msgpack codec and have it decode []string in nil interfaces --- nomad/fsm.go | 11 ++--------- nomad/rpc.go | 13 ++----------- nomad/structs/structs.go | 7 ++++--- nomad/timetable_test.go | 5 +++-- 4 files changed, 11 insertions(+), 25 deletions(-) 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/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 7fde437cc..5ff8d8c3b 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)