bugfix: attach container

[leon3s]

Hey related to this issue: #291

From docker documentation:

When the TTY setting is disabled in POST /containers/create, the HTTP Content-Type header is set to application/vnd.docker.multiplexed-stream and the stream over the hijacked connected is multiplexed to separate out stdout and stderr. The stream consists of a series of frames, each containing a header and a payload.

The header contains the information which the stream writes (stdout or stderr). It also contains the size of the associated frame encoded in the last four bytes (uint32).

The problem was only new line was send where some output is emited without any when we start a container with
Tty: true.
That why the prompt doesn't display because the output send doesn't have new line inside the buffer.

[fussybeaver]

Thanks for this PR.

We had an issue a while back where the logs emitted from the docker server on the aarch64 architecture didn't have a header: #285

For that reason we added a section into the newline parser to fallback to reading newlines instead of reading the length in the header.

Could you change this solution to solve both #291 and #285 ?

Perhaps we need to add a boolean field to the NewlineLogOutputDecoderState struct to toggle the TTY behaviour or perhaps we should use a peekable stream to check if there's a header and toggle behaviour that way.

It's good to see though what the problem/solution is in #291

[leon3s]

@fussybeaver The logs didn't had headers because it's behave the same way as attach when Tty is set to true.

From docker documentation:

Logs returned as a stream in response body.
For the stream format, see the documentation for the attach endpoint.
Note that unlike the attach endpoint, the logs endpoint does not
upgrade the connection and does not set Content-Type.

Stream format when using a TTY

When the TTY setting is enabled in POST /containers/create, the stream is not multiplexed. The data exchanged over the hijacked connection is simply the raw data from the process PTY and client's stdin.

I don't think it's the library responsability to handle new incomming line, but rather the developers that call the function.
Looking at the docker CLI code they inspect the container before logging or attaching to know if a Tty is enabled or not.

People should stop comparing docker CLI with your client btw..

[leon3s]

Looking at the code described on #285

A simple bugfix for him should have been:

   while let Some(Ok(value)) = logs.next().await {
        let data = value.to_string();
        print!("{:?}", data);
    }

Just print without new line

[leon3s]

cc @mrjackwills im taging you because it may introduce a bug in your app

[mrjackwills]

Thanks, I can try to build the application with this PR, and see what, if any, issues arise.

Should be able to look at it this weekend.

[leon3s]

After rethinking about the problem i think i was wrong.
The problem is how the stream is handled in general.
When asking for logs we don't use tcp transport protocol but classic http event stream, that mean curl is buffering what he receive till he have a newline at the end of the buffer before printing, this is why
curl -v --unix-socket /var/run/docker.sock "http://localhost/containers/0ba884c262b3/logs?stdout=true&timestamps=true" have the expected behavior of docker logs

This can be easily replicated with any http client, if we take the response as a stream. So i added a boolean is_tcp to know when we interpret as a tcp stream or as a http event stream
That should not produce bug for your app and give the expected behavior when attaching a container

[fussybeaver]

This looks great, if we can get the log API tested successfully on aarch64 I'm happy to merge.

[mrjackwills]

I built oxker, for all the usual targets (linux x86, linux aarm64, linux armv6, windows) using;

bollard = { git="https://github.com/leon3s/bollard", branch = "fix-attach-container"}

And everything worked as expected, on all platforms, with no difference between this test build and the released application built with Bollard v0.14.0. So I am assuming that it is all good?