Merge pull request #6377 from docker/6316-noslug

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

compose/container.py

@@ -7,6 +7,7 @@
 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 @@ def service(self):
     @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 @@ def slug(self):
     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

@@ -738,16 +738,18 @@ def get_volumes_from_names(self):
         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):
@@ -825,7 +827,7 @@ def _get_container_create_options(
             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])
@@ -834,7 +836,7 @@ def _get_container_create_options(
         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)
 
@@ -1122,12 +1124,12 @@ def labels(self, one_off=False, legacy=False):
     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:
@@ -1384,13 +1386,13 @@ def mode(self):
 # 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
@@ -1579,8 +1581,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

@@ -602,10 +602,10 @@ def test_ps_services_filter_status(self):
     def test_ps_all(self):
         self.project.get_service('simple').create_container(one_off='blahblah')
         result = self.dispatch(['ps'])
-        assert 'simple-composefile_simple_run_1' not in result.stdout
+        assert 'simple-composefile_simple_run_' not in result.stdout
 
         result2 = self.dispatch(['ps', '--all'])
-        assert 'simple-composefile_simple_run_1' in result2.stdout
+        assert 'simple-composefile_simple_run_' in result2.stdout
 
     def test_pull(self):
         result = self.dispatch(['pull'])
@@ -973,11 +973,11 @@ def test_down(self):
         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
@@ -1039,8 +1039,8 @@ def test_up_attached(self):
             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
 
@@ -2340,10 +2340,9 @@ def test_logs_follow_logs_from_restarted_containers(self):
 
         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):
@@ -2714,7 +2713,7 @@ def test_forward_exitval(self):
         )
 
         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_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 @@ def test_scale_with_stopped_containers(self):
         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 @@ def test_labels(self):
         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 @@ def test_service_recreated_when_dependency_created(self):
         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 @@
 
 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 @@ def setUp(self):
                     "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 @@ def test_name(self):
         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 @@ def test_memory_swap_limit(self):
     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 = {}