Merge pull request #4168 from ninoles/b-2117-windows-group-process

B 2117 windows group process
hashicorp · Apr 19, 2018 · d6de69b · d6de69b
2 parents 73b94f5 + 3eba6ca
commit d6de69b
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 90 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,7 +4,7 @@ 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/exec: Create process group for Windows process and send Ctrl-Break signal on Shutdown [[GH-4153](https://github.com/hashicorp/nomad/pull/4153)]
 
 ## 0.8.1 (April 17, 2018)
 

diff --git a/client/driver/executor/executor.go b/client/driver/executor/executor.go
@@ -3,6 +3,7 @@ package executor
 import (
 	"context"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"log"
 	"net"
@@ -28,6 +29,8 @@ import (
 	shelpers "github.com/hashicorp/nomad/helper/stats"
 	"github.com/hashicorp/nomad/nomad/structs"
 
+	syslog "github.com/RackSec/srslog"
+
 	dstructs "github.com/hashicorp/nomad/client/driver/structs"
 	cstructs "github.com/hashicorp/nomad/client/structs"
 )
@@ -777,3 +780,40 @@ func (e *UniversalExecutor) Signal(s os.Signal) error {
 
 	return nil
 }
+
+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'})
+		}
+	}
+}
diff --git a/client/driver/executor/executor_unix.go b/client/driver/executor/executor_unix.go
@@ -4,55 +4,12 @@ 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{}
 	}
@@ -69,11 +26,12 @@ func (e *UniversalExecutor) cleanupChildProcesses(proc *os.Process) error {
 			return err
 		}
 		return nil
-	} else {
-		return proc.Kill()
 	}
+	return proc.Kill()
 }
 
+// Only send the process a shutdown signal (default INT), doesn't
+// necessarily kill it.
 func (e *UniversalExecutor) shutdownProcess(proc *os.Process) error {
 	// Set default kill signal, as some drivers don't support configurable
 	// signals (such as rkt)

diff --git a/client/driver/executor/executor_windows.go b/client/driver/executor/executor_windows.go
@@ -1,55 +1,11 @@
-// +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
@@ -101,6 +57,8 @@ func sendCtrlBreak(pid int) error {
 	return nil
 }
 
+// Send the process a Ctrl-Break event, allowing it to shutdown by itself
+// before being Terminate.
 func (e *UniversalExecutor) shutdownProcess(proc *os.Process) error {
 	if err := sendCtrlBreak(proc.Pid); err != nil {
 		return fmt.Errorf("executor.shutdown error: %v", err)