From 90c4b4c543c63e5940b27b791f7a73a5fbaed77f Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Sat, 15 Nov 2025 17:13:46 +0100 Subject: [PATCH] docker_image(_pull), docker_container: fix compatibility with Docker 29.0.0 (#1192) * Add debug flag to failing task. * Add more debug output. * Fix pull idempotency. * Revert "Add more debug output." This reverts commit 64020149bfee893dd3893653416d05d6d99c4493. * Fix casing. * Remove unreliable test. * Add 'debug: true' to all tasks. * Reformat. * Fix idempotency problem for IPv6 addresses. * Fix expose ranges handling. * Update changelog fragment to also mention other affected modules. --- .../fragments/1192-docker_container.yml | 9 ++ plugins/module_utils/_common_api.py | 17 +++- .../module_utils/_module_container/base.py | 45 +++++---- .../_module_container/docker_api.py | 29 +++--- .../module_utils/_module_container/module.py | 13 +-- plugins/module_utils/_util.py | 23 +++++ .../targets/docker_container/tasks/main.yml | 5 +- .../tasks/tests/image-ids.yml | 1 + .../docker_container/tasks/tests/options.yml | 13 --- .../docker_container/tasks/tests/ports.yml | 96 +++++++++++++++++++ 10 files changed, 197 insertions(+), 54 deletions(-) create mode 100644 changelogs/fragments/1192-docker_container.yml diff --git a/changelogs/fragments/1192-docker_container.yml b/changelogs/fragments/1192-docker_container.yml new file mode 100644 index 00000000..936eaac8 --- /dev/null +++ b/changelogs/fragments/1192-docker_container.yml @@ -0,0 +1,9 @@ +bugfixes: + - "docker_image - fix ``source=pull`` idempotency with Docker 29.0.0 (https://github.com/ansible-collections/community.docker/pull/1192)." + - "docker_image_pull - fix idempotency with Docker 29.0.0 (https://github.com/ansible-collections/community.docker/pull/1192)." + - "docker_container - fix ``pull`` idempotency with Docker 29.0.0 (https://github.com/ansible-collections/community.docker/pull/1192)." + - "docker_container - fix idempotency for IPv6 addresses with Docker 29.0.0 (https://github.com/ansible-collections/community.docker/pull/1192)." + - "docker_container - fix handling of exposed port ranges. So far, the module used an undocumented feature of Docker that was removed from Docker 29.0.0, + that allowed to pass the range to the deamon and let handle it. Now the module explodes ranges into a list of all contained ports, same as the + Docker CLI does. For backwards compatibility with Docker < 29.0.0, it also explodes ranges returned by the API for existing containers so that + comparison should only indicate a difference if the ranges actually change (https://github.com/ansible-collections/community.docker/pull/1192)." diff --git a/plugins/module_utils/_common_api.py b/plugins/module_utils/_common_api.py index 62a860f8..d1a9ec28 100644 --- a/plugins/module_utils/_common_api.py +++ b/plugins/module_utils/_common_api.py @@ -519,6 +519,17 @@ class AnsibleDockerClientBase(Client): except Exception as exc: # pylint: disable=broad-exception-caught self.fail(f"Error inspecting image ID {image_id} - {exc}") + @staticmethod + def _compare_images( + img1: dict[str, t.Any] | None, img2: dict[str, t.Any] | None + ) -> bool: + if img1 is None or img2 is None: + return img1 == img2 + filter_keys = {"Metadata"} + img1_filtered = {k: v for k, v in img1.items() if k not in filter_keys} + img2_filtered = {k: v for k, v in img2.items() if k not in filter_keys} + return img1_filtered == img2_filtered + def pull_image( self, name: str, tag: str = "latest", image_platform: str | None = None ) -> tuple[dict[str, t.Any] | None, bool]: @@ -526,7 +537,7 @@ class AnsibleDockerClientBase(Client): Pull an image """ self.log(f"Pulling image {name}:{tag}") - old_tag = self.find_image(name, tag) + old_image = self.find_image(name, tag) try: repository, image_tag = parse_repository_tag(name) registry, dummy_repo_name = auth.resolve_repository_name(repository) @@ -563,9 +574,9 @@ class AnsibleDockerClientBase(Client): except Exception as exc: # pylint: disable=broad-exception-caught self.fail(f"Error pulling image {name}:{tag} - {exc}") - new_tag = self.find_image(name, tag) + new_image = self.find_image(name, tag) - return new_tag, old_tag == new_tag + return new_image, self._compare_images(old_image, new_image) class AnsibleDockerClient(AnsibleDockerClientBase): diff --git a/plugins/module_utils/_module_container/base.py b/plugins/module_utils/_module_container/base.py index e8dd1f53..88a5fb57 100644 --- a/plugins/module_utils/_module_container/base.py +++ b/plugins/module_utils/_module_container/base.py @@ -1016,7 +1016,7 @@ def _preprocess_ports( else: port_binds = len(container_ports) * [(ipaddr,)] else: - return module.fail_json( + module.fail_json( msg=f'Invalid port description "{port}" - expected 1 to 3 colon-separated parts, but got {p_len}. ' "Maybe you forgot to use square brackets ([...]) around an IPv6 address?" ) @@ -1037,38 +1037,43 @@ def _preprocess_ports( binds[idx] = bind values["published_ports"] = binds - exposed = [] + exposed: set[tuple[int, str]] = set() if "exposed_ports" in values: for port in values["exposed_ports"]: port = to_text(port, errors="surrogate_or_strict").strip() protocol = "tcp" - matcher = re.search(r"(/.+$)", port) - if matcher: - protocol = matcher.group(1).replace("/", "") - port = re.sub(r"/.+$", "", port) - exposed.append((port, protocol)) + parts = port.split("/", maxsplit=1) + if len(parts) == 2: + port, protocol = parts + parts = port.split("-", maxsplit=1) + if len(parts) < 2: + try: + exposed.add((int(port), protocol)) + except ValueError as e: + module.fail_json(msg=f"Cannot parse port {port!r}: {e}") + else: + try: + start_port = int(parts[0]) + end_port = int(parts[1]) + if start_port > end_port: + raise ValueError( + "start port must be smaller or equal to end port." + ) + except ValueError as e: + module.fail_json(msg=f"Cannot parse port range {port!r}: {e}") + for port in range(start_port, end_port + 1): + exposed.add((port, protocol)) if "published_ports" in values: # Any published port should also be exposed for publish_port in values["published_ports"]: - match = False if isinstance(publish_port, str) and "/" in publish_port: port, protocol = publish_port.split("/") port = int(port) else: protocol = "tcp" port = int(publish_port) - for exposed_port in exposed: - if exposed_port[1] != protocol: - continue - if isinstance(exposed_port[0], str) and "-" in exposed_port[0]: - start_port, end_port = exposed_port[0].split("-") - if int(start_port) <= port <= int(end_port): - match = True - elif exposed_port[0] == port: - match = True - if not match: - exposed.append((port, protocol)) - values["ports"] = exposed + exposed.add((port, protocol)) + values["ports"] = sorted(exposed) return values diff --git a/plugins/module_utils/_module_container/docker_api.py b/plugins/module_utils/_module_container/docker_api.py index fdb7cc60..fa66d23f 100644 --- a/plugins/module_utils/_module_container/docker_api.py +++ b/plugins/module_utils/_module_container/docker_api.py @@ -1970,10 +1970,20 @@ def _get_values_ports( config = container["Config"] # "ExposedPorts": null returns None type & causes AttributeError - PR #5517 + expected_exposed: list[str] = [] if config.get("ExposedPorts") is not None: - expected_exposed = [_normalize_port(p) for p in config.get("ExposedPorts", {})] - else: - expected_exposed = [] + for port_and_protocol in config.get("ExposedPorts", {}): + port, protocol = _normalize_port(port_and_protocol).rsplit("/") + try: + start, end = port.split("-", 1) + start_port = int(start) + end_port = int(end) + for port_no in range(start_port, end_port + 1): + expected_exposed.append(f"{port_no}/{protocol}") + continue + except ValueError: + # Either it is not a range, or a broken one - in both cases, simply add the original form + expected_exposed.append(f"{port}/{protocol}") return { "published_ports": host_config.get("PortBindings"), @@ -2027,17 +2037,14 @@ def _get_expected_values_ports( ] expected_values["published_ports"] = expected_bound_ports - image_ports = [] + image_ports: set[str] = set() if image: image_exposed_ports = image["Config"].get("ExposedPorts") or {} - image_ports = [_normalize_port(p) for p in image_exposed_ports] - param_ports = [] + image_ports = {_normalize_port(p) for p in image_exposed_ports} + param_ports: set[str] = set() if "ports" in values: - param_ports = [ - to_text(p[0], errors="surrogate_or_strict") + "/" + p[1] - for p in values["ports"] - ] - result = list(set(image_ports + param_ports)) + param_ports = {f"{p[0]}/{p[1]}" for p in values["ports"]} + result = sorted(image_ports | param_ports) expected_values["exposed_ports"] = result if "publish_all_ports" in values: diff --git a/plugins/module_utils/_module_container/module.py b/plugins/module_utils/_module_container/module.py index 75128cfe..e9f10cd8 100644 --- a/plugins/module_utils/_module_container/module.py +++ b/plugins/module_utils/_module_container/module.py @@ -25,6 +25,7 @@ from ansible_collections.community.docker.plugins.module_utils._util import ( DockerBaseClass, compare_generic, is_image_name_id, + normalize_ip_address, sanitize_result, ) @@ -925,13 +926,13 @@ class ContainerManager(DockerBaseClass, t.Generic[Client]): else: diff = False network_info_ipam = network_info.get("IPAMConfig") or {} - if network.get("ipv4_address") and network[ - "ipv4_address" - ] != network_info_ipam.get("IPv4Address"): + if network.get("ipv4_address") and normalize_ip_address( + network["ipv4_address"] + ) != normalize_ip_address(network_info_ipam.get("IPv4Address")): diff = True - if network.get("ipv6_address") and network[ - "ipv6_address" - ] != network_info_ipam.get("IPv6Address"): + if network.get("ipv6_address") and normalize_ip_address( + network["ipv6_address"] + ) != normalize_ip_address(network_info_ipam.get("IPv6Address")): diff = True if network.get("aliases") and not compare_generic( network["aliases"], diff --git a/plugins/module_utils/_util.py b/plugins/module_utils/_util.py index 2ca60cac..45452a01 100644 --- a/plugins/module_utils/_util.py +++ b/plugins/module_utils/_util.py @@ -7,6 +7,7 @@ from __future__ import annotations +import ipaddress import json import re import typing as t @@ -505,3 +506,25 @@ def omit_none_from_dict(d: dict[str, t.Any]) -> dict[str, t.Any]: Return a copy of the dictionary with all keys with value None omitted. """ return {k: v for (k, v) in d.items() if v is not None} + + +@t.overload +def normalize_ip_address(ip_address: str) -> str: ... + + +@t.overload +def normalize_ip_address(ip_address: str | None) -> str | None: ... + + +def normalize_ip_address(ip_address: str | None) -> str | None: + """ + Given an IP address as a string, normalize it so that it can be + used to compare IP addresses as strings. + """ + if ip_address is None: + return None + try: + return ipaddress.ip_address(ip_address).compressed + except ValueError: + # Fallback for invalid addresses: simply return the input + return ip_address diff --git a/tests/integration/targets/docker_container/tasks/main.yml b/tests/integration/targets/docker_container/tasks/main.yml index 9c93511f..2717a635 100644 --- a/tests/integration/targets/docker_container/tasks/main.yml +++ b/tests/integration/targets/docker_container/tasks/main.yml @@ -37,7 +37,10 @@ register: docker_host_info # Run the tests -- block: +- module_defaults: + community.general.docker_container: + debug: true + block: - ansible.builtin.include_tasks: run-test.yml with_fileglob: - "tests/*.yml" diff --git a/tests/integration/targets/docker_container/tasks/tests/image-ids.yml b/tests/integration/targets/docker_container/tasks/tests/image-ids.yml index 82cf4b4d..e735b8c2 100644 --- a/tests/integration/targets/docker_container/tasks/tests/image-ids.yml +++ b/tests/integration/targets/docker_container/tasks/tests/image-ids.yml @@ -128,6 +128,7 @@ image: "{{ docker_test_image_digest_base }}@sha256:{{ docker_test_image_digest_v1 }}" name: "{{ cname }}" pull: true + debug: true state: present force_kill: true register: digest_3 diff --git a/tests/integration/targets/docker_container/tasks/tests/options.yml b/tests/integration/targets/docker_container/tasks/tests/options.yml index 2592d223..e8492310 100644 --- a/tests/integration/targets/docker_container/tasks/tests/options.yml +++ b/tests/integration/targets/docker_container/tasks/tests/options.yml @@ -3686,18 +3686,6 @@ register: platform_5 ignore_errors: true -- name: platform (idempotency) - community.docker.docker_container: - image: "{{ docker_test_image_simple_1 }}" - name: "{{ cname }}" - state: present - pull: true - platform: 386 - force_kill: true - debug: true - register: platform_6 - ignore_errors: true - - name: cleanup community.docker.docker_container: name: "{{ cname }}" @@ -3712,7 +3700,6 @@ - platform_3 is not changed and platform_3 is not failed - platform_4 is not changed and platform_4 is not failed - platform_5 is changed - - platform_6 is not changed and platform_6 is not failed when: docker_api_version is version('1.41', '>=') - ansible.builtin.assert: that: diff --git a/tests/integration/targets/docker_container/tasks/tests/ports.yml b/tests/integration/targets/docker_container/tasks/tests/ports.yml index 4f70fe89..95a59a78 100644 --- a/tests/integration/targets/docker_container/tasks/tests/ports.yml +++ b/tests/integration/targets/docker_container/tasks/tests/ports.yml @@ -106,6 +106,101 @@ force_kill: true register: published_ports_3 +- name: published_ports -- port range (same range, but listed explicitly) + community.docker.docker_container: + image: "{{ docker_test_image_alpine }}" + command: '/bin/sh -c "sleep 10m"' + name: "{{ cname }}" + state: started + exposed_ports: + - "9001" + - "9010" + - "9011" + - "9012" + - "9013" + - "9014" + - "9015" + - "9016" + - "9017" + - "9018" + - "9019" + - "9020" + - "9021" + - "9022" + - "9023" + - "9024" + - "9025" + - "9026" + - "9027" + - "9028" + - "9029" + - "9030" + - "9031" + - "9032" + - "9033" + - "9034" + - "9035" + - "9036" + - "9037" + - "9038" + - "9039" + - "9040" + - "9041" + - "9042" + - "9043" + - "9044" + - "9045" + - "9046" + - "9047" + - "9048" + - "9049" + - "9050" + published_ports: + - "9001:9001" + - "9020:9020" + - "9021:9021" + - "9022:9022" + - "9023:9023" + - "9024:9024" + - "9025:9025" + - "9026:9026" + - "9027:9027" + - "9028:9028" + - "9029:9029" + - "9030:9030" + - "9031:9031" + - "9032:9032" + - "9033:9033" + - "9034:9034" + - "9035:9035" + - "9036:9036" + - "9037:9037" + - "9038:9038" + - "9039:9039" + - "9040:9040" + - "9041:9041" + - "9042:9042" + - "9043:9043" + - "9044:9044" + - "9045:9045" + - "9046:9046" + - "9047:9047" + - "9048:9048" + - "9049:9049" + - "9050:9050" + - "9051:9051" + - "9052:9052" + - "9053:9053" + - "9054:9054" + - "9055:9055" + - "9056:9056" + - "9057:9057" + - "9058:9058" + - "9059:9059" + - "9060:9060" + force_kill: true + register: published_ports_4 + - name: cleanup community.docker.docker_container: name: "{{ cname }}" @@ -118,6 +213,7 @@ - published_ports_1 is changed - published_ports_2 is not changed - published_ports_3 is changed + - published_ports_4 is not changed #################################################################### ## published_ports: one-element container port range ###############