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

Docker number of lines in terminal changing inside docker #25450

Closed
silgon opened this Issue Aug 5, 2016 · 34 comments

Comments

Projects
None yet
@silgon

silgon commented Aug 5, 2016

I would like to know how to change the following behavior. Let's say my terminal has 28 lines. Then I use the following commands:

$ tput lines # my terminal
28
$ docker run  --rm  -it ubuntu:16.04 tput lines  # docker container
24  ## WHY??
$ docker run  --rm  -it ubuntu:16.04 bash # docker container inside command
root@810effa2777c:/# tput lines
28

As you can see, even when all the results should be 28, when I'm calling the container as docker run --rm -it ubuntu:16.04 tput lines it always gives me 24 despite the size of my terminal. This is not only with the ubuntu container, I also tried with debian (docker run --rm -it debian tput lines) and I'm having the same result 24.

The purpose of this is to use the mdp presentation tool which takes into account the lines in your terminal. When my implementation failed, I tried some other person's docker implementation but I ran to the same error.

Here's my error in an image:

Docker number of lines in terminal changing inside docker

Does anyone has any idea what it could be and how can this be solved?

@silgon

This comment has been minimized.

silgon commented Aug 8, 2016

I got an answer in stackoverflow. I think the problem is interesting if you want to check it out. I'm not sure if it's a intended behavior though. http://stackoverflow.com/a/38825323/2237916

@sudo-bmitch

This comment has been minimized.

sudo-bmitch commented Aug 8, 2016

Here's a bit more expansion on the timing issue being discussed on the stackoverflow thread. The attach and tty resize appears to be run after the container is started.

$ docker run -dit --name test-lines ubuntu /bin/bash -c 'while true; do date; tput lines; sleep 1; done' \
  && sleep 5 && docker logs test-lines && docker attach test-lines
4ba4f5c6991f231f88d11e8dfedb04610576b1555945bc6956123799aace517a
Mon Aug  8 11:59:05 UTC 2016
24
Mon Aug  8 11:59:06 UTC 2016
24
Mon Aug  8 11:59:07 UTC 2016
24
Mon Aug  8 11:59:08 UTC 2016
24
Mon Aug  8 11:59:09 UTC 2016
24
Mon Aug  8 11:59:10 UTC 2016
24
Mon Aug  8 11:59:11 UTC 2016
55
Mon Aug  8 11:59:12 UTC 2016
55
Mon Aug  8 11:59:13 UTC 2016
55
Mon Aug  8 11:59:14 UTC 2016
55
Mon Aug  8 11:59:15 UTC 2016
55
# <cont>-P <cont>-Q used here to detach

@thaJeztah

This comment has been minimized.

Member

thaJeztah commented Aug 8, 2016

I think the conclusion on StackOverflow is correct; docker run -it (docker run) is basically a shorthand implemented in the client that consists of three separate actions;

  1. docker create
  2. docker start
  3. (if -i / --interactive): docker attach

docker create / docker start are executed with the default terminal dimensions (80x24), and attach sets parameters to resize the terminal to the dimensions it detected in the current shell;

If you enable debug on the daemon, and check the logs, you'll see this happening;

docker run -it ubuntu /bin/bash -c 'tput lines'

Daemon logs (relevant lines);

DEBU[0016] Calling POST /v1.25/containers/create
DEBU[0016] Calling POST /v1.25/containers/5314ea9052931f602aa752bbae8c2e01b5057c720a4a296f29a621fefd147f0e/start
DEBU[0016] Calling POST /v1.25/containers/5314ea9052931f602aa752bbae8c2e01b5057c720a4a296f29a621fefd147f0e/resize?h=46&w=221
@thaJeztah

This comment has been minimized.

Member

thaJeztah commented Aug 8, 2016

Ok, so apparently on Windows, there's a ConsoleSize property, which initial console size is set when creating the container;

https://github.com/docker/docker/blob/506234e3f0f6d8f8a0629e09f38dfe96ada025c2/api/client/container/run.go#L145-L150

@mlaventure would it make sense to use this property on Linux as well, and set the initial console size using that value?

@mlaventure

This comment has been minimized.

Contributor

mlaventure commented Aug 8, 2016

It's doable. It'll require changes to containerd to add the needed values to the CreateContainer RPC call though.

@thaJeztah

This comment has been minimized.

Member

thaJeztah commented Aug 8, 2016

@mlaventure would it be a small change? Wondering how corner-case this is, and if it's worth making the change

@mlaventure

This comment has been minimized.

Contributor

mlaventure commented Aug 8, 2016

It should be a trivial change. I personally would consider this a small bug.

I'll assign it to me and try to make a PR to have it fixed for 1.13.0

@cyphar

This comment has been minimized.

Contributor

cyphar commented Sep 20, 2016

This may be related to the current discussion about adding the initial console size to all platforms: opencontainers/runtime-spec#563.

@thaJeztah

This comment has been minimized.

Member

thaJeztah commented Sep 20, 2016

Thanks for linking @cyphar

@mlaventure

This comment has been minimized.

Contributor

mlaventure commented Sep 20, 2016

Yes, I saw that, decided to wait until this gets into runc to implement.

Thanks for linking :)

@shehi

This comment has been minimized.

shehi commented Apr 13, 2017

Is this fixed in v17.04?

@mlaventure

This comment has been minimized.

Contributor

mlaventure commented Apr 13, 2017

@shehi unfortunately no, didn't got around to tackle it yet.

External contributions are always welcome ;-)

@shehi

This comment has been minimized.

shehi commented Apr 13, 2017

I am afraid I am not really into Linux OS/Kernel programming. After I spin up my containers through docker-compose, I get default 80x24 terminal (debian:jessie based container generally happen to have this problem, somehow ubuntu based container happens to have working terminal with host machine's cols and rows values). For the moment I am fixing this problem through stty cols XXX rows YYY command. Thanks for the update!

@dragon788

This comment has been minimized.

Contributor

dragon788 commented Sep 30, 2017

I'd love to see this fixed as I'm following in jessfraz's footsteps of running everything I can in containers, and having the output line wrapping way inside the bounds of my host terminal is rather painful.

@jonasrauber

This comment has been minimized.

jonasrauber commented Nov 15, 2017

The same problem happens with docker exec, btw. Sometimes, the size gets set after a second or so, sometimes it does not get set at all:

$ docker exec -it container /bin/bash -c 'while true; do date; tput lines; sleep 1; done'
Wed Nov 15 10:29:12 Europe 2017
24
Wed Nov 15 10:29:13 Europe 2017
98
Wed Nov 15 10:29:14 Europe 2017
98
...
$ docker exec -it container /bin/bash -c 'while true; do date; tput lines; sleep 1; done'
Wed Nov 15 10:29:27 Europe 2017
24
Wed Nov 15 10:29:28 Europe 2017
24
Wed Nov 15 10:29:29 Europe 2017
24
Wed Nov 15 10:29:30 Europe 2017
24
Wed Nov 15 10:29:31 Europe 2017
24
Wed Nov 15 10:29:32 Europe 2017
24
...

Does anyone know a workaround to always get the first behavior?

@boaz1337

This comment has been minimized.

Member

boaz1337 commented Nov 15, 2017

I am using uxrvt and docker 17.09.0-ce on Fedora 26.
Fortunately, I can't reproduce this bug: the screen height is being updated.

@jonasrauber

This comment has been minimized.

jonasrauber commented Nov 15, 2017

@ripcurld0 For me, it's also updated most of the time (just not always), see above.

My setup: Ubuntu 16.04 with Docker version 17.09.0-ce, build afdb6d4

@boaz1337

This comment has been minimized.

Member

boaz1337 commented Nov 15, 2017

@thaJeztah @mlaventure should we move this issue to runc?
according to @jonasrauber it is not always updated and from what I read above it is now taken care of on the runc side.

@cyphar

This comment has been minimized.

Contributor

cyphar commented Nov 16, 2017

We've already solved this problem in runc -- opencontainers/runc#1275. Docker should update to a version of runc that contains that fix.

@thaJeztah

This comment has been minimized.

Member

thaJeztah commented Nov 16, 2017

Thanks @cyphar I see that fix was merged October 4th, would indeed not be in 17.09 (which uses https://github.com/opencontainers/runc/commits/3f2f8b84a77f73d38244dd690525642a72156c64 (looking at https://github.com/docker/docker-ce/blob/v17.09.0-ce/components/engine/hack/dockerfile/binaries-commits#L6). There were a couple of bumps after that, but those not yet include that fix.

This PR: #35465 was just merged on master, and bumps RunC to https://github.com/opencontainers/runc/commits/b2567b37d7b75eb4cf325b77297b140ea686ce8f (which does include that fix)

@boaz1337

This comment has been minimized.

Member

boaz1337 commented Nov 16, 2017

Can we close this ticket or we should wait till this fix lands into production?

@ston3o

This comment has been minimized.

ston3o commented Dec 28, 2017

docker run --rm -it -e COLUMNS=$COLUMNS -e LINES=$LINES -e TERM=$TERM -it ubuntu:16.04 tput lines
@boaz1337

This comment has been minimized.

Member

boaz1337 commented Dec 28, 2017

Thanks. I'm closing this.

@boaz1337 boaz1337 closed this Dec 28, 2017

mishas added a commit to nadirizr/dazel that referenced this issue Jan 2, 2018

mishas added a commit to nadirizr/dazel that referenced this issue Jan 2, 2018

@dcosson

This comment has been minimized.

dcosson commented Jan 5, 2018

Sorry if this is a dumb question but I don't quite follow the resolution. Is @ston3o's suggestion demonstrating that the fix is out, and you should always explicitly pass COLUMNS and LINES to docker run -it now? Or is this a workaround until an upstream fix makes it into a future release?

@cyphar

This comment has been minimized.

Contributor

cyphar commented Jan 5, 2018

If you are running a container in your terminal, Docker will now (with the fix applied) use your terminal size for the container's PTY. I'm not entirely sure what @ston3o's comment was in relation to.

@collinvandyck

This comment has been minimized.

collinvandyck commented Jan 12, 2018

I just updated to the latest CE docker for mac install and I ran into this issue. I'm only able to get it to "work" by passing in the COLUMNS/LINES/TERM vars into docker exec.

Is this something that hasn't landed yet on docker for mac?

@NamPNQ

This comment has been minimized.

NamPNQ commented Feb 5, 2018

@collinvandyck It bug also appear in docker Linux

@haridsv

This comment has been minimized.

haridsv commented Feb 5, 2018

@ston3o Nice, that really works!

When running from a script, $LINES and $COLUMNS are not available, so use -e LINES=$(tput lines) -e COLUMNS=$(tput cols).

@haridsv

This comment has been minimized.

haridsv commented Feb 5, 2018

@ston3o Any idea why the size changes after a sudo?

$ docker exec -it container bash
[root2container /]# tput lines
66
[root2container /]# tput cols
204
[root2container /]# sudo su -
-bash-4.1# tput lines
24
-bash-4.1# tput cols
80
-bash-4.1#
@ston3o

This comment has been minimized.

ston3o commented Feb 5, 2018

@haridsv

sudo -E su
@markeissler

This comment has been minimized.

markeissler commented Feb 7, 2018

Just to clarify the comment from @collinvandyck, I'm running 17.12.0-ce-mac49 (21995) and find that sometimes terminal dimensions are passed correctly sometimes not (more often not) and using the fix from @ston3o (specifying env vars in the docker run -it command line) works--but that sure is cumbersome. If only there was a .dockerrc file that would always pass these values for us...

@dcosson

This comment has been minimized.

dcosson commented Feb 7, 2018

@cyphar it seems like there's still a misunderstanding, it looks like a lot of people here (including me) are not seeing docker automatically use the terminal size for the PTY as you are suggesting. I still need to pass in $COLUMNS and $LINES or the terminal size is incorrect (or if I manually drag to resize the window once container's shell is already open, that also fixes it and from that point it uses my terminal size).

@thaJeztah

This comment has been minimized.

Member

thaJeztah commented Feb 7, 2018

@dcosson this may be related to #35407, which looks to be a race condition in the integration with containerd 1.0+

@thaJeztah

This comment has been minimized.

Member

thaJeztah commented Jun 8, 2018

#35407 is now fixed on master through #37172

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment