launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-upload-deleted-builds into lp:launchpad.

Commit message:
Move upload to FAILED if its build was deleted (e.g. because of a deleted snap).

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1655334 in Launchpad itself: "process-upload oopses if snap was deleted after build completed"
  https://bugs.launchpad.net/launchpad/+bug/1655334

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-upload-deleted-builds/+merge/314433
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-upload-deleted-builds into lp:launchpad.

=== modified file 'lib/lp/archiveuploader/tests/test_uploadprocessor.py'
--- lib/lp/archiveuploader/tests/test_uploadprocessor.py	2016-07-02 07:55:26 +0000
+++ lib/lp/archiveuploader/tests/test_uploadprocessor.py	2017-01-10 15:41:21 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Functional tests for uploadprocessor.py."""
@@ -2258,6 +2258,28 @@
         self.assertIsNot(None, build.duration)
         self.assertIs(None, build.upload_log)
 
+    def testSnapBuild_deleted_snap(self):
+        # A snap build will fail if the snap is deleted.
+        self.switchToAdmin()
+        build = self.factory.makeSnapBuild()
+        self.switchToUploader()
+        Store.of(build).flush()
+        behaviour = IBuildFarmJobBehaviour(build)
+        leaf_name = behaviour.getUploadDirLeaf(build.build_cookie)
+        os.mkdir(os.path.join(self.incoming_folder, leaf_name))
+        self.options.context = 'buildd'
+        self.options.builds = True
+        build.updateStatus(BuildStatus.UPLOADING)
+        self.switchToAdmin()
+        build.snap.destroySelf()
+        self.switchToUploader()
+        BuildUploadHandler(
+            self.uploadprocessor, self.incoming_folder, leaf_name).process()
+        self.assertFalse(
+            os.path.exists(os.path.join(self.incoming_folder, leaf_name)))
+        self.assertTrue(
+            os.path.exists(os.path.join(self.failed_folder, leaf_name)))
+
     def processUploadWithBuildStatus(self, status):
         upload_dir = self.queueUpload("bar_1.0-1")
         self.processUpload(self.uploadprocessor, upload_dir)

=== modified file 'lib/lp/archiveuploader/uploadprocessor.py'
--- lib/lp/archiveuploader/uploadprocessor.py	2015-08-03 15:07:29 +0000
+++ lib/lp/archiveuploader/uploadprocessor.py	2017-01-10 15:41:21 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Code for 'processing' 'uploads'. Also see nascentupload.py.
@@ -645,7 +645,12 @@
         Build uploads always contain a single package per leaf.
         """
         logger = BufferLogger()
-        if self.build.status == BuildStatus.BUILDING:
+        if self.build is None:
+            self.processor.log.info(
+                "Build %s was deleted. Ignoring." % self.upload)
+            self.moveUpload(UploadStatusEnum.FAILED, logger)
+            return
+        elif self.build.status == BuildStatus.BUILDING:
             # Handoff from buildd-manager isn't complete until the
             # status is UPLOADING.
             self.processor.log.info("Build status is BUILDING. Ignoring.")