docker_container, docker_image_facts: allow to use image IDs (ansible…

…#46324)

* Allow to specify images by hash for docker_container and docker_image_facts.

* flake8

* More sanity checks.

* Added changelog.

* Added test.

* Make compatible with Python < 3.4.

* Remove out-commented imports.

changelogs/fragments/docker-image-ids.yaml

@@ -0,0 +1,3 @@
+minor_changes:
+- "docker_container - Allow to use image ID instead of image name."
+- "docker_image_facts - Allow to use image ID instead of image name."

lib/ansible/module_utils/docker_common.py

@@ -18,9 +18,6 @@
 
 import os
 import re
-import json
-import sys
-import copy
 from distutils.version import LooseVersion
 
 from ansible.module_utils.basic import AnsibleModule, env_fallback
@@ -35,22 +32,18 @@
 try:
     from requests.exceptions import SSLError
     from docker import __version__ as docker_version
-    from docker.errors import APIError, TLSParameterError, NotFound
+    from docker.errors import APIError, TLSParameterError
     from docker.tls import TLSConfig
-    from docker.constants import DEFAULT_DOCKER_API_VERSION
     from docker import auth
 
     if LooseVersion(docker_version) >= LooseVersion('3.0.0'):
         HAS_DOCKER_PY_3 = True
         from docker import APIClient as Client
-        from docker.types import Ulimit, LogConfig
     elif LooseVersion(docker_version) >= LooseVersion('2.0.0'):
         HAS_DOCKER_PY_2 = True
         from docker import APIClient as Client
-        from docker.types import Ulimit, LogConfig
     else:
         from docker import Client
-        from docker.utils.types import Ulimit, LogConfig
 
 except ImportError as exc:
     HAS_DOCKER_ERROR = str(exc)
@@ -62,14 +55,14 @@
 # installed, as they utilize the same namespace are are incompatible
 try:
     # docker
-    import docker.models
+    import docker.models  # noqa: F401
     HAS_DOCKER_MODELS = True
 except ImportError:
     HAS_DOCKER_MODELS = False
 
 try:
     # docker-py
-    import docker.ssladapter
+    import docker.ssladapter  # noqa: F401
     HAS_DOCKER_SSLADAPTER = True
 except ImportError:
     HAS_DOCKER_SSLADAPTER = False
@@ -112,14 +105,21 @@
 if not HAS_DOCKER_PY:
     # No docker-py. Create a place holder client to allow
     # instantiation of AnsibleModule and proper error handing
-    class Client(object):
+    class Client(object):  # noqa: F811
         def __init__(self, **kwargs):
             pass
 
-    class APIError(Exception):
+    class APIError(Exception):  # noqa: F811
         pass
 
 
+def is_image_name_id(name):
+    """Checks whether the given image name is in fact an image ID (hash)."""
+    if re.match('^sha256:[0-9a-fA-F]{64}$', name):
+        return True
+    return False
+
+
 def sanitize_result(data):
     """Sanitize data object for return to Ansible.
 
@@ -428,7 +428,7 @@ def get_container(self, name=None):
 
     def find_image(self, name, tag):
         '''
-        Lookup an image and return the inspection results.
+        Lookup an image (by name and tag) and return the inspection results.
         '''
         if not name:
             return None
@@ -457,6 +457,20 @@ def find_image(self, name, tag):
         self.log("Image %s:%s not found." % (name, tag))
         return None
 
+    def find_image_by_id(self, id):
+        '''
+        Lookup an image (by ID) and return the inspection results.
+        '''
+        if not id:
+            return None
+
+        self.log("Find image %s (by ID)" % id)
+        try:
+            inspection = self.inspect_image(id)
+        except Exception as exc:
+            self.fail("Error inspecting image ID %s - %s" % (id, str(exc)))
+        return inspection
+
     def _image_lookup(self, name, tag):
         '''
         Including a tag in the name parameter sent to the docker-py images method does not

lib/ansible/modules/cloud/docker/docker_container.py

@@ -163,7 +163,9 @@
   image:
     description:
       - Repository path and tag used to create the container. If an image is not found or pull is true, the image
-        will be pulled from the registry. If no tag is included, 'latest' will be used.
+        will be pulled from the registry. If no tag is included, C(latest) will be used.
+      - Can also be an image ID. If this is the case, the image is assumed to be available locally.
+        The C(pull) option is ignored for this case.
   init:
     description:
       - Run an init inside the container that forwards signals and reaps processes.
@@ -312,7 +314,10 @@
       - ports
   pull:
     description:
-       - If true, always pull the latest version of an image. Otherwise, will only pull an image when missing.
+       - If true, always pull the latest version of an image. Otherwise, will only pull an image
+         when missing.
+       - I(Note) that images are only pulled when specified by name. If the image is specified
+         as a image ID (hash), it cannot be pulled.
     type: bool
     default: 'no'
   purge_networks:
@@ -693,7 +698,10 @@
 from distutils.version import LooseVersion
 
 from ansible.module_utils.basic import human_to_bytes
-from ansible.module_utils.docker_common import HAS_DOCKER_PY_2, HAS_DOCKER_PY_3, AnsibleDockerClient, DockerBaseClass, sanitize_result
+from ansible.module_utils.docker_common import (
+    HAS_DOCKER_PY_2, HAS_DOCKER_PY_3, AnsibleDockerClient,
+    DockerBaseClass, sanitize_result, is_image_name_id,
+)
 from ansible.module_utils.six import string_types
 
 try:
@@ -979,7 +987,7 @@ def _get_mounts(self):
             for vol in self.volumes:
                 if ':' in vol:
                     if len(vol.split(':')) == 3:
-                        host, container, _ = vol.split(':')
+                        host, container, dummy = vol.split(':')
                         result.append(container)
                         continue
                     if len(vol.split(':')) == 2:
@@ -1988,19 +1996,22 @@ def _get_image(self):
         if not self.parameters.image:
             self.log('No image specified')
             return None
-        repository, tag = utils.parse_repository_tag(self.parameters.image)
-        if not tag:
-            tag = "latest"
-        image = self.client.find_image(repository, tag)
-        if not self.check_mode:
-            if not image or self.parameters.pull:
-                self.log("Pull the image.")
-                image, alreadyToLatest = self.client.pull_image(repository, tag)
-                if alreadyToLatest:
-                    self.results['changed'] = False
-                else:
-                    self.results['changed'] = True
-                    self.results['actions'].append(dict(pulled_image="%s:%s" % (repository, tag)))
+        if is_image_name_id(self.parameters.image):
+            image = self.client.find_image_by_id(self.parameters.image)
+        else:
+            repository, tag = utils.parse_repository_tag(self.parameters.image)
+            if not tag:
+                tag = "latest"
+            image = self.client.find_image(repository, tag)
+            if not self.check_mode:
+                if not image or self.parameters.pull:
+                    self.log("Pull the image.")
+                    image, alreadyToLatest = self.client.pull_image(repository, tag)
+                    if alreadyToLatest:
+                        self.results['changed'] = False
+                    else:
+                        self.results['changed'] = True
+                        self.results['actions'].append(dict(pulled_image="%s:%s" % (repository, tag)))
         self.log("image")
         self.log(image, pretty_print=True)
         return image

lib/ansible/modules/cloud/docker/docker_image.py

@@ -58,6 +58,7 @@
     description:
       - "Image name. Name format will be one of: name, repository/name, registry_server:port/name.
         When pushing or pulling an image the name can optionally include the tag by appending ':tag_name'."
+      - Note that image IDs (hashes) are not supported.
     required: true
   path:
     description:

lib/ansible/modules/cloud/docker/docker_image_facts.py

@@ -26,8 +26,9 @@
 options:
   name:
     description:
-      - An image name or a list of image names. Name format will be name[:tag] or repository/name[:tag], where tag is
-        optional. If a tag is not provided, 'latest' will be used.
+      - An image name or a list of image names. Name format will be C(name[:tag]) or C(repository/name[:tag]),
+        where C(tag) is optional. If a tag is not provided, C(latest) will be used. Instead of image names, also
+        image IDs can be used.
     required: true
 
 extends_documentation_fragment:
@@ -163,7 +164,7 @@
     # missing docker-py handled in ansible.module_utils.docker_common
     pass
 
-from ansible.module_utils.docker_common import AnsibleDockerClient, DockerBaseClass
+from ansible.module_utils.docker_common import AnsibleDockerClient, DockerBaseClass, is_image_name_id
 
 
 class ImageManager(DockerBaseClass):
@@ -199,11 +200,15 @@ def get_facts(self):
             names = [names]
 
         for name in names:
-            repository, tag = utils.parse_repository_tag(name)
-            if not tag:
-                tag = 'latest'
-            self.log('Fetching image %s:%s' % (repository, tag))
-            image = self.client.find_image(name=repository, tag=tag)
+            if is_image_name_id(name):
+                self.log('Fetching image %s (ID)' % (name))
+                image = self.client.find_image_by_id(name)
+            else:
+                repository, tag = utils.parse_repository_tag(name)
+                if not tag:
+                    tag = 'latest'
+                self.log('Fetching image %s:%s' % (repository, tag))
+                image = self.client.find_image(name=repository, tag=tag)
             if image:
                 results.append(image)
         return results

test/integration/targets/docker_container/tasks/tests/image-ids.yml

@@ -0,0 +1,71 @@
+---
+- name: Registering container name
+  set_fact:
+    cname: "{{ cname_prefix ~ '-iid' }}"
+- name: Registering container name
+  set_fact:
+    cnames: "{{ cnames }} + [cname]"
+
+- name: Pull images
+  docker_image:
+    name: "{{ item }}"
+    pull: true
+  loop:
+    - "hello-world:latest"
+    - "alpine:3.8"
+
+- name: Get image ID of hello-world and alpine images
+  docker_image_facts:
+    name:
+    - "hello-world:latest"
+    - "alpine:3.8"
+  register: image_facts
+
+- assert:
+    that:
+      - image_facts.images | length == 2
+
+- name: Print image IDs
+  debug:
+    msg: "hello-world: {{ image_facts.images[0].Id }}; alpine: {{ image_facts.images[1].Id }}"
+
+- name: Create container with hello-world image via ID
+  docker_container:
+    image: "{{ image_facts.images[0].Id }}"
+    name: "{{ cname }}"
+    state: present
+  register: create_1
+
+- name: Create container with hello-world image via ID (idempotent)
+  docker_container:
+    image: "{{ image_facts.images[0].Id }}"
+    name: "{{ cname }}"
+    state: present
+  register: create_2
+
+- name: Create container with alpine image via ID
+  docker_container:
+    image: "{{ image_facts.images[1].Id }}"
+    name: "{{ cname }}"
+    state: present
+  register: create_3
+
+- name: Create container with alpine image via ID (idempotent)
+  docker_container:
+    image: "{{ image_facts.images[1].Id }}"
+    name: "{{ cname }}"
+    state: present
+  register: create_4
+
+- name: Cleanup
+  docker_container:
+    name: "{{ cname }}"
+    state: absent
+    stop_timeout: 1
+
+- assert:
+    that:
+      - create_1 is changed
+      - create_2 is not changed
+      - create_3 is changed
+      - create_4 is not changed

test/sanity/code-smell/no-underscore-variable.py

@@ -33,7 +33,6 @@ def main():
         'lib/ansible/modules/cloud/amazon/route53_zone.py',
         'lib/ansible/modules/cloud/amazon/s3_sync.py',
         'lib/ansible/modules/cloud/azure/azure_rm_loadbalancer.py',
-        'lib/ansible/modules/cloud/docker/docker_container.py',
         'lib/ansible/modules/cloud/docker/docker_service.py',
         'lib/ansible/modules/cloud/google/gce.py',
         'lib/ansible/modules/cloud/google/gce_eip.py',