You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by zh...@apache.org on 2018/06/12 21:06:07 UTC

[3/3] mesos git commit: Rewrote the `ROOT_BusyMountPoint` test to reflect updated behavior.

Rewrote the `ROOT_BusyMountPoint` test to reflect updated behavior.

The current `ROOT_BusyMountPoint` test would fail because we added
support for unmounting dangling mount points in directory to gc. This
patch rewrote this test to reflect that after unmounting, the gc
succeeded, directory was gone and metrics were correctly reported.

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


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

Branch: refs/heads/master
Commit: d733b1031350e03bce443aa287044eb4eee1053a
Parents: 644ffc8
Author: Zhitao Li <zh...@gmail.com>
Authored: Fri Jun 1 16:58:34 2018 -0700
Committer: Zhitao Li <zh...@gmail.com>
Committed: Tue Jun 12 13:20:46 2018 -0700

----------------------------------------------------------------------
 src/tests/gc_tests.cpp | 90 ++++++++++++++++++++++++++++-----------------
 1 file changed, 57 insertions(+), 33 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/d733b103/src/tests/gc_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/gc_tests.cpp b/src/tests/gc_tests.cpp
index ed0fef3..4f288cf 100644
--- a/src/tests/gc_tests.cpp
+++ b/src/tests/gc_tests.cpp
@@ -899,25 +899,29 @@ TEST_F(GarbageCollectorIntegrationTest, Unschedule)
 
 
 #ifdef __linux__
-// In Mesos it's possible for tasks and isolators to lay down files
-// that are not deletable by GC. This test runs a task that creates a busy
-// mount point which is not directly deletable by GC. We verify that
-// GC deletes all files that it's able to delete in the face of such errors.
-TEST_F(GarbageCollectorIntegrationTest, ROOT_BusyMountPoint)
+// This test creates a persistent volume and runs a task which mounts the volume
+// inside the sandbox, to simulate a dangling mount which agent failed to
+// clean up (see MESOS-8830). We verify that GC process will unmount the
+// dangling mount point successfully and report success in metrics.
+TEST_F(GarbageCollectorIntegrationTest, ROOT_DanglingMount)
 {
   Try<Owned<cluster::Master>> master = StartMaster();
   ASSERT_SOME(master);
 
   slave::Flags flags = CreateSlaveFlags();
+  flags.resources = strings::format("disk(%s):1024", DEFAULT_TEST_ROLE).get();
+
   Owned<MasterDetector> detector = master.get()->createDetector();
   Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), flags);
 
   ASSERT_SOME(slave);
+  FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
+  frameworkInfo.set_roles(0, DEFAULT_TEST_ROLE);
 
   MockScheduler sched;
   MesosSchedulerDriver driver(
       &sched,
-      DEFAULT_FRAMEWORK_INFO,
+      frameworkInfo,
       master.get()->pid,
       DEFAULT_CREDENTIAL);
 
@@ -936,21 +940,33 @@ TEST_F(GarbageCollectorIntegrationTest, ROOT_BusyMountPoint)
   AWAIT_READY(offers);
   ASSERT_FALSE(offers->empty());
 
-  const Offer& offer = offers.get()[0];
-  const SlaveID& slaveId = offer.slave_id();
+  const Offer& offer = offers->at(0);
+
+  string persistenceId = "persistence-id";
+  string containerPath = "path";
+
+  Resource volume = createPersistentVolume(
+      Megabytes(1024),
+      DEFAULT_TEST_ROLE,
+      persistenceId,
+      containerPath,
+      None(),
+      None(),
+      frameworkInfo.principal());
+
+  string mountPoint = "dangling";
+
+  string hostPath = slave::paths::getPersistentVolumePath(
+      flags.work_dir, DEFAULT_TEST_ROLE, persistenceId);
 
-  // The busy mount point goes before the regular file in GC's
-  // directory traversal due to their names. This makes sure that
-  // an error occurs before all deletable files are GCed.
-  string mountPoint = "test1";
-  string regularFile = "test2.txt";
+  string fileInVolume = "foo.txt";
 
   TaskInfo task = createTask(
-      slaveId,
-      Resources::parse("cpus:1;mem:128;disk:1").get(),
-      "touch "+ regularFile + "; "
+      offer.slave_id(),
+      Resources(volume),
+      "touch "+ path::join(containerPath, fileInVolume) + "; "
       "mkdir " + mountPoint + "; "
-      "mount --bind " + mountPoint + " " + mountPoint,
+      "mount --bind " + hostPath + " " + mountPoint,
       None(),
       "test-task123",
       "test-task123");
@@ -966,7 +982,15 @@ TEST_F(GarbageCollectorIntegrationTest, ROOT_BusyMountPoint)
   Future<Nothing> schedule = FUTURE_DISPATCH(
       _, &GarbageCollectorProcess::schedule);
 
-  driver.launchTasks(offer.id(), {task});
+  Future<Nothing> ack1 =
+    FUTURE_DISPATCH(_, &Slave::_statusUpdateAcknowledgement);
+
+  Future<Nothing> ack2 =
+    FUTURE_DISPATCH(_, &Slave::_statusUpdateAcknowledgement);
+
+  driver.acceptOffers(
+      {offer.id()},
+      {CREATE(volume), LAUNCH({task})});
 
   AWAIT_READY(status0);
   EXPECT_EQ(task.task_id(), status0->task_id());
@@ -980,23 +1004,21 @@ TEST_F(GarbageCollectorIntegrationTest, ROOT_BusyMountPoint)
   executorId.set_value("test-task123");
   Result<string> _sandbox = os::realpath(slave::paths::getExecutorLatestRunPath(
       flags.work_dir,
-      slaveId,
+      offer.slave_id(),
       frameworkId.get(),
       executorId));
   ASSERT_SOME(_sandbox);
   string sandbox = _sandbox.get();
   EXPECT_TRUE(os::exists(sandbox));
 
-  // Wait for the task to create these paths.
+  // Wait for the task to create the dangling mount point.
   Timeout timeout = Timeout::in(process::TEST_AWAIT_TIMEOUT);
   while (!os::exists(path::join(sandbox, mountPoint)) ||
-         !os::exists(path::join(sandbox, regularFile)) ||
          !timeout.expired()) {
     os::sleep(Milliseconds(10));
   }
 
   ASSERT_TRUE(os::exists(path::join(sandbox, mountPoint)));
-  ASSERT_TRUE(os::exists(path::join(sandbox, regularFile)));
 
   AWAIT_READY(status2);
   ASSERT_EQ(task.task_id(), status2->task_id());
@@ -1004,15 +1026,16 @@ TEST_F(GarbageCollectorIntegrationTest, ROOT_BusyMountPoint)
 
   AWAIT_READY(schedule);
 
+  ASSERT_TRUE(os::exists(path::join(sandbox, mountPoint)));
+  ASSERT_TRUE(os::exists(path::join(hostPath, fileInVolume)));
+
+  AWAIT_READY(schedule);
+
   Clock::pause();
   Clock::advance(flags.gc_delay);
   Clock::settle();
 
-  EXPECT_TRUE(os::exists(sandbox));
-  EXPECT_TRUE(os::exists(path::join(sandbox, mountPoint)));
-  EXPECT_FALSE(os::exists(path::join(sandbox, regularFile)));
-
-  // Verify that GC metrics show that a path removal failed.
+  // Verify that GC metrics showes no failure.
   JSON::Object metrics = Metrics();
 
   ASSERT_EQ(1u, metrics.values.count("gc/path_removals_pending"));
@@ -1022,18 +1045,19 @@ TEST_F(GarbageCollectorIntegrationTest, ROOT_BusyMountPoint)
   EXPECT_SOME_EQ(
       0u,
       metrics.at<JSON::Number>("gc/path_removals_pending"));
+
   EXPECT_SOME_EQ(
       0u,
-      metrics.at<JSON::Number>("gc/path_removals_succeeded"));
+      metrics.at<JSON::Number>("gc/path_removals_failed"));
 
-  // The sandbox path removal failure will cascade to cause failures to
-  // remove the executor and framework directories. For testing purposes
-  // it is sufficient to verify that some failure was detected.
-  ASSERT_SOME(metrics.at<JSON::Number>("gc/path_removals_failed"));
+  ASSERT_SOME(metrics.at<JSON::Number>("gc/path_removals_succeeded"));
   EXPECT_GT(
-      metrics.at<JSON::Number>("gc/path_removals_failed")->as<unsigned>(),
+      metrics.at<JSON::Number>("gc/path_removals_succeeded")->as<unsigned>(),
       0u);
 
+  ASSERT_FALSE(os::exists(path::join(sandbox, mountPoint)));
+  ASSERT_TRUE(os::exists(path::join(hostPath, fileInVolume)));
+
   Clock::resume();
   driver.stop();
   driver.join();