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/c791d4a6 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c791d4a6 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c791d4a6 Branch: refs/heads/1.2.x Commit: c791d4a690742c3ef3b256273d5c4b557c522a06 Parents: 11ecbfb Author: Jie Yu <[email protected]> Authored: Fri Apr 7 16:38:00 2017 -0700 Committer: Jie Yu <[email protected]> Committed: Tue Apr 11 16:43:13 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/c791d4a6/src/slave/containerizer/docker.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp index c438ec3..029df97 100644 --- a/src/slave/containerizer/docker.cpp +++ b/src/slave/containerizer/docker.cpp @@ -640,6 +640,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 @@ -661,13 +663,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(); }
