You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by qi...@apache.org on 2021/08/09 14:21:35 UTC

[mesos] branch master updated: Fixed NestedMesosContainerizerTest hangs on errors.

This is an automated email from the ASF dual-hosted git repository.

qianzhang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git


The following commit(s) were added to refs/heads/master by this push:
     new 52cfb8e  Fixed NestedMesosContainerizerTest hangs on errors.
52cfb8e is described below

commit 52cfb8ed0f3b48cadc90b7c15236c4c7e5489dd7
Author: Charles-Francois Natali <cf...@gmail.com>
AuthorDate: Mon Aug 9 22:19:09 2021 +0800

    Fixed NestedMesosContainerizerTest hangs on errors.
    
    Those tests would use a named pipe to synchronize with the task being
    started. The problem is that if the task fails to start, reading from
    the pipe would block indefinitely, making the tests just hang.
    
    We could update the code to use a read with a timeout, however it's a
    bit fiddly and it's simpler to just use the presence as a regular file
    as a barrier.
    
    See https://issues.apache.org/jira/browse/MESOS-10226 for context.
    
    Tested by @martin-g
    
    This closes #402
---
 .../nested_mesos_containerizer_tests.cpp           | 59 ++++++++++++----------
 1 file changed, 32 insertions(+), 27 deletions(-)

diff --git a/src/tests/containerizer/nested_mesos_containerizer_tests.cpp b/src/tests/containerizer/nested_mesos_containerizer_tests.cpp
index 8aaf80a..4731596 100644
--- a/src/tests/containerizer/nested_mesos_containerizer_tests.cpp
+++ b/src/tests/containerizer/nested_mesos_containerizer_tests.cpp
@@ -157,6 +157,23 @@ protected:
 
     return containerConfig;
   }
+
+  static bool awaitSynchronizationFile(const string& path)
+  {
+    Duration waited = Duration::zero();
+    Duration interval = Milliseconds(1);
+
+    do {
+      if (os::exists(path)) {
+        return true;
+      }
+
+      os::sleep(interval);
+      waited += interval;
+    } while (waited < process::TEST_AWAIT_TIMEOUT);
+
+    return false;
+  }
 };
 
 
@@ -663,15 +680,14 @@ TEST_F(NestedMesosContainerizerTest,
   ContainerID containerId;
   containerId.set_value(id::UUID::random().toString());
 
-  // Use a pipe to synchronize with the top-level container.
-  string pipe = path::join(sandbox.get(), "pipe");
-  ASSERT_EQ(0, ::mkfifo(pipe.c_str(), 0700));
+  // Use a file to synchronize with the top-level container.
+  string syncFile = path::join(sandbox.get(), "syncFile");
 
   const string filename = "nested_inherits_work_dir";
 
   ExecutorInfo executor = createExecutorInfo(
       "executor",
-      "touch " + filename + "; echo running > " + pipe + "; sleep 1000",
+      "touch " + filename + "; touch " + syncFile + "; sleep 1000",
       "cpus:1");
 
   Try<string> directory = environment->mkdtemp();
@@ -695,9 +711,7 @@ TEST_F(NestedMesosContainerizerTest,
 
   // Wait for the parent container to start running its task
   // before launching a debug container inside it.
-  Result<string> read = os::read(pipe);
-  ASSERT_SOME(read);
-  ASSERT_EQ("running\n", read.get());
+  ASSERT_TRUE(awaitSynchronizationFile(syncFile));
 
   Future<ContainerStatus> status = containerizer->status(containerId);
   AWAIT_READY(status);
@@ -1093,15 +1107,14 @@ TEST_F(NestedMesosContainerizerTest,
   AWAIT_READY(offers);
   ASSERT_EQ(1u, offers->size());
 
-  // Use a pipe to synchronize with the top-level container.
-  string pipe = path::join(sandbox.get(), "pipe");
-  ASSERT_EQ(0, ::mkfifo(pipe.c_str(), 0700));
+  // Use a file to synchronize with the top-level container.
+  string syncFile = path::join(sandbox.get(), "syncFile");
 
   // Launch a command task within the `alpine` docker image.
   TaskInfo task = createTask(
       offers->front().slave_id(),
       offers->front().resources(),
-      "echo running > /tmp/pipe; sleep 1000");
+      "touch /tmp/syncFile; sleep 1000");
 
   task.mutable_container()->CopyFrom(createContainerInfo(
       "alpine", {createVolumeHostPath("/tmp", sandbox.get(), Volume::RW)}));
@@ -1123,9 +1136,7 @@ TEST_F(NestedMesosContainerizerTest,
 
   // Wait for the parent container to start running its task
   // before launching a debug container inside it.
-  Result<string> read = os::read(pipe);
-  ASSERT_SOME(read);
-  ASSERT_EQ("running\n", read.get());
+  ASSERT_TRUE(awaitSynchronizationFile(syncFile));
 
   ASSERT_TRUE(statusRunning->has_slave_id());
   ASSERT_TRUE(statusRunning->has_container_status());
@@ -1207,14 +1218,13 @@ TEST_F(NestedMesosContainerizerTest,
   ContainerID containerId;
   containerId.set_value(id::UUID::random().toString());
 
-  string pipe = path::join(sandbox.get(), "pipe");
-  ASSERT_EQ(0, ::mkfifo(pipe.c_str(), 0700));
+  string syncFile = path::join(sandbox.get(), "syncFile");
 
   const string cmd =
     "(unshare -m sh -c"
     " 'mkdir -p test_mnt; mount tmpfs -t tmpfs test_mnt;"
     " touch test_mnt/check; exec sleep 1000')&"
-    "echo running > " + pipe + "; exec sleep 1000";
+    "touch " + syncFile + "; exec sleep 1000";
 
   ExecutorInfo executor = createExecutorInfo("executor", cmd, "cpus:1");
 
@@ -1231,9 +1241,7 @@ TEST_F(NestedMesosContainerizerTest,
 
   // Wait for the parent container to start running its task
   // before launching a debug nested container.
-  Result<string> read = os::read(pipe);
-  ASSERT_SOME(read);
-  ASSERT_EQ("running\n", read.get());
+  ASSERT_TRUE(awaitSynchronizationFile(syncFile));
 
   // Launch a nested debug container.
   ContainerID nestedContainerId;
@@ -1650,13 +1658,12 @@ TEST_F(NestedMesosContainerizerTest, ROOT_CGROUPS_ParentSigterm)
   ContainerID containerId;
   containerId.set_value(id::UUID::random().toString());
 
-  // 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));
+  // Use a file to synchronize with the top-level container.
+  string syncFile = path::join(sandbox.get(), "syncFile");
 
   ExecutorInfo executor = createExecutorInfo(
       "executor",
-      createCommandInfo("echo running > " + pipe + "; sleep 1000"),
+      createCommandInfo("touch " + syncFile + "; sleep 1000"),
       "cpus:1");
 
   Try<string> directory = environment->mkdtemp();
@@ -1694,9 +1701,7 @@ TEST_F(NestedMesosContainerizerTest, ROOT_CGROUPS_ParentSigterm)
 
   // Wait for the parent container to start running its executor
   // process before sending it a signal.
-  Result<string> read = os::read(pipe);
-  ASSERT_SOME(read);
-  ASSERT_EQ("running\n", read.get());
+  ASSERT_TRUE(awaitSynchronizationFile(syncFile));
 
   ASSERT_EQ(0, os::kill(status->executor_pid(), SIGTERM));