Merge pull request #1835 from aanand/fix-crash-when-container-has-no-name

Ignore containers that don't have a name

compose/container.py

@@ -22,10 +22,14 @@ class Container(object):
         """
         Construct a container object from the output of GET /containers/json.
         """
+        name = get_container_name(dictionary)
+        if name is None:
+            return None
+
         new_dictionary = {
             'Id': dictionary['Id'],
             'Image': dictionary['Image'],
-            'Name': '/' + get_container_name(dictionary),
+            'Name': '/' + name,
         }
         return cls(client, new_dictionary, **kwargs)
 

compose/project.py

@@ -310,11 +310,11 @@ class Project(object):
         else:
             service_names = self.service_names
 
-        containers = [
+        containers = filter(None, [
             Container.from_ps(self.client, container)
             for container in self.client.containers(
                 all=stopped,
-                filters={'label': self.labels(one_off=one_off)})]
+                filters={'label': self.labels(one_off=one_off)})])
 
         def matches_service_names(container):
             return container.labels.get(LABEL_SERVICE) in service_names

compose/service.py

@@ -101,11 +101,11 @@ class Service(object):
         self.options = options
 
     def containers(self, stopped=False, one_off=False):
-        containers = [
+        containers = filter(None, [
             Container.from_ps(self.client, container)
             for container in self.client.containers(
                 all=stopped,
-                filters={'label': self.labels(one_off=one_off)})]
+                filters={'label': self.labels(one_off=one_off)})])
 
         if not containers:
             check_for_legacy_containers(
@@ -494,12 +494,13 @@ class Service(object):
     # TODO: this would benefit from github.com/docker/docker/pull/11943
     # to remove the need to inspect every container
     def _next_container_number(self, one_off=False):
-        numbers = [
-            Container.from_ps(self.client, container).number
+        containers = filter(None, [
+            Container.from_ps(self.client, container)
             for container in self.client.containers(
                 all=True,
                 filters={'label': self.labels(one_off=one_off)})
-        ]
+        ])
+        numbers = [c.number for c in containers]
         return 1 if not numbers else max(numbers) + 1
 
     def _get_links(self, link_to_self):

tests/integration/legacy_test.py

@@ -65,7 +65,7 @@ class UtilitiesTestCase(unittest.TestCase):
             legacy.is_valid_name("composetest_web_lol_1", one_off=True),
         )
 
-    def test_get_legacy_containers_no_labels(self):
+    def test_get_legacy_containers(self):
         client = Mock()
         client.containers.return_value = [
             {
@@ -74,12 +74,23 @@ class UtilitiesTestCase(unittest.TestCase):
                 "Name": "composetest_web_1",
                 "Labels": None,
             },
+            {
+                "Id": "ghi789",
+                "Image": "def456",
+                "Name": None,
+                "Labels": None,
+            },
+            {
+                "Id": "jkl012",
+                "Image": "def456",
+                "Labels": None,
+            },
         ]
 
-        containers = list(legacy.get_legacy_containers(
-            client, "composetest", ["web"]))
+        containers = legacy.get_legacy_containers(client, "composetest", ["web"])
 
         self.assertEqual(len(containers), 1)
+        self.assertEqual(containers[0].id, 'abc123')
 
 
 class LegacyTestCase(DockerClientTestCase):

tests/unit/project_test.py

@@ -3,6 +3,7 @@ from .. import unittest
 from compose.service import Service
 from compose.project import Project
 from compose.container import Container
+from compose.const import LABEL_SERVICE
 
 import mock
 import docker
@@ -260,3 +261,27 @@ class ProjectTest(unittest.TestCase):
 
         service = project.get_service('test')
         self.assertEqual(service._get_net(), 'container:' + container_name)
+
+    def test_container_without_name(self):
+        self.mock_client.containers.return_value = [
+            {'Image': 'busybox:latest', 'Id': '1', 'Name': '1'},
+            {'Image': 'busybox:latest', 'Id': '2', 'Name': None},
+            {'Image': 'busybox:latest', 'Id': '3'},
+        ]
+        self.mock_client.inspect_container.return_value = {
+            'Id': '1',
+            'Config': {
+                'Labels': {
+                    LABEL_SERVICE: 'web',
+                },
+            },
+        }
+        project = Project.from_dicts(
+            'test',
+            [{
+                'name': 'web',
+                'image': 'busybox:latest',
+            }],
+            self.mock_client,
+        )
+        self.assertEqual([c.id for c in project.containers()], ['1'])

tests/unit/service_test.py

@@ -76,6 +76,18 @@ class ServiceTest(unittest.TestCase):
             all=False,
             filters={'label': expected_labels})
 
+    def test_container_without_name(self):
+        self.mock_client.containers.return_value = [
+            {'Image': 'foo', 'Id': '1', 'Name': '1'},
+            {'Image': 'foo', 'Id': '2', 'Name': None},
+            {'Image': 'foo', 'Id': '3'},
+        ]
+        service = Service('db', self.mock_client, 'myproject', image='foo')
+
+        self.assertEqual([c.id for c in service.containers()], ['1'])
+        self.assertEqual(service._next_container_number(), 2)
+        self.assertEqual(service.get_container(1).id, '1')
+
     def test_get_volumes_from_container(self):
         container_id = 'aabbccddee'
         service = Service(