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__