From 936097d0d8e27420c284cccedb8da698918c5602 Mon Sep 17 00:00:00 2001 From: Paul Belanger Date: Fri, 17 Feb 2017 10:02:07 -0500 Subject: [PATCH] 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 (cherry picked from commit 78dcd29fa398fc1ec8bf23c8c74e0fdce6581169) --- nodepool/builder.py | 28 +++++++++++++--------------- nodepool/tests/__init__.py | 9 +++++---- nodepool/tests/test_builder.py | 4 +++- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/nodepool/builder.py b/nodepool/builder.py index 25ea88cb8..64b3d5486 100644 --- a/nodepool/builder.py +++ b/nodepool/builder.py @@ -1122,18 +1122,19 @@ class NodePoolBuilder(object): w.start() self._upload_workers.append(w) - self._janitor = CleanupWorker(0, self._config_path, - self.cleanup_interval, self.zk) - self._janitor.start() + if self.cleanup_interval > 0: + self._janitor = CleanupWorker( + 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 - + self._upload_workers - + [self._janitor]) - ]): + x.running for x in (workers)]): time.sleep(0) def stop(self): @@ -1146,10 +1147,10 @@ class NodePoolBuilder(object): ''' with self._start_lock: self.log.debug("Stopping. NodePoolBuilder shutting down workers") - for worker in (self._build_workers - + self._upload_workers - + [self._janitor] - ): + workers = self._build_workers + self._upload_workers + if self._janitor: + workers += [self._janitor] + for worker in (workers): worker.shutdown() self._running = False @@ -1157,10 +1158,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 - + self._upload_workers - + [self._janitor] - ): + for worker in (workers): worker.join() self.log.debug('Terminating ZooKeeper connection') diff --git a/nodepool/tests/__init__.py b/nodepool/tests/__init__.py index 122eb8fd9..8bc283212 100644 --- a/nodepool/tests/__init__.py +++ b/nodepool/tests/__init__.py @@ -370,15 +370,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' @@ -563,8 +564,8 @@ class DBTestCase(BaseTestCase): self.addCleanup(app.stop) return app - def _useBuilder(self, configfile): - self.useFixture(BuilderFixture(configfile)) + def _useBuilder(self, configfile, cleanup_interval=.5): + self.useFixture(BuilderFixture(configfile, cleanup_interval)) def setupZK(self): f = ZookeeperServerFixture() diff --git a/nodepool/tests/test_builder.py b/nodepool/tests/test_builder.py index 84e78f804..3e99573fc 100644 --- a/nodepool/tests/test_builder.py +++ b/nodepool/tests/test_builder.py @@ -112,7 +112,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') self.waitForNodes(pool)