You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2017/10/19 16:00:55 UTC
[1/2] mesos git commit: Made `getMountNamespaceTarget` more reliable
by ignoring exited childs.
Repository: mesos
Updated Branches:
refs/heads/master c694a450d -> 725dfcd83
Made `getMountNamespaceTarget` more reliable by ignoring exited childs.
To launch a nested container it might be necessarry to enter parent's
mnt namespace. `getMountNamespaceTarget()` is used for doing that.
Previously, `getMountNamespaceTarget` returned a failure when a
parent's child or grandchild process exited after enumerating child
processes and before getting mnt namespace for the process. Mesos
containerizer launches pre-exec hooks that might exit when calling
`getMountNamespaceTarget` during a subsequent launch of a
nested container. This commit makes `getMountNamespaceTarget` tolerant
to blinking child processes.
Review: https://reviews.apache.org/r/63035/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/725dfcd8
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/725dfcd8
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/725dfcd8
Branch: refs/heads/master
Commit: 725dfcd8375b5749f277604f978dda082ea923dd
Parents: 2dffd59
Author: Andrei Budnik <ab...@mesosphere.com>
Authored: Thu Oct 19 09:00:45 2017 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Thu Oct 19 09:00:52 2017 -0700
----------------------------------------------------------------------
src/slave/containerizer/mesos/utils.cpp | 11 +++++++++++
1 file changed, 11 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/725dfcd8/src/slave/containerizer/mesos/utils.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/utils.cpp b/src/slave/containerizer/mesos/utils.cpp
index 2a6a6ad..30e76d1 100644
--- a/src/slave/containerizer/mesos/utils.cpp
+++ b/src/slave/containerizer/mesos/utils.cpp
@@ -62,6 +62,9 @@ Try<pid_t> getMountNamespaceTarget(pid_t parent)
if (parentNamespace.isError()) {
return Error("Cannot get 'mnt' namespace for process"
" '" + stringify(parent) + "': " + parentNamespace.error());
+ } else if (parentNamespace.isNone()) {
+ return Error("Cannot get 'mnt' namespace for non-existing process"
+ " '" + stringify(parent) + "'");
}
// Search for a new mount namespace in all direct children.
@@ -76,6 +79,10 @@ Try<pid_t> getMountNamespaceTarget(pid_t parent)
if (childNamespace.isError()) {
return Error("Cannot get 'mnt' namespace for child process"
" '" + stringify(child) + "': " + childNamespace.error());
+ } else if (childNamespace.isNone()) {
+ VLOG(1) << "Cannot get 'mnt' namespace for non-existing child process"
+ " '" + stringify(child) + "'";
+ continue;
}
if (parentNamespace.get() != childNamespace.get()) {
@@ -98,6 +105,10 @@ Try<pid_t> getMountNamespaceTarget(pid_t parent)
return Error("Cannot get 'mnt' namespace for 2nd-level child process"
" '" + stringify(child2) +
"': " + child2Namespace.error());
+ } else if (child2Namespace.isNone()) {
+ VLOG(1) << "Cannot get 'mnt' namespace for non-existing 2nd-level"
+ " child process '" + stringify(child2) + "'";
+ continue;
}
if (parentNamespace.get() != child2Namespace.get()) {
[2/2] mesos git commit: Changed return type of `ns::getns()` from
`Try` to `Result`.
Posted by ji...@apache.org.
Changed return type of `ns::getns()` from `Try` to `Result`.
Previously, `getns` checked existence of the process's pid before
trying to `stat` the namespace file. If the pid didn't exist, then
it returned a failure. However, the process might exit before `stat`
is called.
Now, `getns` doesn't check existence of the process's pid explicitly.
The fact that the process is gone can be detected by checking returned
errno of `stat`: if it returns ENOENT, then the function returns
`None()`.
Review: https://reviews.apache.org/r/63074/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/2dffd596
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/2dffd596
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/2dffd596
Branch: refs/heads/master
Commit: 2dffd5963d6883037f04015284d99beff7c85b7c
Parents: c694a45
Author: Andrei Budnik <ab...@mesosphere.com>
Authored: Thu Oct 19 09:00:42 2017 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Thu Oct 19 09:00:52 2017 -0700
----------------------------------------------------------------------
src/linux/ns.hpp | 16 +++++++++-------
src/slave/containerizer/mesos/utils.cpp | 6 +++---
src/tests/containerizer/isolator_tests.cpp | 6 +++---
src/tests/containerizer/ns_tests.cpp | 6 +++---
4 files changed, 18 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/2dffd596/src/linux/ns.hpp
----------------------------------------------------------------------
diff --git a/src/linux/ns.hpp b/src/linux/ns.hpp
index e961163..3d3a1fb 100644
--- a/src/linux/ns.hpp
+++ b/src/linux/ns.hpp
@@ -39,6 +39,7 @@
#include <stout/os.hpp>
#include <stout/path.hpp>
#include <stout/proc.hpp>
+#include <stout/result.hpp>
#include <stout/stringify.hpp>
#include <stout/strings.hpp>
#include <stout/try.hpp>
@@ -233,12 +234,8 @@ inline Try<Nothing> setns(pid_t pid, const std::string& ns)
// pid. The inode number identifies the namespace and can be used for
// comparisons, i.e., two processes with the same inode for a given
// namespace type are in the same namespace.
-inline Try<ino_t> getns(pid_t pid, const std::string& ns)
+inline Result<ino_t> getns(pid_t pid, const std::string& ns)
{
- if (!os::exists(pid)) {
- return Error("Pid " + stringify(pid) + " does not exist");
- }
-
if (ns::namespaces().count(ns) < 1) {
return Error("Namespace '" + ns + "' is not supported");
}
@@ -246,8 +243,13 @@ inline Try<ino_t> getns(pid_t pid, const std::string& ns)
std::string path = path::join("/proc", stringify(pid), "ns", ns);
struct stat s;
if (::stat(path.c_str(), &s) < 0) {
- return ErrnoError("Failed to stat " + ns + " namespace handle"
- " for pid " + stringify(pid));
+ if (errno == ENOENT) {
+ // Process is gone.
+ return None();
+ } else {
+ return ErrnoError("Failed to stat " + ns + " namespace handle"
+ " for pid " + stringify(pid));
+ }
}
return s.st_ino;
http://git-wip-us.apache.org/repos/asf/mesos/blob/2dffd596/src/slave/containerizer/mesos/utils.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/utils.cpp b/src/slave/containerizer/mesos/utils.cpp
index ec6d6c7..2a6a6ad 100644
--- a/src/slave/containerizer/mesos/utils.cpp
+++ b/src/slave/containerizer/mesos/utils.cpp
@@ -58,7 +58,7 @@ namespace slave {
// tasks with the default (a.k.a. "pod" executor).
Try<pid_t> getMountNamespaceTarget(pid_t parent)
{
- Try<ino_t> parentNamespace = ns::getns(parent, "mnt");
+ Result<ino_t> parentNamespace = ns::getns(parent, "mnt");
if (parentNamespace.isError()) {
return Error("Cannot get 'mnt' namespace for process"
" '" + stringify(parent) + "': " + parentNamespace.error());
@@ -72,7 +72,7 @@ Try<pid_t> getMountNamespaceTarget(pid_t parent)
}
foreach (pid_t child, children.get()) {
- Try<ino_t> childNamespace = ns::getns(child, "mnt");
+ Result<ino_t> childNamespace = ns::getns(child, "mnt");
if (childNamespace.isError()) {
return Error("Cannot get 'mnt' namespace for child process"
" '" + stringify(child) + "': " + childNamespace.error());
@@ -93,7 +93,7 @@ Try<pid_t> getMountNamespaceTarget(pid_t parent)
}
foreach (pid_t child2, children2.get()) {
- Try<ino_t> child2Namespace = ns::getns(child2, "mnt");
+ Result<ino_t> child2Namespace = ns::getns(child2, "mnt");
if (child2Namespace.isError()) {
return Error("Cannot get 'mnt' namespace for 2nd-level child process"
" '" + stringify(child2) +
http://git-wip-us.apache.org/repos/asf/mesos/blob/2dffd596/src/tests/containerizer/isolator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/isolator_tests.cpp b/src/tests/containerizer/isolator_tests.cpp
index 5072baf..4ad42bc 100644
--- a/src/tests/containerizer/isolator_tests.cpp
+++ b/src/tests/containerizer/isolator_tests.cpp
@@ -145,7 +145,7 @@ TEST_F(NamespacesIsolatorTest, ROOT_PidNamespace)
EXPECT_EQ(0, wait->get().status());
// Check that the command was run in a different pid namespace.
- Try<ino_t> testPidNamespace = ns::getns(::getpid(), "pid");
+ Result<ino_t> testPidNamespace = ns::getns(::getpid(), "pid");
ASSERT_SOME(testPidNamespace);
Try<string> containerPidNamespace = os::read(path::join(directory, "ns"));
@@ -210,7 +210,7 @@ TEST_F(NamespacesIsolatorTest, ROOT_SharePidNamespace)
EXPECT_EQ(0, wait->get().status());
// Check that the command was run in the same pid namespace.
- Try<ino_t> testPidNamespace = ns::getns(::getpid(), "pid");
+ Result<ino_t> testPidNamespace = ns::getns(::getpid(), "pid");
ASSERT_SOME(testPidNamespace);
Try<string> containerPidNamespace = os::read(path::join(directory, "ns"));
@@ -300,7 +300,7 @@ TEST_F(NamespacesIsolatorTest, ROOT_IPCNamespace)
EXPECT_EQ(0, wait->get().status());
// Check that the command was run in a different IPC namespace.
- Try<ino_t> testIPCNamespace = ns::getns(::getpid(), "ipc");
+ Result<ino_t> testIPCNamespace = ns::getns(::getpid(), "ipc");
ASSERT_SOME(testIPCNamespace);
Try<string> containerIPCNamespace = os::read(path::join(directory, "ns"));
http://git-wip-us.apache.org/repos/asf/mesos/blob/2dffd596/src/tests/containerizer/ns_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/ns_tests.cpp b/src/tests/containerizer/ns_tests.cpp
index 2e28139..61adf85 100644
--- a/src/tests/containerizer/ns_tests.cpp
+++ b/src/tests/containerizer/ns_tests.cpp
@@ -161,10 +161,10 @@ TEST(NsTest, ROOT_getns)
ASSERT_NE(-1, pid);
// Continue in parent.
- Try<ino_t> nsParent = ns::getns(::getpid(), ns);
+ Result<ino_t> nsParent = ns::getns(::getpid(), ns);
ASSERT_SOME(nsParent);
- Try<ino_t> nsChild = ns::getns(pid, ns);
+ Result<ino_t> nsChild = ns::getns(pid, ns);
ASSERT_SOME(nsChild);
// Child should be in a different namespace.
@@ -217,7 +217,7 @@ TEST(NsTest, ROOT_clone)
continue;
}
- Try<ino_t> inode = ns::getns(parent, ns);
+ Result<ino_t> inode = ns::getns(parent, ns);
ASSERT_SOME(inode);
EXPECT_SOME_NE(inode.get(), ns::getns(getpid(), ns));
EXPECT_SOME_EQ(inode.get(), ns::getns(child.get(), ns));