Always set the console size #3572
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3572 +/- ##
==========================================
+ Coverage 59.36% 59.37% +0.01%
==========================================
Files 285 285
Lines 24097 24096 -1
==========================================
+ Hits 14306 14308 +2
+ Misses 8930 8928 -2
+ Partials 861 860 -1 |
|
relates to #3554 (see the top description) This change looks correct indeed (at least from the client side); what I noticed though is that the code you linked to is "on windows only"; https://github.com/moby/moby/blob/master/daemon/oci_windows.go#L127-L132 I see the corresponding implementation for linux does not take these values into account; https://github.com/moby/moby/blob/cb07afa3cc1cbc7adfab3ac65d92c497a3271b50/daemon/oci_linux.go#L1001 A quick look at the runtime-spec shows that the And the description of it also doesn't mention it to be "Windows only"; https://github.com/opencontainers/runtime-spec/blob/a3c33d663ebc56c4d35dbceaa447c7bf37f6fab3/config.md#process So perhaps we should (as a follow-up);
It may be worth adding a comment to the lines changed here, that they currently are ignored on Linux daemons |
| if runtime.GOOS == "windows" { | ||
| hostConfig.ConsoleSize[0], hostConfig.ConsoleSize[1] = dockerCli.Out().GetTtySize() | ||
| } | ||
| hostConfig.ConsoleSize[0], hostConfig.ConsoleSize[1] = dockerCli.Out().GetTtySize() |
There was a problem hiding this comment.
See my comment above; perhaps we should (until fixed) add a comment here that this option is currently ignored on Linux daemons
This check doesn't really make sense because the client doesn't know on what OS the daemon is really running. The daemon uses the console size on creation when available (on windows). Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
- What I did
Removed the check for the OS when setting the console size on container creation. This check doesn't really make sense because the client doesn't know on what OS the daemon is really running. The daemon uses the console size on creation when available (on windows).
- How I did it
Checked the daemon code that it uses the console size only when available.
- How to verify it
docker run ...should still work- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)
