Fix tty settings

[datawolf]

When integrating the newest runc with docker and containerd, we found an io issue if we running a container with a tty(-t/--tty).
the command is docker run -ti hello-world

the issue looks like:

$ docker run -ti hello-world

Hello from Docker!
                  This message shows that your installation appears to be working correctly.
				  
                                                                                            To generate this message, Docker took the following steps:
                                                                                                                                                      1. The Docker client contacted the Docker daemon.
                                                  2. The Docker daemon pulled the "hello-world" image from the Docker Hub.
                                                                                                                           3. The Docker daemon created a new container from that image which runs the
                                                    executable that produces the output you are currently reading.
                                                                                                                   4. The Docker daemon streamed that output to the Docker client, which sent it
                                              to your terminal.

                                                               To try something more ambitious, you can run an Ubuntu container with:
                                                                                                                                      $ docker run -it ubuntu bash

            Share images, automate workflows, and more with a free Docker Hub account:
                                                                                       https://hub.docker.com

                                                                                                             For more examples and ideas, visit:
                                                                                                                                                 https://docs.docker.com/engine/userguide/

It took me a long time to solve the problem, the bug is that runc's output is not correct when we running a container with a tty(-t/--tty).

~~~so https://github.com/opencontainers/runc/issues/1145) is not an issue and we should revert pr https://github.com/opencontainers/runc/pull/1146~~~

~~~After reverting the pr https://github.com/opencontainers/runc/pull/1146, we need to fix the integration test cases for exec command.~~~

In addition, the `MakeRaw` method in docker(`pkg/term/tc_linux_cgo.go`) and 
runc(`Godeps/_workspace/src/github.com/docker/docker/pkg/term/tc_linux_cgo.go`) is different, the tty
in docker and runc has different settings. So it is better to update the `github.com/docker/docker/pkg/term/` for runc.

[datawolf]

ping @hqhq @cyphar

[cyphar]

@datawolf The problem is that expecting \r causes issues in a lot of other cases. Now, what we could do is make it so that when you're running runc run or runc start on a non-detached container then we set \r off (but not for detached containers).

However, IMO if you want to ensure that a certain flag is set on the TTY then use --console-socket and set it yourself -- that's the point of that flag.

Also please don't use godep update. Ever. There's no way we're going to merge ~120k lines of vendor changes.

[datawolf]

so that when you're running runc run or runc start on a non-detached container then we set \r off (but not for detached containers).

For the detached containers, if we use -t/-tty, the current code also set \r off. I think that the issue is only related to -t/-tty.

after update the pkg/term/, the output of the runc start is confusing when we running a container with a tty(-t/-tty).

it looks like:

# runc run -b /busybox/ test
hello docker
            and
               hello world
                          #

My config.json is(process.terminial=true):

	"process": {
		"terminal": true,
		"consoleSize": {
			"height": 0,
			"width": 0
		},
		"user": {
			"uid": 0,
			"gid": 0
		},
		"args": [
			"/test.sh"
		],
		"env": [
			"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
			"TERM=xterm"
		],
		"cwd": "/",
		"capabilities": [
			"CAP_AUDIT_WRITE",
			"CAP_KILL",
			"CAP_NET_BIND_SERVICE"
		],
		"rlimits": [
			{
				"type": "RLIMIT_NOFILE",
				"hard": 1024,
				"soft": 1024
			}
		],
		"noNewPrivileges": true
	},

and test.sh is:

# cat /busybox/rootfs/test.sh 
#!/bin/sh

echo "hello docker"
echo "and"
echo "hello world"

Because we set the os.Stdin.Fd() in raw mode, \n cannot set the column to zero. I think we should first set theos.Stdin.Fd() in raw mode and the set the OPOST on c_oflag to enable output process(the only ONLCR output flag is set ).

@cyphar what do you think about it?

[datawolf]

after update the pkg/term/, and enable OPOST for c_oflag can fix the runc start is confusing issue.

[hqhq]

Move appendant OPOST from vendor package to runc package doesn't look right to me, we shouldn't have OPOST in the first place. We should figure out what's the right way for outputs in linux raw terminal.

[hqhq]

I thought more on this and think maybe we should not change the default behavior on linux raw terminal. See moby/moby#30156 (comment) .

So I think maybe we should update term package in vendor to remove the OPOST for raw terminal, and revert the saneTerminal hack in runc, so we'll back to have \r\n in process output, but that's not added by runc, but default raw terminal behavior, it already turned out that touching it would cause a lot of pain on the receive side. @cyphar @mlaventure WDYT.

[cyphar]

@hqhq saneTerminal does actually fix issues though (the \rs broke bats several times, and not to mention that it will cause issues with anyone that tries to script output from runC). And it does not make sense for us to be outputting \r\n in a terminal program because literally no other program does this. I don't know why we're having issues on the recieve side, but on the sending side this is just silly.

The reason why ptys have this default is because when you're outputting to a tele-type console you need to output \r and \n in order to get a proper newline. We are not outputting to regular consoles (and we have a console being proxied by our shell) -- so \r does not make sense.

Remove OPOST and then note the difference between:

% runc exec ctr echo "hello world" | cat -v

and

% echo "hello world" | cat -v

[cyphar]

To answer this comment:

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

I disagree. Why are we outputting random junk to a terminal when there's already another terminal emulator in front of it which also has a pty that is also adding OPOST...

I'm going to look at this tomorrow, but I get the feeling the issue is actually to do with terminal emulators and not runC...

[crosbymichael]

The problem is the docker term package and the vendored one from docker in runc are out of sync. Someone removed the OPOST in docker and that is causing the terms to be out of sync.

I think the OPOST should be added back but am looking into it.

[crosbymichael]

This has been resolved. I have synced the two term packages with the OPOST change and verified it works with saneTerminal properly.