launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging ~twom/launchpad-buildd:fix-scratch-oci-builds into launchpad-buildd:master.

Commit message:
Fix scratch OCI builds

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~twom/launchpad-buildd/+git/launchpad-buildd/+merge/395241

FROM scratch OCI builds, or other builds that don't pull an image, don't create the metadata directory on disk.
Check if it exists before we attempt to read from it.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~twom/launchpad-buildd:fix-scratch-oci-builds into launchpad-buildd:master.

diff --git a/debian/changelog b/debian/changelog
index 6f57d8a..6728dae 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,9 +1,13 @@
 launchpad-buildd (194) UNRELEASED; urgency=medium
 
+  [ Colin Watson ]
   * Stop setting $mailto in .sbuildrc, to work around LP #1859010.
   * Stop overquoting OCI --build-arg options (LP: #1902007).
   * Update production deployment documentation in README.
 
+  [ Tom Wardill ]
+  * Fix OCI builds that don't pull another image. 
+
  -- Colin Watson <cjwatson@xxxxxxxxxx>  Tue, 27 Oct 2020 13:22:05 +0000
 
 launchpad-buildd (193) bionic; urgency=medium
diff --git a/lpbuildd/oci.py b/lpbuildd/oci.py
index 6a404fd..cf8d1d7 100644
--- a/lpbuildd/oci.py
+++ b/lpbuildd/oci.py
@@ -209,14 +209,20 @@ class OCIBuildManager(SnapBuildProxyMixin, DebianBuildManager):
         # used. This is correct for bionic buildd image
         # with apt installed docker.
         sha_path = ('/var/lib/docker/image/'
-                    'vfs/distribution/v2metadata-by-diffid/sha256/')
-        sha_files = [x for x in self.backend.listdir(sha_path)
-                     if not x.startswith('.')]
-        for file in sha_files:
-            self.backend.copy_out(
-                os.path.join(sha_path, file),
-                os.path.join(sha_directory, file)
-            )
+                    'vfs/distribution/v2metadata-by-diffid/sha256')
+        # If there have been no images pulled in the build process
+        # (FROM scratch), then this directory will not exist and
+        # we will have no contents from it.
+        if self.backend.path_exists(sha_path):
+            sha_files = [x for x in self.backend.listdir(sha_path)
+                        if not x.startswith('.')]
+            for file in sha_files:
+                self.backend.copy_out(
+                    os.path.join(sha_path, file),
+                    os.path.join(sha_directory, file)
+                )
+        else:
+            self._builder.log("No metadata directory at {}".format(sha_path))
 
         # Parse the manifest for the other files we need
         manifest_path = os.path.join(extract_path, 'manifest.json')
diff --git a/lpbuildd/tests/test_oci.py b/lpbuildd/tests/test_oci.py
index 23641e4..15a4d64 100644
--- a/lpbuildd/tests/test_oci.py
+++ b/lpbuildd/tests/test_oci.py
@@ -130,8 +130,7 @@ class TestOCIBuildManagerIteration(TestCase):
         self.buildmanager.backend.add_file(
             '/var/lib/docker/image/'
             'vfs/distribution/v2metadata-by-diffid/sha256/diff1',
-            b"""[{"Digest": "test_digest", "SourceRepository": "test"}]"""
-        )
+            b"""[{"Digest": "test_digest", "SourceRepository": "test"}]""")
 
         # After building the package, reap processes.
         yield self.buildmanager.iterate(0)
@@ -271,6 +270,33 @@ class TestOCIBuildManagerIteration(TestCase):
         self.assertFalse(self.builder.wasCalled("buildFail"))
 
     @defer.inlineCallbacks
+    def test_iterate_no_pull(self):
+        # check with no pulled images.
+        # This sha would change as it includes file attributes in the
+        # tar file. Fix it so we can test against a known value.
+        sha_mock = self.useFixture(
+            MockPatch('lpbuildd.oci.OCIBuildManager._calculateLayerSha'))
+        sha_mock.mock.return_value = "testsha"
+        # The build manager iterates a normal build from start to finish.
+        args = {
+            "git_repository": "https://git.launchpad.dev/~example/+git/snap";,
+            "git_path": "master",
+            }
+        expected_options = [
+            "--git-repository", "https://git.launchpad.dev/~example/+git/snap";,
+            "--git-path", "master",
+            ]
+        yield self.startBuild(args, expected_options)
+
+        log_path = os.path.join(self.buildmanager._cachepath, "buildlog")
+        with open(log_path, "w") as log:
+            log.write("I am a build log.")
+
+        self.buildmanager.backend.run.result = MockOCITarSave()
+        yield self.buildmanager.iterate(0)
+        self.assertFalse(self.builder.wasCalled("buildFail"))
+
+    @defer.inlineCallbacks
     def test_iterate_snap_store_proxy(self):
         # The build manager can be told to use a snap store proxy.
         self.builder._config.set(