ctr: make exec pty behavior consistent with run#5527
Merged
estesp merged 2 commits intocontainerd:masterfrom May 25, 2021
Merged
ctr: make exec pty behavior consistent with run#5527estesp merged 2 commits intocontainerd:masterfrom
estesp merged 2 commits intocontainerd:masterfrom
Conversation
Use cio.WithStreams with explicit console device when --tty is passed, consistent with how ctr run behaves. Signed-off-by: Samuel Karp <[email protected]>
|
Build succeeded.
|
ktock
reviewed
May 24, 2021
Member
There was a problem hiding this comment.
The following check in this if block is not needed because tty is always false here.
if tty {
return errors.New("can't use log-uri with tty")
}
Member
Author
There was a problem hiding this comment.
Thanks for catching that! I've restructured the logic here to make it more clear as to which cases are which, though it now does look a bit different from ctr run (which doesn't have the same error checks).
Handle initial pty resize after the exec process has started and the pty is available, consistent with the behavior of ctr run. Signed-off-by: Samuel Karp <[email protected]>
samuelkarp
force-pushed
the
freebsd-ctr-exec
branch
from
6bab7cd to
5dec27b
Compare
|
Build succeeded.
|
ktock
approved these changes
May 25, 2021
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This series adjusts the pty behavior in
ctr task execto be consistent withctr run. Two changes are made:console.Consoleis found prior totask.Execand used as withcio.WithStreamstask.StartBoth of these changes seem necessary on FreeBSD with runj; without change 1 keyboard input did not seem to be correctly sent to the shim and without change 2 the initial resize was lost.
I also tested these changes on Linux and
ctr task execseemed to continue to work fine both before and after these commits. I do not currently have access to a Windows machine to test any behavioral differences there.