[mesos] 02/02: Removed unneeded pluginDir field from CNI isolator.

[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 84b0a51e7174885f45b155ef912772c2593fc398
Author: Jie Yu <yu...@gmail.com>
AuthorDate: Wed Sep 26 21:57:14 2018 -0700

    Removed unneeded pluginDir field from CNI isolator.
    
    The field member is redundant as it's already included in the flags.
    
    Review: https://reviews.apache.org/r/68862
---
 .../mesos/isolators/network/cni/cni.cpp            | 32 ++++++++++++----------
 .../mesos/isolators/network/cni/cni.hpp            |  9 ++----
 2 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp b/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
index 8af853c..ba46552 100644
--- a/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
+++ b/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
@@ -289,8 +289,7 @@ Try<Isolator*> NetworkCniIsolatorProcess::create(const Flags& flags)
           networkConfigs.get(),
           cniDNSMap,
           defaultCniDNS,
-          rootDir.get(),
-          flags.network_cni_plugins_dir.get())));
+          rootDir.get())));
 }
 
 
@@ -860,12 +859,11 @@ Future<Nothing> NetworkCniIsolatorProcess::isolate(
   // level or nested) wants to join non-host networks.
 
   // If the `network/cni` isolator is providing network isolation to a
-  // container its `rootDir` and `pluginDir` should always be set.
-  // These properties of the isolator will not be set only if the
-  // operator does not specify the '--network_cni_plugins_dir' and
-  // '--network_cni_config_dir' flags at agent startup.
+  // container its `rootDir` should always be set.  It will not be set
+  // only if the operator does not specify the
+  // '--network_cni_plugins_dir' and '--network_cni_config_dir' flags
+  // at agent startup.
   CHECK_SOME(rootDir);
-  CHECK_SOME(pluginDir);
 
   // NOTE: DEBUG container should not have Info struct. Thus if the
   // control reaches here, the container is not a DEBUG container.
@@ -1216,10 +1214,12 @@ Future<Nothing> NetworkCniIsolatorProcess::attach(
   }
 
   // Prepare environment variables for CNI plugin.
+  CHECK_SOME(flags.network_cni_plugins_dir);
+
   map<string, string> environment;
   environment["CNI_COMMAND"] = "ADD";
   environment["CNI_CONTAINERID"] = stringify(containerId);
-  environment["CNI_PATH"] = pluginDir.get();
+  environment["CNI_PATH"] = flags.network_cni_plugins_dir.get();
   environment["CNI_IFNAME"] = containerNetwork.ifName;
   environment["CNI_NETNS"] = netNsHandle;
 
@@ -1264,8 +1264,8 @@ Future<Nothing> NetworkCniIsolatorProcess::attach(
 
   // Invoke the CNI plugin.
   //
-  // NOTE: We want to execute only the plugin found in the `pluginDir`
-  // path specified by the operator.
+  // NOTE: We want to execute only the plugin found in the
+  // `flags.network_cni_plugins_dir` path specified by the operator.
   Result<JSON::String> _plugin = networkConfigJSON->at<JSON::String>("type");
   if (!_plugin.isSome()) {
     return Failure(
@@ -1277,7 +1277,7 @@ Future<Nothing> NetworkCniIsolatorProcess::attach(
 
   Option<string> plugin = os::which(
       _plugin->value,
-      pluginDir.get());
+      flags.network_cni_plugins_dir.get());
 
   if (plugin.isNone()) {
     return Failure(
@@ -1699,10 +1699,12 @@ Future<Nothing> NetworkCniIsolatorProcess::detach(
     infos[containerId]->containerNetworks[networkName];
 
   // Prepare environment variables for CNI plugin.
+  CHECK_SOME(flags.network_cni_plugins_dir);
+
   map<string, string> environment;
   environment["CNI_COMMAND"] = "DEL";
   environment["CNI_CONTAINERID"] = stringify(containerId);
-  environment["CNI_PATH"] = pluginDir.get();
+  environment["CNI_PATH"] = flags.network_cni_plugins_dir.get();
   environment["CNI_IFNAME"] = containerNetwork.ifName;
   environment["CNI_NETNS"] =
       paths::getNamespacePath(rootDir.get(), containerId);
@@ -1760,11 +1762,11 @@ Future<Nothing> NetworkCniIsolatorProcess::detach(
 
   // Invoke the CNI plugin.
   //
-  // NOTE: We want to execute only the plugin found in the `pluginDir`
-  // path specified by the operator.
+  // NOTE: We want to execute only the plugin found in the
+  // `flags.network_cni_plugins_dir` path specified by the operator.
   Option<string> plugin = os::which(
       _plugin->value,
-      pluginDir.get());
+      flags.network_cni_plugins_dir.get());
 
   if (plugin.isNone()) {
     return Failure(
diff --git a/src/slave/containerizer/mesos/isolators/network/cni/cni.hpp b/src/slave/containerizer/mesos/isolators/network/cni/cni.hpp
index 4ac55c8..0e23a26 100644
--- a/src/slave/containerizer/mesos/isolators/network/cni/cni.hpp
+++ b/src/slave/containerizer/mesos/isolators/network/cni/cni.hpp
@@ -135,15 +135,13 @@ private:
       const hashmap<std::string, std::string>& _networkConfigs,
       const hashmap<std::string, ContainerDNSInfo::MesosInfo>& _cniDNSMap,
       const Option<ContainerDNSInfo::MesosInfo>& _defaultCniDNS = None(),
-      const Option<std::string>& _rootDir = None(),
-      const Option<std::string>& _pluginDir = None())
+      const Option<std::string>& _rootDir = None())
     : ProcessBase(process::ID::generate("mesos-network-cni-isolator")),
       flags(_flags),
       networkConfigs(_networkConfigs),
       cniDNSMap(_cniDNSMap),
       defaultCniDNS(_defaultCniDNS),
-      rootDir(_rootDir),
-      pluginDir(_pluginDir) {}
+      rootDir(_rootDir) {}
 
   process::Future<Nothing> _isolate(
       const ContainerID& containerId,
@@ -221,9 +219,6 @@ private:
   // CNI network information root directory.
   const Option<std::string> rootDir;
 
-  // CNI plugins directory.
-  const Option<std::string> pluginDir;
-
   // Information of CNI networks that each container joins.
   hashmap<ContainerID, process::Owned<Info>> infos;