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 2017/09/05 22:17:27 UTC
[1/2] mesos git commit: Moved SANDBOX_PATH volume tests to the right
place.
Repository: mesos
Updated Branches:
refs/heads/master 2bb7a8d31 -> 6f98b8d6d
Moved SANDBOX_PATH volume tests to the right place.
This patch moved the SANDBOX_PATH volume tests from linux filesystem
isolator tests to volume sandbox path isolator tests.
Review: https://reviews.apache.org/r/62069
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6f98b8d6
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6f98b8d6
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6f98b8d6
Branch: refs/heads/master
Commit: 6f98b8d6d149c5497d16f588c683a68fccba4fc9
Parents: 1492062
Author: Jie Yu <yu...@gmail.com>
Authored: Mon Sep 4 10:49:28 2017 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Tue Sep 5 15:17:24 2017 -0700
----------------------------------------------------------------------
.../linux_filesystem_isolator_tests.cpp | 131 +-----------------
.../volume_sandbox_path_isolator_tests.cpp | 132 ++++++++++++++++++-
2 files changed, 134 insertions(+), 129 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/6f98b8d6/src/tests/containerizer/linux_filesystem_isolator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/linux_filesystem_isolator_tests.cpp b/src/tests/containerizer/linux_filesystem_isolator_tests.cpp
index 1ad9464..76cc007 100644
--- a/src/tests/containerizer/linux_filesystem_isolator_tests.cpp
+++ b/src/tests/containerizer/linux_filesystem_isolator_tests.cpp
@@ -183,123 +183,6 @@ TEST_F(LinuxFilesystemIsolatorTest, ROOT_Metrics)
}
-// This test verifies that a volume with a relative host path is
-// properly created in the container's sandbox and is properly mounted
-// in the container's mount namespace.
-TEST_F(LinuxFilesystemIsolatorTest, ROOT_VolumeFromSandbox)
-{
- string registry = path::join(sandbox.get(), "registry");
- AWAIT_READY(DockerArchive::create(registry, "test_image"));
-
- slave::Flags flags = CreateSlaveFlags();
- flags.isolation = "filesystem/linux,docker/runtime";
- flags.docker_registry = registry;
- flags.docker_store_dir = path::join(sandbox.get(), "store");
- flags.image_providers = "docker";
-
- Fetcher fetcher(flags);
-
- Try<MesosContainerizer*> create =
- MesosContainerizer::create(flags, true, &fetcher);
-
- ASSERT_SOME(create);
-
- Owned<Containerizer> containerizer(create.get());
-
- ContainerID containerId;
- containerId.set_value(UUID::random().toString());
-
- ExecutorInfo executor = createExecutorInfo(
- "test_executor",
- "echo abc > /tmp/file");
-
- executor.mutable_container()->CopyFrom(createContainerInfo(
- "test_image",
- {createVolumeSandboxPath("/tmp", "tmp", Volume::RW)}));
-
- string directory = path::join(flags.work_dir, "sandbox");
- ASSERT_SOME(os::mkdir(directory));
-
- Future<bool> launch = containerizer->launch(
- containerId,
- createContainerConfig(None(), executor, directory),
- map<string, string>(),
- None());
-
- AWAIT_READY(launch);
-
- Future<Option<ContainerTermination>> wait = containerizer->wait(containerId);
-
- AWAIT_READY(wait);
- ASSERT_SOME(wait.get());
- ASSERT_TRUE(wait->get().has_status());
- EXPECT_WEXITSTATUS_EQ(0, wait->get().status());
-
- EXPECT_SOME_EQ("abc\n", os::read(path::join(directory, "tmp", "file")));
-}
-
-
-// This is a regression test for MESOS-5187. It is a ROOT test to
-// simulate the scenario that the framework user is non-root while
-// the agent process is root, to make sure that non-root user can
-// still have the permission to write to the volume as expected.
-TEST_F(LinuxFilesystemIsolatorTest, ROOT_SandboxVolumeOwnership)
-{
- string registry = path::join(sandbox.get(), "registry");
- AWAIT_READY(DockerArchive::create(registry, "test_image"));
-
- slave::Flags flags = CreateSlaveFlags();
- flags.isolation = "filesystem/linux,docker/runtime";
- flags.docker_registry = registry;
- flags.docker_store_dir = path::join(sandbox.get(), "store");
- flags.image_providers = "docker";
-
- Fetcher fetcher(flags);
-
- Try<MesosContainerizer*> create =
- MesosContainerizer::create(flags, true, &fetcher);
-
- ASSERT_SOME(create);
-
- Owned<Containerizer> containerizer(create.get());
-
- ContainerID containerId;
- containerId.set_value(UUID::random().toString());
-
- ExecutorInfo executor = createExecutorInfo(
- "test_executor",
- "echo abc > /tmp/file");
-
- executor.mutable_container()->CopyFrom(createContainerInfo(
- "test_image",
- {createVolumeSandboxPath("/tmp", "tmp", Volume::RW)}));
-
- string directory = path::join(flags.work_dir, "sandbox");
- ASSERT_SOME(os::mkdir(directory));
-
- // Simulate the executor sandbox ownership as the user
- // from FrameworkInfo.
- ASSERT_SOME(os::chown("nobody", directory));
-
- Future<bool> launch = containerizer->launch(
- containerId,
- createContainerConfig(None(), executor, directory, "nobody"),
- map<string, string>(),
- None());
-
- AWAIT_READY(launch);
-
- Future<Option<ContainerTermination>> wait = containerizer->wait(containerId);
-
- AWAIT_READY(wait);
- ASSERT_SOME(wait.get());
- ASSERT_TRUE(wait->get().has_status());
- EXPECT_WEXITSTATUS_EQ(0, wait->get().status());
-
- EXPECT_SOME_EQ("abc\n", os::read(path::join(directory, "tmp", "file")));
-}
-
-
// This test verifies that persistent volumes are properly mounted in
// the container's root filesystem.
TEST_F(LinuxFilesystemIsolatorTest, ROOT_PersistentVolumeWithRootFilesystem)
@@ -367,10 +250,10 @@ TEST_F(LinuxFilesystemIsolatorTest, ROOT_PersistentVolumeWithRootFilesystem)
}
-// This test verifies that if a persistent volume and host volume
-// are both specified and the host path of the host volume is the
-// same relative path as the persistent volume's container path,
-// the persistent volume will not be neglect and is mounted
+// This test verifies that if a persistent volume and SANDBOX_PATH
+// volume are both specified and the 'path' of the SANDBOX_PATH volume
+// is the same relative path as the persistent volume's container
+// path, the persistent volume will not be neglect and is mounted
// correctly. This is a regression test for MESOS-7770.
TEST_F(LinuxFilesystemIsolatorTest,
ROOT_PersistentVolumeAndHostVolumeWithRootFilesystem)
@@ -396,9 +279,9 @@ TEST_F(LinuxFilesystemIsolatorTest,
ContainerID containerId;
containerId.set_value(UUID::random().toString());
- // Write to an absolute path in the container's mount namespace
- // to verify mounts of the host volume and the persistent volume
- // are done in the proper order.
+ // Write to an absolute path in the container's mount namespace to
+ // verify mounts of the SANDBOX_PATH volume and the persistent
+ // volume are done in the proper order.
ExecutorInfo executor = createExecutorInfo(
"test_executor",
"echo abc > /absolute_path/file");
http://git-wip-us.apache.org/repos/asf/mesos/blob/6f98b8d6/src/tests/containerizer/volume_sandbox_path_isolator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/volume_sandbox_path_isolator_tests.cpp b/src/tests/containerizer/volume_sandbox_path_isolator_tests.cpp
index 0da01c1..2bc9b08 100644
--- a/src/tests/containerizer/volume_sandbox_path_isolator_tests.cpp
+++ b/src/tests/containerizer/volume_sandbox_path_isolator_tests.cpp
@@ -18,6 +18,7 @@
#include <string>
#include <stout/gtest.hpp>
+#include <stout/path.hpp>
#include <process/future.hpp>
#include <process/gtest.hpp>
@@ -25,6 +26,13 @@
#include "tests/environment.hpp"
#include "tests/mesos.hpp"
+#include "slave/containerizer/mesos/containerizer.hpp"
+
+#include "tests/containerizer/docker_archive.hpp"
+
+using std::map;
+using std::string;
+
using process::Future;
using process::Owned;
@@ -35,9 +43,6 @@ using mesos::internal::slave::state::SlaveState;
using mesos::slave::ContainerTermination;
-using std::map;
-using std::string;
-
namespace mesos {
namespace internal {
namespace tests {
@@ -45,11 +50,67 @@ namespace tests {
class VolumeSandboxPathIsolatorTest : public MesosTest {};
+// This test verifies that a SANDBOX_PATH volume with SELF type is
+// properly created in the container's sandbox and is properly mounted
+// in the container's mount namespace.
+TEST_F(VolumeSandboxPathIsolatorTest, ROOT_SelfType)
+{
+ string registry = path::join(sandbox.get(), "registry");
+ AWAIT_READY(DockerArchive::create(registry, "test_image"));
+
+ slave::Flags flags = CreateSlaveFlags();
+ flags.isolation = "filesystem/linux,volume/sandbox_path,docker/runtime";
+ flags.docker_registry = registry;
+ flags.docker_store_dir = path::join(sandbox.get(), "store");
+ flags.image_providers = "docker";
+
+ Fetcher fetcher(flags);
+
+ Try<MesosContainerizer*> create =
+ MesosContainerizer::create(flags, true, &fetcher);
+
+ ASSERT_SOME(create);
+
+ Owned<MesosContainerizer> containerizer(create.get());
+
+ ContainerID containerId;
+ containerId.set_value(UUID::random().toString());
+
+ ExecutorInfo executor = createExecutorInfo(
+ "test_executor",
+ "echo abc > /tmp/file");
+
+ executor.mutable_container()->CopyFrom(createContainerInfo(
+ "test_image",
+ {createVolumeSandboxPath("/tmp", "tmp", Volume::RW)}));
+
+ string directory = path::join(flags.work_dir, "sandbox");
+ ASSERT_SOME(os::mkdir(directory));
+
+ Future<bool> launch = containerizer->launch(
+ containerId,
+ createContainerConfig(None(), executor, directory),
+ map<string, string>(),
+ None());
+
+ AWAIT_READY(launch);
+
+ Future<Option<ContainerTermination>> wait = containerizer->wait(containerId);
+
+ AWAIT_READY(wait);
+ ASSERT_SOME(wait.get());
+ ASSERT_TRUE(wait->get().has_status());
+ EXPECT_WEXITSTATUS_EQ(0, wait->get().status());
+
+ EXPECT_SOME_EQ("abc\n", os::read(path::join(directory, "tmp", "file")));
+}
+
+
// This test verifies that sandbox path volume allows two containers
// nested under the same parent container to share data.
// TODO(jieyu): Parameterize this test to test both linux and posix
// launcher and filesystem isolator.
-TEST_F(VolumeSandboxPathIsolatorTest, SharedVolume)
+TEST_F(VolumeSandboxPathIsolatorTest, SharedParentTypeVolume)
{
slave::Flags flags = CreateSlaveFlags();
flags.isolation = "volume/sandbox_path";
@@ -148,11 +209,72 @@ TEST_F(VolumeSandboxPathIsolatorTest, SharedVolume)
}
+// This is a regression test for MESOS-5187. It is a ROOT test to
+// simulate the scenario that the framework user is non-root while
+// the agent process is root, to make sure that non-root user can
+// still have the permission to write to the volume as expected.
+TEST_F(VolumeSandboxPathIsolatorTest, ROOT_SelfTypeOwnership)
+{
+ string registry = path::join(sandbox.get(), "registry");
+ AWAIT_READY(DockerArchive::create(registry, "test_image"));
+
+ slave::Flags flags = CreateSlaveFlags();
+ flags.isolation = "filesystem/linux,volume/sandbox_path,docker/runtime";
+ flags.docker_registry = registry;
+ flags.docker_store_dir = path::join(sandbox.get(), "store");
+ flags.image_providers = "docker";
+
+ Fetcher fetcher(flags);
+
+ Try<MesosContainerizer*> create =
+ MesosContainerizer::create(flags, true, &fetcher);
+
+ ASSERT_SOME(create);
+
+ Owned<MesosContainerizer> containerizer(create.get());
+
+ ContainerID containerId;
+ containerId.set_value(UUID::random().toString());
+
+ ExecutorInfo executor = createExecutorInfo(
+ "test_executor",
+ "echo abc > /tmp/file");
+
+ executor.mutable_container()->CopyFrom(createContainerInfo(
+ "test_image",
+ {createVolumeSandboxPath("/tmp", "tmp", Volume::RW)}));
+
+ string directory = path::join(flags.work_dir, "sandbox");
+ ASSERT_SOME(os::mkdir(directory));
+
+ // Simulate the executor sandbox ownership as the user
+ // from FrameworkInfo.
+ ASSERT_SOME(os::chown("nobody", directory));
+
+ Future<bool> launch = containerizer->launch(
+ containerId,
+ createContainerConfig(None(), executor, directory, "nobody"),
+ map<string, string>(),
+ None());
+
+ AWAIT_READY(launch);
+
+ Future<Option<ContainerTermination>> wait = containerizer->wait(containerId);
+
+ AWAIT_READY(wait);
+ ASSERT_SOME(wait.get());
+ ASSERT_TRUE(wait->get().has_status());
+ EXPECT_WEXITSTATUS_EQ(0, wait->get().status());
+
+ EXPECT_SOME_EQ("abc\n", os::read(path::join(directory, "tmp", "file")));
+}
+
+
// This is a regression test for MESOS-7830. It is a ROOT test to
// simulate the scenario that the framework user is non-root while
// the agent process is root, to make sure that non-root user can
// still have the permission to write to the volume as expected.
-TEST_F(VolumeSandboxPathIsolatorTest, ROOT_SandboxPathVolumeOwnership)
+TEST_F(VolumeSandboxPathIsolatorTest, ROOT_ParentTypeOwnership)
{
slave::Flags flags = CreateSlaveFlags();
flags.isolation = "volume/sandbox_path";
[2/2] mesos git commit: Moved sandbox path volume logic to the
dedicated isolator.
Posted by ji...@apache.org.
Moved sandbox path volume logic to the dedicated isolator.
Before this patch, the sandbox path volume logics are in two places:
the 'filesystem/linux' isolator and the 'volume/sandbox_path'
isolator, depending on the type of the sandbox path volume (SELF or
PARENT). This patch moved all the sandbox path volume related logics
to the 'volume/sandbox_path' isolator.
Review: https://reviews.apache.org/r/62068
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/14920628
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/14920628
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/14920628
Branch: refs/heads/master
Commit: 14920628c684fb3cb35adeb8714cf4654d5402a8
Parents: 2bb7a8d
Author: Jie Yu <yu...@gmail.com>
Authored: Mon Sep 4 08:27:45 2017 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Tue Sep 5 15:17:24 2017 -0700
----------------------------------------------------------------------
src/slave/containerizer/mesos/containerizer.cpp | 6 +-
.../mesos/isolators/filesystem/linux.cpp | 139 ------------
.../mesos/isolators/volume/sandbox_path.cpp | 214 ++++++++++++++-----
3 files changed, 160 insertions(+), 199 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/14920628/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 4ff014e..4d5dc13 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -261,10 +261,12 @@ Try<MesosContainerizer*> MesosContainerizer::create(
// after all volume isolators, so that the nvidia gpu libraries
// '/usr/local/nvidia' will be overwritten.
if (isolations->contains("filesystem/linux")) {
- // Always enable 'volume/image', 'volume/host_path' on linux if
- // 'filesystem/linux' is enabled for backwards compatibility.
+ // Always enable 'volume/image', 'volume/host_path',
+ // 'volume/sandbox_path' on linux if 'filesystem/linux' is enabled
+ // for backwards compatibility.
isolations->insert("volume/image");
isolations->insert("volume/host_path");
+ isolations->insert("volume/sandbox_path");
}
#endif // __linux__
http://git-wip-us.apache.org/repos/asf/mesos/blob/14920628/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp b/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
index bc14324..faf9490 100644
--- a/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
+++ b/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
@@ -436,145 +436,6 @@ Try<vector<CommandInfo>> LinuxFilesystemIsolatorProcess::getPreExecCommands(
commands.push_back(command);
}
- // Get the parent sandbox user and group info for the source path.
- struct stat s;
- if (::stat(containerConfig.directory().c_str(), &s) < 0) {
- return ErrnoError("Failed to stat '" + containerConfig.directory() + "'");
- }
-
- const uid_t uid = s.st_uid;
- const gid_t gid = s.st_gid;
-
- foreach (const Volume& volume, containerConfig.container_info().volumes()) {
- // NOTE: Volumes with source will be handled by the corresponding
- // isolators (e.g., docker/volume).
- if (volume.has_source()) {
- VLOG(1) << "Ignored a volume with source for container "
- << containerId;
- continue;
- }
-
- if (volume.has_image()) {
- VLOG(1) << "Ignored an image volume for container " << containerId;
- continue;
- }
-
- if (!volume.has_host_path()) {
- return Error("A volume misses 'host_path'");
- }
-
- // If both 'host_path' and 'container_path' are relative paths,
- // return an error because the user can just directly access the
- // volume in the work directory.
- if (!path::absolute(volume.host_path()) &&
- !path::absolute(volume.container_path())) {
- return Error(
- "Both 'host_path' and 'container_path' of a volume are relative");
- }
-
- // Host volumes are now handled by 'volume/host_path' isolator.
- if (path::absolute(volume.host_path())) {
- continue;
- }
-
- // Determine the source of the mount.
- // Path is interpreted as relative to the work directory.
- string source = path::join(
- containerConfig.directory(),
- volume.host_path());
-
- // TODO(jieyu): We need to check that source resolves under the
- // work directory because a user can potentially use a container
- // path like '../../abc'.
-
- // NOTE: Chown should be avoided if the source directory already
- // exists because it may be owned by some other user and should
- // not be mutated.
- if (!os::exists(source)) {
- Try<Nothing> mkdir = os::mkdir(source);
- if (mkdir.isError()) {
- return Error(
- "Failed to create the source of the mount at '" +
- source + "': " + mkdir.error());
- }
-
- LOG(INFO) << "Changing the ownership of the sandbox volume at '"
- << source << "' with UID " << uid << " and GID " << gid;
-
- Try<Nothing> chown = os::chown(uid, gid, source, false);
- if (chown.isError()) {
- return Error(
- "Failed to change the ownership of the sandbox volume at '" +
- source + "' with UID " + stringify(uid) + " and GID " +
- stringify(gid) + ": " + chown.error());
- }
- }
-
- // Determine the target of the mount. The mount target
- // is determined by 'container_path'. It can be either
- // a directory, or the path of a file.
- string target;
-
- CHECK(path::absolute(volume.container_path()));
-
- if (containerConfig.has_rootfs()) {
- target = path::join(
- containerConfig.rootfs(),
- volume.container_path());
-
- if (os::stat::isfile(source)) {
- // The file volume case.
- Try<Nothing> mkdir = os::mkdir(Path(target).dirname());
- if (mkdir.isError()) {
- return Error(
- "Failed to create directory '" +
- Path(target).dirname() + "' "
- "for the target mount file: " + mkdir.error());
- }
-
- Try<Nothing> touch = os::touch(target);
- if (touch.isError()) {
- return Error(
- "Failed to create the target mount file at '" +
- target + "': " + touch.error());
- }
- } else {
- Try<Nothing> mkdir = os::mkdir(target);
- if (mkdir.isError()) {
- return Error(
- "Failed to create the target of the mount at '" +
- target + "': " + mkdir.error());
- }
- }
- } else {
- target = volume.container_path();
-
- // An absolute path must already exist. This is because we
- // want to avoid creating mount points outside the work
- // directory in the host filesystem.
- if (!os::exists(target)) {
- return Error("Absolute container path '" + target + "' "
- "does not exist");
- }
- }
-
- // TODO(jieyu): We need to check that target resolves under
- // 'rootfs' because a user can potentially use a container path
- // like '/../../abc'.
-
- // TODO(jieyu): Consider the mode in the volume.
- CommandInfo command;
- command.set_shell(false);
- command.set_value("mount");
- command.add_arguments("mount");
- command.add_arguments("-n");
- command.add_arguments("--rbind");
- command.add_arguments(source);
- command.add_arguments(target);
-
- commands.push_back(command);
- }
-
return commands;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/14920628/src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp b/src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp
index b321b86..ee5ea3d 100644
--- a/src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp
+++ b/src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp
@@ -14,24 +14,31 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+#include <glog/logging.h>
+
+#include <process/future.hpp>
#include <process/id.hpp>
#include <stout/foreach.hpp>
#include <stout/fs.hpp>
+#include <stout/option.hpp>
+#include <stout/path.hpp>
#include <stout/stringify.hpp>
#include <stout/strings.hpp>
+#include <stout/os/exists.hpp>
#include <stout/os/mkdir.hpp>
+#include <stout/os/stat.hpp>
+#include <stout/os/touch.hpp>
-#ifdef __linux__
-#include "linux/ns.hpp"
-#endif // __linux__
+#include "common/validation.hpp"
#include "slave/containerizer/mesos/isolators/volume/sandbox_path.hpp"
using std::list;
using std::string;
+using process::ErrnoFailure;
using process::Failure;
using process::Future;
using process::Owned;
@@ -108,9 +115,7 @@ Future<Option<ContainerLaunchInfo>> VolumeSandboxPathIsolatorProcess::prepare(
const ContainerInfo& containerInfo = containerConfig.container_info();
if (containerInfo.type() != ContainerInfo::MESOS) {
- return Failure(
- "Can only prepare the sandbox volume isolator "
- "for a MESOS container");
+ return Failure("Only support MESOS containers");
}
if (!bindMountSupported && containerConfig.has_rootfs()) {
@@ -122,9 +127,41 @@ Future<Option<ContainerLaunchInfo>> VolumeSandboxPathIsolatorProcess::prepare(
ContainerLaunchInfo launchInfo;
foreach (const Volume& volume, containerInfo.volumes()) {
- if (!volume.has_source() ||
- !volume.source().has_type() ||
- volume.source().type() != Volume::Source::SANDBOX_PATH) {
+ // NOTE: The validation here is for backwards compatibility. For
+ // example, if an old master (no validation code) is used to
+ // launch a task with a volume.
+ Option<Error> error = common::validation::validateVolume(volume);
+ if (error.isSome()) {
+ return Failure("Invalid volume: " + error->message);
+ }
+
+ Option<Volume::Source::SandboxPath> sandboxPath;
+
+ // NOTE: This is the legacy way of specifying the Volume. The
+ // 'host_path' can be relative in legacy mode, representing
+ // SANDBOX_PATH volumes.
+ if (volume.has_host_path() &&
+ !path::absolute(volume.host_path())) {
+ sandboxPath = Volume::Source::SandboxPath();
+ sandboxPath->set_type(Volume::Source::SandboxPath::SELF);
+ sandboxPath->set_path(volume.host_path());
+ }
+
+ if (volume.has_source() &&
+ volume.source().has_type() &&
+ volume.source().type() == Volume::Source::SANDBOX_PATH) {
+ CHECK(volume.source().has_sandbox_path());
+
+ if (path::absolute(volume.source().sandbox_path().path())) {
+ return Failure(
+ "Path '" + volume.source().sandbox_path().path() + "' "
+ "in SANDBOX_PATH volume is absolute");
+ }
+
+ sandboxPath = volume.source().sandbox_path();
+ }
+
+ if (sandboxPath.isNone()) {
continue;
}
@@ -134,64 +171,83 @@ Future<Option<ContainerLaunchInfo>> VolumeSandboxPathIsolatorProcess::prepare(
"SANDBOX_PATH volume is not supported for DEBUG containers");
}
- if (!volume.source().has_sandbox_path()) {
- return Failure("volume.source.sandbox_path is not specified");
+ if (!bindMountSupported && path::absolute(volume.container_path())) {
+ return Failure(
+ "The 'linux' launcher and 'filesystem/linux' isolator "
+ "must be enabled to support SANDBOX_PATH volume with "
+ "absolute container path");
}
- const Volume::Source::SandboxPath& sandboxPath =
- volume.source().sandbox_path();
+ // TODO(jieyu): We need to check that source resolves under the
+ // work directory because a user can potentially use a container
+ // path like '../../abc'.
- // TODO(jieyu): Support other type of SANDBOX_PATH (e.g., SELF).
- if (!sandboxPath.has_type() ||
- sandboxPath.type() != Volume::Source::SandboxPath::PARENT) {
- return Failure("Only PARENT sandbox path is supported");
+ if (!sandboxPath->has_type()) {
+ return Failure("Unknown SANDBOX_PATH volume type");
}
- if (!containerId.has_parent()) {
- return Failure("PARENT sandbox path only works for nested container");
- }
+ // Prepare the source.
+ string source;
+ string sourceRoot; // The parent directory of 'source'.
+
+ switch (sandboxPath->type()) {
+ case Volume::Source::SandboxPath::SELF:
+ // NOTE: For this case, the user can simply create a symlink
+ // in its sandbox. No need for a volume.
+ if (!path::absolute(volume.container_path())) {
+ return Failure(
+ "'container_path' is relative for "
+ "SANDBOX_PATH volume SELF type");
+ }
- // TODO(jieyu): Validate sandboxPath.path for other invalid chars.
- if (strings::contains(sandboxPath.path(), ".") ||
- strings::contains(sandboxPath.path(), " ")) {
- return Failure("Invalid char found in volume.source.sandbox_path.path");
- }
+ sourceRoot = containerConfig.directory();
+ source = path::join(sourceRoot, sandboxPath->path());
+ break;
+ case Volume::Source::SandboxPath::PARENT:
+ if (!containerId.has_parent()) {
+ return Failure(
+ "SANDBOX_PATH volume PARENT type "
+ "only works for nested container");
+ }
- if (!sandboxes.contains(containerId.parent())) {
- return Failure("Failed to locate the sandbox for the parent container");
- }
+ if (!sandboxes.contains(containerId.parent())) {
+ return Failure(
+ "Failed to locate the sandbox for the parent container");
+ }
- // Prepare the source.
- const string source = path::join(
- sandboxes[containerId.parent()],
- sandboxPath.path());
+ sourceRoot = sandboxes[containerId.parent()];
+ source = path::join(sourceRoot, sandboxPath->path());
+ break;
+ default:
+ return Failure("Unknown SANDBOX_PATH volume type");
+ }
- // NOTE: Chown should be avoided if the source directory already
+ // NOTE: Chown should be avoided if the 'source' directory already
// exists because it may be owned by some other user and should
// not be mutated.
if (!os::exists(source)) {
Try<Nothing> mkdir = os::mkdir(source);
if (mkdir.isError()) {
return Failure(
- "Failed to create the directory in the parent sandbox: " +
- mkdir.error());
+ "Failed to create the directory '" + source + "' "
+ "in the sandbox: " + mkdir.error());
}
- // Get the parent sandbox user and group info for the source path.
+ // Get 'sourceRoot''s user and group info for the source path.
struct stat s;
- if (::stat(sandboxes[containerId.parent()].c_str(), &s) < 0) {
- return Failure(ErrnoError(
- "Failed to stat '" + sandboxes[containerId.parent()] + "'"));
+
+ if (::stat(sourceRoot.c_str(), &s) < 0) {
+ return ErrnoFailure("Failed to stat '" + sourceRoot + "'");
}
- LOG(INFO) << "Changing the ownership of the sandbox_path volume at '"
+ LOG(INFO) << "Changing the ownership of the SANDBOX_PATH volume at '"
<< source << "' with UID " << s.st_uid << " and GID "
<< s.st_gid;
Try<Nothing> chown = os::chown(s.st_uid, s.st_gid, source, false);
if (chown.isError()) {
return Failure(
- "Failed to change the ownership of the sandbox_path volume at '" +
+ "Failed to change the ownership of the SANDBOX_PATH volume at '" +
source + "' with UID " + stringify(s.st_uid) + " and GID " +
stringify(s.st_gid) + ": " + chown.error());
}
@@ -201,33 +257,57 @@ Future<Option<ContainerLaunchInfo>> VolumeSandboxPathIsolatorProcess::prepare(
string target;
if (path::absolute(volume.container_path())) {
- if (!bindMountSupported) {
- return Failure(
- "The 'linux' launcher and 'filesystem/linux' isolator must be "
- "enabled to support absolute container path");
- }
+ CHECK(bindMountSupported);
if (containerConfig.has_rootfs()) {
target = path::join(
containerConfig.rootfs(),
volume.container_path());
- Try<Nothing> mkdir = os::mkdir(target);
- if (mkdir.isError()) {
- return Failure(
- "Failed to create the target of the mount at '" +
- target + "': " + mkdir.error());
+ if (os::stat::isdir(source)) {
+ Try<Nothing> mkdir = os::mkdir(target);
+ if (mkdir.isError()) {
+ return Failure(
+ "Failed to create the mount point at "
+ "'" + target + "': " + mkdir.error());
+ }
+ } else {
+ // The file (regular file or device file) bind mount case.
+ Try<Nothing> mkdir = os::mkdir(Path(target).dirname());
+ if (mkdir.isError()) {
+ return Failure(
+ "Failed to create directory "
+ "'" + Path(target).dirname() + "' "
+ "for the mount point: " + mkdir.error());
+ }
+
+ Try<Nothing> touch = os::touch(target);
+ if (touch.isError()) {
+ return Failure(
+ "Failed to touch the mount point at "
+ "'" + target + "': " + touch.error());
+ }
}
} else {
target = volume.container_path();
+ // An absolute 'container_path' must already exist if the
+ // container rootfs is the same as the host. This is because
+ // we want to avoid creating mount points outside the work
+ // directory in the host filesystem.
if (!os::exists(target)) {
return Failure(
- "Absolute container path '" + target + "' "
- "does not exist");
+ "Mount point '" + target + "' is an absolute path. "
+ "It must exist if the container shares the host filesystem");
}
}
+
+ // TODO(jieyu): We need to check that target resolves under
+ // 'rootfs' because a user can potentially use a container path
+ // like '/../../abc'.
} else {
+ CHECK_EQ(Volume::Source::SandboxPath::PARENT, sandboxPath->type());
+
if (containerConfig.has_rootfs()) {
target = path::join(
containerConfig.rootfs(),
@@ -249,11 +329,29 @@ Future<Option<ContainerLaunchInfo>> VolumeSandboxPathIsolatorProcess::prepare(
containerConfig.directory(),
volume.container_path());
- Try<Nothing> mkdir = os::mkdir(mountPoint);
- if (mkdir.isError()) {
- return Failure(
- "Failed to create the target of the mount at '" +
- mountPoint + "': " + mkdir.error());
+ if (os::stat::isdir(source)) {
+ Try<Nothing> mkdir = os::mkdir(mountPoint);
+ if (mkdir.isError()) {
+ return Failure(
+ "Failed to create the mount point at "
+ "'" + mountPoint + "': " + mkdir.error());
+ }
+ } else {
+ // The file (regular file or device file) bind mount case.
+ Try<Nothing> mkdir = os::mkdir(Path(mountPoint).dirname());
+ if (mkdir.isError()) {
+ return Failure(
+ "Failed to create the directory "
+ "'" + Path(mountPoint).dirname() + "' "
+ "for the mount point: " + mkdir.error());
+ }
+
+ Try<Nothing> touch = os::touch(mountPoint);
+ if (touch.isError()) {
+ return Failure(
+ "Failed to touch the mount point at "
+ "'" + mountPoint+ "': " + touch.error());
+ }
}
}
}