drivers/docker: add support for STOPSIGNAL

This fixes a bug where Nomad overrides a Dockerfile's STOPSIGNAL with
the default kill_signal (SIGTERM).

This adds a check for kill_signal. If it's not set, it calls
StopContainer instead of Signal, which uses STOPSIGNAL if it's
specified. If both kill_signal and STOPSIGNAL are set, Nomad tries to
stop the container with kill_signal first, before then calling
StopContainer.

Fixes #9989

drivers/docker/driver.go

@@ -391,6 +391,7 @@ CREATE:
 		waitCh:                make(chan struct{}),
 		removeContainerOnExit: d.config.GC.Container,
 		net:                   net,
+		stopSignal:            container.Config.StopSignal,
 	}
 
 	if err := handle.SetDriverState(h.buildState()); err != nil {
@@ -1375,40 +1376,13 @@ func (d *Driver) handleWait(ctx context.Context, ch chan *drivers.ExitResult, h
 	}
 }
 
-// parseSignal interprets the signal name into an os.Signal. If no name is
-// provided, the docker driver defaults to SIGTERM. If the OS is Windows and
-// SIGINT is provided, the signal is converted to SIGTERM.
-func (d *Driver) parseSignal(os, signal string) (os.Signal, error) {
-	// Unlike other drivers, docker defaults to SIGTERM, aiming for consistency
-	// with the 'docker stop' command.
-	// https://docs.docker.com/engine/reference/commandline/stop/#extended-description
-	if signal == "" {
-		signal = "SIGTERM"
-	}
-
-	// Windows Docker daemon does not support SIGINT, SIGTERM is the semantic equivalent that
-	// allows for graceful shutdown before being followed up by a SIGKILL.
-	// Supported signals:
-	//   https://github.com/moby/moby/blob/0111ee70874a4947d93f64b672f66a2a35071ee2/pkg/signal/signal_windows.go#L17-L26
-	if os == "windows" && signal == "SIGINT" {
-		signal = "SIGTERM"
-	}
-
-	return signals.Parse(signal)
-}
-
 func (d *Driver) StopTask(taskID string, timeout time.Duration, signal string) error {
 	h, ok := d.tasks.Get(taskID)
 	if !ok {
 		return drivers.ErrTaskNotFound
 	}
 
-	sig, err := d.parseSignal(runtime.GOOS, signal)
-	if err != nil {
-		return fmt.Errorf("failed to parse signal: %v", err)
-	}
-
-	return h.Kill(timeout, sig)
+	return h.Kill(timeout, signal)
 }
 
 func (d *Driver) DestroyTask(taskID string, force bool) error {

drivers/docker/handle.go

@@ -3,13 +3,15 @@ package docker
 import (
 	"fmt"
 	"os"
+	"runtime"
 	"strings"
 	"sync"
 	"syscall"
 	"time"
 
 	"github.com/armon/circbuf"
 	docker "github.com/fsouza/go-dockerclient"
+	"github.com/hashicorp/consul-template/signals"
 	hclog "github.com/hashicorp/go-hclog"
 	plugin "github.com/hashicorp/go-plugin"
 	"github.com/hashicorp/nomad/drivers/docker/docklog"
@@ -31,6 +33,7 @@ type taskHandle struct {
 	waitCh                chan struct{}
 	removeContainerOnExit bool
 	net                   *drivers.DriverNetwork
+	stopSignal            string
 
 	exitResult     *drivers.ExitResult
 	exitResultLock sync.Mutex
@@ -121,17 +124,49 @@ func (h *taskHandle) Signal(ctx context.Context, s os.Signal) error {
 		Context: ctx,
 	}
 	return h.client.KillContainer(opts)
+}
+
+// parseSignal interprets the signal name into an os.Signal. If no name is
+// provided, the docker driver defaults to SIGTERM. If the OS is Windows and
+// SIGINT is provided, the signal is converted to SIGTERM.
+func (h *taskHandle) parseSignal(os, signal string) (os.Signal, error) {
+	// Unlike other drivers, docker defaults to SIGTERM, aiming for consistency
+	// with the 'docker stop' command.
+	// https://docs.docker.com/engine/reference/commandline/stop/#extended-description
+	if signal == "" {
+		signal = "SIGTERM"
+	}
+
+	// Windows Docker daemon does not support SIGINT, SIGTERM is the semantic equivalent that
+	// allows for graceful shutdown before being followed up by a SIGKILL.
+	// Supported signals:
+	//   https://github.com/moby/moby/blob/0111ee70874a4947d93f64b672f66a2a35071ee2/pkg/signal/signal_windows.go#L17-L26
+	if os == "windows" && signal == "SIGINT" {
+		signal = "SIGTERM"
+	}
 
+	return signals.Parse(signal)
 }
 
 // Kill is used to terminate the task.
-func (h *taskHandle) Kill(killTimeout time.Duration, signal os.Signal) error {
-	// Only send signal if killTimeout is set, otherwise stop container
-	if killTimeout > 0 {
+func (h *taskHandle) Kill(killTimeout time.Duration, signal string) error {
+	var err error
+	// Calling StopContainer lets docker handle the stop signal (specified
+	// in the Dockerfile or defaulting to SIGTERM). If kill_signal is specified,
+	// Signal is used to kill the container with the desired signal before
+	// calling StopContainer
+	if signal == "" {
+		err = h.client.StopContainer(h.containerID, uint(killTimeout.Seconds()))
+	} else {
 		ctx, cancel := context.WithTimeout(context.Background(), killTimeout)
 		defer cancel()
 
-		if err := h.Signal(ctx, signal); err != nil {
+		sig, parseErr := h.parseSignal(runtime.GOOS, signal)
+		if parseErr != nil {
+			return fmt.Errorf("failed to parse signal: %v", parseErr)
+		}
+
+		if err := h.Signal(ctx, sig); err != nil {
 			// Container has already been removed.
 			if strings.Contains(err.Error(), NoSuchContainerError) {
 				h.logger.Debug("attempted to signal nonexistent container")
@@ -152,12 +187,12 @@ func (h *taskHandle) Kill(killTimeout time.Duration, signal os.Signal) error {
 			return nil
 		case <-ctx.Done():
 		}
+
+		// Stop the container
+		err = h.client.StopContainer(h.containerID, 0)
 	}
 
-	// Stop the container
-	err := h.client.StopContainer(h.containerID, 0)
 	if err != nil {
-
 		// Container has already been removed.
 		if strings.Contains(err.Error(), NoSuchContainerError) {
 			h.logger.Debug("attempted to stop nonexistent container")