From c3acb9946910fe150b7029018858144d5d23173d Mon Sep 17 00:00:00 2001 From: Federico Rizzo Date: Fri, 20 Jun 2025 16:47:08 +0200 Subject: [PATCH 1/3] fix image build not using cache This commit fixes issue #528 by adding a default value to parameters layers and outputformat. This change aligns the behavior with podman-remote. Function documentation is updated accordingly. A new integration test (test_images.py:test_build_cache) checks that caching is enabled and also that disabling it produces a different output. Signed-off-by: Federico Rizzo --- podman/domain/images_build.py | 14 ++++----- podman/tests/integration/test_images.py | 39 +++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/podman/domain/images_build.py b/podman/domain/images_build.py index 7d8f2983..1850ae70 100644 --- a/podman/domain/images_build.py +++ b/podman/domain/images_build.py @@ -63,9 +63,10 @@ def build(self, **kwargs) -> tuple[Image, Iterator[bytes]]: isolation (str) – Isolation technology used during build. (ignored) use_config_proxy (bool) – (ignored) http_proxy (bool) - Inject http proxy environment variables into container (Podman only) - layers (bool) - Cache intermediate layers during build. + layers (bool) - Cache intermediate layers during build. Default True. output (str) - specifies if any custom build output is selected for following build. outputformat (str) - The format of the output image's manifest and configuration data. + Default to "application/vnd.oci.image.manifest.v1+json" (OCI format). manifest (str) - add the image to the specified manifest list. Creates manifest list if it does not exist. secrets (list[str]) - Secret files/envs to expose to the build @@ -172,7 +173,7 @@ def _render_params(kwargs) -> dict[str, list[Any]]: raise PodmanError("Custom encoding not supported when gzip enabled.") params = { - "dockerfile": kwargs.get("dockerfile"), + "dockerfile": kwargs.get("dockerfile", f".containerfile.{random.getrandbits(160):x}"), "forcerm": kwargs.get("forcerm"), "httpproxy": kwargs.get("http_proxy"), "networkmode": kwargs.get("network_mode"), @@ -187,9 +188,11 @@ def _render_params(kwargs) -> dict[str, list[Any]]: "squash": kwargs.get("squash"), "t": kwargs.get("tag"), "target": kwargs.get("target"), - "layers": kwargs.get("layers"), + "layers": kwargs.get("layers", True), "output": kwargs.get("output"), - "outputformat": kwargs.get("outputformat"), + "outputformat": kwargs.get( + "outputformat", "application/vnd.oci.image.manifest.v1+json" + ), } if "buildargs" in kwargs: @@ -213,8 +216,5 @@ def _render_params(kwargs) -> dict[str, list[Any]]: if "secrets" in kwargs: params["secrets"] = json.dumps(kwargs.get("secrets")) - if params["dockerfile"] is None: - params["dockerfile"] = f".containerfile.{random.getrandbits(160):x}" - # Remove any unset parameters return dict(filter(lambda i: i[1] is not None, params.items())) diff --git a/podman/tests/integration/test_images.py b/podman/tests/integration/test_images.py index 5711e2b2..c2ebf0b9 100644 --- a/podman/tests/integration/test_images.py +++ b/podman/tests/integration/test_images.py @@ -16,11 +16,13 @@ import io import os +import json import platform import tarfile import tempfile import types import unittest +import random import podman.tests.integration.base as base from podman import PodmanClient @@ -151,6 +153,43 @@ def test_build(self): self.assertIsNotNone(image) self.assertIsNotNone(image.id) + def test_build_cache(self): + """Check build caching when enabled + + Build twice with caching enabled (default), then again with nocache + """ + + def look_for_cache(stream) -> bool: + # Search for a line with contents "-> Using cache " + uses_cache = False + for line in stream: + parsed = json.loads(line)['stream'] + if "Using cache" in parsed: + uses_cache = True + break + return uses_cache + + label = str(random.getrandbits(32)) + buffer = io.StringIO(f"""FROM scratch\nLABEL test={label}""") + image, _ = self.client.images.build(fileobj=buffer) + buffer.seek(0) + cached_image, stream = self.client.images.build(fileobj=buffer) + self.assertTrue(look_for_cache(stream)) + self.assertEqual( + cached_image.id, + image.id, + msg="Building twice with cache does not produce the same image id", + ) + # Build again with disabled cache + buffer.seek(0) + uncached_image, stream = self.client.images.build(fileobj=buffer, nocache=True) + self.assertFalse(look_for_cache(stream)) + self.assertNotEqual( + uncached_image.id, + image.id, + msg="Building twice without cache produces the same image id", + ) + def test_build_with_manifest(self): buffer = io.StringIO("""FROM quay.io/libpod/alpine_labels:latest""") From 59ce400d8867c45d3dcc088f01dec33aee5aa2a7 Mon Sep 17 00:00:00 2001 From: Federico Rizzo Date: Tue, 24 Jun 2025 16:01:00 +0200 Subject: [PATCH 2/3] add unit test for images.build default parameters This commit also fixes wrong mock URL in the same unit test Signed-off-by: Federico Rizzo --- podman/tests/unit/test_build.py | 70 ++++++++++++++++++++++++--------- 1 file changed, 52 insertions(+), 18 deletions(-) diff --git a/podman/tests/unit/test_build.py b/podman/tests/unit/test_build.py index e83238ce..e8df8ff6 100644 --- a/podman/tests/unit/test_build.py +++ b/podman/tests/unit/test_build.py @@ -1,4 +1,5 @@ import io +import requests import json import unittest @@ -16,6 +17,20 @@ from podman.domain.images import Image from podman.errors import BuildError, DockerException +good_image_id = "032b8b2855fc" +good_stream = [ + {"stream": " ---\u003e a9eb17255234"}, + {"stream": "Step 1 : VOLUME /data"}, + {"stream": " ---\u003e Running in abdc1e6896c6"}, + {"stream": " ---\u003e 713bca62012e"}, + {"stream": "Removing intermediate container abdc1e6896c6"}, + {"stream": "Step 2 : CMD [\"/bin/sh\"]"}, + {"stream": " ---\u003e Running in dba30f2a1a7e"}, + {"stream": " ---\u003e 032b8b2855fc"}, + {"stream": "Removing intermediate container dba30f2a1a7e"}, + {"stream": f"{good_image_id}\n"}, +] + class TestBuildCase(unittest.TestCase): """Test ImagesManager build(). @@ -41,19 +56,7 @@ def test_build(self, mock_prepare_containerfile, mock_create_tar): mock_prepare_containerfile.return_value = "Containerfile" mock_create_tar.return_value = b"This is a mocked tarball." - stream = [ - {"stream": " ---\u003e a9eb17255234"}, - {"stream": "Step 1 : VOLUME /data"}, - {"stream": " ---\u003e Running in abdc1e6896c6"}, - {"stream": " ---\u003e 713bca62012e"}, - {"stream": "Removing intermediate container abdc1e6896c6"}, - {"stream": "Step 2 : CMD [\"/bin/sh\"]"}, - {"stream": " ---\u003e Running in dba30f2a1a7e"}, - {"stream": " ---\u003e 032b8b2855fc"}, - {"stream": "Removing intermediate container dba30f2a1a7e"}, - {"stream": "032b8b2855fc\n"}, - ] - + stream = good_stream buffer = io.StringIO() for entry in stream: buffer.write(json.JSONEncoder().encode(entry)) @@ -72,9 +75,9 @@ def test_build(self, mock_prepare_containerfile, mock_create_tar): text=buffer.getvalue(), ) mock.get( - tests.LIBPOD_URL + "/images/032b8b2855fc/json", + tests.LIBPOD_URL + f"/images/{good_image_id}/json", json={ - "Id": "032b8b2855fc", + "Id": good_image_id, "ParentId": "", "RepoTags": ["fedora:latest", "fedora:33", ":"], "RepoDigests": [ @@ -104,7 +107,7 @@ def test_build(self, mock_prepare_containerfile, mock_create_tar): secrets=["id=example,src=podman-build-secret123"], ) self.assertIsInstance(image, Image) - self.assertEqual(image.id, "032b8b2855fc") + self.assertEqual(image.id, good_image_id) self.assertIsInstance(logs, Iterable) @patch.object(api, "create_tar") @@ -134,16 +137,47 @@ def test_build_logged_error(self, mock_prepare_containerfile, mock_create_tar): @requests_mock.Mocker() def test_build_no_context(self, mock): - mock.post(tests.LIBPOD_URL + "/images/build") + mock.post(tests.LIBPOD_URL + "/build") with self.assertRaises(TypeError): self.client.images.build() @requests_mock.Mocker() def test_build_encoding(self, mock): - mock.post(tests.LIBPOD_URL + "/images/build") + mock.post(tests.LIBPOD_URL + "/build") with self.assertRaises(DockerException): self.client.images.build(path="/root", gzip=True, encoding="utf-8") + @patch.object(api, "create_tar") + @patch.object(api, "prepare_containerfile") + def test_build_defaults(self, mock_prepare_containerfile, mock_create_tar): + """Check the defaults used by images.build""" + mock_prepare_containerfile.return_value = "Containerfile" + mock_create_tar.return_value = b"This is a mocked tarball." + + stream = good_stream + buffer = io.StringIO() + for entry in stream: + buffer.write(json.dumps(entry)) + buffer.write("\n") + + with requests_mock.Mocker() as mock: + query = "?outputformat=" + ( + requests.utils.quote("application/vnd.oci.image.manifest.v1+json", safe='') + + "&layers=True" + ) + mock.post( + tests.LIBPOD_URL + "/build" + query, + text=buffer.getvalue(), + ) + mock.get( + tests.LIBPOD_URL + f"/images/{good_image_id}/json", + json={ + "Id": "unittest", + }, + ) + img, _ = self.client.images.build(path="/tmp/context_dir") + assert img.id == "unittest" + if __name__ == '__main__': unittest.main() From 781def8964a5b0b0d0b18f0657f5d5c1d39bf897 Mon Sep 17 00:00:00 2001 From: Federico Rizzo Date: Wed, 26 Nov 2025 20:37:58 +0100 Subject: [PATCH 3/3] misc fixes to in integration tests - remove image left by integration test test_containers.py:test_container_rm_anonymous_volume: when caching is enabled, an intermediary image generated by the build function call (corresponding to layer created by "VOLUME myvol", see container_file in the first lines of function test_container_rm_anonymous_volume) breaks test_images.py:test_image_crud sub-test "Delete Image", where the same base image (quay.io/libpod/alpine:latest) is supposed to be removed, but is instead untagged, as it is in "use" by the other layers. This also breaks sub-test "Delete unused Images" and, on on successive runs, "Retrieve Image history". - avoid using external images in test_images.py:test_build - fix deprecation and unused variable warnings Signed-off-by: Federico Rizzo --- podman/tests/integration/test_containers.py | 17 +++++------------ podman/tests/integration/test_images.py | 7 +++---- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/podman/tests/integration/test_containers.py b/podman/tests/integration/test_containers.py index cd32f1ef..4e7e30c6 100644 --- a/podman/tests/integration/test_containers.py +++ b/podman/tests/integration/test_containers.py @@ -1,7 +1,6 @@ import io import random import tarfile -import tempfile import unittest try: @@ -86,7 +85,7 @@ def test_container_crud(self): with self.subTest("Archive /root/unittest"): self.assertTrue(container.put_archive("/root", data=tarball)) - actual, stats = container.get_archive("/root") + actual, _ = container.get_archive("/root") with io.BytesIO() as fd: for chunk in actual: @@ -184,16 +183,8 @@ def test_container_commit(self): def test_container_rm_anonymous_volume(self): with self.subTest("Check anonymous volume is removed"): - container_file = """ -FROM alpine -VOLUME myvol -ENV foo=bar -""" - tmp_file = tempfile.mktemp() - file = open(tmp_file, 'w') - file.write(container_file) - file.close() - self.client.images.build(dockerfile=tmp_file, tag="test-img", path=".") + container_file = io.StringIO("\n".join(["FROM alpine", "VOLUME myvol", "ENV foo=bar"])) + test_img, _ = self.client.images.build(fileobj=container_file, tag="test-img", path=".") # get existing number of containers and volumes existing_containers = self.client.containers.list(all=True) @@ -211,6 +202,8 @@ def test_container_rm_anonymous_volume(self): self.assertEqual(len(container_list), len(existing_containers)) volume_list = self.client.volumes.list() self.assertEqual(len(volume_list), len(existing_volumes)) + # clean up + self.client.images.remove(test_img) def test_container_labels(self): labels = {'label1': 'value1', 'label2': 'value2'} diff --git a/podman/tests/integration/test_images.py b/podman/tests/integration/test_images.py index c2ebf0b9..24f448c9 100644 --- a/podman/tests/integration/test_images.py +++ b/podman/tests/integration/test_images.py @@ -147,9 +147,8 @@ def test_corrupt_load(self): self.assertIn("payload does not match", e.exception.explanation) def test_build(self): - buffer = io.StringIO("""FROM quay.io/libpod/alpine_labels:latest""") - - image, stream = self.client.images.build(fileobj=buffer) + buffer = io.StringIO("""FROM scratch""") + image, _ = self.client.images.build(fileobj=buffer) self.assertIsNotNone(image) self.assertIsNotNone(image.id) @@ -229,7 +228,7 @@ def add_file(name: str, content: str): # If requesting a custom context, currently must specify the dockerfile name self.client.images.build(custom_context=True, fileobj=context) - image, stream = self.client.images.build( + image, _ = self.client.images.build( fileobj=context, dockerfile="MyDockerfile", custom_context=True,