From 33b8fd6eecae3a5dc2c9476a409cf894354bf994 Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Tue, 26 Mar 2019 15:15:40 +0100 Subject: [PATCH 1/4] Xfail test_attach_stream_and_cancel on TLS This test is quite flaky on ssl integration test Signed-off-by: Ulysses Souza --- tests/integration/api_container_test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/integration/api_container_test.py b/tests/integration/api_container_test.py index 83df3424..558441e0 100644 --- a/tests/integration/api_container_test.py +++ b/tests/integration/api_container_test.py @@ -1080,7 +1080,6 @@ class KillTest(BaseAPIIntegrationTest): class PortTest(BaseAPIIntegrationTest): def test_port(self): - port_bindings = { '1111': ('127.0.0.1', '4567'), '2222': ('127.0.0.1', '4568') @@ -1260,6 +1259,9 @@ class AttachContainerTest(BaseAPIIntegrationTest): @pytest.mark.timeout(5) @pytest.mark.skipif(os.environ.get('DOCKER_HOST', '').startswith('ssh://'), reason='No cancellable streams over SSH') + @pytest.mark.xfail(condition=os.environ.get('DOCKER_TLS_VERIFY') or + os.environ.get('DOCKER_CERT_PATH'), + reason='Flaky test on TLS') def test_attach_stream_and_cancel(self): container = self.client.create_container( BUSYBOX, 'sh -c "echo hello && sleep 60"', From b05bfd7b22dd23e425cbe9838957e0f3460d2417 Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Tue, 26 Mar 2019 12:07:25 +0100 Subject: [PATCH 2/4] Fix base_url to keep TCP protocol This fix lets the responsability of changing the protocol to `parse_host` afterwards, letting `base_url` with the original value. Signed-off-by: Ulysses Souza --- docker/utils/utils.py | 4 +--- tests/unit/utils_test.py | 12 ++++++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/docker/utils/utils.py b/docker/utils/utils.py index 61e307ad..7819ace4 100644 --- a/docker/utils/utils.py +++ b/docker/utils/utils.py @@ -352,9 +352,7 @@ def kwargs_from_env(ssl_version=None, assert_hostname=None, environment=None): params = {} if host: - params['base_url'] = ( - host.replace('tcp://', 'https://') if enable_tls else host - ) + params['base_url'] = host if not enable_tls: return params diff --git a/tests/unit/utils_test.py b/tests/unit/utils_test.py index a4e9c9c5..ee660a36 100644 --- a/tests/unit/utils_test.py +++ b/tests/unit/utils_test.py @@ -11,6 +11,7 @@ import unittest from docker.api.client import APIClient +from docker.constants import IS_WINDOWS_PLATFORM from docker.errors import DockerException from docker.utils import ( convert_filters, convert_volume_binds, decode_json_header, kwargs_from_env, @@ -83,15 +84,17 @@ class KwargsFromEnvTest(unittest.TestCase): DOCKER_CERT_PATH=TEST_CERT_DIR, DOCKER_TLS_VERIFY='1') kwargs = kwargs_from_env(assert_hostname=False) - assert 'https://192.168.59.103:2376' == kwargs['base_url'] + assert 'tcp://192.168.59.103:2376' == kwargs['base_url'] assert 'ca.pem' in kwargs['tls'].ca_cert assert 'cert.pem' in kwargs['tls'].cert[0] assert 'key.pem' in kwargs['tls'].cert[1] assert kwargs['tls'].assert_hostname is False assert kwargs['tls'].verify + + parsed_host = parse_host(kwargs['base_url'], IS_WINDOWS_PLATFORM, True) try: client = APIClient(**kwargs) - assert kwargs['base_url'] == client.base_url + assert parsed_host == client.base_url assert kwargs['tls'].ca_cert == client.verify assert kwargs['tls'].cert == client.cert except TypeError as e: @@ -102,15 +105,16 @@ class KwargsFromEnvTest(unittest.TestCase): DOCKER_CERT_PATH=TEST_CERT_DIR, DOCKER_TLS_VERIFY='') kwargs = kwargs_from_env(assert_hostname=True) - assert 'https://192.168.59.103:2376' == kwargs['base_url'] + assert 'tcp://192.168.59.103:2376' == kwargs['base_url'] assert 'ca.pem' in kwargs['tls'].ca_cert assert 'cert.pem' in kwargs['tls'].cert[0] assert 'key.pem' in kwargs['tls'].cert[1] assert kwargs['tls'].assert_hostname is True assert kwargs['tls'].verify is False + parsed_host = parse_host(kwargs['base_url'], IS_WINDOWS_PLATFORM, True) try: client = APIClient(**kwargs) - assert kwargs['base_url'] == client.base_url + assert parsed_host == client.base_url assert kwargs['tls'].cert == client.cert assert not kwargs['tls'].verify except TypeError as e: From cd59491b9a595a70e9a5b33cd0e09d540c738ee2 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Thu, 28 Mar 2019 11:42:02 +0000 Subject: [PATCH 3/4] scripts/version.py: Use regex grouping to extract the version The `lstrip` and `rstrip` functions take a set of characters to remove, not a prefix/suffix. Thus `rstrip('-x86_64')` will remove any trailing characters in the string `'-x86_64'` in any order (in effect it strips the suffix matching the regex `[-_x468]*`). So with `18.09.4` it removes the `4` suffix resulting in trying to `int('')` later on: Traceback (most recent call last): File "/src/scripts/versions.py", line 80, in main() File "/src/scripts/versions.py", line 73, in main versions, reverse=True, key=operator.attrgetter('order') File "/src/scripts/versions.py", line 52, in order return (int(self.major), int(self.minor), int(self.patch)) + stage ValueError: invalid literal for int() with base 10: '' Since we no longer need to check for the arch suffix (since it no longer appears in the URLs we are traversing) we could just drop the `rstrip` and invent a local prefix stripping helper to replace `lstrip('docker-')`. Instead lets take advantage of the behaviour of `re.findall` which is that if the regex contains a single `()` match that will be returned. This lets us match exactly the sub-section of the regex we require. While editing the regex, also ensure that the suffix is precisely `.tgz` and not merely `tgz` by adding an explicit `\.`, previously the literal `.` would be swallowed by the `.*` instead. Signed-off-by: Ian Campbell --- scripts/versions.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/scripts/versions.py b/scripts/versions.py index 7ad1d56a..93fe0d7f 100644 --- a/scripts/versions.py +++ b/scripts/versions.py @@ -62,13 +62,9 @@ def main(): for url in [base_url.format(cat) for cat in categories]: res = requests.get(url) content = res.text - versions = [ - Version.parse( - v.strip('"').lstrip('docker-').rstrip('.tgz').rstrip('-x86_64') - ) for v in re.findall( - r'"docker-[0-9]+\.[0-9]+\.[0-9]+-?.*tgz"', content - ) - ] + versions = [Version.parse(v) for v in re.findall( + r'"docker-([0-9]+\.[0-9]+\.[0-9]+)-?.*tgz"', content + )] sorted_versions = sorted( versions, reverse=True, key=operator.attrgetter('order') ) From 8f2d9a5687c2358cea364a1d4d64fb1c8d0af7c4 Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Thu, 28 Mar 2019 14:23:19 +0100 Subject: [PATCH 4/4] Bump 3.7.2 Signed-off-by: Ulysses Souza --- docker/version.py | 2 +- docs/change-log.md | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/docker/version.py b/docker/version.py index 249475f4..8f81f0d5 100644 --- a/docker/version.py +++ b/docker/version.py @@ -1,2 +1,2 @@ -version = "3.7.1" +version = "3.7.2" 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 9edfee2f..d7c33611 100644 --- a/docs/change-log.md +++ b/docs/change-log.md @@ -1,6 +1,17 @@ Change log ========== +3.7.2 +----- + +[List of PRs / issues for this release](https://github.com/docker/docker-py/milestone/59?closed=1) + +### Bugfixes + +* Fix base_url to keep TCP protocol on utils.py by letting the responsability of changing the +protocol to `parse_host` afterwards, letting `base_url` with the original value. +* XFAIL test_attach_stream_and_cancel on TLS + 3.7.1 -----