Repository: mesos Updated Branches: refs/heads/1.0.x f5721b520 -> e5c19b0a9
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/a20f6da1 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/a20f6da1 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/a20f6da1 Branch: refs/heads/1.0.x Commit: a20f6da1f905ea93a82a36058fc40e1b71424139 Parents: d72f3e1 Author: Jie Yu <[email protected]> Authored: Fri Apr 7 16:38:00 2017 -0700 Committer: Jie Yu <[email protected]> Committed: Wed Apr 12 12:31:34 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/a20f6da1/src/slave/containerizer/docker.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp index 9ad98b5..1524dec 100644 --- a/src/slave/containerizer/docker.cpp +++ b/src/slave/containerizer/docker.cpp @@ -613,6 +613,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 @@ -634,13 +636,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(); }
