Skip to content

Commit

Permalink
Improve image finding by ID, and fix various related bugs.
Browse files Browse the repository at this point in the history
  • Loading branch information
felixfontein committed Feb 18, 2021
1 parent 4537f8c commit 5248b41
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 9 deletions.
5 changes: 5 additions & 0 deletions changelogs/fragments/87-docker_image-load-image-ids.yml
Original file line number Diff line number Diff line change
@@ -1,2 +1,7 @@
minor_changes:
- "docker_image - properly support image IDs (hashes) for loading and tagging images (https://github.com/ansible-collections/community.docker/issues/86, https://github.com/ansible-collections/community.docker/pull/87)."
bugfixes:
- "docker_image - prevent module failure when removing image that is removed between inspection and removal (https://github.com/ansible-collections/community.docker/pull/87)."
- "docker_image - prevent module failure when removing non-existant image by ID (https://github.com/ansible-collections/community.docker/pull/87)."
- "docker_image_info - prevent module failure when image vanishes between listing and inspection (https://github.com/ansible-collections/community.docker/pull/87)."
- "docker_image_info - prevent module failure when querying non-existant image by ID (https://github.com/ansible-collections/community.docker/pull/87)."
10 changes: 9 additions & 1 deletion plugins/module_utils/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -534,14 +534,17 @@ def find_image(self, name, tag):
if len(images) == 1:
try:
inspection = self.inspect_image(images[0]['Id'])
except NotFound:
self.log("Image %s:%s not found." % (name, tag))
return None
except Exception as exc:
self.fail("Error inspecting image %s:%s - %s" % (name, tag, str(exc)))
return inspection

self.log("Image %s:%s not found." % (name, tag))
return None

def find_image_by_id(self, image_id):
def find_image_by_id(self, image_id, accept_not_there=False):
'''
Lookup an image (by ID) and return the inspection results.
'''
Expand All @@ -551,6 +554,11 @@ def find_image_by_id(self, image_id):
self.log("Find image %s (by ID)" % image_id)
try:
inspection = self.inspect_image(image_id)
except NotFound as exc:
if not accept_not_there:
self.fail("Error inspecting image ID %s - %s" % (image_id, str(exc)))
self.log("Image %s not found." % image_id)
return None
except Exception as exc:
self.fail("Error inspecting image ID %s - %s" % (image_id, str(exc)))
return inspection
Expand Down
15 changes: 9 additions & 6 deletions plugins/modules/docker_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@
else:
from docker.auth.auth import resolve_repository_name
from docker.utils.utils import parse_repository_tag
from docker.errors import DockerException
from docker.errors import DockerException, NotFound
except ImportError:
# missing Docker SDK for Python handled in module_utils.docker.common
pass
Expand Down Expand Up @@ -402,7 +402,7 @@ def present(self):
:returns None
'''
if is_image_name_id(self.name):
image = self.client.find_image_by_id(self.name)
image = self.client.find_image_by_id(self.name, accept_not_there=True)
else:
image = self.client.find_image(name=self.name, tag=self.tag)

Expand Down Expand Up @@ -471,7 +471,7 @@ def absent(self):
'''
name = self.name
if is_image_name_id(name):
image = self.client.find_image_by_id(name)
image = self.client.find_image_by_id(name, accept_not_there=True)
else:
image = self.client.find_image(name, self.tag)
if self.tag:
Expand All @@ -480,6 +480,9 @@ def absent(self):
if not self.check_mode:
try:
self.client.remove_image(name, force=self.force_absent)
except NotFound as dummy:
# If the image vanished while we were trying to remove it, don't fail
pass
except Exception as exc:
self.fail("Error removing image %s - %s" % (name, str(exc)))

Expand All @@ -499,7 +502,7 @@ def archive_image(self, name, tag):
tag = "latest"

if is_image_name_id(name):
image = self.client.find_image_by_id(name)
image = self.client.find_image_by_id(name, accept_not_there=True)
image_name = name
else:
image = self.client.find_image(name=name, tag=tag)
Expand Down Expand Up @@ -631,7 +634,7 @@ def _extract_output_line(line, output):
# Make sure we have a string (assuming that line['stream'] and
# line['status'] are either not defined, falsish, or a string)
text_line = line.get('stream') or line.get('status') or ''
output.append(text_line)
output.extend(text_line.splitlines())

def build_image(self):
'''
Expand Down Expand Up @@ -782,7 +785,7 @@ def load_image(self):
', '.join(sorted(["'%s'" % image for image in loaded_images] + list(loaded_image_ids))), ))

if is_image_name_id(self.name):
return self.client.find_image_by_id(self.name)
return self.client.find_image_by_id(self.name, accept_not_there=True)
else:
return self.client.find_image(self.name, self.tag)

Expand Down
6 changes: 4 additions & 2 deletions plugins/modules/docker_image_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@

try:
from docker import utils
from docker.errors import DockerException
from docker.errors import DockerException, NotFound
except ImportError:
# missing Docker SDK for Python handled in ansible.module_utils.docker.common
pass
Expand Down Expand Up @@ -215,7 +215,7 @@ def get_facts(self):
for name in names:
if is_image_name_id(name):
self.log('Fetching image %s (ID)' % (name))
image = self.client.find_image_by_id(name)
image = self.client.find_image_by_id(name, accept_not_there=True)
else:
repository, tag = utils.parse_repository_tag(name)
if not tag:
Expand All @@ -232,6 +232,8 @@ def get_all_images(self):
for image in images:
try:
inspection = self.client.inspect_image(image['Id'])
except NotFound:
pass
except Exception as exc:
self.fail("Error inspecting image %s - %s" % (image['Id'], str(exc)))
results.append(inspection)
Expand Down

0 comments on commit 5248b41

Please sign in to comment.