You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jp...@apache.org on 2017/10/19 23:34:30 UTC

[09/11] mesos git commit: Ignored containers that join CNI networks.

Ignored containers that join CNI networks.

Working on the assumption that containers with CNI networks will
get their own IP addresses and don't need port isolation, ignore
any containers that are joining CNI networks.

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


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

Branch: refs/heads/master
Commit: 38b0bbb91957f35fb3d4c1cff18a57212c728ed2
Parents: 2fc779b
Author: James Peach <jp...@apache.org>
Authored: Thu Oct 19 15:35:58 2017 -0700
Committer: James Peach <jp...@apache.org>
Committed: Thu Oct 19 16:33:35 2017 -0700

----------------------------------------------------------------------
 src/slave/containerizer/mesos/containerizer.cpp | 15 ++--
 .../mesos/isolators/network/ports.cpp           | 87 ++++++++++++++++++--
 .../mesos/isolators/network/ports.hpp           |  2 +
 3 files changed, 93 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/38b0bbb9/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 78fdd21..4479817 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -235,17 +235,22 @@ Try<MesosContainerizer*> MesosContainerizer::create(
   }
 
 #ifdef __linux__
-  // One and only one `network` isolator is required. The network
-  // isolator is responsible for preparing the network namespace for
-  // containers. If the user does not specify one, 'network/cni'
-  // isolator will be used.
+
+  // The network isolator is responsible for preparing the network
+  // namespace for containers. In general, one and only one `network`
+  // isolator is required (e.g. `network/cni` and `network/port_mapping`
+  // cannot co-exist). However, since the `network/ports` isolator
+  // only deals with ports resources and does not configure any network
+  // namespaces, it doesn't count for the purposes of this check.
   switch (std::count_if(
       isolations->begin(),
       isolations->end(),
       [](const string& s) {
-        return strings::startsWith(s, "network/");
+        return strings::startsWith(s, "network/") && s != "network/ports";
       })) {
     case 0:
+      // If the user does not specify anything, the 'network/cni'
+      // isolator will be used.
       isolations->insert("network/cni");
       break;
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/38b0bbb9/src/slave/containerizer/mesos/isolators/network/ports.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/network/ports.cpp b/src/slave/containerizer/mesos/isolators/network/ports.cpp
index 243ba01..7d5ff4f 100644
--- a/src/slave/containerizer/mesos/isolators/network/ports.cpp
+++ b/src/slave/containerizer/mesos/isolators/network/ports.cpp
@@ -341,6 +341,7 @@ Try<Isolator*> NetworkPortsIsolatorProcess::create(const Flags& flags)
 
   return new MesosIsolator(process::Owned<MesosIsolatorProcess>(
       new NetworkPortsIsolatorProcess(
+          strings::contains(flags.isolation, "network/cni"),
           flags.container_ports_watch_interval,
           flags.cgroups_root,
           freezerHierarchy.get(),
@@ -349,11 +350,13 @@ Try<Isolator*> NetworkPortsIsolatorProcess::create(const Flags& flags)
 
 
 NetworkPortsIsolatorProcess::NetworkPortsIsolatorProcess(
+    bool _cniIsolatorEnabled,
     const Duration& _watchInterval,
     const string& _cgroupsRoot,
     const string& _freezerHierarchy,
     const Option<IntervalSet<uint16_t>>& _agentPorts)
   : ProcessBase(process::ID::generate("network-ports-isolator")),
+    cniIsolatorEnabled(_cniIsolatorEnabled),
     watchInterval(_watchInterval),
     cgroupsRoot(_cgroupsRoot),
     freezerHierarchy(_freezerHierarchy),
@@ -368,17 +371,74 @@ bool NetworkPortsIsolatorProcess::supportsNesting()
 }
 
 
+// Return whether this ContainerInfo has a NetworkInfo with a name. This
+// is our signal that the container is (or will be) joined to a CNI network.
+static bool hasNamedNetwork(const ContainerInfo& container_info)
+{
+  foreach (const auto& networkInfo, container_info.network_infos()) {
+    if (networkInfo.has_name()) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+
 Future<Nothing> NetworkPortsIsolatorProcess::recover(
     const list<ContainerState>& states,
     const hashset<ContainerID>& orphans)
 {
+  // First, recover all the root level containers.
+  foreach (const auto& state, states) {
+    if (state.container_id().has_parent()) {
+      continue;
+    }
+
+    CHECK(!infos.contains(state.container_id()))
+      << "Duplicate ContainerID " << state.container_id();
+
+    if (!cniIsolatorEnabled) {
+      infos.emplace(state.container_id(), Owned<Info>(new Info()));
+      update(state.container_id(), state.executor_info().resources());
+      continue;
+    }
+
+    // A root level container ought to always have an executor_info.
+    CHECK(state.has_executor_info());
+
+    // Ignore containers that will be network isolated by the
+    // `network/cni` isolator on the rationale that they ought
+    // to be getting a per-container IP address.
+    if (!state.executor_info().has_container() ||
+        !hasNamedNetwork(state.executor_info().container())) {
+      infos.emplace(state.container_id(), Owned<Info>(new Info()));
+
+      // Update the resources for this container so that we can isolate
+      // it from now until the executor re-registers and the containerizer
+      // sends us a new update.
+      update(state.container_id(), state.executor_info().resources());
+    }
+  }
+
+  // Now that we know which root level containers we are isolating, we can
+  // decide which child containers we also want.
   foreach (const auto& state, states) {
+    if (!state.container_id().has_parent()) {
+      continue;
+    }
+
     CHECK(!infos.contains(state.container_id()))
       << "Duplicate ContainerID " << state.container_id();
 
-    infos.emplace(state.container_id(), Owned<Info>(new Info()));
+    if (infos.contains(protobuf::getRootContainerId(state.container_id()))) {
+      infos.emplace(state.container_id(), Owned<Info>(new Info()));
+    }
   }
 
+  // We don't need to worry about any orphans since we don't have any state
+  // to clean up and we know the containerizer will destroy them soon.
+
   return Nothing();
 }
 
@@ -391,10 +451,25 @@ Future<Option<ContainerLaunchInfo>> NetworkPortsIsolatorProcess::prepare(
     return Failure("Container has already been prepared");
   }
 
-  // TODO(jpeach) Figure out how to ignore tasks that are not going to use the
-  // host network. If they are in a network namespace (CNI network) then
-  // there's no point restricting them and we would have to implement any
-  // restructions by entering the right namespaces anyway.
+  if (cniIsolatorEnabled) {
+    // A nested container implicitly joins a parent CNI network. The
+    // network configuration is always set from the top of the tree of
+    // nested containers, so we know that we should only isolate the
+    // child if we already have the root of the container tree.
+    if (containerId.has_parent()) {
+      if (!infos.contains(protobuf::getRootContainerId(containerId))) {
+        return None();
+      }
+    } else {
+      // Ignore containers that will be network isolated by the
+      // `network/cni` isolator on the rationale that they ought
+      // to be getting a per-container IP address.
+      if (containerConfig.has_container_info() &&
+          hasNamedNetwork(containerConfig.container_info())) {
+        return None();
+      }
+    }
+  }
 
   infos.emplace(containerId, Owned<Info>(new Info()));
 
@@ -504,7 +579,7 @@ Future<Nothing> NetworkPortsIsolatorProcess::check(
 
       LOG(INFO) << message;
 
-      infos.at(containerId)->limitation.set(
+      info->limitation.set(
           protobuf::slave::createContainerLimitation(
               Resources(resource),
               message,

http://git-wip-us.apache.org/repos/asf/mesos/blob/38b0bbb9/src/slave/containerizer/mesos/isolators/network/ports.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/network/ports.hpp b/src/slave/containerizer/mesos/isolators/network/ports.hpp
index 6ce35e7..ba71087 100644
--- a/src/slave/containerizer/mesos/isolators/network/ports.hpp
+++ b/src/slave/containerizer/mesos/isolators/network/ports.hpp
@@ -83,6 +83,7 @@ protected:
 
 private:
   NetworkPortsIsolatorProcess(
+      bool _cniIsolatorEnabled,
       const Duration& _watchInterval,
       const std::string& _cgroupsRoot,
       const std::string& _freezerHierarchy,
@@ -94,6 +95,7 @@ private:
     process::Promise<mesos::slave::ContainerLimitation> limitation;
   };
 
+  const bool cniIsolatorEnabled;
   const Duration watchInterval;
   const std::string cgroupsRoot;
   const std::string freezerHierarchy;