From 7e3f1c21b060762c39139d59f73ad589a54e03fa Mon Sep 17 00:00:00 2001 From: Marco Vedovati Date: Thu, 13 Jun 2019 19:33:58 +0200 Subject: libpod: specify a detach keys sequence in libpod.conf Add the ability of specifying a detach keys sequence in libpod.conf Signed-off-by: Marco Vedovati --- docs/libpod.conf.5.md | 3 +++ docs/podman-attach.1.md | 10 +++++++--- docs/podman-create.1.md | 10 ++++++---- docs/podman-run.1.md | 10 ++++++---- docs/podman-start.1.md | 6 ++++-- libpod.conf | 7 +++++++ libpod/container_attach_linux.go | 27 +++++++++++++++------------ libpod/runtime.go | 7 +++++++ pkg/adapter/containers.go | 16 +++++++++++----- utils/utils.go | 5 ----- 10 files changed, 66 insertions(+), 35 deletions(-) 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-` where `` 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-`, where `` 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-` where `` 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-`, where `` 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-` where `` 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-`, where `` 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-` where `` 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-`, where `` is one of: +`a-z`, `@`, `^`, `[`, `\\`, `]`, `^` or `_`. **--interactive**, **-i** diff --git a/libpod.conf b/libpod.conf index 32f7a56ae..b95efe6d8 100644 --- a/libpod.conf +++ b/libpod.conf @@ -105,6 +105,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-`, where `` is one of: +# `a-z`, `@`, `^`, `[`, `\`, `]`, `^` or `_` +# +# detach_keys = "ctrl-p,ctrl-q" + # Default OCI runtime runtime = "runc" diff --git a/libpod/container_attach_linux.go b/libpod/container_attach_linux.go index 5293480f0..3de5ea1cf 100644 --- a/libpod/container_attach_linux.go +++ b/libpod/container_attach_linux.go @@ -38,29 +38,32 @@ func (c *Container) attach(streams *AttachStreams, keys string, resize <-chan re 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, wg) } // attachContainerSocket connects to the container's attach socket and deals with the IO. // wg 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 { +func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSize, keys string, streams *AttachStreams, startContainer bool, wg *sync.WaitGroup) error { if startContainer && wg == nil { return errors.Wrapf(define.ErrInternal, "wait group 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) { controlPath := filepath.Join(c.bundlePath(), "ctl") controlFile, err := os.OpenFile(controlPath, unix.O_WRONLY, 0) diff --git a/libpod/runtime.go b/libpod/runtime.go index 152af031a..b7d6dc245 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 @@ -311,6 +317,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 5751a9c3a..38c04cceb 100644 --- a/pkg/adapter/containers.go +++ b/pkg/adapter/containers.go @@ -381,8 +381,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 @@ -407,10 +417,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) -- cgit v1.2.3-54-g00ecf From 6e9b490f5e2c84a903e6cc86440599e3ea7c63d2 Mon Sep 17 00:00:00 2001 From: Marco Vedovati Date: Thu, 13 Jun 2019 19:35:10 +0200 Subject: podman: clarify the format of --detach-keys argument Signed-off-by: Marco Vedovati --- cmd/podman/attach.go | 2 +- cmd/podman/common.go | 2 +- cmd/podman/start.go | 2 +- 3 files changed, 3 insertions(+), 3 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- where 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-`, where `` 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 7a3052653..13b6c3bca 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-` where `` 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-`, where `` 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- where 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-`, where `` 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)") -- cgit v1.2.3-54-g00ecf From 4f56964d556a7379c09a903258fd44c2232a686a Mon Sep 17 00:00:00 2001 From: Marco Vedovati Date: Mon, 17 Jun 2019 15:14:54 +0200 Subject: libpod: fix hang on container start and attach When a container is attached upon start, the WaitGroup counter may never be decremented if an error is raised before start, causing the caller to hang. Synchronize with the start & attach goroutine using a channel, to be able to detect failures before start. Signed-off-by: Marco Vedovati --- libpod/container_api.go | 17 ++++++++++------- libpod/container_attach_linux.go | 15 +++++++-------- libpod/container_attach_unsupported.go | 4 +--- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/libpod/container_api.go b/libpod/container_api.go index b8c339a39..d5d8e9906 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 3de5ea1cf..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,22 +32,22 @@ 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") } logrus.Debugf("Attaching to container %s", c.ID()) - return c.attachContainerSocket(resize, keys, 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, keys string, 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 @@ -100,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 } -- cgit v1.2.3-54-g00ecf