You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ya...@apache.org on 2017/02/04 07:22:18 UTC

[1/3] mesos git commit: Use the stout ELF parser to implement ldd.

Repository: mesos
Updated Branches:
  refs/heads/master c20744a99 -> f200f89a9


Use the stout ELF parser to implement ldd.

Use the stout ELF parser to implement an ldd() API that recursively
gathers the link dependencies of a given program.

Review: https://reviews.apache.org/r/54712/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/58a6b03c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/58a6b03c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/58a6b03c

Branch: refs/heads/master
Commit: 58a6b03c802a4e422091670912578bb2ee9045bf
Parents: c20744a
Author: James Peach <jp...@apache.org>
Authored: Fri Feb 3 23:10:06 2017 -0800
Committer: Jiang Yan Xu <xu...@apple.com>
Committed: Fri Feb 3 23:10:06 2017 -0800

----------------------------------------------------------------------
 src/CMakeLists.txt |  1 +
 src/Makefile.am    |  2 ++
 src/linux/ldd.cpp  | 96 +++++++++++++++++++++++++++++++++++++++++++++++++
 src/linux/ldd.hpp  | 34 ++++++++++++++++++
 4 files changed, 133 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/58a6b03c/src/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 09ef1ae..c09bcde 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -268,6 +268,7 @@ set(LINUX_SRC
   linux/cgroups.cpp
   linux/fs.cpp
   linux/ldcache.cpp
+  linux/ldd.cpp
   linux/perf.cpp
   linux/systemd.cpp
   slave/containerizer/mesos/linux_launcher.cpp

http://git-wip-us.apache.org/repos/asf/mesos/blob/58a6b03c/src/Makefile.am
----------------------------------------------------------------------
diff --git a/src/Makefile.am b/src/Makefile.am
index 6c9be54..0e41441 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1124,6 +1124,7 @@ MESOS_LINUX_FILES =									\
   linux/cgroups.cpp									\
   linux/fs.cpp										\
   linux/ldcache.cpp									\
+  linux/ldd.cpp										\
   linux/perf.cpp									\
   linux/systemd.cpp									\
   slave/containerizer/mesos/linux_launcher.cpp						\
@@ -1164,6 +1165,7 @@ MESOS_LINUX_FILES +=									\
   linux/cgroups.hpp									\
   linux/fs.hpp										\
   linux/ldcache.hpp									\
+  linux/ldd.hpp										\
   linux/ns.hpp										\
   linux/perf.hpp									\
   linux/sched.hpp									\

http://git-wip-us.apache.org/repos/asf/mesos/blob/58a6b03c/src/linux/ldd.cpp
----------------------------------------------------------------------
diff --git a/src/linux/ldd.cpp b/src/linux/ldd.cpp
new file mode 100644
index 0000000..dd5c9c5
--- /dev/null
+++ b/src/linux/ldd.cpp
@@ -0,0 +1,96 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "linux/ldd.hpp"
+
+#include <process/owned.hpp>
+
+#include <stout/elf.hpp>
+#include <stout/nothing.hpp>
+
+using std::string;
+using std::vector;
+
+using process::Owned;
+
+Try<hashset<string>> ldd(
+    const string& path,
+    const vector<ldcache::Entry>& cache)
+{
+  hashset<string> dependencies;
+
+  // Keep a queue of paths that are candidates to have their
+  // ELF dependencies scanned. Once we actually scan something,
+  // we push it into the dependencies set. This lets us avoid
+  // scanning paths more than once.
+  vector<string> candidates;
+
+  // The first candidate is the input path.
+  candidates.push_back(path);
+
+  while (!candidates.empty()) {
+    // Take the next candidate from the back of the
+    // queue. There's no special significance to this, other
+    // than it avoids unnecessary copies that would occur if
+    // we popped the front.
+    const string candidate = candidates.back();
+    candidates.pop_back();
+
+    // If we already have this path, don't scan it again.
+    if (dependencies.contains(candidate)) {
+      continue;
+    }
+
+    Try<elf::File*> load = elf::File::load(candidate);
+    if (load.isError()) {
+      return Error(load.error());
+    }
+
+    Owned<elf::File> elf(load.get());
+
+    Try<vector<string>> _dependencies =
+      elf->get_dynamic_strings(elf::DynamicTag::NEEDED);
+    if (_dependencies.isError()) {
+      return Error(_dependencies.error());
+    }
+
+    // Collect the ELF dependencies of this path into the needed
+    // list, scanning the ld.so cache to find the actual path of
+    // each needed library.
+    foreach (const string& dependency, _dependencies.get()) {
+      auto entry = std::find_if(
+          cache.begin(),
+          cache.end(),
+          [&dependency](const ldcache::Entry& e) {
+            return e.name == dependency;
+          });
+
+      if (entry == cache.end()) {
+        return Error("'" + dependency + "' is not in the ld.so cache");
+      }
+
+      candidates.push_back(entry->path);
+    }
+
+    dependencies.insert(candidate);
+  }
+
+  // Since we are only finding the dependencies, we should
+  // not include the initial file itself.
+  dependencies.erase(path);
+
+  return dependencies;
+}

http://git-wip-us.apache.org/repos/asf/mesos/blob/58a6b03c/src/linux/ldd.hpp
----------------------------------------------------------------------
diff --git a/src/linux/ldd.hpp b/src/linux/ldd.hpp
new file mode 100644
index 0000000..21423d1
--- /dev/null
+++ b/src/linux/ldd.hpp
@@ -0,0 +1,34 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef __LINUX_LDD_HPP__
+#define __LINUX_LDD_HPP__
+
+#include <string>
+#include <vector>
+
+#include <stout/hashset.hpp>
+#include <stout/try.hpp>
+
+#include "linux/ldcache.hpp"
+
+// Given an ldcache, recursively expand the ELF link dependencies
+// for the given path.
+Try<hashset<std::string>> ldd(
+    const std::string& path,
+    const std::vector<ldcache::Entry>& cache);
+
+#endif // __LINUX_LDD_HPP__


Re: [3/3] mesos git commit: Use the stout ELF parser to collect Linux rootfs files.

Posted by Michael Park <mp...@apache.org>.
On Fri, Feb 3, 2017 at 11:22 PM, <ya...@apache.org> wrote:

> +#ifndef __linux__
> +#error "tests/containerizer/rootfs.hpp is only available on Linux
> systems"
> +#endif
>

This seems to break the OS X build as well as the Windows build.

Specifically in the `src/tests/health_check_tests.cpp`.
It's not clear to me what the correct fix is though.

@yan, @jpeach, Could you guys take a look please?

Re: [3/3] mesos git commit: Use the stout ELF parser to collect Linux rootfs files.

Posted by Michael Park <mp...@apache.org>.
On Fri, Feb 3, 2017 at 11:22 PM, <ya...@apache.org> wrote:

> +#ifndef __linux__
> +#error "tests/containerizer/rootfs.hpp is only available on Linux
> systems"
> +#endif
>

This seems to break the OS X build as well as the Windows build.

Specifically in the `src/tests/health_check_tests.cpp`.
It's not clear to me what the correct fix is though.

@yan, @jpeach, Could you guys take a look please?

[3/3] mesos git commit: Use the stout ELF parser to collect Linux rootfs files.

Posted by ya...@apache.org.
Use the stout ELF parser to collect Linux rootfs files.

The dependencies for the programs we need in the Linux root
filesystem vary over time and across distributions. Since stout
has support for parsing the library dependencies of ELF binaries,
use this to collect the necessary dependencies when constructing
a root filesystem for the tests.

Review: https://reviews.apache.org/r/53791/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/f200f89a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f200f89a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f200f89a

Branch: refs/heads/master
Commit: f200f89a948714550393c7a2b2130686df30ca91
Parents: 2e397f7
Author: James Peach <jp...@apache.org>
Authored: Fri Feb 3 23:10:33 2017 -0800
Committer: Jiang Yan Xu <xu...@apple.com>
Committed: Fri Feb 3 23:15:17 2017 -0800

----------------------------------------------------------------------
 src/tests/containerizer/rootfs.cpp | 154 ++++++++++++++++++--------------
 src/tests/containerizer/rootfs.hpp |  14 ++-
 2 files changed, 100 insertions(+), 68 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/f200f89a/src/tests/containerizer/rootfs.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/rootfs.cpp b/src/tests/containerizer/rootfs.cpp
index f11f126..513c1b5 100644
--- a/src/tests/containerizer/rootfs.cpp
+++ b/src/tests/containerizer/rootfs.cpp
@@ -23,6 +23,15 @@
 #include <stout/os.hpp>
 #include <stout/strings.hpp>
 
+#include <stout/os/stat.hpp>
+
+#include "linux/ldd.hpp"
+
+using std::string;
+using std::vector;
+
+using process::Owned;
+
 namespace mesos {
 namespace internal {
 namespace tests {
@@ -35,50 +44,71 @@ Rootfs::~Rootfs()
 }
 
 
-Try<Nothing> Rootfs::add(const std::string& path)
+Try<Nothing> Rootfs::add(const string& path)
 {
-  if (!os::exists(path)) {
-    return Error("File or directory not found on the host");
+  Option<string> source = None();
+
+  // If we are copying a symlink, follow it and copy the
+  // target instead. While this is a little inefficient on
+  // disk space, it avoids complexity in dealing with chains
+  // of symlinks and symlinks in intermediate path components.
+  if (os::stat::islink(path)) {
+    Result<string> target = os::realpath(path);
+    if (target.isNone()) {
+      return Error("Failed to resolve '" + path + "'");
+    }
+
+    if (target.isError()) {
+      return Error("Failed to resolve '" + path + "': " + target.error());
+    }
+
+    source = target.get();
   }
 
-  if (!strings::startsWith(path, "/")) {
-    return Error("Not an absolute path");
+  Try<Nothing> copy = copyPath(source.isSome() ? source.get() : path, path);
+  if (copy.isError()) {
+    return Error("Failed to copy '" + path + "' to rootfs: " + copy.error());
   }
 
-  std::string dirname = Path(path).dirname();
-  std::string target = path::join(root, dirname);
+  return Nothing();
+}
 
-  if (!os::exists(target)) {
-    Try<Nothing> mkdir = os::mkdir(target);
+
+Try<Nothing> Rootfs::copyPath(const string& source, const string& destination)
+{
+  if (!os::exists(source)) {
+    return Error("'" + source + "' not found");
+  }
+
+  if (!strings::startsWith(source, "/")) {
+    return Error("'" + source + "' is not an absolute path");
+  }
+
+  string rootfsDestination = path::join(root, destination);
+  string rootfsDirectory = Path(rootfsDestination).dirname();
+
+  if (!os::exists(rootfsDirectory)) {
+    Try<Nothing> mkdir = os::mkdir(rootfsDirectory);
     if (mkdir.isError()) {
-      return Error("Failed to create directory in rootfs: " +
-                    mkdir.error());
+      return Error(
+          "Failed to create directory '" + rootfsDirectory +
+          "': " + mkdir.error());
     }
   }
 
-  // TODO(jieyu): Make sure 'path' is not under 'root'.
-
-  // Copy the files. We perserve all attributes so that e.g., `ping`
+  // Copy the files. We preserve all attributes so that e.g., `ping`
   // keeps its file-based capabilities.
-  if (os::stat::isdir(path)) {
-    if (os::system(strings::format(
-            "cp -r --preserve=all '%s' '%s'",
-            path, target).get()) != 0) {
-      return ErrnoError("Failed to copy '" + path + "' to rootfs");
-    }
-  } else {
-    if (os::system(strings::format(
-            "cp --preserve=all '%s' '%s'",
-            path, target).get()) != 0) {
-      return ErrnoError("Failed to copy '" + path + "' to rootfs");
-    }
+  if (os::spawn(
+          "cp",
+          {"cp", "-r", "--preserve=all", source, rootfsDestination}) != 0) {
+    return Error("Failed to copy '" + source + "' to rootfs");
   }
 
   return Nothing();
 }
 
 
-Try<process::Owned<Rootfs>> LinuxRootfs::create(const std::string& root)
+Try<process::Owned<Rootfs>> LinuxRootfs::create(const string& root)
 {
   process::Owned<Rootfs> rootfs(new LinuxRootfs(root));
 
@@ -89,64 +119,56 @@ Try<process::Owned<Rootfs>> LinuxRootfs::create(const std::string& root)
     }
   }
 
-  std::vector<std::string> files = {
+  Try<vector<ldcache::Entry>> cache = ldcache::parse();
+
+  if (cache.isError()) {
+    return Error("Failed to parse ld.so cache: " + cache.error());
+  }
+
+  const std::vector<string> programs = {
     "/bin/echo",
     "/bin/ls",
     "/bin/ping",
     "/bin/sh",
     "/bin/sleep",
-    "/usr/bin/sh",
-    "/lib/x86_64-linux-gnu",
-    "/lib64/ld-linux-x86-64.so.2",
-    "/lib64/libc.so.6",
-    "/lib64/libdl.so.2",
-    "/lib64/libidn.so.11",
-    "/lib64/libtinfo.so.5",
-    "/lib64/libselinux.so.1",
-    "/lib64/libpcre.so.1",
-    "/lib64/liblzma.so.5",
-    "/lib64/libpthread.so.0",
-    "/lib64/libcap.so.2",
-    "/lib64/libacl.so.1",
-    "/lib64/libattr.so.1",
-    "/lib64/librt.so.1",
+  };
+
+  hashset<string> files = {
     "/etc/passwd"
   };
 
-  foreach (const std::string& file, files) {
-    // Some linux distros are moving all binaries and libraries to
-    // /usr, in which case /bin, /lib, and /lib64 will be symlinks
-    // to their equivalent directories in /usr.
-    Result<std::string> realpath = os::realpath(file);
-    if (realpath.isSome()) {
-      Try<Nothing> result = rootfs->add(realpath.get());
-      if (result.isError()) {
-        return Error("Failed to add '" + realpath.get() +
-                     "' to rootfs: " + result.error());
-      }
-
-      if (file != realpath.get()) {
-        result = rootfs->add(file);
-        if (result.isError()) {
-          return Error("Failed to add '" + file + "' to rootfs: " +
-                       result.error());
-        }
-      }
+  foreach (const string& program, programs) {
+    Try<hashset<string>> dependencies = ldd(program, cache.get());
+    if (dependencies.isError()) {
+      return Error(
+          "Failed to find dependencies for '" + program + "': " +
+          dependencies.error());
+    }
+
+    files |= dependencies.get();
+    files.insert(program);
+  }
+
+  foreach (const string& file, files) {
+    Try<Nothing> result = rootfs->add(file);
+    if (result.isError()) {
+      return Error(result.error());
     }
   }
 
-  std::vector<std::string> directories = {
+  const std::vector<string> directories = {
     "/proc",
     "/sys",
     "/dev",
     "/tmp"
   };
 
-  foreach (const std::string& directory, directories) {
+  foreach (const string& directory, directories) {
     Try<Nothing> mkdir = os::mkdir(path::join(root, directory));
     if (mkdir.isError()) {
-      return Error("Failed to create '" + directory +
-                   "' in rootfs: " + mkdir.error());
+      return Error(
+          "Failed to create '" + directory + "' in rootfs: " +
+          mkdir.error());
     }
   }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/f200f89a/src/tests/containerizer/rootfs.hpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/rootfs.hpp b/src/tests/containerizer/rootfs.hpp
index 9648638..8835a92 100644
--- a/src/tests/containerizer/rootfs.hpp
+++ b/src/tests/containerizer/rootfs.hpp
@@ -17,6 +17,10 @@
 #ifndef __TEST_ROOTFS_HPP__
 #define __TEST_ROOTFS_HPP__
 
+#ifndef __linux__
+#error "tests/containerizer/rootfs.hpp is only available on Linux systems"
+#endif
+
 #include <string>
 
 #include <process/owned.hpp>
@@ -32,14 +36,20 @@ class Rootfs {
 public:
   virtual ~Rootfs();
 
-  // Add a host directory or file to the root filesystem. Note that
-  // the host directory or file needs to be an absolute path.
+  // Add a host path to the root filesystem. If the given
+  // host path is a symlink, both the link target and the
+  // link itself will be copied into the root.
   Try<Nothing> add(const std::string& path);
 
   const std::string root;
 
 protected:
   Rootfs(const std::string& _root) : root(_root) {}
+
+private:
+  Try<Nothing> copyPath(
+      const std::string& source,
+      const std::string& destination);
 };
 
 


[2/3] mesos git commit: Add some simple ldd() tests.

Posted by ya...@apache.org.
Add some simple ldd() tests.

Review: https://reviews.apache.org/r/54878/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/2e397f79
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/2e397f79
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/2e397f79

Branch: refs/heads/master
Commit: 2e397f79596d8fa4a266f0c90e8ae8b37db2c093
Parents: 58a6b03
Author: James Peach <jp...@apache.org>
Authored: Fri Feb 3 23:10:25 2017 -0800
Committer: Jiang Yan Xu <xu...@apple.com>
Committed: Fri Feb 3 23:15:08 2017 -0800

----------------------------------------------------------------------
 src/Makefile.am          |  1 +
 src/tests/CMakeLists.txt |  1 +
 src/tests/ldd_tests.cpp  | 86 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 88 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/2e397f79/src/Makefile.am
----------------------------------------------------------------------
diff --git a/src/Makefile.am b/src/Makefile.am
index 0e41441..78d55d8 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2334,6 +2334,7 @@ mesos_tests_DEPENDENCIES =					\
 if OS_LINUX
 mesos_tests_SOURCES +=						\
   tests/ldcache_tests.cpp					\
+  tests/ldd_tests.cpp						\
   tests/containerizer/linux_capabilities_isolator_tests.cpp	\
   tests/containerizer/capabilities_tests.cpp			\
   tests/containerizer/capabilities_test_helper.cpp		\

http://git-wip-us.apache.org/repos/asf/mesos/blob/2e397f79/src/tests/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt
index 44b74ee..c12a9f7 100644
--- a/src/tests/CMakeLists.txt
+++ b/src/tests/CMakeLists.txt
@@ -203,6 +203,7 @@ if (LINUX)
   set(MESOS_TESTS_SRC
     ${MESOS_TESTS_SRC}
     ldcache_tests.cpp
+    ldd_tests.cpp
     containerizer/capabilities_tests.cpp
     containerizer/cgroups_isolator_tests.cpp
     containerizer/cgroups_tests.cpp

http://git-wip-us.apache.org/repos/asf/mesos/blob/2e397f79/src/tests/ldd_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/ldd_tests.cpp b/src/tests/ldd_tests.cpp
new file mode 100644
index 0000000..2352399
--- /dev/null
+++ b/src/tests/ldd_tests.cpp
@@ -0,0 +1,86 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <string>
+#include <vector>
+
+#include <process/owned.hpp>
+
+#include <stout/elf.hpp>
+#include <stout/path.hpp>
+#include <stout/strings.hpp>
+#include <stout/try.hpp>
+
+#include "linux/ldd.hpp"
+
+#include "tests/mesos.hpp"
+
+using process::Owned;
+
+using std::string;
+using std::vector;
+
+namespace mesos {
+namespace internal {
+namespace tests {
+
+TEST(Ldd, BinSh)
+{
+  Try<vector<ldcache::Entry>> cache = ldcache::parse();
+  ASSERT_SOME(cache);
+
+  Try<hashset<string>> dependencies = ldd("/bin/sh", cache.get());
+  ASSERT_SOME(dependencies);
+
+  EXPECT_FALSE(dependencies.get().contains("/bin/sh"));
+
+  auto libc = std::find_if(
+      dependencies.get().begin(),
+      dependencies.get().end(),
+      [](const string& dependency) {
+        // On most Linux systems, libc would be in libc.so.6, but
+        // checking the unversioned prefix is robust and is enough
+        // to know that ldd() worked.
+        string basename = Path(dependency).basename();
+        return strings::startsWith(basename, "libc.so");
+      });
+
+  EXPECT_TRUE(libc != dependencies.get().end());
+}
+
+
+TEST(Ldd, EmptyCache)
+{
+  vector<ldcache::Entry> cache;
+
+  Try<hashset<string>> dependencies = ldd("/bin/sh", cache);
+  ASSERT_ERROR(dependencies);
+}
+
+
+TEST(Ldd, MissingFile)
+{
+  Try<vector<ldcache::Entry>> cache = ldcache::parse();
+  ASSERT_SOME(cache);
+
+  Try<hashset<string>> dependencies =
+    ldd("/this/path/is/not/here", cache.get());
+  ASSERT_ERROR(dependencies);
+}
+
+} // namespace tests {
+} // namespace internal {
+} // namespace mesos {