Do not merge - make image archive/save idempotent

.gitignore

@@ -134,3 +134,6 @@ dmypy.json
 
 # Pyre type checker
 .pyre/
+
+# PyCharm
+.idea

plugins/modules/docker_image.py

@@ -335,6 +335,7 @@
 import errno
 import json
 import os
+import tarfile
 import traceback
 
 from ansible.module_utils.common.text.converters import to_native
@@ -549,9 +550,22 @@ def archive_image(self, name, tag):
             self.log("archive image: image %s not found" % image_name)
             return
 
+        # Will have a 'sha256:' prefix
+        new_image_id = image['Id']
+
+        archived_image_error, archived_id = archived_image_id(self.archive_path)
+        changed = (new_image_id != ('sha256:%s' % archived_id))
+
+        if archived_id:
+            self.results['actions'].append('Previous image ID was %s' % archived_id)
+        else:
+            self.results['actions'].append('Previous image ID not available; %s' % archived_image_error)
+
+        self.results['actions'].append('New image ID is %s' % new_image_id)
         self.results['actions'].append('Archived image %s to %s' % (image_name, self.archive_path))
-        self.results['changed'] = True
-        if not self.check_mode:
+        self.results['changed'] = changed
+
+        if (not self.check_mode) and changed:
             self.log("Getting archive of image %s" % image_name)
             try:
                 saved_image = self.client._stream_raw_result(
@@ -569,8 +583,8 @@ def archive_image(self, name, tag):
             except Exception as exc:
                 self.fail("Error writing image archive %s - %s" % (self.archive_path, to_native(exc)))
 
-        if image:
-            self.results['image'] = image
+        self.results['old_image_id'] = archived_id
+        self.results['image'] = image
 
     def push_image(self, name, tag=None):
         '''
@@ -879,6 +893,64 @@ def load_image(self):
             return self.client.find_image(self.name, self.tag)
 
 
+def archived_image_id(archive_path):
+    """
+    Attempts to get Image.Id from meta data stored in the image archive tar file.
+
+    The tar should contain a file "manifest.json" with an array with a single entry,
+    and the entry should have a Config field with the image ID in its file name.
+
+    Returns:
+        Either a failure message or the image ID. Unexpected failures are non-fatal;
+        they simply result in the tar being extracted again.
+
+    :return (str, str)
+    """
+
+    try:
+        tf = tarfile.open(archive_path, 'r')
+        try:
+            try:
+                ef = tf.extractfile('manifest.json')
+
+                if hasattr(ef, '__exit__'):
+                    # Python 3
+                    with ef:
+                        try:
+                            text = ef.read().decode('utf-8')
+                            manifest = json.loads(text)
+                        except Exception as exc:
+                            return "Failed to deserialize manifest: %s" % to_native(exc), None
+                else:
+                    # Python 2.7 or 2.6
+                    manifest = json.load(ef)
+
+                try:
+                    config_file = manifest[0]['Config']
+                except Exception as exc:
+                    return "Failed to get config file name from manifest: %s" % to_native(exc), None
+
+                # Returns hash without 'sha256:' prefix
+                try:
+                    # Strip off .json filename extension, leaving just the hash.
+                    return None, os.path.splitext(config_file)[0]
+                except Exception as exc:
+                    return (
+                        "Failed to extract image id from config file name %s: %s" % (config_file, to_native(exc)),
+                        None
+                    )
+            except Exception as exc:
+                return "Failed to extract manifest from tar file: %s" % to_native(exc), None
+        finally:
+            # In Python 2.6, TarFile does not have exit
+            if hasattr(tf, '__exit__'):
+                with tf:
+                    pass
+
+    except Exception as exc:
+        return "Failed to open tar file: %s" % to_native(exc), None
+
+
 def main():
     argument_spec = dict(
         source=dict(type='str', choices=['build', 'load', 'pull', 'local']),

tests/integration/targets/docker_image/tasks/tests/options.yml

@@ -235,6 +235,21 @@
     source: pull
   register: archive_image
 
+- assert:
+    that:
+      - archive_image is changed
+
+- name: Archive image again (idempotent)
+  docker_image:
+    name: "{{ docker_test_image_hello_world }}"
+    archive_path: "{{ remote_tmp_dir }}/image.tar"
+    source: pull
+  register: archive_image
+
+- assert:
+    that:
+      - archive_image is not changed
+
 - name: Archive image by ID
   docker_image:
     name: "{{ archive_image.image.Id }}"

tests/unit/plugins/modules/test_docker_image.py

@@ -0,0 +1,60 @@
+# Copyright 2022 Red Hat | Ansible
+# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt)
+# SPDX-License-Identifier: GPL-3.0-or-later
+
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import os.path
+import pytest
+import tempfile
+
+from ansible_collections.community.docker.plugins.modules.docker_image import archived_image_id
+
+from .test_resources.docker_image_load.busybox_latest import busybox_latest_tar_bytes
+from .test_resources.docker_image_load.hello_world_latest import hello_world_latest_tar_bytes
+
+
+@pytest.mark.parametrize('fn, expected_id', [
+    (
+        busybox_latest_tar_bytes,
+        '9d5226e6ce3fb6aee2822206a5ef85f38c303d2b37bfc894b419fca2c0501269'
+    ),
+    (
+        hello_world_latest_tar_bytes,
+        'feb5d9fea6a5e9606aa995e879d862b825965ba48de054caab5ef356dc6b3412'
+    ),
+])
+def test_archived_image_id_extracts(fn, expected_id):
+    tar_bytes = fn()
+
+    file_name = tempfile.mkstemp(suffix='.tar')[1]
+
+    with open(file_name, 'wb') as f:
+        f.write(tar_bytes)
+
+    error, image_id = archived_image_id(file_name)
+
+    if error is not None:
+        raise Exception('boom: %r' % error)
+
+    assert error is None
+    assert image_id == expected_id
+
+    os.remove(file_name)
+
+
+def test_archived_image_id_extracts_nothing_when_file_not_present():
+    full_name = os.path.join(os.path.split(__file__)[0], 'does-not-exist.tar')
+
+    error, image_id = archived_image_id(full_name)
+
+    assert error is not None
+    assert image_id is None
+
+
+def test_archived_image_id_extracts_nothing_when_file_not_a_tar():
+    error, image_id = archived_image_id(__file__)
+
+    assert error is not None
+    assert image_id is None