From 42b91885a29885daee8ef46b8e6364d68c09a4f3 Mon Sep 17 00:00:00 2001 From: Sam Thursfield Date: Wed, 17 Sep 2014 14:38:14 +0000 Subject: [PATCH] Rework Unix connection code so that 'chunked' transfers work This allows streaming a system to the 'import' command, rather than having to read the whole thing into memory before sending it. Previously both the UnixAdapter and the docker.Client objects would track the 'base URL' of the Docker daemon (socket path in the case of local Unix-domain socket connections). The Client object would construct URLs which contained the path to the socket with the path of the Docker API call appended. The UnixHTTPConnection instance would then remove the known socket path from the URL. This relied on all calls going through the HTTPConnection.request() function, where the URL could be rewritten. In the case of 'chunked' HTTP POST requests this doesn't happen, so such calls would request a path still including the socket path and would receive a 404 error. The client now constructs URLs containing just the path of the desired API endpoint, and expects the Unix socket transport to know the path to the Docker daemon's socket. --- docker/client.py | 24 ++++++++++++++---------- docker/unixconn/unixconn.py | 27 +++++++++++---------------- tests/fake_api.py | 2 +- tests/test.py | 12 ++++++++---- 4 files changed, 34 insertions(+), 31 deletions(-) diff --git a/docker/client.py b/docker/client.py index 1aa9ed6d..d6c96a2c 100644 --- a/docker/client.py +++ b/docker/client.py @@ -42,31 +42,35 @@ class Client(requests.Session): def __init__(self, base_url=None, version=DEFAULT_DOCKER_API_VERSION, 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://".') + if not isinstance(version, six.string_types): raise errors.DockerException( 'version parameter must be a string. Found {0}'.format( type(version).__name__ ) ) - self.base_url = base_url + self._version = version 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 def _set_request_timeout(self, kwargs): """Prepare the kwargs for an HTTP request by inserting the timeout diff --git a/docker/unixconn/unixconn.py b/docker/unixconn/unixconn.py index 295d426c..5b136b76 100644 --- a/docker/unixconn/unixconn.py +++ b/docker/unixconn/unixconn.py @@ -41,17 +41,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(connectionpool.HTTPConnectionPool): def __init__(self, base_url, socket_path, timeout=60): @@ -67,23 +59,26 @@ class UnixHTTPConnectionPool(connectionpool.HTTPConnectionPool): class UnixAdapter(requests.adapters.HTTPAdapter): - def __init__(self, base_url, timeout=60): - 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, + pool = UnixHTTPConnectionPool(url, + self.socket_path, self.timeout) - self.pools[socket_path] = pool + self.pools[url] = pool return pool diff --git a/tests/fake_api.py b/tests/fake_api.py index a006f512..9807fe5f 100644 --- a/tests/fake_api.py +++ b/tests/fake_api.py @@ -324,7 +324,7 @@ def post_fake_tag_image(): # Maps real api url to fake response callback -prefix = 'http+unix://var/run/docker.sock' +prefix = 'http+docker://localunixsocket' fake_responses = { '{1}/{0}/version'.format(CURRENT_VERSION, prefix): get_fake_version, diff --git a/tests/test.py b/tests/test.py index bd56d646..4a17a8ab 100644 --- a/tests/test.py +++ b/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) @@ -1267,20 +1267,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")