cli/container: Fill ConsoleSize in create #3619
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This makes the containers have an expected console size not only for `run` but also for `create`. Also remove the comment, as this is no longer ignored on Linux daemon since e994efcf64c133de799f16f5cd6feb1fc41fade4 Signed-off-by: Paweł Gronowski <[email protected]>
|
Just wondering if this makes sense if config.Tty is false? |
vvoland
commented
May 18, 2022
| @@ -261,6 +261,8 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerConfig | |||
| } | |||
| } | |||
|
|
|||
| hostConfig.ConsoleSize[0], hostConfig.ConsoleSize[1] = dockerCli.Out().GetTtySize() | |||
There was a problem hiding this comment.
Shouldn't this be filled only if config.Tty?
There was a problem hiding this comment.
Yes, I was somewhat looking at that; then again, if I'm not mistaken, that's also gated at the daemon-side (so it only applies the options if there's a TTY), and possibly (need to check) again on the runtime (containerd) side.
The code was already taking this approach, so I didn't consider it a blocker, but we could look into different scenarios (feel free to do so 😃).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to moby/moby#43593
- What I did
Fill HostConfig.ConsoleSize for both
docker runanddocker create- How I did it
Move the code that sets the ConsoleSize from runContainer to createContainer.
This does not impact the runContainer, as it calls createContainer anyway.
- How to verify it
- A picture of a cute animal (not mandatory but encouraged)