Minor Windows executor cleanups

CHANGELOG.md

@@ -4,7 +4,8 @@ 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)]
+ * driver/raw_exec: Create process group for Windows process and send
+   Ctrl-Break signal on Shutdown [[GH-4153](https://github.com/hashicorp/nomad/issues/4153)]
 
 ## 0.8.1 (April 17, 2018)
 

client/driver/executor/executor.go

@@ -3,6 +3,7 @@ package executor
 import (
 	"context"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"log"
 	"net"
@@ -16,6 +17,7 @@ import (
 	"syscall"
 	"time"
 
+	syslog "github.com/RackSec/srslog"
 	"github.com/armon/circbuf"
 	"github.com/hashicorp/go-multierror"
 	"github.com/mitchellh/go-ps"
@@ -288,6 +290,45 @@ func (e *UniversalExecutor) LaunchCmd(command *ExecCommand) (*ProcessState, erro
 	return &ProcessState{Pid: e.cmd.Process.Pid, ExitCode: -1, IsolationConfig: ic, Time: time.Now()}, nil
 }
 
+// LaunchSyslogServer starts the syslog compatible listener to provide logging
+// to subprocesses.
+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'})
+		}
+	}
+}
+
 // Exec a command inside a container for exec and java drivers.
 func (e *UniversalExecutor) Exec(deadline time.Time, name string, args []string) ([]byte, int, error) {
 	ctx, cancel := context.WithDeadline(context.Background(), deadline)

client/driver/executor/executor_unix.go

@@ -4,52 +4,10 @@ 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
@@ -69,9 +27,8 @@ func (e *UniversalExecutor) cleanupChildProcesses(proc *os.Process) error {
 			return err
 		}
 		return nil
-	} else {
-		return proc.Kill()
 	}
+	return proc.Kill()
 }
 
 func (e *UniversalExecutor) shutdownProcess(proc *os.Process) error {

client/driver/executor/executor_windows.go

@@ -1,58 +1,13 @@
-// +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
+// setNewProcessGroup sets process group flag for the command about to be run.
 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{}
 	}