Merge pull request #189 from dcermak/dont-pull-always

Don't pull always

.github/workflows/ci.yml

@@ -166,13 +166,16 @@ jobs:
           mkdir ./tmp/
           chmod 1777 ./tmp
           export TMPDIR="$(pwd)/tmp"
+          export PULL_ALWAYS=0
+          nox -s "test-${{ matrix.python_version }}(${{ matrix.container_runtime }})" -- -x -n auto --reruns 3 --pytest-container-log-level DEBUG
+          export PULL_ALWAYS=1
           nox -s "test-${{ matrix.python_version }}(${{ matrix.container_runtime }})" -- -x -n auto --reruns 3 --pytest-container-log-level DEBUG
           nox -s "test-${{ matrix.python_version }}(${{ matrix.container_runtime }})" -- -x --reruns 3 --pytest-container-log-level DEBUG
           nox -s coverage
 
       - name: verify that no stray containers are left
         run: |
-          [[ $(${{ matrix.container_runtime }} ps -aq|wc -l) = "0" ]]
+          [[ $(podman ps -aq|wc -l) = '0' ]] || (podman ps -aq|xargs podman inspect; exit 1)
 
       - name: verify that no stray volumes are left
         run: |

CHANGELOG.rst

@@ -3,6 +3,10 @@ Next Release
 
 Breaking changes:
 
+- add the parameter ``container_runtime`` to
+  :py:func:`~pytest_container.container.ContainerBaseABC.prepare_container` and
+  :py:func:`~pytest_container.build.MultiStageBuild.prepare_build`.
+
 - deprecate the function ``pytest_container.container_from_pytest_param``,
   please use
   :py:func:`~pytest_container.container.container_and_marks_from_pytest_param`
@@ -14,6 +18,10 @@ Breaking changes:
 
 Improvements and new features:
 
+- Allow to configure whether container images are always pulled before test runs
+  or whether cached images can be used via the environment variable
+  ``PULL_ALWAYS`` (see :ref:`controlling-image-pulling-behavior`).
+
 - Add attributes :py:attr:`~pytest_container.inspect.ContainerInspect.name` and
   :py:attr:`~pytest_container.inspect.ContainerNetworkSettings.ip_address`
   exposing the container's name & IP

pytest_container/build.py

@@ -168,6 +168,7 @@ def containerfile(self) -> str:
     def prepare_build(
         self,
         tmp_path: Path,
+        container_runtime: OciRuntimeBase,
         rootdir: Path,
         extra_build_args: Optional[List[str]] = None,
     ) -> None:
@@ -182,7 +183,9 @@ def prepare_build(
             if not isinstance(container, str):
                 container_and_marks_from_pytest_param(container)[
                     0
-                ].prepare_container(rootdir, extra_build_args)
+                ].prepare_container(
+                    container_runtime, rootdir, extra_build_args
+                )
 
         dockerfile_dest = tmp_path / "Dockerfile"
         with open(dockerfile_dest, "w", encoding="utf-8") as containerfile:
@@ -271,6 +274,7 @@ def build(
         )
         self.prepare_build(
             tmp_path,
+            runtime,
             root,
             extra_build_args,
         )

pytest_container/container.py

@@ -44,6 +44,7 @@
 import testinfra
 from _pytest.mark import ParameterSet
 from filelock import FileLock
+from pytest_container.helpers import get_always_pull_option
 from pytest_container.inspect import ContainerHealth
 from pytest_container.inspect import ContainerInspect
 from pytest_container.inspect import PortForwarding
@@ -625,7 +626,10 @@ class ContainerBaseABC(ABC):
 
     @abstractmethod
     def prepare_container(
-        self, rootdir: Path, extra_build_args: Optional[List[str]]
+        self,
+        container_runtime: OciRuntimeBase,
+        rootdir: Path,
+        extra_build_args: Optional[List[str]],
     ) -> None:
         """Prepares the container so that it can be launched."""
 
@@ -649,19 +653,30 @@ def baseurl(self) -> Optional[str]:
 class Container(ContainerBase, ContainerBaseABC):
     """This class stores information about the Container Image under test."""
 
-    def pull_container(self) -> None:
+    def pull_container(self, container_runtime: OciRuntimeBase) -> None:
         """Pulls the container with the given url using the currently selected
         container runtime"""
-        runtime = get_selected_runtime()
-        _logger.debug("Pulling %s via %s", self.url, runtime.runner_binary)
-        check_output([runtime.runner_binary, "pull", self.url])
+        _logger.debug(
+            "Pulling %s via %s", self.url, container_runtime.runner_binary
+        )
+        check_output([container_runtime.runner_binary, "pull", self.url])
 
     def prepare_container(
-        self, rootdir: Path, extra_build_args: Optional[List[str]] = None
+        self,
+        container_runtime: OciRuntimeBase,
+        rootdir: Path,
+        extra_build_args: Optional[List[str]] = None,
     ) -> None:
         """Prepares the container so that it can be launched."""
-        if not self._is_local:
-            self.pull_container()
+        if self._is_local:
+            return
+
+        if get_always_pull_option():
+            self.pull_container(container_runtime)
+            return
+
+        if call([container_runtime.runner_binary, "inspect", self.url]) != 0:
+            self.pull_container(container_runtime)
 
     def get_base(self) -> "Container":
         return self
@@ -723,25 +738,32 @@ def get_base(self) -> Union[Container, "DerivedContainer"]:
         return self.base
 
     def prepare_container(
-        self, rootdir: Path, extra_build_args: Optional[List[str]] = None
+        self,
+        container_runtime: OciRuntimeBase,
+        rootdir: Path,
+        extra_build_args: Optional[List[str]] = None,
     ) -> None:
         _logger.debug("Preparing derived container based on %s", self.base)
         if isinstance(self.base, str):
             # we need to pull the container so that the inspect in the launcher
             # doesn't fail
             Container(url=self.base).prepare_container(
-                rootdir, extra_build_args
+                container_runtime, rootdir, extra_build_args
             )
         else:
-            self.base.prepare_container(rootdir, extra_build_args)
+            self.base.prepare_container(
+                container_runtime, rootdir, extra_build_args
+            )
 
         runtime = get_selected_runtime()
 
         # do not build containers without a containerfile and where no build
         # tags are added
         if not self.containerfile and not self.add_build_tags:
             base = self.get_base()
-            base.prepare_container(rootdir, extra_build_args)
+            base.prepare_container(
+                container_runtime, rootdir, extra_build_args
+            )
             self.container_id, self.url = base.container_id, base.url
             return
 
@@ -1042,7 +1064,7 @@ def release_lock() -> None:
         try:
             lock.acquire()
             self.container.prepare_container(
-                self.rootdir, self.extra_build_args
+                self.container_runtime, self.rootdir, self.extra_build_args
             )
         except:
             release_lock()

pytest_container/helpers.py

@@ -4,6 +4,7 @@
 
 """
 import logging
+import os
 from typing import List
 
 from _pytest.config import Config
@@ -143,3 +144,14 @@ def get_extra_pod_create_args(pytestconfig: Config) -> List[str]:
 
     """
     return pytestconfig.getoption("extra_pod_create_args", default=[]) or []
+
+
+def get_always_pull_option() -> bool:
+    """Returns whether images should be always pulled before launching the
+    container or whether the container runtime can use the locally cached
+    image. This setting is controlled via the environment variable
+    ``PULL_ALWAYS``. If the environment variable is unset, then the default is
+    ``True``.
+
+    """
+    return bool(int(os.getenv("PULL_ALWAYS", "1")))

source/usage.rst

@@ -104,6 +104,22 @@ A Container defined in this way can be used like any other Container
 instance.
 
 
+.. _controlling-image-pulling-behavior:
+
+Controlling the image pulling behavior
+--------------------------------------
+
+``pytest_container`` will by default pull all container images from the defined
+registry before launching containers for tests. This is to ensure that stale
+images are not used by accident. The downside is, that tests take longer to
+execute, as the container runtime will try to pull images before every test.
+
+This behavior can be configured via the environment variable
+``PULL_ALWAYS``. Setting it to ``0`` results in ``pytest_container`` relying on
+the image cache and only pulling images if they are not present in the local
+container storage.
+
+
 Container Runtime version
 -------------------------
 

tests/test_container.py

@@ -7,6 +7,7 @@
 from pytest_container import Container
 from pytest_container import DerivedContainer
 from pytest_container.container import ImageFormat
+from pytest_container.runtime import OciRuntimeBase
 
 from . import images
 
@@ -37,16 +38,18 @@ def test_image_format() -> None:
     assert str(ImageFormat.OCIv1) == "oci"
 
 
-def test_local_image_url() -> None:
+def test_local_image_url(container_runtime: OciRuntimeBase) -> None:
     url = "docker.io/library/iDontExistHopefully/bazbarf/something"
     cont = Container(url=f"containers-storage:{url}")
     assert cont.local_image
     assert cont.url == url
     # prepare must not call `$runtime pull` as that would fail
-    cont.prepare_container(Path("."), [])
+    cont.prepare_container(container_runtime, Path("."), [])
 
 
-def test_lockfile_path(pytestconfig: pytest.Config) -> None:
+def test_lockfile_path(
+    container_runtime: OciRuntimeBase, pytestconfig: pytest.Config
+) -> None:
     """Check that the attribute
     :py:attr:`~pytest_container.ContainerBase.lockfile_filename` does change by
     the container having the attribute
@@ -58,7 +61,7 @@ def test_lockfile_path(pytestconfig: pytest.Config) -> None:
     )
     original_lock_fname = cont.filelock_filename
 
-    cont.prepare_container(pytestconfig.rootpath)
+    cont.prepare_container(container_runtime, pytestconfig.rootpath)
     assert cont.container_id, "container_id must not be empty"
     assert cont.filelock_filename == original_lock_fname
 
@@ -73,9 +76,11 @@ def test_lockfile_unique() -> None:
     assert cont1.filelock_filename != cont2.filelock_filename
 
 
-def test_derived_container_build_tag(pytestconfig: pytest.Config) -> None:
+def test_derived_container_build_tag(
+    container_runtime: OciRuntimeBase, pytestconfig: pytest.Config
+) -> None:
     cont = DerivedContainer(base=images.OPENSUSE_BUSYBOX_URL)
-    cont.prepare_container(pytestconfig.rootpath)
+    cont.prepare_container(container_runtime, pytestconfig.rootpath)
     assert cont._build_tag == images.OPENSUSE_BUSYBOX_URL
 
 

tests/test_container_build.py

@@ -97,7 +97,7 @@ def test_container_data(container: ContainerData):
 def test_local_container_image_ref(
     container_runtime: OciRuntimeBase, pytestconfig: Config
 ):
-    LEAP_WITH_TAG.prepare_container(pytestconfig.rootpath)
+    LEAP_WITH_TAG.prepare_container(container_runtime, pytestconfig.rootpath)
 
     # this container only works if LEAP_WITH_TAG exists already
     local_container = Container(url=f"containers-storage:{TAG1}")
@@ -206,7 +206,7 @@ def test_container_size(
     container_runtime: OciRuntimeBase, pytestconfig: Config
 ):
     for container in [BUSYBOX_WITH_ENTRYPOINT, BUSYBOX_WITH_GARBAGE]:
-        container.prepare_container(pytestconfig.rootpath)
+        container.prepare_container(container_runtime, pytestconfig.rootpath)
 
     assert container_runtime.get_image_size(
         BUSYBOX_WITH_ENTRYPOINT

tests/test_launcher.py

@@ -1,10 +1,12 @@
 # pylint: disable=missing-function-docstring,missing-module-docstring
 import os
+import subprocess
 import tempfile
+from contextlib import ExitStack
 from pathlib import Path
-from subprocess import CalledProcessError
 from time import sleep
 from typing import Any
+from unittest.mock import patch
 
 import pytest
 from pytest_container import inspect
@@ -223,13 +225,7 @@ def test_launcher_does_not_override_stopsignal_for_entrypoint(
     assert container.inspect.config.stop_signal in (9, "SIGKILL")
 
 
-@pytest.mark.parametrize(
-    "container",
-    [
-        CMDLINE_APP_CONTAINER,
-    ],
-    indirect=True,
-)
+@pytest.mark.parametrize("container", [CMDLINE_APP_CONTAINER], indirect=True)
 def test_launcher_does_can_check_binaries_with_entrypoint(
     container: ContainerData,
 ) -> None:
@@ -256,6 +252,70 @@ def test_derived_container_pulls_base(
         assert launcher.container_data.container_id
 
 
+def test_pulls_container(
+    container_runtime: OciRuntimeBase,
+    pytestconfig: pytest.Config,
+    monkeypatch: pytest.MonkeyPatch,
+):
+    """Test of the pull-behavior switching via the environment variable
+    ``PULL_ALWAYS``
+
+    """
+    quay_busybox = "quay.io/libpod/busybox"
+
+    with ExitStack() as stack:
+        # mock setup
+        mock_check_output = stack.enter_context(
+            patch("pytest_container.container.check_output")
+        )
+        mock_check_call = stack.enter_context(
+            patch("pytest_container.container.call")
+        )
+        mock_check_output.return_value = None
+
+        _pull = lambda: Container(url=quay_busybox).prepare_container(
+            container_runtime, pytestconfig.rootpath
+        )
+
+        # first test: should always pull the image
+        monkeypatch.setenv("PULL_ALWAYS", "1")
+        _pull()
+
+        mock_check_output.assert_called_once_with(
+            [container_runtime.runner_binary, "pull", quay_busybox]
+        )
+        mock_check_call.assert_not_called()
+
+        mock_check_output.reset_mock()
+        mock_check_call.reset_mock()
+
+        # second test: should only pull the image if inspect fails
+        # in this case we mock the inspect call to return 0, i.e. image is there
+        monkeypatch.setenv("PULL_ALWAYS", "0")
+        mock_check_call.return_value = 0
+
+        _pull()
+        mock_check_call.assert_called_once_with(
+            [container_runtime.runner_binary, "inspect", quay_busybox]
+        )
+        mock_check_output.assert_not_called()
+
+        mock_check_output.reset_mock()
+        mock_check_call.reset_mock()
+
+        # third test: pull the image if inspect fails, so we mock the inspect
+        # call to return 1
+        mock_check_call.return_value = 1
+
+        _pull()
+        mock_check_call.assert_called_once_with(
+            [container_runtime.runner_binary, "inspect", quay_busybox]
+        )
+        mock_check_output.assert_called_once_with(
+            [container_runtime.runner_binary, "pull", quay_busybox]
+        )
+
+
 def test_launcher_unlocks_on_preparation_failure(
     container_runtime: OciRuntimeBase, pytestconfig: pytest.Config
 ) -> None:
@@ -264,7 +324,7 @@ def test_launcher_unlocks_on_preparation_failure(
     )
 
     def try_launch():
-        with pytest.raises(CalledProcessError):
+        with pytest.raises(subprocess.CalledProcessError):
             with ContainerLauncher(
                 container_with_wrong_url,
                 container_runtime,