Lazily unmount persistent volumes in MesosContainerizer. Use MNT_DETACH when unmounting persistent volumes in Linux filesystem isolator to workaround an issue of incorrect handling of container destroy failures. Currently, if isolator cleanup returns a failure, the slave will treat the container as terminated, and will schedule the cleanup of the container's sandbox. Since the mount hasn't been removed in the sandbox (e.g., due to EBUSY), that'll result in data in the persistent volume being incorrectly deleted. Use MNT_DETACH so that the mount point in the sandbox will be removed immediately. See MESOS-7366 for more details.
Review: https://reviews.apache.org/r/58278 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/f96f5b6b Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f96f5b6b Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f96f5b6b Branch: refs/heads/master Commit: f96f5b6b25a444309fe021fa229b3b8286093b5e Parents: 76d42c3 Author: Jie Yu <[email protected]> Authored: Fri Apr 7 16:33:53 2017 -0700 Committer: Jie Yu <[email protected]> Committed: Tue Apr 11 16:31:06 2017 -0700 ---------------------------------------------------------------------- .../mesos/isolators/filesystem/linux.cpp | 22 ++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/f96f5b6b/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 ae0031d..69804ee 100644 --- a/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp +++ b/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp @@ -818,6 +818,8 @@ Future<Nothing> LinuxFilesystemIsolatorProcess::cleanup( return Failure("Failed to get mount table: " + table.error()); } + vector<string> unmountErrors; + // Reverse unmount order to handle nested mount points. foreach (const fs::MountInfoTable::Entry& entry, adaptor::reverse(table->entries)) { @@ -828,15 +830,31 @@ Future<Nothing> LinuxFilesystemIsolatorProcess::cleanup( LOG(INFO) << "Unmounting volume '" << entry.target << "' for container " << containerId; - Try<Nothing> unmount = fs::unmount(entry.target); + // TODO(jieyu): Use MNT_DETACH here to workaround an issue of + // incorrect handling of container destroy failures. Currently, + // if isolator cleanup returns a failure, the slave will treat + // the container as terminated, and will schedule the cleanup of + // the container's sandbox. Since the mount hasn't been removed + // in the sandbox, that'll result in data in the persistent + // volume being incorrectly deleted. Use MNT_DETACH here so that + // the mount point in the sandbox will be removed immediately. + // See MESOS-7366 for more details. + Try<Nothing> unmount = fs::unmount(entry.target, MNT_DETACH); if (unmount.isError()) { - return Failure( + // NOTE: Instead of short circuit, we try to perform as many + // unmount as possible. We'll accumulate the errors together + // in the end. + unmountErrors.push_back( "Failed to unmount volume '" + entry.target + "': " + unmount.error()); } } } + if (!unmountErrors.empty()) { + return Failure(strings::join(", ", unmountErrors)); + } + return Nothing(); }
