[mesos] 01/03: Made CNI isolator cleanup more robust.

[ji...@apache.org]

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

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

commit 3c79314e24592b6bd82457249746500d27c4e072
Author: Jie Yu <yu...@gmail.com>
AuthorDate: Mon Aug 13 15:51:30 2018 -0700

    Made CNI isolator cleanup more robust.
    
    If the container is destroyed while in isolator preparing state, the
    cleanup might fail due to missing files or directories. This patch makes
    the cleanup path in CNI isolator more robust so that the cleanup does
    not fail in those scenarios.
    
    Review: https://reviews.apache.org/r/68333
---
 .../mesos/isolators/network/cni/cni.cpp            | 32 +++++++++++++++++-----
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp b/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
index f46c962..cbbf029 100644
--- a/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
+++ b/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
@@ -1565,14 +1565,16 @@ Future<Nothing> NetworkCniIsolatorProcess::_cleanup(
               << target << "' for container " << containerId;
   }
 
-  Try<Nothing> rmdir = os::rmdir(containerDir);
-  if (rmdir.isError()) {
-    return Failure(
-        "Failed to remove the container directory '" +
-        containerDir + "': " + rmdir.error());
-  }
+  if (os::exists(containerDir)) {
+    Try<Nothing> rmdir = os::rmdir(containerDir);
+    if (rmdir.isError()) {
+      return Failure(
+          "Failed to remove the container directory '" +
+          containerDir + "': " + rmdir.error());
+    }
 
-  LOG(INFO) << "Removed the container directory '" << containerDir << "'";
+    LOG(INFO) << "Removed the container directory '" << containerDir << "'";
+  }
 
   infos.erase(containerId);
 
@@ -1616,6 +1618,22 @@ Future<Nothing> NetworkCniIsolatorProcess::detach(
       containerId,
       networkName);
 
+  // There are two cases that the network config file might not exist
+  // when `detach` happens:
+  // (1) The container is destroyed when preparing. In that case, we
+  //     know that `attach` hasn't been called. Therefore, no need to
+  //     call `detach`.
+  // (2) The agent crashes when isolating, but before the network
+  //     config file is checkpointed. In that case, we also know that
+  //     CNI ADD command hasn't been called. Therefore, no need to
+  //     call `detach`.
+  if (!os::exists(networkConfigPath)) {
+    LOG(WARNING) << "Skip detach since network config file for container "
+                 << containerId << " and network name '" << networkName << "' "
+                 << "does not exist";
+    return Nothing();
+  }
+
   Try<JSON::Object> networkConfigJSON = getNetworkConfigJSON(
       networkName,
       networkConfigPath);