Merge pull request #889 from docker/725-devices-format

Improve host devices support

5 files changed, 94 insertions, 20 deletions

diff --git a/docker/utils/__init__.py b/docker/utils/__init__.py
index 70c0e9d..2e6d152 100644
--- a/docker/utils/__init__.py
+++ b/docker/utils/__init__.py
@@ -4,7 +4,7 @@ from .utils import (
     kwargs_from_env, convert_filters, datetime_to_timestamp, create_host_config,
     create_container_config, parse_bytes, ping_registry, parse_env_file,
     version_lt, version_gte, decode_json_header, split_command,
-    create_ipam_config, create_ipam_pool,
+    create_ipam_config, create_ipam_pool, parse_devices
 ) # flake8: noqa
 
 from .types import Ulimit, LogConfig # flake8: noqa
diff --git a/docker/utils/utils.py b/docker/utils/utils.py
index 3f1be64..b33c5bd 100644
--- a/docker/utils/utils.py
+++ b/docker/utils/utils.py
@@ -400,7 +400,7 @@ def parse_host(addr, platform=None):
             port = int(port)
         except Exception:
             raise errors.DockerException(
-                "Invalid port: %s", addr
+                "Invalid port: {0}".format(addr)
             )
 
     elif proto in ("http", "https") and ':' not in addr:
@@ -417,7 +417,14 @@ def parse_host(addr, platform=None):
 def parse_devices(devices):
     device_list = []
     for device in devices:
-        device_mapping = device.split(":")
+        if isinstance(device, dict):
+            device_list.append(device)
+            continue
+        if not isinstance(device, six.string_types):
+            raise errors.DockerException(
+                'Invalid device type {0}'.format(type(device))
+            )
+        device_mapping = device.split(':')
         if device_mapping:
             path_on_host = device_mapping[0]
             if len(device_mapping) > 1:
@@ -428,9 +435,11 @@ def parse_devices(devices):
                 permissions = device_mapping[2]
             else:
                 permissions = 'rwm'
-            device_list.append({"PathOnHost": path_on_host,
-                                "PathInContainer": path_in_container,
-                                "CgroupPermissions": permissions})
+            device_list.append({
+                'PathOnHost': path_on_host,
+                'PathInContainer': path_in_container,
+                'CgroupPermissions': permissions
+            })
     return device_list
 
 
diff --git a/docs/host-devices.md b/docs/host-devices.md
index a35d340..150a686 100644
--- a/docs/host-devices.md
+++ b/docs/host-devices.md
@@ -12,7 +12,18 @@ cli.create_container(
 )
 ```
 
-Each string is a single mapping using the colon (':') as the separator. So the
-above example essentially allow the container to have read write permissions to
-access the host's /dev/sda via a node named /dev/xvda in the container. The
-devices parameter is a list to allow multiple devices to be mapped.
+Each string is a single mapping using the following format:
+`<path_on_host>:<path_in_container>:<cgroup_permissions>`
+The above example allows the container to have read-write access to
+the host's `/dev/sda` via a node named `/dev/xvda` inside the container.
+
+As a more verbose alternative, each host device definition can be specified as
+a dictionary with the following keys:
+
+```python
+{
+    'PathOnHost': '/dev/sda1',
+    'PathInContainer': '/dev/xvda',
+    'CgroupPermissions': 'rwm'
+}
+```
diff --git a/docs/hostconfig.md b/docs/hostconfig.md
index 6b11798..b7b3b66 100644
--- a/docs/hostconfig.md
+++ b/docs/hostconfig.md
@@ -104,17 +104,12 @@ for example:
 * mem_swappiness (int): Tune a container's memory swappiness behavior.
   Accepts number between 0 and 100.
 * cpu_group (int): The length of a CPU period in microseconds.
-* cpu_period (int): Microseconds of CPU time that the container can get in a CPU period.
+* cpu_period (int): Microseconds of CPU time that the container can get in a
+  CPU period.
 * group_add (list): List of additional group names and/or IDs that the
   container process will run as.
-* devices (list): A list of devices to add to the container specified as dicts
-  in the form:
-  ```
-    { "PathOnHost": "/dev/deviceName",
-      "PathInContainer": "/dev/deviceName",
-      "CgroupPermissions": "mrw"
-    }
-  ```
+* devices (list): Host device bindings. See [host devices](host-devices.md)
+  for more information.
 
 **Returns** (dict) HostConfig dictionary
 
diff --git a/tests/unit/utils_test.py b/tests/unit/utils_test.py
index caf35b2..58ea746 100644
--- a/tests/unit/utils_test.py
+++ b/tests/unit/utils_test.py
@@ -18,7 +18,7 @@ from docker.utils import (
     parse_repository_tag, parse_host, convert_filters, kwargs_from_env,
     create_host_config, Ulimit, LogConfig, parse_bytes, parse_env_file,
     exclude_paths, convert_volume_binds, decode_json_header, tar,
-    split_command, create_ipam_config, create_ipam_pool,
+    split_command, create_ipam_config, create_ipam_pool, parse_devices,
 )
 from docker.utils.utils import create_endpoint_config
 from docker.utils.ports import build_port_bindings, split_port
@@ -406,6 +406,65 @@ class ParseRepositoryTagTest(base.BaseTestCase):
         )
 
 
+class ParseDeviceTest(base.BaseTestCase):
+    def test_dict(self):
+        devices = parse_devices([{
+            'PathOnHost': '/dev/sda1',
+            'PathInContainer': '/dev/mnt1',
+            'CgroupPermissions': 'r'
+        }])
+        self.assertEqual(devices[0], {
+            'PathOnHost': '/dev/sda1',
+            'PathInContainer': '/dev/mnt1',
+            'CgroupPermissions': 'r'
+        })
+
+    def test_partial_string_definition(self):
+        devices = parse_devices(['/dev/sda1'])
+        self.assertEqual(devices[0], {
+            'PathOnHost': '/dev/sda1',
+            'PathInContainer': '/dev/sda1',
+            'CgroupPermissions': 'rwm'
+        })
+
+    def test_permissionless_string_definition(self):
+        devices = parse_devices(['/dev/sda1:/dev/mnt1'])
+        self.assertEqual(devices[0], {
+            'PathOnHost': '/dev/sda1',
+            'PathInContainer': '/dev/mnt1',
+            'CgroupPermissions': 'rwm'
+        })
+
+    def test_full_string_definition(self):
+        devices = parse_devices(['/dev/sda1:/dev/mnt1:r'])
+        self.assertEqual(devices[0], {
+            'PathOnHost': '/dev/sda1',
+            'PathInContainer': '/dev/mnt1',
+            'CgroupPermissions': 'r'
+        })
+
+    def test_hybrid_list(self):
+        devices = parse_devices([
+            '/dev/sda1:/dev/mnt1:rw',
+            {
+                'PathOnHost': '/dev/sda2',
+                'PathInContainer': '/dev/mnt2',
+                'CgroupPermissions': 'r'
+            }
+        ])
+
+        self.assertEqual(devices[0], {
+            'PathOnHost': '/dev/sda1',
+            'PathInContainer': '/dev/mnt1',
+            'CgroupPermissions': 'rw'
+        })
+        self.assertEqual(devices[1], {
+            'PathOnHost': '/dev/sda2',
+            'PathInContainer': '/dev/mnt2',
+            'CgroupPermissions': 'r'
+        })
+
+
 class UtilsTest(base.BaseTestCase):
     longMessage = True