Exposed unknown container case from Containerizer::destroy.

Currently the callers of `destroy` cannot determine if the call
succeeds or fails (without a secondary call to `wait()`).

This also allows the caller to distinguish between a failure and
waiting on an unknown container. This is important for the upcoming
agent child container API, as the end-user would benefit from the
distinction.

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


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

Branch: refs/heads/master
Commit: 2d20e0f53168c862a84c206f875379bae226dc89
Parents: f73f55c
Author: Benjamin Mahler <bmah...@apache.org>
Authored: Fri Sep 16 18:19:48 2016 -0700
Committer: Benjamin Mahler <bmah...@apache.org>
Committed: Wed Sep 21 15:55:01 2016 -0700

----------------------------------------------------------------------
 src/slave/containerizer/composing.cpp           | 111 +++++++++++++------
 src/slave/containerizer/composing.hpp           |   2 +-
 src/slave/containerizer/containerizer.hpp       |   6 +-
 src/slave/containerizer/docker.cpp              |  29 +++--
 src/slave/containerizer/docker.hpp              |   4 +-
 src/slave/containerizer/mesos/containerizer.cpp |  45 +++++---
 src/slave/containerizer/mesos/containerizer.hpp |   8 +-
 src/tests/containerizer.cpp                     |  14 ++-
 src/tests/containerizer.hpp                     |   8 +-
 .../composing_containerizer_tests.cpp           |  40 +++++--
 .../docker_containerizer_tests.cpp              |  23 ++++
 .../containerizer/mesos_containerizer_tests.cpp |  23 ++++
 12 files changed, 225 insertions(+), 88 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/2d20e0f5/src/slave/containerizer/composing.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/composing.cpp 
b/src/slave/containerizer/composing.cpp
index 9181309..179304a 100644
--- a/src/slave/containerizer/composing.cpp
+++ b/src/slave/containerizer/composing.cpp
@@ -83,7 +83,7 @@ public:
   Future<Option<ContainerTermination>> wait(
       const ContainerID& containerId);
 
-  void destroy(const ContainerID& containerId);
+  Future<bool> destroy(const ContainerID& containerId);
 
   Future<hashset<ContainerID>> containers();
 
@@ -115,13 +115,16 @@ private:
   {
     LAUNCHING,
     LAUNCHED,
-    DESTROYED
+    DESTROYING,
+    // No need for DESTROYED, since we remove containers
+    // once the destroy completes.
   };
 
   struct Container
   {
     State state;
     Containerizer* containerizer;
+    Promise<bool> destroyed;
   };
 
   hashmap<ContainerID, Container*> containers_;
@@ -213,9 +216,11 @@ Future<Option<ContainerTermination>> 
ComposingContainerizer::wait(
 }
 
 
-void ComposingContainerizer::destroy(const ContainerID& containerId)
+Future<bool> ComposingContainerizer::destroy(const ContainerID& containerId)
 {
-  dispatch(process, &ComposingContainerizerProcess::destroy, containerId);
+  return dispatch(process,
+                  &ComposingContainerizerProcess::destroy,
+                  containerId);
 }
 
 
@@ -301,12 +306,14 @@ Future<bool> ComposingContainerizerProcess::_launch(
     vector<Containerizer*>::iterator containerizer,
     bool launched)
 {
-  // The container struct won't be cleaned up by destroy because
-  // in destroy we only forward the destroy, and wait until the
-  // launch returns and clean up here.
+  // The container struct will not be cleaned up by destroy
+  // when the container is in the LAUNCHING state.
   CHECK(containers_.contains(containerId));
-  Container* container = containers_[containerId];
-  if (container->state == DESTROYED) {
+  Container* container = containers_.at(containerId);
+
+  // A destroy arrived in the interim.
+  if (container->state == DESTROYING) {
+    container->destroyed.set(true);
     containers_.erase(containerId);
     delete container;
     return Failure("Container was destroyed while launching");
@@ -447,40 +454,72 @@ Future<Option<ContainerTermination>> 
ComposingContainerizerProcess::wait(
 }
 
 
-void ComposingContainerizerProcess::destroy(const ContainerID& containerId)
+Future<bool> ComposingContainerizerProcess::destroy(
+    const ContainerID& containerId)
 {
   if (!containers_.contains(containerId)) {
-    LOG(WARNING) << "Container '" << containerId.value() << "' not found";
-    return;
-  }
-
-  Container* container = containers_[containerId];
+    // TODO(bmahler): Currently the agent does not log destroy
+    // failures or unknown containers, so we log it here for now.
+    // Move this logging into the callers.
+    LOG(WARNING) << "Attempted to destroy unknown container " << containerId;
 
-  if (container->state == DESTROYED) {
-    LOG(WARNING) << "Container '" << containerId.value()
-                 << "' is already destroyed";
-    return;
+    return false;
   }
 
-  // It's ok to forward destroy to any containerizer which is currently
-  // launching the container, because we expect each containerizer to
-  // handle calling destroy on non-existing container.
-  // The composing containerizer will not move to the next
-  // containerizer for a container that is destroyed as well.
-  container->containerizer->destroy(containerId);
-
-  if (container->state == LAUNCHING) {
-    // Record the fact that this container was asked to be destroyed
-    // so that we won't try and launch this container using any other
-    // containerizers in the event the current containerizer has
-    // decided it can't launch the container.
-    container->state = DESTROYED;
-    return;
+  Container* container = containers_.at(containerId);
+
+  switch (container->state) {
+    case DESTROYING:
+      break; // No-op.
+
+    case LAUNCHING:
+      container->state = DESTROYING;
+
+      // Forward the destroy request to the containerizer. Note that
+      // a containerizer is expected to handle a destroy while
+      // `launch()` is in progress. If the containerizer could not
+      // handle launching the container (`launch()` returns false),
+      // then the containerizer may no longer know about this
+      // container. If the launch returns false, we will stop trying
+      // to launch the container on other containerizers.
+      container->containerizer->destroy(containerId)
+        .onAny(defer(self(), [=](const Future<bool>& destroy) {
+          // We defer the association of the promise in order to
+          // surface a successful destroy (by setting
+          // `Container.destroyed` to true in `_launch()`) when
+          // the containerizer cannot handle this type of container
+          // (`launch()` returns false). If we do not defer here and
+          // instead associate the future right away, the setting of
+          // `Container.destroy` in `_launch()` will be a no-op;
+          // this might result in users waiting on the future
+          // incorrectly thinking that the destroy failed when in
+          // fact the destory is implicitly successful because the
+          // launch failed.
+          if (containers_.contains(containerId)) {
+            containers_.at(containerId)->destroyed.associate(destroy);
+          }
+        }));
+
+      break;
+
+    case LAUNCHED:
+      container->state = DESTROYING;
+
+      container->destroyed.associate(
+          container->containerizer->destroy(containerId));
+
+      container->destroyed.future()
+        .onAny(defer(self(), [=](const Future<bool>& destroy) {
+          if (containers_.contains(containerId)) {
+            delete containers_.at(containerId);
+            containers_.erase(containerId);
+          }
+        }));
+
+      break;
   }
 
-  // If the container is launched, then we can simply cleanup.
-  containers_.erase(containerId);
-  delete container;
+  return container->destroyed.future();
 }
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/2d20e0f5/src/slave/containerizer/composing.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/composing.hpp 
b/src/slave/containerizer/composing.hpp
index 3df7604..9be7802 100644
--- a/src/slave/containerizer/composing.hpp
+++ b/src/slave/containerizer/composing.hpp
@@ -75,7 +75,7 @@ public:
   virtual process::Future<Option<mesos::slave::ContainerTermination>> wait(
       const ContainerID& containerId);
 
-  virtual void destroy(const ContainerID& containerId);
+  virtual process::Future<bool> destroy(const ContainerID& containerId);
 
   virtual process::Future<hashset<ContainerID>> containers();
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/2d20e0f5/src/slave/containerizer/containerizer.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/containerizer.hpp 
b/src/slave/containerizer/containerizer.hpp
index 5ee1107..47a8582 100644
--- a/src/slave/containerizer/containerizer.hpp
+++ b/src/slave/containerizer/containerizer.hpp
@@ -129,10 +129,12 @@ public:
       const ContainerID& containerId) = 0;
 
   // Destroy a running container, killing all processes and releasing
-  // all resources.
+  // all resources. Returns false when the container cannot be found,
+  // or a failure if something went wrong.
+  //
   // NOTE: You cannot wait() on containers that have been destroyed,
   // so you should always call wait() before destroy().
-  virtual void destroy(const ContainerID& containerId) = 0;
+  virtual process::Future<bool> destroy(const ContainerID& containerId) = 0;
 
   virtual process::Future<hashset<ContainerID>> containers() = 0;
 };

http://git-wip-us.apache.org/repos/asf/mesos/blob/2d20e0f5/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp 
b/src/slave/containerizer/docker.cpp
index 27a49a2..1d27761 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -724,9 +724,9 @@ Future<Option<ContainerTermination>> 
DockerContainerizer::wait(
 }
 
 
-void DockerContainerizer::destroy(const ContainerID& containerId)
+Future<bool> DockerContainerizer::destroy(const ContainerID& containerId)
 {
-  dispatch(
+  return dispatch(
       process.get(),
       &DockerContainerizerProcess::destroy,
       containerId, true);
@@ -1794,15 +1794,19 @@ Future<Option<ContainerTermination>> 
DockerContainerizerProcess::wait(
 }
 
 
-void DockerContainerizerProcess::destroy(
+Future<bool> DockerContainerizerProcess::destroy(
     const ContainerID& containerId,
     bool killed)
 {
   CHECK(!containerId.has_parent());
 
   if (!containers_.contains(containerId)) {
-    LOG(WARNING) << "Ignoring destroy of unknown container " << containerId;
-    return;
+    // TODO(bmahler): Currently the agent does not log destroy
+    // failures or unknown containers, so we log it here for now.
+    // Move this logging into the callers.
+    LOG(WARNING) << "Attempted to destroy unknown container " << containerId;
+
+    return false;
   }
 
   Container* container = containers_.at(containerId);
@@ -1821,12 +1825,12 @@ void DockerContainerizerProcess::destroy(
     containers_.erase(containerId);
     delete container;
 
-    return;
+    return true;
   }
 
   if (container->state == Container::DESTROYING) {
-    // Destroy has already been initiated.
-    return;
+    return container->termination.future()
+      .then([]() { return true; });
   }
 
   LOG(INFO) << "Destroying container " << containerId;
@@ -1867,7 +1871,7 @@ void DockerContainerizerProcess::destroy(
     containers_.erase(containerId);
     delete container;
 
-    return;
+    return true;
   }
 
   if (container->state == Container::PULLING) {
@@ -1882,7 +1886,7 @@ void DockerContainerizerProcess::destroy(
     containers_.erase(containerId);
     delete container;
 
-    return;
+    return true;
   }
 
   if (container->state == Container::MOUNTING) {
@@ -1903,7 +1907,7 @@ void DockerContainerizerProcess::destroy(
     containers_.erase(containerId);
     delete container;
 
-    return;
+    return true;
   }
 
   CHECK(container->state == Container::RUNNING);
@@ -1935,6 +1939,9 @@ void DockerContainerizerProcess::destroy(
   // above.
   container->status.future()
     .onAny(defer(self(), &Self::_destroy, containerId, killed));
+
+  return container->termination.future()
+    .then([]() { return true; });
 }
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/2d20e0f5/src/slave/containerizer/docker.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.hpp 
b/src/slave/containerizer/docker.hpp
index aa6a9d6..8da6310 100644
--- a/src/slave/containerizer/docker.hpp
+++ b/src/slave/containerizer/docker.hpp
@@ -103,7 +103,7 @@ public:
   virtual process::Future<Option<mesos::slave::ContainerTermination>> wait(
       const ContainerID& containerId);
 
-  virtual void destroy(const ContainerID& containerId);
+  virtual process::Future<bool> destroy(const ContainerID& containerId);
 
   virtual process::Future<hashset<ContainerID>> containers();
 
@@ -153,7 +153,7 @@ public:
   virtual process::Future<Option<mesos::slave::ContainerTermination>> wait(
       const ContainerID& containerId);
 
-  virtual void destroy(
+  virtual process::Future<bool> destroy(
       const ContainerID& containerId,
       bool killed = true); // process is either killed or reaped.
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/2d20e0f5/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp 
b/src/slave/containerizer/mesos/containerizer.cpp
index a6f0d79..144b0db 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -547,11 +547,11 @@ Future<Option<ContainerTermination>> 
MesosContainerizer::wait(
 }
 
 
-void MesosContainerizer::destroy(const ContainerID& containerId)
+Future<bool> MesosContainerizer::destroy(const ContainerID& containerId)
 {
-  dispatch(process.get(),
-           &MesosContainerizerProcess::destroy,
-           containerId);
+  return dispatch(process.get(),
+                  &MesosContainerizerProcess::destroy,
+                  containerId);
 }
 
 
@@ -1409,7 +1409,7 @@ Future<Option<ContainerTermination>> 
MesosContainerizerProcess::wait(
     return None();
   }
 
-  return containers_.at(containerId)->promise.future()
+  return containers_.at(containerId)->termination.future()
     .then(Option<ContainerTermination>::some);
 }
 
@@ -1575,7 +1575,7 @@ Future<ContainerStatus> MesosContainerizerProcess::status(
 }
 
 
-void MesosContainerizerProcess::destroy(
+Future<bool> MesosContainerizerProcess::destroy(
     const ContainerID& containerId)
 {
   CHECK(!containerId.has_parent());
@@ -1593,15 +1593,20 @@ void MesosContainerizerProcess::destroy(
     //
     // The guard here and `if (container->state == DESTROYING)` below
     // make sure redundant destroys short-circuit.
-    VLOG(1) << "Ignoring destroy of unknown container " << containerId;
-    return;
+
+    // TODO(bmahler): Currently the agent does not log destroy
+    // failures or unknown containers, so we log it here for now.
+    // Move this logging into the callers.
+    LOG(WARNING) << "Attempted to destroy unknown container " << containerId;
+
+    return false;
   }
 
   const Owned<Container>& container = containers_.at(containerId);
 
   if (container->state == DESTROYING) {
-    VLOG(1) << "Destroy has already been initiated for " << containerId;
-    return;
+    return container->termination.future()
+      .then([]() { return true; });
   }
 
   LOG(INFO) << "Destroying container " << containerId;
@@ -1621,7 +1626,8 @@ void MesosContainerizerProcess::destroy(
           containerId,
           list<Future<Nothing>>()));
 
-    return;
+    return container->termination.future()
+      .then([]() { return true; });
   }
 
   if (container->state == PREPARING) {
@@ -1646,7 +1652,8 @@ void MesosContainerizerProcess::destroy(
             : None())
       .onAny(defer(self(), &Self::___destroy, containerId));
 
-    return;
+    return container->termination.future()
+      .then([]() { return true; });
   }
 
   if (container->state == ISOLATING) {
@@ -1660,7 +1667,8 @@ void MesosContainerizerProcess::destroy(
     container->isolation
       .onAny(defer(self(), &Self::_destroy, containerId));
 
-    return;
+    return container->termination.future()
+      .then([]() { return true; });
   }
 
   // Either RUNNING or FETCHING at this point.
@@ -1670,6 +1678,9 @@ void MesosContainerizerProcess::destroy(
 
   container->state = DESTROYING;
   _destroy(containerId);
+
+  return container->termination.future()
+    .then([]() { return true; });
 }
 
 
@@ -1699,7 +1710,7 @@ void MesosContainerizerProcess::__destroy(
   // TODO(idownes): This is a pretty bad state to be in but we should
   // consider cleaning up here.
   if (!future.isReady()) {
-    container->promise.fail(
+    container->termination.fail(
         "Failed to kill all processes in the container: " +
         (future.isFailed() ? future.failure() : "discarded future"));
 
@@ -1753,7 +1764,7 @@ void MesosContainerizerProcess::____destroy(
   }
 
   if (!errors.empty()) {
-    container->promise.fail(
+    container->termination.fail(
         "Failed to clean up an isolator when destroying container: " +
         strings::join("; ", errors));
 
@@ -1777,7 +1788,7 @@ void MesosContainerizerProcess::_____destroy(
   const Owned<Container>& container = containers_.at(containerId);
 
   if (!destroy.isReady()) {
-    container->promise.fail(
+    container->termination.fail(
         "Failed to destroy the provisioned rootfs when destroying container: " 
+
         (destroy.isFailed() ? destroy.failure() : "discarded future"));
 
@@ -1816,7 +1827,7 @@ void MesosContainerizerProcess::_____destroy(
     termination.set_message(strings::join("; ", messages));
   }
 
-  container->promise.set(termination);
+  container->termination.set(termination);
 
   containers_.erase(containerId);
 }

http://git-wip-us.apache.org/repos/asf/mesos/blob/2d20e0f5/src/slave/containerizer/mesos/containerizer.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.hpp 
b/src/slave/containerizer/mesos/containerizer.hpp
index a29da73..16f9e3e 100644
--- a/src/slave/containerizer/mesos/containerizer.hpp
+++ b/src/slave/containerizer/mesos/containerizer.hpp
@@ -104,7 +104,8 @@ public:
   virtual process::Future<Option<mesos::slave::ContainerTermination>> wait(
       const ContainerID& containerId);
 
-  virtual void destroy(const ContainerID& containerId);
+  virtual process::Future<bool> destroy(
+      const ContainerID& containerId);
 
   virtual process::Future<hashset<ContainerID>> containers();
 
@@ -172,7 +173,8 @@ public:
       const ContainerID& containerId,
       int pipeWrite);
 
-  virtual void destroy(const ContainerID& containerId);
+  virtual process::Future<bool> destroy(
+      const ContainerID& containerId);
 
   virtual process::Future<hashset<ContainerID>> containers();
 
@@ -283,7 +285,7 @@ private:
     Container() : sequence("mesos-container-status-updates") {}
 
     // Promise for futures returned from wait().
-    process::Promise<mesos::slave::ContainerTermination> promise;
+    process::Promise<mesos::slave::ContainerTermination> termination;
 
     // We keep track of the future exit status for the container if it
     // has been launched. If the container has not been launched yet,

http://git-wip-us.apache.org/repos/asf/mesos/blob/2d20e0f5/src/tests/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer.cpp b/src/tests/containerizer.cpp
index 78e18e8..89c5400 100644
--- a/src/tests/containerizer.cpp
+++ b/src/tests/containerizer.cpp
@@ -208,7 +208,7 @@ Future<Option<ContainerTermination>> 
TestContainerizer::_wait(
 }
 
 
-void TestContainerizer::destroy(
+Future<bool> TestContainerizer::destroy(
     const FrameworkID& frameworkId,
     const ExecutorID& executorId)
 {
@@ -216,17 +216,17 @@ void TestContainerizer::destroy(
   if (!containers_.contains(key)) {
     LOG(WARNING) << "Ignoring destroy of unknown container for executor '"
                  << executorId << "' of framework " << frameworkId;
-    return;
+    return false;
   }
 
-  _destroy(containers_[key]);
+  return _destroy(containers_.at(key));
 }
 
 
-void TestContainerizer::_destroy(const ContainerID& containerId)
+Future<bool> TestContainerizer::_destroy(const ContainerID& containerId)
 {
   if (drivers.contains(containerId)) {
-    Owned<MesosExecutorDriver> driver = drivers[containerId];
+    Owned<MesosExecutorDriver> driver = drivers.at(containerId);
     driver->stop();
     driver->join();
     drivers.erase(containerId);
@@ -241,9 +241,11 @@ void TestContainerizer::_destroy(const ContainerID& 
containerId)
     termination.set_message("Killed executor");
     termination.set_status(0);
 
-    promises[containerId]->set(termination);
+    promises.at(containerId)->set(termination);
     promises.erase(containerId);
   }
+
+  return true;
 }
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/2d20e0f5/src/tests/containerizer.hpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer.hpp b/src/tests/containerizer.hpp
index 8f69972..3cf418a 100644
--- a/src/tests/containerizer.hpp
+++ b/src/tests/containerizer.hpp
@@ -108,11 +108,13 @@ public:
 
   MOCK_METHOD1(
       destroy,
-      void(const ContainerID&));
+      process::Future<bool>(const ContainerID&));
 
   // Additional destroy method for testing because we won't know the
   // ContainerID created for each container.
-  void destroy(const FrameworkID& frameworkId, const ExecutorID& executorId);
+  process::Future<bool> destroy(
+      const FrameworkID& frameworkId,
+      const ExecutorID& executorId);
 
 private:
   void setup();
@@ -131,7 +133,7 @@ private:
   process::Future<Option<mesos::slave::ContainerTermination>> _wait(
       const ContainerID& containerId) const;
 
-  void _destroy(const ContainerID& containerID);
+  process::Future<bool> _destroy(const ContainerID& containerID);
 
   hashmap<ExecutorID, Executor*> executors;
   hashmap<ExecutorID, std::shared_ptr<MockV1HTTPExecutor>> v1Executors;

http://git-wip-us.apache.org/repos/asf/mesos/blob/2d20e0f5/src/tests/containerizer/composing_containerizer_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/composing_containerizer_tests.cpp 
b/src/tests/containerizer/composing_containerizer_tests.cpp
index 19e31b1..78d666d 100644
--- a/src/tests/containerizer/composing_containerizer_tests.cpp
+++ b/src/tests/containerizer/composing_containerizer_tests.cpp
@@ -24,6 +24,7 @@
 #include <process/gmock.hpp>
 
 #include <stout/option.hpp>
+#include <stout/uuid.hpp>
 
 #include "messages/messages.hpp"
 
@@ -39,6 +40,7 @@ using namespace process;
 using std::vector;
 
 using testing::_;
+using testing::DoAll;
 using testing::Return;
 
 using mesos::slave::ContainerTermination;
@@ -88,7 +90,7 @@ public:
 
   MOCK_METHOD1(
       destroy,
-      void(const ContainerID&));
+      process::Future<bool>(const ContainerID&));
 
   MOCK_METHOD0(
       containers,
@@ -96,11 +98,13 @@ public:
 };
 
 
-// This test checks if destroy is called while container is being
-// launched, the composing containerizer still calls the underlying
-// containerizer's destroy and skip calling the rest of the
-// containerizers.
-TEST_F(ComposingContainerizerTest, DestroyWhileLaunching)
+// This test ensures that destroy can be called while in the
+// launch loop. The composing containerizer still calls the
+// underlying containerizer's destroy (because it's not sure
+// if the containerizer can handle the type of container being
+// launched). Once the launch is rejected, the composing
+// containerizer should stop the launch loop.
+TEST_F(ComposingContainerizerTest, DestroyDuringLaunchLoop)
 {
   vector<Containerizer*> containerizers;
 
@@ -126,7 +130,8 @@ TEST_F(ComposingContainerizerTest, DestroyWhileLaunching)
   Future<Nothing> destroy;
 
   EXPECT_CALL(*mockContainerizer, destroy(_))
-    .WillOnce(FutureSatisfy(&destroy));
+    .WillOnce(DoAll(FutureSatisfy(&destroy),
+                    Return(Future<bool>(false))));
 
   Future<bool> launch = containerizer.launch(
       containerId,
@@ -157,6 +162,27 @@ TEST_F(ComposingContainerizerTest, DestroyWhileLaunching)
 }
 
 
+// Ensures the containerizer responds correctly (false Future) to
+// a request to destroy an unknown container.
+TEST_F(ComposingContainerizerTest, DestroyUnknownContainer)
+{
+  vector<Containerizer*> containerizers;
+
+  MockContainerizer* mockContainerizer = new MockContainerizer();
+  MockContainerizer* mockContainerizer2 = new MockContainerizer();
+
+  containerizers.push_back(mockContainerizer);
+  containerizers.push_back(mockContainerizer2);
+
+  ComposingContainerizer containerizer(containerizers);
+
+  ContainerID containerId;
+  containerId.set_value(UUID::random().toString());
+
+  AWAIT_EXPECT_FALSE(containerizer.destroy(containerId));
+}
+
+
 // Ensures the containerizer responds correctly (returns None)
 // to a request to wait on an unknown container.
 TEST_F(ComposingContainerizerTest, WaitUnknownContainer)

http://git-wip-us.apache.org/repos/asf/mesos/blob/2d20e0f5/src/tests/containerizer/docker_containerizer_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/docker_containerizer_tests.cpp 
b/src/tests/containerizer/docker_containerizer_tests.cpp
index ef4039b..7e91494 100644
--- a/src/tests/containerizer/docker_containerizer_tests.cpp
+++ b/src/tests/containerizer/docker_containerizer_tests.cpp
@@ -26,6 +26,7 @@
 #include <process/subprocess.hpp>
 
 #include <stout/duration.hpp>
+#include <stout/uuid.hpp>
 
 #ifdef __linux__
 #include "linux/cgroups.hpp"
@@ -3316,6 +3317,28 @@ TEST_F(DockerContainerizerTest, 
ROOT_DOCKER_DestroyWhilePulling)
 }
 
 
+// Ensures the containerizer responds correctly (false Future) to
+// a request to destroy an unknown container.
+TEST_F(DockerContainerizerTest, ROOT_DOCKER_DestroyUnknownContainer)
+{
+  slave::Flags flags = CreateSlaveFlags();
+
+  Fetcher fetcher;
+
+  Try<DockerContainerizer*> create =
+    DockerContainerizer::create(flags, &fetcher);
+
+  ASSERT_SOME(create);
+
+  DockerContainerizer* containerizer = create.get();
+
+  ContainerID containerId;
+  containerId.set_value(UUID::random().toString());
+
+  AWAIT_EXPECT_FALSE(containerizer->destroy(containerId));
+}
+
+
 // This test checks that when a docker containerizer update failed
 // and the container failed before the executor started, the executor
 // is properly killed and cleaned up.

http://git-wip-us.apache.org/repos/asf/mesos/blob/2d20e0f5/src/tests/containerizer/mesos_containerizer_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/mesos_containerizer_tests.cpp 
b/src/tests/containerizer/mesos_containerizer_tests.cpp
index d7455d2..4a94829 100644
--- a/src/tests/containerizer/mesos_containerizer_tests.cpp
+++ b/src/tests/containerizer/mesos_containerizer_tests.cpp
@@ -32,6 +32,7 @@
 
 #include <stout/net.hpp>
 #include <stout/strings.hpp>
+#include <stout/uuid.hpp>
 
 #include "slave/flags.hpp"
 
@@ -771,6 +772,28 @@ TEST_F(MesosContainerizerDestroyTest, 
DestroyWhilePreparing)
 }
 
 
+// Ensures the containerizer responds correctly (false Future) to
+// a request to destroy an unknown container.
+TEST_F(MesosContainerizerDestroyTest, DestroyUnknownContainer)
+{
+  slave::Flags flags = CreateSlaveFlags();
+
+  Fetcher fetcher;
+
+  Try<MesosContainerizer*> create =
+    MesosContainerizer::create(flags, true, &fetcher);
+
+  ASSERT_SOME(create);
+
+  MesosContainerizer* containerizer = create.get();
+
+  ContainerID containerId;
+  containerId.set_value(UUID::random().toString());
+
+  AWAIT_EXPECT_FALSE(containerizer->destroy(containerId));
+}
+
+
 class MesosContainerizerProvisionerTest : public MesosTest {};
 
 

Reply via email to