fix: podman.domain.containers.run pulls image if not found

Signed-off-by: Simone Tollardo <simone.tollardo@redhat.com>

Author: tollsimy

podman/domain/containers_create.py

@@ -7,13 +7,13 @@
 from contextlib import suppress
 from typing import Any, Union
 from collections.abc import MutableMapping
+import requests
 
 from podman import api
 from podman.domain.containers import Container
 from podman.domain.images import Image
 from podman.domain.pods import Pod
 from podman.domain.secrets import Secret
-from podman.errors import ImageNotFound
 
 logger = logging.getLogger("podman.containers")
 
@@ -36,6 +36,12 @@ def create(
             command: Command to run in the container.
 
         Keyword Args:
+            all_tags (bool): Pull all tags from repository. Default: False.
+                Only used if the method needs to pull the requested image.
+            auth_config (Mapping[str, str]): Override the credentials that are found in the
+                config for this request. auth_config should contain the username and password
+                keys to be valid.
+                Only used if the method needs to pull the requested image.
             auto_remove (bool): Enable auto-removal of the container on daemon side when the
                 container's process exits.
             blkio_weight_device (dict[str, Any]): Block IO weight (relative device weight)
@@ -55,6 +61,9 @@ def create(
             cpuset_cpus (str): CPUs in which to allow execution (0-3, 0,1).
             cpuset_mems (str): Memory nodes (MEMs) in which to allow execution (0-3, 0,1).
                 Only effective on NUMA systems.
+            decode (bool): Decode the JSON data from the server into dicts.
+                Only applies with ``stream=True``. Default: False.
+                Only used if the method needs to pull the requested image.
             detach (bool): Run container in the background and return a Container object.
             device_cgroup_rules (list[str]): A list of cgroup rules to apply to the container.
             device_read_bps: Limit read rate (bytes per second) from a device in the form of:
@@ -210,6 +219,8 @@ def create(
             pids_limit (int): Tune a container's pids limit. Set -1 for unlimited.
             platform (str): Platform in the format os[/arch[/variant]]. Only used if the method
                 needs to pull the requested image.
+            policy (str): Pull policy. "missing" (default for create), "always", "never", "newer".
+                Only used if the method needs to pull the requested image.
             ports (
                 dict[
                     Union[int, str],
@@ -271,7 +282,12 @@ def create(
 
                     }
             privileged (bool): Give extended privileges to this container.
+            progress_bar (bool): Display a progress bar with the image pull progress (uses
+                the compat endpoint). Default: False.
+                Only used if the method needs to pull the requested image.
             publish_all_ports (bool): Publish all ports to the host.
+            pull_stream (bool): When True, the pull progress will be published as received.
+                Default: False. Only used if the method needs to pull the requested image.
             read_only (bool): Mount the container's root filesystem as read only.
             read_write_tmpfs (bool): Mount temporary file systems as read write,
                 in case of read_only options set to True. Default: False
@@ -340,6 +356,8 @@ def create(
 
                 For example: {'/mnt/vol2': '', '/mnt/vol1': 'size=3G,uid=1000'}
 
+            tls_verify (bool): Require TLS verification. Default: True.
+                Only used if the method needs to pull the requested image.
             tty (bool): Allocate a pseudo-TTY.
             ulimits (list[Ulimit]): Ulimits to set inside the container.
             use_config_proxy (bool): If True, and if the docker client configuration
@@ -391,7 +409,6 @@ def create(
             A Container object.
 
         Raises:
-            ImageNotFound: when Image not found by Podman service
             APIError: when Podman service reports an error
         """
         if isinstance(image, Image):
@@ -409,7 +426,24 @@ def create(
             headers={"content-type": "application/json"},
             data=payload,
         )
-        response.raise_for_status(not_found=ImageNotFound)
+        if response.status_code == requests.codes.not_found:
+            self.podman_client.images.pull(
+                image,
+                all_tags=kwargs.get("all_tags", False),
+                auth_config=kwargs.get("auth_config"),
+                decode=kwargs.get("decode", False),
+                platform=kwargs.get("platform"),
+                policy=kwargs.get("policy", "missing"),
+                progress_bar=kwargs.get("progress_bar", False),
+                stream=kwargs.get("pull_stream", False),
+                tls_verify=kwargs.get("tls_verify", True),
+            )
+            response = self.client.post(
+                "/containers/create",
+                headers={"content-type": "application/json"},
+                data=payload,
+            )
+            response.raise_for_status()
 
         container_id = response.json()["Id"]
 
@@ -479,6 +513,13 @@ def _render_payload(kwargs: MutableMapping[str, Any]) -> dict[str, Any]:
             "stdout",  # used by caller
             "stream",  # used by caller
             "detach",  # used by caller
+            "all_tags",  # used by caller
+            "auth_config",  # used by caller
+            "decode",  # used by caller
+            "policy",  # used by caller
+            "progress_bar",  # used by caller
+            "pull_stream",  # used by caller
+            "tls_verify",  # used by caller
             "volume_driver",
         ):
             with suppress(KeyError):

podman/domain/containers_run.py

@@ -8,7 +8,7 @@
 
 from podman.domain.containers import Container
 from podman.domain.images import Image
-from podman.errors import ContainerError, ImageNotFound
+from podman.errors import ContainerError
 
 logger = logging.getLogger("podman.containers")
 
@@ -62,7 +62,6 @@ def run(
 
         Raises:
             ContainerError: when Container exists with a non-zero code
-            ImageNotFound: when Image not found by Podman service
             APIError: when Podman service reports an error
         """
         if isinstance(image, Image):
@@ -72,16 +71,7 @@ def run(
         if isinstance(command, str):
             command = [command]
 
-        try:
-            container = self.create(image=image_id, command=command, **kwargs)  # type: ignore[attr-defined]
-        except ImageNotFound:
-            self.podman_client.images.pull(  # type: ignore[attr-defined]
-                image_id,
-                auth_config=kwargs.get("auth_config"),
-                platform=kwargs.get("platform"),
-                policy=kwargs.get("policy", "missing"),
-            )
-            container = self.create(image=image_id, command=command, **kwargs)  # type: ignore[attr-defined]
+        container = self.create(image=image_id, command=command, **kwargs)  # type: ignore[attr-defined]
 
         container.start()
         container.reload()

podman/tests/unit/test_containersmanager.py

@@ -16,7 +16,7 @@
 from podman.domain.containers import Container
 from podman.domain.containers_create import CreateMixin
 from podman.domain.containers_manager import ContainersManager
-from podman.errors import ImageNotFound, NotFound
+from podman.errors import NotFound
 
 FIRST_CONTAINER = {
     "Id": "87e1325c82424e49a00abdd4de08009eb76c7de8d228426a9b8af9318ced5ecd",
@@ -338,17 +338,50 @@ def test_create(self, mock):
 
     @requests_mock.Mocker()
     def test_create_404(self, mock):
+        # mock the first POST to return 404,
+        # then the second POST (after pulling the image) to return 201
         mock.post(
             tests.LIBPOD_URL + "/containers/create",
-            status_code=404,
-            json={
-                "cause": "Image not found",
-                "message": "Image not found",
-                "response": 404,
-            },
+            [
+                {
+                    "status_code": 404,
+                    "json": {
+                        "cause": "Image not found",
+                        "message": "Image not found",
+                        "response": 404,
+                    },
+                },
+                {
+                    "status_code": 201,
+                    "json": {
+                        "Id": "87e1325c82424e49a00abdd4de08009eb76c7de8d228426a9b8af9318ced5ecd",
+                        "Warnings": [],
+                    },
+                },
+            ],
         )
-        with self.assertRaises(ImageNotFound):
-            self.client.containers.create("fedora", "/usr/bin/ls", cpu_count=9999)
+        self.client.images.pull = MagicMock()
+        mock.get(
+            tests.LIBPOD_URL + f"/containers/{FIRST_CONTAINER['Id']}/json",
+            json=FIRST_CONTAINER,
+        )
+        actual = self.client.containers.create("fedora", "/usr/bin/ls", cpu_count=9999)
+        self.client.images.pull.assert_called_once_with(
+            "fedora",
+            all_tags=False,
+            auth_config=None,
+            decode=False,
+            platform=None,
+            policy="missing",
+            progress_bar=False,
+            stream=False,
+            tls_verify=True,
+        )
+        self.assertIsInstance(actual, Container)
+        self.assertEqual(actual.id, FIRST_CONTAINER['Id'])
+        # 2 POSTs for create
+        # 1 GET for container json
+        self.assertEqual(mock.call_count, 3)
 
     @requests_mock.Mocker()
     def test_create_parse_host_port(self, mock):
@@ -644,6 +677,72 @@ def test_run(self, mock):
                 self.assertEqual(next(actual), b"This is a unittest - line 1")
                 self.assertEqual(next(actual), b"This is a unittest - line 2")
 
+    @requests_mock.Mocker()
+    def test_run_404(self, mock):
+        # mock the first POST to return 404,
+        # then the second POST (after pulling the image) to return 201
+        mock.post(
+            tests.LIBPOD_URL + "/containers/create",
+            [
+                {
+                    "status_code": 404,
+                    "json": {
+                        "cause": "Image not found",
+                        "message": "Image not found",
+                        "response": 404,
+                    },
+                },
+                {
+                    "status_code": 201,
+                    "json": {
+                        "Id": "87e1325c82424e49a00abdd4de08009eb76c7de8d228426a9b8af9318ced5ecd",
+                        "Warnings": [],
+                    },
+                },
+            ],
+        )
+        self.client.images.pull = MagicMock()
+        mock.post(
+            tests.LIBPOD_URL
+            + "/containers/87e1325c82424e49a00abdd4de08009eb76c7de8d228426a9b8af9318ced5ecd/start",
+            status_code=204,
+        )
+        mock.get(
+            tests.LIBPOD_URL + f"/containers/{FIRST_CONTAINER['Id']}/json",
+            json=FIRST_CONTAINER,
+        )
+
+        mock_logs = (
+            b"This is a unittest - line 1",
+            b"This is a unittest - line 2",
+        )
+
+        with patch.multiple(Container, logs=DEFAULT, wait=DEFAULT, autospec=True) as mock_container:
+            mock_container["wait"].return_value = 0
+            mock_container["logs"].return_value = iter(mock_logs)
+
+            actual = self.client.containers.run("fedora", "/usr/bin/ls")
+            self.client.images.pull.assert_called_once_with(
+                "fedora",
+                auth_config=None,
+                platform=None,
+                policy="missing",
+                progress_bar=False,
+                tls_verify=True,
+                decode=False,
+                all_tags=False,
+                stream=False,
+            )
+            self.assertIsInstance(actual, bytes)
+            self.assertEqual(actual, b"This is a unittest - line 1This is a unittest - line 2")
+            # 2 POSTs for create
+            # 1 POST for start
+            # 1 GET for container json
+            # 1 GET for reload
+            for r in mock.request_history:
+                print(r)
+            self.assertEqual(mock.call_count, 5)
+
     @requests_mock.Mocker()
     def test_create_all_healthcheck_parameters(self, mock):
         """Test that all healthcheck parameters are correctly passed to the API."""