Merge pull request docker#716 from bfirsh/pull-latest-tag-by-default

Pull latest tag by default

fig/service.py

@@ -403,6 +403,8 @@ def _get_container_create_options(self, override_options, one_off=False):
             if len(self.client.images(name=self._build_tag_name())) == 0:
                 self.build()
             container_options['image'] = self._build_tag_name()
+        else:
+            container_options['image'] = self._get_image_name(container_options['image'])
 
         # Delete options which are only used when starting
         for key in ['privileged', 'net', 'dns', 'restart', 'cap_add', 'cap_drop', 'env_file']:
@@ -411,6 +413,12 @@ def _get_container_create_options(self, override_options, one_off=False):
 
         return container_options
 
+    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)
 
@@ -457,9 +465,10 @@ def can_be_scaled(self):
 
     def pull(self, insecure_registry=False):
         if 'image' in self.options:
-            log.info('Pulling %s (%s)...' % (self.name, self.options.get('image')))
+            image_name = self._get_image_name(self.options['image'])
+            log.info('Pulling %s (%s)...' % (self.name, image_name))
             self.client.pull(
-                self.options.get('image'),
+                image_name,
                 insecure_registry=insecure_registry
             )
 
@@ -531,6 +540,15 @@ def parse_volume_spec(volume_config):
     return VolumeSpec(external, internal, mode)
 
 
+def parse_repository_tag(s):
+    if ":" not in s:
+        return s, ""
+    repo, tag = s.rsplit(":", 1)
+    if "/" in tag:
+        return s, ""
+    return repo, tag
+
+
 def build_volume_binding(volume_spec):
     internal = {'bind': volume_spec.internal, 'ro': volume_spec.mode == 'ro'}
     external = os.path.expanduser(volume_spec.external)

tests/unit/service_test.py

@@ -17,6 +17,7 @@
     parse_volume_spec,
     build_volume_binding,
     APIError,
+    parse_repository_tag,
 )
 
 
@@ -131,7 +132,7 @@ def test_build_port_bindings_with_nonmatching_internal_ports(self):
     def test_split_domainname_none(self):
         service = Service('foo', hostname='name', client=self.mock_client)
         self.mock_client.containers.return_value = []
-        opts = service._get_container_create_options({})
+        opts = service._get_container_create_options({'image': 'foo'})
         self.assertEqual(opts['hostname'], 'name', 'hostname')
         self.assertFalse('domainname' in opts, 'domainname')
 
@@ -140,7 +141,7 @@ def test_split_domainname_fqdn(self):
                 hostname='name.domain.tld',
                 client=self.mock_client)
         self.mock_client.containers.return_value = []
-        opts = service._get_container_create_options({})
+        opts = service._get_container_create_options({'image': 'foo'})
         self.assertEqual(opts['hostname'], 'name', 'hostname')
         self.assertEqual(opts['domainname'], 'domain.tld', 'domainname')
 
@@ -150,7 +151,7 @@ def test_split_domainname_both(self):
                 domainname='domain.tld',
                 client=self.mock_client)
         self.mock_client.containers.return_value = []
-        opts = service._get_container_create_options({})
+        opts = service._get_container_create_options({'image': 'foo'})
         self.assertEqual(opts['hostname'], 'name', 'hostname')
         self.assertEqual(opts['domainname'], 'domain.tld', 'domainname')
 
@@ -160,7 +161,7 @@ def test_split_domainname_weird(self):
                 domainname='domain.tld',
                 client=self.mock_client)
         self.mock_client.containers.return_value = []
-        opts = service._get_container_create_options({})
+        opts = service._get_container_create_options({'image': 'foo'})
         self.assertEqual(opts['hostname'], 'name.sub', 'hostname')
         self.assertEqual(opts['domainname'], 'domain.tld', 'domainname')
 
@@ -203,6 +204,20 @@ def test_create_container_from_insecure_registry(self, mock_log):
         self.mock_client.pull.assert_called_once_with('someimage:sometag', insecure_registry=True, stream=True)
         mock_log.info.assert_called_once_with('Pulling image someimage:sometag...')
 
+    def test_parse_repository_tag(self):
+        self.assertEqual(parse_repository_tag("root"), ("root", ""))
+        self.assertEqual(parse_repository_tag("root:tag"), ("root", "tag"))
+        self.assertEqual(parse_repository_tag("user/repo"), ("user/repo", ""))
+        self.assertEqual(parse_repository_tag("user/repo:tag"), ("user/repo", "tag"))
+        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):
+        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')
+
 
 class ServiceVolumesTest(unittest.TestCase):