Merge pull request #3324 from marcov/detach-keys-configurable

libpod: specify a detach keys sequence in libpod.conf

15 files changed, 85 insertions, 54 deletions

diff --git a/cmd/podman/attach.go b/cmd/podman/attach.go
index 37f8afbad..48a25a3e2 100644
--- a/cmd/podman/attach.go
+++ b/cmd/podman/attach.go
@@ -31,7 +31,7 @@ func init() {
 	attachCommand.SetHelpTemplate(HelpTemplate())
 	attachCommand.SetUsageTemplate(UsageTemplate())
 	flags := attachCommand.Flags()
-	flags.StringVar(&attachCommand.DetachKeys, "detach-keys", "", "Override the key sequence for detaching a container. Format is a single character [a-Z] or ctrl-<value> where <value> is one of: a-z, @, ^, [, , or _")
+	flags.StringVar(&attachCommand.DetachKeys, "detach-keys", "", "Override the key sequence for detaching a container. Format is a single character `[a-Z]` or a comma separated sequence of `ctrl-<value>`, where `<value>` is one of: `a-z`, `@`, `^`, `[`, `\\`, `]`, `^` or `_`")
 	flags.BoolVar(&attachCommand.NoStdin, "no-stdin", false, "Do not attach STDIN. The default is false")
 	flags.BoolVar(&attachCommand.SigProxy, "sig-proxy", true, "Proxy received signals to the process")
 	flags.BoolVarP(&attachCommand.Latest, "latest", "l", false, "Act on the latest container podman is aware of")
diff --git a/cmd/podman/common.go b/cmd/podman/common.go
index 578094718..3cc645f95 100644
--- a/cmd/podman/common.go
+++ b/cmd/podman/common.go
@@ -181,7 +181,7 @@ func getCreateFlags(c *cliconfig.PodmanCommand) {
 	)
 	createFlags.String(
 		"detach-keys", "",
-		"Override the key sequence for detaching a container. Format is a single character `[a-Z]` or `ctrl-<value>` where `<value>` is one of: `a-z`, `@`, `^`, `[`, `\\`, `]`, `^` or `_`",
+		"Override the key sequence for detaching a container. Format is a single character `[a-Z]` or a comma separated sequence of `ctrl-<value>`, where `<value>` is one of: `a-z`, `@`, `^`, `[`, `\\`, `]`, `^` or `_`",
 	)
 	createFlags.StringSlice(
 		"device", []string{},
diff --git a/cmd/podman/start.go b/cmd/podman/start.go
index 904cca297..165273114 100644
--- a/cmd/podman/start.go
+++ b/cmd/podman/start.go
@@ -35,7 +35,7 @@ func init() {
 	startCommand.SetUsageTemplate(UsageTemplate())
 	flags := startCommand.Flags()
 	flags.BoolVarP(&startCommand.Attach, "attach", "a", false, "Attach container's STDOUT and STDERR")
-	flags.StringVar(&startCommand.DetachKeys, "detach-keys", "", "Override the key sequence for detaching a container. Format is a single character [a-Z] or ctrl-<value> where <value> is one of: a-z, @, ^, [, , or _")
+	flags.StringVar(&startCommand.DetachKeys, "detach-keys", "", "Override the key sequence for detaching a container. Format is a single character `[a-Z]` or a comma separated sequence of `ctrl-<value>`, where `<value>` is one of: `a-z`, `@`, `^`, `[`, `\\`, `]`, `^` or `_`")
 	flags.BoolVarP(&startCommand.Interactive, "interactive", "i", false, "Keep STDIN open even if not attached")
 	flags.BoolVarP(&startCommand.Latest, "latest", "l", false, "Act on the latest container podman is aware of")
 	flags.BoolVar(&startCommand.SigProxy, "sig-proxy", false, "Proxy received signals to the process (default true if attaching, false otherwise)")
diff --git a/docs/libpod.conf.5.md b/docs/libpod.conf.5.md
index cb08f0eb0..c57dcca8f 100644
--- a/docs/libpod.conf.5.md
+++ b/docs/libpod.conf.5.md
@@ -98,6 +98,9 @@ libpod to manage containers.
 **events_logger**=""
   Default method to use when logging events. Valid values are "journald" and "file".
 
+**detach_keys**=""
+  Keys sequence used for detaching a container
+
 ## FILES
   `/usr/share/containers/libpod.conf`, default libpod configuration path
 
diff --git a/docs/podman-attach.1.md b/docs/podman-attach.1.md
index b8d2fa9a1..4caa87792 100644
--- a/docs/podman-attach.1.md
+++ b/docs/podman-attach.1.md
@@ -11,12 +11,16 @@ The attach command allows you to attach to a running container using the contain
 or name, either to view its ongoing output or to control it interactively.
 
 You can detach from the container (and leave it running) using a configurable key sequence. The default
-sequence is `ctrl-p,ctrl-q`. You configure the key sequence using the --detach-keys option
+sequence is `ctrl-p,ctrl-q`.
+Configure the keys sequence using the **--detach-keys** option, or specifying
+it in the **libpod.conf** file: see **libpod.conf(5)** for more information.
 
 ## OPTIONS
-**--detach-keys**=*char*
+**--detach-keys**=*sequence*
 
-Override the key sequence for detaching a container. Format is a single character `[a-Z]` or `ctrl-<value>` where `<value>` is one of: `a-z`, `@`, `^`, `[`, `\\`, `]`, `^` or `_`.
+Override the key sequence for detaching a container. Format is a single character `[a-Z]` or
+a comma separated sequence of `ctrl-<value>`, where `<value>` is one of:
+`a-z`, `@`, `^`, `[`, `\\`, `]`, `^` or `_`.
 
 **--latest**, **-l**
 
diff --git a/docs/podman-create.1.md b/docs/podman-create.1.md
index b08488d7b..e22666402 100644
--- a/docs/podman-create.1.md
+++ b/docs/podman-create.1.md
@@ -175,12 +175,14 @@ detached container with **podman attach**.
 
 When attached in the tty mode, you can detach from the container (and leave it
 running) using a configurable key sequence. The default sequence is `ctrl-p,ctrl-q`.
-You configure the key sequence using the **--detach-keys** option or a configuration file.
-See **config-json(5)** for documentation on using a configuration file.
+Configure the keys sequence using the **--detach-keys** option, or specifying
+it in the **libpod.conf** file: see **libpod.conf(5)** for more information.
 
-**--detach-keys**=*char*
+**--detach-keys**=*sequence*
 
-Override the key sequence for detaching a container. Format is a single character `[a-Z]` or `ctrl-<value>` where `<value>` is one of: `a-z`, `@`, `^`, `[`, `\\`, `]`, `^` or `_`.
+Override the key sequence for detaching a container. Format is a single character `[a-Z]` or
+a comma separated sequence of `ctrl-<value>`, where `<value>` is one of:
+`a-z`, `@`, `^`, `[`, `\\`, `]`, `^` or `_`.
 
 **--device**=*device*
 
diff --git a/docs/podman-run.1.md b/docs/podman-run.1.md
index 042a7a561..30242080b 100644
--- a/docs/podman-run.1.md
+++ b/docs/podman-run.1.md
@@ -181,12 +181,14 @@ detached container with **podman attach**.
 
 When attached in the tty mode, you can detach from the container (and leave it
 running) using a configurable key sequence. The default sequence is `ctrl-p,ctrl-q`.
-You configure the key sequence using the **--detach-keys** option or a configuration file.
-See **config-json(5)** for documentation on using a configuration file.
+Configure the keys sequence using the **--detach-keys** option, or specifying
+it in the **libpod.conf** file: see **libpod.conf(5)** for more information.
 
-**--detach-keys**=*char*
+**--detach-keys**=*sequence*
 
-Override the key sequence for detaching a container. Format is a single character `[a-Z]` or `ctrl-<value>` where `<value>` is one of: `a-z`, `@`, `^`, `[`, `,` or `_`.
+Override the key sequence for detaching a container. Format is a single character `[a-Z]` or
+a comma separated sequence of `ctrl-<value>`, where `<value>` is one of:
+`a-z`, `@`, `^`, `[`, `\\`, `]`, `^` or `_`.
 
 **--device**=*device*
 
diff --git a/docs/podman-start.1.md b/docs/podman-start.1.md
index af7094bca..5ec6e2ea2 100644
--- a/docs/podman-start.1.md
+++ b/docs/podman-start.1.md
@@ -19,9 +19,11 @@ attach to the container.
 Attach container's STDOUT and STDERR.  The default is false. This option cannot be used when
 starting multiple containers.
 
-**--detach-keys**=*char*
+**--detach-keys**=*sequence*
 
-Override the key sequence for detaching a container. Format is a single character `[a-Z]` or `ctrl-<value>` where `<value>` is one of: `a-z`, `@`, `^`, `[`, `\\`, `]`, `^` or `_`.
+Override the key sequence for detaching a container. Format is a single character `[a-Z]` or
+a comma separated sequence of `ctrl-<value>`, where `<value>` is one of:
+`a-z`, `@`, `^`, `[`, `\\`, `]`, `^` or `_`.
 
 **--interactive**, **-i**
 
diff --git a/libpod.conf b/libpod.conf
index 2b5df0e66..71ac79dd4 100644
--- a/libpod.conf
+++ b/libpod.conf
@@ -102,6 +102,13 @@ num_locks = 2048
 # are `journald` or `file`.
 # events_logger = "journald"
 
+# Specify the keys sequence used to detach a container.
+# Format is a single character [a-Z] or a comma separated sequence of
+# `ctrl-<value>`, where `<value>` is one of:
+# `a-z`, `@`, `^`, `[`, `\`, `]`, `^` or `_`
+#
+# detach_keys = "ctrl-p,ctrl-q"
+
 # Default OCI runtime
 runtime = "runc"
 
diff --git a/libpod/container_api.go b/libpod/container_api.go
index 8d1e5751b..3dd84b02c 100644
--- a/libpod/container_api.go
+++ b/libpod/container_api.go
@@ -7,7 +7,6 @@ import (
 	"io/ioutil"
 	"os"
 	"strconv"
-	"sync"
 	"time"
 
 	"github.com/containers/libpod/libpod/define"
@@ -120,20 +119,24 @@ func (c *Container) StartAndAttach(ctx context.Context, streams *AttachStreams,
 	attachChan := make(chan error)
 
 	// We need to ensure that we don't return until start() fired in attach.
-	// Use a WaitGroup to sync this.
-	wg := new(sync.WaitGroup)
-	wg.Add(1)
+	// Use a channel to sync
+	startedChan := make(chan bool)
 
 	// Attach to the container before starting it
 	go func() {
-		if err := c.attach(streams, keys, resize, true, wg); err != nil {
+		if err := c.attach(streams, keys, resize, true, startedChan); err != nil {
 			attachChan <- err
 		}
 		close(attachChan)
 	}()
 
-	wg.Wait()
-	c.newContainerEvent(events.Attach)
+	select {
+	case err := <-attachChan:
+		return nil, err
+	case <-startedChan:
+		c.newContainerEvent(events.Attach)
+	}
+
 	return attachChan, nil
 }
 
diff --git a/libpod/container_attach_linux.go b/libpod/container_attach_linux.go
index 5293480f0..f5aac5794 100644
--- a/libpod/container_attach_linux.go
+++ b/libpod/container_attach_linux.go
@@ -8,7 +8,6 @@ import (
 	"net"
 	"os"
 	"path/filepath"
-	"sync"
 
 	"github.com/containers/libpod/libpod/define"
 	"github.com/containers/libpod/pkg/kubeutils"
@@ -33,32 +32,35 @@ const (
 
 // Attach to the given container
 // Does not check if state is appropriate
-func (c *Container) attach(streams *AttachStreams, keys string, resize <-chan remotecommand.TerminalSize, startContainer bool, wg *sync.WaitGroup) error {
+func (c *Container) attach(streams *AttachStreams, keys string, resize <-chan remotecommand.TerminalSize, startContainer bool, started chan bool) error {
 	if !streams.AttachOutput && !streams.AttachError && !streams.AttachInput {
 		return errors.Wrapf(define.ErrInvalidArg, "must provide at least one stream to attach to")
 	}
 
-	// Check the validity of the provided keys first
-	var err error
-	detachKeys := []byte{}
-	if len(keys) > 0 {
-		detachKeys, err = term.ToBytes(keys)
-		if err != nil {
-			return errors.Wrapf(err, "invalid detach keys")
-		}
-	}
-
 	logrus.Debugf("Attaching to container %s", c.ID())
 
-	return c.attachContainerSocket(resize, detachKeys, streams, startContainer, wg)
+	return c.attachContainerSocket(resize, keys, streams, startContainer, started)
 }
 
 // attachContainerSocket connects to the container's attach socket and deals with the IO.
-// wg is only required if startContainer is true
+// started is only required if startContainer is true
 // TODO add a channel to allow interrupting
-func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSize, detachKeys []byte, streams *AttachStreams, startContainer bool, wg *sync.WaitGroup) error {
-	if startContainer && wg == nil {
-		return errors.Wrapf(define.ErrInternal, "wait group not passed when startContainer set")
+func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSize, keys string, streams *AttachStreams, startContainer bool, started chan bool) error {
+	if startContainer && started == nil {
+		return errors.Wrapf(define.ErrInternal, "started chan not passed when startContainer set")
+	}
+
+	// Use default detach keys when keys aren't passed or specified in libpod.conf
+	if len(keys) == 0 {
+		keys = DefaultDetachKeys
+	}
+
+	// Check the validity of the provided keys
+	detachKeys := []byte{}
+	var err error
+	detachKeys, err = term.ToBytes(keys)
+	if err != nil {
+		return errors.Wrapf(err, "invalid detach keys")
 	}
 
 	kubeutils.HandleResizing(resize, func(size remotecommand.TerminalSize) {
@@ -97,7 +99,7 @@ func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSi
 		if err := c.start(); err != nil {
 			return err
 		}
-		wg.Done()
+		started <- true
 	}
 
 	receiveStdoutError := make(chan error)
diff --git a/libpod/container_attach_unsupported.go b/libpod/container_attach_unsupported.go
index 2c8718c67..c27ce0799 100644
--- a/libpod/container_attach_unsupported.go
+++ b/libpod/container_attach_unsupported.go
@@ -3,12 +3,10 @@
 package libpod
 
 import (
-	"sync"
-
 	"github.com/containers/libpod/libpod/define"
 	"k8s.io/client-go/tools/remotecommand"
 )
 
-func (c *Container) attach(streams *AttachStreams, keys string, resize <-chan remotecommand.TerminalSize, startContainer bool, wg *sync.WaitGroup) error {
+func (c *Container) attach(streams *AttachStreams, keys string, resize <-chan remotecommand.TerminalSize, startContainer bool, started chan bool) error {
 	return define.ErrNotImplemented
 }
diff --git a/libpod/runtime.go b/libpod/runtime.go
index 5a618f592..02aa76731 100644
--- a/libpod/runtime.go
+++ b/libpod/runtime.go
@@ -81,6 +81,10 @@ var (
 	DefaultSHMLockPath = "/libpod_lock"
 	// DefaultRootlessSHMLockPath is the default path for rootless SHM locks
 	DefaultRootlessSHMLockPath = "/libpod_rootless_lock"
+
+	// DefaultDetachKeys is the default keys sequence for detaching a
+	// container
+	DefaultDetachKeys = "ctrl-p,ctrl-q"
 )
 
 // A RuntimeOption is a functional option which alters the Runtime created by
@@ -236,6 +240,8 @@ type RuntimeConfig struct {
 	EventsLogger string `toml:"events_logger"`
 	// EventsLogFilePath is where the events log is stored.
 	EventsLogFilePath string `toml:-"events_logfile_path"`
+	//DetachKeys is the sequence of keys used to detach a container
+	DetachKeys string `toml:"detach_keys"`
 }
 
 // runtimeConfiguredFrom is a struct used during early runtime init to help
@@ -308,6 +314,7 @@ func defaultRuntimeConfig() (RuntimeConfig, error) {
 		EnableLabeling:        true,
 		NumLocks:              2048,
 		EventsLogger:          events.DefaultEventerType.String(),
+		DetachKeys:            DefaultDetachKeys,
 	}, nil
 }
 
diff --git a/pkg/adapter/containers.go b/pkg/adapter/containers.go
index 898df5fce..10720886b 100644
--- a/pkg/adapter/containers.go
+++ b/pkg/adapter/containers.go
@@ -384,8 +384,18 @@ func (r *LocalRuntime) Run(ctx context.Context, c *cliconfig.RunValues, exitCode
 			}
 		}
 	}
+
+	config, err := r.Runtime.GetConfig()
+	if err != nil {
+		return exitCode, err
+	}
+	detachKeys := c.String("detach-keys")
+	if detachKeys == "" {
+		detachKeys = config.DetachKeys
+	}
+
 	// if the container was created as part of a pod, also start its dependencies, if any.
-	if err := StartAttachCtr(ctx, ctr, outputStream, errorStream, inputStream, c.String("detach-keys"), c.Bool("sig-proxy"), true, c.IsSet("pod")); err != nil {
+	if err := StartAttachCtr(ctx, ctr, outputStream, errorStream, inputStream, detachKeys, c.Bool("sig-proxy"), true, c.IsSet("pod")); err != nil {
 		// We've manually detached from the container
 		// Do not perform cleanup, or wait for container exit code
 		// Just exit immediately
@@ -410,10 +420,6 @@ func (r *LocalRuntime) Run(ctx context.Context, c *cliconfig.RunValues, exitCode
 		if errors.Cause(err) == define.ErrNoSuchCtr {
 			// The container may have been removed
 			// Go looking for an exit file
-			config, err := r.Runtime.GetConfig()
-			if err != nil {
-				return exitCode, err
-			}
 			ctrExitCode, err := ReadExitFile(config.TmpDir, ctr.ID())
 			if err != nil {
 				logrus.Errorf("Cannot get exit code: %v", err)
diff --git a/utils/utils.go b/utils/utils.go
index 86adfb967..0ac6bc6d3 100644
--- a/utils/utils.go
+++ b/utils/utils.go
@@ -59,11 +59,6 @@ var ErrDetach = errors.New("detached from container")
 
 // CopyDetachable is similar to io.Copy but support a detach key sequence to break out.
 func CopyDetachable(dst io.Writer, src io.Reader, keys []byte) (written int64, err error) {
-	if len(keys) == 0 {
-		// Default keys : ctrl-p,ctrl-q
-		keys = []byte{16, 17}
-	}
-
 	buf := make([]byte, 32*1024)
 	for {
 		nr, er := src.Read(buf)