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

Proposed by Tom Wardill
Status: Merged
Approved by: Tom Wardill
Approved revision: c1689cc46073bdbc2f0f5f8cbc31a171407c0a87
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~twom/launchpad-buildd:error-handling
Merge into: launchpad-buildd:master
Diff against target: 70 lines (+16/-6)
2 files modified
debian/changelog (+3/-0)
lpbuildd/oci.py (+13/-6)
Reviewer Review Type Date Requested Status
Colin Watson Approve
Review via email: mp+387288@code.launchpad.net

Commit message

Send exceptions to log file

Description of the change

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

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) :
review: Approve
Revision history for this message
Colin Watson (cjwatson) wrote :

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

c1689cc... by Tom Wardill

Update changelog

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 9e45fea..bac55ca 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -34,6 +34,9 @@ launchpad-buildd (190) UNRELEASED; urgency=medium
6 [ Dimitri John Ledkov ]
7 * lxd: Add riscv64 to arch table.
8
9+ [ Tom Wardill ]
10+ * Improve error logging in OCI post-build
11+
12 -- Colin Watson <cjwatson@ubuntu.com> Tue, 28 Apr 2020 10:19:27 +0100
13
14 launchpad-buildd (189) xenial; urgency=medium
15diff --git a/lpbuildd/oci.py b/lpbuildd/oci.py
16index 27d34f1..c89a7c0 100644
17--- a/lpbuildd/oci.py
18+++ b/lpbuildd/oci.py
19@@ -150,19 +150,21 @@ class OCIBuildManager(SnapBuildProxyMixin, DebianBuildManager):
20 def gatherResults(self):
21 """Gather the results of the build and add them to the file cache."""
22 extract_path = tempfile.mkdtemp(prefix=self.name)
23- proc = self.backend.run(
24- ['docker', 'save', self.name],
25- get_output=True, return_process=True)
26 try:
27+ proc = self.backend.run(
28+ ['docker', 'save', self.name],
29+ get_output=True, return_process=True)
30 tar = tarfile.open(fileobj=proc.stdout, mode="r|")
31 except Exception as e:
32- print(e)
33+ self._builder.log("Unable to save image: {}".format(e))
34+ raise
35
36 current_dir = ''
37 directory_tar = None
38 try:
39 # The tarfile is a stream and must be processed in order
40 for file in tar:
41+ self._builder.log("Processing tar file: {}".format(file.name))
42 # Directories are just nodes, you can't extract the children
43 # directly, so keep track of what dir we're in.
44 if file.isdir():
45@@ -188,13 +190,17 @@ class OCIBuildManager(SnapBuildProxyMixin, DebianBuildManager):
46 # If it's not in a directory, we need that
47 tar.extract(file, extract_path)
48 except Exception as e:
49- print(e)
50+ self._builder.log("Tar file processing failed: {}".format(e))
51+ raise
52 finally:
53 if directory_tar is not None:
54 directory_tar.close()
55
56 # We need these mapping files
57 sha_directory = tempfile.mkdtemp()
58+ # This can change depending on the kernel options / docker package
59+ # used. This is correct for bionic buildd image
60+ # with apt installed docker.
61 sha_path = ('/var/lib/docker/image/'
62 'vfs/distribution/v2metadata-by-diffid/sha256/')
63 sha_files = [x for x in self.backend.listdir(sha_path)
64@@ -222,4 +228,5 @@ class OCIBuildManager(SnapBuildProxyMixin, DebianBuildManager):
65 json.dump(digest_maps, digest_map_fp)
66 self._builder.addWaitingFile(digest_map_file)
67 except Exception as e:
68- print(e)
69+ self._builder.log("Failed to parse manifest: {}".format(e))
70+ raise

Subscribers

People subscribed via source and target branches