Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create new process group on process startup. #3572

Merged
merged 4 commits into from
Apr 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion client/driver/executor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,11 @@ func (e *UniversalExecutor) LaunchCmd(command *ExecCommand) (*ProcessState, erro
// set the task dir as the working directory for the command
e.cmd.Dir = e.ctx.TaskDir

// start command in separate process group
if err := e.setNewProcessGroup(); err != nil {
return nil, err
}

// configuring the chroot, resource container, and start the plugin
// process in the chroot.
if err := e.configureIsolation(); err != nil {
Expand Down Expand Up @@ -435,6 +440,10 @@ var (
// finishedErr is the error message received when trying to kill and already
// exited process.
finishedErr = "os: process already finished"

// noSuchProcessErr is the error message received when trying to kill a non
// existing process (e.g. when killing a process group).
noSuchProcessErr = "no such process"
)

// ClientCleanup is the cleanup routine that a Nomad Client uses to remove the
Expand All @@ -443,6 +452,20 @@ func ClientCleanup(ic *dstructs.IsolationConfig, pid int) error {
return clientCleanup(ic, pid)
}

// Cleanup any still hanging user processes
func (e *UniversalExecutor) cleanupChildProcesses(proc *os.Process) error {
// If new process group was created upon command execution
// we can kill the whole process group now to cleanup any leftovers.
if e.cmd.SysProcAttr != nil && e.cmd.SysProcAttr.Setpgid {
if err := syscall.Kill(-proc.Pid, syscall.SIGKILL); err != nil && err.Error() != noSuchProcessErr {
return err
}
return nil
} else {
return proc.Kill()
}
}

// Exit cleans up the alloc directory, destroys resource container and kills the
// user process
func (e *UniversalExecutor) Exit() error {
Expand Down Expand Up @@ -470,7 +493,7 @@ func (e *UniversalExecutor) Exit() error {
if err != nil {
e.logger.Printf("[ERR] executor: can't find process with pid: %v, err: %v",
e.cmd.Process.Pid, err)
} else if err := proc.Kill(); err != nil && err.Error() != finishedErr {
} else if err := e.cleanupChildProcesses(proc); err != nil && err.Error() != finishedErr {
merr.Errors = append(merr.Errors,
fmt.Errorf("can't kill process with pid: %v, err: %v", e.cmd.Process.Pid, err))
}
Expand Down
15 changes: 15 additions & 0 deletions client/driver/executor/executor_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ package executor
import (
"fmt"
"io"
"runtime"
"syscall"

syslog "github.com/RackSec/srslog"

Expand Down Expand Up @@ -47,3 +49,16 @@ func (e *UniversalExecutor) collectLogs(we io.Writer, wo io.Writer) {
}
}
}

// configure new process group for child process
func (e *UniversalExecutor) setNewProcessGroup() error {
// We need to check that as build flags includes windows for this file
if runtime.GOOS == "windows" {
return nil
}
if e.cmd.SysProcAttr == nil {
e.cmd.SysProcAttr = &syscall.SysProcAttr{}
}
e.cmd.SysProcAttr.Setpgid = true
return nil
}