Merge pull request #559 from fulminemizzega/main

fix image build not using cache

Author: openshift-merge-bot[bot]

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()))

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'}

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
@@ -145,12 +147,48 @@ 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)
 
+    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 <image id>"
+            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""")
 
@@ -190,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,

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", "<none>:<none>"],
                     "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()