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

[mesos] branch master updated: Allowed CNI root directory to be in a persistent location.

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


The following commit(s) were added to refs/heads/master by this push:
     new 5ab822c  Allowed CNI root directory to be in a persistent location.
5ab822c is described below

commit 5ab822cfce6d27c86832645cbc007eee8791db5f
Author: Deepak Goel <de...@gmail.com>
AuthorDate: Tue Dec 18 22:04:06 2018 -0800

    Allowed CNI root directory to be in a persistent location.
    
    This patch added the support to configure the CNI root directory to be
    under `work_dir` so that the network related information persist across
    reboot which will allow cni isolator to do proper cleanup post reboot.
    
    A new agent flag `--network_cni_root_dir_persist` has been introduced.
    
    Review: https://reviews.apache.org/r/69590/
---
 docs/cni.md                                        |  7 +++
 docs/configuration/agent.md                        | 10 +++++
 .../mesos/isolators/network/cni/cni.cpp            | 30 ++++++++-----
 .../mesos/isolators/network/cni/paths.cpp          | 10 +++++
 .../mesos/isolators/network/cni/paths.hpp          |  9 +++-
 src/slave/flags.cpp                                |  6 +++
 src/slave/flags.hpp                                |  1 +
 src/tests/containerizer/cni_isolator_tests.cpp     | 51 +++++++++++++++++++---
 8 files changed, 104 insertions(+), 20 deletions(-)

diff --git a/docs/cni.md b/docs/cni.md
index 4a0e0bf..ced047e 100644
--- a/docs/cni.md
+++ b/docs/cni.md
@@ -103,6 +103,13 @@ after Agent startup, the Agent needs to be restarted. The
 and hence restarting the Agent (and therefore the `network/cni`
 isolator) will not affect container orchestration.
 
+Optionally, the operator could specify the
+`--network_cni_root_dir_persist` flag. This flag would allow
+`network/cni` isolator to persist the network related information
+across reboot and allow `network/cni` isolator to carry out network
+cleanup post reboot. This is useful for the CNI networks that depend
+on the isolator to clean their network state.
+
 #### <a name="adding-modifying-deleting"></a>Adding/Deleting/Modifying CNI networks
 
 The `network/cni` isolator learns about all the CNI networks by
diff --git a/docs/configuration/agent.md b/docs/configuration/agent.md
index 7a8df68..330283f 100644
--- a/docs/configuration/agent.md
+++ b/docs/configuration/agent.md
@@ -1176,6 +1176,16 @@ a network configuration file in JSON format in the specified directory.
   </td>
 </tr>
 
+<tr id="network_cni_root_dir_persist">
+  <td>
+    --[no-]network_cni_root_dir_persist
+  </td>
+  <td>
+This setting controls whether the CNI root directory persists across
+reboot or not.
+  </td>
+</tr>
+
 <tr id="network_cni_metrics">
   <td>
     --[no-]network_cni_metrics
diff --git a/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp b/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
index f19edce..cc23428 100644
--- a/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
+++ b/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
@@ -142,19 +142,21 @@ Try<Isolator*> NetworkCniIsolatorProcess::create(const Flags& flags)
     return Error("Unable to load CNI config: " + networkConfigs.error());
   }
 
+  const string cniRootDir = paths::getCniRootDir(flags);
+
   // Create the CNI network information root directory if it does not exist.
-  Try<Nothing> mkdir = os::mkdir(paths::ROOT_DIR);
+  Try<Nothing> mkdir = os::mkdir(cniRootDir);
   if (mkdir.isError()) {
     return Error(
         "Failed to create CNI network information root directory at '" +
-        string(paths::ROOT_DIR) + "': " + mkdir.error());
+        cniRootDir + "': " + mkdir.error());
   }
 
-  Result<string> rootDir = os::realpath(paths::ROOT_DIR);
+  Result<string> rootDir = os::realpath(cniRootDir);
   if (!rootDir.isSome()) {
     return Error(
         "Failed to determine canonical path of CNI network information root"
-        " directory '" + string(paths::ROOT_DIR) + "': " +
+        " directory '" + cniRootDir + "': " +
         (rootDir.isError() ? rootDir.error() : "No such file or directory"));
   }
 
@@ -1303,11 +1305,13 @@ Future<Nothing> NetworkCniIsolatorProcess::attach(
         stringify(networkConfigJSON.get()) + "': " + checkpoint.error());
   }
 
-  VLOG(1) << "Invoking CNI plugin '" << plugin.get()
-          << "' with network configuration '"
+  LOG(INFO) << "Invoking CNI plugin '" << plugin.get()
+            << "' to attach container " << containerId
+            << " to network '" << networkName << "'";
+
+  VLOG(1) << "Using network configuration '"
           << stringify(networkConfigJSON.get())
-          << "' to attach container " << containerId << " to network '"
-          << networkName << "'";
+          << "' for container " << containerId;
 
   Try<Subprocess> s = subprocess(
       plugin.get(),
@@ -1775,10 +1779,12 @@ Future<Nothing> NetworkCniIsolatorProcess::detach(
         " to network '" + networkName + "'");
   }
 
-  VLOG(1) << "Invoking CNI plugin '" << plugin.get()
-          << "' with network configuration '" << networkConfigPath
-          << "' to detach container " << containerId << " from network '"
-          << networkName << "'";
+  LOG(INFO) << "Invoking CNI plugin '" << plugin.get()
+            << "' to detach container " << containerId
+            << " from network '" << networkName << "'";
+
+  VLOG(1) << "Using network configuration at '" << networkConfigPath
+          << "' for container " << containerId;
 
   Try<Subprocess> s = subprocess(
       plugin.get(),
diff --git a/src/slave/containerizer/mesos/isolators/network/cni/paths.cpp b/src/slave/containerizer/mesos/isolators/network/cni/paths.cpp
index c1cb4f7..5598b7d 100644
--- a/src/slave/containerizer/mesos/isolators/network/cni/paths.cpp
+++ b/src/slave/containerizer/mesos/isolators/network/cni/paths.cpp
@@ -31,6 +31,16 @@ namespace slave {
 namespace cni {
 namespace paths {
 
+string getCniRootDir(const Flags& flags)
+{
+  string workDir = flags.network_cni_root_dir_persist
+    ? flags.work_dir
+    : flags.runtime_dir;
+
+  return path::join(workDir, paths::CNI_DIR);
+}
+
+
 string getContainerDir(
     const string& rootDir,
     const ContainerID& containerId)
diff --git a/src/slave/containerizer/mesos/isolators/network/cni/paths.hpp b/src/slave/containerizer/mesos/isolators/network/cni/paths.hpp
index 3b06fb1..135a359 100644
--- a/src/slave/containerizer/mesos/isolators/network/cni/paths.hpp
+++ b/src/slave/containerizer/mesos/isolators/network/cni/paths.hpp
@@ -24,6 +24,8 @@
 
 #include <stout/try.hpp>
 
+#include "slave/flags.hpp"
+
 namespace mesos {
 namespace internal {
 namespace slave {
@@ -32,7 +34,7 @@ namespace paths {
 
 // The root directory where we keep the information of CNI networks that each
 // container joins. The layout is as follows:
-//   /var/run/mesos/isolators/network/cni/
+//   /<work_dir|runtime_dir>/isolators/network/cni/
 //    |- <ID of Container1>/
 //    |  |-- ns -> /proc/<pid>/ns/net (bind mount)
 //    |  |-- <Name of CNI network 1>/
@@ -45,7 +47,10 @@ namespace paths {
 //    |          |-- network.info
 //    |-- <ID of ContainerID 2>/
 //    | ...
-constexpr char ROOT_DIR[] = "/var/run/mesos/isolators/network/cni";
+constexpr char CNI_DIR[] = "isolators/network/cni";
+
+
+std::string getCniRootDir(const Flags& flags);
 
 
 std::string getContainerDir(
diff --git a/src/slave/flags.cpp b/src/slave/flags.cpp
index ccaf650..6bac8e1 100644
--- a/src/slave/flags.cpp
+++ b/src/slave/flags.cpp
@@ -1184,6 +1184,12 @@ mesos::internal::slave::Flags::Flags()
       "the operator should install a network configuration file in JSON\n"
       "format in the specified directory.");
 
+  add(&Flags::network_cni_root_dir_persist,
+      "network_cni_root_dir_persist",
+      "This setting controls whether the CNI root directory\n"
+      "persists across reboot or not.",
+      false);
+
   add(&Flags::network_cni_metrics,
       "network_cni_metrics",
       "This setting controls whether the networking metrics of the CNI\n"
diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp
index 29d8b79..494ae02 100644
--- a/src/slave/flags.hpp
+++ b/src/slave/flags.hpp
@@ -162,6 +162,7 @@ public:
 
   Option<std::string> network_cni_plugins_dir;
   Option<std::string> network_cni_config_dir;
+  bool network_cni_root_dir_persist;
   bool network_cni_metrics;
   Duration container_disk_watch_interval;
   bool enforce_container_disk_quota;
diff --git a/src/tests/containerizer/cni_isolator_tests.cpp b/src/tests/containerizer/cni_isolator_tests.cpp
index 5473737..eb20e63 100644
--- a/src/tests/containerizer/cni_isolator_tests.cpp
+++ b/src/tests/containerizer/cni_isolator_tests.cpp
@@ -483,24 +483,25 @@ TEST_F(CniIsolatorTest, ROOT_VerifyCheckpointedInfo)
   ContainerID containerId = *(containers->begin());
 
   // Check if the CNI related information is checkpointed successfully.
+  const string cniRootDir = paths::getCniRootDir(flags);
   const string containerDir =
-    paths::getContainerDir(paths::ROOT_DIR, containerId);
+    paths::getContainerDir(cniRootDir, containerId);
 
   EXPECT_TRUE(os::exists(containerDir));
   EXPECT_TRUE(os::exists(paths::getNetworkDir(
-      paths::ROOT_DIR, containerId, "__MESOS_TEST__")));
+      cniRootDir, containerId, "__MESOS_TEST__")));
 
   EXPECT_TRUE(os::exists(paths::getNetworkConfigPath(
-      paths::ROOT_DIR, containerId, "__MESOS_TEST__")));
+      cniRootDir, containerId, "__MESOS_TEST__")));
 
   EXPECT_TRUE(os::exists(paths::getInterfaceDir(
-      paths::ROOT_DIR, containerId, "__MESOS_TEST__", "eth0")));
+      cniRootDir, containerId, "__MESOS_TEST__", "eth0")));
 
   EXPECT_TRUE(os::exists(paths::getNetworkInfoPath(
-      paths::ROOT_DIR, containerId, "__MESOS_TEST__", "eth0")));
+      cniRootDir, containerId, "__MESOS_TEST__", "eth0")));
 
   EXPECT_TRUE(os::exists(paths::getNamespacePath(
-      paths::ROOT_DIR, containerId)));
+      cniRootDir, containerId)));
 
   EXPECT_TRUE(os::exists(path::join(containerDir, "hostname")));
   EXPECT_TRUE(os::exists(path::join(containerDir, "hosts")));
@@ -2537,6 +2538,44 @@ TEST_F(CniIsolatorTest, VETH_VerifyResourceStatistics)
   driver.join();
 }
 
+
+// This test verifies CNI root directory path.
+TEST_F(CniIsolatorTest, ROOT_VerifyCniRootDir)
+{
+  Try<Owned<cluster::Master>> master = StartMaster();
+  ASSERT_SOME(master);
+
+  slave::Flags flags = CreateSlaveFlags();
+  flags.isolation = "network/cni";
+
+  flags.network_cni_plugins_dir = cniPluginDir;
+  flags.network_cni_config_dir = cniConfigDir;
+
+  Owned<MasterDetector> detector = master.get()->createDetector();
+
+  Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), flags);
+  ASSERT_SOME(slave);
+
+  string cniRootDir = paths::getCniRootDir(flags);
+
+  ASSERT_EQ(path::join(flags.runtime_dir, paths::CNI_DIR), cniRootDir);
+  EXPECT_TRUE(os::exists(cniRootDir));
+
+  slave.get()->terminate();
+
+  // Enable the flag to test whether the directory
+  // has moved to a persistent location.
+  flags.network_cni_root_dir_persist = true;
+
+  slave = StartSlave(detector.get(), flags);
+  ASSERT_SOME(slave);
+
+  cniRootDir = paths::getCniRootDir(flags);
+
+  ASSERT_EQ(path::join(flags.work_dir, paths::CNI_DIR), cniRootDir);
+  EXPECT_TRUE(os::exists(cniRootDir));
+}
+
 } // namespace tests {
 } // namespace internal {
 } // namespace mesos {