launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/faster-cleanup into lp:launchpad-buildd.

Commit message:
If the extra build arguments include fast_cleanup: True, then skip the final cleanup steps of the build.  This can be used when building in a VM that is guaranteed to be torn down after the build.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/faster-cleanup/+merge/344938
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/faster-cleanup into lp:launchpad-buildd.

=== modified file 'debian/changelog'
--- debian/changelog	2018-04-23 08:45:51 +0000
+++ debian/changelog	2018-05-02 09:01:56 +0000
@@ -1,3 +1,11 @@
+launchpad-buildd (162) UNRELEASED; urgency=medium
+
+  * If the extra build arguments include fast_cleanup: True, then skip the
+    final cleanup steps of the build.  This can be used when building in a
+    VM that is guaranteed to be torn down after the build.
+
+ -- Colin Watson <cjwatson@xxxxxxxxxx>  Wed, 02 May 2018 09:58:15 +0100
+
 launchpad-buildd (161) xenial; urgency=medium
 
   * Pass build URL to snapcraft using SNAPCRAFT_IMAGE_INFO.

=== modified file 'lpbuildd/debian.py'
--- lpbuildd/debian.py	2017-08-22 13:45:46 +0000
+++ lpbuildd/debian.py	2018-05-02 09:01:56 +0000
@@ -115,7 +115,7 @@
         finally:
             chfile.close()
 
-    def iterate(self, success):
+    def iterate(self, success, quiet=False):
         # When a Twisted ProcessControl class is killed by SIGTERM,
         # which we call 'build process aborted', 'None' is returned as
         # exit_code.
@@ -123,8 +123,9 @@
             # We may have been aborted in between subprocesses; pretend that
             # we were terminated by a signal, which is close enough.
             success = 128 + signal.SIGKILL
-        log.msg("Iterating with success flag %s against stage %s"
-                % (success, self._state))
+        if not quiet:
+            log.msg("Iterating with success flag %s against stage %s"
+                    % (success, self._state))
         func = getattr(self, "iterate_" + self._state, None)
         if func is None:
             raise ValueError("Unknown internal state " + self._state)

=== modified file 'lpbuildd/slave.py'
--- lpbuildd/slave.py	2018-03-08 11:29:25 +0000
+++ lpbuildd/slave.py	2018-05-02 09:01:56 +0000
@@ -203,20 +203,27 @@
 
     def doCleanup(self):
         """Remove the build tree etc."""
-        self.runTargetSubProcess("remove-build")
+        if not self.fast_cleanup:
+            self.runTargetSubProcess("remove-build")
 
         # Sanitize the URLs in the buildlog file if this is a build
         # in a private archive.
         if self.needs_sanitized_logs:
             self._slave.sanitizeBuildlog(self._slave.cachePath("buildlog"))
 
+        if self.fast_cleanup:
+            self.iterate(0, quiet=True)
+
     def doMounting(self):
         """Mount things in the chroot, e.g. proc."""
         self.runTargetSubProcess("mount-chroot")
 
     def doUnmounting(self):
         """Unmount the chroot."""
-        self.runTargetSubProcess("umount-chroot")
+        if self.fast_cleanup:
+            self.iterate(0, quiet=True)
+        else:
+            self.runTargetSubProcess("umount-chroot")
 
     def initiate(self, files, chroot, extra_args):
         """Initiate a build given the input files.
@@ -237,6 +244,7 @@
 
         self.series = extra_args['series']
         self.arch_tag = extra_args.get('arch_tag', self._slave.getArch())
+        self.fast_cleanup = extra_args.get('fast_cleanup', False)
 
         # Check whether this is a build in a private archive and
         # whether the URLs in the buildlog file should be sanitized
@@ -259,7 +267,7 @@
         """
         return {}
 
-    def iterate(self, success):
+    def iterate(self, success, quiet=False):
         """Perform an iteration of the slave.
 
         The BuildManager tends to work by invoking several

=== modified file 'lpbuildd/tests/test_debian.py'
--- lpbuildd/tests/test_debian.py	2017-08-22 14:46:10 +0000
+++ lpbuildd/tests/test_debian.py	2018-05-02 09:01:56 +0000
@@ -328,3 +328,96 @@
         self.assertFalse(self.slave.wasCalled('depFail'))
         self.assertTrue(self.slave.wasCalled('buildOK'))
         self.assertTrue(self.slave.wasCalled('buildComplete'))
+
+    def test_iterate_fast_cleanup(self):
+        # The build manager can be told that it doesn't need to do the final
+        # cleanup steps, because the VM is about to be torn down anyway.  It
+        # iterates such a build from start to finish, but without calling
+        # umount-chroot or remove-build.
+        extra_args = {
+            'arch_tag': 'amd64',
+            'archives': [
+                'deb http://ppa.launchpad.dev/owner/name/ubuntu xenial main',
+                ],
+            'fast_cleanup': True,
+            'series': 'xenial',
+            }
+        self.startBuild(extra_args)
+
+        self.buildmanager.iterate(0)
+        self.assertEqual(DebianBuildState.UNPACK, self.getState())
+        self.assertEqual(
+            (['sharepath/slavebin/in-target', 'in-target',
+              'unpack-chroot',
+              '--backend=chroot', '--series=xenial', '--arch=amd64',
+              self.buildid,
+              os.path.join(self.buildmanager._cachepath, 'chroot.tar.gz')],
+             None),
+            self.buildmanager.commands[-1])
+        self.assertEqual(
+            self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+        self.buildmanager.iterate(0)
+        self.assertEqual(DebianBuildState.MOUNT, self.getState())
+        self.assertEqual(
+            (['sharepath/slavebin/in-target', 'in-target',
+              'mount-chroot',
+              '--backend=chroot', '--series=xenial', '--arch=amd64',
+              self.buildid],
+             None),
+            self.buildmanager.commands[-1])
+        self.assertEqual(
+            self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+        self.buildmanager.iterate(0)
+        self.assertEqual(DebianBuildState.SOURCES, self.getState())
+        self.assertEqual(
+            (['sharepath/slavebin/in-target', 'in-target',
+              'override-sources-list',
+              '--backend=chroot', '--series=xenial', '--arch=amd64',
+              self.buildid,
+              'deb http://ppa.launchpad.dev/owner/name/ubuntu xenial main'],
+             None),
+            self.buildmanager.commands[-1])
+        self.assertEqual(
+            self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+        self.buildmanager.iterate(0)
+        self.assertEqual(DebianBuildState.UPDATE, self.getState())
+        self.assertEqual(
+            (['sharepath/slavebin/in-target', 'in-target',
+              'update-debian-chroot',
+              '--backend=chroot', '--series=xenial', '--arch=amd64',
+              self.buildid],
+             None),
+            self.buildmanager.commands[-1])
+        self.assertEqual(
+            self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+        self.buildmanager.iterate(0)
+        self.assertEqual(MockBuildState.MAIN, self.getState())
+        self.assertEqual(
+            (['/bin/true', 'true'], None), self.buildmanager.commands[-1])
+        self.assertEqual(
+            self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+        self.buildmanager.iterate(0)
+        self.assertEqual(MockBuildState.MAIN, self.getState())
+        self.assertEqual(
+            (['sharepath/slavebin/in-target', 'in-target',
+              'scan-for-processes',
+              '--backend=chroot', '--series=xenial', '--arch=amd64',
+              self.buildid],
+             None),
+            self.buildmanager.commands[-1])
+        self.assertNotEqual(
+            self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+        self.buildmanager.iterateReap(self.getState(), 0)
+        self.assertFalse(self.slave.wasCalled('builderFail'))
+        self.assertFalse(self.slave.wasCalled('chrootFail'))
+        self.assertFalse(self.slave.wasCalled('buildFail'))
+        self.assertFalse(self.slave.wasCalled('depFail'))
+        self.assertTrue(self.slave.wasCalled('buildOK'))
+        self.assertTrue(self.slave.wasCalled('buildComplete'))
+