You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bm...@apache.org on 2016/06/09 21:13:36 UTC
[2/2] mesos git commit: Created a `cgroups/devices` isolator.
Created a `cgroups/devices` isolator.
Previously, all the logic for the `cgroups/devices` isolator was
bundled into the Nvidia GPU Isolator. Now we have abstracted it out
into it's own component and removed the redundant logic from the
Nvidia GPU Isolator. Because of the new guaranteed ordering between
isolators, we can be sure that the dependency order between the
`cgroups/devices` and `gpu/nvidia` isolators is met.
Review: https://reviews.apache.org/r/48478/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/1d60fdaf
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/1d60fdaf
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/1d60fdaf
Branch: refs/heads/master
Commit: 1d60fdafe6abfa29ff147bd5a56efc4968c7d637
Parents: 6fb9c02
Author: Kevin Klues <kl...@gmail.com>
Authored: Thu Jun 9 13:41:09 2016 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Thu Jun 9 14:09:54 2016 -0700
----------------------------------------------------------------------
src/CMakeLists.txt | 1 +
src/Makefile.am | 2 +
src/slave/containerizer/mesos/containerizer.cpp | 12 +-
.../mesos/isolators/cgroups/devices.cpp | 370 +++++++++++++++++++
.../mesos/isolators/cgroups/devices.hpp | 98 +++++
.../isolators/cgroups/devices/gpus/nvidia.cpp | 204 +---------
.../isolators/cgroups/devices/gpus/nvidia.hpp | 4 -
.../containerizer/nvidia_gpu_isolator_tests.cpp | 4 +-
8 files changed, 494 insertions(+), 201 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/1d60fdaf/src/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 2c59a42..038c805 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -148,6 +148,7 @@ set(LINUX_SRC
linux/systemd.cpp
slave/containerizer/mesos/linux_launcher.cpp
slave/containerizer/mesos/isolators/cgroups/cpushare.cpp
+ slave/containerizer/mesos/isolators/cgroups/devices.cpp
slave/containerizer/mesos/isolators/cgroups/mem.cpp
slave/containerizer/mesos/isolators/cgroups/net_cls.cpp
slave/containerizer/mesos/isolators/cgroups/perf_event.cpp
http://git-wip-us.apache.org/repos/asf/mesos/blob/1d60fdaf/src/Makefile.am
----------------------------------------------------------------------
diff --git a/src/Makefile.am b/src/Makefile.am
index 29525c9..b656702 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1001,6 +1001,7 @@ MESOS_LINUX_FILES = \
linux/systemd.cpp \
slave/containerizer/mesos/linux_launcher.cpp \
slave/containerizer/mesos/isolators/cgroups/cpushare.cpp \
+ slave/containerizer/mesos/isolators/cgroups/devices.cpp \
slave/containerizer/mesos/isolators/cgroups/mem.cpp \
slave/containerizer/mesos/isolators/cgroups/net_cls.cpp \
slave/containerizer/mesos/isolators/cgroups/perf_event.cpp \
@@ -1025,6 +1026,7 @@ MESOS_LINUX_FILES += \
slave/containerizer/mesos/linux_launcher.hpp \
slave/containerizer/mesos/isolators/cgroups/constants.hpp \
slave/containerizer/mesos/isolators/cgroups/cpushare.hpp \
+ slave/containerizer/mesos/isolators/cgroups/devices.hpp \
slave/containerizer/mesos/isolators/cgroups/mem.hpp \
slave/containerizer/mesos/isolators/cgroups/net_cls.hpp \
slave/containerizer/mesos/isolators/cgroups/perf_event.hpp \
http://git-wip-us.apache.org/repos/asf/mesos/blob/1d60fdaf/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index b151121..7fae1ef 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -69,6 +69,7 @@
#ifdef __linux__
#include "slave/containerizer/mesos/isolators/cgroups/cpushare.hpp"
+#include "slave/containerizer/mesos/isolators/cgroups/devices.hpp"
#include "slave/containerizer/mesos/isolators/cgroups/mem.hpp"
#include "slave/containerizer/mesos/isolators/cgroups/net_cls.hpp"
#include "slave/containerizer/mesos/isolators/cgroups/perf_event.hpp"
@@ -267,6 +268,10 @@ Try<MesosContainerizer*> MesosContainerizer::create(
// (1) The filesystem isolator must be the first isolator so
// that the runtime isolators have a consistent view of
// the prepared filesystem (e.g. volume mounts).
+ //
+ // (2) The Nvidia GPU isolator must come after the cgroups
+ // devices isolator, as it relies on the 'devices' cgroup
+ // being set up.
const vector<pair<string, lambda::function<Try<Isolator*>(const Flags&)>>>
creators = {
@@ -300,14 +305,15 @@ Try<MesosContainerizer*> MesosContainerizer::create(
#endif // __WINDOWS__
#ifdef __linux__
{"cgroups/cpu", &CgroupsCpushareIsolatorProcess::create},
+ {"cgroups/devices", &CgroupsDevicesIsolatorProcess::create},
{"cgroups/mem", &CgroupsMemIsolatorProcess::create},
{"cgroups/net_cls", &CgroupsNetClsIsolatorProcess::create},
{"cgroups/perf_event", &CgroupsPerfEventIsolatorProcess::create},
-#ifdef ENABLE_NVIDIA_GPU_SUPPORT
- {"cgroups/devices/gpus/nvidia", &CgroupsNvidiaGpuIsolatorProcess::create},
-#endif
{"docker/runtime", &DockerRuntimeIsolatorProcess::create},
{"docker/volume", &DockerVolumeIsolatorProcess::create},
+#ifdef ENABLE_NVIDIA_GPU_SUPPORT
+ {"gpu/nvidia", &CgroupsNvidiaGpuIsolatorProcess::create},
+#endif
{"namespaces/pid", &NamespacesPidIsolatorProcess::create},
{"network/cni", &NetworkCniIsolatorProcess::create},
#endif // __linux__
http://git-wip-us.apache.org/repos/asf/mesos/blob/1d60fdaf/src/slave/containerizer/mesos/isolators/cgroups/devices.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/cgroups/devices.cpp b/src/slave/containerizer/mesos/isolators/cgroups/devices.cpp
new file mode 100644
index 0000000..16f9628
--- /dev/null
+++ b/src/slave/containerizer/mesos/isolators/cgroups/devices.cpp
@@ -0,0 +1,370 @@
+// 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.
+
+#include <stdint.h>
+
+#include <list>
+#include <set>
+#include <string>
+#include <vector>
+
+#include <process/defer.hpp>
+#include <process/future.hpp>
+#include <process/owned.hpp>
+#include <process/shared.hpp>
+
+#include <stout/error.hpp>
+#include <stout/foreach.hpp>
+#include <stout/hashset.hpp>
+#include <stout/nothing.hpp>
+#include <stout/option.hpp>
+#include <stout/os.hpp>
+#include <stout/try.hpp>
+
+#include "linux/cgroups.hpp"
+
+#include "slave/flags.hpp"
+
+#include "slave/containerizer/containerizer.hpp"
+
+#include "slave/containerizer/mesos/isolator.hpp"
+
+#include "slave/containerizer/mesos/isolators/cgroups/devices.hpp"
+
+using cgroups::devices::Entry;
+
+using mesos::slave::ContainerConfig;
+using mesos::slave::ContainerLaunchInfo;
+using mesos::slave::ContainerLimitation;
+using mesos::slave::ContainerState;
+using mesos::slave::Isolator;
+
+using mesos::internal::slave::Containerizer;
+
+using process::Failure;
+using process::Future;
+using process::PID;
+using process::Shared;
+
+using std::list;
+using std::set;
+using std::string;
+using std::vector;
+
+namespace mesos {
+namespace internal {
+namespace slave {
+
+// The default list of devices to whitelist when device isolation is
+// turned on. The full list of devices can be found here:
+// https://www.kernel.org/doc/Documentation/devices.txt
+//
+// Device whitelisting is described here:
+// https://www.kernel.org/doc/Documentation/cgroup-v1/devices.txt
+static const char* DEFAULT_WHITELIST_ENTRIES[] = {
+ "c *:* m", // Make new character devices.
+ "b *:* m", // Make new block devices.
+ "c 5:1 rwm", // /dev/console
+ "c 4:0 rwm", // /dev/tty0
+ "c 4:1 rwm", // /dev/tty1
+ "c 136:* rwm", // /dev/pts/*
+ "c 5:2 rwm", // /dev/ptmx
+ "c 10:200 rwm", // /dev/net/tun
+ "c 1:3 rwm", // /dev/null
+ "c 1:5 rwm", // /dev/zero
+ "c 1:7 rwm", // /dev/full
+ "c 5:0 rwm", // /dev/tty
+ "c 1:9 rwm", // /dev/urandom
+ "c 1:8 rwm", // /dev/random
+};
+
+
+CgroupsDevicesIsolatorProcess::CgroupsDevicesIsolatorProcess(
+ const Flags& _flags,
+ const string& _hierarchy)
+ : flags(_flags),
+ hierarchy(_hierarchy) {}
+
+
+Try<Isolator*> CgroupsDevicesIsolatorProcess::create(const Flags& flags)
+{
+ // Prepare the cgroups device hierarchy.
+ Try<string> hierarchy = cgroups::prepare(
+ flags.cgroups_hierarchy,
+ "devices",
+ flags.cgroups_root);
+
+ if (hierarchy.isError()) {
+ return Error("Failed to prepare hierarchy for 'devices' subsystem: " +
+ hierarchy.error());
+ }
+
+ // Ensure that no other subsystem is attached to the hierarchy.
+ Try<set<string>> subsystems = cgroups::subsystems(hierarchy.get());
+ if (subsystems.isError()) {
+ return Error("Failed to get subsystems attached to hierarchy"
+ " '" + stringify(hierarchy.get()) + "': " +
+ subsystems.error());
+ } else if (subsystems->size() != 1) {
+ return Error("Unexpected subsystems attached to hierarchy"
+ " '" + stringify(hierarchy.get()) + "': " +
+ stringify(subsystems.get()));
+ }
+
+ process::Owned<MesosIsolatorProcess> process(
+ new CgroupsDevicesIsolatorProcess(
+ flags,
+ hierarchy.get()));
+
+ return new MesosIsolator(process);
+}
+
+
+Future<Nothing> CgroupsDevicesIsolatorProcess::recover(
+ const list<ContainerState>& states,
+ const hashset<ContainerID>& orphans)
+{
+ foreach (const ContainerState& state, states) {
+ const ContainerID& containerId = state.container_id();
+ const string cgroup = path::join(flags.cgroups_root, containerId.value());
+
+ Try<bool> exists = cgroups::exists(hierarchy, cgroup);
+ if (exists.isError()) {
+ foreachvalue (Info* info, infos) {
+ delete info;
+ }
+ infos.clear();
+ return Failure("Failed to check cgroup for container '" +
+ stringify(containerId) + "'");
+ }
+
+ if (!exists.get()) {
+ // This may occur if the executor has exited and the isolator
+ // has destroyed the cgroup but the slave dies before noticing
+ // this. This will be detected when the containerizer tries to
+ // monitor the executor's pid.
+ VLOG(1) << "Couldn't find cgroup for container " << containerId;
+ continue;
+ }
+
+ infos[containerId] = new Info(containerId, cgroup);
+ }
+
+ // Remove orphan cgroups.
+ Try<vector<string>> cgroups = cgroups::get(hierarchy, flags.cgroups_root);
+ if (cgroups.isError()) {
+ foreachvalue (Info* info, infos) {
+ delete info;
+ }
+ infos.clear();
+ return Failure(cgroups.error());
+ }
+
+ foreach (const string& cgroup, cgroups.get()) {
+ // Ignore the slave cgroup (see the --agent_subsystems flag).
+ // TODO(idownes): Remove this when the cgroups layout is updated,
+ // see MESOS-1185.
+ if (cgroup == path::join(flags.cgroups_root, "slave")) {
+ continue;
+ }
+
+ ContainerID containerId;
+ containerId.set_value(Path(cgroup).basename());
+
+ if (infos.contains(containerId)) {
+ continue;
+ }
+
+ // Known orphan cgroups will be destroyed by the containerizer
+ // using the normal cleanup path. See MESOS-2367 for details.
+ if (orphans.contains(containerId)) {
+ infos[containerId] = new Info(containerId, cgroup);
+ continue;
+ }
+
+ LOG(INFO) << "Removing unknown orphaned cgroup '" << cgroup << "'";
+
+ // We don't wait on the destroy as we don't want to block recovery.
+ cgroups::destroy(hierarchy, cgroup, cgroups::DESTROY_TIMEOUT);
+ }
+
+ return Nothing();
+}
+
+
+Future<Option<ContainerLaunchInfo>> CgroupsDevicesIsolatorProcess::prepare(
+ const ContainerID& containerId,
+ const mesos::slave::ContainerConfig& containerConfig)
+{
+ if (infos.contains(containerId)) {
+ return Failure("Container has already been prepared");
+ }
+
+ // TODO(bmahler): Don't insert into 'infos' unless we create the
+ // cgroup successfully. It's safe for now because 'cleanup' gets
+ // called if we return a Failure, but cleanup will fail because the
+ // cgroup does not exist when cgroups::destroy is called.
+ Info* info = new Info(
+ containerId, path::join(flags.cgroups_root, containerId.value()));
+
+ infos[containerId] = info;
+
+ Try<bool> exists = cgroups::exists(hierarchy, info->cgroup);
+ if (exists.isError()) {
+ return Failure("Failed to prepare isolator: " + exists.error());
+ } else if (exists.get()) {
+ return Failure("Failed to prepare isolator: cgroup already exists");
+ }
+
+ Try<Nothing> create = cgroups::create(hierarchy, info->cgroup);
+ if (create.isError()) {
+ return Failure("Failed to prepare isolator: " + create.error());
+ }
+
+ // Chown the cgroup so the executor can create nested cgroups. Do
+ // not recurse so the control files are still owned by the slave
+ // user and thus cannot be changed by the executor.
+ if (containerConfig.has_user()) {
+ Try<Nothing> chown = os::chown(
+ containerConfig.user(),
+ path::join(hierarchy, info->cgroup),
+ false);
+ if (chown.isError()) {
+ return Failure("Failed to prepare isolator: " + chown.error());
+ }
+ }
+
+ // When a devices cgroup is first created, its whitelist inherits
+ // all devices from its parent's whitelist (i.e., "a *:* rwm" by
+ // default). In theory, we should be able to add and remove devices
+ // from the whitelist by writing to the respective `devices.allow`
+ // and `devices.deny` files associated with the cgroup. However, the
+ // semantics of the whitelist are such that writing to the deny file
+ // will only remove entries in the whitelist that are explicitly
+ // listed in there (i.e., denying "b 1:3 rwm" when the whitelist
+ // only contains "a *:* rwm" will not modify the whitelist because
+ // "b 1:3 rwm" is not explicitly listed). Although the whitelist
+ // doesn't change, access to the device is still denied as expected
+ // (there is just no way of querying the system to detect it).
+ // Because of this, we first deny access to all devices and
+ // selectively add some back in so we can control the entries in the
+ // whitelist explicitly.
+ cgroups::devices::Entry all;
+ all.selector.type = Entry::Selector::Type::ALL;
+ all.selector.major = None();
+ all.selector.minor = None();
+ all.access.read = true;
+ all.access.write = true;
+ all.access.mknod = true;
+
+ Try<Nothing> deny = cgroups::devices::deny(hierarchy, info->cgroup, all);
+
+ if (deny.isError()) {
+ return Failure("Failed to deny all devices: " + deny.error());
+ }
+
+ foreach (const char* _entry, DEFAULT_WHITELIST_ENTRIES) {
+ Try<cgroups::devices::Entry> entry =
+ cgroups::devices::Entry::parse(_entry);
+
+ CHECK_SOME(entry);
+
+ Try<Nothing> allow =
+ cgroups::devices::allow(hierarchy, info->cgroup, entry.get());
+
+ if (allow.isError()) {
+ return Failure("Failed to whitelist default device"
+ " '" + stringify(entry.get()) + "': " + allow.error());
+ }
+ }
+
+ return update(containerId, containerConfig.executor_info().resources())
+ .then([]() -> Future<Option<ContainerLaunchInfo>> {
+ return None();
+ });
+}
+
+
+Future<Nothing> CgroupsDevicesIsolatorProcess::isolate(
+ const ContainerID& containerId,
+ pid_t pid)
+{
+ if (!infos.contains(containerId)) {
+ return Failure("Unknown container");
+ }
+
+ Info* info = CHECK_NOTNULL(infos[containerId]);
+
+ Try<Nothing> assign = cgroups::assign(
+ hierarchy, info->cgroup, pid);
+
+ if (assign.isError()) {
+ LOG(ERROR) << "Failed to assign container '" << info->containerId << "'"
+ << " to cgroup '" << path::join(hierarchy, info->cgroup) << "': "
+ << assign.error();
+
+ return Failure("Failed to isolate container: " + assign.error());
+ }
+
+ return Nothing();
+}
+
+
+Future<Nothing> CgroupsDevicesIsolatorProcess::cleanup(
+ const ContainerID& containerId)
+{
+ // Multiple calls may occur during test clean up.
+ if (!infos.contains(containerId)) {
+ VLOG(1) << "Ignoring cleanup request for unknown container " << containerId;
+
+ return Nothing();
+ }
+
+ Info* info = CHECK_NOTNULL(infos[containerId]);
+
+ return cgroups::destroy(hierarchy, info->cgroup, cgroups::DESTROY_TIMEOUT)
+ .onAny(defer(PID<CgroupsDevicesIsolatorProcess>(this),
+ &CgroupsDevicesIsolatorProcess::_cleanup,
+ containerId,
+ lambda::_1));
+}
+
+
+Future<Nothing> CgroupsDevicesIsolatorProcess::_cleanup(
+ const ContainerID& containerId,
+ const Future<Nothing>& future)
+{
+ if (!infos.contains(containerId)) {
+ return Failure("Unknown container");
+ }
+
+ Info* info = CHECK_NOTNULL(infos[containerId]);
+
+ if (!future.isReady()) {
+ return Failure(
+ "Failed to clean up container " + stringify(containerId) + ": " +
+ (future.isFailed() ? future.failure() : "discarded"));
+ }
+
+ delete info;
+ infos.erase(containerId);
+
+ return Nothing();
+}
+
+} // namespace slave {
+} // namespace internal {
+} // namespace mesos {
http://git-wip-us.apache.org/repos/asf/mesos/blob/1d60fdaf/src/slave/containerizer/mesos/isolators/cgroups/devices.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/cgroups/devices.hpp b/src/slave/containerizer/mesos/isolators/cgroups/devices.hpp
new file mode 100644
index 0000000..7ad9644
--- /dev/null
+++ b/src/slave/containerizer/mesos/isolators/cgroups/devices.hpp
@@ -0,0 +1,98 @@
+// 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 __CGROUPS_DEVICES_ISOLATOR_HPP__
+#define __CGROUPS_DEVICES_ISOLATOR_HPP__
+
+#include <list>
+#include <string>
+
+#include <process/future.hpp>
+
+#include <stout/hashmap.hpp>
+#include <stout/hashset.hpp>
+#include <stout/nothing.hpp>
+#include <stout/option.hpp>
+#include <stout/try.hpp>
+
+#include "linux/cgroups.hpp"
+
+#include "slave/flags.hpp"
+
+#include "slave/containerizer/mesos/isolator.hpp"
+
+namespace mesos {
+namespace internal {
+namespace slave {
+
+// This isolator uses the cgroups devices subsystem to
+// restrict access to devices in `/dev`. A small set of
+// default devices are whitelisted upon container creation,
+// and access to all other devices is restricted. It is
+// assumed that other isolators will be used to allow / deny
+// access to devices outside the default whitelist.
+class CgroupsDevicesIsolatorProcess : public MesosIsolatorProcess
+{
+public:
+ static Try<mesos::slave::Isolator*> create(const Flags& flags);
+
+ virtual process::Future<Nothing> recover(
+ const std::list<mesos::slave::ContainerState>& states,
+ const hashset<ContainerID>& orphans);
+
+ virtual process::Future<Option<mesos::slave::ContainerLaunchInfo>> prepare(
+ const ContainerID& containerId,
+ const mesos::slave::ContainerConfig& containerConfig);
+
+ virtual process::Future<Nothing> isolate(
+ const ContainerID& containerId,
+ pid_t pid);
+
+ virtual process::Future<Nothing> cleanup(
+ const ContainerID& containerId);
+
+private:
+ struct Info
+ {
+ Info(const ContainerID& _containerId, const std::string& _cgroup)
+ : containerId(_containerId), cgroup(_cgroup) {}
+
+ const ContainerID containerId;
+ const std::string cgroup;
+ };
+
+ CgroupsDevicesIsolatorProcess(
+ const Flags& _flags,
+ const std::string& hierarchy);
+
+ process::Future<Nothing> _cleanup(
+ const ContainerID& containerId,
+ const process::Future<Nothing>& future);
+
+ const Flags flags;
+
+ // The path to the cgroups subsystem hierarchy root.
+ const std::string hierarchy;
+
+ // TODO(bmahler): Use Owned<Info>.
+ hashmap<ContainerID, Info*> infos;
+};
+
+} // namespace slave {
+} // namespace internal {
+} // namespace mesos {
+
+#endif // __CGROUPS_DEVICES_ISOLATOR_HPP__
http://git-wip-us.apache.org/repos/asf/mesos/blob/1d60fdaf/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp b/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp
index 7849e51..d7557a0 100644
--- a/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp
+++ b/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp
@@ -101,29 +101,6 @@ static const cgroups::devices::Entry* NVIDIA_UVM_DEVICE_ENTRY =
}
});
-// The default list of devices to whitelist when device isolation is
-// turned on. The full list of devices can be found here:
-// https://www.kernel.org/doc/Documentation/devices.txt
-//
-// Device whitelisting is described here:
-// https://www.kernel.org/doc/Documentation/cgroup-v1/devices.txt
-static const char* DEFAULT_WHITELIST_ENTRIES[] = {
- "c *:* m", // Make new character devices.
- "b *:* m", // Make new block devices.
- "c 5:1 rwm", // /dev/console
- "c 4:0 rwm", // /dev/tty0
- "c 4:1 rwm", // /dev/tty1
- "c 136:* rwm", // /dev/pts/*
- "c 5:2 rwm", // /dev/ptmx
- "c 10:200 rwm", // /dev/net/tun
- "c 1:3 rwm", // /dev/null
- "c 1:5 rwm", // /dev/zero
- "c 1:7 rwm", // /dev/full
- "c 5:0 rwm", // /dev/tty
- "c 1:9 rwm", // /dev/urandom
- "c 1:8 rwm", // /dev/random
-};
-
CgroupsNvidiaGpuIsolatorProcess::CgroupsNvidiaGpuIsolatorProcess(
const Flags& _flags,
@@ -145,6 +122,14 @@ CgroupsNvidiaGpuIsolatorProcess::~CgroupsNvidiaGpuIsolatorProcess()
Try<Isolator*> CgroupsNvidiaGpuIsolatorProcess::create(const Flags& flags)
{
+ // Make sure the `cgroups/devices` isolator is present.
+ vector<string> isolators_ = strings::tokenize(flags.isolation, ",");
+ set<string> isolators = set<string>(isolators_.begin(), isolators_.end());
+ if (isolators.count("cgroups/devices") == 0) {
+ return Error("The 'cgroups/devices' isolator must be enabled in"
+ " order to use the gpu/devices isolator");
+ }
+
// Initialize NVML.
nvmlReturn_t result = nvmlInit();
if (result != NVML_SUCCESS) {
@@ -184,38 +169,15 @@ Try<Isolator*> CgroupsNvidiaGpuIsolatorProcess::create(const Flags& flags)
}
}
- // Prepare the cgroups device hierarchy.
- //
- // TODO(klueska): Once we have other devices, we will
- // have to do this initialization at a higher level.
- Try<string> hierarchy = cgroups::prepare(
- flags.cgroups_hierarchy,
- "devices",
- flags.cgroups_root);
+ // Retrieve the cgroups devices hierarchy.
+ Result<string> hierarchy = cgroups::hierarchy("devices");
if (hierarchy.isError()) {
return Error(
- "Failed to prepare hierarchy for 'devices' subsystem: " +
+ "Error retrieving the 'devices' subsystem hierarchy: " +
hierarchy.error());
}
- // Ensure that no other subsystem is attached to the hierarchy.
- //
- // TODO(klueska): Once we have other devices, we will
- // have to do this check at a higher level.
- Try<set<string>> subsystems = cgroups::subsystems(hierarchy.get());
- if (subsystems.isError()) {
- return Error(
- "Failed to get subsystems attached to hierarchy"
- " '" + stringify(hierarchy.get()) + "': " +
- subsystems.error());
- } else if (subsystems.get().size() != 1) {
- return Error(
- "Unexpected subsystems attached to hierarchy"
- " '" + stringify(hierarchy.get()) + "': " +
- stringify(subsystems.get()));
- }
-
process::Owned<MesosIsolatorProcess> process(
new CgroupsNvidiaGpuIsolatorProcess(flags, hierarchy.get(), gpus));
@@ -273,44 +235,6 @@ Future<Nothing> CgroupsNvidiaGpuIsolatorProcess::recover(
}
}
- // Remove orphan cgroups.
- Try<vector<string>> cgroups = cgroups::get(hierarchy, flags.cgroups_root);
- if (cgroups.isError()) {
- foreachvalue (Info* info, infos) {
- delete info;
- }
- infos.clear();
- return Failure(cgroups.error());
- }
-
- foreach (const string& cgroup, cgroups.get()) {
- // Ignore the slave cgroup (see the --agent_subsystems flag).
- // TODO(idownes): Remove this when the cgroups layout is updated,
- // see MESOS-1185.
- if (cgroup == path::join(flags.cgroups_root, "slave")) {
- continue;
- }
-
- ContainerID containerId;
- containerId.set_value(Path(cgroup).basename());
-
- if (infos.contains(containerId)) {
- continue;
- }
-
- // Known orphan cgroups will be destroyed by the containerizer
- // using the normal cleanup path. See MESOS-2367 for details.
- if (orphans.contains(containerId)) {
- infos[containerId] = new Info(containerId, cgroup);
- continue;
- }
-
- LOG(INFO) << "Removing unknown orphaned cgroup '" << cgroup << "'";
-
- // We don't wait on the destroy as we don't want to block recovery.
- cgroups::destroy(hierarchy, cgroup, cgroups::DESTROY_TIMEOUT);
- }
-
return Nothing();
}
@@ -323,88 +247,9 @@ Future<Option<ContainerLaunchInfo>> CgroupsNvidiaGpuIsolatorProcess::prepare(
return Failure("Container has already been prepared");
}
- // TODO(bmahler): Don't insert into 'infos' unless we create the
- // cgroup successfully. It's safe for now because 'cleanup' gets
- // called if we return a Failure, but cleanup will fail because the
- // cgroup does not exist when cgroups::destroy is called.
- Info* info = new Info(
+ infos[containerId] = new Info(
containerId, path::join(flags.cgroups_root, containerId.value()));
- infos[containerId] = info;
-
- Try<bool> exists = cgroups::exists(hierarchy, info->cgroup);
- if (exists.isError()) {
- return Failure("Failed to prepare isolator: " + exists.error());
- } else if (exists.get()) {
- return Failure("Failed to prepare isolator: cgroup already exists");
- }
-
- Try<Nothing> create = cgroups::create(hierarchy, info->cgroup);
- if (create.isError()) {
- return Failure("Failed to prepare isolator: " + create.error());
- }
-
- // Chown the cgroup so the executor can create nested cgroups. Do
- // not recurse so the control files are still owned by the slave
- // user and thus cannot be changed by the executor.
- if (containerConfig.has_user()) {
- Try<Nothing> chown = os::chown(
- containerConfig.user(),
- path::join(hierarchy, info->cgroup),
- false);
- if (chown.isError()) {
- return Failure("Failed to prepare isolator: " + chown.error());
- }
- }
-
- // When a devices cgroup is first created, its whitelist inherits
- // all devices from its parent's whitelist (i.e., "a *:* rwm" by
- // default). In theory, we should be able to add and remove devices
- // from the whitelist by writing to the respective `devices.allow`
- // and `devices.deny` files associated with the cgroup. However, the
- // semantics of the whitelist are such that writing to the deny file
- // will only remove entries in the whitelist that are explicitly
- // listed in there (i.e., denying "b 1:3 rwm" when the whitelist
- // only contains "a *:* rwm" will not modify the whitelist because
- // "b 1:3 rwm" is not explicitly listed). Although the whitelist
- // doesn't change, access to the device is still denied as expected
- // (there is just no way of querying the system to detect it).
- // Because of this, we first deny access to all devices and
- // selectively add some back in so we can control the entries in the
- // whitelist explicitly.
- //
- // TODO(klueska): This should really be part of a generic device
- // isolator rather than stuck in the Nvidia GPU specific device
- // isolator. We should move this once we have such a component.
- cgroups::devices::Entry all;
- all.selector.type = Entry::Selector::Type::ALL;
- all.selector.major = None();
- all.selector.minor = None();
- all.access.read = true;
- all.access.write = true;
- all.access.mknod = true;
-
- Try<Nothing> deny = cgroups::devices::deny(hierarchy, info->cgroup, all);
-
- if (deny.isError()) {
- return Failure("Failed to deny all devices: " + deny.error());
- }
-
- foreach (const char* _entry, DEFAULT_WHITELIST_ENTRIES) {
- Try<cgroups::devices::Entry> entry =
- cgroups::devices::Entry::parse(_entry);
-
- CHECK_SOME(entry);
-
- Try<Nothing> allow =
- cgroups::devices::allow(hierarchy, info->cgroup, entry.get());
-
- if (allow.isError()) {
- return Failure("Failed to whitelist default device"
- " '" + stringify(entry.get()) + "': " + allow.error());
- }
- }
-
return update(containerId, containerConfig.executor_info().resources())
.then([]() -> Future<Option<ContainerLaunchInfo>> {
return None();
@@ -412,31 +257,6 @@ Future<Option<ContainerLaunchInfo>> CgroupsNvidiaGpuIsolatorProcess::prepare(
}
-Future<Nothing> CgroupsNvidiaGpuIsolatorProcess::isolate(
- const ContainerID& containerId,
- pid_t pid)
-{
- if (!infos.contains(containerId)) {
- return Failure("Unknown container");
- }
-
- Info* info = CHECK_NOTNULL(infos[containerId]);
-
- Try<Nothing> assign = cgroups::assign(
- hierarchy, info->cgroup, pid);
-
- if (assign.isError()) {
- LOG(ERROR) << "Failed to assign container '" << info->containerId << "'"
- << " to cgroup '" << path::join(hierarchy, info->cgroup) << "': "
- << assign.error();
-
- return Failure("Failed to isolate container: " + assign.error());
- }
-
- return Nothing();
-}
-
-
Future<Nothing> CgroupsNvidiaGpuIsolatorProcess::update(
const ContainerID& containerId,
const Resources& resources)
http://git-wip-us.apache.org/repos/asf/mesos/blob/1d60fdaf/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp b/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp
index f290e3c..181a2aa 100644
--- a/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp
+++ b/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp
@@ -87,10 +87,6 @@ public:
const ContainerID& containerId,
const mesos::slave::ContainerConfig& containerConfig);
- virtual process::Future<Nothing> isolate(
- const ContainerID& containerId,
- pid_t pid);
-
virtual process::Future<Nothing> update(
const ContainerID& containerId,
const Resources& resources);
http://git-wip-us.apache.org/repos/asf/mesos/blob/1d60fdaf/src/tests/containerizer/nvidia_gpu_isolator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/nvidia_gpu_isolator_tests.cpp b/src/tests/containerizer/nvidia_gpu_isolator_tests.cpp
index b1e64b2..65ffbaf 100644
--- a/src/tests/containerizer/nvidia_gpu_isolator_tests.cpp
+++ b/src/tests/containerizer/nvidia_gpu_isolator_tests.cpp
@@ -77,7 +77,7 @@ TEST_F(NvidiaGpuTest, ROOT_CGROUPS_NVIDIA_GPU_VerifyDeviceAccess)
// Turn on Nvidia GPU isolation.
// Assume at least one GPU is available for isolation.
slave::Flags flags = CreateSlaveFlags();
- flags.isolation = "cgroups/devices/gpus/nvidia";
+ flags.isolation = "cgroups/devices,gpu/nvidia";
flags.nvidia_gpu_devices = vector<unsigned int>({0u});
flags.resources = "gpus:1";
@@ -169,7 +169,7 @@ TEST_F(NvidiaGpuTest, ROOT_CGROUPS_NVIDIA_GPU_FractionalResources)
// Turn on Nvidia GPU isolation.
// Assume at least one GPU is available for isolation.
slave::Flags flags = CreateSlaveFlags();
- flags.isolation = "cgroups/devices/gpus/nvidia";
+ flags.isolation = "cgroups/devices,gpu/nvidia";
flags.nvidia_gpu_devices = vector<unsigned int>({0u});
flags.resources = "gpus:1";