apocas / dockerode

Docker + Node = Dockerode (Node.js module for Docker's Remote API)
Apache License 2.0
4.32k stars 460 forks source link

container.logs doesn't always return a stream #456

Open Cherry opened 6 years ago

Cherry commented 6 years ago

As per the example at https://github.com/apocas/dockerode/blob/master/examples/logs.js, it would seem that container.logs always returns a stream, however after updating to Docker 18.04, we're seeing this return a long string, newline delimited. We're passing the following options:

Here's a full test script to replicate the issue, using Ubuntu 16.04.4 LTS, Docker 18.04, Node 8.11.1, and Dockerode 2.5.5.

https://gist.github.com/Cherry/0bc3b769d72d24fe61c61e38a55d6f7d

You'll have to run it once to create the container, and then a second time to find the stream as a string.

This was not occurring prior to upgrading our Docker version.

aguegu commented 5 years ago

It is an interesting bug.

My Docker version is "18.06.1-ce".

By setting follow: false, in dockerode 2.5.0, await container.logs() return a stream, while in version 2.5.7, it return a string.

Somehow this issue is not new, as #237

aguegu commented 5 years ago

As I test with dockerode 2.5.7.

By setting option follow: true, await contatiner.logs() returns a stream, with follow: false it returns a string.

Cherry commented 5 years ago

FYI to anyone following along with this issue, Dockerode 2.5.8 appears to now return a Buffer, instead of a string as per https://github.com/apocas/docker-modem/commit/287161b0905d18b059fbe0ddc184a70e22af3849, so you need to add a Buffer.isBuffer check and toString the result. I've submitted a PR to resolve this and return the string, even if the parseJSON fails: https://github.com/apocas/docker-modem/pull/100

softprops commented 5 years ago

@Cherry I just bumped into this issue. I just followed this option all the way back to the modem code. Thanks for the tip in discerning between the two types.

How do we work around demuxing the buffer if stdout and stderr are both true? I want to do something specifically different with stdout than the stdout data but I can't figure out how to separate them.

lesomnus commented 3 years ago

Is there any update? I got a Buffer that starts with 8 bytes header for each line rather than ReadableStream and the header specification is can be found this (on Stream format section). Is this an intended result?

rhyek commented 3 years ago

Is it really necessary to jump through so many hoops to get a simple string output from logs? Seems like a wasted opportunity to have this function to all of the work for you if that's not its current purpose.

maxsatula commented 2 years ago

By setting option follow: true, await contatiner.logs() returns a stream, with follow: false it returns a string.

Sometimes with follow: false an output log is still enormous; I would keep a possibility to return a stream regardless of a follow option. On the other hand, for those who do not want to jump through hoops processing streams, I would keep a possibility to get a string. How about creating an option to specify an expected result type which is independend of follow ?

nwalters512 commented 1 year ago

I was able to repurpose demuxStream from docker-modem into a function to "demux" a Buffer like you get when follow: false is set:

function demuxOutput(buffer) {
  var nextDataLength = null;
  let output = Buffer.from([]);

  while (buffer.length > 0) {
    console.log(buffer.length, buffer.toString('utf8'));
    var header = bufferSlice(8);
    nextDataLength = header.readUInt32BE(4);

    var content = bufferSlice(nextDataLength);
    output = Buffer.concat([output, content]);
  }

  function bufferSlice(end) {
    var out = buffer.slice(0, end);
    buffer = Buffer.from(buffer.slice(end, buffer.length));
    return out;
  }

  return output;
}

I was only interested in a single output stream, so it doesn't actually split it into stdout/stderr, but that'd be an easy change to make. Caveat: I have not tested edge cases here; there may be situations where this actually infinite-loops.

I'll plus-one everyone else saying that it would be great if Dockerode handled this for us!

fefrei commented 1 year ago

Thank you, @nwalters512!

Here's a version for TypeScript that also separates stdout/stderr (and only uses Buffer.concat once per stream):

const demuxOutput = (buffer: Buffer): { stdout: string, stderr: string } => {
    const stdouts: Buffer[] = [];
    const stderrs: Buffer[] = [];

    function bufferSlice(end: number) {
        const out = buffer.slice(0, end);
        buffer = Buffer.from(buffer.slice(end, buffer.length));
        return out;
    }

    while (buffer.length > 0) {
        const header = bufferSlice(8);
        const nextDataType = header.readUInt8(0);
        const nextDataLength = header.readUInt32BE(4);

        const content = bufferSlice(nextDataLength);
        switch (nextDataType) {
            case 1:
                stdouts.push(content);
                break;
            case 2:
                stderrs.push(content);
                break;
            default:
                // ignore
        }
    }

    return { stdout: Buffer.concat(stdouts).toString("utf8"), stderr: Buffer.concat(stderrs).toString("utf8") };
}