Merge pull request #447 from mheon/sig_proxy

Add signal proxying to podman run and attach

12 files changed, 175 insertions, 12 deletions

diff --git a/cmd/podman/attach.go b/cmd/podman/attach.go
index 7f7e4d192..20c1c306d 100644
--- a/cmd/podman/attach.go
+++ b/cmd/podman/attach.go
@@ -16,6 +16,10 @@ var (
 			Name:  "no-stdin",
 			Usage: "Do not attach STDIN. The default is false.",
 		},
+		cli.BoolTFlag{
+			Name:  "sig-proxy",
+			Usage: "proxy received signals to the process (default true)",
+		},
 		LatestFlag,
 	}
 	attachDescription = "The podman attach command allows you to attach to a running container using the container's ID or name, either to view its ongoing output or to control it interactively."
@@ -63,6 +67,10 @@ func attachCmd(c *cli.Context) error {
 		return errors.Errorf("you can only attach to running containers")
 	}
 
+	if c.BoolT("sig-proxy") {
+		ProxySignals(ctr)
+	}
+
 	if err := ctr.Attach(c.Bool("no-stdin"), c.String("detach-keys")); err != nil {
 		return errors.Wrapf(err, "error attaching to container %s", ctr.ID())
 	}
diff --git a/cmd/podman/common.go b/cmd/podman/common.go
index 60cc48803..93de80e29 100644
--- a/cmd/podman/common.go
+++ b/cmd/podman/common.go
@@ -334,10 +334,6 @@ var createFlags = []cli.Flag{
 		Usage: "Size of `/dev/shm`. The format is `<number><unit>`.",
 		Value: "65536k",
 	},
-	cli.BoolFlag{
-		Name:  "sig-proxy",
-		Usage: "Proxy received signals to the process (default true)",
-	},
 	cli.StringFlag{
 		Name:  "stop-signal",
 		Usage: "Signal to stop a container. Default is SIGTERM",
diff --git a/cmd/podman/create.go b/cmd/podman/create.go
index 462c56a51..efd7458ea 100644
--- a/cmd/podman/create.go
+++ b/cmd/podman/create.go
@@ -117,7 +117,6 @@ type createConfig struct {
 	Resources          createResourceConfig
 	Rm                 bool //rm
 	ShmDir             string
-	SigProxy           bool              //sig-proxy
 	StopSignal         syscall.Signal    // stop-signal
 	StopTimeout        uint              // stop-timeout
 	Sysctl             map[string]string //sysctl
@@ -715,7 +714,6 @@ func parseCreateOpts(c *cli.Context, runtime *libpod.Runtime, imageName string,
 		},
 		Rm:          c.Bool("rm"),
 		ShmDir:      shmDir,
-		SigProxy:    c.Bool("sig-proxy"),
 		StopSignal:  stopSignal,
 		StopTimeout: c.Uint("stop-timeout"),
 		Sysctl:      sysctl,
diff --git a/cmd/podman/run.go b/cmd/podman/run.go
index f68db9036..7b840a387 100644
--- a/cmd/podman/run.go
+++ b/cmd/podman/run.go
@@ -13,11 +13,16 @@ import (
 
 var runDescription = "Runs a command in a new container from the given image"
 
+var runFlags []cli.Flag = append(createFlags, cli.BoolTFlag{
+	Name:  "sig-proxy",
+	Usage: "proxy received signals to the process (default true)",
+})
+
 var runCommand = cli.Command{
 	Name:                   "run",
 	Usage:                  "run a command in a new container",
 	Description:            runDescription,
-	Flags:                  createFlags,
+	Flags:                  runFlags,
 	Action:                 runCmd,
 	ArgsUsage:              "IMAGE [COMMAND [ARG...]]",
 	SkipArgReorder:         true,
@@ -133,6 +138,10 @@ func runCmd(c *cli.Context) error {
 		return errors.Wrapf(err, "unable to start container %q", ctr.ID())
 	}
 
+	if c.BoolT("sig-proxy") {
+		ProxySignals(ctr)
+	}
+
 	// Wait for attach to complete
 	err = <-attachChan
 	if err != nil {
diff --git a/cmd/podman/sigproxy.go b/cmd/podman/sigproxy.go
new file mode 100644
index 000000000..fd1415dc6
--- /dev/null
+++ b/cmd/podman/sigproxy.go
@@ -0,0 +1,33 @@
+package main
+
+import (
+	"os"
+	"syscall"
+
+	"github.com/docker/docker/pkg/signal"
+	"github.com/projectatomic/libpod/libpod"
+	"github.com/sirupsen/logrus"
+)
+
+func ProxySignals(ctr *libpod.Container) {
+	sigBuffer := make(chan os.Signal, 128)
+	signal.CatchAll(sigBuffer)
+
+	logrus.Debugf("Enabling signal proxying")
+
+	go func() {
+		for s := range sigBuffer {
+			// Ignore SIGCHLD and SIGPIPE - these are mostly likely
+			// intended for the podman command itself.
+			if s == signal.SIGCHLD || s == signal.SIGPIPE {
+				continue
+			}
+
+			if err := ctr.Kill(uint(s.(syscall.Signal))); err != nil {
+				logrus.Errorf("Error forwarding signal %d to container %s: %v", s, ctr.ID(), err)
+			}
+		}
+	}()
+
+	return
+}
diff --git a/cmd/podman/start.go b/cmd/podman/start.go
index c90fab645..366d5c3fc 100644
--- a/cmd/podman/start.go
+++ b/cmd/podman/start.go
@@ -25,6 +25,10 @@ var (
 			Name:  "interactive, i",
 			Usage: "Keep STDIN open even if not attached",
 		},
+		cli.BoolFlag{
+			Name:  "sig-proxy",
+			Usage: "proxy received signals to the process",
+		},
 		LatestFlag,
 	}
 	startDescription = `
@@ -60,6 +64,10 @@ func startCmd(c *cli.Context) error {
 		return err
 	}
 
+	if c.Bool("sig-proxy") && !attach {
+		return errors.Wrapf(libpod.ErrInvalidArg, "you cannot use sig-proxy without --attach")
+	}
+
 	runtime, err := getRuntime(c)
 	if err != nil {
 		return errors.Wrapf(err, "error creating libpod runtime")
@@ -106,6 +114,10 @@ func startCmd(c *cli.Context) error {
 				return errors.Wrapf(err, "unable to start container %s", ctr.ID())
 			}
 
+			if c.Bool("sig-proxy") {
+				ProxySignals(ctr)
+			}
+
 			// Wait for attach to complete
 			err = <-attachChan
 			if err != nil {
diff --git a/completions/bash/podman b/completions/bash/podman
index 7903cc812..4313fe94f 100644
--- a/completions/bash/podman
+++ b/completions/bash/podman
@@ -654,6 +654,7 @@ _podman_attach() {
      --latest
      -l
      --no-stdin
+     --sig-proxy
      "
      _complete_ "$options_with_args" "$boolean_options"
 }
@@ -1487,7 +1488,9 @@ _podman_start() {
      -i
      --interactive
      --latest
-     -l"
+     -l
+     --sig-proxy
+     "
      _complete_ "$options_with_args" "$boolean_options"
 }
 _podman_stop() {
diff --git a/docs/podman-attach.1.md b/docs/podman-attach.1.md
index 64163b340..a1c058022 100644
--- a/docs/podman-attach.1.md
+++ b/docs/podman-attach.1.md
@@ -27,6 +27,9 @@ to run containers such as CRI-O, the last started container could be from either
 **--no-stdin**
 Do not attach STDIN. The default is false.
 
+**--sig-proxy**=*true*|*false*
+Proxy received signals to the process (non-TTY mode only). SIGCHLD, SIGSTOP, and SIGKILL are not proxied. The default is *true*.
+
 ## EXAMPLES ##
 
 ```
diff --git a/docs/podman-create.1.md b/docs/podman-create.1.md
index 044ab23d3..b28b785f1 100644
--- a/docs/podman-create.1.md
+++ b/docs/podman-create.1.md
@@ -419,9 +419,6 @@ its root filesystem mounted as read only prohibiting any writes.
    Unit is optional and can be `b` (bytes), `k` (kilobytes), `m`(megabytes), or `g` (gigabytes).
    If you omit the unit, the system uses bytes. If you omit the size entirely, the system uses `64m`.
 
-**--sig-proxy**=*true*|*false*
-   Proxy received signals to the process (non-TTY mode only). SIGCHLD, SIGSTOP, and SIGKILL are not proxied. The default is *true*.
-
 **--stop-signal**=*SIGTERM*
   Signal to stop a container. Default is SIGTERM.
 
diff --git a/docs/podman-start.1.md b/docs/podman-start.1.md
index 492787e6c..24fc49a8e 100644
--- a/docs/podman-start.1.md
+++ b/docs/podman-start.1.md
@@ -33,6 +33,9 @@ Attach container's STDIN. The default is false.
 Instead of providing the container name or ID, use the last created container. If you use methods other than Podman
 to run containers such as CRI-O, the last started container could be from either of those methods.
 
+**--sig-proxy**=*true*|*false*
+Proxy received signals to the process (non-TTY mode only). SIGCHLD, SIGSTOP, and SIGKILL are not proxied. The default is false.
+
 ## EXAMPLE
 
 podman start mywebserver
diff --git a/test/e2e/libpod_suite_test.go b/test/e2e/libpod_suite_test.go
index dc6292ace..634c38b29 100644
--- a/test/e2e/libpod_suite_test.go
+++ b/test/e2e/libpod_suite_test.go
@@ -162,7 +162,7 @@ func (p *PodmanTest) Podman(args []string) *PodmanSession {
 	command := exec.Command(p.PodmanBinary, podmanOptions...)
 	session, err := gexec.Start(command, GinkgoWriter, GinkgoWriter)
 	if err != nil {
-		Fail(fmt.Sprintf("unable to run podman command: %s", strings.Join(podmanOptions, " ")))
+		Fail(fmt.Sprintf("unable to run podman command: %s\n%v", strings.Join(podmanOptions, " "), err))
 	}
 	return &PodmanSession{session}
 }
diff --git a/test/e2e/run_signal_test.go b/test/e2e/run_signal_test.go
new file mode 100644
index 000000000..095288277
--- /dev/null
+++ b/test/e2e/run_signal_test.go
@@ -0,0 +1,101 @@
+package integration
+
+import (
+	"fmt"
+	"os"
+	"os/exec"
+	"strings"
+	"syscall"
+
+	"github.com/onsi/gomega/gexec"
+	"golang.org/x/sys/unix"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+// PodmanPID execs podman and returns its PID
+func (p *PodmanTest) PodmanPID(args []string) (*PodmanSession, int) {
+	podmanOptions := p.MakeOptions()
+	podmanOptions = append(podmanOptions, strings.Split(p.StorageOptions, " ")...)
+	podmanOptions = append(podmanOptions, args...)
+	fmt.Printf("Running: %s %s\n", p.PodmanBinary, strings.Join(podmanOptions, " "))
+	command := exec.Command(p.PodmanBinary, podmanOptions...)
+	session, err := gexec.Start(command, GinkgoWriter, GinkgoWriter)
+	if err != nil {
+		Fail(fmt.Sprintf("unable to run podman command: %s", strings.Join(podmanOptions, " ")))
+	}
+	return &PodmanSession{session}, command.Process.Pid
+}
+
+const sigCatch = "for NUM in `seq 1 64`; do trap \"echo Received $NUM\" $NUM; done; echo READY; while :; do sleep 0.1; done"
+
+var _ = Describe("Podman run with --sig-proxy", func() {
+	var (
+		tmpdir     string
+		err        error
+		podmanTest PodmanTest
+	)
+
+	BeforeEach(func() {
+		tmpdir, err = CreateTempDirInTempDir()
+		if err != nil {
+			os.Exit(1)
+		}
+		podmanTest = PodmanCreate(tmpdir)
+		podmanTest.RestoreAllArtifacts()
+		podmanTest.RestoreArtifact(fedoraMinimal)
+	})
+
+	AfterEach(func() {
+		podmanTest.Cleanup()
+
+	})
+
+	Specify("signals are forwarded to container using sig-proxy", func() {
+		signal := syscall.SIGPOLL
+		session, pid := podmanTest.PodmanPID([]string{"run", "--name", "test1", fedoraMinimal, "bash", "-c", sigCatch})
+
+		ok := WaitForContainer(&podmanTest)
+		Expect(ok).To(BeTrue())
+
+		// Kill with given signal
+		if err := unix.Kill(pid, signal); err != nil {
+			Fail(fmt.Sprintf("error killing podman process %d: %v", pid, err))
+		}
+
+		// Kill with -9 to guarantee the container dies
+		killSession := podmanTest.Podman([]string{"kill", "-s", "9", "test1"})
+		killSession.WaitWithDefaultTimeout()
+		Expect(killSession.ExitCode()).To(Equal(0))
+
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		ok, _ = session.GrepString(fmt.Sprintf("Received %d", signal))
+		Expect(ok).To(BeTrue())
+	})
+
+	Specify("signals are not forwarded to container with sig-proxy false", func() {
+		signal := syscall.SIGPOLL
+		session, pid := podmanTest.PodmanPID([]string{"run", "--name", "test2", "--sig-proxy=false", fedoraMinimal, "bash", "-c", sigCatch})
+
+		ok := WaitForContainer(&podmanTest)
+		Expect(ok).To(BeTrue())
+
+		// Kill with given signal
+		// Should be no output, SIGPOLL is usually ignored
+		if err := unix.Kill(pid, signal); err != nil {
+			Fail(fmt.Sprintf("error killing podman process %d: %v", pid, err))
+		}
+
+		// Kill with -9 to guarantee the container dies
+		killSession := podmanTest.Podman([]string{"kill", "-s", "9", "test2"})
+		killSession.WaitWithDefaultTimeout()
+		Expect(killSession.ExitCode()).To(Equal(0))
+
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		ok, _ = session.GrepString(fmt.Sprintf("Received %d", signal))
+		Expect(ok).To(BeFalse())
+	})
+})