You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bm...@apache.org on 2016/07/04 00:20:15 UTC

[1/2] mesos git commit: Updated Docker::inspect to parse devices.

Repository: mesos
Updated Branches:
  refs/heads/master f888a1c59 -> 6eb2ce908


Updated Docker::inspect to parse devices.

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


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

Branch: refs/heads/master
Commit: 6eb2ce90871ef373b97e0b713fe78e951595d701
Parents: 380f507
Author: Benjamin Mahler <bm...@apache.org>
Authored: Sun Jul 3 16:32:25 2016 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Sun Jul 3 17:19:36 2016 -0700

----------------------------------------------------------------------
 src/docker/docker.cpp                    | 50 ++++++++++++++++++-
 src/docker/docker.hpp                    |  8 ++-
 src/tests/containerizer/docker_tests.cpp | 71 +++++++++++++++++++++++++++
 3 files changed, 126 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/6eb2ce90/src/docker/docker.cpp
----------------------------------------------------------------------
diff --git a/src/docker/docker.cpp b/src/docker/docker.cpp
index 939f69d..aad5380 100755
--- a/src/docker/docker.cpp
+++ b/src/docker/docker.cpp
@@ -17,10 +17,15 @@
 #include <map>
 #include <vector>
 
+#include <stout/error.hpp>
+#include <stout/foreach.hpp>
+#include <stout/json.hpp>
 #include <stout/lambda.hpp>
 #include <stout/os.hpp>
+#include <stout/path.hpp>
 #include <stout/result.hpp>
 #include <stout/strings.hpp>
+#include <stout/stringify.hpp>
 
 #include <stout/os/killtree.hpp>
 #include <stout/os/read.hpp>
@@ -349,7 +354,50 @@ Try<Docker::Container> Docker::Container::create(const string& output)
     }
   }
 
-  return Docker::Container(output, id, name, optionalPid, started, ipAddress);
+  vector<Device> devices;
+
+  Result<JSON::Array> devicesArray =
+    json.find<JSON::Array>("HostConfig.Devices");
+
+  if (devicesArray.isError()) {
+    return Error("Failed to parse HostConfig.Devices: " + devicesArray.error());
+  }
+
+  if (devicesArray.isSome()) {
+    foreach (const JSON::Value& entry, devicesArray->values) {
+      if (!entry.is<JSON::Object>()) {
+        return Error("Malformed HostConfig.Devices"
+                     " entry '" + stringify(entry) + "'");
+      }
+
+      JSON::Object object = entry.as<JSON::Object>();
+
+      Result<JSON::String> hostPath =
+        object.at<JSON::String>("PathOnHost");
+      Result<JSON::String> containerPath =
+        object.at<JSON::String>("PathInContainer");
+      Result<JSON::String> permissions =
+        object.at<JSON::String>("CgroupPermissions");
+
+      if (!hostPath.isSome() ||
+          !containerPath.isSome() ||
+          !permissions.isSome()) {
+        return Error("Malformed HostConfig.Devices entry"
+                     " '" + stringify(object) + "'");
+      }
+
+      Device device;
+      device.hostPath = Path(hostPath->value);
+      device.containerPath = Path(containerPath->value);
+      device.access.read = strings::contains(permissions->value, "r");
+      device.access.write = strings::contains(permissions->value, "w");
+      device.access.mknod = strings::contains(permissions->value, "m");
+
+      devices.push_back(device);
+    }
+  }
+
+  return Container(output, id, name, optionalPid, started, ipAddress, devices);
 }
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/6eb2ce90/src/docker/docker.hpp
----------------------------------------------------------------------
diff --git a/src/docker/docker.hpp b/src/docker/docker.hpp
index 64333b8..9093371 100644
--- a/src/docker/docker.hpp
+++ b/src/docker/docker.hpp
@@ -95,6 +95,8 @@ public:
     // been not been assigned.
     const Option<std::string> ipAddress;
 
+    const std::vector<Device> devices;
+
   private:
     Container(
         const std::string& output,
@@ -102,13 +104,15 @@ public:
         const std::string& name,
         const Option<pid_t>& pid,
         bool started,
-        const Option<std::string>& ipAddress)
+        const Option<std::string>& ipAddress,
+        const std::vector<Device>& devices)
       : output(output),
         id(id),
         name(name),
         pid(pid),
         started(started),
-        ipAddress(ipAddress) {}
+        ipAddress(ipAddress),
+        devices(devices) {}
   };
 
   class Image

http://git-wip-us.apache.org/repos/asf/mesos/blob/6eb2ce90/src/tests/containerizer/docker_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/docker_tests.cpp b/src/tests/containerizer/docker_tests.cpp
index c586fd2..00ccc2f 100644
--- a/src/tests/containerizer/docker_tests.cpp
+++ b/src/tests/containerizer/docker_tests.cpp
@@ -712,6 +712,77 @@ TEST_F(DockerTest, ROOT_DOCKER_NVIDIA_GPU_DeviceAllow)
   EXPECT_EQ(0, WEXITSTATUS(status->get())) << status->get();
 }
 
+
+// Tests that devices are parsed correctly from 'docker inspect'.
+//
+// TODO(bmahler): Avoid needing Nvidia GPUs to test this and
+// merge this into a more general inspect test.
+TEST_F(DockerTest, ROOT_DOCKER_NVIDIA_GPU_InspectDevices)
+{
+  const string containerName = NAME_PREFIX + "-test";
+  Resources resources = Resources::parse("cpus:1;mem:512;gpus:1").get();
+
+  Owned<Docker> docker = Docker::create(
+      tests::flags.docker,
+      tests::flags.docker_socket,
+      false).get();
+
+  Try<string> directory = environment->mkdtemp();
+  ASSERT_SOME(directory);
+
+  ContainerInfo containerInfo;
+  containerInfo.set_type(ContainerInfo::DOCKER);
+
+  ContainerInfo::DockerInfo dockerInfo;
+  dockerInfo.set_image("alpine");
+  containerInfo.mutable_docker()->CopyFrom(dockerInfo);
+
+  // Make sure the additional device is exposed (/dev/nvidiactl) and
+  // make sure that the default devices (e.g. /dev/null) are still
+  // accessible. We then sleep to allow time to inspect and verify
+  // that the device is correctly parsed from the json.
+  CommandInfo commandInfo;
+  commandInfo.set_value("touch /dev/nvidiactl && touch /dev/null && sleep 120");
+
+  Docker::Device nvidiaCtl;
+  nvidiaCtl.hostPath = Path("/dev/nvidiactl");
+  nvidiaCtl.containerPath = Path("/dev/nvidiactl");
+  nvidiaCtl.access.read = true;
+  nvidiaCtl.access.write = true;
+  nvidiaCtl.access.mknod = false;
+
+  vector<Docker::Device> devices = { nvidiaCtl };
+
+  Future<Option<int>> status = docker->run(
+      containerInfo,
+      commandInfo,
+      containerName,
+      directory.get(),
+      "/mnt/mesos/sandbox",
+      resources,
+      None(),
+      devices);
+
+  Future<Docker::Container> container =
+    docker->inspect(containerName, Milliseconds(1));
+
+  AWAIT_READY(container);
+
+  EXPECT_EQ(1u, container->devices.size());
+  EXPECT_EQ(nvidiaCtl.hostPath, container->devices[0].hostPath);
+  EXPECT_EQ(nvidiaCtl.containerPath, container->devices[0].hostPath);
+  EXPECT_EQ(nvidiaCtl.access.read, container->devices[0].access.read);
+  EXPECT_EQ(nvidiaCtl.access.write, container->devices[0].access.write);
+  EXPECT_EQ(nvidiaCtl.access.mknod, container->devices[0].access.mknod);
+
+  AWAIT_READY(docker->kill(containerName, SIGKILL));
+
+  AWAIT_READY(status);
+  ASSERT_SOME(status.get());
+  EXPECT_TRUE(WIFEXITED(status->get())) << status->get();
+  EXPECT_EQ(128 + SIGKILL, WEXITSTATUS(status->get())) << status->get();
+}
+
 } // namespace tests {
 } // namespace internal {
 } // namespace mesos {


[2/2] mesos git commit: Added container path to Docker::Device.

Posted by bm...@apache.org.
Added container path to Docker::Device.

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


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

Branch: refs/heads/master
Commit: 380f507926eff3dc48ad90e23e072207f1ed60ac
Parents: f888a1c
Author: Benjamin Mahler <bm...@apache.org>
Authored: Sun Jul 3 16:21:59 2016 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Sun Jul 3 17:19:36 2016 -0700

----------------------------------------------------------------------
 src/docker/docker.cpp                    | 12 ++++++++----
 src/docker/docker.hpp                    |  3 ++-
 src/tests/containerizer/docker_tests.cpp |  3 ++-
 3 files changed, 12 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/380f5079/src/docker/docker.cpp
----------------------------------------------------------------------
diff --git a/src/docker/docker.cpp b/src/docker/docker.cpp
index c6ed312..939f69d 100755
--- a/src/docker/docker.cpp
+++ b/src/docker/docker.cpp
@@ -664,8 +664,8 @@ Future<Option<int>> Docker::run(
 
   if (devices.isSome()) {
     foreach (const Device& device, devices.get()) {
-      if (!device.path.absolute()) {
-        return Failure("Device path '" + device.path.string() + "'"
+      if (!device.hostPath.absolute()) {
+        return Failure("Device path '" + device.hostPath.string() + "'"
                        " is not an absolute path");
       }
 
@@ -678,14 +678,18 @@ Future<Option<int>> Docker::run(
       // that an absolute path is not being provided).
       if (permissions.empty()) {
         return Failure("At least one access required for --devices:"
-                       " none specified for '" + device.path.string() + "'");
+                       " none specified for"
+                       " '" + device.hostPath.string() + "'");
       }
 
       // Note that docker silently does not handle default devices
       // passed in with restricted permissions (e.g. /dev/null), so
       // we don't bother checking this case either.
 
-      argv.push_back("--device=" + device.path.string() + ":" + permissions);
+      argv.push_back("--device=" +
+                     device.hostPath.string() + ":" +
+                     device.containerPath.string() + ":" +
+                     permissions);
     }
   }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/380f5079/src/docker/docker.hpp
----------------------------------------------------------------------
diff --git a/src/docker/docker.hpp b/src/docker/docker.hpp
index 41443c8..64333b8 100644
--- a/src/docker/docker.hpp
+++ b/src/docker/docker.hpp
@@ -55,7 +55,8 @@ public:
 
   struct Device
   {
-    Path path;
+    Path hostPath;
+    Path containerPath;
 
     struct Access
     {

http://git-wip-us.apache.org/repos/asf/mesos/blob/380f5079/src/tests/containerizer/docker_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/docker_tests.cpp b/src/tests/containerizer/docker_tests.cpp
index 49bd7c2..c586fd2 100644
--- a/src/tests/containerizer/docker_tests.cpp
+++ b/src/tests/containerizer/docker_tests.cpp
@@ -688,7 +688,8 @@ TEST_F(DockerTest, ROOT_DOCKER_NVIDIA_GPU_DeviceAllow)
   commandInfo.set_value("touch /dev/nvidiactl && touch /dev/null");
 
   Docker::Device nvidiaCtl;
-  nvidiaCtl.path = Path("/dev/nvidiactl");
+  nvidiaCtl.hostPath = Path("/dev/nvidiactl");
+  nvidiaCtl.containerPath = Path("/dev/nvidiactl");
   nvidiaCtl.access.read = true;
   nvidiaCtl.access.write = true;
   nvidiaCtl.access.mknod = true;