From d207643e0c0f869fe6e7f49c27747938c32a1b34 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Sun, 16 Nov 2025 10:09:23 +0100 Subject: [PATCH] docker_image(_push): fix push detection (#1199) * Fix IP address retrieval for registry setup. * Adjust push detection to Docker 29. * Idempotency for export no longer works. * Disable pull idempotency checks that play with architecture. * Add more known image IDs. * Adjust load tests. * Adjust error message check. * Allow for more digests. * Make sure a new enough cryptography version is installed. --- .../fragments/1199-docker_image-push.yml | 5 ++ plugins/modules/docker_image.py | 4 +- plugins/modules/docker_image_export.py | 8 ++- plugins/modules/docker_image_push.py | 2 +- plugins/modules/docker_image_remove.py | 1 + .../docker_image/tasks/tests/options.yml | 4 ++ .../docker_image_export/tasks/tests/basic.yml | 4 ++ .../docker_image_load/tasks/tests/basic.yml | 49 ++++++++++++++++--- .../docker_image_pull/tasks/tests/basic.yml | 7 +++ .../tasks/tests/image-ids.yml | 12 ++--- .../docker_image_push/tasks/tests/basic.yml | 2 +- .../tasks/tests/registry.yml | 4 +- .../docker_image_remove/tasks/main.yml | 34 ++++++++----- .../targets/setup_docker/vars/main.yml | 12 +++-- .../tasks/setup-frontend.yml | 12 ++++- .../targets/setup_openssl/tasks/main.yml | 2 +- 16 files changed, 125 insertions(+), 37 deletions(-) create mode 100644 changelogs/fragments/1199-docker_image-push.yml diff --git a/changelogs/fragments/1199-docker_image-push.yml b/changelogs/fragments/1199-docker_image-push.yml new file mode 100644 index 00000000..f861ea7e --- /dev/null +++ b/changelogs/fragments/1199-docker_image-push.yml @@ -0,0 +1,5 @@ +bugfixes: + - "docker_image, docker_image_push - adjust image push detection to Docker 29 (https://github.com/ansible-collections/community.docker/pull/1199)." +known_issues: + - "docker_image, docker_image_export - idempotency for archiving images depends on whether the image IDs used by the image storage backend correspond to the IDs used in the tarball's ``manifest.json`` files. + The new default backend in Docker 29 apparently uses image IDs that no longer correspond, whence idempotency no longer works (https://github.com/ansible-collections/community.docker/pull/1199)." diff --git a/plugins/modules/docker_image.py b/plugins/modules/docker_image.py index f0544cf5..2fc62b79 100644 --- a/plugins/modules/docker_image.py +++ b/plugins/modules/docker_image.py @@ -21,6 +21,8 @@ description: notes: - Building images is done using Docker daemon's API. It is not possible to use BuildKit / buildx this way. Use M(community.docker.docker_image_build) to build images with BuildKit. + - Exporting images is generally not idempotent. It depends on whether the image ID equals the IDs found in the generated tarball's C(manifest.json). + This was the case with the default storage backend up to Docker 28, but seems to have changed in Docker 29. extends_documentation_fragment: - community.docker._docker.api_documentation - community.docker._attributes @@ -803,7 +805,7 @@ class ImageManager(DockerBaseClass): if line.get("errorDetail"): raise RuntimeError(line["errorDetail"]["message"]) status = line.get("status") - if status == "Pushing": + if status in ("Pushing", "Pushed"): changed = True self.results["changed"] = changed except Exception as exc: # pylint: disable=broad-exception-caught diff --git a/plugins/modules/docker_image_export.py b/plugins/modules/docker_image_export.py index 51c1af97..4f83527f 100644 --- a/plugins/modules/docker_image_export.py +++ b/plugins/modules/docker_image_export.py @@ -28,7 +28,13 @@ attributes: diff_mode: support: none idempotent: - support: full + support: partial + details: + - Whether the module is idempotent depends on the storage API used for images, + which determines how the image ID is computed. The idempotency check needs + that the image ID equals the ID stored in archive's C(manifest.json). + This seemed to have worked fine with the default storage backend up to Docker 28, + but seems to have changed in Docker 29. options: names: diff --git a/plugins/modules/docker_image_push.py b/plugins/modules/docker_image_push.py index 8d8100dd..4ad585b6 100644 --- a/plugins/modules/docker_image_push.py +++ b/plugins/modules/docker_image_push.py @@ -159,7 +159,7 @@ class ImagePusher(DockerBaseClass): if line.get("errorDetail"): raise RuntimeError(line["errorDetail"]["message"]) status = line.get("status") - if status == "Pushing": + if status in ("Pushing", "Pushed"): results["changed"] = True except Exception as exc: # pylint: disable=broad-exception-caught if "unauthorized" in str(exc): diff --git a/plugins/modules/docker_image_remove.py b/plugins/modules/docker_image_remove.py index 2b3f0e16..af59f5bb 100644 --- a/plugins/modules/docker_image_remove.py +++ b/plugins/modules/docker_image_remove.py @@ -219,6 +219,7 @@ class ImageRemover(DockerBaseClass): elif is_image_name_id(name): deleted.append(image["Id"]) + # TODO: the following is no longer correct with Docker 29+... untagged[:] = sorted( (image.get("RepoTags") or []) + (image.get("RepoDigests") or []) ) diff --git a/tests/integration/targets/docker_image/tasks/tests/options.yml b/tests/integration/targets/docker_image/tasks/tests/options.yml index 845c95d9..b391801a 100644 --- a/tests/integration/targets/docker_image/tasks/tests/options.yml +++ b/tests/integration/targets/docker_image/tasks/tests/options.yml @@ -256,6 +256,10 @@ - ansible.builtin.assert: that: - archive_image_2 is not changed + when: docker_cli_version is version("29.0.0", "<") + # Apparently idempotency no longer works with the default storage backend + # in Docker 29.0.0. + # https://github.com/ansible-collections/community.docker/pull/1199 - name: Archive image 3rd time, should overwrite due to different id community.docker.docker_image: diff --git a/tests/integration/targets/docker_image_export/tasks/tests/basic.yml b/tests/integration/targets/docker_image_export/tasks/tests/basic.yml index e81dd691..45345673 100644 --- a/tests/integration/targets/docker_image_export/tasks/tests/basic.yml +++ b/tests/integration/targets/docker_image_export/tasks/tests/basic.yml @@ -67,3 +67,7 @@ manifests_json: "{{ manifests.results | map(attribute='stdout') | map('from_json') }}" manifest_json_images: "{{ item.2 | map(attribute='Config') | map('regex_replace', '.json$', '') | map('regex_replace', '^blobs/sha256/', '') | sort }}" export_image_ids: "{{ item.1 | map('regex_replace', '^sha256:', '') | unique | sort }}" + when: docker_cli_version is version("29.0.0", "<") + # Apparently idempotency no longer works with the default storage backend + # in Docker 29.0.0. + # https://github.com/ansible-collections/community.docker/pull/1199 diff --git a/tests/integration/targets/docker_image_load/tasks/tests/basic.yml b/tests/integration/targets/docker_image_load/tasks/tests/basic.yml index 73d6482c..3748c55d 100644 --- a/tests/integration/targets/docker_image_load/tasks/tests/basic.yml +++ b/tests/integration/targets/docker_image_load/tasks/tests/basic.yml @@ -73,11 +73,17 @@ loop: "{{ all_images }}" when: remove_all_images is failed +- name: Show all images + ansible.builtin.command: docker image ls + - name: Load all images (IDs) community.docker.docker_image_load: path: "{{ remote_tmp_dir }}/archive-2.tar" register: result +- name: Show all images + ansible.builtin.command: docker image ls + - name: Print loaded image names ansible.builtin.debug: var: result.image_names @@ -110,11 +116,17 @@ name: "{{ item }}" loop: "{{ all_images }}" +- name: Show all images + ansible.builtin.command: docker image ls + - name: Load all images (mixed images and IDs) community.docker.docker_image_load: path: "{{ remote_tmp_dir }}/archive-3.tar" register: result +- name: Show all images + ansible.builtin.command: docker image ls + - name: Print loading log ansible.builtin.debug: var: result.stdout_lines @@ -127,10 +139,14 @@ that: - result is changed # For some reason, *sometimes* only the named image is found; in fact, in that case, the log only mentions that image and nothing else - - "result.images | length == 3 or ('Loaded image: ' ~ docker_test_image_hello_world) == result.stdout" - - (result.image_names | sort) in [[image_names[0], image_ids[0], image_ids[1]] | sort, [image_names[0]]] - - result.images | length in [1, 3] - - (result.images | map(attribute='Id') | sort) in [[image_ids[0], image_ids[0], image_ids[1]] | sort, [image_ids[0]]] + # With Docker 29, a third possibility appears: just two entries. + - >- + result.images | length == 3 + or ('Loaded image: ' ~ docker_test_image_hello_world) == result.stdout + or result.images | length == 2 + - (result.image_names | sort) in [[image_names[0], image_ids[0], image_ids[1]] | sort, [image_names[0], image_ids[1]] | sort, [image_names[0]]] + - result.images | length in [1, 2, 3] + - (result.images | map(attribute='Id') | sort) in [[image_ids[0], image_ids[0], image_ids[1]] | sort, [image_ids[0], image_ids[1]] | sort, [image_ids[0]]] # Same image twice @@ -139,11 +155,17 @@ name: "{{ item }}" loop: "{{ all_images }}" +- name: Show all images + ansible.builtin.command: docker image ls + - name: Load all images (same image twice) community.docker.docker_image_load: path: "{{ remote_tmp_dir }}/archive-4.tar" register: result +- name: Show all images + ansible.builtin.command: docker image ls + - name: Print loaded image names ansible.builtin.debug: var: result.image_names @@ -151,10 +173,11 @@ - ansible.builtin.assert: that: - result is changed - - result.image_names | length == 1 - - result.image_names[0] == image_names[0] - - result.images | length == 1 + - result.image_names | length in [1, 2] + - (result.image_names | sort) in [[image_names[0]], [image_names[0], image_ids[0]] | sort] + - result.images | length in [1, 2] - result.images[0].Id == image_ids[0] + - result.images[1].Id | default(image_ids[0]) == image_ids[0] # Single image by ID @@ -163,11 +186,17 @@ name: "{{ item }}" loop: "{{ all_images }}" +- name: Show all images + ansible.builtin.command: docker image ls + - name: Load all images (single image by ID) community.docker.docker_image_load: path: "{{ remote_tmp_dir }}/archive-5.tar" register: result +- name: Show all images + ansible.builtin.command: docker image ls + - name: Print loaded image names ansible.builtin.debug: var: result.image_names @@ -197,11 +226,17 @@ name: "{{ item }}" loop: "{{ all_images }}" +- name: Show all images + ansible.builtin.command: docker image ls + - name: Load all images (names) community.docker.docker_image_load: path: "{{ remote_tmp_dir }}/archive-1.tar" register: result +- name: Show all images + ansible.builtin.command: docker image ls + - name: Print loaded image names ansible.builtin.debug: var: result.image_names diff --git a/tests/integration/targets/docker_image_pull/tasks/tests/basic.yml b/tests/integration/targets/docker_image_pull/tasks/tests/basic.yml index 38282f96..7bb8daa0 100644 --- a/tests/integration/targets/docker_image_pull/tasks/tests/basic.yml +++ b/tests/integration/targets/docker_image_pull/tasks/tests/basic.yml @@ -142,6 +142,8 @@ - present_3_check.actions[0] == ('Pulled image ' ~ image_name) - present_3_check.diff.before.id == present_1.diff.after.id - present_3_check.diff.after.id == 'unknown' + - ansible.builtin.assert: + that: - present_3 is changed - present_3.actions | length == 1 - present_3.actions[0] == ('Pulled image ' ~ image_name) @@ -166,6 +168,11 @@ - present_5.actions[0] == ('Pulled image ' ~ image_name) - present_5.diff.before.id == present_3.diff.after.id - present_5.diff.after.id == present_1.diff.after.id + when: docker_cli_version is version("29.0.0", "<") + # From Docker 29 on, Docker won't pull images for other architectures + # if there are better matching ones. The above tests assume it will + # just do what it is told, and thus fail from 29.0.0 on. + # https://github.com/ansible-collections/community.docker/pull/1199 always: - name: cleanup diff --git a/tests/integration/targets/docker_image_pull/tasks/tests/image-ids.yml b/tests/integration/targets/docker_image_pull/tasks/tests/image-ids.yml index b28f8989..59c3d180 100644 --- a/tests/integration/targets/docker_image_pull/tasks/tests/image-ids.yml +++ b/tests/integration/targets/docker_image_pull/tasks/tests/image-ids.yml @@ -7,11 +7,9 @@ block: - name: Make sure images are not there community.docker.docker_image_remove: - name: "{{ item }}" + name: "sha256:{{ item }}" force: true - loop: - - "sha256:{{ docker_test_image_digest_v1_image_id }}" - - "sha256:{{ docker_test_image_digest_v2_image_id }}" + loop: "{{ docker_test_image_digest_v1_image_ids + docker_test_image_digest_v2_image_ids }}" - name: Pull image 1 community.docker.docker_image_pull: @@ -82,8 +80,6 @@ always: - name: cleanup community.docker.docker_image_remove: - name: "{{ item }}" + name: "sha256:{{ item }}" force: true - loop: - - "sha256:{{ docker_test_image_digest_v1_image_id }}" - - "sha256:{{ docker_test_image_digest_v2_image_id }}" + loop: "{{ docker_test_image_digest_v1_image_ids + docker_test_image_digest_v2_image_ids }}" diff --git a/tests/integration/targets/docker_image_push/tasks/tests/basic.yml b/tests/integration/targets/docker_image_push/tasks/tests/basic.yml index b9fab64c..5f780f85 100644 --- a/tests/integration/targets/docker_image_push/tasks/tests/basic.yml +++ b/tests/integration/targets/docker_image_push/tasks/tests/basic.yml @@ -18,7 +18,7 @@ - name: Push image ID (must fail) community.docker.docker_image_push: - name: "sha256:{{ docker_test_image_digest_v1_image_id }}" + name: "sha256:{{ docker_test_image_digest_v1_image_ids[0] }}" register: fail_2 ignore_errors: true diff --git a/tests/integration/targets/docker_image_push/tasks/tests/registry.yml b/tests/integration/targets/docker_image_push/tasks/tests/registry.yml index db44e7d4..b46d7c71 100644 --- a/tests/integration/targets/docker_image_push/tasks/tests/registry.yml +++ b/tests/integration/targets/docker_image_push/tasks/tests/registry.yml @@ -80,4 +80,6 @@ that: - push_4 is failed - >- - push_4.msg == ('Error pushing image ' ~ image_name_base2 ~ ':' ~ image_tag ~ ': no basic auth credentials') + push_4.msg.startswith('Error pushing image ' ~ image_name_base2 ~ ':' ~ image_tag ~ ': ') + - >- + push_4.msg.endswith(': no basic auth credentials') diff --git a/tests/integration/targets/docker_image_remove/tasks/main.yml b/tests/integration/targets/docker_image_remove/tasks/main.yml index 7f59069c..1a9df982 100644 --- a/tests/integration/targets/docker_image_remove/tasks/main.yml +++ b/tests/integration/targets/docker_image_remove/tasks/main.yml @@ -8,15 +8,16 @@ # and should not be used as examples of how to write Ansible roles # #################################################################### -- block: +- vars: + image: "{{ docker_test_image_hello_world }}" + image_ids: "{{ docker_test_image_hello_world_image_ids }}" + block: - name: Pick image prefix ansible.builtin.set_fact: iname_prefix: "{{ 'ansible-docker-test-%0x' % ((2**32) | random) }}" - name: Define image names ansible.builtin.set_fact: - image: "{{ docker_test_image_hello_world }}" - image_id: "{{ docker_test_image_hello_world_image_id }}" image_names: - "{{ iname_prefix }}-tagged-1:latest" - "{{ iname_prefix }}-tagged-1:foo" @@ -24,8 +25,9 @@ - name: Remove image complete community.docker.docker_image_remove: - name: "{{ image_id }}" + name: "{{ item }}" force: true + loop: "{{ image_ids }}" - name: Remove tagged images community.docker.docker_image_remove: @@ -102,10 +104,11 @@ - remove_2 is changed - remove_2.diff.before.id == pulled_image.image.Id - remove_2.diff.before.tags | length == 4 - - remove_2.diff.before.digests | length == 1 + # With Docker 29, there are now two digests in before and after: + - remove_2.diff.before.digests | length in [1, 2] - remove_2.diff.after.id == pulled_image.image.Id - remove_2.diff.after.tags | length == 3 - - remove_2.diff.after.digests | length == 1 + - remove_2.diff.after.digests | length in [1, 2] - remove_2.deleted | length == 0 - remove_2.untagged | length == 1 - remove_2.untagged[0] == (iname_prefix ~ '-tagged-1:latest') @@ -174,10 +177,11 @@ - remove_4 is changed - remove_4.diff.before.id == pulled_image.image.Id - remove_4.diff.before.tags | length == 3 - - remove_4.diff.before.digests | length == 1 + # With Docker 29, there are now two digests in before and after: + - remove_4.diff.before.digests | length in [1, 2] - remove_4.diff.after.id == pulled_image.image.Id - remove_4.diff.after.tags | length == 2 - - remove_4.diff.after.digests | length == 1 + - remove_4.diff.after.digests | length in [1, 2] - remove_4.deleted | length == 0 - remove_4.untagged | length == 1 - remove_4.untagged[0] == (iname_prefix ~ '-tagged-1:foo') @@ -245,16 +249,22 @@ - remove_6 is changed - remove_6.diff.before.id == pulled_image.image.Id - remove_6.diff.before.tags | length == 2 - - remove_6.diff.before.digests | length == 1 + # With Docker 29, there are now two digests in before and after: + - remove_6.diff.before.digests | length in [1, 2] - remove_6.diff.after.exists is false - - remove_6.deleted | length > 1 + - remove_6.deleted | length >= 1 - pulled_image.image.Id in remove_6.deleted - - remove_6.untagged | length == 3 + - remove_6.untagged | length in [2, 3] - (iname_prefix ~ '-tagged-1:bar') in remove_6.untagged - image in remove_6.untagged - remove_6_check.deleted | length == 1 - remove_6_check.deleted[0] == pulled_image.image.Id - - remove_6_check.untagged == remove_6.untagged + # The following is only true for Docker < 29... + # We use the CLI version as a proxy... + - >- + remove_6_check.untagged == remove_6.untagged + or + docker_cli_version is version("29.0.0", ">=") - info_5.images | length == 0 - name: Remove image ID (force, idempotent, check mode) diff --git a/tests/integration/targets/setup_docker/vars/main.yml b/tests/integration/targets/setup_docker/vars/main.yml index 4fbb4496..97445dd4 100644 --- a/tests/integration/targets/setup_docker/vars/main.yml +++ b/tests/integration/targets/setup_docker/vars/main.yml @@ -4,12 +4,18 @@ # SPDX-License-Identifier: GPL-3.0-or-later docker_test_image_digest_v1: e004c2cc521c95383aebb1fb5893719aa7a8eae2e7a71f316a4410784edb00a9 -docker_test_image_digest_v1_image_id: 758ec7f3a1ee85f8f08399b55641bfb13e8c1109287ddc5e22b68c3d653152ee +docker_test_image_digest_v1_image_ids: + - 758ec7f3a1ee85f8f08399b55641bfb13e8c1109287ddc5e22b68c3d653152ee # Docker 28 and before + - e004c2cc521c95383aebb1fb5893719aa7a8eae2e7a71f316a4410784edb00a9 # Docker 29 docker_test_image_digest_v2: ee44b399df993016003bf5466bd3eeb221305e9d0fa831606bc7902d149c775b -docker_test_image_digest_v2_image_id: dc3bacd8b5ea796cea5d6070c8f145df9076f26a6bc1c8981fd5b176d37de843 +docker_test_image_digest_v2_image_ids: + - dc3bacd8b5ea796cea5d6070c8f145df9076f26a6bc1c8981fd5b176d37de843 # Docker 28 and before + - ee44b399df993016003bf5466bd3eeb221305e9d0fa831606bc7902d149c775b # Docker 29 docker_test_image_digest_base: quay.io/ansible/docker-test-containers docker_test_image_hello_world: quay.io/ansible/docker-test-containers:hello-world -docker_test_image_hello_world_image_id: sha256:bf756fb1ae65adf866bd8c456593cd24beb6a0a061dedf42b26a993176745f6b +docker_test_image_hello_world_image_ids: + - sha256:bf756fb1ae65adf866bd8c456593cd24beb6a0a061dedf42b26a993176745f6b # Docker 28 and before + - sha256:90659bf80b44ce6be8234e6ff90a1ac34acbeb826903b02cfa0da11c82cbc042 # Docker 29 docker_test_image_hello_world_base: quay.io/ansible/docker-test-containers docker_test_image_busybox: quay.io/ansible/docker-test-containers:busybox docker_test_image_alpine: quay.io/ansible/docker-test-containers:alpine3.8 diff --git a/tests/integration/targets/setup_docker_registry/tasks/setup-frontend.yml b/tests/integration/targets/setup_docker_registry/tasks/setup-frontend.yml index 1eb2a188..4e467307 100644 --- a/tests/integration/targets/setup_docker_registry/tasks/setup-frontend.yml +++ b/tests/integration/targets/setup_docker_registry/tasks/setup-frontend.yml @@ -102,7 +102,17 @@ # This host/port combination cannot be used if the tests are running inside a docker container. docker_registry_frontend_address: localhost:{{ nginx_container.container.NetworkSettings.Ports['5000/tcp'].0.HostPort }} # The following host/port combination can be used from inside the docker container. - docker_registry_frontend_address_internal: "{{ nginx_container.container.NetworkSettings.Networks[current_container_network_ip].IPAddress if current_container_network_ip else nginx_container.container.NetworkSettings.IPAddress }}:5000" + docker_registry_frontend_address_internal: >- + {{ + nginx_container.container.NetworkSettings.Networks[current_container_network_ip].IPAddress + if current_container_network_ip else + ( + nginx_container.container.NetworkSettings.IPAddress + | default(nginx_container.container.NetworkSettings.Networks['bridge'].IPAddress) + ) + }}:5000 + # Since Docker 29, nginx_container.container.NetworkSettings.IPAddress no longer exists. + # Use the bridge network's IP address instead... - name: Wait for registry frontend ansible.builtin.uri: diff --git a/tests/integration/targets/setup_openssl/tasks/main.yml b/tests/integration/targets/setup_openssl/tasks/main.yml index c6ba8c73..d71de719 100644 --- a/tests/integration/targets/setup_openssl/tasks/main.yml +++ b/tests/integration/targets/setup_openssl/tasks/main.yml @@ -27,7 +27,7 @@ - name: Install cryptography (Darwin, and potentially upgrade for other OSes) become: true ansible.builtin.pip: - name: cryptography>=1.3.0 + name: cryptography>=3.3.0 extra_args: "-c {{ remote_constraints }}" - name: Register cryptography version