Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attaching and streaming logs from containers started with tty=true does not work properly #630

Closed
dano opened this issue Jun 7, 2015 · 1 comment
Milestone

Comments

@dano
Copy link
Contributor

dano commented Jun 7, 2015

Currently, when docker-py attaches to a container, it will always try to read the output stream as if it was multiplexed, unless the API version is below 1.6:

        # Stream multi-plexing was only introduced in API v1.6. Anything before
        # that needs old-style streaming.

        if utils.compare_version('1.6', self._version) < 0:
            def stream_result():
                self._raise_for_status(response)
                for line in response.iter_lines(chunk_size=1,
                                                decode_unicode=True):
                    # filter out keep-alive new lines
                    if line:
                        yield line

            return stream_result() if stream else \
                self._result(response, binary=True)

        sep = bytes() if six.PY3 else str()

        if stream:
            return self._multiplexed_response_stream_helper(response)
        else:
            return sep.join(
                [x for x in self._multiplexed_buffer_helper(response)]
            )

Client.logs similarly assumes a multiplexed response stream after calling the container/<id>/logs API, unless the API is below version 1.11, in which cases it just calls attach with the logs=True keyword argument.

However, the docker API docs note that if a container is opened with the tty=true option, you just get the a raw stream, not a multiplexed one:

Stream details:

When using the TTY setting is enabled in POST /containers/create , the stream is the raw data from >the process PTY and client's stdin. When the TTY is disabled, then the stream is multiplexed to >separate stdout and stderr.

The code for both attach and logs need to handle cases where the container that is being attached to/streamed from has tty=true. Currently, you'll end up not seeing any output, or seeing delayed and incompleted output from such containers.

dano added a commit to dano/docker-py that referenced this issue Jun 7, 2015
Treat output from TTY-enabled containers as raw streams, rather than
as multiplexed streams. The docker API docs specify that tty-enabled
containers don't multiplex. Also update tests to pass with these
changes.

Addresses issue docker#630

Signed-off-by: Dan O'Reilly <[email protected]>
dano added a commit to dano/docker-py that referenced this issue Jun 7, 2015
Treat output from TTY-enabled containers as raw streams, rather than
as multiplexed streams. The docker API docs specify that tty-enabled
containers don't multiplex. Also update tests to pass with these
changes, and changed the code used to read raw streams to not
read line-by-line, and to not skip empty lines.

Addresses issue docker#630

Signed-off-by: Dan O'Reilly <[email protected]>
@shin- shin- modified the milestone: 1.3.0 Jun 16, 2015
dano added a commit to dano/docker-py that referenced this issue Jul 6, 2015
Treat output from TTY-enabled containers as raw streams, rather than
as multiplexed streams. The docker API docs specify that tty-enabled
containers don't multiplex. Also update tests to pass with these
changes, and changed the code used to read raw streams to not
read line-by-line, and to not skip empty lines.

Addresses issue docker#630

Signed-off-by: Dan O'Reilly <[email protected]>
dano added a commit to dano/docker-py that referenced this issue Jul 6, 2015
Treat output from TTY-enabled containers as raw streams, rather than
as multiplexed streams. The docker API docs specify that tty-enabled
containers don't multiplex. Also update tests to pass with these
changes, and changed the code used to read raw streams to not
read line-by-line, and to not skip empty lines.

Addresses issue docker#630

Signed-off-by: Dan O'Reilly <[email protected]>
@shin-
Copy link
Contributor

shin- commented Jul 8, 2015

Closing via #669

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants