You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by gi...@apache.org on 2019/02/22 03:42:43 UTC
[mesos] branch master updated (c0b691b -> b68acfd)
This is an automated email from the ASF dual-hosted git repository.
gilbert pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from c0b691b Cleaned up Jenkins workspace for RPM build job.
new 281acd4 Allowed creating non-existing host paths for Mesos Containerizer.
new b68acfd Added test cases for the `volume/host_path` isolator.
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
src/CMakeLists.txt | 1 +
src/Makefile.am | 2 +
.../mesos/isolators/volume/host_path.cpp | 48 +++++++++++-
.../mesos/isolators/volume/host_path.hpp | 6 ++
.../{docker/volume/paths.cpp => volume/utils.cpp} | 29 +++++---
.../{provisioner => isolators/volume}/utils.hpp | 26 +++++--
src/slave/flags.cpp | 6 ++
src/slave/flags.hpp | 1 +
.../volume_host_path_isolator_tests.cpp | 86 ++++++++++++++++++++++
9 files changed, 186 insertions(+), 19 deletions(-)
copy src/slave/containerizer/mesos/isolators/{docker/volume/paths.cpp => volume/utils.cpp} (57%)
copy src/slave/containerizer/mesos/{provisioner => isolators/volume}/utils.hpp (68%)
[mesos] 01/02: Allowed creating non-existing host paths for Mesos
Containerizer.
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gilbert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 281acd44a904d98c10b0bb9f57abb2b110fc6dce
Author: Jason Lai <ja...@jasonlai.net>
AuthorDate: Thu Feb 21 18:29:50 2019 -0800
Allowed creating non-existing host paths for Mesos Containerizer.
Added a new agent flag `--host_path_volume_force_creation` for
the `volume/host_path` isolator. The flag takes a colon-separated
whitelist of paths, under which non-existing host paths are allowed to
be created.
If the flag is not specified, the isolator behaves in the original way
of prohibiting all non-existing host paths from being created.
Review: https://reviews.apache.org/r/69286/
---
src/CMakeLists.txt | 1 +
src/Makefile.am | 2 +
.../mesos/isolators/volume/host_path.cpp | 48 +++++++++++++++++++--
.../mesos/isolators/volume/host_path.hpp | 6 +++
.../isolators/volume/{host_path.hpp => utils.cpp} | 49 ++++++++++++----------
.../isolators/volume/{host_path.hpp => utils.hpp} | 33 +++++++--------
src/slave/flags.cpp | 6 +++
src/slave/flags.hpp | 1 +
8 files changed, 103 insertions(+), 43 deletions(-)
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 9a200d8..687dc85 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -307,6 +307,7 @@ set(LINUX_SRC
slave/containerizer/mesos/isolators/volume/host_path.cpp
slave/containerizer/mesos/isolators/volume/image.cpp
slave/containerizer/mesos/isolators/volume/secret.cpp
+ slave/containerizer/mesos/isolators/volume/utils.cpp
slave/containerizer/mesos/provisioner/backends/aufs.cpp
slave/containerizer/mesos/provisioner/backends/bind.cpp
slave/containerizer/mesos/provisioner/backends/overlay.cpp)
diff --git a/src/Makefile.am b/src/Makefile.am
index e2de7ad..283d5ed 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1409,6 +1409,8 @@ MESOS_LINUX_FILES = \
slave/containerizer/mesos/isolators/volume/image.hpp \
slave/containerizer/mesos/isolators/volume/secret.cpp \
slave/containerizer/mesos/isolators/volume/secret.hpp \
+ slave/containerizer/mesos/isolators/volume/utils.cpp \
+ slave/containerizer/mesos/isolators/volume/utils.hpp \
slave/containerizer/mesos/provisioner/backends/aufs.cpp \
slave/containerizer/mesos/provisioner/backends/aufs.hpp \
slave/containerizer/mesos/provisioner/backends/bind.cpp \
diff --git a/src/slave/containerizer/mesos/isolators/volume/host_path.cpp b/src/slave/containerizer/mesos/isolators/volume/host_path.cpp
index 64abfc1..a20f7ac 100644
--- a/src/slave/containerizer/mesos/isolators/volume/host_path.cpp
+++ b/src/slave/containerizer/mesos/isolators/volume/host_path.cpp
@@ -38,6 +38,7 @@
#include "linux/fs.hpp"
#include "slave/containerizer/mesos/isolators/volume/host_path.hpp"
+#include "slave/containerizer/mesos/isolators/volume/utils.hpp"
using std::string;
@@ -53,6 +54,8 @@ using mesos::slave::ContainerLaunchInfo;
using mesos::slave::ContainerMountInfo;
using mesos::slave::Isolator;
+using mesos::internal::slave::volume::PathValidator;
+
namespace mesos {
namespace internal {
namespace slave {
@@ -69,7 +72,11 @@ Try<Isolator*> VolumeHostPathIsolatorProcess::create(
}
Owned<MesosIsolatorProcess> process(
- new VolumeHostPathIsolatorProcess(flags));
+ flags.host_path_volume_force_creation.isSome()
+ ? new VolumeHostPathIsolatorProcess(
+ flags,
+ PathValidator::parse(flags.host_path_volume_force_creation.get()))
+ : new VolumeHostPathIsolatorProcess(flags));
return new MesosIsolator(process);
}
@@ -78,7 +85,16 @@ Try<Isolator*> VolumeHostPathIsolatorProcess::create(
VolumeHostPathIsolatorProcess::VolumeHostPathIsolatorProcess(
const Flags& _flags)
: ProcessBase(process::ID::generate("volume-host-path-isolator")),
- flags(_flags) {}
+ flags(_flags),
+ pathValidator() {}
+
+
+VolumeHostPathIsolatorProcess::VolumeHostPathIsolatorProcess(
+ const Flags& _flags,
+ const PathValidator& _pathValidator)
+ : ProcessBase(process::ID::generate("volume-host-path-isolator")),
+ flags(_flags),
+ pathValidator(_pathValidator) {}
VolumeHostPathIsolatorProcess::~VolumeHostPathIsolatorProcess() {}
@@ -164,8 +180,32 @@ Future<Option<ContainerLaunchInfo>> VolumeHostPathIsolatorProcess::prepare(
}
if (!os::exists(hostPath.get())) {
- return Failure(
- "Path '" + hostPath.get() + "' in HOST_PATH volume does not exist");
+ if (pathValidator.isNone()) {
+ return Failure(
+ "Path '" + hostPath.get() + "' in HOST_PATH volume does not exist");
+ }
+
+ Try<string> normalizedPath = path::normalize(hostPath.get());
+ if (normalizedPath.isError()) {
+ return Failure(
+ "Failed to normalized the host path '" + hostPath.get() + "': " +
+ normalizedPath.error());
+ }
+
+ Try<Nothing> validate = pathValidator->validate(normalizedPath.get());
+ if (validate.isError()) {
+ return Failure(
+ "Path '" + hostPath.get() + "' in HOST_PATH volume does not exist "
+ "and is not whitelisted for creation: " + validate.error());
+ }
+
+ // Always assume the non-existing host path as a directory.
+ Try<Nothing> mkdir = os::mkdir(normalizedPath.get());
+ if (mkdir.isError()) {
+ return Failure(
+ "Failed to create the host path at '" + hostPath.get() + "': " +
+ mkdir.error());
+ }
}
// Determine the mount point for the host volume.
diff --git a/src/slave/containerizer/mesos/isolators/volume/host_path.hpp b/src/slave/containerizer/mesos/isolators/volume/host_path.hpp
index 4b509e9..08f9946 100644
--- a/src/slave/containerizer/mesos/isolators/volume/host_path.hpp
+++ b/src/slave/containerizer/mesos/isolators/volume/host_path.hpp
@@ -21,6 +21,8 @@
#include "slave/containerizer/mesos/isolator.hpp"
+#include "slave/containerizer/mesos/isolators/volume/utils.hpp"
+
namespace mesos {
namespace internal {
namespace slave {
@@ -41,8 +43,12 @@ public:
private:
VolumeHostPathIsolatorProcess(const Flags& flags);
+ VolumeHostPathIsolatorProcess(
+ const Flags& flags,
+ const volume::PathValidator& pathValidator);
const Flags flags;
+ const Option<volume::PathValidator> pathValidator;
};
} // namespace slave {
diff --git a/src/slave/containerizer/mesos/isolators/volume/host_path.hpp b/src/slave/containerizer/mesos/isolators/volume/utils.cpp
similarity index 52%
copy from src/slave/containerizer/mesos/isolators/volume/host_path.hpp
copy to src/slave/containerizer/mesos/isolators/volume/utils.cpp
index 4b509e9..3b353f4 100644
--- a/src/slave/containerizer/mesos/isolators/volume/host_path.hpp
+++ b/src/slave/containerizer/mesos/isolators/volume/utils.cpp
@@ -14,39 +14,44 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-#ifndef __VOLUME_HOST_PATH_ISOLATOR_HPP__
-#define __VOLUME_HOST_PATH_ISOLATOR_HPP__
+#include <stout/path.hpp>
+#include <stout/stringify.hpp>
+#include <stout/strings.hpp>
-#include "slave/flags.hpp"
+#include "slave/containerizer/mesos/isolators/volume/utils.hpp"
-#include "slave/containerizer/mesos/isolator.hpp"
+using std::string;
+using std::vector;
namespace mesos {
namespace internal {
namespace slave {
+namespace volume {
-class VolumeHostPathIsolatorProcess : public MesosIsolatorProcess
-{
-public:
- static Try<mesos::slave::Isolator*> create(const Flags& flags);
-
- ~VolumeHostPathIsolatorProcess() override;
-
- bool supportsNesting() override;
- bool supportsStandalone() override;
+PathValidator::PathValidator(const vector<string>& _whitelist)
+ : whitelist(_whitelist) {}
- process::Future<Option<mesos::slave::ContainerLaunchInfo>> prepare(
- const ContainerID& containerId,
- const mesos::slave::ContainerConfig& containerConfig) override;
-private:
- VolumeHostPathIsolatorProcess(const Flags& flags);
+PathValidator PathValidator::parse(const string& whitelist)
+{
+ return PathValidator(strings::split(whitelist, HOST_PATH_WHITELIST_DELIM));
+}
- const Flags flags;
-};
+Try<Nothing> PathValidator::validate(const string& path) const
+{
+ foreach (const string& allowedPath, whitelist) {
+ const string allowedDirectory = path::join(
+ allowedPath, stringify(os::PATH_SEPARATOR));
+ if (path == allowedPath || strings::startsWith(path, allowedDirectory)) {
+ return Nothing();
+ }
+ }
+
+ return Error("Path '" + path + "' is not whitelisted");
+}
+
+} // namespace volume {
} // namespace slave {
} // namespace internal {
} // namespace mesos {
-
-#endif // __VOLUME_HOST_PATH_ISOLATOR_HPP__
diff --git a/src/slave/containerizer/mesos/isolators/volume/host_path.hpp b/src/slave/containerizer/mesos/isolators/volume/utils.hpp
similarity index 57%
copy from src/slave/containerizer/mesos/isolators/volume/host_path.hpp
copy to src/slave/containerizer/mesos/isolators/volume/utils.hpp
index 4b509e9..9628e6d 100644
--- a/src/slave/containerizer/mesos/isolators/volume/host_path.hpp
+++ b/src/slave/containerizer/mesos/isolators/volume/utils.hpp
@@ -14,39 +14,38 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-#ifndef __VOLUME_HOST_PATH_ISOLATOR_HPP__
-#define __VOLUME_HOST_PATH_ISOLATOR_HPP__
+#ifndef __VOLUME_UTILS_HPP__
+#define __VOLUME_UTILS_HPP__
-#include "slave/flags.hpp"
+#include <string>
+#include <vector>
-#include "slave/containerizer/mesos/isolator.hpp"
+#include <stout/nothing.hpp>
+#include <stout/try.hpp>
namespace mesos {
namespace internal {
namespace slave {
+namespace volume {
-class VolumeHostPathIsolatorProcess : public MesosIsolatorProcess
+constexpr char HOST_PATH_WHITELIST_DELIM[] = ":";
+
+class PathValidator
{
public:
- static Try<mesos::slave::Isolator*> create(const Flags& flags);
-
- ~VolumeHostPathIsolatorProcess() override;
-
- bool supportsNesting() override;
- bool supportsStandalone() override;
+ static PathValidator parse(const std::string& whitelist);
- process::Future<Option<mesos::slave::ContainerLaunchInfo>> prepare(
- const ContainerID& containerId,
- const mesos::slave::ContainerConfig& containerConfig) override;
+ Try<Nothing> validate(const std::string& path) const;
private:
- VolumeHostPathIsolatorProcess(const Flags& flags);
+ PathValidator(const std::vector<std::string>& whitelist);
- const Flags flags;
+ const std::vector<std::string> whitelist;
};
+} // namespace volume {
} // namespace slave {
} // namespace internal {
} // namespace mesos {
-#endif // __VOLUME_HOST_PATH_ISOLATOR_HPP__
+#endif // __VOLUME_UTILS_HPP__
diff --git a/src/slave/flags.cpp b/src/slave/flags.cpp
index a5b7d90..2a1f3f9 100644
--- a/src/slave/flags.cpp
+++ b/src/slave/flags.cpp
@@ -650,6 +650,12 @@ mesos::internal::slave::Flags::Flags()
"no cgroup limits are set, they are inherited from the root mesos\n"
"cgroup.");
+ add(&Flags::host_path_volume_force_creation,
+ "host_path_volume_force_creation",
+ "A colon-separated list of directories where descendant directories\n"
+ "are allowed to be created by the `volume/host_path` isolator,\n"
+ "if the directories do not exist.");
+
add(&Flags::nvidia_gpu_devices,
"nvidia_gpu_devices",
"A comma-separated list of Nvidia GPU devices. When `gpus` is\n"
diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp
index 7346ba5..c64edd0 100644
--- a/src/slave/flags.hpp
+++ b/src/slave/flags.hpp
@@ -112,6 +112,7 @@ public:
Option<std::string> cgroups_net_cls_secondary_handles;
Option<DeviceWhitelist> allowed_devices;
Option<std::string> agent_subsystems;
+ Option<std::string> host_path_volume_force_creation;
Option<std::vector<unsigned int>> nvidia_gpu_devices;
Option<std::string> perf_events;
Duration perf_interval;
[mesos] 02/02: Added test cases for the `volume/host_path` isolator.
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gilbert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit b68acfda4ee2a501acb49a7b50438166807dd6c7
Author: Jason Lai <ja...@jasonlai.net>
AuthorDate: Thu Feb 21 18:29:52 2019 -0800
Added test cases for the `volume/host_path` isolator.
Added test cases for the `volume/host_path` isolator for whitelisted
non-existing host paths.
Review: https://reviews.apache.org/r/69287/
---
.../volume_host_path_isolator_tests.cpp | 86 ++++++++++++++++++++++
1 file changed, 86 insertions(+)
diff --git a/src/tests/containerizer/volume_host_path_isolator_tests.cpp b/src/tests/containerizer/volume_host_path_isolator_tests.cpp
index 81bf72e..330af25 100644
--- a/src/tests/containerizer/volume_host_path_isolator_tests.cpp
+++ b/src/tests/containerizer/volume_host_path_isolator_tests.cpp
@@ -28,6 +28,8 @@
#include "slave/containerizer/mesos/containerizer.hpp"
+#include "slave/containerizer/mesos/isolators/volume/utils.hpp"
+
#include "tests/cluster.hpp"
#include "tests/mesos.hpp"
@@ -48,6 +50,8 @@ using mesos::internal::slave::Containerizer;
using mesos::internal::slave::Fetcher;
using mesos::internal::slave::MesosContainerizer;
+using mesos::internal::slave::volume::HOST_PATH_WHITELIST_DELIM;
+
using mesos::master::detector::MasterDetector;
using mesos::slave::ContainerTermination;
@@ -345,6 +349,88 @@ TEST_F(VolumeHostPathIsolatorTest, ROOT_FileVolumeFromHostSandboxMountPoint)
}
+// This test verifies that non-existing host paths under whitelist paths
+// specified by the `host_path_volume_force_creation` agent flag can be
+// properly created and mounted into container's mount namespace.
+TEST_F(VolumeHostPathIsolatorTest, ROOT_VolumeFromHostForceCreation)
+{
+ const string imageName = "test-image";
+ const string registryPath = path::join(sandbox.get(), "registry");
+ AWAIT_READY(DockerArchive::create(registryPath, imageName));
+
+ const string storePath = path::join(sandbox.get(), "store");
+ const string mntPath = path::join(sandbox.get(), "mnt");
+ const vector<string> whitelistedPaths = {
+ path::join(mntPath, "whitelist-01"),
+ path::join(mntPath, "whitelist-02")
+ };
+
+ slave::Flags flags = CreateSlaveFlags();
+ flags.docker_registry = registryPath;
+ flags.docker_store_dir = storePath;
+ flags.image_providers = "docker";
+ flags.isolation = "filesystem/linux,docker/runtime";
+ flags.host_path_volume_force_creation = strings::join(
+ HOST_PATH_WHITELIST_DELIM, whitelistedPaths);
+
+ Fetcher fetcher(flags);
+
+ const Try<MesosContainerizer*> create =
+ MesosContainerizer::create(flags, true, &fetcher);
+
+ ASSERT_SOME(create);
+
+ Owned<Containerizer> containerizer(create.get());
+
+ const string sandboxPath = path::join(flags.work_dir, "sandbox");
+ ASSERT_SOME(os::mkdir(sandboxPath));
+
+ // Specify non-existing paths.
+ const vector<string> hostPaths = {
+ path::join(mntPath, "whitelist-01", "a", "b", "c"),
+ path::join(mntPath, "whitelist-02", "d", "e", "f")
+ };
+
+ // Ensure none of `hostPaths` exists.
+ foreach (const string& hostPath, hostPaths) {
+ ASSERT_FALSE(os::exists(hostPath));
+ }
+
+ ContainerID containerId;
+ containerId.set_value(id::UUID::random().toString());
+
+ ContainerInfo containerInfo = createContainerInfo(imageName, {
+ createVolumeHostPath("/mnt/foo", hostPaths.front(), Volume::RW),
+ createVolumeHostPath("/mnt/bar", hostPaths.back(), Volume::RW)});
+
+ ExecutorInfo executor = createExecutorInfo(
+ "test-executor",
+ "test -d /mnt/foo -a -d /mnt/bar");
+ executor.mutable_container()->CopyFrom(containerInfo);
+
+ Future<Containerizer::LaunchResult> launch = containerizer->launch(
+ containerId,
+ createContainerConfig(None(), executor, sandboxPath),
+ map<string, string>(),
+ None());
+
+ AWAIT_ASSERT_EQ(Containerizer::LaunchResult::SUCCESS, 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 `hostPath` was created and is a directory.
+ foreach (const string& hostPath, hostPaths) {
+ EXPECT_TRUE(os::stat::isdir(hostPath));
+ }
+}
+
+
TEST_F(VolumeHostPathIsolatorTest, ROOT_MountPropagation)
{
slave::Flags flags = CreateSlaveFlags();