Merge pull request #43176 from terminalmage/issue42935

docker_image states: Handle Hub images prefixed with "docker.io/"

salt/modules/dockermod.py

@@ -232,6 +232,7 @@ except ImportError:
 # pylint: enable=import-error
 
 HAS_NSENTER = bool(salt.utils.which('nsenter'))
+HUB_PREFIX = 'docker.io/'
 
 # Set up logging
 log = logging.getLogger(__name__)
@@ -1486,6 +1487,43 @@ def list_tags():
     return sorted(ret)
 
 
+def resolve_tag(name, tags=None):
+    '''
+    .. versionadded:: 2017.7.2,Oxygen
+
+    Given an image tag, check the locally-pulled tags (using
+    :py:func:`docker.list_tags <salt.modules.dockermod.list_tags>`) and return
+    the matching tag. This helps disambiguate differences on some platforms
+    where images from the Docker Hub are prefixed with ``docker.io/``. If an
+    image name with no tag is passed, a tag of ``latest`` is assumed.
+
+    If the specified image is not pulled locally, this function will return
+    ``False``.
+
+    tags
+        An optional Python list of tags to check against. If passed, then
+        :py:func:`docker.list_tags <salt.modules.dockermod.list_tags>` will not
+        be run to get a list of tags. This is useful when resolving a number of
+        tags at the same time.
+
+    CLI Examples:
+
+    .. code-block:: bash
+
+        salt myminion docker.resolve_tag busybox
+        salt myminion docker.resolve_tag busybox:latest
+    '''
+    tag_name = ':'.join(salt.utils.docker.get_repo_tag(name))
+    if tags is None:
+        tags = list_tags()
+    if tag_name in tags:
+        return tag_name
+    full_name = HUB_PREFIX + tag_name
+    if not name.startswith(HUB_PREFIX) and full_name in tags:
+        return full_name
+    return False
+
+
 def logs(name):
     '''
     Returns the logs for the container. Equivalent to running the ``docker

salt/states/docker_image.py

@@ -135,13 +135,14 @@ def present(name,
         .. versionadded:: 2016.11.0
 
     sls
-        Allow for building images with ``dockerng.sls_build`` by specify the
-        SLS files to build with. This can be a list or comma-seperated string.
+        Allow for building of image with :py:func:`docker.sls_build
+        <salt.modules.dockermod.sls_build>` by specifying the SLS files with
+        which to build. This can be a list or comma-seperated string.
 
         .. code-block:: yaml
 
             myuser/myimage:mytag:
-              dockerng.image_present:
+              docker_image.present:
                 - sls:
                     - webapp1
                     - webapp2
@@ -151,12 +152,14 @@ def present(name,
         .. versionadded: 2017.7.0
 
     base
-        Base image with which to start ``dockerng.sls_build``
+        Base image with which to start :py:func:`docker.sls_build
+        <salt.modules.dockermod.sls_build>`
 
         .. versionadded: 2017.7.0
 
     saltenv
-        environment from which to pull sls files for ``dockerng.sls_build``.
+        Environment from which to pull SLS files for :py:func:`docker.sls_build
+        <salt.modules.dockermod.sls_build>`
 
         .. versionadded: 2017.7.0
     '''
@@ -169,11 +172,14 @@ def present(name,
         ret['comment'] = 'Only one of \'build\' or \'load\' is permitted.'
         return ret
 
-    # Ensure that we have repo:tag notation
     image = ':'.join(salt.utils.docker.get_repo_tag(name))
-    all_tags = __salt__['docker.list_tags']()
+    resolved_tag = __salt__['docker.resolve_tag'](image)
 
-    if image in all_tags:
+    if resolved_tag is False:
+        # Specified image is not present
+        image_info = None
+    else:
+        # Specified image is present
         if not force:
             ret['result'] = True
             ret['comment'] = 'Image \'{0}\' already present'.format(name)
@@ -185,8 +191,6 @@ def present(name,
                 ret['comment'] = \
                     'Unable to get info for image \'{0}\': {1}'.format(name, exc)
                 return ret
-    else:
-        image_info = None
 
     if build or sls:
         action = 'built'
@@ -197,15 +201,15 @@ def present(name,
 
     if __opts__['test']:
         ret['result'] = None
-        if (image in all_tags and force) or image not in all_tags:
+        if (resolved_tag is not False and force) or resolved_tag is False:
             ret['comment'] = 'Image \'{0}\' will be {1}'.format(name, action)
             return ret
 
     if build:
         try:
             image_update = __salt__['docker.build'](path=build,
-                                                      image=image,
-                                                      dockerfile=dockerfile)
+                                                    image=image,
+                                                    dockerfile=dockerfile)
         except Exception as exc:
             ret['comment'] = (
                 'Encountered error building {0} as {1}: {2}'
@@ -219,10 +223,10 @@ def present(name,
         if isinstance(sls, list):
             sls = ','.join(sls)
         try:
-            image_update = __salt__['dockerng.sls_build'](name=image,
-                                                          base=base,
-                                                          mods=sls,
-                                                          saltenv=saltenv)
+            image_update = __salt__['docker.sls_build'](name=image,
+                                                        base=base,
+                                                        mods=sls,
+                                                        saltenv=saltenv)
         except Exception as exc:
             ret['comment'] = (
                 'Encountered error using sls {0} for building {1}: {2}'
@@ -252,10 +256,8 @@ def present(name,
                 client_timeout=client_timeout
             )
         except Exception as exc:
-            ret['comment'] = (
-                'Encountered error pulling {0}: {1}'
-                .format(image, exc)
-            )
+            ret['comment'] = \
+                'Encountered error pulling {0}: {1}'.format(image, exc)
             return ret
         if (image_info is not None and image_info['Id'][:12] == image_update
                 .get('Layers', {})
@@ -267,7 +269,7 @@ def present(name,
             # Only add to the changes dict if layers were pulled
             ret['changes'] = image_update
 
-    ret['result'] = image in __salt__['docker.list_tags']()
+    ret['result'] = bool(__salt__['docker.resolve_tag'](image))
 
     if not ret['result']:
         # This shouldn't happen, failure to pull should be caught above
@@ -345,23 +347,16 @@ def absent(name=None, images=None, force=False):
         ret['comment'] = 'One of \'name\' and \'images\' must be provided'
         return ret
     elif images is not None:
-        targets = []
-        for target in images:
-            try:
-                targets.append(':'.join(salt.utils.docker.get_repo_tag(target)))
-            except TypeError:
-                # Don't stomp on images with unicode characters in Python 2,
-                # only force image to be a str if it wasn't already (which is
-                # very unlikely).
-                targets.append(':'.join(salt.utils.docker.get_repo_tag(str(target))))
+        targets = images
     elif name:
-        try:
-            targets = [':'.join(salt.utils.docker.get_repo_tag(name))]
-        except TypeError:
-            targets = [':'.join(salt.utils.docker.get_repo_tag(str(name)))]
+        targets = [name]
 
     pre_tags = __salt__['docker.list_tags']()
-    to_delete = [x for x in targets if x in pre_tags]
+    to_delete = []
+    for target in targets:
+        resolved_tag = __salt__['docker.resolve_tag'](target, tags=pre_tags)
+        if resolved_tag is not False:
+            to_delete.append(resolved_tag)
     log.debug('targets = {0}'.format(targets))
     log.debug('to_delete = {0}'.format(to_delete))
 

tests/unit/modules/test_dockermod.py

@@ -679,9 +679,9 @@ class DockerTestCase(TestCase, LoaderModuleMockMixin):
         self.assertEqual({"retcode": 0, "comment": "container cmd"}, ret)
 
     def test_images_with_empty_tags(self):
-        """
+        '''
         docker 1.12 reports also images without tags with `null`.
-        """
+        '''
         client = Mock()
         client.api_version = '1.24'
         client.images = Mock(
@@ -724,3 +724,24 @@ class DockerTestCase(TestCase, LoaderModuleMockMixin):
             with patch.object(docker_mod, 'inspect_image', inspect_image_mock):
                 ret = docker_mod.compare_container('container1', 'container2')
                 self.assertEqual(ret, {})
+
+    def test_resolve_tag(self):
+        '''
+        Test the resolve_tag function
+        '''
+        with_prefix = 'docker.io/foo:latest'
+        no_prefix = 'bar:latest'
+        with patch.object(docker_mod,
+                          'list_tags',
+                          MagicMock(return_value=[with_prefix])):
+            self.assertEqual(docker_mod.resolve_tag('foo'), with_prefix)
+            self.assertEqual(docker_mod.resolve_tag('foo:latest'), with_prefix)
+            self.assertEqual(docker_mod.resolve_tag(with_prefix), with_prefix)
+            self.assertEqual(docker_mod.resolve_tag('foo:bar'), False)
+
+        with patch.object(docker_mod,
+                          'list_tags',
+                          MagicMock(return_value=[no_prefix])):
+            self.assertEqual(docker_mod.resolve_tag('bar'), no_prefix)
+            self.assertEqual(docker_mod.resolve_tag(no_prefix), no_prefix)
+            self.assertEqual(docker_mod.resolve_tag('bar:baz'), False)

tests/unit/states/test_docker_image.py

@@ -10,14 +10,13 @@ from __future__ import absolute_import
 from tests.support.mixins import LoaderModuleMockMixin
 from tests.support.unit import skipIf, TestCase
 from tests.support.mock import (
-    Mock,
+    MagicMock,
     NO_MOCK,
     NO_MOCK_REASON,
     patch
 )
 
 # Import Salt Libs
-from salt.exceptions import CommandExecutionError
 import salt.modules.dockermod as docker_mod
 import salt.states.docker_image as docker_state
 
@@ -50,21 +49,19 @@ class DockerImageTestCase(TestCase, LoaderModuleMockMixin):
         if ``image:latest`` is already downloaded locally the state
         should not report changes.
         '''
-        docker_inspect_image = Mock(
-            return_value={'Id': 'abcdefghijk'})
-        docker_pull = Mock(
+        docker_inspect_image = MagicMock(return_value={'Id': 'abcdefghijkl'})
+        docker_pull = MagicMock(
             return_value={'Layers':
-                          {'Already_Pulled': ['abcdefghijk'],
+                          {'Already_Pulled': ['abcdefghijkl'],
                            'Pulled': []},
                           'Status': 'Image is up to date for image:latest',
                           'Time_Elapsed': 1.1})
-        docker_list_tags = Mock(
-            return_value=['image:latest']
-        )
+        docker_list_tags = MagicMock(return_value=['image:latest'])
+        docker_resolve_tag = MagicMock(return_value='image:latest')
         __salt__ = {'docker.list_tags': docker_list_tags,
                     'docker.pull': docker_pull,
                     'docker.inspect_image': docker_inspect_image,
-                    }
+                    'docker.resolve_tag': docker_resolve_tag}
         with patch.dict(docker_state.__dict__,
                         {'__salt__': __salt__}):
             ret = docker_state.present('image:latest', force=True)
@@ -89,29 +86,24 @@ class DockerImageTestCase(TestCase, LoaderModuleMockMixin):
         if ``image:latest`` is not downloaded and force is true
         should pull a new image successfuly.
         '''
-        docker_inspect_image = Mock(
-            side_effect=CommandExecutionError(
-                'Error 404: No such image/container: image:latest'))
-        docker_pull = Mock(
+        docker_inspect_image = MagicMock(return_value={'Id': '1234567890ab'})
+        docker_pull = MagicMock(
             return_value={'Layers':
-                          {'Already_Pulled': ['abcdefghijk'],
-                           'Pulled': ['abcdefghijk']},
-                          'Status': "Image 'image:latest' was pulled",
-                          'Time_Elapsed': 1.1})
-        docker_list_tags = Mock(
-            side_effect=[[], ['image:latest']]
-        )
+                          {'Pulled': ['abcdefghijkl']},
+                           'Status': "Image 'image:latest' was pulled",
+                           'Time_Elapsed': 1.1})
+        docker_list_tags = MagicMock(side_effect=[[], ['image:latest']])
+        docker_resolve_tag = MagicMock(return_value='image:latest')
         __salt__ = {'docker.list_tags': docker_list_tags,
                     'docker.pull': docker_pull,
                     'docker.inspect_image': docker_inspect_image,
-                    }
+                    'docker.resolve_tag': docker_resolve_tag}
         with patch.dict(docker_state.__dict__,
                         {'__salt__': __salt__}):
             ret = docker_state.present('image:latest', force=True)
         self.assertEqual(ret,
                          {'changes': {
-                             'Layers': {'Already_Pulled': ['abcdefghijk'],
-                                        'Pulled': ['abcdefghijk']},
+                             'Layers': {'Pulled': ['abcdefghijkl']},
                              'Status': "Image 'image:latest' was pulled",
                              'Time_Elapsed': 1.1},
                              'result': True,