diff --git a/changelogs/fragments/87-docker_image-load-image-ids.yml b/changelogs/fragments/87-docker_image-load-image-ids.yml new file mode 100644 index 00000000..098b5d73 --- /dev/null +++ b/changelogs/fragments/87-docker_image-load-image-ids.yml @@ -0,0 +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)." diff --git a/plugins/module_utils/common.py b/plugins/module_utils/common.py index 9373d25e..e2a79918 100644 --- a/plugins/module_utils/common.py +++ b/plugins/module_utils/common.py @@ -534,6 +534,9 @@ class AnsibleDockerClientBase(Client): 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 @@ -541,7 +544,7 @@ class AnsibleDockerClientBase(Client): 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_missing_image=False): ''' Lookup an image (by ID) and return the inspection results. ''' @@ -551,6 +554,11 @@ class AnsibleDockerClientBase(Client): self.log("Find image %s (by ID)" % image_id) try: inspection = self.inspect_image(image_id) + except NotFound as exc: + if not accept_missing_image: + 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 diff --git a/plugins/modules/docker_image.py b/plugins/modules/docker_image.py index b4d1b0fb..af5b704a 100644 --- a/plugins/modules/docker_image.py +++ b/plugins/modules/docker_image.py @@ -155,9 +155,9 @@ options: default: false name: 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. + - "Image name. Name format will be one of: C(name), C(repository/name), C(registry_server:port/name). + When pushing or pulling an image the name can optionally include the tag by appending C(:tag_name)." + - Note that image IDs (hashes) are only supported for I(state=absent), and for I(state=present) with I(source=load). type: str required: yes pull: @@ -344,7 +344,7 @@ if docker_version is not None: 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 @@ -397,6 +397,12 @@ class ImageManager(DockerBaseClass): self.name = repo self.tag = repo_tag + # Sanity check: fail early when we know that something will fail later + if self.repository and is_image_name_id(self.repository): + self.fail("`repository` must not be an image ID; got: %s" % self.repository) + if not self.repository and self.push and is_image_name_id(self.name): + self.fail("Cannot push an image by ID; specify `repository` to tag and push the image with ID %s instead" % self.name) + if self.state == 'present': self.present() elif self.state == 'absent': @@ -412,10 +418,16 @@ class ImageManager(DockerBaseClass): :returns None ''' - image = self.client.find_image(name=self.name, tag=self.tag) + if is_image_name_id(self.name): + image = self.client.find_image_by_id(self.name, accept_missing_image=True) + else: + image = self.client.find_image(name=self.name, tag=self.tag) if not image or self.force_source: if self.source == 'build': + if is_image_name_id(self.name): + self.fail("Image name must not be an image ID for source=build; got: %s" % self.name) + # Build the image if not os.path.isdir(self.build_path): self.fail("Requested build path %s could not be found or you do not have access." % self.build_path) @@ -434,13 +446,16 @@ class ImageManager(DockerBaseClass): self.fail("Error loading image %s. Specified path %s does not exist." % (self.name, self.load_path)) image_name = self.name - if self.tag: + if self.tag and not is_image_name_id(image_name): image_name = "%s:%s" % (self.name, self.tag) self.results['actions'].append("Loaded image %s from %s" % (image_name, self.load_path)) self.results['changed'] = True if not self.check_mode: self.results['image'] = self.load_image() elif self.source == 'pull': + if is_image_name_id(self.name): + self.fail("Image name must not be an image ID for source=pull; got: %s" % self.name) + # pull the image self.results['actions'].append('Pulled image %s:%s' % (self.name, self.tag)) self.results['changed'] = True @@ -449,11 +464,13 @@ class ImageManager(DockerBaseClass): elif self.source == 'local': if image is None: name = self.name - if self.tag: + if self.tag and not is_image_name_id(name): name = "%s:%s" % (self.name, self.tag) self.client.fail('Cannot find the image %s locally.' % name) if not self.check_mode and image and image['Id'] == self.results['image']['Id']: self.results['changed'] = False + else: + self.results['image'] = image if self.archive_path: self.archive_image(self.name, self.tag) @@ -471,7 +488,7 @@ class ImageManager(DockerBaseClass): ''' 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_missing_image=True) else: image = self.client.find_image(name, self.tag) if self.tag: @@ -480,6 +497,9 @@ class ImageManager(DockerBaseClass): if not self.check_mode: try: self.client.remove_image(name, force=self.force_absent) + except NotFound: + # 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))) @@ -498,33 +518,37 @@ class ImageManager(DockerBaseClass): if not tag: tag = "latest" - image = self.client.find_image(name=name, tag=tag) + if is_image_name_id(name): + image = self.client.find_image_by_id(name, accept_missing_image=True) + image_name = name + else: + image = self.client.find_image(name=name, tag=tag) + image_name = "%s:%s" % (name, tag) + if not image: - self.log("archive image: image %s:%s not found" % (name, tag)) + self.log("archive image: image %s not found" % image_name) return - image_name = "%s:%s" % (name, tag) self.results['actions'].append('Archived image %s to %s' % (image_name, self.archive_path)) self.results['changed'] = True if not self.check_mode: self.log("Getting archive of image %s" % image_name) try: - image = self.client.get_image(image_name) + saved_image = self.client.get_image(image_name) except Exception as exc: self.fail("Error getting image %s - %s" % (image_name, str(exc))) try: with open(self.archive_path, 'wb') as fd: if self.client.docker_py_version >= LooseVersion('3.0.0'): - for chunk in image: + for chunk in saved_image: fd.write(chunk) else: - for chunk in image.stream(2048, decode_content=False): + for chunk in saved_image.stream(2048, decode_content=False): fd.write(chunk) except Exception as exc: self.fail("Error writing image archive %s - %s" % (self.archive_path, str(exc))) - image = self.client.find_image(name=name, tag=tag) if image: self.results['image'] = image @@ -537,6 +561,9 @@ class ImageManager(DockerBaseClass): :return: None ''' + if is_image_name_id(name): + self.fail("Cannot push an image ID: %s" % name) + repository = name if not tag: repository, tag = parse_repository_tag(name) @@ -624,7 +651,7 @@ class ImageManager(DockerBaseClass): # 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): ''' @@ -745,27 +772,39 @@ class ImageManager(DockerBaseClass): if has_output: # We can only do this when we actually got some output from Docker daemon loaded_images = set() + loaded_image_ids = set() for line in load_output: if line.startswith('Loaded image:'): loaded_images.add(line[len('Loaded image:'):].strip()) + if line.startswith('Loaded image ID:'): + loaded_image_ids.add(line[len('Loaded image ID:'):].strip().lower()) - if not loaded_images: + if not loaded_images and not loaded_image_ids: 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: + if is_image_name_id(self.name): + expected_image = self.name.lower() + found_image = expected_image not in loaded_image_ids + else: + expected_image = '%s:%s' % (self.name, self.tag) + found_image = expected_image not in loaded_images + if found_image: self.client.fail( "The archive did not contain image '%s'. Instead, found %s." % ( - expected_image, ', '.join(["'%s'" % image for image in sorted(loaded_images)])), + expected_image, + ', '.join(sorted(["'%s'" % image for image in loaded_images] + list(loaded_image_ids)))), 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)]), )) + ', '.join(sorted(["'%s'" % image for image in loaded_images] + list(loaded_image_ids))), )) - return self.client.find_image(self.name, self.tag) + if is_image_name_id(self.name): + return self.client.find_image_by_id(self.name, accept_missing_image=True) + else: + return self.client.find_image(self.name, self.tag) def main(): diff --git a/plugins/modules/docker_image_info.py b/plugins/modules/docker_image_info.py index 5d855fa2..86c7b093 100644 --- a/plugins/modules/docker_image_info.py +++ b/plugins/modules/docker_image_info.py @@ -167,7 +167,7 @@ import traceback 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 @@ -215,7 +215,7 @@ class ImageManager(DockerBaseClass): 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_missing_image=True) else: repository, tag = utils.parse_repository_tag(name) if not tag: @@ -232,6 +232,8 @@ class ImageManager(DockerBaseClass): 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) diff --git a/tests/integration/targets/docker_image/tasks/tests/basic.yml b/tests/integration/targets/docker_image/tasks/tests/basic.yml index 374ea6af..acb45ea0 100644 --- a/tests/integration/targets/docker_image/tasks/tests/basic.yml +++ b/tests/integration/targets/docker_image/tasks/tests/basic.yml @@ -70,13 +70,66 @@ force_tag: yes register: tag_3 +- name: Tag image with ID instead of name + docker_image: + source: local + name: "{{ present_1.image.Id }}" + repository: "{{ docker_test_image_hello_world_base }}:alias" + register: tag_4 + - assert: that: - tag_1 is changed - tag_2 is not changed - tag_3 is not changed + - tag_4 is not changed - name: Cleanup alias tag docker_image: name: "{{ docker_test_image_hello_world_base }}:alias" state: absent + +- name: Tag image with ID instead of name (use ID for repository, must fail) + docker_image: + source: local + name: "{{ docker_test_image_hello_world }}" + repository: "{{ present_1.image.Id }}" + register: fail_1 + ignore_errors: true + +- name: Push image with ID (must fail) + docker_image: + source: local + name: "{{ present_1.image.Id }}" + push: true + register: fail_2 + ignore_errors: true + +- name: Pull image ID (must fail) + docker_image: + source: pull + name: "{{ present_1.image.Id }}" + force_source: true + register: fail_3 + ignore_errors: true + +- name: buildargs + docker_image: + source: build + name: "{{ present_1.image.Id }}" + build: + path: "{{ output_dir }}/files" + force_source: true + register: fail_4 + ignore_errors: yes + +- assert: + that: + - fail_1 is failed + - "'`repository` must not be an image ID' in fail_1.msg" + - fail_2 is failed + - "'Cannot push an image by ID' in fail_2.msg" + - fail_3 is failed + - "'Image name must not be an image ID for source=pull' in fail_3.msg" + - fail_4 is failed + - "'Image name must not be an image ID for source=build' in fail_4.msg" diff --git a/tests/integration/targets/docker_image/tasks/tests/options.yml b/tests/integration/targets/docker_image/tasks/tests/options.yml index a966ea2c..246f95fe 100644 --- a/tests/integration/targets/docker_image/tasks/tests/options.yml +++ b/tests/integration/targets/docker_image/tasks/tests/options.yml @@ -241,6 +241,13 @@ source: pull register: archive_image +- name: Archive image by ID + docker_image: + name: "{{ archive_image.image.Id }}" + archive_path: "{{ output_dir }}/image_id.tar" + source: pull + register: archive_image_id + - name: Create invalid archive copy: dest: "{{ output_dir }}/image-invalid.tar" @@ -290,6 +297,13 @@ api_version: "1.22" register: load_image_4 +- name: load image (ID, idempotency) + docker_image: + name: "{{ archive_image.image.Id }}" + load_path: "{{ output_dir }}/image_id.tar" + source: load + register: load_image_5 + - assert: that: - load_image is changed @@ -302,6 +316,7 @@ - '"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" + - load_image_5 is not changed #################################################################### ## build.path ######################################################