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

Forward STDOUT/STDERR #42

Closed
alixaxel opened this issue Nov 5, 2015 · 4 comments
Closed

Forward STDOUT/STDERR #42

alixaxel opened this issue Nov 5, 2015 · 4 comments
Labels
Milestone

Comments

@alixaxel
Copy link
Contributor

alixaxel commented Nov 5, 2015

With a simple PHP script that writes to both STDOUT and STDERR:

fwrite(STDOUT, "stdout\n");
fwrite(STDERR, "stderr\n");

The output streams work as expected when executed with PHP directly:

alix@ThinkPad:~/dexec_1.0.2_linux_386$ php streams.php 1>/dev/null
stderr
alix@ThinkPad:~/dexec_1.0.2_linux_386$ php streams.php 2>/dev/null
stdout

However, when running via dexec, STDERR is treated as if it was STDOUT:

alix@ThinkPad:~/dexec_1.0.2_linux_386$ dexec streams.php 1>/dev/null
alix@ThinkPad:~/dexec_1.0.2_linux_386$ dexec streams.php 2>/dev/null
stdout
stderr

The exact same thing happens with a Node.js script:

console.log("stdout");
console.error("stderr");
alix@ThinkPad:~/dexec_1.0.2_linux_386$ node streams.js 1>/dev/null
stderr
alix@ThinkPad:~/dexec_1.0.2_linux_386$ node streams.js 2>/dev/null
stdout
alix@ThinkPad:~/dexec_1.0.2_linux_386$ dexec streams.js 1>/dev/null
alix@ThinkPad:~/dexec_1.0.2_linux_386$ dexec streams.js 2>/dev/null
stdout
stderr

Is there any option to make dexec respect and forward STDOUT and STDERR?

@andystanton
Copy link
Member

Hi @alixaxel

There is not an option for this at the moment, but there should be! I'll add this in for the next release.

@alixaxel
Copy link
Contributor Author

@andystanton Thanks for getting back to me. Yeah, I think that this feature is a important one to have.

If I use the following code as it's being used by RunAnonymousContainer, everything works as expected.

package main

import (
    "os"
    "os/exec"
)

func main() {
    cmd := exec.Command("node", "/home/alix/streams.js")

    cmd.Stdout = os.Stdout
    cmd.Stderr = os.Stderr

    cmd.Run()
}

So my assumption is that Docker itself is not forwarding the streams properly.

@alixaxel
Copy link
Contributor Author

I found this old related issue (moby/moby#725), but it was fixed a long time ago.

So I did a couple of tests running the Docker container directly:

alix@900X4C:~/$ docker run --rm -v $(pwd -P)/streams.js:/tmp/dexec/build/streams.js -t dexec/lang-node streams.js
stdout
stderr
alix@900X4C:~/$ docker run --rm -v $(pwd -P)/streams.js:/tmp/dexec/build/streams.js -t dexec/lang-node streams.js 1>/dev/null
alix@900X4C:~/$ docker run --rm -v $(pwd -P)/streams.js:/tmp/dexec/build/streams.js -t dexec/lang-node streams.js 2>/dev/null
stdout
stderr

Same behavior. However, if you remove the -t flag:

alix@900X4C:~/$ docker run --rm -v $(pwd -P)/streams.js:/tmp/dexec/build/streams.js dexec/lang-node streams.js
stderr
stdout
alix@900X4C:~/$ docker run --rm -v $(pwd -P)/streams.js:/tmp/dexec/build/streams.js dexec/lang-node streams.js 1>/dev/null
stderr
alix@900X4C:~/$ docker run --rm -v $(pwd -P)/streams.js:/tmp/dexec/build/streams.js dexec/lang-node streams.js 2>/dev/null
stdout

It works out nicely. The flag -t allocates a new TTY but I'm not sure why we even need this in dexec.

  -t, --tty=false            Allocate a pseudo-TTY

Would you accept a PR that removes it or is this critical for something else?

alixaxel added a commit to alixaxel/dexec that referenced this issue Nov 13, 2015
andystanton added a commit that referenced this issue Nov 13, 2015
@andystanton andystanton added this to the v1.0.3 milestone Nov 13, 2015
@andystanton
Copy link
Member

Thanks for the fix @alixaxel I've accepted the PR and will make a 1.0.3 patch release containing this fix.

Ultimately I want to move away from shelling out commands to the Docker CLI. I created #35 to address this and have done some work, but I think a better approach would be to make dexec use one of the existing Go libraries: https://docs.docker.com/engine/reference/api/remote_api_client_libraries/

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

No branches or pull requests

3 participants