Disable CleanupWorker thread for test_image_upload_fail

We currently have a race condition between our cleanup worker and our
unit test. My hope is, if we agree to disable the CleanupWorker thread
for the test, we still consider this a valid test.

Change-Id: I04b87ef044de7f99cc9cbd0c08747e53d383693b
Signed-off-by: Paul Belanger <pabelanger@redhat.com>

nodepool/builder.py

@@ -1114,18 +1114,19 @@ class NodePoolBuilder(object):
                 w.start()
                 self._upload_workers.append(w)
 
-            self._janitor = CleanupWorker(0, self._config_path,
+            if self.cleanup_interval > 0:
-                                          self.cleanup_interval, self.zk)
+                self._janitor = CleanupWorker(
-            self._janitor.start()
+                    0, self._config_path, self.cleanup_interval, self.zk)
+                self._janitor.start()
 
             # Wait until all threads are running. Otherwise, we have a race
             # on the worker _running attribute if shutdown() is called before
             # run() actually begins.
+            workers = self._build_workers + self._upload_workers
+            if self._janitor:
+                workers += [self._janitor]
             while not all([
-                x.running for x in (self._build_workers
+                x.running for x in (workers)]):
-                                    + self._upload_workers
-                                    + [self._janitor])
-            ]):
                 time.sleep(0)
 
     def stop(self):
@@ -1138,10 +1139,10 @@ class NodePoolBuilder(object):
         '''
         with self._start_lock:
             self.log.debug("Stopping. NodePoolBuilder shutting down workers")
-            for worker in (self._build_workers
+            workers = self._build_workers + self._upload_workers
-                           + self._upload_workers
+            if self._janitor:
-                           + [self._janitor]
+                workers += [self._janitor]
-            ):
+            for worker in (workers):
                 worker.shutdown()
 
         self._running = False
@@ -1149,10 +1150,7 @@ class NodePoolBuilder(object):
         self.log.debug('Waiting for jobs to complete')
 
         # Do not exit until all of our owned threads exit.
-        for worker in (self._build_workers
+        for worker in (workers):
-                       + self._upload_workers
-                       + [self._janitor]
-        ):
             worker.join()
 
         self.log.debug('Terminating ZooKeeper connection')

nodepool/tests/__init__.py

@@ -272,15 +272,16 @@ class MySQLSchemaFixture(fixtures.Fixture):
 
 
 class BuilderFixture(fixtures.Fixture):
-    def __init__(self, configfile):
+    def __init__(self, configfile, cleanup_interval):
         super(BuilderFixture, self).__init__()
         self.configfile = configfile
+        self.cleanup_interval = cleanup_interval
         self.builder = None
 
     def setUp(self):
         super(BuilderFixture, self).setUp()
         self.builder = builder.NodePoolBuilder(self.configfile)
-        self.builder.cleanup_interval = .5
+        self.builder.cleanup_interval = self.cleanup_interval
         self.builder.build_interval = .1
         self.builder.upload_interval = .1
         self.builder.dib_cmd = 'nodepool/tests/fake-image-create'
@@ -467,8 +468,8 @@ class DBTestCase(BaseTestCase):
         self.addCleanup(app.stop)
         return app
 
-    def _useBuilder(self, configfile):
+    def _useBuilder(self, configfile, cleanup_interval=.5):
-        self.useFixture(BuilderFixture(configfile))
+        self.useFixture(BuilderFixture(configfile, cleanup_interval))
 
     def setupZK(self):
         f = ZookeeperServerFixture()

nodepool/tests/test_builder.py

@@ -15,7 +15,6 @@
 
 import os
 import fixtures
-from unittest import skip
 
 from nodepool import builder, exceptions, fakeprovider, tests
 from nodepool import zk
@@ -96,7 +95,6 @@ class TestNodePoolBuilder(tests.DBTestCase):
         nb.start()
         nb.stop()
 
-    @skip("Disabled for early v3 development")
     def test_image_upload_fail(self):
         """Test that image upload fails are handled properly."""
 
@@ -115,7 +113,9 @@ class TestNodePoolBuilder(tests.DBTestCase):
 
         configfile = self.setup_config('node.yaml')
         pool = self.useNodepool(configfile, watermark_sleep=1)
-        self._useBuilder(configfile)
+        # NOTE(pabelanger): Disable CleanupWorker thread for nodepool-builder
+        # as we currently race it to validate our failed uploads.
+        self._useBuilder(configfile, cleanup_interval=0)
         pool.start()
         self.waitForImage('fake-provider', 'fake-image')
         nodes = self.waitForNodes('fake-label')