launchpad-reviewers team mailing list archive

[Colin Watson <mp+423375@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:buildd-manager-clear-grabbing into launchpad:master.

Commit message:
Clear grabbing directory on buildd-manager startup

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/423375

This can easily fill up with quite large files if buildd-manager is restarted while gathering builds, and the previous contents are never useful after a restart.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:buildd-manager-clear-grabbing into launchpad:master.

diff --git a/lib/lp/buildmaster/manager.py b/lib/lp/buildmaster/manager.py
index 03a21b3..894e632 100644
--- a/lib/lp/buildmaster/manager.py
+++ b/lib/lp/buildmaster/manager.py
@@ -14,6 +14,8 @@ from collections import defaultdict
 import datetime
 import functools
 import logging
+import os.path
+import shutil
 
 import six
 from storm.expr import (
@@ -53,6 +55,7 @@ from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
 from lp.buildmaster.interfaces.processor import IProcessorSet
 from lp.buildmaster.model.builder import Builder
 from lp.buildmaster.model.buildqueue import BuildQueue
+from lp.services.config import config
 from lp.services.database.bulk import dbify_value
 from lp.services.database.interfaces import IStore
 from lp.services.database.stormexpr import (
@@ -796,6 +799,17 @@ class BuilddManager(service.Service):
 
     def startService(self):
         """Service entry point, called when the application starts."""
+        # Clear "grabbing" directory, used by
+        # BuildFarmJobBehaviourBase.handleSuccess as temporary storage for
+        # results of builds.  They're moved to "incoming" once they've been
+        # gathered completely, so any files still here when buildd-manager
+        # starts are useless, and we can easily end up with quite large
+        # leftovers here if buildd-manager was restarted while gathering
+        # builds.  The behaviour will recreate this directory as needed.
+        try:
+            shutil.rmtree(os.path.join(config.builddmaster.root, "grabbing"))
+        except FileNotFoundError:
+            pass
         # Add and start WorkerScanners for each current builder, and any
         # added in the future.
         self.scan_builders_loop, self.scan_builders_deferred = (
diff --git a/lib/lp/buildmaster/tests/test_manager.py b/lib/lp/buildmaster/tests/test_manager.py
index 3a6e086..1716b94 100644
--- a/lib/lp/buildmaster/tests/test_manager.py
+++ b/lib/lp/buildmaster/tests/test_manager.py
@@ -1209,6 +1209,20 @@ class TestBuilddManager(TestCase):
         scanner_names = {scanner.builder_name for scanner in scanners}
         self.assertEqual(builder_names, scanner_names)
 
+    def test_startService_clears_grabbing(self):
+        # When startService is called, the manager clears out the "grabbing"
+        # directory.
+        self._stub_out_scheduleNextScanCycle()
+        tempdir = self.makeTemporaryDirectory()
+        self.pushConfig("builddmaster", root=tempdir)
+        os.makedirs(os.path.join(tempdir, "grabbing", "some-upload"))
+        clock = task.Clock()
+        manager = BuilddManager(clock=clock)
+
+        manager.startService()
+
+        self.assertFalse(os.path.exists(os.path.join(tempdir, "grabbing")))
+
     def test_startService_adds_scanBuilders_loop(self):
         # When startService is called, the manager will start up a
         # scanBuilders loop.