Don't append slugs to containers created by "up"

This change reverts the new naming convention introduced in 1.23 for service containers.
One-off containers will now use a slug instead of a sequential number as they do not
present addressability concerns and benefit from being capable of running in parallel.

Signed-off-by: Joffrey F <[email protected]>

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

@@ -736,16 +736,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):
@@ -823,7 +825,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])
@@ -832,7 +834,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)
 
@@ -1120,12 +1122,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):
-        if self.custom_container_name and not one_off:
+    def get_container_name(self, service_name, number, slug=None):
+        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:
@@ -1382,13 +1384,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:
-        bits.append('run')
-    return '_'.join(
-        bits + ([str(number), truncate_id(slug)] if slug else [str(number)])
-    )
+    if slug:
+        bits.extend(['run', truncate_id(slug)])
+    else:
+        bits.append(str(number))
+    return '_'.join(bits)
 
 
 # Images
@@ -1577,8 +1579,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)
-    labels[LABEL_SLUG] = slug
+    if number is not None:
+        labels[LABEL_CONTAINER_NUMBER] = str(number)
+    if slug is not None:
+        labels[LABEL_SLUG] = slug
     labels[LABEL_VERSION] = __version__
 
     if config_hash:

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_2' in result.stderr
+        assert 'Removing v2-full_web_run_' 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 '{} | another'.format(another_name) in result.stdout
+        assert '{}   | simple'.format(simple_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(
-            r'logs-restart-composefile_another_1_[a-f0-9]{12} exited with code 1',
-            result.stdout
-        )) == 3
+        assert result.stdout.count(
+            r'logs-restart-composefile_another_1 exited with code 1'
+        ) == 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=valid_numbers[0])
-        service.create_container(number=valid_numbers[1])
+        service.create_container(number=1)
+        service.create_container(number=2)
 
         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/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):
-        self.container_dict['Name'] = "/composetest_web_7"
-        del self.container_dict['Config']['Labels'][LABEL_SLUG]
+    def test_name_without_project_one_off(self):
+        self.container_dict['Name'] = "/composetest_web_092cd63296f"
+        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

@@ -175,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 = {}