----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68810/#review209133 -----------------------------------------------------------
Ship it! Ship It! - Jie Yu On Sept. 29, 2018, 12:05 p.m., Kevin Klues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68810/ > ----------------------------------------------------------- > > (Updated Sept. 29, 2018, 12:05 p.m.) > > > Review request for mesos, Gilbert Song and Jie Yu. > > > Bugs: MESOS-8978 > https://issues.apache.org/jira/browse/MESOS-8978 > > > Repository: mesos > > > Description > ------- > > Previously, we would unconditionally call 'setsid()' in the command > executor whenever we launched a process. This causes the process it > launches to start a new session and NOT inherit the controlling TTY > from it's parent. This obviously causes problems, if the goal of > attaching a TTY to a task is so that we can actually give it control > of that TTY while it is executing. > > Interestingly, even if process does not control its TTY, it can still > read and write from the file descriptors associated with it (it just > can't receive any signals associated with it, such as SIGWINCH, > SIGHUP, etc.). This is why things "appeared" to mostly work until this > point because stdin/stdout/stderr were all being redirected properly > to it. > > Where we saw problems was with trying to 'attach' to an already > running process launched via the command executor using the > ATTACH_NESTED_CONTAINER_INPUT/OUTPUT calls. If you ran something like > 'bash', you would not be able to do job control, and you could not > resize the screen properly when running things like 'vim'. > > This commit fixes this issue by checking to see if a TTY has been > attached to a task launched by the command executor, and skipping the > 'setsid()' call when forking it. We considered a number of alternative > approaches, but finally settled on this one since it was the least > invasive. I.e. only tasks launched with a TTY (which is a failry new > concept in Mesos) will be affected by this change; the semantics of > all traditional tasks launched via the command executor will go > unchanged. > > Note: this problem does not exists for the default executor because > the agent does the job of launching all containers, and there is no > executor sitting between the containerizer and the actual process of a > task intervening to call an extra 'setsid()'. This is why containers > launched via LAUNCH_NESTED_CONTAINER_SESSION have always worked as > expected. > > > Diffs > ----- > > src/launcher/executor.cpp 86fb87dfbaa4daba22ec491ca5fca0e562797521 > src/slave/containerizer/mesos/io/switchboard.cpp > 498c008c36725b4ed43b4905d3640508beaaae93 > > > Diff: https://reviews.apache.org/r/68810/diff/2/ > > > Testing > ------- > > $ make -j 40 check > > > Thanks, > > Kevin Klues > >
