diff --git a/CHANGELOG.md b/CHANGELOG.md index bfd695588ecc..c904ce1f152a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,9 @@ IMPROVEMENTS: * client: Create new process group on process startup. [[GH-3572](https://github.com/hashicorp/nomad/issues/3572)] +BUG FIXES: + * driver/exec: Create process group for Windows process and send Ctrl-Break signal on Shutdown [[GH-2117](https://github.com/hashicorp/nomad/issues/2117)] + ## 0.8.0 (April 12, 2018) __BACKWARDS INCOMPATIBILITIES:__ diff --git a/client/driver/executor/executor.go b/client/driver/executor/executor.go index e8db0905f455..386619f0f3f2 100644 --- a/client/driver/executor/executor.go +++ b/client/driver/executor/executor.go @@ -452,20 +452,6 @@ 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 { @@ -516,27 +502,7 @@ func (e *UniversalExecutor) ShutDown() error { if err != nil { return fmt.Errorf("executor.shutdown failed to find process: %v", err) } - if runtime.GOOS == "windows" { - if err := proc.Kill(); err != nil && err.Error() != finishedErr { - return err - } - return nil - } - - // 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) - } - - return nil + return e.shutdownProcess(proc) } // pidStats returns the resource usage stats per pid diff --git a/client/driver/executor/executor_unix.go b/client/driver/executor/executor_unix.go index 81b79e6f4bd0..875ef9edbb5a 100644 --- a/client/driver/executor/executor_unix.go +++ b/client/driver/executor/executor_unix.go @@ -1,11 +1,11 @@ -// +build darwin dragonfly freebsd linux netbsd openbsd solaris windows +// +build darwin dragonfly freebsd linux netbsd openbsd solaris package executor import ( "fmt" "io" - "runtime" + "os" "syscall" syslog "github.com/RackSec/srslog" @@ -53,12 +53,40 @@ 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 } + +// 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() + } +} + +func (e *UniversalExecutor) shutdownProcess(proc *os.Process) error { + // 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) + } + + return nil +} diff --git a/client/driver/executor/executor_windows.go b/client/driver/executor/executor_windows.go new file mode 100644 index 000000000000..0a0cd4fbb700 --- /dev/null +++ b/client/driver/executor/executor_windows.go @@ -0,0 +1,111 @@ +// +build windows + +package executor + +import ( + "fmt" + "io" + "os" + "syscall" + + syslog "github.com/RackSec/srslog" + + "github.com/hashicorp/nomad/client/driver/logging" +) + +func (e *UniversalExecutor) LaunchSyslogServer() (*SyslogServerState, error) { + // Ensure the context has been set first + if e.ctx == nil { + return nil, fmt.Errorf("SetContext must be called before launching the Syslog Server") + } + + e.syslogChan = make(chan *logging.SyslogMessage, 2048) + l, err := e.getListener(e.ctx.PortLowerBound, e.ctx.PortUpperBound) + if err != nil { + return nil, err + } + e.logger.Printf("[DEBUG] syslog-server: launching syslog server on addr: %v", l.Addr().String()) + if err := e.configureLoggers(); err != nil { + return nil, err + } + + e.syslogServer = logging.NewSyslogServer(l, e.syslogChan, e.logger) + go e.syslogServer.Start() + go e.collectLogs(e.lre, e.lro) + syslogAddr := fmt.Sprintf("%s://%s", l.Addr().Network(), l.Addr().String()) + return &SyslogServerState{Addr: syslogAddr}, nil +} + +func (e *UniversalExecutor) collectLogs(we io.Writer, wo io.Writer) { + for logParts := range e.syslogChan { + // If the severity of the log line is err then we write to stderr + // otherwise all messages go to stdout + if logParts.Severity == syslog.LOG_ERR { + e.lre.Write(logParts.Message) + e.lre.Write([]byte{'\n'}) + } else { + e.lro.Write(logParts.Message) + e.lro.Write([]byte{'\n'}) + } + } +} + +// 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 e.cmd.SysProcAttr == nil { + e.cmd.SysProcAttr = &syscall.SysProcAttr{} + } + e.cmd.SysProcAttr.CreationFlags = syscall.CREATE_NEW_PROCESS_GROUP + return nil +} + +// Cleanup any still hanging user processes +func (e *UniversalExecutor) cleanupChildProcesses(proc *os.Process) error { + // We must first verify if the process is still running. + // (Windows process often lingered around after being reported as killed). + handle, err := syscall.OpenProcess(syscall.PROCESS_TERMINATE|syscall.SYNCHRONIZE|syscall.PROCESS_QUERY_INFORMATION, false, uint32(proc.Pid)) + if err != nil { + return os.NewSyscallError("OpenProcess", err) + } + defer syscall.CloseHandle(handle) + + result, err := syscall.WaitForSingleObject(syscall.Handle(handle), 0) + + switch result { + case syscall.WAIT_OBJECT_0: + return nil + case syscall.WAIT_TIMEOUT: + // Process still running. Just kill it. + return proc.Kill() + default: + return os.NewSyscallError("WaitForSingleObject", err) + } +} + +// Send a Ctrl-Break signal for shutting down the process, +// like in https://golang.org/src/os/signal/signal_windows_test.go +func sendCtrlBreak(pid int) error { + dll, err := syscall.LoadDLL("kernel32.dll") + if err != nil { + return fmt.Errorf("Error loading kernel32.dll: %v", err) + } + proc, err := dll.FindProc("GenerateConsoleCtrlEvent") + if err != nil { + return fmt.Errorf("Cannot find procedure GenerateConsoleCtrlEvent: %v", err) + } + result, _, err := proc.Call(syscall.CTRL_BREAK_EVENT, uintptr(pid)) + if result == 0 { + return fmt.Errorf("Error sending ctrl-break event: %v", err) + } + return nil +} + +func (e *UniversalExecutor) shutdownProcess(proc *os.Process) error { + if err := sendCtrlBreak(proc.Pid); err != nil { + return fmt.Errorf("executor.shutdown error: %v", err) + } + e.logger.Printf("Sent Ctrl-Break to process %v", proc.Pid) + + return nil +}