You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jo...@apache.org on 2016/03/28 18:19:12 UTC

[6/7] mesos git commit: Subprocess: [6/7] Refactored isolator tests to use parentHook.

Subprocess: [6/7] Refactored isolator tests to use parentHook.

The isolator tests parent process isolates the child while the child
is being blocked. This this the exact patter of a parentHook.

Review: https://reviews.apache.org/r/45236/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/0e2895c7
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/0e2895c7
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/0e2895c7

Branch: refs/heads/master
Commit: 0e2895c744ea2ae19ae72c2597862efefe0c2479
Parents: 85f6145
Author: Joerg Schad <jo...@mesosphere.io>
Authored: Mon Mar 28 17:02:23 2016 +0200
Committer: Joris Van Remoortere <jo...@gmail.com>
Committed: Mon Mar 28 18:17:21 2016 +0200

----------------------------------------------------------------------
 src/tests/containerizer/isolator_tests.cpp | 261 +++++++++++++++---------
 1 file changed, 161 insertions(+), 100 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/0e2895c7/src/tests/containerizer/isolator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/isolator_tests.cpp b/src/tests/containerizer/isolator_tests.cpp
index 47fbd39..4dde729 100644
--- a/src/tests/containerizer/isolator_tests.cpp
+++ b/src/tests/containerizer/isolator_tests.cpp
@@ -16,6 +16,7 @@
 
 #include <unistd.h>
 
+#include <functional>
 #include <iostream>
 #include <string>
 #include <vector>
@@ -34,6 +35,7 @@
 #include <process/reap.hpp>
 
 #include <stout/abort.hpp>
+#include <stout/duration.hpp>
 #include <stout/gtest.hpp>
 #include <stout/hashset.hpp>
 #include <stout/os.hpp>
@@ -113,6 +115,81 @@ namespace mesos {
 namespace internal {
 namespace tests {
 
+// Flag describing whether a the isolatePid parentHook should check the cgroups
+// before and after isolation.
+enum CheckCgroups
+{
+  CHECK_CGROUPS,
+  NO_CHECK_CGROUPS,
+};
+
+// A hook that is executed in the parent process. It attempts to isolate
+// a process and can optionally check the cgroups before and after isolation.
+//
+// NOTE: The child process is blocked by the hook infrastructure while
+// these hooks are executed.
+// NOTE: Returning an Error implies the child process will be killed.
+Try<Nothing> isolatePid(
+    pid_t child,
+    const Owned<Isolator>& isolator,
+    const ContainerID& containerId,
+    const CheckCgroups checkCgroups = NO_CHECK_CGROUPS)
+{
+  if (checkCgroups == CHECK_CGROUPS) {
+    // Before isolation, the cgroup is empty.
+    Future<ResourceStatistics> usage = isolator->usage(containerId);
+
+    // Note this is following the implementation of AWAIT_READY.
+    if (!process::internal::await(usage, Seconds(15))) {
+      return Error("Could check cgroup usage");
+    }
+    if (usage.isDiscarded() || usage.isFailed()) {
+      return Error("Could check cgroup usage");
+    }
+
+    if (0U != usage.get().processes()) {
+      return Error("Cgroups processes not empty before isolation");
+    }
+    if (0U != usage.get().threads()) {
+      return Error("Cgroups threads not empty before isolation");
+    }
+  }
+
+  // Isolate process.
+  process::Future<Nothing> isolate = isolator->isolate(containerId, child);
+
+  // Note this is following the implementation of AWAIT_READY.
+  if (!process::internal::await(isolate, Seconds(15))) {
+    return Error("Could not isolate pid");
+  }
+  if (isolate.isDiscarded() || isolate.isFailed()) {
+    return Error("Could not isolate pid");
+  }
+
+  if (checkCgroups == CHECK_CGROUPS) {
+    // After isolation, there should be one process in the cgroup.
+    Future<ResourceStatistics> usage = isolator->usage(containerId);
+
+    // Note this is following the implementation of AWAIT_READY.
+    if (!process::internal::await(usage, Seconds(15))) {
+      return Error("Could check cgroup usage");
+    }
+    if (usage.isDiscarded() || usage.isFailed()) {
+      return Error("Could check cgroup usage");
+    }
+
+    if (1U != usage.get().processes()) {
+      return Error("Cgroups processes empty after isolation");
+    }
+    if (1U != usage.get().threads()) {
+      return Error("Cgroups threads empty after isolation");
+    }
+  }
+
+  return Nothing();
+}
+
+
 template <typename T>
 class CpuIsolatorTest : public MesosTest {};
 
@@ -127,7 +204,7 @@ typedef ::testing::Types<
 
 TYPED_TEST_CASE(CpuIsolatorTest, CpuIsolatorTypes);
 
-// TODO(joerg84): Move isolate call to parentHook tests.
+
 TYPED_TEST(CpuIsolatorTest, UserCpuUsage)
 {
   slave::Flags flags;
@@ -168,14 +245,26 @@ TYPED_TEST(CpuIsolatorTest, UserCpuUsage)
     "touch " + file + "; " // Signals the command is running.
     "sleep 60";
 
-  int pipes[2];
-  ASSERT_NE(-1, ::pipe(pipes));
-
   vector<string> argv(3);
   argv[0] = "sh";
   argv[1] = "-c";
   argv[2] = command;
 
+  vector<Subprocess::Hook> parentHooks;
+
+  // Create parent Hook to isolate child.
+  //
+  // NOTE: We can safely use references here as the hook will be executed
+  // during the call to `fork`.
+  const lambda::function<Try<Nothing>(pid_t)> isolatePidHook = lambda::bind(
+      isolatePid,
+      lambda::_1,
+      std::cref(isolator),
+      std::cref(containerId),
+      NO_CHECK_CGROUPS);
+
+  parentHooks.emplace_back(Subprocess::Hook(isolatePidHook));
+
   Try<pid_t> pid = launcher->fork(
       containerId,
       "sh",
@@ -183,29 +272,16 @@ TYPED_TEST(CpuIsolatorTest, UserCpuUsage)
       Subprocess::FD(STDIN_FILENO),
       Subprocess::FD(STDOUT_FILENO),
       Subprocess::FD(STDERR_FILENO),
-      false,
       None(),
       None(),
-      lambda::bind(&childSetup, pipes),
-      None());
+      None(),
+      parentHooks);
 
   ASSERT_SOME(pid);
 
   // Reap the forked child.
   Future<Option<int> > status = process::reap(pid.get());
 
-  // Continue in the parent.
-  ASSERT_SOME(os::close(pipes[0]));
-
-  // Isolate the forked child.
-  AWAIT_READY(isolator->isolate(containerId, pid.get()));
-
-  // Now signal the child to continue.
-  char dummy;
-  ASSERT_LT(0, ::write(pipes[1], &dummy, sizeof(dummy)));
-
-  ASSERT_SOME(os::close(pipes[1]));
-
   // Wait for the command to start.
   while (!os::exists(file));
 
@@ -282,14 +358,26 @@ TYPED_TEST(CpuIsolatorTest, SystemCpuUsage)
     "touch " + file + "; " // Signals the command is running.
     "sleep 60";
 
-  int pipes[2];
-  ASSERT_NE(-1, ::pipe(pipes));
-
   vector<string> argv(3);
   argv[0] = "sh";
   argv[1] = "-c";
   argv[2] = command;
 
+  vector<Subprocess::Hook> parentHooks;
+
+  // Create parent Hook to isolate child.
+  //
+  // NOTE: We can safely use references here as the hook will be executed
+  // during the call to `fork`.
+  const lambda::function<Try<Nothing>(pid_t)> isolatePidHook = lambda::bind(
+      isolatePid,
+      lambda::_1,
+      std::cref(isolator),
+      std::cref(containerId),
+      NO_CHECK_CGROUPS);
+
+  parentHooks.emplace_back(Subprocess::Hook(isolatePidHook));
+
   Try<pid_t> pid = launcher->fork(
       containerId,
       "sh",
@@ -299,26 +387,14 @@ TYPED_TEST(CpuIsolatorTest, SystemCpuUsage)
       Subprocess::FD(STDERR_FILENO),
       None(),
       None(),
-      lambda::bind(&childSetup, pipes),
-      None());
+      None(),
+      parentHooks);
 
   ASSERT_SOME(pid);
 
   // Reap the forked child.
   Future<Option<int> > status = process::reap(pid.get());
 
-  // Continue in the parent.
-  ASSERT_SOME(os::close(pipes[0]));
-
-  // Isolate the forked child.
-  AWAIT_READY(isolator->isolate(containerId, pid.get()));
-
-  // Now signal the child to continue.
-  char dummy;
-  ASSERT_LT(0, ::write(pipes[1], &dummy, sizeof(dummy)));
-
-  ASSERT_SOME(os::close(pipes[1]));
-
   // Wait for the command to start.
   while (!os::exists(file));
 
@@ -565,14 +641,26 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_CFS_Enable_Cfs)
     "sleep 0.5 && "
     "kill $MESOS_TEST_PID";
 
-  int pipes[2];
-  ASSERT_NE(-1, ::pipe(pipes));
-
   vector<string> argv(3);
   argv[0] = "sh";
   argv[1] = "-c";
   argv[2] = command;
 
+  vector<Subprocess::Hook> parentHooks;
+
+  // Create parent Hook to isolate child.
+  //
+  // NOTE: We can safely use references here as the hook will be executed
+  // during the call to `fork`.
+  const lambda::function<Try<Nothing>(pid_t)> isolatePidHook = lambda::bind(
+      isolatePid,
+      lambda::_1,
+      std::cref(isolator),
+      std::cref(containerId),
+      NO_CHECK_CGROUPS);
+
+  parentHooks.emplace_back(Subprocess::Hook(isolatePidHook));
+
   Try<pid_t> pid = launcher->fork(
       containerId,
       "sh",
@@ -582,26 +670,14 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_CFS_Enable_Cfs)
       Subprocess::FD(STDERR_FILENO),
       None(),
       None(),
-      lambda::bind(&childSetup, pipes),
-      prepare.get().isSome() ? prepare.get().get().namespaces() : 0);
+      prepare.get().isSome() ? prepare.get().get().namespaces() : 0,
+      parentHooks);
 
   ASSERT_SOME(pid);
 
   // Reap the forked child.
   Future<Option<int> > status = process::reap(pid.get());
 
-  // Continue in the parent.
-  ASSERT_SOME(os::close(pipes[0]));
-
-  // Isolate the forked child.
-  AWAIT_READY(isolator->isolate(containerId, pid.get()));
-
-  // Now signal the child to continue.
-  char dummy;
-  ASSERT_LT(0, ::write(pipes[1], &dummy, sizeof(dummy)));
-
-  ASSERT_SOME(os::close(pipes[1]));
-
   // Wait for the command to complete.
   AWAIT_READY(status);
 
@@ -671,14 +747,25 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_CFS_Big_Quota)
 
   AWAIT_READY(prepare);
 
-  int pipes[2];
-  ASSERT_NE(-1, ::pipe(pipes));
-
   vector<string> argv(3);
   argv[0] = "sh";
   argv[1] = "-c";
   argv[2] = "exit 0";
 
+  vector<Subprocess::Hook> parentHooks;
+
+  // Create parent Hook to isolate child.
+  // Note: We can safely use references here as we are sure that the life time
+  // of the parent will exceed the child.
+  const lambda::function<Try<Nothing>(pid_t)> isolatePidHook = lambda::bind(
+      isolatePid,
+      lambda::_1,
+      std::cref(isolator),
+      std::cref(containerId),
+      NO_CHECK_CGROUPS);
+
+  parentHooks.emplace_back(Subprocess::Hook(isolatePidHook));
+
   Try<pid_t> pid = launcher->fork(
       containerId,
       "sh",
@@ -688,26 +775,14 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_CFS_Big_Quota)
       Subprocess::FD(STDERR_FILENO),
       None(),
       None(),
-      lambda::bind(&childSetup, pipes),
-      prepare.get().isSome() ? prepare.get().get().namespaces() : 0);
+      prepare.get().isSome() ? prepare.get().get().namespaces() : 0,
+      parentHooks);
 
   ASSERT_SOME(pid);
 
   // Reap the forked child.
   Future<Option<int> > status = process::reap(pid.get());
 
-  // Continue in the parent.
-  ASSERT_SOME(os::close(pipes[0]));
-
-  // Isolate the forked child.
-  AWAIT_READY(isolator->isolate(containerId, pid.get()));
-
-  // Now signal the child to continue.
-  char dummy;
-  ASSERT_LT(0, ::write(pipes[1], &dummy, sizeof(dummy)));
-
-  ASSERT_SOME(os::close(pipes[1]));
-
   // Wait for the command to complete successfully.
   AWAIT_READY(status);
   ASSERT_SOME_EQ(0, status.get());
@@ -765,9 +840,6 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Pids_and_Tids)
   EXPECT_EQ(0U, usage.get().processes());
   EXPECT_EQ(0U, usage.get().threads());
 
-  int pipes[2];
-  ASSERT_NE(-1, ::pipe(pipes));
-
   // Use these to communicate with the test process after it has
   // exec'd to make sure it is running.
   int inputPipes[2];
@@ -779,6 +851,20 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Pids_and_Tids)
   vector<string> argv(1);
   argv[0] = "cat";
 
+  vector<Subprocess::Hook> parentHooks;
+
+  // Create parent Hook to isolate child.
+  // Note: We can safely use references here as we are sure that the life time
+  // of the parent will exceed the child.
+  const lambda::function<Try<Nothing>(pid_t)> isolatePidHook = lambda::bind(
+      isolatePid,
+      lambda::_1,
+      std::cref(isolator),
+      std::cref(containerId),
+      CHECK_CGROUPS);
+
+  parentHooks.emplace_back(Subprocess::Hook(isolatePidHook));
+
   Try<pid_t> pid = launcher->fork(
       containerId,
       "cat",
@@ -788,39 +874,14 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Pids_and_Tids)
       Subprocess::FD(STDERR_FILENO),
       None(),
       None(),
-      lambda::bind(&childSetup, pipes),
-      prepare.get().isSome() ? prepare.get().get().namespaces() : 0);
+      prepare.get().isSome() ? prepare.get().get().namespaces() : 0,
+      parentHooks);
 
   ASSERT_SOME(pid);
 
   // Reap the forked child.
   Future<Option<int>> status = process::reap(pid.get());
 
-  // Continue in the parent.
-  ASSERT_SOME(os::close(pipes[0]));
-
-  // Before isolation, the cgroup is empty.
-  usage = isolator->usage(containerId);
-  AWAIT_READY(usage);
-  EXPECT_EQ(0U, usage.get().processes());
-  EXPECT_EQ(0U, usage.get().threads());
-
-  // Isolate the forked child.
-  AWAIT_READY(isolator->isolate(containerId, pid.get()));
-
-  // After the isolation, the cgroup is not empty, even though the
-  // process hasn't exec'd yet.
-  usage = isolator->usage(containerId);
-  AWAIT_READY(usage);
-  EXPECT_EQ(1U, usage.get().processes());
-  EXPECT_EQ(1U, usage.get().threads());
-
-  // Now signal the child to continue.
-  char dummy;
-  ASSERT_LT(0, ::write(pipes[1], &dummy, sizeof(dummy)));
-
-  ASSERT_SOME(os::close(pipes[1]));
-
   // Write to the test process and wait for an echoed result.
   // This observation ensures that the "cat" process has
   // completed its part of the exec() procedure and is now