Add signal proxying to podman run, start, and attach

Also removes sig-proxy from 'podman create', where is does not
make sense.

Signed-off-by: Matthew Heon <matthew.heon@gmail.com>

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())
 	}

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",

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,

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 {

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
+}

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 {

completions/bash/podman

@@ -654,6 +654,7 @@ _podman_attach() {
      --latest
      -l
      --no-stdin
+     --sig-proxy
      "
      _complete_ "$options_with_args" "$boolean_options"
 }
@@ -1478,7 +1479,9 @@ _podman_start() {
      -i
      --interactive
      --latest
-     -l"
+     -l
+     --sig-proxy
+     "
      _complete_ "$options_with_args" "$boolean_options"
 }
 _podman_stop() {

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 ##
 
 ```

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.
 

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

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}
 }

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())
+	})
+})