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/07/11 21:52:22 UTC
[1/2] mesos git commit: Fixed persistent volume and host volume
conflict issue.
Repository: mesos
Updated Branches:
refs/heads/master 33093c893 -> c3632f67d
Fixed persistent volume and host volume conflict issue.
This is the fix for MESOS-7770. Basically, if a persistent volume
and a host volume are both specified and the source path of the
host volume is the same as the container path of the persistent
volume, then the persistent volume will be skipped and is not
mounted correctly. We should precisely check the mount table
to determine whether the persistent volume is mounted or not.
If not mounted, make sure we do mount the persistent volume.
Review: https://reviews.apache.org/r/60729/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/2106fc7a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/2106fc7a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/2106fc7a
Branch: refs/heads/master
Commit: 2106fc7a2fd5f54e4a6454ba3cf7de023f732561
Parents: 33093c8
Author: Gilbert Song <so...@gmail.com>
Authored: Tue Jul 11 14:14:28 2017 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Tue Jul 11 14:14:28 2017 -0700
----------------------------------------------------------------------
.../mesos/isolators/filesystem/linux.cpp | 91 +++++++++++++-------
1 file changed, 60 insertions(+), 31 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/2106fc7a/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 69804ee..bf35b7f 100644
--- a/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
+++ b/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
@@ -727,45 +727,74 @@ Future<Nothing> LinuxFilesystemIsolatorProcess::update(
string target = path::join(info->directory, containerPath);
if (os::exists(target)) {
- // NOTE: This is possible because 'info->resources' will be
- // reset when slave restarts and recovers. When the slave calls
- // 'containerizer->update' after the executor re-registers,
- // we'll try to re-mount all the already mounted volumes.
-
- // TODO(jieyu): Check the source of the mount matches the entry
- // with the same target in the mount table if one can be found.
- // If not, mount the persistent volume as we did below. This is
+ // NOTE: There are two scenarios that we may have the mount
+ // target existed:
+ // 1. This is possible because 'info->resources' will be reset
+ // when slave restarts and recovers. When the slave calls
+ // 'containerizer->update' after the executor re-registers,
+ // we'll try to re-mount all the already mounted volumes.
+ // 2. There may be multiple references to the persistent
+ // volume's mount target. E.g., a host volume and a
+ // persistent volume are both specified, and the source
+ // of the host volume is the same as the container path
+ // of the persistent volume.
+
+ // Check the source of the mount matches the entry with the
+ // same target in the mount table if one can be found. If
+ // not, mount the persistent volume as we did below. This is
// possible because the slave could crash after it unmounts the
// volume but before it is able to delete the mount point.
- } else {
- Try<Nothing> mkdir = os::mkdir(target);
- if (mkdir.isError()) {
- return Failure(
- "Failed to create persistent volume mount point at '" +
- target + "': " + mkdir.error());
+ Try<fs::MountInfoTable> table = fs::MountInfoTable::read();
+ if (table.isError()) {
+ return Failure("Failed to get mount table: " + table.error());
}
- LOG(INFO) << "Mounting '" << source << "' to '" << target
- << "' for persistent volume " << resource
- << " of container " << containerId;
+ // Check a particular persistent volume is mounted or not.
+ bool volumeMounted = false;
+
+ foreach (const fs::MountInfoTable::Entry& entry, table->entries) {
+ // TODO(gilbert): Check source of the mount matches the entry's
+ // root. Note that the root is relative to the root of its parent
+ // mount. See:
+ // http://man7.org/linux/man-pages/man5/proc.5.html
+ if (target == entry.target) {
+ volumeMounted = true;
+ break;
+ }
+ }
- Try<Nothing> mount = fs::mount(source, target, None(), MS_BIND, nullptr);
- if (mount.isError()) {
- return Failure(
- "Failed to mount persistent volume from '" +
- source + "' to '" + target + "': " + mount.error());
+ if (volumeMounted) {
+ continue;
}
+ }
- // If the mount needs to be read-only, do a remount.
- if (resource.disk().volume().mode() == Volume::RO) {
- mount = fs::mount(
- None(), target, None(), MS_BIND | MS_RDONLY | MS_REMOUNT, nullptr);
+ Try<Nothing> mkdir = os::mkdir(target);
+ if (mkdir.isError()) {
+ return Failure(
+ "Failed to create persistent volume mount point at '" +
+ target + "': " + mkdir.error());
+ }
- if (mount.isError()) {
- return Failure(
- "Failed to remount persistent volume as read-only from '" +
- source + "' to '" + target + "': " + mount.error());
- }
+ LOG(INFO) << "Mounting '" << source << "' to '" << target
+ << "' for persistent volume " << resource
+ << " of container " << containerId;
+
+ Try<Nothing> mount = fs::mount(source, target, None(), MS_BIND, nullptr);
+ if (mount.isError()) {
+ return Failure(
+ "Failed to mount persistent volume from '" +
+ source + "' to '" + target + "': " + mount.error());
+ }
+
+ // If the mount needs to be read-only, do a remount.
+ if (resource.disk().volume().mode() == Volume::RO) {
+ mount = fs::mount(
+ None(), target, None(), MS_BIND | MS_RDONLY | MS_REMOUNT, nullptr);
+
+ if (mount.isError()) {
+ return Failure(
+ "Failed to remount persistent volume as read-only from '" +
+ source + "' to '" + target + "': " + mount.error());
}
}
}
[2/2] mesos git commit: Added unit tests for persistent volume and
host volume conflict issue.
Posted by ji...@apache.org.
Added unit tests for persistent volume and host volume conflict issue.
Review: https://reviews.apache.org/r/60750/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/c3632f67
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c3632f67
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c3632f67
Branch: refs/heads/master
Commit: c3632f67df4435f4dc3d9cb2d4d50db63aa8bcf8
Parents: 2106fc7
Author: Gilbert Song <so...@gmail.com>
Authored: Tue Jul 11 14:14:31 2017 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Tue Jul 11 14:14:31 2017 -0700
----------------------------------------------------------------------
.../linux_filesystem_isolator_tests.cpp | 76 ++++++++++++++++++++
1 file changed, 76 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/c3632f67/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 803758c..457311e 100644
--- a/src/tests/containerizer/linux_filesystem_isolator_tests.cpp
+++ b/src/tests/containerizer/linux_filesystem_isolator_tests.cpp
@@ -534,6 +534,82 @@ 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
+// correctly. This is a regression test for MESOS-7770.
+TEST_F(LinuxFilesystemIsolatorTest,
+ ROOT_PersistentVolumeAndHostVolumeWithRootFilesystem)
+{
+ 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());
+
+ // 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.
+ ExecutorInfo executor = createExecutorInfo(
+ "test_executor",
+ "echo abc > /absolute_path/file");
+
+ executor.add_resources()->CopyFrom(createPersistentVolume(
+ Megabytes(32),
+ "test_role",
+ "persistent_volume_id",
+ "volume"));
+
+ executor.mutable_container()->CopyFrom(createContainerInfo(
+ "test_image",
+ {createVolumeFromHostPath("/absolute_path", "volume", Volume::RW)}));
+
+ // Create a persistent volume.
+ string volume = slave::paths::getPersistentVolumePath(
+ flags.work_dir,
+ "test_role",
+ "persistent_volume_id");
+
+ ASSERT_SOME(os::mkdir(volume));
+
+ 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(volume, "file")));
+}
+
+
// This test verifies that persistent volumes are properly mounted if
// the container does not specify a root filesystem.
TEST_F(LinuxFilesystemIsolatorTest, ROOT_PersistentVolumeWithoutRootFilesystem)