Merge lp:~wgrant/launchpad/couple-of-oopses into lp:launchpad

[William Grant]

Don't double-upload build results when the log download fails, don't squash IntegrityErrors in LibraryFileAliasSet.create, and verify that a Swift connection is what it says it is before returning it to the librarian ConnectionPool.

An OOPS fix, an OOPS change and an added OOPS:

 - Bug #1422199: Minimise the risk of duplicate buildd-manager uploads
   by moving the non-transactional filesystem operations as close to
   the commit as possible. Previously the log download could time out
   and abort the transaction after the upload had already been moved
   into incoming/.

 - Bug #1422207: Stop squashing IntegrityErrors in
   LibraryFileAliasSet.create. Pre-check the filename rather than
   waiting for the database to reject it and hoping that the error
   we're catching is the right one. This will let us diagnose some odd
   process-upload failures.

 - Bug #1420046: Librarian's Swift ConnectionPool now verifies that
   anything it's given is actually a swiftclient.Connection. Hopefully
   this will give us useful tracebacks at the source of the None
   connections.

[Colin Watson (cjwatson)]

review: Approve

=== modified file 'lib/lp/buildmaster/model/buildfarmjobbehaviour.py'
--- lib/lp/buildmaster/model/buildfarmjobbehaviour.py	2014-06-26 12:33:51 +0000
+++ lib/lp/buildmaster/model/buildfarmjobbehaviour.py	2015-02-16 01:16:18 +0000
@@ -221,19 +221,24 @@
             % (self.build.build_cookie, self.build.title,
                self.build.buildqueue_record.builder.name, status))
         if status == 'OK':
+            yield self.storeLogFromSlave()
+            # handleSuccess will sometimes perform write operations
+            # outside the database transaction, so a failure between
+            # here and the commit can cause duplicated results. For
+            # example, a BinaryPackageBuild will end up in the upload
+            # queue twice if notify() crashes.
             yield self.handleSuccess(slave_status, logger)
         elif status in fail_status_map:
             # XXX wgrant: The builder should be set long before here, but
             # currently isn't.
+            yield self.storeLogFromSlave()
             self.build.updateStatus(
                 fail_status_map[status],
                 builder=self.build.buildqueue_record.builder,
                 slave_status=slave_status)
-            transaction.commit()
         else:
             raise BuildDaemonError(
                 "Build returned unexpected status: %r" % status)
-        yield self.storeLogFromSlave()
         if notify:
             self.build.notify()
         self.build.buildqueue_record.destroySelf()

=== modified file 'lib/lp/services/librarian/model.py'
--- lib/lp/services/librarian/model.py	2014-09-01 11:44:56 +0000
+++ lib/lp/services/librarian/model.py	2015-02-16 01:16:18 +0000
@@ -255,11 +255,9 @@
             client = getUtility(IRestrictedLibrarianClient)
         else:
             client = getUtility(ILibrarianClient)
-        try:
-            fid = client.addFile(
-                name, size, file, contentType, expires, debugID)
-        except IntegrityError:
+        if '/' in name:
             raise InvalidFilename("Filename cannot contain slashes.")
+        fid = client.addFile(name, size, file, contentType, expires, debugID)
         lfa = IMasterStore(LibraryFileAlias).find(
             LibraryFileAlias, LibraryFileAlias.id == fid).one()
         assert lfa is not None, "client.addFile didn't!"

=== modified file 'lib/lp/services/librarianserver/swift.py'
--- lib/lp/services/librarianserver/swift.py	2014-12-16 09:20:15 +0000
+++ lib/lp/services/librarianserver/swift.py	2015-02-16 01:16:18 +0000
@@ -346,6 +346,9 @@
         exception has been raised (apart from a 404), don't trust the
         swift_connection and throw it away.
         '''
+        if not isinstance(swift_connection, swiftclient.Connection):
+            raise AssertionError(
+                "%r is not a swiftclient Connection." % swift_connection)
         if swift_connection not in self._pool:
             self._pool.append(swift_connection)
             while len(self._pool) > self.MAX_POOL_SIZE: