Merge pull request #157 from docker/kill_child_process_on_cancel

Kill child process "docker-classic" on cancel

Author: rumpl

Makefile

@@ -66,7 +66,7 @@ lint: ## run linter(s)
 	--target lint
 
 classic-link: ## create docker-classic symlink if does not already exist
-	ln -s /usr/local/bin/docker-classic /Applications/Docker.app/Contents/Resources/bin/docker
+	ln -s /Applications/Docker.app/Contents/Resources/bin/docker /usr/local/bin/docker-classic
 
 help: ## Show help
 	@echo Please specify a build target. The choices are:

cli/main.go

@@ -186,7 +186,7 @@ func execMoby(ctx context.Context) {
 	// Only run original docker command if the current context is not
 	// ours.
 	if err != nil {
-		cmd := exec.Command("docker-classic", os.Args[1:]...)
+		cmd := exec.CommandContext(ctx, "docker-classic", os.Args[1:]...)
 		cmd.Stdin = os.Stdin
 		cmd.Stdout = os.Stdout
 		cmd.Stderr = os.Stderr

tests/e2e/e2e_test.go

@@ -29,17 +29,20 @@ package main
 
 import (
 	"fmt"
+	"io/ioutil"
+	"log"
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
-	"gotest.tools/golden"
-
 	. "github.com/onsi/gomega"
 	"github.com/stretchr/testify/suite"
 
+	"gotest.tools/golden"
+
 	. "github.com/docker/api/tests/framework"
 )
 
@@ -79,6 +82,37 @@ func (s *E2eSuite) TestSetupError() {
 	})
 }
 
+func (s *E2eSuite) TestKillChildOnCancel() {
+	It("should kill docker-classic if parent command is cancelled", func() {
+		out := s.NewCommand("ps", "-x").ExecOrDie()
+		Expect(out).NotTo(ContainSubstring("docker-classic"))
+
+		dir := s.ConfigDir
+		Expect(ioutil.WriteFile(filepath.Join(dir, "Dockerfile"), []byte(`FROM alpine:3.10
+RUN sleep 100`), 0644)).To(Succeed())
+		shutdown := make(chan time.Time)
+		errs := make(chan error)
+		ctx := s.NewDockerCommand("build", "--no-cache", "-t", "test-sleep-image", ".").WithinDirectory(dir).WithTimeout(shutdown)
+		go func() {
+			_, err := ctx.Exec()
+			errs <- err
+		}()
+		err := WaitFor(time.Second, 3*time.Second, errs, func() bool {
+			out := s.NewCommand("ps", "-x").ExecOrDie()
+			return strings.Contains(out, "docker-classic")
+		})
+		Expect(err).NotTo(HaveOccurred())
+		log.Println("Killing docker process")
+
+		close(shutdown)
+		err = WaitFor(time.Second, 4*time.Second, nil, func() bool {
+			out := s.NewCommand("ps", "-x").ExecOrDie()
+			return !strings.Contains(out, "docker-classic")
+		})
+		Expect(err).NotTo(HaveOccurred())
+	})
+}
+
 func (s *E2eSuite) TestLegacy() {
 	It("should list all legacy commands", func() {
 		output := s.NewDockerCommand("--help").ExecOrDie()

tests/framework/exec.go

@@ -33,6 +33,7 @@ import (
 	"io"
 	"os/exec"
 	"strings"
+	"syscall"
 	"time"
 
 	"github.com/onsi/gomega"
@@ -129,6 +130,29 @@ func (b CmdContext) Exec() (string, error) {
 	}
 }
 
+//WaitFor waits for a condition to be true
+func WaitFor(interval, duration time.Duration, abort <-chan error, condition func() bool) error {
+	ticker := time.NewTicker(interval)
+	defer ticker.Stop()
+	timeout := make(chan int)
+	go func() {
+		time.Sleep(duration)
+		close(timeout)
+	}()
+	for {
+		select {
+		case err := <-abort:
+			return err
+		case <-timeout:
+			return fmt.Errorf("timeout after %v", duration)
+		case <-ticker.C:
+			if condition() {
+				return nil
+			}
+		}
+	}
+}
+
 // Execute executes a command.
 // The command cannot be re-used afterwards.
 func Execute(cmd *exec.Cmd, timeout <-chan time.Time) (string, error) {
@@ -152,7 +176,7 @@ func Execute(cmd *exec.Cmd, timeout <-chan time.Time) (string, error) {
 		}
 	case <-timeout:
 		log.Debugf("%s %s timed-out", cmd.Path, strings.Join(cmd.Args[1:], " "))
-		if err := cmd.Process.Kill(); err != nil {
+		if err := cmd.Process.Signal(syscall.SIGTERM); err != nil {
 			return "", err
 		}
 		return "", fmt.Errorf(