Merge pull request #47367 from terminalmage/issue47325

Start docker containers with image name instead of ID

salt/modules/dockermod.py

@@ -483,20 +483,6 @@ def _clear_context():
             pass
 
 
-def _pull_if_needed(image, client_timeout):
-    '''
-    Pull the desired image if not present, and return the image ID or name
-    '''
-    image_id = resolve_image_id(image)
-    if not image_id:
-        pull(image, client_timeout=client_timeout)
-        # Avoid another inspect and just use the passed image. docker-py
-        # will do the right thing and resolve the tag for us if we pass it
-        # a tagged image.
-        image_id = image
-    return image_id
-
-
 def _get_md5(name, path):
     '''
     Get the MD5 checksum of a file from a container
@@ -3115,8 +3101,8 @@ def create(image,
         # Create a CentOS 7 container that will stay running once started
         salt myminion docker.create centos:7 name=mycent7 interactive=True tty=True command=bash
     '''
-    image_id = image if not kwargs.pop('inspect', True) \
-        else _pull_if_needed(image, client_timeout)
+    if kwargs.pop('inspect', True) and not resolve_image_id(image):
+        pull(image, client_timeout=client_timeout)
 
     kwargs, unused_kwargs = _get_create_kwargs(
         skip_translate=skip_translate,
@@ -3138,7 +3124,7 @@ def create(image,
     )
     time_started = time.time()
     response = _client_wrapper('create_container',
-                               image_id,
+                               image,
                                name=name,
                                **kwargs)
     response['Time_Elapsed'] = time.time() - time_started
@@ -3232,8 +3218,8 @@ def run_container(image,
         # net1 using automatic IP, net2 using static IPv4 address
         salt myminion docker.run_container myuser/myimage command='perl /scripts/sync.py' networks='{"net1": {}, "net2": {"ipv4_address": "192.168.27.12"}}'
     '''
-    image_id = image if not kwargs.pop('inspect', True) \
-        else _pull_if_needed(image, client_timeout)
+    if kwargs.pop('inspect', True) and not resolve_image_id(image):
+        pull(image, client_timeout=client_timeout)
 
     removed_ids = None
     if name is not None:
@@ -3303,7 +3289,7 @@ def run_container(image,
 
     time_started = time.time()
     # Create the container
-    ret = _client_wrapper('create_container', image_id, name=name, **kwargs)
+    ret = _client_wrapper('create_container', image, name=name, **kwargs)
 
     if removed_ids:
         ret['Replaces'] = removed_ids

salt/states/docker_container.py

@@ -1728,7 +1728,7 @@ def running(name,
     # container does not already exist)
     try:
         temp_container = __salt__['docker.create'](
-            image_id,
+            image,
             name=name if not exists else None,
             skip_translate=skip_translate,
             ignore_collisions=ignore_collisions,
@@ -2185,7 +2185,7 @@ def run(name,
     try:
         if 'networks' in kwargs:
             kwargs['networks'] = _parse_networks(kwargs['networks'])
-        image_id = _resolve_image(ret, image, client_timeout)
+        _resolve_image(ret, image, client_timeout)
     except CommandExecutionError as exc:
         ret['result'] = False
         if exc.info is not None:
@@ -2242,7 +2242,7 @@ def run(name,
 
     try:
         ret['changes'] = __salt__['docker.run_container'](
-            image_id,
+            image,
             name=name,
             skip_translate=skip_translate,
             ignore_collisions=ignore_collisions,

tests/integration/states/test_docker_container.py

@@ -16,7 +16,7 @@ from tests.support.unit import skipIf
 from tests.support.case import ModuleCase
 from tests.support.docker import with_network, random_name
 from tests.support.paths import FILES, TMP
-from tests.support.helpers import destructiveTest
+from tests.support.helpers import destructiveTest, with_tempdir
 from tests.support.mixins import SaltReturnAssertsMixin
 
 # Import Salt Libs
@@ -33,24 +33,6 @@ log = logging.getLogger(__name__)
 IPV6_ENABLED = bool(salt.utils.network.ip_addrs6(include_loopback=True))
 
 
-def with_temp_dir(func):
-    '''
-    Generate a temp directory for a test
-    '''
-    @functools.wraps(func)
-    def wrapper(self, *args, **kwargs):
-        tempdir = tempfile.mkdtemp(dir=TMP)
-        try:
-            return func(self, tempdir, *args, **kwargs)
-        finally:
-            try:
-                salt.utils.files.rm_rf(tempdir)
-            except OSError as exc:
-                if exc.errno != errno.ENOENT:
-                    raise
-    return wrapper
-
-
 def container_name(func):
     '''
     Generate a randomized name for a container and clean it up afterward
@@ -126,7 +108,7 @@ class DockerContainerTestCase(ModuleCase, SaltReturnAssertsMixin):
         log.debug('ret = %s', ret)
         return ret
 
-    @with_temp_dir
+    @with_tempdir()
     @container_name
     def test_running_with_no_predefined_volume(self, name, bind_dir_host):
         '''
@@ -572,6 +554,21 @@ class DockerContainerTestCase(ModuleCase, SaltReturnAssertsMixin):
             'Forcibly removed container \'{0}\''.format(name)
         )
 
+    @container_name
+    def test_running_image_name(self, name):
+        '''
+        Ensure that we create the container using the image name instead of ID
+        '''
+        ret = self.run_state(
+            'docker_container.running',
+            name=name,
+            image=self.image,
+            shutdown_timeout=1,
+        )
+        self.assertSaltTrueReturn(ret)
+        ret = self.run_function('docker.inspect_container', [name])
+        self.assertEqual(ret['Config']['Image'], self.image)
+
     @container_name
     def test_env_with_running_container(self, name):
         '''