diff --git a/libpod/container_exec.go b/libpod/container_exec.go index c359f1e5df..8681b4e13e 100644 --- a/libpod/container_exec.go +++ b/libpod/container_exec.go @@ -276,9 +276,10 @@ func (c *Container) ExecStart(sessionID string) error { } // ExecStartAndAttach starts and attaches to an exec session in a container. +// newSize resizes the tty to this size before the process is started, must be nil if the exec session has no tty // TODO: Should we include detach keys in the signature to allow override? // TODO: How do we handle AttachStdin/AttachStdout/AttachStderr? -func (c *Container) ExecStartAndAttach(sessionID string, streams *define.AttachStreams) error { +func (c *Container) ExecStartAndAttach(sessionID string, streams *define.AttachStreams, newSize *define.TerminalSize) error { if !c.batched { c.lock.Lock() defer c.lock.Unlock() @@ -309,7 +310,7 @@ func (c *Container) ExecStartAndAttach(sessionID string, streams *define.AttachS return err } - pid, attachChan, err := c.ociRuntime.ExecContainer(c, session.ID(), opts, streams) + pid, attachChan, err := c.ociRuntime.ExecContainer(c, session.ID(), opts, streams, newSize) if err != nil { return err } @@ -372,7 +373,9 @@ func (c *Container) ExecStartAndAttach(sessionID string, streams *define.AttachS } // ExecHTTPStartAndAttach starts and performs an HTTP attach to an exec session. -func (c *Container) ExecHTTPStartAndAttach(sessionID string, r *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, detachKeys *string, cancel <-chan bool, hijackDone chan<- bool) error { +// newSize resizes the tty to this size before the process is started, must be nil if the exec session has no tty +func (c *Container) ExecHTTPStartAndAttach(sessionID string, r *http.Request, w http.ResponseWriter, + streams *HTTPAttachStreams, detachKeys *string, cancel <-chan bool, hijackDone chan<- bool, newSize *define.TerminalSize) error { // TODO: How do we combine streams with the default streams set in the exec session? // Ensure that we don't leak a goroutine here @@ -430,7 +433,7 @@ func (c *Container) ExecHTTPStartAndAttach(sessionID string, r *http.Request, w close(holdConnOpen) }() - pid, attachChan, err := c.ociRuntime.ExecContainerHTTP(c, session.ID(), execOpts, r, w, streams, cancel, hijackDone, holdConnOpen) + pid, attachChan, err := c.ociRuntime.ExecContainerHTTP(c, session.ID(), execOpts, r, w, streams, cancel, hijackDone, holdConnOpen, newSize) if err != nil { session.State = define.ExecStateStopped session.ExitCode = define.TranslateExecErrorToExitCode(define.ExecErrorCodeGeneric, err) @@ -733,7 +736,10 @@ func (c *Container) Exec(config *ExecConfig, streams *define.AttachStreams, resi // API there. // TODO: Refactor so this is closed here, before we remove the exec // session. + var size *define.TerminalSize if resize != nil { + s := <-resize + size = &s go func() { logrus.Debugf("Sending resize events to exec session %s", sessionID) for resizeRequest := range resize { @@ -751,7 +757,7 @@ func (c *Container) Exec(config *ExecConfig, streams *define.AttachStreams, resi }() } - if err := c.ExecStartAndAttach(sessionID, streams); err != nil { + if err := c.ExecStartAndAttach(sessionID, streams, size); err != nil { return -1, err } diff --git a/libpod/oci.go b/libpod/oci.go index 1f2c7dd711..c92d9a0778 100644 --- a/libpod/oci.go +++ b/libpod/oci.go @@ -72,13 +72,16 @@ type OCIRuntime interface { // has completed, as one might expect. The attach session will remain // running, in a goroutine that will return via the chan error in the // return signature. - ExecContainer(ctr *Container, sessionID string, options *ExecOptions, streams *define.AttachStreams) (int, chan error, error) + // newSize resizes the tty to this size before the process is started, must be nil if the exec session has no tty + ExecContainer(ctr *Container, sessionID string, options *ExecOptions, streams *define.AttachStreams, newSize *define.TerminalSize) (int, chan error, error) // ExecContainerHTTP executes a command in a running container and // attaches its standard streams to a provided hijacked HTTP session. // Maintains the same invariants as ExecContainer (returns on session // start, with a goroutine running in the background to handle attach). // The HTTP attach itself maintains the same invariants as HTTPAttach. - ExecContainerHTTP(ctr *Container, sessionID string, options *ExecOptions, r *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, cancel <-chan bool, hijackDone chan<- bool, holdConnOpen <-chan bool) (int, chan error, error) + // newSize resizes the tty to this size before the process is started, must be nil if the exec session has no tty + ExecContainerHTTP(ctr *Container, sessionID string, options *ExecOptions, r *http.Request, w http.ResponseWriter, + streams *HTTPAttachStreams, cancel <-chan bool, hijackDone chan<- bool, holdConnOpen <-chan bool, newSize *define.TerminalSize) (int, chan error, error) // ExecContainerDetached executes a command in a running container, but // does not attach to it. Returns the PID of the exec session and an // error (if starting the exec session failed) diff --git a/libpod/oci_attach_linux.go b/libpod/oci_attach_linux.go index b5040de3e6..de435b58a8 100644 --- a/libpod/oci_attach_linux.go +++ b/libpod/oci_attach_linux.go @@ -94,17 +94,18 @@ func (c *Container) attach(streams *define.AttachStreams, keys string, resize <- // this ensures attachToExec gets all of the output of the called process // conmon will then send the exit code of the exec process, or an error in the exec session // startFd must be the input side of the fd. +// newSize resizes the tty to this size before the process is started, must be nil if the exec session has no tty // conmon will wait to start the exec session until the parent process has setup the console socket. // Once attachToExec successfully attaches to the console socket, the child conmon process responsible for calling runtime exec // will read from the output side of start fd, thus learning to start the child process. // Thus, the order goes as follow: // 1. conmon parent process sets up its console socket. sends on attachFd -// 2. attachToExec attaches to the console socket after reading on attachFd +// 2. attachToExec attaches to the console socket after reading on attachFd and resizes the tty // 3. child waits on startFd for attachToExec to attach to said console socket // 4. attachToExec sends on startFd, signalling it has attached to the socket and child is ready to go // 5. child receives on startFd, runs the runtime exec command // attachToExec is responsible for closing startFd and attachFd -func (c *Container) attachToExec(streams *define.AttachStreams, keys *string, sessionID string, startFd, attachFd *os.File) error { +func (c *Container) attachToExec(streams *define.AttachStreams, keys *string, sessionID string, startFd, attachFd *os.File, newSize *define.TerminalSize) error { if !streams.AttachOutput && !streams.AttachError && !streams.AttachInput { return errors.Wrapf(define.ErrInvalidArg, "must provide at least one stream to attach to") } @@ -137,6 +138,14 @@ func (c *Container) attachToExec(streams *define.AttachStreams, keys *string, se return err } + // resize before we start the container process + if newSize != nil { + err = c.ociRuntime.ExecAttachResize(c, sessionID, *newSize) + if err != nil { + logrus.Warn("resize failed", err) + } + } + // 2: then attach conn, err := openUnixSocket(sockPath) if err != nil { diff --git a/libpod/oci_conmon_exec_linux.go b/libpod/oci_conmon_exec_linux.go index 76338b86c0..09d3d18334 100644 --- a/libpod/oci_conmon_exec_linux.go +++ b/libpod/oci_conmon_exec_linux.go @@ -25,7 +25,7 @@ import ( ) // ExecContainer executes a command in a running container -func (r *ConmonOCIRuntime) ExecContainer(c *Container, sessionID string, options *ExecOptions, streams *define.AttachStreams) (int, chan error, error) { +func (r *ConmonOCIRuntime) ExecContainer(c *Container, sessionID string, options *ExecOptions, streams *define.AttachStreams, newSize *define.TerminalSize) (int, chan error, error) { if options == nil { return -1, nil, errors.Wrapf(define.ErrInvalidArg, "must provide an ExecOptions struct to ExecContainer") } @@ -68,7 +68,7 @@ func (r *ConmonOCIRuntime) ExecContainer(c *Container, sessionID string, options attachChan := make(chan error) go func() { // attachToExec is responsible for closing pipes - attachChan <- c.attachToExec(streams, options.DetachKeys, sessionID, pipes.startPipe, pipes.attachPipe) + attachChan <- c.attachToExec(streams, options.DetachKeys, sessionID, pipes.startPipe, pipes.attachPipe, newSize) close(attachChan) }() @@ -83,7 +83,8 @@ func (r *ConmonOCIRuntime) ExecContainer(c *Container, sessionID string, options // ExecContainerHTTP executes a new command in an existing container and // forwards its standard streams over an attach -func (r *ConmonOCIRuntime) ExecContainerHTTP(ctr *Container, sessionID string, options *ExecOptions, req *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, cancel <-chan bool, hijackDone chan<- bool, holdConnOpen <-chan bool) (int, chan error, error) { +func (r *ConmonOCIRuntime) ExecContainerHTTP(ctr *Container, sessionID string, options *ExecOptions, req *http.Request, w http.ResponseWriter, + streams *HTTPAttachStreams, cancel <-chan bool, hijackDone chan<- bool, holdConnOpen <-chan bool, newSize *define.TerminalSize) (int, chan error, error) { if streams != nil { if !streams.Stdin && !streams.Stdout && !streams.Stderr { return -1, nil, errors.Wrapf(define.ErrInvalidArg, "must provide at least one stream to attach to") @@ -133,7 +134,7 @@ func (r *ConmonOCIRuntime) ExecContainerHTTP(ctr *Container, sessionID string, o conmonPipeDataChan := make(chan conmonPipeData) go func() { // attachToExec is responsible for closing pipes - attachChan <- attachExecHTTP(ctr, sessionID, req, w, streams, pipes, detachKeys, options.Terminal, cancel, hijackDone, holdConnOpen, execCmd, conmonPipeDataChan, ociLog) + attachChan <- attachExecHTTP(ctr, sessionID, req, w, streams, pipes, detachKeys, options.Terminal, cancel, hijackDone, holdConnOpen, execCmd, conmonPipeDataChan, ociLog, newSize) close(attachChan) }() @@ -486,7 +487,7 @@ func (r *ConmonOCIRuntime) startExec(c *Container, sessionID string, options *Ex } // Attach to a container over HTTP -func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, pipes *execPipes, detachKeys []byte, isTerminal bool, cancel <-chan bool, hijackDone chan<- bool, holdConnOpen <-chan bool, execCmd *exec.Cmd, conmonPipeDataChan chan<- conmonPipeData, ociLog string) (deferredErr error) { +func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, pipes *execPipes, detachKeys []byte, isTerminal bool, cancel <-chan bool, hijackDone chan<- bool, holdConnOpen <-chan bool, execCmd *exec.Cmd, conmonPipeDataChan chan<- conmonPipeData, ociLog string, newSize *define.TerminalSize) (deferredErr error) { // NOTE: As you may notice, the attach code is quite complex. // Many things happen concurrently and yet are interdependent. // If you ever change this function, make sure to write to the @@ -524,6 +525,14 @@ func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.Resp return err } + // resize before we start the container process + if newSize != nil { + err = c.ociRuntime.ExecAttachResize(c, sessionID, *newSize) + if err != nil { + logrus.Warn("resize failed", err) + } + } + // 2: then attach conn, err := openUnixSocket(sockPath) if err != nil { diff --git a/libpod/oci_missing.go b/libpod/oci_missing.go index 10526f368d..fcf2ffca84 100644 --- a/libpod/oci_missing.go +++ b/libpod/oci_missing.go @@ -119,12 +119,13 @@ func (r *MissingRuntime) AttachResize(ctr *Container, newSize define.TerminalSiz } // ExecContainer is not available as the runtime is missing -func (r *MissingRuntime) ExecContainer(ctr *Container, sessionID string, options *ExecOptions, streams *define.AttachStreams) (int, chan error, error) { +func (r *MissingRuntime) ExecContainer(ctr *Container, sessionID string, options *ExecOptions, streams *define.AttachStreams, newSize *define.TerminalSize) (int, chan error, error) { return -1, nil, r.printError() } // ExecContainerHTTP is not available as the runtime is missing -func (r *MissingRuntime) ExecContainerHTTP(ctr *Container, sessionID string, options *ExecOptions, req *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, cancel <-chan bool, hijackDone chan<- bool, holdConnOpen <-chan bool) (int, chan error, error) { +func (r *MissingRuntime) ExecContainerHTTP(ctr *Container, sessionID string, options *ExecOptions, req *http.Request, w http.ResponseWriter, + streams *HTTPAttachStreams, cancel <-chan bool, hijackDone chan<- bool, holdConnOpen <-chan bool, newSize *define.TerminalSize) (int, chan error, error) { return -1, nil, r.printError() } diff --git a/pkg/api/handlers/compat/exec.go b/pkg/api/handlers/compat/exec.go index 1b7b884e05..77e62c1126 100644 --- a/pkg/api/handlers/compat/exec.go +++ b/pkg/api/handlers/compat/exec.go @@ -178,8 +178,16 @@ func ExecStartHandler(w http.ResponseWriter, r *http.Request) { logrus.Error(errors.Wrapf(e, "error attaching to container %s exec session %s", sessionCtr.ID(), sessionID)) } + var size *define.TerminalSize + if bodyParams.Tty && (bodyParams.Height > 0 || bodyParams.Width > 0) { + size = &define.TerminalSize{ + Height: bodyParams.Height, + Width: bodyParams.Width, + } + } + hijackChan := make(chan bool, 1) - err = sessionCtr.ExecHTTPStartAndAttach(sessionID, r, w, nil, nil, nil, hijackChan) + err = sessionCtr.ExecHTTPStartAndAttach(sessionID, r, w, nil, nil, nil, hijackChan, size) if <-hijackChan { // If connection was Hijacked, we have to signal it's being closed diff --git a/pkg/api/handlers/compat/resize.go b/pkg/api/handlers/compat/resize.go index f65e313fc7..844fb74c4c 100644 --- a/pkg/api/handlers/compat/resize.go +++ b/pkg/api/handlers/compat/resize.go @@ -73,7 +73,7 @@ func ResizeTTY(w http.ResponseWriter, r *http.Request) { return } if err := ctnr.ExecResize(name, sz); err != nil { - if errors.Cause(err) != define.ErrCtrStateInvalid || !query.IgnoreNotRunning { + if errors.Cause(err) != define.ErrExecSessionStateInvalid || !query.IgnoreNotRunning { utils.InternalServerError(w, errors.Wrapf(err, "cannot resize session")) return } diff --git a/pkg/api/handlers/types.go b/pkg/api/handlers/types.go index 52d7633af2..d350242b44 100644 --- a/pkg/api/handlers/types.go +++ b/pkg/api/handlers/types.go @@ -166,8 +166,10 @@ type ExecCreateResponse struct { } type ExecStartConfig struct { - Detach bool `json:"Detach"` - Tty bool `json:"Tty"` + Detach bool `json:"Detach"` + Tty bool `json:"Tty"` + Height uint16 `json:"h"` + Width uint16 `json:"w"` } func ImageToImageSummary(l *libimage.Image) (*entities.ImageSummary, error) { diff --git a/pkg/api/server/register_exec.go b/pkg/api/server/register_exec.go index 3716ef6a2f..e353d714cb 100644 --- a/pkg/api/server/register_exec.go +++ b/pkg/api/server/register_exec.go @@ -269,10 +269,16 @@ func (s *APIServer) registerExecHandlers(r *mux.Router) error { // properties: // Detach: // type: boolean - // description: Detach from the command. Not presently supported. + // description: Detach from the command. // Tty: // type: boolean - // description: Allocate a pseudo-TTY. Presently ignored. + // description: Allocate a pseudo-TTY. + // h: + // type: integer + // description: Height of the TTY session in characters. Tty must be set to true to use it. + // w: + // type: integer + // description: Width of the TTY session in characters. Tty must be set to true to use it. // produces: // - application/json // responses: diff --git a/pkg/bindings/containers/attach.go b/pkg/bindings/containers/attach.go index adef1e7c84..cc12c8ab7f 100644 --- a/pkg/bindings/containers/attach.go +++ b/pkg/bindings/containers/attach.go @@ -343,7 +343,7 @@ func attachHandleResize(ctx, winCtx context.Context, winChange chan os.Signal, i resizeErr = ResizeContainerTTY(ctx, id, new(ResizeTTYOptions).WithHeight(h).WithWidth(w)) } if resizeErr != nil { - logrus.Warnf("failed to resize TTY: %v", resizeErr) + logrus.Infof("failed to resize TTY: %v", resizeErr) } } @@ -408,6 +408,17 @@ func ExecStartAndAttach(ctx context.Context, sessionID string, options *ExecStar // If we are in TTY mode, we need to set raw mode for the terminal. // TODO: Share all of this with Attach() for containers. needTTY := terminalFile != nil && terminal.IsTerminal(int(terminalFile.Fd())) && isTerm + + body := struct { + Detach bool `json:"Detach"` + TTY bool `json:"Tty"` + Height uint16 `json:"h"` + Width uint16 `json:"w"` + }{ + Detach: false, + TTY: needTTY, + } + if needTTY { state, err := setRawTerminal(terminalFile) if err != nil { @@ -419,13 +430,14 @@ func ExecStartAndAttach(ctx context.Context, sessionID string, options *ExecStar } logrus.SetFormatter(&logrus.TextFormatter{}) }() + w, h, err := terminal.GetSize(int(terminalFile.Fd())) + if err != nil { + logrus.Warnf("failed to obtain TTY size: %v", err) + } + body.Width = uint16(w) + body.Height = uint16(h) } - body := struct { - Detach bool `json:"Detach"` - }{ - Detach: false, - } bodyJSON, err := json.Marshal(body) if err != nil { return err diff --git a/pkg/domain/infra/abi/terminal/terminal_linux.go b/pkg/domain/infra/abi/terminal/terminal_linux.go index ab71f8f6fb..09c0f802d2 100644 --- a/pkg/domain/infra/abi/terminal/terminal_linux.go +++ b/pkg/domain/infra/abi/terminal/terminal_linux.go @@ -15,12 +15,13 @@ import ( // ExecAttachCtr execs and attaches to a container func ExecAttachCtr(ctx context.Context, ctr *libpod.Container, execConfig *libpod.ExecConfig, streams *define.AttachStreams) (int, error) { - resize := make(chan define.TerminalSize) + var resize chan define.TerminalSize haveTerminal := terminal.IsTerminal(int(os.Stdin.Fd())) // Check if we are attached to a terminal. If we are, generate resize // events, and set the terminal to raw mode if haveTerminal && execConfig.Terminal { + resize = make(chan define.TerminalSize) cancel, oldTermState, err := handleTerminalAttach(ctx, resize) if err != nil { return -1, err @@ -32,7 +33,6 @@ func ExecAttachCtr(ctx context.Context, ctr *libpod.Container, execConfig *libpo } }() } - return ctr.Exec(execConfig, streams, resize) } diff --git a/test/system/450-interactive.bats b/test/system/450-interactive.bats index a2db39492c..53925b3c8a 100644 --- a/test/system/450-interactive.bats +++ b/test/system/450-interactive.bats @@ -57,7 +57,16 @@ function teardown() { # ...and make sure stty under podman reads that. run_podman run -it --name mystty $IMAGE stty size <$PODMAN_TEST_PTY - is "$output" "$rows $cols" "stty under podman reads the correct dimensions" + is "$output" "$rows $cols" "stty under podman run reads the correct dimensions" + + run_podman rm -f mystty + + # check that the same works for podman exec + run_podman run -d --name mystty $IMAGE top + run_podman exec -it mystty stty size <$PODMAN_TEST_PTY + is "$output" "$rows $cols" "stty under podman exec reads the correct dimensions" + + run_podman rm -f mystty }