Merge pull request #6382 from docker/bump-1.23.2

Bump 1.23.2

CHANGELOG.md

@@ -1,6 +1,30 @@
 Change log
 ==========
 
+1.23.2 (2018-11-28)
+-------------------
+
+### Bugfixes
+
+- Reverted a 1.23.0 change that appended random strings to container names
+  created by `docker-compose up`, causing addressability issues.
+  Note: Containers created by `docker-compose run` will continue to use
+  randomly generated names to avoid collisions during parallel runs.
+
+- Fixed an issue where some `dockerfile` paths would fail unexpectedly when
+  attempting to build on Windows.
+
+- Fixed a bug where build context URLs would fail to build on Windows.
+
+- Fixed a bug that caused `run` and `exec` commands to fail for some otherwise
+  accepted values of the `--host` parameter.
+
+- Fixed an issue where overrides for the `storage_opt` and `isolation` keys in
+  service definitions weren't properly applied.
+
+- Fixed a bug where some invalid Compose files would raise an uncaught
+  exception during validation.
+
 1.23.1 (2018-11-01)
 -------------------
 

compose/__init__.py

@@ -1,4 +1,4 @@
 from __future__ import absolute_import
 from __future__ import unicode_literals
 
-__version__ = '1.23.1'
+__version__ = '1.23.2'

compose/cli/main.py

@@ -1452,7 +1452,9 @@ def call_docker(args, dockeropts):
     if verify:
         tls_options.append('--tlsverify')
     if host:
-        tls_options.extend(['--host', host.lstrip('=')])
+        tls_options.extend(
+            ['--host', re.sub(r'^https?://', 'tcp://', host.lstrip('='))]
+        )
 
     args = [executable_path] + tls_options + args
     log.debug(" ".join(map(pipes.quote, args)))

compose/config/__init__.py

@@ -6,6 +6,7 @@ from . import environment
 from .config import ConfigurationError
 from .config import DOCKER_CONFIG_KEYS
 from .config import find
+from .config import is_url
 from .config import load
 from .config import merge_environment
 from .config import merge_labels

compose/config/config.py

@@ -91,6 +91,7 @@ DOCKER_CONFIG_KEYS = [
     'healthcheck',
     'image',
     'ipc',
+    'isolation',
     'labels',
     'links',
     'mac_address',
@@ -1042,6 +1043,7 @@ def merge_service_dicts(base, override, version):
     md.merge_mapping('networks', parse_networks)
     md.merge_mapping('sysctls', parse_sysctls)
     md.merge_mapping('depends_on', parse_depends_on)
+    md.merge_mapping('storage_opt', parse_flat_dict)
     md.merge_sequence('links', ServiceLink.parse)
     md.merge_sequence('secrets', types.ServiceSecret.parse)
     md.merge_sequence('configs', types.ServiceConfig.parse)

compose/config/validation.py

@@ -330,7 +330,10 @@ def handle_generic_error(error, path):
 
 
 def parse_key_from_error_msg(error):
-    return error.message.split("'")[1]
+    try:
+        return error.message.split("'")[1]
+    except IndexError:
+        return error.message.split('(')[1].split(' ')[0].strip("'")
 
 
 def path_string(path):

compose/container.py

@@ -7,6 +7,7 @@ import six
 from docker.errors import ImageNotFound
 
 from .const import LABEL_CONTAINER_NUMBER
+from .const import LABEL_ONE_OFF
 from .const import LABEL_PROJECT
 from .const import LABEL_SERVICE
 from .const import LABEL_SLUG
@@ -82,12 +83,16 @@ class Container(object):
     @property
     def name_without_project(self):
         if self.name.startswith('{0}_{1}'.format(self.project, self.service)):
-            return '{0}_{1}{2}'.format(self.service, self.number, '_' + self.slug if self.slug else '')
+            return '{0}_{1}'.format(self.service, self.number if self.number is not None else self.slug)
         else:
             return self.name
 
     @property
     def number(self):
+        if self.one_off:
+            # One-off containers are no longer assigned numbers and use slugs instead.
+            return None
+
         number = self.labels.get(LABEL_CONTAINER_NUMBER)
         if not number:
             raise ValueError("Container {0} does not have a {1} label".format(
@@ -104,6 +109,10 @@ class Container(object):
     def full_slug(self):
         return self.labels.get(LABEL_SLUG)
 
+    @property
+    def one_off(self):
+        return self.labels.get(LABEL_ONE_OFF) == 'True'
+
     @property
     def ports(self):
         self.inspect_if_not_inspected()

compose/service.py

@@ -27,6 +27,7 @@ from . import __version__
 from . import const
 from . import progress_stream
 from .config import DOCKER_CONFIG_KEYS
+from .config import is_url
 from .config import merge_environment
 from .config import merge_labels
 from .config.errors import DependencyError
@@ -85,6 +86,7 @@ HOST_CONFIG_KEYS = [
     'group_add',
     'init',
     'ipc',
+    'isolation',
     'read_only',
     'log_driver',
     'log_opt',
@@ -127,7 +129,7 @@ class NoSuchImageError(Exception):
     pass
 
 
-ServiceName = namedtuple('ServiceName', 'project service number slug')
+ServiceName = namedtuple('ServiceName', 'project service number')
 
 
 ConvergencePlan = namedtuple('ConvergencePlan', 'action containers')
@@ -443,13 +445,11 @@ class Service(object):
 
         containers, errors = parallel_execute(
             [
-                ServiceName(self.project, self.name, index, generate_random_id())
+                ServiceName(self.project, self.name, index)
                 for index in range(i, i + scale)
             ],
             lambda service_name: create_and_start(self, service_name.number),
-            lambda service_name: self.get_container_name(
+            lambda service_name: self.get_container_name(service_name.service, service_name.number),
-                service_name.service, service_name.number, service_name.slug
-            ),
             "Creating"
         )
         for error in errors.values():
@@ -734,16 +734,18 @@ class Service(object):
         return [s.source.name for s in self.volumes_from if isinstance(s.source, Service)]
 
     def _next_container_number(self, one_off=False):
+        if one_off:
+            return None
         containers = itertools.chain(
             self._fetch_containers(
                 all=True,
-                filters={'label': self.labels(one_off=one_off)}
+                filters={'label': self.labels(one_off=False)}
             ), self._fetch_containers(
                 all=True,
-                filters={'label': self.labels(one_off=one_off, legacy=True)}
+                filters={'label': self.labels(one_off=False, legacy=True)}
             )
         )
-        numbers = [c.number for c in containers]
+        numbers = [c.number for c in containers if c.number is not None]
         return 1 if not numbers else max(numbers) + 1
 
     def _fetch_containers(self, **fetch_options):
@@ -821,7 +823,7 @@ class Service(object):
             one_off=False,
             previous_container=None):
         add_config_hash = (not one_off and not override_options)
-        slug = generate_random_id() if previous_container is None else previous_container.full_slug
+        slug = generate_random_id() if one_off else None
 
         container_options = dict(
             (k, self.options[k])
@@ -830,7 +832,7 @@ class Service(object):
         container_options.update(override_options)
 
         if not container_options.get('name'):
-            container_options['name'] = self.get_container_name(self.name, number, slug, one_off)
+            container_options['name'] = self.get_container_name(self.name, number, slug)
 
         container_options.setdefault('detach', True)
 
@@ -1118,12 +1120,12 @@ class Service(object):
     def custom_container_name(self):
         return self.options.get('container_name')
 
-    def get_container_name(self, service_name, number, slug, one_off=False):
+    def get_container_name(self, service_name, number, slug=None):
-        if self.custom_container_name and not one_off:
+        if self.custom_container_name and slug is None:
             return self.custom_container_name
 
         container_name = build_container_name(
-            self.project, service_name, number, slug, one_off,
+            self.project, service_name, number, slug,
         )
         ext_links_origins = [l.split(':')[0] for l in self.options.get('external_links', [])]
         if container_name in ext_links_origins:
@@ -1380,13 +1382,13 @@ class ServiceNetworkMode(object):
 # Names
 
 
-def build_container_name(project, service, number, slug, one_off=False):
+def build_container_name(project, service, number, slug=None):
     bits = [project.lstrip('-_'), service]
-    if one_off:
+    if slug:
-        bits.append('run')
+        bits.extend(['run', truncate_id(slug)])
-    return '_'.join(
+    else:
-        bits + ([str(number), truncate_id(slug)] if slug else [str(number)])
+        bits.append(str(number))
-    )
+    return '_'.join(bits)
 
 
 # Images
@@ -1575,8 +1577,10 @@ def build_mount(mount_spec):
 def build_container_labels(label_options, service_labels, number, config_hash, slug):
     labels = dict(label_options or {})
     labels.update(label.split('=', 1) for label in service_labels)
-    labels[LABEL_CONTAINER_NUMBER] = str(number)
+    if number is not None:
-    labels[LABEL_SLUG] = slug
+        labels[LABEL_CONTAINER_NUMBER] = str(number)
+    if slug is not None:
+        labels[LABEL_SLUG] = slug
     labels[LABEL_VERSION] = __version__
 
     if config_hash:
@@ -1673,7 +1677,7 @@ def rewrite_build_path(path):
     if not six.PY3 and not IS_WINDOWS_PLATFORM:
         path = path.encode('utf8')
 
-    if IS_WINDOWS_PLATFORM and not path.startswith(WINDOWS_LONGPATH_PREFIX):
+    if IS_WINDOWS_PLATFORM and not is_url(path) and not path.startswith(WINDOWS_LONGPATH_PREFIX):
         path = WINDOWS_LONGPATH_PREFIX + os.path.normpath(path)
 
     return path

requirements.txt

@@ -3,7 +3,7 @@ cached-property==1.3.0
 certifi==2017.4.17
 chardet==3.0.4
 colorama==0.4.0; sys_platform == 'win32'
-docker==3.5.0
+docker==3.6.0
 docker-pycreds==0.3.0
 dockerpty==0.4.1
 docopt==0.6.2

script/run/run.sh

@@ -15,7 +15,7 @@
 
 set -e
 
-VERSION="1.23.1"
+VERSION="1.23.2"
 IMAGE="docker/compose:$VERSION"
 
 

setup.py

@@ -36,7 +36,7 @@ install_requires = [
     'requests >= 2.6.1, != 2.11.0, != 2.12.2, != 2.18.0, < 2.21',
     'texttable >= 0.9.0, < 0.10',
     'websocket-client >= 0.32.0, < 1.0',
-    'docker >= 3.5.0, < 4.0',
+    'docker >= 3.6.0, < 4.0',
     'dockerpty >= 0.4.1, < 0.5',
     'six >= 1.3.0, < 2',
     'jsonschema >= 2.5.1, < 3',

tests/acceptance/cli_test.py

@@ -965,11 +965,11 @@ class CLITestCase(DockerClientTestCase):
         result = self.dispatch(['down', '--rmi=local', '--volumes'])
         assert 'Stopping v2-full_web_1' in result.stderr
         assert 'Stopping v2-full_other_1' in result.stderr
-        assert 'Stopping v2-full_web_run_2' in result.stderr
+        assert 'Stopping v2-full_web_run_' in result.stderr
         assert 'Removing v2-full_web_1' in result.stderr
         assert 'Removing v2-full_other_1' in result.stderr
-        assert 'Removing v2-full_web_run_1' in result.stderr
+        assert 'Removing v2-full_web_run_' in result.stderr
-        assert 'Removing v2-full_web_run_2' in result.stderr
+        assert 'Removing v2-full_web_run_' in result.stderr
         assert 'Removing volume v2-full_data' in result.stderr
         assert 'Removing image v2-full_web' in result.stderr
         assert 'Removing image busybox' not in result.stderr
@@ -1031,8 +1031,8 @@ class CLITestCase(DockerClientTestCase):
             stopped=True
         )[0].name_without_project
 
-        assert '{} | simple'.format(simple_name) in result.stdout
+        assert '{}   | simple'.format(simple_name) in result.stdout
-        assert '{} | another'.format(another_name) in result.stdout
+        assert '{}  | another'.format(another_name) in result.stdout
         assert '{} exited with code 0'.format(simple_name) in result.stdout
         assert '{} exited with code 0'.format(another_name) in result.stdout
 
@@ -2332,10 +2332,9 @@ class CLITestCase(DockerClientTestCase):
 
         result = wait_on_process(proc)
 
-        assert len(re.findall(
+        assert result.stdout.count(
-            r'logs-restart-composefile_another_1_[a-f0-9]{12} exited with code 1',
+            r'logs-restart-composefile_another_1 exited with code 1'
-            result.stdout
+        ) == 3
-        )) == 3
         assert result.stdout.count('world') == 3
 
     def test_logs_default(self):
@@ -2706,7 +2705,7 @@ class CLITestCase(DockerClientTestCase):
         )
 
         result = wait_on_process(proc, returncode=1)
-        assert re.findall(r'exit-code-from_another_1_[a-f0-9]{12} exited with code 1', result.stdout)
+        assert 'exit-code-from_another_1 exited with code 1' in result.stdout
 
     def test_exit_code_from_signal_stop(self):
         self.base_dir = 'tests/fixtures/exit-code-from'

tests/integration/service_test.py

@@ -32,7 +32,6 @@ from compose.const import LABEL_CONTAINER_NUMBER
 from compose.const import LABEL_ONE_OFF
 from compose.const import LABEL_PROJECT
 from compose.const import LABEL_SERVICE
-from compose.const import LABEL_SLUG
 from compose.const import LABEL_VERSION
 from compose.container import Container
 from compose.errors import OperationFailedError
@@ -1269,16 +1268,15 @@ class ServiceTest(DockerClientTestCase):
         test that those containers are restarted and not removed/recreated.
         """
         service = self.create_service('web')
-        valid_numbers = [service._next_container_number(), service._next_container_number()]
+        service.create_container(number=1)
-        service.create_container(number=valid_numbers[0])
+        service.create_container(number=2)
-        service.create_container(number=valid_numbers[1])
 
         ParallelStreamWriter.instance = None
         with mock.patch('sys.stderr', new_callable=StringIO) as mock_stderr:
             service.scale(2)
         for container in service.containers():
             assert container.is_running
-            assert container.number in valid_numbers
+            assert container.number in [1, 2]
 
         captured_output = mock_stderr.getvalue()
         assert 'Creating' not in captured_output
@@ -1610,7 +1608,6 @@ class ServiceTest(DockerClientTestCase):
         labels = ctnr.labels.items()
         for pair in expected.items():
             assert pair in labels
-        assert ctnr.labels[LABEL_SLUG] == ctnr.full_slug
 
     def test_empty_labels(self):
         labels_dict = {'foo': '', 'bar': ''}

tests/integration/state_test.py

@@ -198,14 +198,14 @@ class ProjectWithDependenciesTest(ProjectTestCase):
         db, = [c for c in containers if c.service == 'db']
 
         assert set(get_links(web)) == {
-            'composetest_db_{}_{}'.format(db.number, db.slug),
+            'composetest_db_1',
             'db',
-            'db_{}_{}'.format(db.number, db.slug)
+            'db_1',
         }
         assert set(get_links(nginx)) == {
-            'composetest_web_{}_{}'.format(web.number, web.slug),
+            'composetest_web_1',
             'web',
-            'web_{}_{}'.format(web.number, web.slug)
+            'web_1',
         }
 
 

tests/unit/cli/main_test.py

@@ -155,6 +155,14 @@ class TestCallDocker(object):
             'docker', '--host', 'tcp://mydocker.net:2333', 'ps'
         ]
 
+    def test_with_http_host(self):
+        with mock.patch('subprocess.call') as fake_call:
+            call_docker(['ps'], {'--host': 'http://mydocker.net:2333'})
+
+        assert fake_call.call_args[0][0] == [
+            'docker', '--host', 'tcp://mydocker.net:2333', 'ps',
+        ]
+
     def test_with_host_option_shorthand_equal(self):
         with mock.patch('subprocess.call') as fake_call:
             call_docker(['ps'], {'--host': '=tcp://mydocker.net:2333'})

tests/unit/config/config_test.py

@@ -613,6 +613,19 @@ class ConfigTest(unittest.TestCase):
             excinfo.exconly()
         )
 
+    def test_config_integer_service_property_raise_validation_error(self):
+        with pytest.raises(ConfigurationError) as excinfo:
+            config.load(
+                build_config_details({
+                    'version': '2.1',
+                    'services': {'foobar': {'image': 'busybox', 1234: 'hah'}}
+                }, 'working_dir', 'filename.yml')
+            )
+
+        assert (
+            "Unsupported config option for services.foobar: '1234'" in excinfo.exconly()
+        )
+
     def test_config_invalid_service_name_raise_validation_error(self):
         with pytest.raises(ConfigurationError) as excinfo:
             config.load(
@@ -2644,6 +2657,45 @@ class ConfigTest(unittest.TestCase):
             ['c 7:128 rwm', 'x 3:244 rw', 'f 0:128 n']
         )
 
+    def test_merge_isolation(self):
+        base = {
+            'image': 'bar',
+            'isolation': 'default',
+        }
+
+        override = {
+            'isolation': 'hyperv',
+        }
+
+        actual = config.merge_service_dicts(base, override, V2_3)
+        assert actual == {
+            'image': 'bar',
+            'isolation': 'hyperv',
+        }
+
+    def test_merge_storage_opt(self):
+        base = {
+            'image': 'bar',
+            'storage_opt': {
+                'size': '1G',
+                'readonly': 'false',
+            }
+        }
+
+        override = {
+            'storage_opt': {
+                'size': '2G',
+                'encryption': 'aes',
+            }
+        }
+
+        actual = config.merge_service_dicts(base, override, V2_3)
+        assert actual['storage_opt'] == {
+            'size': '2G',
+            'readonly': 'false',
+            'encryption': 'aes',
+        }
+
     def test_external_volume_config(self):
         config_details = build_config_details({
             'version': '2',

tests/unit/container_test.py

@@ -5,6 +5,7 @@ import docker
 
 from .. import mock
 from .. import unittest
+from compose.const import LABEL_ONE_OFF
 from compose.const import LABEL_SLUG
 from compose.container import Container
 from compose.container import get_container_name
@@ -32,7 +33,6 @@ class ContainerTest(unittest.TestCase):
                     "com.docker.compose.project": "composetest",
                     "com.docker.compose.service": "web",
                     "com.docker.compose.container-number": "7",
-                    "com.docker.compose.slug": "092cd63296fdc446ad432d3905dd1fcbe12a2ba6b52"
                 },
             }
         }
@@ -88,20 +88,23 @@ class ContainerTest(unittest.TestCase):
         assert container.name == "composetest_db_1"
 
     def test_name_without_project(self):
-        self.container_dict['Name'] = "/composetest_web_7_092cd63296fd"
+        self.container_dict['Name'] = "/composetest_web_7"
         container = Container(None, self.container_dict, has_been_inspected=True)
-        assert container.name_without_project == "web_7_092cd63296fd"
+        assert container.name_without_project == "web_7"
 
     def test_name_without_project_custom_container_name(self):
         self.container_dict['Name'] = "/custom_name_of_container"
         container = Container(None, self.container_dict, has_been_inspected=True)
         assert container.name_without_project == "custom_name_of_container"
 
-    def test_name_without_project_noslug(self):
+    def test_name_without_project_one_off(self):
-        self.container_dict['Name'] = "/composetest_web_7"
+        self.container_dict['Name'] = "/composetest_web_092cd63296f"
-        del self.container_dict['Config']['Labels'][LABEL_SLUG]
+        self.container_dict['Config']['Labels'][LABEL_SLUG] = (
+            "092cd63296fdc446ad432d3905dd1fcbe12a2ba6b52"
+        )
+        self.container_dict['Config']['Labels'][LABEL_ONE_OFF] = 'True'
         container = Container(None, self.container_dict, has_been_inspected=True)
-        assert container.name_without_project == 'web_7'
+        assert container.name_without_project == 'web_092cd63296fd'
 
     def test_inspect_if_not_inspected(self):
         mock_client = mock.create_autospec(docker.APIClient)

tests/unit/service_test.py

@@ -21,6 +21,7 @@ from compose.const import LABEL_ONE_OFF
 from compose.const import LABEL_PROJECT
 from compose.const import LABEL_SERVICE
 from compose.const import SECRETS_PATH
+from compose.const import WINDOWS_LONGPATH_PREFIX
 from compose.container import Container
 from compose.errors import OperationFailedError
 from compose.parallel import ParallelStreamWriter
@@ -38,6 +39,7 @@ from compose.service import NeedsBuildError
 from compose.service import NetworkMode
 from compose.service import NoSuchImageError
 from compose.service import parse_repository_tag
+from compose.service import rewrite_build_path
 from compose.service import Service
 from compose.service import ServiceNetworkMode
 from compose.service import warn_on_masked_volume
@@ -173,10 +175,10 @@ class ServiceTest(unittest.TestCase):
     def test_self_reference_external_link(self):
         service = Service(
             name='foo',
-            external_links=['default_foo_1_bdfa3ed91e2c']
+            external_links=['default_foo_1']
         )
         with pytest.raises(DependencyError):
-            service.get_container_name('foo', 1, 'bdfa3ed91e2c')
+            service.get_container_name('foo', 1)
 
     def test_mem_reservation(self):
         self.mock_client.create_host_config.return_value = {}
@@ -1486,3 +1488,28 @@ class ServiceSecretTest(unittest.TestCase):
 
         assert volumes[0].source == secret1['file']
         assert volumes[0].target == '{}/{}'.format(SECRETS_PATH, secret1['secret'].source)
+
+
+class RewriteBuildPathTest(unittest.TestCase):
+    @mock.patch('compose.service.IS_WINDOWS_PLATFORM', True)
+    def test_rewrite_url_no_prefix(self):
+        urls = [
+            'http://test.com',
+            'https://test.com',
+            'git://test.com',
+            'github.com/test/test',
+            'git@test.com',
+        ]
+        for u in urls:
+            assert rewrite_build_path(u) == u
+
+    @mock.patch('compose.service.IS_WINDOWS_PLATFORM', True)
+    def test_rewrite_windows_path(self):
+        assert rewrite_build_path('C:\\context') == WINDOWS_LONGPATH_PREFIX + 'C:\\context'
+        assert rewrite_build_path(
+            rewrite_build_path('C:\\context')
+        ) == rewrite_build_path('C:\\context')
+
+    @mock.patch('compose.service.IS_WINDOWS_PLATFORM', False)
+    def test_rewrite_unix_path(self):
+        assert rewrite_build_path('/context') == '/context'