Lazily unmount persistent volumes in DockerContainerizer. Use MNT_DETACH to unmount persistent volumes in DockerContainerizer to workaround an issue of incorrect handling of container destroy failures. Currently, if unmount fails there, the containerizer will still treat the container as terminated, and the agent 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/58279 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/cf7d1ae9 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/cf7d1ae9 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/cf7d1ae9 Branch: refs/heads/master Commit: cf7d1ae9849d654e7e7eafefdff6824146a102b4 Parents: f96f5b6 Author: Jie Yu <[email protected]> Authored: Fri Apr 7 16:38:00 2017 -0700 Committer: Jie Yu <[email protected]> Committed: Tue Apr 11 16:31:06 2017 -0700 ---------------------------------------------------------------------- src/slave/containerizer/docker.cpp | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/cf7d1ae9/src/slave/containerizer/docker.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp index 06751f1..ef04a21 100644 --- a/src/slave/containerizer/docker.cpp +++ b/src/slave/containerizer/docker.cpp @@ -645,6 +645,8 @@ Try<Nothing> DockerContainerizerProcess::unmountPersistentVolumes( return Error("Failed to get mount table: " + table.error()); } + vector<string> unmountErrors; + foreach (const fs::MountInfoTable::Entry& entry, adaptor::reverse(table.get().entries)) { // TODO(tnachen): We assume there is only one docker container @@ -666,13 +668,30 @@ Try<Nothing> DockerContainerizerProcess::unmountPersistentVolumes( strings::contains(entry.target, containerId.value())) { LOG(INFO) << "Unmounting volume 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 unmount fails there, the containerizer will still treat + // the container as terminated, and the agent 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 Error("Failed to unmount volume '" + entry.target + - "': " + unmount.error()); + // 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 Error(strings::join(", ", unmountErrors)); + } #endif // __linux__ return Nothing(); }
