Merge branch 'sam/import-improvements' of https://github.com/ssssam/docker-py…

Merge branch 'sam/import-improvements' of https://github.com/ssssam/docker-py into ssssam-sam/import-improvements

Conflicts:
	docker/client.py
	docker/unixconn/unixconn.py
	tests/integration_test.py

docker/client.py

@@ -43,24 +43,29 @@ class Client(requests.Session):
     def __init__(self, base_url=None, version=None,
                  timeout=DEFAULT_TIMEOUT_SECONDS, tls=False):
         super(Client, self).__init__()
-        base_url = utils.parse_host(base_url)
-        if 'http+unix:///' in base_url:
-            base_url = base_url.replace('unix:/', 'unix:')
+
         if tls and not base_url.startswith('https://'):
             raise errors.TLSParameterError(
                 'If using TLS, the base_url argument must begin with '
                 '"https://".')
+
         self.base_url = base_url
         self.timeout = timeout
+
         self._auth_configs = auth.load_config()
 
-        # Use SSLAdapter for the ability to specify SSL version
-        if isinstance(tls, TLSConfig):
-            tls.configure_client(self)
-        elif tls:
-            self.mount('https://', ssladapter.SSLAdapter())
+        base_url = utils.parse_host(base_url)
+        if base_url.startswith('http+unix://'):
+            unix_socket_adapter = unixconn.UnixAdapter(base_url, timeout)
+            self.mount('http+docker://', unix_socket_adapter)
+            self.base_url = 'http+docker://localunixsocket'
         else:
-            self.mount('http+unix://', unixconn.UnixAdapter(base_url, timeout))
+            # Use SSLAdapter for the ability to specify SSL version
+            if isinstance(tls, TLSConfig):
+                tls.configure_client(self)
+            elif tls:
+                self.mount('https://', ssladapter.SSLAdapter())
+            self.base_url = base_url
 
         # version detection needs to be after unix adapter mounting
         if version is None:
@@ -576,33 +581,86 @@ class Client(requests.Session):
         return res
 
     def import_image(self, src=None, repository=None, tag=None, image=None):
+        if src:
+            if isinstance(src, six.string_types):
+                try:
+                    result = self.import_image_from_file(
+                        src, repository=repository, tag=tag)
+                except IOError:
+                    result = self.import_image_from_url(
+                        src, repository=repository, tag=tag)
+            else:
+                result = self.import_image_from_data(
+                    src, repository=repository, tag=tag)
+        elif image:
+            result = self.import_image_from_image(
+                image, repository=repository, tag=tag)
+        else:
+            raise Exception("Must specify a src or image")
+
+        return result
+
+    def import_image_from_data(self, data, repository=None, tag=None):
         u = self._url("/images/create")
         params = {
+            'fromSrc': '-',
             'repo': repository,
             'tag': tag
         }
+        headers = {
+            'Content-Type': 'application/tar',
+        }
+        return self._result(
+            self._post(u, data=data, params=params, headers=headers))
 
-        if src:
-            try:
-                # XXX: this is ways not optimal but the only way
-                # for now to import tarballs through the API
-                fic = open(src)
-                data = fic.read()
-                fic.close()
-                src = "-"
-            except IOError:
-                # file does not exists or not a file (URL)
-                data = None
-            if isinstance(src, six.string_types):
-                params['fromSrc'] = src
-                return self._result(self._post(u, data=data, params=params))
-            return self._result(self._post(u, data=src, params=params))
+    def import_image_from_file(self, filename, repository=None, tag=None):
+        u = self._url("/images/create")
+        params = {
+            'fromSrc': '-',
+            'repo': repository,
+            'tag': tag
+        }
+        headers = {
+            'Content-Type': 'application/tar',
+        }
+        with open(filename, 'rb') as f:
+            return self._result(
+                self._post(u, data=f, params=params, headers=headers,
+                           timeout=None))
 
-        if image:
-            params['fromImage'] = image
-            return self._result(self._post(u, data=None, params=params))
+    def import_image_from_stream(self, stream, repository=None, tag=None):
+        u = self._url("/images/create")
+        params = {
+            'fromSrc': '-',
+            'repo': repository,
+            'tag': tag
+        }
+        headers = {
+            'Content-Type': 'application/tar',
+            'Transfer-Encoding': 'chunked',
+        }
+        return self._result(
+            self._post(u, data=stream, params=params, headers=headers))
+
+    def import_image_from_url(self, url, repository=None, tag=None):
+        u = self._url("/images/create")
+        params = {
+            'fromSrc': url,
+            'repo': repository,
+            'tag': tag
+        }
+        return self._result(
+            self._post(u, data=None, params=params))
 
-        raise Exception("Must specify a src or image")
+    def import_image_from_image(self, image, repository=None, tag=None):
+        u = self._url("/images/create")
+        params = {
+            'fromImage': image,
+            'repo': repository,
+            'tag': tag
+        }
+        return self._result(
+            self._post(u, data=None, params=params))
 
     def info(self):
         return self._result(self._get(self._url("/info")),

docker/unixconn/unixconn.py

@@ -25,6 +25,8 @@ try:
 except ImportError:
     import urllib3
 
+RecentlyUsedContainer = urllib3._collections.RecentlyUsedContainer
+
 
 class UnixHTTPConnection(httplib.HTTPConnection, object):
     def __init__(self, base_url, unix_socket, timeout=60):
@@ -36,17 +38,9 @@ class UnixHTTPConnection(httplib.HTTPConnection, object):
     def connect(self):
         sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
         sock.settimeout(self.timeout)
-        sock.connect(self.base_url.replace("http+unix:/", ""))
+        sock.connect(self.unix_socket)
         self.sock = sock
 
-    def _extract_path(self, url):
-        # remove the base_url entirely..
-        return url.replace(self.base_url, "")
-
-    def request(self, method, url, **kwargs):
-        url = self._extract_path(self.unix_socket)
-        super(UnixHTTPConnection, self).request(method, url, **kwargs)
-
 
 class UnixHTTPConnectionPool(urllib3.connectionpool.HTTPConnectionPool):
     def __init__(self, base_url, socket_path, timeout=60):
@@ -63,24 +57,26 @@ class UnixHTTPConnectionPool(urllib3.connectionpool.HTTPConnectionPool):
 
 
 class UnixAdapter(requests.adapters.HTTPAdapter):
-    def __init__(self, base_url, timeout=60):
-        RecentlyUsedContainer = urllib3._collections.RecentlyUsedContainer
-        self.base_url = base_url
+    def __init__(self, socket_url, timeout=60):
+        socket_path = socket_url.replace('http+unix://', '')
+        if not socket_path.startswith('/'):
+            socket_path = '/' + socket_path
+        self.socket_path = socket_path
         self.timeout = timeout
         self.pools = RecentlyUsedContainer(10,
                                            dispose_func=lambda p: p.close())
         super(UnixAdapter, self).__init__()
 
-    def get_connection(self, socket_path, proxies=None):
+    def get_connection(self, url, proxies=None):
         with self.pools.lock:
-            pool = self.pools.get(socket_path)
+            pool = self.pools.get(url)
             if pool:
                 return pool
 
-            pool = UnixHTTPConnectionPool(
-                self.base_url, socket_path, self.timeout
-            )
-            self.pools[socket_path] = pool
+            pool = UnixHTTPConnectionPool(url,
+                                          self.socket_path,
+                                          self.timeout)
+            self.pools[url] = pool
 
         return pool
 

docs/api.md

@@ -345,20 +345,66 @@ layers)
 
 ## import_image
 
-Identical to the `docker import` command. If `src` is a string or unicode
-string, it will be treated as a URL to fetch the image from. To import an image
-from the local machine, `src` needs to be a file-like object or bytes
-collection.  To import from a tarball use your absolute path to your tarball.
-To load arbitrary data as tarball use whatever you want as src and your
-tarball content in data.
+Similar to the `docker import` command.
+
+If `src` is a string or unicode string, it will first be treated as a path to
+a tarball on the local system. If there is an error reading from that file,
+src will be treated as a URL instead to fetch the image from. You can also pass
+an open file handle as 'src', in which case the data will be read from that
+file.
+
+If `src` is unset but `image` is set, the `image` paramater will be taken as
+the name of an existing image to import from.
 
 **Params**:
 
-* src (str or file): Path to tarfile or URL
+* src (str or file): Path to tarfile, URL, or file-like object
 * repository (str): The repository to create
 * tag (str): The tag to apply
 * image (str): Use another image like the `FROM` Dockerfile parameter
 
+## import_image_from_data
+
+Like `.import_image()`, but allows importing in-memory bytes data.
+
+**Params**:
+
+* data (bytes collection): Bytes collection containing valid tar data
+* repository (str): The repository to create
+* tag (str): The tag to apply
+
+## import_image_from_file
+
+Like `.import_image()`, but only supports importing from a tar file on
+disk. If the file doesn't exist it will raise `IOError`.
+
+**Params**:
+
+* filename (str): Full path to a tar file.
+* repository (str): The repository to create
+* tag (str): The tag to apply
+
+## import_image_from_url
+
+Like `.import_image()`, but only supports importing from a URL.
+
+**Params**:
+
+* url (str): A URL pointing to a tar file.
+* repository (str): The repository to create
+* tag (str): The tag to apply
+
+## import_image_from_image
+
+Like `.import_image()`, but only supports importing from another image,
+like the `FROM` Dockerfile parameter.
+
+**Params**:
+
+* image (str): Image name to import from
+* repository (str): The repository to create
+* tag (str): The tag to apply
+
 ## info
 
 Display system-wide information. Identical to the `docker info` command.

tests/fake_api.py

@@ -356,7 +356,7 @@ def get_fake_stats():
     return status_code, response
 
 # Maps real api url to fake response callback
-prefix = 'http+unix://var/run/docker.sock'
+prefix = 'http+docker://localunixsocket'
 fake_responses = {
     '{0}/version'.format(prefix):
     get_fake_raw_version,

tests/integration_test.py

@@ -12,25 +12,31 @@
 #    See the License for the specific language governing permissions and
 #    limitations under the License.
 
-import time
 import base64
+import contextlib
 import json
 import io
 import os
 import shutil
 import signal
+import socket
+import tarfile
 import tempfile
+import threading
+import time
 import unittest
 import warnings
 
 import docker
 import six
 
+from six.moves import BaseHTTPServer
+from six.moves import socketserver
+
 from test import Cleanup
 
 # FIXME: missing tests for
-# export; history; import_image; insert; port; push; tag; get; load; stats;
-
+# export; history; insert; port; push; tag; get; load; stats
 DEFAULT_BASE_URL = os.environ.get('DOCKER_HOST')
 EXEC_DRIVER_IS_NATIVE = True
 
@@ -1229,6 +1235,157 @@ class TestRemoveImage(BaseTestCase):
         res = [x for x in images if x['Id'].startswith(img_id)]
         self.assertEqual(len(res), 0)
 
+
+##################
+#  IMPORT TESTS  #
+##################
+
+
+class ImportTestCase(BaseTestCase):
+    '''Base class for `docker import` test cases.'''
+
+    # Use a large file size to increase the chance of triggering any
+    # MemoryError exceptions we might hit.
+    TAR_SIZE = 512 * 1024 * 1024
+
+    def write_dummy_tar_content(self, n_bytes, tar_fd):
+        def extend_file(f, n_bytes):
+            f.seek(n_bytes - 1)
+            f.write(bytearray([65]))
+            f.seek(0)
+
+        tar = tarfile.TarFile(fileobj=tar_fd, mode='w')
+
+        with tempfile.NamedTemporaryFile() as f:
+            extend_file(f, n_bytes)
+            tarinfo = tar.gettarinfo(name=f.name, arcname='testdata')
+            tar.addfile(tarinfo, fileobj=f)
+
+        tar.close()
+
+    @contextlib.contextmanager
+    def dummy_tar_stream(self, n_bytes):
+        '''Yields a stream that is valid tar data of size n_bytes.'''
+        with tempfile.NamedTemporaryFile() as tar_file:
+            self.write_dummy_tar_content(n_bytes, tar_file)
+            tar_file.seek(0)
+            yield tar_file
+
+    @contextlib.contextmanager
+    def dummy_tar_file(self, n_bytes):
+        '''Yields the name of a valid tar file of size n_bytes.'''
+        with tempfile.NamedTemporaryFile() as tar_file:
+            self.write_dummy_tar_content(n_bytes, tar_file)
+            tar_file.seek(0)
+            yield tar_file.name
+
+
+class TestImportFromBytes(ImportTestCase):
+    '''Tests importing an image from in-memory byte data.'''
+
+    def runTest(self):
+        with self.dummy_tar_stream(n_bytes=500) as f:
+            content = f.read()
+
+        # The generic import_image() function cannot import in-memory bytes
+        # data that happens to be represented as a string type, because
+        # import_image() will try to use it as a filename and usually then
+        # trigger an exception. So we test the import_image_from_data()
+        # function instead.
+        statuses = self.client.import_image_from_data(
+            content, repository='test/import-from-bytes')
+
+        result_text = statuses.splitlines()[-1]
+        result = json.loads(result_text)
+
+        self.assertNotIn('error', result)
+
+        img_id = result['status']
+        self.tmp_imgs.append(img_id)
+
+
+class TestImportFromFile(ImportTestCase):
+    '''Tests importing an image from a tar file on disk.'''
+
+    def runTest(self):
+        with self.dummy_tar_file(n_bytes=self.TAR_SIZE) as tar_filename:
+            # statuses = self.client.import_image(
+            #     src=tar_filename, repository='test/import-from-file')
+            statuses = self.client.import_image_from_file(
+                tar_filename, repository='test/import-from-file')
+
+        result_text = statuses.splitlines()[-1]
+        result = json.loads(result_text)
+
+        self.assertNotIn('error', result)
+
+        self.assertIn('status', result)
+        img_id = result['status']
+        self.tmp_imgs.append(img_id)
+
+
+class TestImportFromStream(ImportTestCase):
+    '''Tests importing an image from a stream containing tar data.'''
+
+    def runTest(self):
+        with self.dummy_tar_stream(n_bytes=self.TAR_SIZE) as tar_stream:
+            statuses = self.client.import_image(
+                src=tar_stream, repository='test/import-from-stream')
+            # statuses = self.client.import_image_from_stream(
+            #     tar_stream, repository='test/import-from-stream')
+        result_text = statuses.splitlines()[-1]
+        result = json.loads(result_text)
+
+        self.assertNotIn('error', result)
+
+        self.assertIn('status', result)
+        img_id = result['status']
+        self.tmp_imgs.append(img_id)
+
+
+class TestImportFromURL(ImportTestCase):
+    '''Tests downloading an image over HTTP.'''
+
+    @contextlib.contextmanager
+    def temporary_http_file_server(self, stream):
+        '''Serve data from an IO stream over HTTP.'''
+
+        class Handler(BaseHTTPServer.BaseHTTPRequestHandler):
+            def do_GET(self):
+                self.send_response(200)
+                self.send_header('Content-Type', 'application/x-tar')
+                self.end_headers()
+                shutil.copyfileobj(stream, self.wfile)
+
+        server = socketserver.TCPServer(('', 0), Handler)
+        thread = threading.Thread(target=server.serve_forever)
+        thread.setDaemon(True)
+        thread.start()
+
+        yield 'http://%s:%s' % (socket.gethostname(), server.server_address[1])
+
+        server.shutdown()
+
+    def runTest(self):
+        # The crappy test HTTP server doesn't handle large files well, so use
+        # a small file.
+        TAR_SIZE = 10240
+
+        with self.dummy_tar_stream(n_bytes=TAR_SIZE) as tar_data:
+            with self.temporary_http_file_server(tar_data) as url:
+                statuses = self.client.import_image(
+                    src=url, repository='test/import-from-url')
+
+        result_text = statuses.splitlines()[-1]
+        result = json.loads(result_text)
+
+        self.assertNotIn('error', result)
+
+        self.assertIn('status', result)
+        img_id = result['status']
+        self.tmp_imgs.append(img_id)
+
+
 #################
 # BUILDER TESTS #
 #################

tests/test.py

@@ -71,7 +71,7 @@ def fake_resp(url, data=None, **kwargs):
 
 
 fake_request = mock.Mock(side_effect=fake_resp)
-url_prefix = 'http+unix://var/run/docker.sock/v{0}/'.format(
+url_prefix = 'http+docker://localunixsocket/v{0}/'.format(
     docker.client.DEFAULT_DOCKER_API_VERSION)
 
 
@@ -1412,20 +1412,24 @@ class DockerClientTest(Cleanup, unittest.TestCase):
             timeout=None
         )
 
+    def _socket_path_for_client_session(self, client):
+        socket_adapter = client.get_adapter('http+docker://')
+        return socket_adapter.socket_path
+
     def test_url_compatibility_unix(self):
         c = docker.Client(base_url="unix://socket")
 
-        assert c.base_url == "http+unix://socket"
+        assert self._socket_path_for_client_session(c) == '/socket'
 
     def test_url_compatibility_unix_triple_slash(self):
         c = docker.Client(base_url="unix:///socket")
 
-        assert c.base_url == "http+unix://socket"
+        assert self._socket_path_for_client_session(c) == '/socket'
 
     def test_url_compatibility_http_unix_triple_slash(self):
         c = docker.Client(base_url="http+unix:///socket")
 
-        assert c.base_url == "http+unix://socket"
+        assert self._socket_path_for_client_session(c) == '/socket'
 
     def test_url_compatibility_http(self):
         c = docker.Client(base_url="http://hostname:1234")
@@ -1853,12 +1857,11 @@ class DockerClientTest(Cleanup, unittest.TestCase):
             timeout=docker.client.DEFAULT_TIMEOUT_SECONDS
         )
 
-    def test_import_image_from_file(self):
-        buf = tempfile.NamedTemporaryFile(delete=False)
+    def test_import_image_from_bytes(self):
+        stream = (i for i in range(0, 100))
         try:
-            # pretent the buffer is a file
             self.client.import_image(
-                buf.name,
+                stream,
                 repository=fake_api.FAKE_REPO_NAME,
                 tag=fake_api.FAKE_TAG_NAME
             )
@@ -1870,13 +1873,14 @@ class DockerClientTest(Cleanup, unittest.TestCase):
             params={
                 'repo': fake_api.FAKE_REPO_NAME,
                 'tag': fake_api.FAKE_TAG_NAME,
-                'fromSrc': '-'
+                'fromSrc': '-',
+            },
+            headers={
+                'Content-Type': 'application/tar',
             },
-            data='',
+            data=stream,
             timeout=docker.client.DEFAULT_TIMEOUT_SECONDS
         )
-        buf.close()
-        os.remove(buf.name)
 
     def test_import_image_from_image(self):
         try: