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/08/29 15:55:43 UTC
[08/12] mesos git commit: Integrated 'volume/host_path' into
MesosContainerizer.
Integrated 'volume/host_path' into MesosContainerizer.
This patch removed the host volume logics from the 'filesystem/linux'
isolator, and integrated the new 'volume/host_path' isolator. For
backward compatibility, we always enable 'volume/host_path' isolator
if 'filesystem/linux' isolator is enabled.
Review: https://reviews.apache.org/r/61905
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/2622bade
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/2622bade
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/2622bade
Branch: refs/heads/master
Commit: 2622badedef856810db9a8a274a3eff6eb6231cf
Parents: faa8aae
Author: Jie Yu <yu...@gmail.com>
Authored: Thu Aug 24 12:37:18 2017 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Tue Aug 29 08:55:32 2017 -0700
----------------------------------------------------------------------
src/slave/containerizer/mesos/containerizer.cpp | 11 +-
.../mesos/isolators/filesystem/linux.cpp | 161 +++++++------------
2 files changed, 63 insertions(+), 109 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/2622bade/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index f91eef2..5f9373a 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -71,8 +71,8 @@
#include "slave/containerizer/mesos/isolators/posix.hpp"
#include "slave/containerizer/mesos/isolators/posix/disk.hpp"
#include "slave/containerizer/mesos/isolators/posix/rlimits.hpp"
+#include "slave/containerizer/mesos/isolators/volume/host_path.hpp"
#include "slave/containerizer/mesos/isolators/volume/sandbox_path.hpp"
-#include "slave/containerizer/mesos/isolators/volume/secret.hpp"
#include "slave/containerizer/mesos/provisioner/provisioner.hpp"
@@ -95,7 +95,9 @@
#include "slave/containerizer/mesos/isolators/namespaces/ipc.hpp"
#include "slave/containerizer/mesos/isolators/namespaces/pid.hpp"
#include "slave/containerizer/mesos/isolators/network/cni/cni.hpp"
+#include "slave/containerizer/mesos/isolators/volume/host_path.hpp"
#include "slave/containerizer/mesos/isolators/volume/image.hpp"
+#include "slave/containerizer/mesos/isolators/volume/secret.hpp"
#endif // __linux__
#ifdef ENABLE_PORT_MAPPING_ISOLATOR
@@ -255,14 +257,14 @@ Try<MesosContainerizer*> MesosContainerizer::create(
"Using multiple network isolators simultaneously is disallowed");
}
- // Always enable 'volume/image' on linux if 'filesystem/linux' is
- // enabled, to ensure backwards compatibility.
- //
// TODO(gilbert): Make sure the 'gpu/nvidia' isolator to be created
// 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.
isolations->insert("volume/image");
+ isolations->insert("volume/host_path");
}
#endif // __linux__
@@ -392,6 +394,7 @@ Try<MesosContainerizer*> MesosContainerizer::create(
{"docker/runtime", &DockerRuntimeIsolatorProcess::create},
{"docker/volume", &DockerVolumeIsolatorProcess::create},
{"linux/capabilities", &LinuxCapabilitiesIsolatorProcess::create},
+ {"volume/host_path", &VolumeHostPathIsolatorProcess::create},
{"volume/image",
[&provisioner] (const Flags& flags) -> Try<Isolator*> {
http://git-wip-us.apache.org/repos/asf/mesos/blob/2622bade/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 814e044..662947d 100644
--- a/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
+++ b/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
@@ -468,45 +468,41 @@ Try<vector<CommandInfo>> LinuxFilesystemIsolatorProcess::getPreExecCommands(
"Both 'host_path' and 'container_path' of a volume are relative");
}
- // Determine the source of the mount.
- string source;
-
- if (strings::startsWith(volume.host_path(), "/")) {
- source = volume.host_path();
+ // Host volumes are now handled by 'volume/host_path' isolator.
+ if (path::absolute(volume.host_path())) {
+ continue;
+ }
- // An absolute path must already exist.
- if (!os::exists(source)) {
- return Error("Absolute host path '" + source + "' does not exist");
+ // 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());
}
- } else {
- // Path is interpreted as relative to the work directory.
- 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;
+ 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());
- }
+ 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());
}
}
@@ -515,98 +511,53 @@ Try<vector<CommandInfo>> LinuxFilesystemIsolatorProcess::getPreExecCommands(
// a directory, or the path of a file.
string target;
- if (strings::startsWith(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'.
- } else {
- if (containerConfig.has_rootfs()) {
- target = path::join(containerConfig.rootfs(),
- flags.sandbox_directory,
- volume.container_path());
- } else {
- target = path::join(containerConfig.directory(),
- volume.container_path());
- }
-
- // TODO(jieyu): We need to check that target resolves under the
- // sandbox because a user can potentially use a container path
- // like '../../abc'.
+ CHECK(strings::startsWith(volume.container_path(), "/"));
- // NOTE: We cannot create the mount point at 'target' if
- // container has rootfs defined. The bind mount of the sandbox
- // will hide what's inside 'target'. So we should always create
- // the mount point in 'directory'.
- string mountPoint = path::join(
- containerConfig.directory(),
+ 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(mountPoint).dirname());
+ Try<Nothing> mkdir = os::mkdir(Path(target).dirname());
if (mkdir.isError()) {
return Error(
- "Failed to create the target mount file directory at '" +
- Path(mountPoint).dirname() + "': " + mkdir.error());
+ "Failed to create directory '" +
+ Path(target).dirname() + "' "
+ "for the target mount file: " + mkdir.error());
}
- Try<Nothing> touch = os::touch(mountPoint);
+ 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(mountPoint);
+ Try<Nothing> mkdir = os::mkdir(target);
if (mkdir.isError()) {
return Error(
"Failed to create the target of the mount at '" +
- mountPoint + "': " + mkdir.error());
+ 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);