You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2016/08/01 16:06:15 UTC
[1/2] mesos git commit: Added check for root permissions to
'NvidiaVolume::create()'.
Repository: mesos
Updated Branches:
refs/heads/master 7864eb860 -> 48a492cd9
Added check for root permissions to 'NvidiaVolume::create()'.
Review: https://reviews.apache.org/r/50644/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ad1f6105
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ad1f6105
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ad1f6105
Branch: refs/heads/master
Commit: ad1f610508ca669b32b1cb7a4d5baf5f3b337b70
Parents: 7864eb8
Author: Kevin Klues <kl...@gmail.com>
Authored: Mon Aug 1 09:06:04 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Aug 1 09:06:04 2016 -0700
----------------------------------------------------------------------
src/slave/containerizer/mesos/isolators/gpu/volume.cpp | 11 +++++++++++
src/tests/containerizer/nvidia_gpu_isolator_tests.cpp | 4 ++--
2 files changed, 13 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/ad1f6105/src/slave/containerizer/mesos/isolators/gpu/volume.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/gpu/volume.cpp b/src/slave/containerizer/mesos/isolators/gpu/volume.cpp
index 4b3651a..4d87c11 100644
--- a/src/slave/containerizer/mesos/isolators/gpu/volume.cpp
+++ b/src/slave/containerizer/mesos/isolators/gpu/volume.cpp
@@ -191,6 +191,17 @@ const string& NvidiaVolume::CONTAINER_PATH() const
Try<NvidiaVolume> NvidiaVolume::create()
{
+ // We need root permissions in order to create the volume.
+ Result<string> user = os::user();
+ if (!user.isSome()) {
+ return Error("Failed to determine user: " +
+ (user.isError() ? user.error() : "username not found"));
+ }
+
+ if (user.get() != "root") {
+ return Error("NvidiaVolume::create() requires root privileges");
+ }
+
// Append the Nvidia driver version to the name of the volume.
Try<Nothing> initialized = nvml::initialize();
if (initialized.isError()) {
http://git-wip-us.apache.org/repos/asf/mesos/blob/ad1f6105/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 02d82e3..fea1f9f 100644
--- a/src/tests/containerizer/nvidia_gpu_isolator_tests.cpp
+++ b/src/tests/containerizer/nvidia_gpu_isolator_tests.cpp
@@ -584,7 +584,7 @@ TEST_F(NvidiaGpuTest, NVIDIA_GPU_Allocator)
// Tests that we can create the volume that consolidates
// the Nvidia libraries and binaries.
-TEST_F(NvidiaGpuTest, NVIDIA_GPU_VolumeCreation)
+TEST_F(NvidiaGpuTest, ROOT_NVIDIA_GPU_VolumeCreation)
{
Try<NvidiaVolume> volume = NvidiaVolume::create();
ASSERT_SOME(volume);
@@ -603,7 +603,7 @@ TEST_F(NvidiaGpuTest, NVIDIA_GPU_VolumeCreation)
// Tests that we can properly detect when an Nvidia volume should be
// injected into a Docker container given its ImageManifest.
-TEST_F(NvidiaGpuTest, NVIDIA_GPU_VolumeShouldInject)
+TEST_F(NvidiaGpuTest, ROOT_NVIDIA_GPU_VolumeShouldInject)
{
Try<JSON::Object> json = JSON::parse<JSON::Object>(
R"~(
[2/2] mesos git commit: Updated NvidiaVolume to mount as 'tmpfs' if
parent fs is 'noexec'.
Posted by ji...@apache.org.
Updated NvidiaVolume to mount as 'tmpfs' if parent fs is 'noexec'.
This patch is in response to an issue we ran into on Ubuntu 14.04,
where '/run' is being mounted as 'noexec' (MESOS-5923). Since our
NvidiaVolume is created below this mount point, we are unable to
execute any binaries we add to this volume. This causes problems, for
example, when trying to execute 'nvidia-smi' from within a container
that has this volume mounted in.
To work around this issue, we detect if any mount point above the path
where we create the volume is marked as 'noexec', and if so, we create
a new 'tmpfs' mount for the volume without 'noexec' set.
Review: https://reviews.apache.org/r/50592/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/48a492cd
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/48a492cd
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/48a492cd
Branch: refs/heads/master
Commit: 48a492cd9d7d0a194735b9b4107a35b489c596e1
Parents: ad1f610
Author: Kevin Klues <kl...@gmail.com>
Authored: Mon Aug 1 09:06:07 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Aug 1 09:06:07 2016 -0700
----------------------------------------------------------------------
.../mesos/isolators/gpu/volume.cpp | 45 +++++++++++++++++++-
1 file changed, 43 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/48a492cd/src/slave/containerizer/mesos/isolators/gpu/volume.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/gpu/volume.cpp b/src/slave/containerizer/mesos/isolators/gpu/volume.cpp
index 4d87c11..478e106 100644
--- a/src/slave/containerizer/mesos/isolators/gpu/volume.cpp
+++ b/src/slave/containerizer/mesos/isolators/gpu/volume.cpp
@@ -23,6 +23,7 @@
#include <process/owned.hpp>
+#include <stout/adaptor.hpp>
#include <stout/elf.hpp>
#include <stout/error.hpp>
#include <stout/foreach.hpp>
@@ -38,6 +39,7 @@
#include <stout/os/rmdir.hpp>
#include <stout/os/shell.hpp>
+#include "linux/fs.hpp"
#include "linux/ldcache.hpp"
#include "slave/containerizer/mesos/isolators/gpu/nvml.hpp"
@@ -222,6 +224,45 @@ Try<NvidiaVolume> NvidiaVolume::create()
}
}
+ // If the filesystem where we are creating this volume has the
+ // `noexec` bit set, we will not be able to execute any of the
+ // nvidia binaries we place in the volume (e.g. `nvidia-smi`). To
+ // fix this, we mount a `tmpfs` over the volume `hostPath` without
+ // the `noexec` bit set. See MESOS-5923 for more information.
+ Try<fs::MountInfoTable> table = fs::MountInfoTable::read();
+ if (table.isError()) {
+ return Error("Failed to get mount table: " + table.error());
+ }
+
+ Result<string> realpath = os::realpath(hostPath);
+ if (!realpath.isSome()) {
+ return Error("Failed to os::realpath '" + hostPath + "':"
+ " " + (realpath.isError()
+ ? realpath.error()
+ : "No such file or directory"));
+ }
+
+ // Do a reverse search through the list of mounted filesystems to
+ // find the filesystem that is mounted with the longest overlapping
+ // path to our `hostPath` (which may include the `hostPath` itself).
+ // Only mount a new `tmpfs` over the `hostPath` if the filesysem we
+ // find is marked as `noexec`.
+ foreach (const fs::MountInfoTable::Entry& entry,
+ adaptor::reverse(table->entries)) {
+ if (strings::startsWith(realpath.get(), entry.target)) {
+ if (strings::contains(entry.vfsOptions, "noexec")) {
+ Try<Nothing> mnt = fs::mount(
+ "tmpfs", hostPath, "tmpfs", MS_NOSUID | MS_NODEV, "mode=755");
+
+ if (mnt.isError()) {
+ return Error("Failed to mount '" + hostPath + "': " + mnt.error());
+ }
+ }
+
+ break;
+ }
+ }
+
// Create some directories in the volume if they don't yet exist.
string directories[] = {"bin", "lib", "lib64" };
foreach (const string& directory, directories) {
@@ -345,7 +386,7 @@ Try<NvidiaVolume> NvidiaVolume::create()
if (!os::exists(symlinkPath)) {
Try<Nothing> symlink =
- fs::symlink(Path(realpath.get()).basename(), symlinkPath);
+ ::fs::symlink(Path(realpath.get()).basename(), symlinkPath);
if (symlink.isError()) {
return Error("Failed to fs::symlink"
" '" + symlinkPath + "'"
@@ -370,7 +411,7 @@ Try<NvidiaVolume> NvidiaVolume::create()
if (!os::exists(symlinkPath)) {
Try<Nothing> symlink =
- fs::symlink(Path(realpath.get()).basename(), symlinkPath);
+ ::fs::symlink(Path(realpath.get()).basename(), symlinkPath);
if (symlink.isError()) {
return Error("Failed to fs::symlink"
" '" + symlinkPath + "'"