You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by al...@apache.org on 2018/08/31 11:58:25 UTC

[mesos] branch 1.4.x updated (4853e95 -> 05ec5d1)

This is an automated email from the ASF dual-hosted git repository.

alexr pushed a change to branch 1.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git.


    from 4853e95  Added MESOS-9170 to 1.4.3 CHANGELOG.
     new c37eb59  Fixed incorrect `mnt` namespace detection of command executor's task.
     new 05ec5d1  Fixed wrong `mnt` namespace detection for non-command executor tasks.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 src/slave/containerizer/mesos/containerizer.cpp | 27 ++++++++++++------
 src/slave/containerizer/mesos/utils.cpp         | 37 +++++++++++++++++++------
 2 files changed, 47 insertions(+), 17 deletions(-)


[mesos] 01/02: Fixed incorrect `mnt` namespace detection of command executor's task.

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

alexr pushed a commit to branch 1.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit c37eb59e4c4b7b6c16509f317c78207da6eeb485
Author: Andrei Budnik <ab...@mesosphere.com>
AuthorDate: Mon Aug 20 16:28:31 2018 +0200

    Fixed incorrect `mnt` namespace detection of command executor's task.
    
    Previously, we were walking the process tree from the container's
    `init` process to find the first process along the way whose `mnt`
    namespace differs from the `init` process. We expected this algorithm
    to always return the PID of the command executor's task.
    
    However, if someone launches multiple nested containers within the
    process tree, the aforementioned algorithm might detect the PID of
    one of those nested container instead of the command executor's task.
    Even though the `mnt` namespace will be the same across all these
    candidates, the detected PID might belong to a short-lived container,
    which might terminate before the containerizer launcher (aka `nanny`
    process) tries to enter its `mnt` namespace.
    
    This patch fixes the detection algorithm so that it always returns
    the PID of the command executor's task.
    
    Review: https://reviews.apache.org/r/68257/
    (cherry picked from commit e78f636d84f2709da17275f7d70265520c0f4f94)
---
 src/slave/containerizer/mesos/utils.cpp | 37 +++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/src/slave/containerizer/mesos/utils.cpp b/src/slave/containerizer/mesos/utils.cpp
index e05f529..a02d85a 100644
--- a/src/slave/containerizer/mesos/utils.cpp
+++ b/src/slave/containerizer/mesos/utils.cpp
@@ -49,8 +49,13 @@ namespace slave {
 // `pid` of the `init` process of these containers. For now, we
 // compensate for this by simply walking the process tree from the
 // container's `init` process up to 2-levels down (where the task
-// process would exist) and look to see if any process along the way
-// has a different `mnt` namespace. If it does, we return a reference
+// process would exist) and look to see if any 2-nd level process
+// has a different `mnt` namespace. Only one pair of processes matches
+// this property - command executor and its task. One important detail
+// is that we skip all 1st-level processes whose `mnt` namespace is not
+// the same as the `mnt` namespace of the `init` process.
+//
+// If we found such a 2-nd level process, we return a reference
 // to its `pid` as the `pid` for entering the `mnt` namespace of the
 // container.  Otherwise, we return the `init` process's `pid`.
 //
@@ -67,13 +72,16 @@ Try<pid_t> getMountNamespaceTarget(pid_t parent)
                  " '" + stringify(parent) + "'");
   }
 
-  // Search for a new mount namespace in all direct children.
   Try<set<pid_t>> children = os::children(parent, false);
   if (children.isError()) {
     return Error("Cannot get children for process"
                  " '" + stringify(parent) + "'");
   }
 
+  pid_t candidate = parent;
+  int numCandidates = 0;
+  bool hasGrandchild = false;
+
   foreach (pid_t child, children.get()) {
     Result<ino_t> childNamespace = ns::getns(child, "mnt");
     if (childNamespace.isError()) {
@@ -86,12 +94,11 @@ Try<pid_t> getMountNamespaceTarget(pid_t parent)
     }
 
     if (parentNamespace.get() != childNamespace.get()) {
-      return child;
+      // We skip this child, because we know that it's not a command executor.
+      continue;
     }
-  }
 
-  // Search for a new mount namespace in all 2nd-level children.
-  foreach (pid_t child, children.get()) {
+    // Search for a new mount namespace in 2nd-level children.
     Try<set<pid_t>> children2 = os::children(child, false);
     if (children2.isError()) {
       return Error("Cannot get 2nd-level children for process"
@@ -111,13 +118,25 @@ Try<pid_t> getMountNamespaceTarget(pid_t parent)
         continue;
       }
 
+      hasGrandchild = true;
+
       if (parentNamespace.get() != child2Namespace.get()) {
-        return child2;
+        ++numCandidates;
+        candidate = child2;
       }
     }
   }
 
-  return parent;
+  if (!hasGrandchild) {
+    return Error("Cannot detect task process: no 2nd-level processes found");
+  }
+
+  if (numCandidates > 1) {
+    return Error("Cannot detect task process: unexpected number of candidates"
+                 " found: " + stringify(numCandidates));
+  }
+
+  return candidate;
 }
 #endif // __linux__
 


[mesos] 02/02: Fixed wrong `mnt` namespace detection for non-command executor tasks.

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

alexr pushed a commit to branch 1.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 05ec5d1770aeda25b4995487e40f690fe8fa6b19
Author: Andrei Budnik <ab...@mesosphere.com>
AuthorDate: Mon Aug 20 16:28:44 2018 +0200

    Fixed wrong `mnt` namespace detection for non-command executor tasks.
    
    Previously, we were calling `getMountNamespaceTarget()` not only in
    case of the command executor but in all other cases too, including
    the default executor. That might lead to various subtle bugs, caused by
    wrong detection of `mnt` namespace target. This patch fixes the issue
    by setting a parent PID as `mnt` namespace target in case of
    non-command executor task.
    
    Review: https://reviews.apache.org/r/68348/
    (cherry picked from commit b3c9c6939964831170e819f88134af7b275ffe1b)
---
 src/slave/containerizer/mesos/containerizer.cpp | 27 +++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 5f29fe1..ac7cd6f 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -1710,20 +1710,31 @@ Future<bool> MesosContainerizerProcess::_launch(
       return Failure("Unknown parent container");
     }
 
-    if (containers_.at(containerId.parent())->pid.isNone()) {
+    const Owned<Container>& parentContainer =
+      containers_.at(containerId.parent());
+
+    if (parentContainer->pid.isNone()) {
       return Failure("Unknown parent container pid");
     }
 
-    pid_t parentPid = containers_.at(containerId.parent())->pid.get();
+    const pid_t parentPid = parentContainer->pid.get();
 
-    Try<pid_t> mountNamespaceTarget = getMountNamespaceTarget(parentPid);
-    if (mountNamespaceTarget.isError()) {
-      return Failure(
-          "Cannot get target mount namespace from process " +
-          stringify(parentPid) + ": " + mountNamespaceTarget.error());
+    // For the command executor case, we need to find a PID of its task,
+    // which will be used to enter the task's mount namespace.
+    if (parentContainer->config.isSome() &&
+        parentContainer->config->has_task_info()) {
+      Try<pid_t> mountNamespaceTarget = getMountNamespaceTarget(parentPid);
+      if (mountNamespaceTarget.isError()) {
+        return Failure(
+            "Cannot get target mount namespace from process " +
+            stringify(parentPid) + ": " + mountNamespaceTarget.error());
+      }
+
+      launchFlags.namespace_mnt_target = mountNamespaceTarget.get();
+    } else {
+      launchFlags.namespace_mnt_target = parentPid;
     }
 
-    launchFlags.namespace_mnt_target = mountNamespaceTarget.get();
     _enterNamespaces = _enterNamespaces.get() & ~CLONE_NEWNS;
   }
 #endif // __linux__