You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by vi...@apache.org on 2016/12/12 04:58:30 UTC
[1/2] mesos git commit: Fixed EOF bug when tty is enabled in switch
board server.
Repository: mesos
Updated Branches:
refs/heads/master 26e3361a0 -> 370fb4338
Fixed EOF bug when tty is enabled in switch board server.
stdin fd should only be closed when a tty is not attached.
Review: https://reviews.apache.org/r/54637
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/370fb433
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/370fb433
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/370fb433
Branch: refs/heads/master
Commit: 370fb43380c49aa9485fd283cd2be7d31c392c6d
Parents: a660ce2
Author: Vinod Kone <vi...@gmail.com>
Authored: Sat Dec 10 20:44:52 2016 -0800
Committer: Vinod Kone <vi...@gmail.com>
Committed: Sun Dec 11 20:58:08 2016 -0800
----------------------------------------------------------------------
src/slave/containerizer/mesos/io/switchboard.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/370fb433/src/slave/containerizer/mesos/io/switchboard.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/io/switchboard.cpp b/src/slave/containerizer/mesos/io/switchboard.cpp
index 1c73ffb..8b5c821 100644
--- a/src/slave/containerizer/mesos/io/switchboard.cpp
+++ b/src/slave/containerizer/mesos/io/switchboard.cpp
@@ -1465,8 +1465,9 @@ Future<http::Response> IOSwitchboardServerProcess::attachContainerInput(
}
case agent::ProcessIO::DATA: {
// Receiving a `DATA` message with length 0 indicates
- // `EOF` so we should close `stdinToFd`.
- if (message.data().data().length() == 0) {
+ // `EOF`, so we should close `stdinToFd` if there is no tty.
+ // If tty is enabled, the client is expected to send `EOT` instead.
+ if (!tty && message.data().data().length() == 0) {
os::close(stdinToFd);
return true;
}
[2/2] mesos git commit: Removed "enable_io_switchboard_server" agent
flag.
Posted by vi...@apache.org.
Removed "enable_io_switchboard_server" agent flag.
Instead of an agent wide flag now we automatically launch the server
for DEBUG and TTY containers.
Review: https://reviews.apache.org/r/54620
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/a660ce25
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/a660ce25
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/a660ce25
Branch: refs/heads/master
Commit: a660ce253ae2aa9e6e69be105ac7651442716c4f
Parents: 26e3361
Author: Vinod Kone <vi...@gmail.com>
Authored: Fri Dec 9 19:15:10 2016 -0800
Committer: Vinod Kone <vi...@gmail.com>
Committed: Sun Dec 11 20:58:08 2016 -0800
----------------------------------------------------------------------
.../containerizer/mesos/io/switchboard.cpp | 21 +++++----------
.../containerizer/mesos/io/switchboard.hpp | 19 +++++++++++++
src/slave/flags.cpp | 11 --------
src/slave/flags.hpp | 1 -
src/tests/api_tests.cpp | 28 +++++++++++++-------
.../containerizer/io_switchboard_tests.cpp | 18 ++++++++-----
src/tests/mesos.cpp | 2 --
7 files changed, 55 insertions(+), 45 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/a660ce25/src/slave/containerizer/mesos/io/switchboard.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/io/switchboard.cpp b/src/slave/containerizer/mesos/io/switchboard.cpp
index a354188..1c73ffb 100644
--- a/src/slave/containerizer/mesos/io/switchboard.cpp
+++ b/src/slave/containerizer/mesos/io/switchboard.cpp
@@ -164,8 +164,8 @@ Future<Nothing> IOSwitchboard::recover(
// If we don't have a checkpoint directory created for this
// container's io switchboard, there is nothing to recover. This
- // can only happen for legacy containers, containers that were
- // launched with `--io_switchboard_enable_server=false`.
+ // can only happen for containers that were launched with `DEFAULT`
+ // ContainerClass *and* no `TTYInfo` set.
if (!os::exists(path)) {
continue;
}
@@ -285,27 +285,18 @@ Future<Option<ContainerLaunchInfo>> IOSwitchboard::_prepare(
const ContainerLogger::SubprocessInfo& loggerInfo)
{
// On windows, we do not yet support running an io switchboard
- // server, so we must error out if the agent
- // `io_switchboard_enable_server` flag is enabled.
+ // server, so we must error out if it is required.
#ifdef __WINDOWS__
- if (flags.io_switchboard_enable_server) {
+ if (IOSwitchboardServer::isRequired(containerConfig)) {
return Failure(
- "Setting the agent flag"
- " '--io_switchboard_enable_server=true'"
- " is not supported on windows");
+ "IO Switchboard server is not supported on windows");
}
#endif
bool hasTTY = containerConfig.has_container_info() &&
containerConfig.container_info().has_tty_info();
- if (!flags.io_switchboard_enable_server) {
- // TTY support requires I/O switchboard server so that stdio can
- // be properly redirected to logger.
- if (hasTTY) {
- return Failure("TTY support requires I/O switchboard server");
- }
-
+ if (!IOSwitchboardServer::isRequired(containerConfig)) {
ContainerLaunchInfo launchInfo;
ContainerIO* out = launchInfo.mutable_out();
http://git-wip-us.apache.org/repos/asf/mesos/blob/a660ce25/src/slave/containerizer/mesos/io/switchboard.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/io/switchboard.hpp b/src/slave/containerizer/mesos/io/switchboard.hpp
index ae4c48d..fb720f0 100644
--- a/src/slave/containerizer/mesos/io/switchboard.hpp
+++ b/src/slave/containerizer/mesos/io/switchboard.hpp
@@ -149,6 +149,25 @@ public:
// has been started with `waitForConnection` set to `true`.
process::Future<Nothing> unblock();
+ // Helper function that returns `true` if `IOSwitchboardServer`
+ // needs to be enabled for the given `ContainerConfig`. It must
+ // be enabled for `DEBUG` containers and ones that need `TTYInfo`.
+ static bool isRequired(const mesos::slave::ContainerConfig& containerConfig)
+ {
+ if (containerConfig.has_container_info() &&
+ containerConfig.container_info().has_tty_info()) {
+ return true;
+ }
+
+ if (containerConfig.has_container_class() &&
+ containerConfig.container_class() ==
+ mesos::slave::ContainerClass::DEBUG) {
+ return true;
+ }
+
+ return false;
+ }
+
private:
IOSwitchboardServer(
bool tty,
http://git-wip-us.apache.org/repos/asf/mesos/blob/a660ce25/src/slave/flags.cpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.cpp b/src/slave/flags.cpp
index 74a6c99..a5f5203 100644
--- a/src/slave/flags.cpp
+++ b/src/slave/flags.cpp
@@ -262,17 +262,6 @@ mesos::internal::slave::Flags::Flags()
"NOTE: This feature is not yet supported on Windows agent, and\n"
"therefore the flag currently does not exist on that platform.",
true);
-
- add(&Flags::io_switchboard_enable_server,
- "io_switchboard_enable_server",
- "If set to `true`, the agent will launch a per-container sidecar\n"
- "process that runs an HTTP serve to handle incoming\n"
- "'ATTACH_CONTAINER_INPUT' and 'ATTACH_CONTAINER_OUTPUT' calls on\n"
- "behalf of a container. If set to 'false', this functionality\n"
- "will not be available. The default is 'false'.\n"
- "NOTE: This feature is not yet supported on Windows agent, and\n"
- "therefore the flag currently does not exist on that platform.",
- false);
#endif // __WINDOWS__
add(&Flags::frameworks_home,
http://git-wip-us.apache.org/repos/asf/mesos/blob/a660ce25/src/slave/flags.hpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp
index 02af284..3c292ba 100644
--- a/src/slave/flags.hpp
+++ b/src/slave/flags.hpp
@@ -70,7 +70,6 @@ public:
#ifndef __WINDOWS__
bool switch_user;
#endif // __WINDOWS__
- bool io_switchboard_enable_server;
std::string frameworks_home; // TODO(benh): Make an Option.
Duration registration_backoff_factor;
Duration authentication_backoff_factor;
http://git-wip-us.apache.org/repos/asf/mesos/blob/a660ce25/src/tests/api_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/api_tests.cpp b/src/tests/api_tests.cpp
index 2ae6d11..82c0fc2 100644
--- a/src/tests/api_tests.cpp
+++ b/src/tests/api_tests.cpp
@@ -4324,8 +4324,8 @@ INSTANTIATE_TEST_CASE_P(
ContentType::STREAMING_PROTOBUF, ContentType::STREAMING_JSON));
-// This test launches a child container with the 'cat' command as the
-// entrypoint and attaches to its STDOUT via the attach output call.
+// This test launches a child container with TTY and the 'cat' command
+// as the entrypoint and attaches to its STDOUT via the attach output call.
// It then verifies that any data streamed to the container via the
// attach input call is received by the client on the output stream.
TEST_P(AgentAPIStreamingTest, AttachContainerInput)
@@ -4385,7 +4385,7 @@ TEST_P(AgentAPIStreamingTest, AttachContainerInput)
containerId.set_value(UUID::random().toString());
containerId.mutable_parent()->set_value(containerIds->begin()->value());
- // Launch the child container and then attach to it's output.
+ // Launch the child container with TTY and then attach to it's output.
{
v1::agent::Call call;
@@ -4397,6 +4397,12 @@ TEST_P(AgentAPIStreamingTest, AttachContainerInput)
call.mutable_launch_nested_container()->mutable_command()
->CopyFrom(v1::createCommandInfo("cat"));
+ call.mutable_launch_nested_container()->mutable_container()
+ ->set_type(mesos::v1::ContainerInfo::MESOS);
+
+ call.mutable_launch_nested_container()->mutable_container()
+ ->mutable_tty_info();
+
Future<http::Response> response = http::post(
slave.get()->pid,
"api/v1",
@@ -4441,11 +4447,10 @@ TEST_P(AgentAPIStreamingTest, AttachContainerInput)
"aliquip ex ea commodo consequat. Duis aute irure dolor in "
"reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla "
"pariatur. Excepteur sint occaecat cupidatat non proident, sunt in "
- "culpa qui officia deserunt mollit anim id est laborum.";
+ "culpa qui officia deserunt mollit anim id est laborum.\n";
- while (Bytes(data.size()) < Megabytes(1)) {
- data.append(data);
- }
+ // Terminal transforms "\n" to "\r\n".
+ string stdoutExpected = strings::trim(data) + "\r\n";
http::Pipe pipe;
http::Pipe::Writer writer = pipe.writer();
@@ -4492,7 +4497,7 @@ TEST_P(AgentAPIStreamingTest, AttachContainerInput)
writer.write(encoder.encode(call));
}
- // Signal `EOF` to the 'cat' command.
+ // Signal `EOT` to the terminal so that it sends `EOF` to `cat` command.
{
v1::agent::Call call;
call.set_type(v1::agent::Call::ATTACH_CONTAINER_INPUT);
@@ -4505,7 +4510,7 @@ TEST_P(AgentAPIStreamingTest, AttachContainerInput)
v1::agent::ProcessIO* processIO = attach->mutable_process_io();
processIO->set_type(v1::agent::ProcessIO::DATA);
processIO->mutable_data()->set_type(v1::agent::ProcessIO::Data::STDIN);
- processIO->mutable_data()->set_data("");
+ processIO->mutable_data()->set_data("\x04");
writer.write(encoder.encode(call));
}
@@ -4553,8 +4558,11 @@ TEST_P(AgentAPIStreamingTest, AttachContainerInput)
tie(stdoutReceived, stderrReceived) = received.get();
+ // `stdoutExpected` appears twice in stdout because the terminal in raw mode
+ // echoes the data once and `cat` outputs it once.
+ ASSERT_EQ(stdoutExpected + stdoutExpected, stdoutReceived);
+
ASSERT_TRUE(stderrReceived.empty());
- ASSERT_EQ(data, stdoutReceived);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/a660ce25/src/tests/containerizer/io_switchboard_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/io_switchboard_tests.cpp b/src/tests/containerizer/io_switchboard_tests.cpp
index ecadeee..2f3a1d7 100644
--- a/src/tests/containerizer/io_switchboard_tests.cpp
+++ b/src/tests/containerizer/io_switchboard_tests.cpp
@@ -464,7 +464,6 @@ TEST_F(IOSwitchboardTest, ContainerAttach)
slave::Flags flags = CreateSlaveFlags();
flags.launcher = "posix";
flags.isolation = "posix/cpu";
- flags.io_switchboard_enable_server = true;
Fetcher fetcher;
@@ -493,6 +492,10 @@ TEST_F(IOSwitchboardTest, ContainerAttach)
"sleep 1000",
"cpus:1");
+ // Request a tty for the container to enable attaching.
+ executorInfo.mutable_container()->set_type(ContainerInfo::MESOS);
+ executorInfo.mutable_container()->mutable_tty_info();
+
Future<bool> launch = containerizer->launch(
containerId,
None(),
@@ -527,7 +530,6 @@ TEST_F(IOSwitchboardTest, OutputRedirectionWithTTY)
slave::Flags flags = CreateSlaveFlags();
flags.launcher = "posix";
flags.isolation = "posix/cpu";
- flags.io_switchboard_enable_server = true;
Fetcher fetcher;
@@ -593,7 +595,6 @@ TEST_F(IOSwitchboardTest, KillSwitchboardContainerDestroyed)
slave::Flags flags = CreateSlaveFlags();
flags.launcher = "posix";
flags.isolation = "posix/cpu";
- flags.io_switchboard_enable_server = true;
Fetcher fetcher;
@@ -622,6 +623,10 @@ TEST_F(IOSwitchboardTest, KillSwitchboardContainerDestroyed)
"sleep 1000",
"cpus:1");
+ // Request a tty for the container to start the switchboard server.
+ executorInfo.mutable_container()->set_type(ContainerInfo::MESOS);
+ executorInfo.mutable_container()->mutable_tty_info();
+
Future<bool> launch = containerizer->launch(
containerId,
None(),
@@ -664,7 +669,6 @@ TEST_F(IOSwitchboardTest, RecoverThenKillSwitchboardContainerDestroyed)
slave::Flags flags = CreateSlaveFlags();
flags.launcher = "posix";
flags.isolation = "posix/cpu";
- flags.io_switchboard_enable_server = true;
Fetcher fetcher;
@@ -707,11 +711,13 @@ TEST_F(IOSwitchboardTest, RecoverThenKillSwitchboardContainerDestroyed)
AWAIT_READY(offers);
EXPECT_NE(0u, offers.get().size());
- // Launch a task.
+ // Launch a task with tty to start the switchboard server.
TaskInfo task = createTask(offers.get()[0], "sleep 1000");
+ task.mutable_container()->set_type(ContainerInfo::MESOS);
+ task.mutable_container()->mutable_tty_info();
// Drop the status update from the slave to the master so the
- // scheduler never recieves the first task update.
+ // scheduler never receives the first task update.
Future<StatusUpdateMessage> update =
DROP_PROTOBUF(StatusUpdateMessage(), slave.get()->pid, master.get()->pid);
http://git-wip-us.apache.org/repos/asf/mesos/blob/a660ce25/src/tests/mesos.cpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp
index 73f17a7..629872f 100644
--- a/src/tests/mesos.cpp
+++ b/src/tests/mesos.cpp
@@ -244,8 +244,6 @@ slave::Flags MesosTest::CreateSlaveFlags()
flags.isolation = tests::flags.isolation.get();
}
- flags.io_switchboard_enable_server = true;
-
return flags;
}