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/fbc66394 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/fbc66394 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/fbc66394 Branch: refs/heads/1.1.x Commit: fbc663944e37e5385826ad6d29c51b74ac05e34f Parents: 389b5aa Author: Jie Yu <[email protected]> Authored: Fri Apr 7 16:38:00 2017 -0700 Committer: Jie Yu <[email protected]> Committed: Tue Apr 11 16:42:58 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/fbc66394/src/slave/containerizer/docker.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp index 8ec4c0a..750f1b6 100644 --- a/src/slave/containerizer/docker.cpp +++ b/src/slave/containerizer/docker.cpp @@ -615,6 +615,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 @@ -636,13 +638,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(); }
