You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2018/08/20 23:34:41 UTC
[mesos] 01/03: Changed default executor tests to not use pipes for
synchronization.
This is an automated email from the ASF dual-hosted git repository.
jieyu pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 7029d511f618a4c3d7ef71a2adf76540d593a215
Author: Benjamin Bannier <be...@mesosphere.io>
AuthorDate: Mon Jun 25 20:08:43 2018 +0200
Changed default executor tests to not use pipes for synchronization.
Some tests of nested container functionality used pipes passed to
launched tasks to detect whether a task has actually started executing
the workload (`TASK_RUNNING` updates might be sent before the task
workload is actually started).
Once we avoid leaking unspecified file descriptors into forked
processes, this test setup will be broken. In this patch we replace
the use of pipes for synchronization with HTTP requests to an actor
running in the tests, or wait on other observable side effects.
Review: https://reviews.apache.org/r/67398/
(cherry picked from commit a666047c9324a0b24b26fa8d89b3fdb73537f44f)
---
src/tests/check_tests.cpp | 41 ++----
.../nested_mesos_containerizer_tests.cpp | 156 ++++++---------------
2 files changed, 54 insertions(+), 143 deletions(-)
diff --git a/src/tests/check_tests.cpp b/src/tests/check_tests.cpp
index d48febf..73ea5a9 100644
--- a/src/tests/check_tests.cpp
+++ b/src/tests/check_tests.cpp
@@ -1971,47 +1971,22 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(
.WillOnce(FutureArg<1>(&updateCheckResult))
.WillRepeatedly(Return()); // Ignore subsequent updates.
- // Default executor delegates launching both the task and its check to the
- // agent. To avoid a race, we explicitly synchronize.
- Try<std::array<int_fd, 2>> pipes_ = os::pipe();
- ASSERT_SOME(pipes_);
-
- const std::array<int_fd, 2>& pipes = pipes_.get();
-
const string filename = "nested_inherits_work_dir";
- // NOTE: We use a non-shell command here to use 'bash -c' to execute
- // the 'echo', which deals with the file descriptor, because of a bug
- // in ubuntu dash. Multi-digit file descriptor is not supported in
- // ubuntu dash, which executes the shell command.
- v1::CommandInfo command;
- command.set_shell(false);
- command.set_value("/bin/bash");
- command.add_arguments("bash");
- command.add_arguments("-c");
- command.add_arguments(
- "touch " + filename + ";echo running >&" +
- stringify(pipes[1]) + ";sleep 1000");
-
- v1::TaskInfo taskInfo = v1::createTask(agentId, resources, command);
+ v1::TaskInfo taskInfo = v1::createTask(
+ agentId,
+ resources,
+ v1::createCommandInfo(
+ strings::format("touch %s; sleep 1000", filename).get()));
v1::CheckInfo* checkInfo = taskInfo.mutable_check();
checkInfo->set_type(v1::CheckInfo::COMMAND);
checkInfo->set_delay_seconds(0);
checkInfo->set_interval_seconds(0);
- // NOTE: We use a non-shell command here to use 'bash -c' to execute
- // the 'read', which deals with the file descriptor, because of a bug
- // in ubuntu dash. Multi-digit file descriptor is not supported in
- // ubuntu dash, which executes the shell command.
- v1::CommandInfo* checkCommand =
- checkInfo->mutable_command()->mutable_command();
- checkCommand->set_shell(false);
- checkCommand->set_value("/bin/bash");
- checkCommand->add_arguments("bash");
- checkCommand->add_arguments("-c");
- checkCommand->add_arguments(
- "read INPUT <&" + stringify(pipes[0]) + ";ls " + filename);
+ // Wait in a busy loop until the file has been created.
+ checkInfo->mutable_command()->mutable_command()->CopyFrom(
+ v1::createCommandInfo("while [ -f " + filename + "]; do :; done"));
v1::TaskGroupInfo taskGroup;
taskGroup.add_tasks()->CopyFrom(taskInfo);
diff --git a/src/tests/containerizer/nested_mesos_containerizer_tests.cpp b/src/tests/containerizer/nested_mesos_containerizer_tests.cpp
index 92c282d..ba4e9e2 100644
--- a/src/tests/containerizer/nested_mesos_containerizer_tests.cpp
+++ b/src/tests/containerizer/nested_mesos_containerizer_tests.cpp
@@ -14,6 +14,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+#include <sys/stat.h>
#include <sys/wait.h>
#include <map>
@@ -452,23 +453,11 @@ TEST_F(NestedMesosContainerizerTest,
ContainerID containerId;
containerId.set_value(id::UUID::random().toString());
- // Use a pipe to pass parent's MESOS_SANDBOX value to a child container.
- Try<std::array<int_fd, 2>> pipes_ = os::pipe();
- ASSERT_SOME(pipes_);
-
- const std::array<int_fd, 2>& pipes = pipes_.get();
+ string pipe = path::join(sandbox.get(), "pipe");
+ ASSERT_EQ(0, ::mkfifo(pipe.c_str(), 0700));
- // NOTE: We use a non-shell command here to use 'bash -c' to execute
- // the 'echo', which deals with the file descriptor, because of a bug
- // in ubuntu dash. Multi-digit file descriptor is not supported in
- // ubuntu dash, which executes the shell command.
- CommandInfo command;
- command.set_shell(false);
- command.set_value("/bin/bash");
- command.add_arguments("bash");
- command.add_arguments("-c");
- command.add_arguments(
- "echo $MESOS_SANDBOX >&" + stringify(pipes[1]) + ";" + "sleep 1000");
+ CommandInfo command =
+ createCommandInfo("echo ${MESOS_SANDBOX} > " + pipe + " ; sleep 1000");
ExecutorInfo executor = createExecutorInfo("executor", command, "cpus:1");
@@ -486,30 +475,16 @@ TEST_F(NestedMesosContainerizerTest,
AWAIT_ASSERT_EQ(Containerizer::LaunchResult::SUCCESS, launch);
- // Wait for the parent container to start running its task
- // before launching a debug container inside it.
- AWAIT_READY(process::io::poll(pipes[0], process::io::READ));
- close(pipes[1]);
-
- // Launch a nested debug container that compares MESOS_SANDBOX
+ // Launch a nested debug container that compares `MESOS_SANDBOX`
// it sees with the one its parent sees.
{
ContainerID nestedContainerId;
nestedContainerId.mutable_parent()->CopyFrom(containerId);
nestedContainerId.set_value(id::UUID::random().toString());
- // NOTE: We use a non-shell command here to use 'bash -c' to execute
- // the 'read', which deals with the file descriptor, because of a bug
- // in ubuntu dash. Multi-digit file descriptor is not supported in
- // ubuntu dash, which executes the shell command.
- CommandInfo nestedCommand;
- nestedCommand.set_shell(false);
- nestedCommand.set_value("/bin/bash");
- nestedCommand.add_arguments("bash");
- nestedCommand.add_arguments("-c");
- nestedCommand.add_arguments(
- "read PARENT_SANDBOX <&" + stringify(pipes[0]) + ";"
- "[ ${PARENT_SANDBOX} == ${MESOS_SANDBOX} ] && exit 0 || exit 1;");
+ CommandInfo nestedCommand = createCommandInfo(
+ "read PARENT_SANDBOX < " + pipe + ";"
+ "[ ${PARENT_SANDBOX} = ${MESOS_SANDBOX} ] && exit 0 || exit 1;");
Future<Containerizer::LaunchResult> launchNested = containerizer->launch(
nestedContainerId,
@@ -529,8 +504,6 @@ TEST_F(NestedMesosContainerizerTest,
ASSERT_SOME(waitNested.get());
ASSERT_TRUE(waitNested.get()->has_status());
EXPECT_WEXITSTATUS_EQ(0, waitNested.get()->status());
-
- close(pipes[0]);
}
// Destroy the containerizer with all associated containers.
@@ -574,27 +547,15 @@ TEST_F(NestedMesosContainerizerTest,
containerId.set_value(id::UUID::random().toString());
// Use a pipe to synchronize with the top-level container.
- Try<std::array<int_fd, 2>> pipes_ = os::pipe();
- ASSERT_SOME(pipes_);
-
- const std::array<int_fd, 2>& pipes = pipes_.get();
+ string pipe = path::join(sandbox.get(), "pipe");
+ ASSERT_EQ(0, ::mkfifo(pipe.c_str(), 0700));
const string filename = "nested_inherits_work_dir";
- // NOTE: We use a non-shell command here to use 'bash -c' to execute
- // the 'echo', which deals with the file descriptor, because of a bug
- // in ubuntu dash. Multi-digit file descriptor is not supported in
- // ubuntu dash, which executes the shell command.
- CommandInfo command;
- command.set_shell(false);
- command.set_value("/bin/bash");
- command.add_arguments("bash");
- command.add_arguments("-c");
- command.add_arguments(
- "touch " + filename + ";echo running >&" +
- stringify(pipes[1]) + ";sleep 1000");
-
- ExecutorInfo executor = createExecutorInfo("executor", command, "cpus:1");
+ ExecutorInfo executor = createExecutorInfo(
+ "executor",
+ "touch " + filename + "; echo running > " + pipe + "; sleep 1000",
+ "cpus:1");
Try<string> directory = environment->mkdtemp();
ASSERT_SOME(directory);
@@ -617,9 +578,9 @@ TEST_F(NestedMesosContainerizerTest,
// Wait for the parent container to start running its task
// before launching a debug container inside it.
- AWAIT_READY(process::io::poll(pipes[0], process::io::READ));
- close(pipes[1]);
- close(pipes[0]);
+ Result<string> read = os::read(pipe);
+ ASSERT_SOME(read);
+ ASSERT_EQ("running\n", read.get());
Future<ContainerStatus> status = containerizer->status(containerId);
AWAIT_READY(status);
@@ -1019,19 +980,17 @@ TEST_F(NestedMesosContainerizerTest,
ASSERT_EQ(1u, offers->size());
// Use a pipe to synchronize with the top-level container.
- Try<std::array<int_fd, 2>> pipes_ = os::pipe();
- ASSERT_SOME(pipes_);
-
- const std::array<int_fd, 2>& pipes = pipes_.get();
+ string pipe = path::join(sandbox.get(), "pipe");
+ ASSERT_EQ(0, ::mkfifo(pipe.c_str(), 0700));
- // Launch a command task within the `alpine` docker image and
- // synchronize its launch with the launch of a debug container below.
+ // Launch a command task within the `alpine` docker image.
TaskInfo task = createTask(
offers->front().slave_id(),
offers->front().resources(),
- "echo running >&" + stringify(pipes[1]) + ";" + "sleep 1000");
+ "echo running > /tmp/pipe; sleep 1000");
- task.mutable_container()->CopyFrom(createContainerInfo("alpine"));
+ task.mutable_container()->CopyFrom(createContainerInfo(
+ "alpine", {createVolumeHostPath("/tmp", sandbox.get(), Volume::RW)}));
Future<TaskStatus> statusStarting;
Future<TaskStatus> statusRunning;
@@ -1048,12 +1007,11 @@ TEST_F(NestedMesosContainerizerTest,
AWAIT_READY_FOR(statusRunning, Seconds(120));
ASSERT_EQ(TASK_RUNNING, statusRunning->state());
- close(pipes[1]);
-
// Wait for the parent container to start running its task
// before launching a debug container inside it.
- AWAIT_READY(process::io::poll(pipes[0], process::io::READ));
- close(pipes[0]);
+ Result<string> read = os::read(pipe);
+ ASSERT_SOME(read);
+ ASSERT_EQ("running\n", read.get());
ASSERT_TRUE(statusRunning->has_slave_id());
ASSERT_TRUE(statusRunning->has_container_status());
@@ -1499,23 +1457,14 @@ TEST_F(NestedMesosContainerizerTest, ROOT_CGROUPS_ParentExit)
ContainerID containerId;
containerId.set_value(id::UUID::random().toString());
- Try<std::array<int_fd, 2>> pipes_ = os::pipe();
- ASSERT_SOME(pipes_);
-
- const std::array<int_fd, 2>& pipes = pipes_.get();
-
- // NOTE: We use a non-shell command here to use 'bash -c' to execute
- // the 'read', which deals with the file descriptor, because of a bug
- // in ubuntu dash. Multi-digit file descriptor is not supported in
- // ubuntu dash, which executes the shell command.
- CommandInfo command;
- command.set_shell(false);
- command.set_value("/bin/bash");
- command.add_arguments("bash");
- command.add_arguments("-c");
- command.add_arguments("read key <&" + stringify(pipes[0]));
+ string pipe = path::join(sandbox.get(), "pipe");
+ ASSERT_EQ(0, ::mkfifo(pipe.c_str(), 0700));
- ExecutorInfo executor = createExecutorInfo("executor", command, "cpus:1");
+ // We launch a blocking `read` after which we return with a non-success code.
+ ExecutorInfo executor = createExecutorInfo(
+ "executor",
+ createCommandInfo("read < " + pipe + " && exit 1"),
+ "cpus:1");
Try<string> directory = environment->mkdtemp();
ASSERT_SOME(directory);
@@ -1526,8 +1475,6 @@ TEST_F(NestedMesosContainerizerTest, ROOT_CGROUPS_ParentExit)
map<string, string>(),
None());
- close(pipes[0]); // We're never going to read.
-
AWAIT_ASSERT_EQ(Containerizer::LaunchResult::SUCCESS, launch);
// Now launch nested container.
@@ -1548,7 +1495,8 @@ TEST_F(NestedMesosContainerizerTest, ROOT_CGROUPS_ParentExit)
Future<Option<ContainerTermination>> nestedWait = containerizer->wait(
nestedContainerId);
- close(pipes[1]); // Force the 'read key' to exit!
+ // Write to the fifo to unblock the `read` in the parent container.
+ os::write(pipe, "");
AWAIT_READY(wait);
ASSERT_SOME(wait.get());
@@ -1593,25 +1541,14 @@ TEST_F(NestedMesosContainerizerTest, ROOT_CGROUPS_ParentSigterm)
ContainerID containerId;
containerId.set_value(id::UUID::random().toString());
- // Use a pipe to synchronize with the top-level container.
- Try<std::array<int_fd, 2>> pipes_ = os::pipe();
- ASSERT_SOME(pipes_);
-
- const std::array<int_fd, 2>& pipes = pipes_.get();
-
- // NOTE: We use a non-shell command here to use 'bash -c' to execute
- // the 'echo', which deals with the file descriptor, because of a bug
- // in ubuntu dash. Multi-digit file descriptor is not supported in
- // ubuntu dash, which executes the shell command.
- CommandInfo command;
- command.set_shell(false);
- command.set_value("/bin/bash");
- command.add_arguments("bash");
- command.add_arguments("-c");
- command.add_arguments(
- "echo running >&" + stringify(pipes[1]) + ";" + "sleep 1000");
+ // Use a fifo to synchronize with the top-level container.
+ string pipe = path::join(sandbox.get(), "pipe");
+ ASSERT_EQ(0, ::mkfifo(pipe.c_str(), 0700));
- ExecutorInfo executor = createExecutorInfo("executor", command, "cpus:1");
+ ExecutorInfo executor = createExecutorInfo(
+ "executor",
+ createCommandInfo("echo running > " + pipe + "; sleep 1000"),
+ "cpus:1");
Try<string> directory = environment->mkdtemp();
ASSERT_SOME(directory);
@@ -1624,8 +1561,6 @@ TEST_F(NestedMesosContainerizerTest, ROOT_CGROUPS_ParentSigterm)
AWAIT_ASSERT_EQ(Containerizer::LaunchResult::SUCCESS, launch);
- close(pipes[1]);
-
// Now launch nested container.
ContainerID nestedContainerId;
nestedContainerId.mutable_parent()->CopyFrom(containerId);
@@ -1650,8 +1585,9 @@ TEST_F(NestedMesosContainerizerTest, ROOT_CGROUPS_ParentSigterm)
// Wait for the parent container to start running its executor
// process before sending it a signal.
- AWAIT_READY(process::io::poll(pipes[0], process::io::READ));
- close(pipes[0]);
+ Result<string> read = os::read(pipe);
+ ASSERT_SOME(read);
+ ASSERT_EQ("running\n", read.get());
ASSERT_EQ(0, os::kill(status->executor_pid(), SIGTERM));