Repository: mesos
Updated Branches:
  refs/heads/master e71cb47c4 -> 8b27afc9b


Updated the generic 'cgroups' isolator to be nested container aware.

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


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

Branch: refs/heads/master
Commit: 442987e9f1a5742ddba9291044d82bed5200ec4e
Parents: e71cb47
Author: Kevin Klues <klue...@gmail.com>
Authored: Fri Sep 16 14:58:28 2016 -0700
Committer: Jie Yu <yujie....@gmail.com>
Committed: Fri Sep 16 14:58:28 2016 -0700

----------------------------------------------------------------------
 src/Makefile.am                                 |  1 +
 .../mesos/isolators/cgroups/cgroups.cpp         | 66 ++++++++++++++++++--
 src/slave/containerizer/mesos/utils.hpp         | 40 ++++++++++++
 3 files changed, 103 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/442987e9/src/Makefile.am
----------------------------------------------------------------------
diff --git a/src/Makefile.am b/src/Makefile.am
index 44fd8a2..6fb095f 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -951,6 +951,7 @@ libmesos_no_3rdparty_la_SOURCES +=                          
        \
   slave/containerizer/mesos/launch.hpp                                 \
   slave/containerizer/mesos/launcher.hpp                               \
   slave/containerizer/mesos/mount.hpp                                  \
+  slave/containerizer/mesos/utils.hpp                                  \
   slave/containerizer/mesos/isolators/posix.hpp                                
\
   slave/containerizer/mesos/isolators/filesystem/posix.hpp             \
   slave/containerizer/mesos/isolators/filesystem/windows.hpp           \

http://git-wip-us.apache.org/repos/asf/mesos/blob/442987e9/src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
b/src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
index e87d055..ecfa5db 100644
--- a/src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
+++ b/src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
@@ -30,6 +30,8 @@
 
 #include "linux/cgroups.hpp"
 
+#include "slave/containerizer/mesos/utils.hpp"
+
 #include "slave/containerizer/mesos/isolators/cgroups/cgroups.hpp"
 #include "slave/containerizer/mesos/isolators/cgroups/constants.hpp"
 
@@ -162,6 +164,13 @@ Future<Nothing> CgroupsIsolatorProcess::recover(
   // Recover active containers first.
   list<Future<Nothing>> recovers;
   foreach (const ContainerState& state, states) {
+    // If we are a nested container, we do not need to recover
+    // anything since only top-level containers will have cgroups
+    // created for them.
+    if (state.container_id().has_parent()) {
+      continue;
+    }
+
     recovers.push_back(___recover(state.container_id()));
   }
 
@@ -358,6 +367,13 @@ Future<Option<ContainerLaunchInfo>> 
CgroupsIsolatorProcess::prepare(
     const ContainerID& containerId,
     const ContainerConfig& containerConfig)
 {
+  // If we are a nested container, we do not need to prepare
+  // anything since only top-level containers should have cgroups
+  // created for them.
+  if (containerId.has_parent()) {
+    return None();
+  }
+
   if (infos.contains(containerId)) {
     return Failure("Container has already been prepared");
   }
@@ -492,21 +508,25 @@ Future<Nothing> CgroupsIsolatorProcess::isolate(
     const ContainerID& containerId,
     pid_t pid)
 {
-  if (!infos.contains(containerId)) {
-    return Failure("Failed to isolate the container: Unknown container");
+  // If we are a nested container, we inherit
+  // the cgroup from our root ancestor.
+  ContainerID rootContainerId = getRootContainerId(containerId);
+
+  if (!infos.contains(rootContainerId)) {
+    return Failure("Failed to isolate the container: Unknown root container");
   }
 
   // TODO(haosdent): Use foreachkey once MESOS-5037 is resolved.
   foreach (const string& hierarchy, subsystems.keys()) {
     Try<Nothing> assign = cgroups::assign(
         hierarchy,
-        infos[containerId]->cgroup,
+        infos[rootContainerId]->cgroup,
         pid);
 
     if (assign.isError()) {
       string message =
         "Failed to assign pid " + stringify(pid) + " to cgroup at "
-        "'" + path::join(hierarchy, infos[containerId]->cgroup) + "'"
+        "'" + path::join(hierarchy, infos[rootContainerId]->cgroup) + "'"
         ": " + assign.error();
 
       LOG(ERROR) << message;
@@ -515,6 +535,22 @@ Future<Nothing> CgroupsIsolatorProcess::isolate(
     }
   }
 
+  // We currently can't call `subsystem->isolate()` on nested
+  // containers, because we don't call `prepare()`, `recover()`, or
+  // `cleanup()` on them either. If we were to call `isolate()` on
+  // them, the call would likely fail because the subsystem doesn't
+  // know about the container. This is currently OK because the only
+  // cgroup isolator that even implements `isolate()` is the
+  // `NetClsSubsystem` and it doesn't do anything with the `pid`
+  // passed in.
+  //
+  // TODO(klueska): In the future we should revisit this to make
+  // sure that doing things this way is sufficient (or otherwise
+  // update our invariants to allow us to call this here).
+  if (containerId.has_parent()) {
+    return Nothing();
+  }
+
   list<Future<Nothing>> isolates;
   foreachvalue (const Owned<Subsystem>& subsystem, subsystems) {
     isolates.push_back(subsystem->isolate(containerId, pid));
@@ -553,6 +589,10 @@ Future<Nothing> CgroupsIsolatorProcess::_isolate(
 Future<ContainerLimitation> CgroupsIsolatorProcess::watch(
     const ContainerID& containerId)
 {
+  if (containerId.has_parent()) {
+    return Failure("Not supported for nested containers");
+  }
+
   if (!infos.contains(containerId)) {
     return Failure("Unknown container");
   }
@@ -588,6 +628,10 @@ Future<Nothing> CgroupsIsolatorProcess::update(
     const ContainerID& containerId,
     const Resources& resources)
 {
+  if (containerId.has_parent()) {
+    return Failure("Not supported for nested containers");
+  }
+
   if (!infos.contains(containerId)) {
     return Failure("Unknown container");
   }
@@ -630,6 +674,10 @@ Future<Nothing> CgroupsIsolatorProcess::_update(
 Future<ResourceStatistics> CgroupsIsolatorProcess::usage(
     const ContainerID& containerId)
 {
+  if (containerId.has_parent()) {
+    return Failure("Not supported for nested containers");
+  }
+
   if (!infos.contains(containerId)) {
     return Failure("Unknown container");
   }
@@ -662,6 +710,10 @@ Future<ResourceStatistics> CgroupsIsolatorProcess::usage(
 Future<ContainerStatus> CgroupsIsolatorProcess::status(
     const ContainerID& containerId)
 {
+  if (containerId.has_parent()) {
+    return Failure("Not supported for nested containers");
+  }
+
   if (!infos.contains(containerId)) {
     return Failure("Unknown container");
   }
@@ -693,6 +745,12 @@ Future<ContainerStatus> CgroupsIsolatorProcess::status(
 Future<Nothing> CgroupsIsolatorProcess::cleanup(
     const ContainerID& containerId)
 {
+  // If we are a nested container, we do not need to clean anything up
+  // since only top-level containers should have cgroups created for them.
+  if (containerId.has_parent()) {
+    return Nothing();
+  }
+
   if (!infos.contains(containerId)) {
     VLOG(1) << "Ignoring cleanup request for unknown container " << 
containerId;
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/442987e9/src/slave/containerizer/mesos/utils.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/utils.hpp 
b/src/slave/containerizer/mesos/utils.hpp
new file mode 100644
index 0000000..2bb55c1
--- /dev/null
+++ b/src/slave/containerizer/mesos/utils.hpp
@@ -0,0 +1,40 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef __MESOS_CONTAINERIZER_UTILS_HPP__
+#define __MESOS_CONTAINERIZER_UTILS_HPP__
+
+#include <mesos/mesos.hpp>
+
+namespace mesos {
+namespace internal {
+namespace slave {
+
+static ContainerID getRootContainerId(const ContainerID& containerId)
+{
+  ContainerID rootContainerId = containerId;
+  while (rootContainerId.has_parent()) {
+    rootContainerId = rootContainerId.parent();
+  }
+
+  return rootContainerId;
+}
+
+} // namespace slave {
+} // namespace internal {
+} // namespace mesos {
+
+#endif // __MESOS_CONTAINERIZER_UTILS_HPP__

Reply via email to