Respect follow option on logs endpoint (default false)

[conradoqg]

By default the follow option in the container and service endpoint is false. This fix follows this definition and sets the isStream variable accordingly.

Best

[Seikho]

How is this an improvement or different from the original? This won't coalesce to false if something false-y like null is provided here.

[conradoqg]

Agree, changing.

[Seikho]

This could also use the original logic from lib/task.js that you replaced.

[conradoqg]

Agree, changing.

[conradoqg]

Hey. Could you review the last commit?

I changed the three endpoints that can have logs: container, service, and task.

Thanks

[Seikho]

Looks good to me!

[apocas]

Completely agree on following the Docker's default behavior.

Since we are changing dockerode's default behavior, can't publish this in a minor.
Will ping when published.

[jysperm]

I think this PR introduced a breaking change and published as a patch version (2.5.5).

container.log({follow: false}) will callback with a stream before, but now will callback with a string.

[aguegu]

@jysperm , as discussed in #456

[aguegu]

I understand this merge make dockerode behave more native to docker api. But the outcome is not as good as expected.

With the raw binary output from docker api when follow: false, we have to do hijack the stream format, as https://docs.docker.com/engine/api/v1.37/#operation/ContainerAttach

But this header/body section way does not always bring the right output because the exact body length may not always equal to body size specified in the header later 4 bytes, The body may be cut due to actions like stop or restart, etc. Once that happened, it would require a lot of code to seek back the next header.

This can be verified by sample code like

constresult=awaitcontainer.logs({follow: false,stdout: true,stderr: true,});constbuffer=Buffer.from(result);letp=0;logger.debug(buffer.length);while(p<buffer.length){constheader=buffer.slice(p,p+8);constlen=header.readInt32BE(4);// logger.debug({ p, header, len });p+=8;constcontent=buffer.toString('utf8',p,p+len);console.log(content);p+=len;}

On the other hand, by setting isStream to true in the modem.dial function, in the data in the stream.on('data') event handler args, the first 8 bytes would always be the header, even if the body is shorter than then length it should be.