Merge pull request #1344 from dnephin/fix_pull_with_sha

Support image with ids instead of names

compose/service.py

@@ -194,13 +194,7 @@ class Service(object):
             return Container.create(self.client, **container_options)
         except APIError as e:
             if e.response.status_code == 404 and e.explanation and 'No such image' in str(e.explanation):
-                log.info('Pulling image %s...' % container_options['image'])
-                output = self.client.pull(
-                    container_options['image'],
-                    stream=True,
-                    insecure_registry=insecure_registry
-                )
-                stream_output(output, sys.stdout)
+                self.pull(insecure_registry=insecure_registry)
                 return Container.create(self.client, **container_options)
             raise
 
@@ -414,8 +408,6 @@ class Service(object):
 
         if self.can_be_built():
             container_options['image'] = self.full_name
-        else:
-            container_options['image'] = self._get_image_name(container_options['image'])
 
         # Delete options which are only used when starting
         for key in DOCKER_START_KEYS:
@@ -467,12 +459,6 @@ class Service(object):
             pid_mode=pid
         )
 
-    def _get_image_name(self, image):
-        repo, tag = parse_repository_tag(image)
-        if tag == "":
-            tag = "latest"
-        return '%s:%s' % (repo, tag)
-
     def build(self, no_cache=False):
         log.info('Building %s...' % self.name)
 
@@ -520,13 +506,18 @@ class Service(object):
         return True
 
     def pull(self, insecure_registry=False):
-        if 'image' in self.options:
-            image_name = self._get_image_name(self.options['image'])
-            log.info('Pulling %s (%s)...' % (self.name, image_name))
-            self.client.pull(
-                image_name,
-                insecure_registry=insecure_registry
-            )
+        if 'image' not in self.options:
+            return
+
+        repo, tag = parse_repository_tag(self.options['image'])
+        tag = tag or 'latest'
+        log.info('Pulling %s (%s:%s)...' % (self.name, repo, tag))
+        output = self.client.pull(
+            repo,
+            tag=tag,
+            stream=True,
+            insecure_registry=insecure_registry)
+        stream_output(output, sys.stdout)
 
 
 NAME_RE = re.compile(r'^([^_]+)_([^_]+)_(run_)?(\d+)$')

tests/integration/service_test.py

@@ -457,6 +457,11 @@ class ServiceTest(DockerClientTestCase):
             ],
         })
 
+    def test_start_with_image_id(self):
+        # Image id for the current busybox:latest
+        service = self.create_service('foo', image='8c2e06607696')
+        self.assertTrue(service.start_or_create_containers())
+
     def test_scale(self):
         service = self.create_service('web')
         service.scale(1)

tests/integration/testcases.py

@@ -22,7 +22,7 @@ class DockerClientTestCase(unittest.TestCase):
                 self.client.remove_image(i)
 
     def create_service(self, name, **kwargs):
-        kwargs['image'] = "busybox:latest"
+        kwargs['image'] = kwargs.pop('image', 'busybox:latest')
 
         if 'command' not in kwargs:
             kwargs['command'] = ["/bin/sleep", "300"]

tests/unit/service_test.py

@@ -221,9 +221,22 @@ class ServiceTest(unittest.TestCase):
     def test_pull_image(self, mock_log):
         service = Service('foo', client=self.mock_client, image='someimage:sometag')
         service.pull(insecure_registry=True)
-        self.mock_client.pull.assert_called_once_with('someimage:sometag', insecure_registry=True)
+        self.mock_client.pull.assert_called_once_with(
+            'someimage',
+            tag='sometag',
+            insecure_registry=True,
+            stream=True)
         mock_log.info.assert_called_once_with('Pulling foo (someimage:sometag)...')
 
+    def test_pull_image_no_tag(self):
+        service = Service('foo', client=self.mock_client, image='ababab')
+        service.pull()
+        self.mock_client.pull.assert_called_once_with(
+            'ababab',
+            tag='latest',
+            insecure_registry=False,
+            stream=True)
+
     @mock.patch('compose.service.Container', autospec=True)
     @mock.patch('compose.service.log', autospec=True)
     def test_create_container_from_insecure_registry(
@@ -243,11 +256,12 @@ class ServiceTest(unittest.TestCase):
             service.create_container(insecure_registry=True)
 
         self.mock_client.pull.assert_called_once_with(
-            'someimage:sometag',
+            'someimage',
+            tag='sometag',
             insecure_registry=True,
             stream=True)
         mock_log.info.assert_called_once_with(
-            'Pulling image someimage:sometag...')
+            'Pulling foo (someimage:sometag)...')
 
     def test_parse_repository_tag(self):
         self.assertEqual(parse_repository_tag("root"), ("root", ""))
@@ -257,11 +271,20 @@ class ServiceTest(unittest.TestCase):
         self.assertEqual(parse_repository_tag("url:5000/repo"), ("url:5000/repo", ""))
         self.assertEqual(parse_repository_tag("url:5000/repo:tag"), ("url:5000/repo", "tag"))
 
-    def test_latest_is_used_when_tag_is_not_specified(self):
+    @mock.patch('compose.service.Container', autospec=True)
+    def test_create_container_latest_is_used_when_no_tag_specified(self, mock_container):
+        mock_container.create.side_effect = APIError(
+            "oops",
+            mock.Mock(status_code=404),
+            "No such image")
         service = Service('foo', client=self.mock_client, image='someimage')
-        Container.create = mock.Mock()
+        with self.assertRaises(APIError):
             service.create_container()
-        self.assertEqual(Container.create.call_args[1]['image'], 'someimage:latest')
+        self.mock_client.pull.assert_called_once_with(
+            'someimage',
+            tag='latest',
+            insecure_registry=False,
+            stream=True)
 
     def test_create_container_with_build(self):
         self.mock_client.images.return_value = []