This is an automated email from the ASF dual-hosted git repository.
gilbert pushed a commit to branch 1.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit d23b711f7511b58010d2b0f3a43e67e46e054737
Author: Benjamin Bannier <benjamin.bann...@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 0810851..bb0b3e8 100644
--- a/src/tests/check_tests.cpp
+++ b/src/tests/check_tests.cpp
@@ -1889,47 +1889,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 f8b4423..c05c916 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>
@@ -449,23 +450,11 @@ TEST_F(NestedMesosContainerizerTest,
ContainerID containerId;
containerId.set_value(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_);
+ string pipe = path::join(sandbox.get(), "pipe");
+ ASSERT_EQ(0, ::mkfifo(pipe.c_str(), 0700));
- 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 $MESOS_SANDBOX >&" + stringify(pipes[1]) + ";" + "sleep 1000");
+ CommandInfo command =
+ createCommandInfo("echo ${MESOS_SANDBOX} > " + pipe + " ; sleep 1000");
ExecutorInfo executor = createExecutorInfo("executor", command, "cpus:1");
@@ -483,30 +472,16 @@ TEST_F(NestedMesosContainerizerTest,
AWAIT_ASSERT_TRUE(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(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<bool> launchNested = containerizer->launch(
nestedContainerId,
@@ -526,8 +501,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.
@@ -571,27 +544,15 @@ TEST_F(NestedMesosContainerizerTest,
containerId.set_value(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);
@@ -614,9 +575,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);
@@ -1016,19 +977,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", {createVolumeFromHostPath("/tmp", sandbox.get(),
Volume::RW)}));
Future<TaskStatus> statusRunning;
EXPECT_CALL(sched, statusUpdate(_, _))
@@ -1041,12 +1000,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());
@@ -1385,23 +1343,14 @@ TEST_F(NestedMesosContainerizerTest,
ROOT_CGROUPS_ParentExit)
ContainerID containerId;
containerId.set_value(UUID::random().toString());
- Try<std::array<int_fd, 2>> pipes_ = os::pipe();
- ASSERT_SOME(pipes_);
+ string pipe = path::join(sandbox.get(), "pipe");
+ ASSERT_EQ(0, ::mkfifo(pipe.c_str(), 0700));
- 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]));
-
- 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);
@@ -1412,8 +1361,6 @@ TEST_F(NestedMesosContainerizerTest,
ROOT_CGROUPS_ParentExit)
map<string, string>(),
None());
- close(pipes[0]); // We're never going to read.
-
AWAIT_ASSERT_TRUE(launch);
// Now launch nested container.
@@ -1434,7 +1381,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());
@@ -1479,25 +1427,14 @@ TEST_F(NestedMesosContainerizerTest,
ROOT_CGROUPS_ParentSigterm)
ContainerID containerId;
containerId.set_value(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);
@@ -1510,8 +1447,6 @@ TEST_F(NestedMesosContainerizerTest,
ROOT_CGROUPS_ParentSigterm)
AWAIT_ASSERT_TRUE(launch);
- close(pipes[1]);
-
// Now launch nested container.
ContainerID nestedContainerId;
nestedContainerId.mutable_parent()->CopyFrom(containerId);
@@ -1536,8 +1471,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(0u, os::kill(status->executor_pid(), SIGTERM));
