diff --git a/compose/service.py b/compose/service.py index 5afaa30fa8a..eeda8f32bf3 100644 --- a/compose/service.py +++ b/compose/service.py @@ -193,13 +193,7 @@ def create_container(self, 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 @@ -413,8 +407,6 @@ def _get_container_create_options(self, override_options, one_off=False, interme 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: @@ -463,12 +455,6 @@ def _get_container_host_config(self, override_options, one_off=False, intermedia 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) @@ -515,13 +501,18 @@ def can_be_scaled(self): 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+)$') diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 4abd4a9096e..891caae585e 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -391,6 +391,11 @@ def test_port_with_explicit_interface(self): ], }) + 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) diff --git a/tests/integration/testcases.py b/tests/integration/testcases.py index d5ca1debc8b..715b135c43d 100644 --- a/tests/integration/testcases.py +++ b/tests/integration/testcases.py @@ -22,7 +22,7 @@ def setUp(self): 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"] diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index ec17018ed57..b9f968db10a 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -221,9 +221,22 @@ def test_get_container(self, mock_container_class): 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 @@ def test_create_container_from_insecure_registry( 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 @@ def test_parse_repository_tag(self): 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() - service.create_container() - self.assertEqual(Container.create.call_args[1]['image'], 'someimage:latest') + with self.assertRaises(APIError): + service.create_container() + 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 = []