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

container output becomes windows-ised #30156

Open
datawolf opened this issue Jan 14, 2017 · 9 comments · Fixed by containerd/containerd#429
Open

container output becomes windows-ised #30156

datawolf opened this issue Jan 14, 2017 · 9 comments · Fixed by containerd/containerd#429
Labels
area/runtime kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. version/master

Comments

@datawolf
Copy link
Contributor

Description
If we running a container with a tty(-t/--tty), the output gets a \n translated to \r\n on output.This happens for both docker run and docker exec.

# docker run -t ubuntu:latest echo hello | cat -v
hello^M
# docker run -tid ubuntu:latest 
845634fb34f7d91112680303f302905ca260f7101e4f1305155476537e65a02e
# docker exec -t 845634fb34f7d91112680303f302905ca260f7101e4f1305155476537e65a02e echo hello | cat -v
hello^M

Steps to reproduce the issue:

  1. use the latest docker
  2. run a container with -t/-tty
 docker run -t ubuntu:latest echo hello | cat -v

Describe the results you received:

# docker run -t ubuntu:latest echo hello | cat -v
hello^M

Describe the results you expected:

# docker run -t ubuntu:latest echo hello | cat -v
hello

Additional information you deem important (e.g. issue happens only occasionally):

Output of docker version:

# docker version
Client:
 Version:      1.14.0-dev
 API version:  1.26
 Go version:   go1.7.4
 Git commit:   1eafa0f
 Built:        Sat Jan 14 03:08:43 2017
 OS/Arch:      linux/amd64

Server:
 Version:      1.14.0-dev
 API version:  1.26 (minimum version 1.12)
 Go version:   go1.7.4
 Git commit:   1eafa0f
 Built:        Sat Jan 14 03:08:43 2017
 OS/Arch:      linux/amd64
 Experimental: false

Additional environment details (AWS, VirtualBox, physical, etc.):

@datawolf
Copy link
Contributor Author

The default terminal setting for a new pty on linux(unix98) has +ONLCR, on containerd-shim we create a net pty, resulting in \n writes by a container process to be converted to \r\n for the output.

Should we make a PR like opencontainers/runc@eea28f4?

@thaJeztah
Copy link
Member

Reopening this, because the version of containerd that's used has not been bumped yet;
https://github.com/docker/docker/blob/36ed7d58bb19e46445c81b6a00a7b75a6280e654/hack/dockerfile/binaries-commits#L5

@thaJeztah thaJeztah reopened this Jan 17, 2017
@thaJeztah thaJeztah added area/runtime kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. labels Jan 17, 2017
@thaJeztah
Copy link
Member

@mlaventure can containerd be bumped, or should that go in sync with RunC?

@mlaventure
Copy link
Contributor

containerd alone should be enough, the shim is the one creating the console.

runC master isn't compatible with containerd at the moment, so it would prove problematic to sync them.

@thaJeztah
Copy link
Member

opened #30226 to bump containerd version

@mlaventure
Copy link
Contributor

Thinking about this more, I think we should probably revert that change from containerd.

The issue is actually more complex than I first thought:

  • The container was created with a tty via -t/--tty, so a new console is created for it and put into raw mode
  • The content of the above console is sent via a pipe as is to dockerd which itself just transfers it to the docker client
    1. Everything works fine if the docker client is run from within a terminal
    2. If the docker client is piped into another program, it doesn't enable raw mode (which is the correct behavior), but the data it received is already "raw" hence the behavior you are seeing.

Note that the logs created by the daemon will also record the output of a raw terminal (which is the expected behavior AFAIC).

So really the solution for the client side would be to have it filter all the terminal escape characters when it is receiving a terminal output but is not outputing to one.

However, I'm not sure it is worth the effort.

@mlaventure
Copy link
Contributor

/cc @crosbymichael maybe you have a different POV on this.

@thaJeztah
Copy link
Member

Also see the discussion on #8513 (comment). Is -t needed in this case? i.e., if you want to pipe the output? I'm not sure either if the fix is correct, given that it's standard behavior of a pty

@hqhq
Copy link
Contributor

hqhq commented Jan 26, 2017

@mlaventure If the receiver is not a terminal, then it should not use -t to create container at all, I doubt this is a real issue now, as long as we have the precondition that process output from a raw terminal would need a raw terminal on the client side to have the right output view.

The case in this issue:

$ docker run -ti --rm ubuntu echo hello | cat -v
hello^M

I think it's invalid because the cat is out of container. If it's in container, everything works right.

qhuang@ubuntu-25:~/tools/runc$ docker run -ti --rm ubuntu bash
root@1b86097ad770:/# echo hello | cat -v
hello

or:

qhuang@ubuntu-25:~/tools/runc$ docker run -ti --rm ubuntu bash -c 'echo hello | cat -v'
hello

If you want to print the output out of docker client (some terminal that hasn't been setup to raw mode), you just shouldn't use -t:

qhuang@ubuntu-25:~/tools/runc$ docker run --rm ubuntu echo hello | cat -v
hello

Which also works well.
All of these also apply to docker exec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/runtime kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. version/master
Projects
None yet
6 participants