From e577f5d61d96a0673b97e92cf60873fcbce57426 Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Tue, 12 Mar 2019 15:36:58 +0100 Subject: [PATCH 1/4] Sets a different default number of pools to SSH This is because default the number of connections in OpenSSH is 10 Signed-off-by: Ulysses Souza --- docker/api/client.py | 10 +++++++--- docker/constants.py | 6 ++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/docker/api/client.py b/docker/api/client.py index 668dfeef8..dc1d01559 100644 --- a/docker/api/client.py +++ b/docker/api/client.py @@ -22,8 +22,8 @@ from .. import auth from ..constants import ( DEFAULT_TIMEOUT_SECONDS, DEFAULT_USER_AGENT, IS_WINDOWS_PLATFORM, - DEFAULT_DOCKER_API_VERSION, STREAM_HEADER_SIZE_BYTES, DEFAULT_NUM_POOLS, - MINIMUM_DOCKER_API_VERSION + DEFAULT_DOCKER_API_VERSION, MINIMUM_DOCKER_API_VERSION, + STREAM_HEADER_SIZE_BYTES, DEFAULT_NUM_POOLS_SSH, DEFAULT_NUM_POOLS ) from ..errors import ( DockerException, InvalidVersion, TLSParameterError, @@ -101,7 +101,7 @@ class APIClient( def __init__(self, base_url=None, version=None, timeout=DEFAULT_TIMEOUT_SECONDS, tls=False, - user_agent=DEFAULT_USER_AGENT, num_pools=DEFAULT_NUM_POOLS, + user_agent=DEFAULT_USER_AGENT, num_pools=None, credstore_env=None): super(APIClient, self).__init__() @@ -132,6 +132,10 @@ def __init__(self, base_url=None, version=None, base_url = utils.parse_host( base_url, IS_WINDOWS_PLATFORM, tls=bool(tls) ) + # SSH has a different default for num_pools to all other adapters + num_pools = num_pools or DEFAULT_NUM_POOLS_SSH if \ + base_url.startswith('ssh://') else DEFAULT_NUM_POOLS + if base_url.startswith('http+unix://'): self._custom_adapter = UnixAdapter( base_url, timeout, pool_connections=num_pools diff --git a/docker/constants.py b/docker/constants.py index 1ab11ec05..dcba0de26 100644 --- a/docker/constants.py +++ b/docker/constants.py @@ -18,4 +18,10 @@ DEFAULT_USER_AGENT = "docker-sdk-python/{0}".format(version) DEFAULT_NUM_POOLS = 25 + +# The OpenSSH server default value for MaxSessions is 10 which means we can +# use up to 9, leaving the final session for the underlying SSH connection. +# For more details see: https://github.com/docker/docker-py/issues/2246 +DEFAULT_NUM_POOLS_SSH = 9 + DEFAULT_DATA_CHUNK_SIZE = 1024 * 2048 From 313f73648842b486417b2736d97381bb07a9a627 Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Wed, 13 Mar 2019 19:07:50 +0100 Subject: [PATCH 2/4] Homogenize adapters close() behaviour. - Adds a BaseHTTPAdapter with a close method to ensure that the pools is clean on close() - Makes SSHHTTPAdapter reopen a closed connection when needed like the others Signed-off-by: Ulysses Souza --- docker/api/client.py | 15 ++++++++------- docker/tls.py | 4 ++-- docker/transport/__init__.py | 8 ++++---- docker/transport/basehttpadapter.py | 6 ++++++ docker/transport/npipeconn.py | 8 +++----- docker/transport/sshconn.py | 23 ++++++++++++++++------- docker/transport/ssladapter.py | 8 +++++--- docker/transport/unixconn.py | 8 +++----- 8 files changed, 47 insertions(+), 33 deletions(-) create mode 100644 docker/transport/basehttpadapter.py diff --git a/docker/api/client.py b/docker/api/client.py index dc1d01559..b8ae48426 100644 --- a/docker/api/client.py +++ b/docker/api/client.py @@ -30,18 +30,18 @@ create_api_error_from_http_exception ) from ..tls import TLSConfig -from ..transport import SSLAdapter, UnixAdapter +from ..transport import SSLHTTPAdapter, UnixHTTPAdapter from ..utils import utils, check_resource, update_headers, config from ..utils.socket import frames_iter, consume_socket_output, demux_adaptor from ..utils.json_stream import json_stream from ..utils.proxy import ProxyConfig try: - from ..transport import NpipeAdapter + from ..transport import NpipeHTTPAdapter except ImportError: pass try: - from ..transport import SSHAdapter + from ..transport import SSHHTTPAdapter except ImportError: pass @@ -137,7 +137,7 @@ def __init__(self, base_url=None, version=None, base_url.startswith('ssh://') else DEFAULT_NUM_POOLS if base_url.startswith('http+unix://'): - self._custom_adapter = UnixAdapter( + self._custom_adapter = UnixHTTPAdapter( base_url, timeout, pool_connections=num_pools ) self.mount('http+docker://', self._custom_adapter) @@ -151,7 +151,7 @@ def __init__(self, base_url=None, version=None, 'The npipe:// protocol is only supported on Windows' ) try: - self._custom_adapter = NpipeAdapter( + self._custom_adapter = NpipeHTTPAdapter( base_url, timeout, pool_connections=num_pools ) except NameError: @@ -162,7 +162,7 @@ def __init__(self, base_url=None, version=None, self.base_url = 'http+docker://localnpipe' elif base_url.startswith('ssh://'): try: - self._custom_adapter = SSHAdapter( + self._custom_adapter = SSHHTTPAdapter( base_url, timeout, pool_connections=num_pools ) except NameError: @@ -177,7 +177,8 @@ def __init__(self, base_url=None, version=None, if isinstance(tls, TLSConfig): tls.configure_client(self) elif tls: - self._custom_adapter = SSLAdapter(pool_connections=num_pools) + self._custom_adapter = SSLHTTPAdapter( + pool_connections=num_pools) self.mount('https://', self._custom_adapter) self.base_url = base_url diff --git a/docker/tls.py b/docker/tls.py index 4900e9fdf..d4671d126 100644 --- a/docker/tls.py +++ b/docker/tls.py @@ -2,7 +2,7 @@ import ssl from . import errors -from .transport import SSLAdapter +from .transport import SSLHTTPAdapter class TLSConfig(object): @@ -105,7 +105,7 @@ def configure_client(self, client): if self.cert: client.cert = self.cert - client.mount('https://', SSLAdapter( + client.mount('https://', SSLHTTPAdapter( ssl_version=self.ssl_version, assert_hostname=self.assert_hostname, assert_fingerprint=self.assert_fingerprint, diff --git a/docker/transport/__init__.py b/docker/transport/__init__.py index d2cf2a7af..e37fc3ba2 100644 --- a/docker/transport/__init__.py +++ b/docker/transport/__init__.py @@ -1,13 +1,13 @@ # flake8: noqa -from .unixconn import UnixAdapter -from .ssladapter import SSLAdapter +from .unixconn import UnixHTTPAdapter +from .ssladapter import SSLHTTPAdapter try: - from .npipeconn import NpipeAdapter + from .npipeconn import NpipeHTTPAdapter from .npipesocket import NpipeSocket except ImportError: pass try: - from .sshconn import SSHAdapter + from .sshconn import SSHHTTPAdapter except ImportError: pass diff --git a/docker/transport/basehttpadapter.py b/docker/transport/basehttpadapter.py new file mode 100644 index 000000000..d10c115bc --- /dev/null +++ b/docker/transport/basehttpadapter.py @@ -0,0 +1,6 @@ +import requests.adapters + + +class BaseHTTPAdapter(requests.adapters.HTTPAdapter): + def close(self): + self.pools.clear() diff --git a/docker/transport/npipeconn.py b/docker/transport/npipeconn.py index ab9b90480..aa05538dd 100644 --- a/docker/transport/npipeconn.py +++ b/docker/transport/npipeconn.py @@ -1,6 +1,7 @@ import six import requests.adapters +from docker.transport.basehttpadapter import BaseHTTPAdapter from .. import constants from .npipesocket import NpipeSocket @@ -68,7 +69,7 @@ def _get_conn(self, timeout): return conn or self._new_conn() -class NpipeAdapter(requests.adapters.HTTPAdapter): +class NpipeHTTPAdapter(BaseHTTPAdapter): __attrs__ = requests.adapters.HTTPAdapter.__attrs__ + ['npipe_path', 'pools', @@ -81,7 +82,7 @@ def __init__(self, base_url, timeout=60, self.pools = RecentlyUsedContainer( pool_connections, dispose_func=lambda p: p.close() ) - super(NpipeAdapter, self).__init__() + super(NpipeHTTPAdapter, self).__init__() def get_connection(self, url, proxies=None): with self.pools.lock: @@ -103,6 +104,3 @@ def request_url(self, request, proxies): # anyway, we simply return the path URL directly. # See also: https://github.com/docker/docker-sdk-python/issues/811 return request.path_url - - def close(self): - self.pools.clear() diff --git a/docker/transport/sshconn.py b/docker/transport/sshconn.py index 0f6bb51fc..5a8ceb08b 100644 --- a/docker/transport/sshconn.py +++ b/docker/transport/sshconn.py @@ -2,6 +2,7 @@ import requests.adapters import six +from docker.transport.basehttpadapter import BaseHTTPAdapter from .. import constants if six.PY3: @@ -68,7 +69,7 @@ def _get_conn(self, timeout): return conn or self._new_conn() -class SSHAdapter(requests.adapters.HTTPAdapter): +class SSHHTTPAdapter(BaseHTTPAdapter): __attrs__ = requests.adapters.HTTPAdapter.__attrs__ + [ 'pools', 'timeout', 'ssh_client', @@ -79,15 +80,19 @@ def __init__(self, base_url, timeout=60, self.ssh_client = paramiko.SSHClient() self.ssh_client.load_system_host_keys() - parsed = six.moves.urllib_parse.urlparse(base_url) - self.ssh_client.connect( - parsed.hostname, parsed.port, parsed.username, - ) + self.base_url = base_url + self._connect() self.timeout = timeout self.pools = RecentlyUsedContainer( pool_connections, dispose_func=lambda p: p.close() ) - super(SSHAdapter, self).__init__() + super(SSHHTTPAdapter, self).__init__() + + def _connect(self): + parsed = six.moves.urllib_parse.urlparse(self.base_url) + self.ssh_client.connect( + parsed.hostname, parsed.port, parsed.username, + ) def get_connection(self, url, proxies=None): with self.pools.lock: @@ -95,6 +100,10 @@ def get_connection(self, url, proxies=None): if pool: return pool + # Connection is closed try a reconnect + if not self.ssh_client.get_transport(): + self._connect() + pool = SSHConnectionPool( self.ssh_client, self.timeout ) @@ -103,5 +112,5 @@ def get_connection(self, url, proxies=None): return pool def close(self): - self.pools.clear() + super(SSHHTTPAdapter, self).close() self.ssh_client.close() diff --git a/docker/transport/ssladapter.py b/docker/transport/ssladapter.py index 8fafec355..12de76cdc 100644 --- a/docker/transport/ssladapter.py +++ b/docker/transport/ssladapter.py @@ -7,6 +7,8 @@ from distutils.version import StrictVersion from requests.adapters import HTTPAdapter +from docker.transport.basehttpadapter import BaseHTTPAdapter + try: import requests.packages.urllib3 as urllib3 except ImportError: @@ -22,7 +24,7 @@ urllib3.connection.match_hostname = match_hostname -class SSLAdapter(HTTPAdapter): +class SSLHTTPAdapter(BaseHTTPAdapter): '''An HTTPS Transport Adapter that uses an arbitrary SSL version.''' __attrs__ = HTTPAdapter.__attrs__ + ['assert_fingerprint', @@ -34,7 +36,7 @@ def __init__(self, ssl_version=None, assert_hostname=None, self.ssl_version = ssl_version self.assert_hostname = assert_hostname self.assert_fingerprint = assert_fingerprint - super(SSLAdapter, self).__init__(**kwargs) + super(SSLHTTPAdapter, self).__init__(**kwargs) def init_poolmanager(self, connections, maxsize, block=False): kwargs = { @@ -57,7 +59,7 @@ def get_connection(self, *args, **kwargs): But we still need to take care of when there is a proxy poolmanager """ - conn = super(SSLAdapter, self).get_connection(*args, **kwargs) + conn = super(SSLHTTPAdapter, self).get_connection(*args, **kwargs) if conn.assert_hostname != self.assert_hostname: conn.assert_hostname = self.assert_hostname return conn diff --git a/docker/transport/unixconn.py b/docker/transport/unixconn.py index c59821a84..b61910324 100644 --- a/docker/transport/unixconn.py +++ b/docker/transport/unixconn.py @@ -3,6 +3,7 @@ import socket from six.moves import http_client as httplib +from docker.transport.basehttpadapter import BaseHTTPAdapter from .. import constants try: @@ -69,7 +70,7 @@ def _new_conn(self): ) -class UnixAdapter(requests.adapters.HTTPAdapter): +class UnixHTTPAdapter(BaseHTTPAdapter): __attrs__ = requests.adapters.HTTPAdapter.__attrs__ + ['pools', 'socket_path', @@ -85,7 +86,7 @@ def __init__(self, socket_url, timeout=60, self.pools = RecentlyUsedContainer( pool_connections, dispose_func=lambda p: p.close() ) - super(UnixAdapter, self).__init__() + super(UnixHTTPAdapter, self).__init__() def get_connection(self, url, proxies=None): with self.pools.lock: @@ -107,6 +108,3 @@ def request_url(self, request, proxies): # anyway, we simply return the path URL directly. # See also: https://github.com/docker/docker-py/issues/811 return request.path_url - - def close(self): - self.pools.clear() From 89485bf26ebb7cf4570549cda42543061d6b4fd7 Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Mon, 18 Mar 2019 15:42:54 +0100 Subject: [PATCH 3/4] Fix BaseHTTPAdapter for the SSL case Signed-off-by: Ulysses Souza --- docker/transport/basehttpadapter.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docker/transport/basehttpadapter.py b/docker/transport/basehttpadapter.py index d10c115bc..4d819b669 100644 --- a/docker/transport/basehttpadapter.py +++ b/docker/transport/basehttpadapter.py @@ -3,4 +3,6 @@ class BaseHTTPAdapter(requests.adapters.HTTPAdapter): def close(self): - self.pools.clear() + super(BaseHTTPAdapter, self).close() + if hasattr(self, 'pools'): + self.pools.clear() From 963818a4d21509696988bffdd755214d8268a75f Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Wed, 20 Mar 2019 11:56:24 +0100 Subject: [PATCH 4/4] Bump 3.7.1 Signed-off-by: Ulysses Souza --- docker/version.py | 2 +- docs/change-log.md | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/docker/version.py b/docker/version.py index c3edb8a35..249475f45 100644 --- a/docker/version.py +++ b/docker/version.py @@ -1,2 +1,2 @@ -version = "3.7.0" +version = "3.7.1" version_info = tuple([int(d) for d in version.split("-")[0].split(".")]) diff --git a/docs/change-log.md b/docs/change-log.md index 008a2ad27..9edfee2f8 100644 --- a/docs/change-log.md +++ b/docs/change-log.md @@ -1,6 +1,19 @@ Change log ========== +3.7.1 +----- + +[List of PRs / issues for this release](https://github.com/docker/docker-py/milestone/58?closed=1) + +### Bugfixes + +* Set a different default number (which is now 9) for SSH pools +* Adds a BaseHTTPAdapter with a close method to ensure that the +pools is clean on close() +* Makes SSHHTTPAdapter reopen a closed connection when needed +like the others + 3.7.0 -----