Backport of https://github.com/ansible-collections/community.docker/commit/8702713ac3cca15f8b92804e0a40f7a1ae833b7b (#73638)

3 files changed, 65 insertions, 25 deletions

diff --git a/changelogs/fragments/community.docker-73-docker_image-fix-old-docker-py-version.yml b/changelogs/fragments/community.docker-73-docker_image-fix-old-docker-py-version.yml
new file mode 100644
index 0000000000..a7b8799343
--- /dev/null
+++ b/changelogs/fragments/community.docker-73-docker_image-fix-old-docker-py-version.yml
@@ -0,0 +1,2 @@
+bugfixes:
+- "docker_image - fix crash on loading images with versions of Docker SDK for Python before 2.5.0 (https://github.com/ansible-collections/community.docker/issues/72, https://github.com/ansible-collections/community.docker/pull/73)."
diff --git a/lib/ansible/modules/cloud/docker/docker_image.py b/lib/ansible/modules/cloud/docker/docker_image.py
index 52992007b0..b5beec19c9 100644
--- a/lib/ansible/modules/cloud/docker/docker_image.py
+++ b/lib/ansible/modules/cloud/docker/docker_image.py
@@ -788,15 +788,41 @@ class ImageManager(DockerBaseClass):
         '''
         # Load image(s) from file
         load_output = []
+        has_output = False
         try:
             self.log("Opening image %s" % self.load_path)
             with open(self.load_path, 'rb') as image_tar:
                 self.log("Loading image from %s" % self.load_path)
-                for line in self.client.load_image(image_tar):
-                    self.log(line, pretty_print=True)
-                    if "stream" in line or "status" in line:
-                        load_line = line.get("stream") or line.get("status") or ''
-                        load_output.append(load_line)
+                output = self.client.load_image(image_tar)
+                if output is not None:
+                    # Old versions of Docker SDK of Python (before version 2.5.0) do not return anything.
+                    # (See https://github.com/docker/docker-py/commit/7139e2d8f1ea82340417add02090bfaf7794f159)
+                    # Note that before that commit, something else than None was returned, but that was also
+                    # only introduced in a commit that first appeared in 2.5.0 (see
+                    # https://github.com/docker/docker-py/commit/9e793806ff79559c3bc591d8c52a3bbe3cdb7350).
+                    # So the above check works for every released version of Docker SDK for Python.
+                    has_output = True
+                    for line in output:
+                        self.log(line, pretty_print=True)
+                        if "stream" in line or "status" in line:
+                            load_line = line.get("stream") or line.get("status") or ''
+                            load_output.append(load_line)
+                else:
+                    if LooseVersion(docker_version) < LooseVersion('2.5.0'):
+                        self.client.module.warn(
+                            'The installed version of the Docker SDK for Python does not return the loading results'
+                            ' from the Docker daemon. Therefore, we cannot verify whether the expected image was'
+                            ' loaded, whether multiple images where loaded, or whether the load actually succeeded.'
+                            ' If you are not stuck with Python 2.6, *please* upgrade to a version newer than 2.5.0'
+                            ' (2.5.0 was released in August 2017).'
+                        )
+                    else:
+                        self.client.module.warn(
+                            'The API version of your Docker daemon is < 1.23, which does not return the image'
+                            ' loading result from the Docker daemon. Therefore, we cannot verify whether the'
+                            ' expected image was loaded, whether multiple images where loaded, or whether the load'
+                            ' actually succeeded. You should consider upgrading your Docker daemon.'
+                        )
         except EnvironmentError as exc:
             if exc.errno == errno.ENOENT:
                 self.client.fail("Error opening image %s - %s" % (self.load_path, str(exc)))
@@ -805,26 +831,28 @@ class ImageManager(DockerBaseClass):
             self.client.fail("Error loading image %s - %s" % (self.name, str(exc)), stdout='\n'.join(load_output))
 
         # Collect loaded images
-        loaded_images = set()
-        for line in load_output:
-            if line.startswith('Loaded image:'):
-                loaded_images.add(line[len('Loaded image:'):].strip())
-
-        if not loaded_images:
-            self.client.fail("Detected no loaded images. Archive potentially corrupt?", stdout='\n'.join(load_output))
-
-        expected_image = '%s:%s' % (self.name, self.tag)
-        if expected_image not in loaded_images:
-            self.client.fail(
-                "The archive did not contain image '%s'. Instead, found %s." % (
-                    expected_image, ', '.join(["'%s'" % image for image in sorted(loaded_images)])),
-                stdout='\n'.join(load_output))
-        loaded_images.remove(expected_image)
-
-        if loaded_images:
-            self.client.module.warn(
-                "The archive contained more images than specified: %s" % (
-                    ', '.join(["'%s'" % image for image in sorted(loaded_images)]), ))
+        if has_output:
+            # We can only do this when we actually got some output from Docker daemon
+            loaded_images = set()
+            for line in load_output:
+                if line.startswith('Loaded image:'):
+                    loaded_images.add(line[len('Loaded image:'):].strip())
+
+            if not loaded_images:
+                self.client.fail("Detected no loaded images. Archive potentially corrupt?", stdout='\n'.join(load_output))
+
+            expected_image = '%s:%s' % (self.name, self.tag)
+            if expected_image not in loaded_images:
+                self.client.fail(
+                    "The archive did not contain image '%s'. Instead, found %s." % (
+                        expected_image, ', '.join(["'%s'" % image for image in sorted(loaded_images)])),
+                    stdout='\n'.join(load_output))
+            loaded_images.remove(expected_image)
+
+            if loaded_images:
+                self.client.module.warn(
+                    "The archive contained more images than specified: %s" % (
+                        ', '.join(["'%s'" % image for image in sorted(loaded_images)]), ))
 
         return self.client.find_image(self.name, self.tag)
 
diff --git a/test/integration/targets/docker_image/tasks/tests/options.yml b/test/integration/targets/docker_image/tasks/tests/options.yml
index 71cb9c94a2..bac4a721da 100644
--- a/test/integration/targets/docker_image/tasks/tests/options.yml
+++ b/test/integration/targets/docker_image/tasks/tests/options.yml
@@ -223,6 +223,14 @@
   register: load_image_3
   ignore_errors: true
 
+- name: load image (invalid image, old API version)
+  docker_image:
+    name: foo:bar
+    load_path: "{{ output_dir }}/image-invalid.tar"
+    source: load
+    api_version: "1.22"
+  register: load_image_4
+
 - assert:
     that:
     - load_image is changed
@@ -233,6 +241,8 @@
       "The archive did not contain image 'foo:bar'. Instead, found 'quay.io/ansible/docker-test-containers:hello-world'." == load_image_2.msg
     - load_image_3 is failed
     - '"Detected no loaded images. Archive potentially corrupt?" == load_image_3.msg'
+    - load_image_4 is changed
+    - "'The API version of your Docker daemon is < 1.23, which does not return the image loading result from the Docker daemon. Therefore, we cannot verify whether the expected image was loaded, whether multiple images where loaded, or whether the load actually succeeded. You should consider upgrading your Docker daemon.' in load_image_4.warnings"
 
 ####################################################################
 ## path ############################################################