You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by kl...@apache.org on 2018/10/02 09:41:35 UTC
[mesos] branch master updated (7aaef19 -> 12b6ab6)
This is an automated email from the ASF dual-hosted git repository.
klueska pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from 7aaef19 Added MESOS-9231 in CHANGELOG and updated upgrades.md.
new 716bb9f Added the ability to launch tasks with a TTY attached to mesos-execute.
new 12b6ab6 Removed call to 'setsid()' in command executor if TTY attached.
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
src/cli/execute.cpp | 20 ++++++++++++++++++--
src/launcher/executor.cpp | 18 +++++++++++++++++-
src/slave/containerizer/mesos/io/switchboard.cpp | 7 +++++++
3 files changed, 42 insertions(+), 3 deletions(-)
[mesos] 01/02: Added the ability to launch tasks with a TTY
attached to mesos-execute.
Posted by kl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
klueska pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 716bb9f65974e4479d9ad794c145aa8293cea203
Author: Kevin Klues <kl...@gmail.com>
AuthorDate: Tue Oct 2 11:40:52 2018 +0200
Added the ability to launch tasks with a TTY attached to mesos-execute.
Review: https://reviews.apache.org/r/68724/
---
src/cli/execute.cpp | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/src/cli/execute.cpp b/src/cli/execute.cpp
index e0a8fc8..eb5c564 100644
--- a/src/cli/execute.cpp
+++ b/src/cli/execute.cpp
@@ -356,6 +356,11 @@ public:
"and 'protobuf' are valid choices.",
"protobuf");
+ add(&Flags::tty,
+ "tty",
+ "Attach a TTY to the task being launched",
+ false);
+
add(&Flags::partition_aware,
"partition_aware",
"Enable partition-awareness for the framework.",
@@ -390,6 +395,7 @@ public:
Option<string> secret;
string content_type;
bool partition_aware;
+ bool tty;
};
@@ -787,7 +793,8 @@ static Result<ContainerInfo> getContainerInfo(
const Option<string>& dockerImage,
const Option<CapabilityInfo>& effective_capabilities,
const Option<CapabilityInfo>& bounding_capabilities,
- const Option<RLimitInfo>& rlimits)
+ const Option<RLimitInfo>& rlimits,
+ const bool tty)
{
if (containerizer.empty()) {
return None();
@@ -870,6 +877,10 @@ static Result<ContainerInfo> getContainerInfo(
containerInfo.mutable_rlimit_info()->CopyFrom(rlimits.get());
}
+ if (tty) {
+ containerInfo.mutable_tty_info();
+ }
+
return containerInfo;
} else if (containerizer == "docker") {
// 'docker' containerizer only supports 'docker' images.
@@ -892,6 +903,10 @@ static Result<ContainerInfo> getContainerInfo(
}
}
+ if (tty) {
+ return Error("'Docker' containerizer does not allow attaching a TTY");
+ }
+
return containerInfo;
}
@@ -1190,7 +1205,8 @@ int main(int argc, char** argv)
dockerImage,
flags.effective_capabilities,
flags.bounding_capabilities,
- flags.rlimits);
+ flags.rlimits,
+ flags.tty);
if (containerInfo.isError()){
EXIT(EXIT_FAILURE) << containerInfo.error();
[mesos] 02/02: Removed call to 'setsid()' in command executor if
TTY attached.
Posted by kl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
klueska pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 12b6ab6bdabfc37f73439ea55ef0a7f0ee680983
Author: Kevin Klues <kl...@gmail.com>
AuthorDate: Tue Oct 2 11:41:12 2018 +0200
Removed call to 'setsid()' in command executor if TTY attached.
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.
Review: https://reviews.apache.org/r/68810/
---
src/launcher/executor.cpp | 18 +++++++++++++++++-
src/slave/containerizer/mesos/io/switchboard.cpp | 7 +++++++
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/src/launcher/executor.cpp b/src/launcher/executor.cpp
index 86fb87d..6b1204d 100644
--- a/src/launcher/executor.cpp
+++ b/src/launcher/executor.cpp
@@ -133,6 +133,7 @@ public:
const Option<Environment>& _taskEnvironment,
const Option<CapabilityInfo>& _effectiveCapabilities,
const Option<CapabilityInfo>& _boundingCapabilities,
+ const Option<string>& _ttySlavePath,
const Option<ContainerLaunchInfo>& _taskLaunchInfo,
const FrameworkID& _frameworkId,
const ExecutorID& _executorId,
@@ -158,6 +159,7 @@ public:
taskEnvironment(_taskEnvironment),
effectiveCapabilities(_effectiveCapabilities),
boundingCapabilities(_boundingCapabilities),
+ ttySlavePath(_ttySlavePath),
taskLaunchInfo(_taskLaunchInfo),
frameworkId(_frameworkId),
executorId(_executorId),
@@ -411,6 +413,7 @@ protected:
const Option<string>& workingDirectory,
const Option<CapabilityInfo>& effectiveCapabilities,
const Option<CapabilityInfo>& boundingCapabilities,
+ const Option<string>& ttySlavePath,
const Option<ContainerLaunchInfo>& taskLaunchInfo)
{
// Prepare the flags to pass to the launch process.
@@ -513,6 +516,11 @@ protected:
[](pid_t pid) { return os::set_job_kill_on_close_limit(pid); }));
#endif // __WINDOWS__
+ vector<process::Subprocess::ChildHook> childHooks;
+ if (ttySlavePath.isNone()) {
+ childHooks.emplace_back(Subprocess::ChildHook::SETSID());
+ }
+
Try<Subprocess> s = subprocess(
path::join(launcherDir, MESOS_CONTAINERIZER),
argv,
@@ -523,7 +531,7 @@ protected:
None(),
None(),
parentHooks,
- {Subprocess::ChildHook::SETSID()});
+ childHooks);
if (s.isError()) {
ABORT("Failed to launch '" + commandString + "': " + s.error());
@@ -688,6 +696,7 @@ protected:
workingDirectory,
effectiveCapabilities,
boundingCapabilities,
+ ttySlavePath,
taskLaunchInfo);
LOG(INFO) << "Forked command at " << pid.get();
@@ -1218,6 +1227,7 @@ private:
Option<Environment> taskEnvironment;
Option<CapabilityInfo> effectiveCapabilities;
Option<CapabilityInfo> boundingCapabilities;
+ Option<string> ttySlavePath;
Option<ContainerLaunchInfo> taskLaunchInfo;
const FrameworkID frameworkId;
const ExecutorID executorId;
@@ -1282,6 +1292,10 @@ public:
"task_launch_info",
"The launch info to run the task.");
+ add(&Flags::tty_slave_path,
+ "tty_slave_path",
+ "A path to the slave end of the attached TTY if there is one.");
+
add(&Flags::launcher_dir,
"launcher_dir",
"Directory path of Mesos binaries.",
@@ -1299,6 +1313,7 @@ public:
Option<Environment> task_environment;
Option<mesos::CapabilityInfo> effective_capabilities;
Option<mesos::CapabilityInfo> bounding_capabilities;
+ Option<string> tty_slave_path;
Option<JSON::Object> task_launch_info;
string launcher_dir;
};
@@ -1390,6 +1405,7 @@ int main(int argc, char** argv)
flags.task_environment,
flags.effective_capabilities,
flags.bounding_capabilities,
+ flags.tty_slave_path,
task_launch_info,
frameworkId,
executorId,
diff --git a/src/slave/containerizer/mesos/io/switchboard.cpp b/src/slave/containerizer/mesos/io/switchboard.cpp
index 498c008..e96504d 100644
--- a/src/slave/containerizer/mesos/io/switchboard.cpp
+++ b/src/slave/containerizer/mesos/io/switchboard.cpp
@@ -443,6 +443,13 @@ Future<Option<ContainerLaunchInfo>> IOSwitchboard::_prepare(
containerIO.err = containerIO.in;
launchInfo.set_tty_slave_path(slavePath.get());
+
+ // The command executor requires the `tty_slave_path`
+ // to also be passed as a command line argument.
+ if (containerConfig.has_task_info()) {
+ launchInfo.mutable_command()->add_arguments(
+ "--tty_slave_path=" + slavePath.get());
+ }
} else {
Try<std::array<int_fd, 2>> infds_ = os::pipe();
if (infds_.isError()) {