From 079e6b5fb1c3695eb22f6781cad907bb79d55954 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Tue, 21 Nov 2017 17:27:52 -0500 Subject: [PATCH 01/14] allow controlling the stop signal in exec/raw_exec --- CHANGELOG.md | 1 + client/driver/exec.go | 9 +++++++-- client/driver/executor/executor.go | 28 +++++++++++++++++++++++++++- 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68e3b03519a..3a5476d5f0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ IMPROVEMENTS: * driver/docker: Adds support for `ulimit` and `sysctl` options [GH-3568] * driver/docker: Adds support for StopTimeout (set to the same value as kill_timeout [GH-3601] + * driver/exec: allow controlling the stop signal in exec/raw_exec [GH-1755] * driver/rkt: Add support for passing through user [GH-3612] * driver/qemu: Support graceful shutdowns on unix platforms [GH-3411] * template: Updated to consul template 0.19.4 [GH-3543] diff --git a/client/driver/exec.go b/client/driver/exec.go index c06b4a330f9..5c4f93966bc 100644 --- a/client/driver/exec.go +++ b/client/driver/exec.go @@ -31,8 +31,9 @@ type ExecDriver struct { } type ExecDriverConfig struct { - Command string `mapstructure:"command"` - Args []string `mapstructure:"args"` + Command string `mapstructure:"command"` + Args []string `mapstructure:"args"` + KillSignal string `mapstructure:"kill_signal"` } // execHandle is returned from Start/Open as a handle to the PID @@ -67,6 +68,9 @@ func (d *ExecDriver) Validate(config map[string]interface{}) error { "args": { Type: fields.TypeArray, }, + "kill_signal": { + Type: fields.TypeString, + }, }, } @@ -132,6 +136,7 @@ func (d *ExecDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse execCmd := &executor.ExecCommand{ Cmd: command, Args: driverConfig.Args, + KillSignal: driverConfig.KillSignal, FSIsolation: true, ResourceLimits: true, User: getExecutorUser(task), diff --git a/client/driver/executor/executor.go b/client/driver/executor/executor.go index a3073e1eee5..8d2a95df1e4 100644 --- a/client/driver/executor/executor.go +++ b/client/driver/executor/executor.go @@ -98,6 +98,8 @@ type ExecCommand struct { // Args is the args of the command that the user wants to run. Args []string + KillSignal string + // FSIsolation determines whether the command would be run in a chroot. FSIsolation bool @@ -481,6 +483,21 @@ func (e *UniversalExecutor) Exit() error { return merr.ErrorOrNil() } +// Look to see if the user has specified a kill command. If non has been +// specified, default to SIGQUIT. +func getMatchingSyscall(sys string) syscall.Signal { + switch sys { + case "SIGINT": + return syscall.SIGINT + case "SIGUSR1": + return syscall.SIGUSR1 + case "SIGUSR2": + return syscall.SIGUSR2 + default: + return syscall.SIGQUIT + } +} + // Shutdown sends an interrupt signal to the user process func (e *UniversalExecutor) ShutDown() error { if e.cmd.Process == nil { @@ -496,9 +513,18 @@ func (e *UniversalExecutor) ShutDown() error { } return nil } - if err = proc.Signal(os.Interrupt); err != nil && err.Error() != finishedErr { + + var killErr error + if e.command.KillSignal != "" { + sys := getMatchingSyscall(e.command.KillSignal) + killErr = syscall.Kill(e.cmd.Process.Pid, sys) + } else { + killErr = proc.Signal(os.Interrupt) + } + if killErr != nil && killErr.Error() != finishedErr { return fmt.Errorf("executor.shutdown error: %v", err) } + return nil } From 9e49836d4f2955f4176e1194916b9585a0e43a10 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Thu, 30 Nov 2017 16:53:35 -0500 Subject: [PATCH 02/14] move kill_signal to task level, extend to docker --- api/tasks.go | 1 + client/driver/docker.go | 7 ++- client/driver/docker_test.go | 82 ++++++++++++++++++++++++++++++ client/driver/exec.go | 19 ++++--- client/driver/executor/executor.go | 30 +++-------- client/driver/raw_exec.go | 16 ++++-- jobspec/parse.go | 8 +++ jobspec/parse_test.go | 24 +++++++++ nomad/job_endpoint_test.go | 32 ++++++++++++ nomad/structs/structs.go | 9 ++++ 10 files changed, 195 insertions(+), 33 deletions(-) diff --git a/api/tasks.go b/api/tasks.go index 2b9f97b2d94..2cfd7ae20a7 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -370,6 +370,7 @@ type Task struct { DispatchPayload *DispatchPayloadConfig Leader bool ShutdownDelay time.Duration `mapstructure:"shutdown_delay"` + KillSignal string } func (t *Task) Canonicalize(tg *TaskGroup, job *Job) { diff --git a/client/driver/docker.go b/client/driver/docker.go index 85496346646..f14aab4af27 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -201,6 +201,7 @@ type DockerDriverConfig struct { MacAddress string `mapstructure:"mac_address"` // Pin mac address to container SecurityOpt []string `mapstructure:"security_opt"` // Flags to pass directly to security-opt Devices []DockerDevice `mapstructure:"devices"` // To allow mounting USB or other serial control devices + StopSignal string `mapstructure:"stop_signal"` // allow passing through a specific stop signal } func sliceMergeUlimit(ulimitsRaw map[string]string) ([]docker.ULimit, error) { @@ -712,7 +713,6 @@ func (d *DockerDriver) Prestart(ctx *ExecContext, task *structs.Task) (*Prestart } func (d *DockerDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse, error) { - pluginLogFile := filepath.Join(ctx.TaskDir.Dir, "executor.out") executorConfig := &dstructs.ExecutorConfig{ LogFile: pluginLogFile, @@ -1055,6 +1055,11 @@ func (d *DockerDriver) createContainerConfig(ctx *ExecContext, task *structs.Tas StopTimeout: int(task.KillTimeout.Seconds()), } + // Set the stop signal for the docker container. + if task.KillSignal != "" { + config.StopSignal = task.KillSignal + } + if driverConfig.WorkDir != "" { config.WorkingDir = driverConfig.WorkDir } diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 058361ebb6f..0e563cc697d 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -2045,3 +2045,85 @@ func TestDockerDriver_Device_Success(t *testing.T) { assert.Equal(t, expectedDevice, container.HostConfig.Devices[0], "Incorrect device ") } + +func TestDockerDriver_Kill(t *testing.T) { + assert := assert.New(t) + if !tu.IsTravis() { + t.Parallel() + } + if !testutil.DockerIsConnected(t) { + t.Skip("Docker not connected") + } + + // Tasks started with a signal that is not supported should error + { + task := &structs.Task{ + Name: "nc-demo", + Driver: "docker", + KillSignal: "ABCDEF", + Config: map[string]interface{}{ + "load": "busybox.tar", + "image": "busybox", + "command": "/bin/nc", + "args": []string{"-l", "127.0.0.1", "-p", "0"}, + }, + LogConfig: &structs.LogConfig{ + MaxFiles: 10, + MaxFileSizeMB: 10, + }, + Resources: basicResources, + } + + ctx := testDockerDriverContexts(t, task) + defer ctx.AllocDir.Destroy() + d := NewDockerDriver(ctx.DriverCtx) + copyImage(t, ctx.ExecCtx.TaskDir, "busybox.tar") + + _, err := d.Prestart(ctx.ExecCtx, task) + if err != nil { + t.Fatalf("error in prestart: %v", err) + } + + _, err = d.Start(ctx.ExecCtx, task) + assert.NotNil(err) + } + + // Tasks started with a signal that is not supported should not error + { + task := &structs.Task{ + Name: "nc-demo", + Driver: "docker", + KillSignal: "SIGQUIT", + Config: map[string]interface{}{ + "load": "busybox.tar", + "image": "busybox", + "command": "/bin/nc", + "args": []string{"-l", "127.0.0.1", "-p", "0"}, + }, + LogConfig: &structs.LogConfig{ + MaxFiles: 10, + MaxFileSizeMB: 10, + }, + Resources: basicResources, + } + + ctx := testDockerDriverContexts(t, task) + defer ctx.AllocDir.Destroy() + d := NewDockerDriver(ctx.DriverCtx) + copyImage(t, ctx.ExecCtx.TaskDir, "busybox.tar") + + _, err := d.Prestart(ctx.ExecCtx, task) + if err != nil { + t.Fatalf("error in prestart: %v", err) + } + + resp, err := d.Start(ctx.ExecCtx, task) + assert.Nil(err) + assert.NotNil(resp.Handle) + + handle := resp.Handle.(*DockerHandle) + waitForExist(t, client, handle) + err = handle.Kill() + assert.Nil(err) + } +} diff --git a/client/driver/exec.go b/client/driver/exec.go index 5c4f93966bc..741f63b87eb 100644 --- a/client/driver/exec.go +++ b/client/driver/exec.go @@ -9,6 +9,7 @@ import ( "path/filepath" "time" + "github.com/hashicorp/consul-template/signals" "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-plugin" "github.com/hashicorp/nomad/client/allocdir" @@ -31,9 +32,8 @@ type ExecDriver struct { } type ExecDriverConfig struct { - Command string `mapstructure:"command"` - Args []string `mapstructure:"args"` - KillSignal string `mapstructure:"kill_signal"` + Command string `mapstructure:"command"` + Args []string `mapstructure:"args"` } // execHandle is returned from Start/Open as a handle to the PID @@ -68,9 +68,6 @@ func (d *ExecDriver) Validate(config map[string]interface{}) error { "args": { Type: fields.TypeArray, }, - "kill_signal": { - Type: fields.TypeString, - }, }, } @@ -133,10 +130,18 @@ func (d *ExecDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse return nil, fmt.Errorf("failed to set executor context: %v", err) } + var taskKillSignal os.Signal + if task.KillSignal != "" { + taskKillSignal = signals.SignalLookup[task.KillSignal] + if taskKillSignal == nil { + return nil, fmt.Errorf("Signal %s is not supported", task.KillSignal) + } + } + execCmd := &executor.ExecCommand{ Cmd: command, Args: driverConfig.Args, - KillSignal: driverConfig.KillSignal, + TaskKillSignal: taskKillSignal, FSIsolation: true, ResourceLimits: true, User: getExecutorUser(task), diff --git a/client/driver/executor/executor.go b/client/driver/executor/executor.go index 8d2a95df1e4..0bfc472757e 100644 --- a/client/driver/executor/executor.go +++ b/client/driver/executor/executor.go @@ -98,7 +98,8 @@ type ExecCommand struct { // Args is the args of the command that the user wants to run. Args []string - KillSignal string + // TaskKillSignal is an optional field which signal to kill the process + TaskKillSignal os.Signal // FSIsolation determines whether the command would be run in a chroot. FSIsolation bool @@ -483,21 +484,6 @@ func (e *UniversalExecutor) Exit() error { return merr.ErrorOrNil() } -// Look to see if the user has specified a kill command. If non has been -// specified, default to SIGQUIT. -func getMatchingSyscall(sys string) syscall.Signal { - switch sys { - case "SIGINT": - return syscall.SIGINT - case "SIGUSR1": - return syscall.SIGUSR1 - case "SIGUSR2": - return syscall.SIGUSR2 - default: - return syscall.SIGQUIT - } -} - // Shutdown sends an interrupt signal to the user process func (e *UniversalExecutor) ShutDown() error { if e.cmd.Process == nil { @@ -514,14 +500,14 @@ func (e *UniversalExecutor) ShutDown() error { return nil } - var killErr error - if e.command.KillSignal != "" { - sys := getMatchingSyscall(e.command.KillSignal) - killErr = syscall.Kill(e.cmd.Process.Pid, sys) + var osSignal os.Signal + if e.command.TaskKillSignal != nil { + osSignal = e.command.TaskKillSignal } else { - killErr = proc.Signal(os.Interrupt) + osSignal = os.Interrupt } - if killErr != nil && killErr.Error() != finishedErr { + + if err = proc.Signal(osSignal); err != nil && err.Error() != finishedErr { return fmt.Errorf("executor.shutdown error: %v", err) } diff --git a/client/driver/raw_exec.go b/client/driver/raw_exec.go index 966a2affb58..99208a3441e 100644 --- a/client/driver/raw_exec.go +++ b/client/driver/raw_exec.go @@ -9,6 +9,7 @@ import ( "path/filepath" "time" + "github.com/hashicorp/consul-template/signals" "github.com/hashicorp/go-plugin" "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/config" @@ -144,10 +145,19 @@ func (d *RawExecDriver) Start(ctx *ExecContext, task *structs.Task) (*StartRespo return nil, fmt.Errorf("failed to set executor context: %v", err) } + var taskKillSignal os.Signal + if task.KillSignal != "" { + taskKillSignal = signals.SignalLookup[task.KillSignal] + if taskKillSignal == nil { + return nil, fmt.Errorf("Signal %s is not supported", task.KillSignal) + } + } + execCmd := &executor.ExecCommand{ - Cmd: command, - Args: driverConfig.Args, - User: task.User, + Cmd: command, + Args: driverConfig.Args, + User: task.User, + TaskKillSignal: taskKillSignal, } ps, err := exec.LaunchCmd(execCmd) if err != nil { diff --git a/jobspec/parse.go b/jobspec/parse.go index e48ce46c9a4..03efb07a025 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -591,6 +591,7 @@ func parseTasks(jobName string, taskGroupName string, result *[]*api.Task, list "template", "user", "vault", + "kill_signal", } if err := helper.CheckHCLKeys(listVal, valid); err != nil { return multierror.Prefix(err, fmt.Sprintf("'%s' ->", n)) @@ -623,6 +624,13 @@ func parseTasks(jobName string, taskGroupName string, result *[]*api.Task, list WeaklyTypedInput: true, Result: &t, }) + + // this needs to be manually assigned + killsig := m["kill_signal"] + if killsig != nil { + t.KillSignal = killsig.(string) + } + if err != nil { return err } diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 9196f4a3c7a..002b3584f1d 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -559,6 +559,30 @@ func TestParse(t *testing.T) { }, false, }, + { + "job-with-kill-signal.hcl", + &api.Job{ + ID: helper.StringToPtr("example"), + Name: helper.StringToPtr("example"), + + TaskGroups: []*api.TaskGroup{ + { + Name: helper.StringToPtr("webservice"), + Tasks: []*api.Task{ + { + Name: "webservice", + Driver: "docker", + KillSignal: "SIGINT", + Config: map[string]interface{}{ + "image": "hashicorp/image", + }, + }, + }, + }, + }, + }, + false, + }, } for _, tc := range cases { diff --git a/nomad/job_endpoint_test.go b/nomad/job_endpoint_test.go index c9357594e55..772c1d4249e 100644 --- a/nomad/job_endpoint_test.go +++ b/nomad/job_endpoint_test.go @@ -3748,6 +3748,38 @@ func TestJobEndpoint_ValidateJob_InvalidSignals(t *testing.T) { } } +func TestJobEndpoint_ValidateJob_KillSignal(t *testing.T) { + assert := assert.New(t) + t.Parallel() + + // test validate fails if the driver does not support sending signals, but a + // stop_signal has been specified + { + job := mock.Job() + job.TaskGroups[0].Tasks[0].Driver = "qemu" // qemu does not support sending signals + job.TaskGroups[0].Tasks[0].KillSignal = "SIGINT" + + err, warnings := validateJob(job) + if err == nil || !strings.Contains(err.Error(), "support sending signals") { + t.Fatalf("Expected signal feasibility error; got %v", err) + } + assert.Nil(warnings) + } + + // test validate succeeds if the driver does support sending signals, and + // a stop_signal has been specified + { + job := mock.Job() + job.TaskGroups[0].Tasks[0].KillSignal = "SIGINT" + + err, warnings := validateJob(job) + if err != nil { + t.Fatalf("Expected error to be nil; got %v", err.Error()) + } + assert.Nil(warnings) + } +} + func TestJobEndpoint_ValidateJobUpdate(t *testing.T) { t.Parallel() old := mock.Job() diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index ab0ab548df8..92be40ccd3c 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1978,6 +1978,11 @@ func (j *Job) RequiredSignals() map[string]map[string][]string { taskSignals[task.Vault.ChangeSignal] = struct{}{} } + // If a user has specified a KillSignal, add it to required signals + if task.KillSignal != "" { + taskSignals[task.KillSignal] = struct{}{} + } + // Check if any template change mode uses signals for _, t := range task.Templates { if t.ChangeMode != TemplateChangeModeSignal { @@ -3221,6 +3226,10 @@ type Task struct { // ShutdownDelay is the duration of the delay between deregistering a // task from Consul and sending it a signal to shutdown. See #2441 ShutdownDelay time.Duration + + // The kill signal to use for the task. This is an optional specification, + // and if not set, the driver will default to SIGINT + KillSignal string } func (t *Task) Copy() *Task { From 8356c6183c92cba1c8bf0a1587bc9ea39cca1a64 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Wed, 6 Dec 2017 12:16:12 -0500 Subject: [PATCH 03/14] extract signal helper into utils --- client/driver/exec.go | 10 +++------- client/driver/raw_exec.go | 10 +++------- client/driver/utils.go | 15 +++++++++++++++ 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/client/driver/exec.go b/client/driver/exec.go index 741f63b87eb..41ab5237b68 100644 --- a/client/driver/exec.go +++ b/client/driver/exec.go @@ -9,7 +9,6 @@ import ( "path/filepath" "time" - "github.com/hashicorp/consul-template/signals" "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-plugin" "github.com/hashicorp/nomad/client/allocdir" @@ -130,12 +129,9 @@ func (d *ExecDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse return nil, fmt.Errorf("failed to set executor context: %v", err) } - var taskKillSignal os.Signal - if task.KillSignal != "" { - taskKillSignal = signals.SignalLookup[task.KillSignal] - if taskKillSignal == nil { - return nil, fmt.Errorf("Signal %s is not supported", task.KillSignal) - } + taskKillSignal, err := getTaskKillSignal(task.KillSignal) + if err != nil { + return nil, err } execCmd := &executor.ExecCommand{ diff --git a/client/driver/raw_exec.go b/client/driver/raw_exec.go index 99208a3441e..86b4406e9a4 100644 --- a/client/driver/raw_exec.go +++ b/client/driver/raw_exec.go @@ -9,7 +9,6 @@ import ( "path/filepath" "time" - "github.com/hashicorp/consul-template/signals" "github.com/hashicorp/go-plugin" "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/config" @@ -145,12 +144,9 @@ func (d *RawExecDriver) Start(ctx *ExecContext, task *structs.Task) (*StartRespo return nil, fmt.Errorf("failed to set executor context: %v", err) } - var taskKillSignal os.Signal - if task.KillSignal != "" { - taskKillSignal = signals.SignalLookup[task.KillSignal] - if taskKillSignal == nil { - return nil, fmt.Errorf("Signal %s is not supported", task.KillSignal) - } + taskKillSignal, err := getTaskKillSignal(task.KillSignal) + if err != nil { + return nil, err } execCmd := &executor.ExecCommand{ diff --git a/client/driver/utils.go b/client/driver/utils.go index 2f456b3b265..072ae9a3d99 100644 --- a/client/driver/utils.go +++ b/client/driver/utils.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/hashicorp/consul-template/signals" "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-plugin" "github.com/hashicorp/nomad/client/allocdir" @@ -204,3 +205,17 @@ func SetEnvvars(envBuilder *env.Builder, fsi cstructs.FSIsolation, taskDir *allo envBuilder.SetHostEnvvars(filter) } } + +// getTaskKillSignal looks up the signal specified for the task if it has been +// specified. If it is not supported on the platform, returns an error. +func getTaskKillSignal(signal string) (os.Signal, error) { + if signal == "" { + return nil, nil + } + + taskKillSignal := signals.SignalLookup[signal] + if taskKillSignal == nil { + return nil, fmt.Errorf("Signal %s is not supported", signal) + } + return taskKillSignal, nil +} From a6936662b6edbb2648a6f71fdcf168b1c4cad3e5 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Wed, 6 Dec 2017 13:33:39 -0500 Subject: [PATCH 04/14] add missing new file --- jobspec/test-fixtures/job-with-kill-signal.hcl | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 jobspec/test-fixtures/job-with-kill-signal.hcl diff --git a/jobspec/test-fixtures/job-with-kill-signal.hcl b/jobspec/test-fixtures/job-with-kill-signal.hcl new file mode 100644 index 00000000000..df85633cc16 --- /dev/null +++ b/jobspec/test-fixtures/job-with-kill-signal.hcl @@ -0,0 +1,12 @@ +job "example" { + task "webservice" { + kill_signal = "SIGINT" + driver = "docker" + config + { + image = "hashicorp/image" + } + } + +} + From 359613e115d2ffba1dbeff2b61561424fa56ccce Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Wed, 6 Dec 2017 13:50:22 -0500 Subject: [PATCH 05/14] fix up basic test add conversion for KillSignal for api/struct representation of task --- command/agent/job_endpoint.go | 3 +++ jobspec/parse_test.go | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index bcddd800a01..9a154679e95 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -666,6 +666,8 @@ func ApiTgToStructsTG(taskGroup *api.TaskGroup, tg *structs.TaskGroup) { } } +// ApiTaskToStructsTask is a copy and type conversion between the API +// representation of a task from a struct representation of a task. func ApiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) { structsTask.Name = apiTask.Name structsTask.Driver = apiTask.Driver @@ -676,6 +678,7 @@ func ApiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) { structsTask.Meta = apiTask.Meta structsTask.KillTimeout = *apiTask.KillTimeout structsTask.ShutdownDelay = apiTask.ShutdownDelay + structsTask.KillSignal = apiTask.KillSignal if l := len(apiTask.Constraints); l != 0 { structsTask.Constraints = make([]*structs.Constraint, l) diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 002b3584f1d..9dc3fdaa126 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -202,7 +202,8 @@ func TestParse(t *testing.T) { RightDelim: helper.StringToPtr("__"), }, }, - Leader: true, + Leader: true, + KillSignal: "", }, { Name: "storagelocker", From 0c17fd5a333fcbf7e65084b85d1a52ffa2fcc63a Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Wed, 6 Dec 2017 15:03:02 -0500 Subject: [PATCH 06/14] use assert library --- nomad/job_endpoint_test.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/nomad/job_endpoint_test.go b/nomad/job_endpoint_test.go index 772c1d4249e..d9ff378194e 100644 --- a/nomad/job_endpoint_test.go +++ b/nomad/job_endpoint_test.go @@ -3760,9 +3760,8 @@ func TestJobEndpoint_ValidateJob_KillSignal(t *testing.T) { job.TaskGroups[0].Tasks[0].KillSignal = "SIGINT" err, warnings := validateJob(job) - if err == nil || !strings.Contains(err.Error(), "support sending signals") { - t.Fatalf("Expected signal feasibility error; got %v", err) - } + assert.NotNil(err) + assert.True(strings.Contains(err.Error(), "support sending signals")) assert.Nil(warnings) } @@ -3773,9 +3772,7 @@ func TestJobEndpoint_ValidateJob_KillSignal(t *testing.T) { job.TaskGroups[0].Tasks[0].KillSignal = "SIGINT" err, warnings := validateJob(job) - if err != nil { - t.Fatalf("Expected error to be nil; got %v", err.Error()) - } + assert.Nil(err) assert.Nil(warnings) } } From 20d1a3b54d53299c66da442a9f6114088a746e2b Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Wed, 6 Dec 2017 16:23:24 -0500 Subject: [PATCH 07/14] fix up test fixture to properly parse --- api/tasks.go | 2 +- jobspec/parse.go | 6 ------ jobspec/parse_test.go | 11 +++++------ jobspec/test-fixtures/job-with-kill-signal.hcl | 18 ++++++++---------- 4 files changed, 14 insertions(+), 23 deletions(-) diff --git a/api/tasks.go b/api/tasks.go index 2cfd7ae20a7..462d9f54931 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -370,7 +370,7 @@ type Task struct { DispatchPayload *DispatchPayloadConfig Leader bool ShutdownDelay time.Duration `mapstructure:"shutdown_delay"` - KillSignal string + KillSignal string `mapstructure:"kill_signal"` } func (t *Task) Canonicalize(tg *TaskGroup, job *Job) { diff --git a/jobspec/parse.go b/jobspec/parse.go index 03efb07a025..f44cb75406f 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -625,12 +625,6 @@ func parseTasks(jobName string, taskGroupName string, result *[]*api.Task, list Result: &t, }) - // this needs to be manually assigned - killsig := m["kill_signal"] - if killsig != nil { - t.KillSignal = killsig.(string) - } - if err != nil { return err } diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 9dc3fdaa126..5922edeff51 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -563,17 +563,16 @@ func TestParse(t *testing.T) { { "job-with-kill-signal.hcl", &api.Job{ - ID: helper.StringToPtr("example"), - Name: helper.StringToPtr("example"), - + ID: helper.StringToPtr("foo"), + Name: helper.StringToPtr("foo"), TaskGroups: []*api.TaskGroup{ { - Name: helper.StringToPtr("webservice"), + Name: helper.StringToPtr("bar"), Tasks: []*api.Task{ { - Name: "webservice", + Name: "bar", Driver: "docker", - KillSignal: "SIGINT", + KillSignal: "SIGQUIT", Config: map[string]interface{}{ "image": "hashicorp/image", }, diff --git a/jobspec/test-fixtures/job-with-kill-signal.hcl b/jobspec/test-fixtures/job-with-kill-signal.hcl index df85633cc16..920042b3b5c 100644 --- a/jobspec/test-fixtures/job-with-kill-signal.hcl +++ b/jobspec/test-fixtures/job-with-kill-signal.hcl @@ -1,12 +1,10 @@ -job "example" { - task "webservice" { - kill_signal = "SIGINT" - driver = "docker" - config - { - image = "hashicorp/image" - } - } +job "foo" { + task "bar" { + driver = "docker" + kill_signal = "SIGQUIT" + config { + image = "hashicorp/image" + } + } } - From b1eda324c6de454cf7d63aed74d8b0a43db85524 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Wed, 6 Dec 2017 16:37:47 -0500 Subject: [PATCH 08/14] code review fixups --- command/agent/job_endpoint_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 8ad40760352..8eb74a9b92b 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -1260,6 +1260,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { "lol": "code", }, KillTimeout: helper.TimeToPtr(10 * time.Second), + KillSignal: "SIGQUIT", LogConfig: &api.LogConfig{ MaxFiles: helper.IntToPtr(10), MaxFileSizeMB: helper.IntToPtr(100), @@ -1455,6 +1456,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { "lol": "code", }, KillTimeout: 10 * time.Second, + KillSignal: "SIGQUIT", LogConfig: &structs.LogConfig{ MaxFiles: 10, MaxFileSizeMB: 100, From 927c0a4d88b434504716fdb7d63b5f32c08e3851 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Wed, 6 Dec 2017 16:52:44 -0500 Subject: [PATCH 09/14] change location of default kill signal --- client/driver/docker.go | 4 +- client/driver/docker_test.go | 97 ++++++++++-------------------- client/driver/executor/executor.go | 9 +-- client/driver/utils.go | 3 +- client/driver/utils_test.go | 35 +++++++++++ 5 files changed, 71 insertions(+), 77 deletions(-) diff --git a/client/driver/docker.go b/client/driver/docker.go index f14aab4af27..5eccf46085c 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -160,6 +160,7 @@ type DockerVolumeDriverConfig struct { Options []map[string]string `mapstructure:"options"` } +// DockerDriverConfig defines the user specified config block in a jobspec type DockerDriverConfig struct { ImageName string `mapstructure:"image"` // Container's Image Name LoadImage string `mapstructure:"load"` // LoadImage is a path to an image archive file @@ -201,7 +202,6 @@ type DockerDriverConfig struct { MacAddress string `mapstructure:"mac_address"` // Pin mac address to container SecurityOpt []string `mapstructure:"security_opt"` // Flags to pass directly to security-opt Devices []DockerDevice `mapstructure:"devices"` // To allow mounting USB or other serial control devices - StopSignal string `mapstructure:"stop_signal"` // allow passing through a specific stop signal } func sliceMergeUlimit(ulimitsRaw map[string]string) ([]docker.ULimit, error) { @@ -1046,6 +1046,7 @@ func (d *DockerDriver) createContainerConfig(ctx *ExecContext, task *structs.Tas return c, err } + // create the config block that will later be consumed by go-dockerclient config := &docker.Config{ Image: d.imageID, Hostname: driverConfig.Hostname, @@ -1055,7 +1056,6 @@ func (d *DockerDriver) createContainerConfig(ctx *ExecContext, task *structs.Tas StopTimeout: int(task.KillTimeout.Seconds()), } - // Set the stop signal for the docker container. if task.KillSignal != "" { config.StopSignal = task.KillSignal } diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 0e563cc697d..b0b5b64b9bc 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -2055,75 +2055,40 @@ func TestDockerDriver_Kill(t *testing.T) { t.Skip("Docker not connected") } - // Tasks started with a signal that is not supported should error - { - task := &structs.Task{ - Name: "nc-demo", - Driver: "docker", - KillSignal: "ABCDEF", - Config: map[string]interface{}{ - "load": "busybox.tar", - "image": "busybox", - "command": "/bin/nc", - "args": []string{"-l", "127.0.0.1", "-p", "0"}, - }, - LogConfig: &structs.LogConfig{ - MaxFiles: 10, - MaxFileSizeMB: 10, - }, - Resources: basicResources, - } - - ctx := testDockerDriverContexts(t, task) - defer ctx.AllocDir.Destroy() - d := NewDockerDriver(ctx.DriverCtx) - copyImage(t, ctx.ExecCtx.TaskDir, "busybox.tar") - - _, err := d.Prestart(ctx.ExecCtx, task) - if err != nil { - t.Fatalf("error in prestart: %v", err) - } - - _, err = d.Start(ctx.ExecCtx, task) - assert.NotNil(err) - } - // Tasks started with a signal that is not supported should not error - { - task := &structs.Task{ - Name: "nc-demo", - Driver: "docker", - KillSignal: "SIGQUIT", - Config: map[string]interface{}{ - "load": "busybox.tar", - "image": "busybox", - "command": "/bin/nc", - "args": []string{"-l", "127.0.0.1", "-p", "0"}, - }, - LogConfig: &structs.LogConfig{ - MaxFiles: 10, - MaxFileSizeMB: 10, - }, - Resources: basicResources, - } + task := &structs.Task{ + Name: "nc-demo", + Driver: "docker", + KillSignal: "SIGKILL", + Config: map[string]interface{}{ + "load": "busybox.tar", + "image": "busybox", + "command": "/bin/nc", + "args": []string{"-l", "127.0.0.1", "-p", "0"}, + }, + LogConfig: &structs.LogConfig{ + MaxFiles: 10, + MaxFileSizeMB: 10, + }, + Resources: basicResources, + } - ctx := testDockerDriverContexts(t, task) - defer ctx.AllocDir.Destroy() - d := NewDockerDriver(ctx.DriverCtx) - copyImage(t, ctx.ExecCtx.TaskDir, "busybox.tar") + ctx := testDockerDriverContexts(t, task) + defer ctx.AllocDir.Destroy() + d := NewDockerDriver(ctx.DriverCtx) + copyImage(t, ctx.ExecCtx.TaskDir, "busybox.tar") - _, err := d.Prestart(ctx.ExecCtx, task) - if err != nil { - t.Fatalf("error in prestart: %v", err) - } + _, err := d.Prestart(ctx.ExecCtx, task) + if err != nil { + t.Fatalf("error in prestart: %v", err) + } - resp, err := d.Start(ctx.ExecCtx, task) - assert.Nil(err) - assert.NotNil(resp.Handle) + resp, err := d.Start(ctx.ExecCtx, task) + assert.Nil(err) + assert.NotNil(resp.Handle) - handle := resp.Handle.(*DockerHandle) - waitForExist(t, client, handle) - err = handle.Kill() - assert.Nil(err) - } + handle := resp.Handle.(*DockerHandle) + waitForExist(t, client, handle) + err = handle.Kill() + assert.Nil(err) } diff --git a/client/driver/executor/executor.go b/client/driver/executor/executor.go index 0bfc472757e..63ceda90fc2 100644 --- a/client/driver/executor/executor.go +++ b/client/driver/executor/executor.go @@ -500,14 +500,7 @@ func (e *UniversalExecutor) ShutDown() error { return nil } - var osSignal os.Signal - if e.command.TaskKillSignal != nil { - osSignal = e.command.TaskKillSignal - } else { - osSignal = os.Interrupt - } - - if err = proc.Signal(osSignal); err != nil && err.Error() != finishedErr { + if err = proc.Signal(e.command.TaskKillSignal); err != nil && err.Error() != finishedErr { return fmt.Errorf("executor.shutdown error: %v", err) } diff --git a/client/driver/utils.go b/client/driver/utils.go index 072ae9a3d99..23a64c89c84 100644 --- a/client/driver/utils.go +++ b/client/driver/utils.go @@ -210,12 +210,13 @@ func SetEnvvars(envBuilder *env.Builder, fsi cstructs.FSIsolation, taskDir *allo // specified. If it is not supported on the platform, returns an error. func getTaskKillSignal(signal string) (os.Signal, error) { if signal == "" { - return nil, nil + return os.Interrupt, nil } taskKillSignal := signals.SignalLookup[signal] if taskKillSignal == nil { return nil, fmt.Errorf("Signal %s is not supported", signal) } + return taskKillSignal, nil } diff --git a/client/driver/utils_test.go b/client/driver/utils_test.go index 6841ac68eeb..e7f934732b9 100644 --- a/client/driver/utils_test.go +++ b/client/driver/utils_test.go @@ -1,8 +1,13 @@ package driver import ( + "os" + "runtime" + "syscall" "testing" "time" + + "github.com/stretchr/testify/assert" ) func TestDriver_KillTimeout(t *testing.T) { @@ -21,3 +26,33 @@ func TestDriver_KillTimeout(t *testing.T) { t.Fatalf("KillTimeout() returned %v; want %v", actual, expected) } } + +func TestDriver_getTaskKillSignal(t *testing.T) { + assert := assert.New(t) + t.Parallel() + + if runtime.GOOS != "linux" { + t.Skip("Linux only test") + } + + // Test that the default is SIGINT + { + sig, err := getTaskKillSignal("") + assert.Nil(err) + assert.Equal(sig, os.Interrupt) + } + + // Test that unsupported signals return an error + { + _, err := getTaskKillSignal("ABCDEF") + assert.NotNil(err) + assert.Contains(err.Error(), "Signal ABCDEF is not supported") + } + + // Test that supported signals return that signal + { + sig, err := getTaskKillSignal("SIGKILL") + assert.Nil(err) + assert.Equal(sig, syscall.SIGKILL) + } +} From 27b666e9355077136c37e19338f6914eb67b949d Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Thu, 7 Dec 2017 10:07:00 -0500 Subject: [PATCH 10/14] extend configurable kill signal to java driver --- client/driver/java.go | 6 +++ client/driver/java_test.go | 83 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/client/driver/java.go b/client/driver/java.go index 40c4789bef1..0c660547cb6 100644 --- a/client/driver/java.go +++ b/client/driver/java.go @@ -265,12 +265,18 @@ func (d *JavaDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse return nil, err } + taskKillSignal, err := getTaskKillSignal(task.KillSignal) + if err != nil { + return nil, err + } + execCmd := &executor.ExecCommand{ Cmd: absPath, Args: args, FSIsolation: true, ResourceLimits: true, User: getExecutorUser(task), + TaskKillSignal: taskKillSignal, } ps, err := execIntf.LaunchCmd(execCmd) if err != nil { diff --git a/client/driver/java_test.go b/client/driver/java_test.go index edae22a538e..b1f6dc3f166 100644 --- a/client/driver/java_test.go +++ b/client/driver/java_test.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" + "github.com/stretchr/testify/assert" ctestutils "github.com/hashicorp/nomad/client/testutil" ) @@ -432,3 +433,85 @@ func TestJavaDriver_Start_Wait_Class(t *testing.T) { t.Fatalf("Error: %s", err) } } + +func TestJavaDriver_Start_Kill(t *testing.T) { + assert := assert.New(t) + + if !testutil.IsTravis() { + t.Parallel() + } + if !javaLocated() { + t.Skip("Java not found; skipping") + } + + // Test that a valid kill signal will successfully stop the process + { + ctestutils.JavaCompatible(t) + task := &structs.Task{ + Name: "demo-app", + Driver: "java", + KillSignal: "SIGKILL", + Config: map[string]interface{}{ + "jar_path": "demoapp.jar", + "args": []string{"5"}, + }, + LogConfig: &structs.LogConfig{ + MaxFiles: 10, + MaxFileSizeMB: 10, + }, + Resources: basicResources, + } + + ctx := testDriverContexts(t, task) + defer ctx.AllocDir.Destroy() + d := NewJavaDriver(ctx.DriverCtx) + + // Copy the test jar into the task's directory + dst := ctx.ExecCtx.TaskDir.Dir + copyFile("./test-resources/java/demoapp.jar", filepath.Join(dst, "demoapp.jar"), t) + + _, err := d.Prestart(ctx.ExecCtx, task) + assert.Nil(err) + + resp, err := d.Start(ctx.ExecCtx, task) + assert.Nil(err) + + assert.NotNil(resp.Handle) + err = resp.Handle.Kill() + assert.Nil(err) + } + + // Test that an unsupported kill signal will return an error + { + ctestutils.JavaCompatible(t) + task := &structs.Task{ + Name: "demo-app", + Driver: "java", + KillSignal: "ABCDEF", + Config: map[string]interface{}{ + "jar_path": "demoapp.jar", + "args": []string{"5"}, + }, + LogConfig: &structs.LogConfig{ + MaxFiles: 10, + MaxFileSizeMB: 10, + }, + Resources: basicResources, + } + + ctx := testDriverContexts(t, task) + defer ctx.AllocDir.Destroy() + d := NewJavaDriver(ctx.DriverCtx) + + // Copy the test jar into the task's directory + dst := ctx.ExecCtx.TaskDir.Dir + copyFile("./test-resources/java/demoapp.jar", filepath.Join(dst, "demoapp.jar"), t) + + _, err := d.Prestart(ctx.ExecCtx, task) + assert.Nil(err) + + _, err = d.Start(ctx.ExecCtx, task) + assert.NotNil(err) + assert.Contains(err.Error(), "Signal ABCDEF is not supported") + } +} From f09e3ec798fc3239c92f3558292ac17317247440 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Thu, 7 Dec 2017 10:33:22 -0500 Subject: [PATCH 11/14] set default kill signal on executor shutdown --- client/driver/executor/executor.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/client/driver/executor/executor.go b/client/driver/executor/executor.go index 63ceda90fc2..2307b06109a 100644 --- a/client/driver/executor/executor.go +++ b/client/driver/executor/executor.go @@ -500,7 +500,16 @@ func (e *UniversalExecutor) ShutDown() error { return nil } - if err = proc.Signal(e.command.TaskKillSignal); err != nil && err.Error() != finishedErr { + // Set default kill signal, as some drivers don't support configurable + // signals (such as rkt) + var osSignal os.Signal + if e.command.TaskKillSignal != nil { + osSignal = e.command.TaskKillSignal + } else { + osSignal = os.Interrupt + } + + if err = proc.Signal(osSignal); err != nil && err.Error() != finishedErr { return fmt.Errorf("executor.shutdown error: %v", err) } From 7d8dda89429aa10308fff31eb1eef589bf97e056 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Thu, 7 Dec 2017 10:45:21 -0500 Subject: [PATCH 12/14] add test for kill signal in required signals update changelog --- CHANGELOG.md | 3 ++- nomad/structs/structs.go | 4 +++- nomad/structs/structs_test.go | 24 ++++++++++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a5476d5f0b..1757c75bb04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ __BACKWARDS INCOMPATIBILITIES:__ IMPROVEMENTS: * core: Allow operators to reload TLS certificate and key files via SIGHUP [GH-3479] + * core: allow configurable stop signals for a task, when drivers support + sending stop signals. [GH-1755] * core: Allow agents to be run in `rpc_upgrade_mode` when migrating a cluster to TLS rather than changing `heartbeat_grace` * api: Allocations now track and return modify time in addition to create time @@ -30,7 +32,6 @@ IMPROVEMENTS: * driver/docker: Adds support for `ulimit` and `sysctl` options [GH-3568] * driver/docker: Adds support for StopTimeout (set to the same value as kill_timeout [GH-3601] - * driver/exec: allow controlling the stop signal in exec/raw_exec [GH-1755] * driver/rkt: Add support for passing through user [GH-3612] * driver/qemu: Support graceful shutdowns on unix platforms [GH-3411] * template: Updated to consul template 0.19.4 [GH-3543] diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 92be40ccd3c..1836aae828f 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -3228,7 +3228,9 @@ type Task struct { ShutdownDelay time.Duration // The kill signal to use for the task. This is an optional specification, - // and if not set, the driver will default to SIGINT + + // KillSignal is the kill signal to use for the task. This is an optional + // specification and defaults to SIGINT KillSignal string } diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index 3451e8d88a0..3840b9cf40e 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -806,6 +806,26 @@ func TestJob_RequiredSignals(t *testing.T) { }, } + j2 := &Job{ + TaskGroups: []*TaskGroup{ + { + Name: "foo", + Tasks: []*Task{ + { + Name: "t1", + KillSignal: "SIGQUIT", + }, + }, + }, + }, + } + + e2 := map[string]map[string][]string{ + "foo": { + "t1": {"SIGQUIT"}, + }, + } + cases := []struct { Job *Job Expected map[string]map[string][]string @@ -818,6 +838,10 @@ func TestJob_RequiredSignals(t *testing.T) { Job: j1, Expected: e1, }, + { + Job: j2, + Expected: e2, + }, } for i, c := range cases { From 73a4a4620dcaa1d9eb75b162f8ca41d00454d01c Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Thu, 7 Dec 2017 12:33:05 -0500 Subject: [PATCH 13/14] add documention --- website/source/docs/job-specification/task.html.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/website/source/docs/job-specification/task.html.md b/website/source/docs/job-specification/task.html.md index a61590ab312..e2a3a89100a 100644 --- a/website/source/docs/job-specification/task.html.md +++ b/website/source/docs/job-specification/task.html.md @@ -54,6 +54,11 @@ job "docs" { [`max_kill_timeout`][max_kill] on the agent running the task, which has a default value of 30 seconds. +- `kill_signal` `(string)` - Specifies a configurable kill signal for a task, + where the default is SIGINT. Note tha this is only supported for drivers + which accept sending signals (currently docker, exec, raw_exec, and java + drivers). + - `leader` `(bool: false)` - Specifies whether the task is the leader task of the task group. If set to true, when the leader task completes, all other tasks within the task group will be gracefully shutdown. From ae10b638db20c3430b3e62d325dc97a662f2aa68 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Thu, 7 Dec 2017 13:46:25 -0500 Subject: [PATCH 14/14] code review fixes --- CHANGELOG.md | 4 ++-- client/driver/docker.go | 5 +---- website/source/docs/job-specification/task.html.md | 4 ++-- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1757c75bb04..147143aba73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,8 +11,8 @@ __BACKWARDS INCOMPATIBILITIES:__ IMPROVEMENTS: * core: Allow operators to reload TLS certificate and key files via SIGHUP [GH-3479] - * core: allow configurable stop signals for a task, when drivers support - sending stop signals. [GH-1755] + * core: Allow configurable stop signals for a task, when drivers support + sending stop signals [GH-1755] * core: Allow agents to be run in `rpc_upgrade_mode` when migrating a cluster to TLS rather than changing `heartbeat_grace` * api: Allocations now track and return modify time in addition to create time diff --git a/client/driver/docker.go b/client/driver/docker.go index 5eccf46085c..d2b3827c064 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -1054,10 +1054,7 @@ func (d *DockerDriver) createContainerConfig(ctx *ExecContext, task *structs.Tas Tty: driverConfig.TTY, OpenStdin: driverConfig.Interactive, StopTimeout: int(task.KillTimeout.Seconds()), - } - - if task.KillSignal != "" { - config.StopSignal = task.KillSignal + StopSignal: task.KillSignal, } if driverConfig.WorkDir != "" { diff --git a/website/source/docs/job-specification/task.html.md b/website/source/docs/job-specification/task.html.md index e2a3a89100a..f2340629702 100644 --- a/website/source/docs/job-specification/task.html.md +++ b/website/source/docs/job-specification/task.html.md @@ -55,8 +55,8 @@ job "docs" { default value of 30 seconds. - `kill_signal` `(string)` - Specifies a configurable kill signal for a task, - where the default is SIGINT. Note tha this is only supported for drivers - which accept sending signals (currently docker, exec, raw_exec, and java + where the default is SIGINT. Note that this is only supported for drivers + which accept sending signals (currently Docker, exec, raw_exec, and Java drivers). - `leader` `(bool: false)` - Specifies whether the task is the leader task of