Merge ~twom/launchpad-buildd:error-handling into launchpad-buildd:master

[Tom Wardill]

Send exceptions to log file

OCI exceptions were being print()'d and then just continued.
Lets put them in the builder log and actually raise.

[Colin Watson (cjwatson)]

review: Approve

[Colin Watson (cjwatson)]

Oh, if you could add an entry to debian/changelog too, that'd be great.

c1689cc... by Tom Wardill on 2020-07-13

Update changelog

diff --git a/debian/changelog b/debian/changelog
index 9e45fea..bac55ca 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -34,6 +34,9 @@ launchpad-buildd (190) UNRELEASED; urgency=medium
   [ Dimitri John Ledkov ]
   * lxd: Add riscv64 to arch table.
 
+  [ Tom Wardill ]
+  * Improve error logging in OCI post-build 
+
  -- Colin Watson <[e-mail]>  Tue, 28 Apr 2020 10:19:27 +0100
 
 launchpad-buildd (189) xenial; urgency=medium
diff --git a/lpbuildd/oci.py b/lpbuildd/oci.py
index 27d34f1..c89a7c0 100644
--- a/lpbuildd/oci.py
+++ b/lpbuildd/oci.py
@@ -150,19 +150,21 @@ class OCIBuildManager(SnapBuildProxyMixin, DebianBuildManager):
     def gatherResults(self):
         """Gather the results of the build and add them to the file cache."""
         extract_path = tempfile.mkdtemp(prefix=self.name)
-        proc = self.backend.run(
-            ['docker', 'save', self.name],
-            get_output=True, return_process=True)
         try:
+            proc = self.backend.run(
+                ['docker', 'save', self.name],
+                get_output=True, return_process=True)
             tar = tarfile.open(fileobj=proc.stdout, mode="r|")
         except Exception as e:
-            print(e)
+            self._builder.log("Unable to save image: {}".format(e))
+            raise
 
         current_dir = ''
         directory_tar = None
         try:
             # The tarfile is a stream and must be processed in order
             for file in tar:
+                self._builder.log("Processing tar file: {}".format(file.name))
                 # Directories are just nodes, you can't extract the children
                 # directly, so keep track of what dir we're in.
                 if file.isdir():
@@ -188,13 +190,17 @@ class OCIBuildManager(SnapBuildProxyMixin, DebianBuildManager):
                     # If it's not in a directory, we need that
                     tar.extract(file, extract_path)
         except Exception as e:
-            print(e)
+            self._builder.log("Tar file processing failed: {}".format(e))
+            raise
         finally:
             if directory_tar is not None:
                 directory_tar.close()
 
         # We need these mapping files
         sha_directory = tempfile.mkdtemp()
+        # This can change depending on the kernel options / docker package
+        # 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)
@@ -222,4 +228,5 @@ class OCIBuildManager(SnapBuildProxyMixin, DebianBuildManager):
                 json.dump(digest_maps, digest_map_fp)
             self._builder.addWaitingFile(digest_map_file)
         except Exception as e:
-            print(e)
+            self._builder.log("Failed to parse manifest: {}".format(e))
+            raise