Builders can be run on their own

Add entrypoint so builders can be run on their own using the
nodepool-builder command. Also update config processing so builders do
not require things like a zmq-publisher.

Change-Id: Ied1142990ca630f22044a472db77920daa4c2e5a

devstack/plugin.sh

@@ -263,7 +263,8 @@ function start_nodepool {
     # start gearman server
     run_process geard "geard -p 8991 -d"
 
-    run_process nodepool "nodepoold -c $NODEPOOL_CONFIG -s $NODEPOOL_SECURE -l $NODEPOOL_LOGGING -d"
+    run_process nodepool "nodepoold --no-builder -c $NODEPOOL_CONFIG -s $NODEPOOL_SECURE -l $NODEPOOL_LOGGING -d"
+    run_process nodepool-builder "nodepool-builder -c $NODEPOOL_CONFIG -d"
     :
 }
 

devstack/settings

@@ -12,3 +12,4 @@ SHADE_REPO_REF=${SHADE_REPO_REF:-master}
 
 enable_service geard
 enable_service nodepool
+enable_service nodepool-builder

nodepool/builder.py

@@ -80,6 +80,7 @@ class NodePoolBuilder(object):
     def __init__(self, config_path):
         self._config_path = config_path
         self._running = False
+        self._stop_running = True
         self._built_image_ids = set()
         self._start_lock = threading.Lock()
         self._gearman_worker = None
@@ -89,21 +90,35 @@ class NodePoolBuilder(object):
     def running(self):
         return self._running
 
-    def start(self):
+    def runForever(self):
-        self.load_config(self._config_path)
-
         with self._start_lock:
             if self._running:
                 raise exceptions.BuilderError('Cannot start, already running.')
-            self._running = True
+
+            self.load_config(self._config_path)
+            self._validate_config()
 
             self._gearman_worker = self._initializeGearmanWorker(
                 self._config.gearman_servers.values())
+
             self._registerGearmanFunctions(self._config.diskimages.values())
             self._registerExistingImageUploads()
-            self.thread = threading.Thread(target=self._run,
+
-                                           name='NodePool Builder')
+            self._stop_running = False
-            self.thread.start()
+            self._running = True
+
+        self.log.debug('Starting listener for build jobs')
+
+        while not self._stop_running:
+            try:
+                job = self._gearman_worker.getJob()
+                self._handleJob(job)
+            except gear.InterruptedError:
+                pass
+            except Exception:
+                self.log.exception('Exception while getting job')
+
+        self._running = False
 
     def stop(self):
         with self._start_lock:
@@ -112,13 +127,13 @@ class NodePoolBuilder(object):
                 self.log.warning("Stop called when we are already stopped.")
                 return
 
-            self._running = False
+            self._stop_running = True
 
             self._gearman_worker.stopWaitingForJobs()
 
-            self.log.debug('Waiting for builder thread to complete.')
+            # Wait for the builder to complete any currently running jobs
-            self.thread.join()
+            while self._running:
-            self.log.debug('Builder thread completed.')
+                time.sleep(1)
 
             try:
                 self._gearman_worker.shutdown()
@@ -136,16 +151,12 @@ class NodePoolBuilder(object):
             self._config, config)
         self._config = config
 
-    def _run(self):
+    def _validate_config(self):
-        self.log.debug('Starting listener for build jobs')
+        if not self._config.gearman_servers.values():
-        while self._running:
+            raise RuntimeError('No gearman servers specified in config.')
-            try:
+
-                job = self._gearman_worker.getJob()
+        if not self._config.imagesdir:
-                self._handleJob(job)
+            raise RuntimeError('No images-dir specified in config.')
-            except gear.InterruptedError:
-                pass
-            except Exception:
-                self.log.exception('Exception while getting job')
 
     def _initializeGearmanWorker(self, servers):
         worker = gear.Worker('Nodepool Builder')

nodepool/cmd/builder.py

@@ -0,0 +1,57 @@
+#!/usr/bin/env python
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import argparse
+import extras
+import signal
+import sys
+
+import daemon
+
+from nodepool import builder
+
+
+# as of python-daemon 1.6 it doesn't bundle pidlockfile anymore
+# instead it depends on lockfile-0.9.1 which uses pidfile.
+pid_file_module = extras.try_imports(['daemon.pidlockfile', 'daemon.pidfile'])
+
+def main():
+    parser = argparse.ArgumentParser(description='NodePool Image Builder.')
+    parser.add_argument('-c', dest='config',
+                        default='/etc/nodepool/nodepool.yaml',
+                        help='path to config file')
+    parser.add_argument('-p', dest='pidfile',
+                        help='path to pid file',
+                        default='/var/run/nodepool/nodepool.pid')
+    parser.add_argument('-d', dest='nodaemon', action='store_true',
+                        help='do not run as a daemon')
+    args = parser.parse_args()
+
+    nb = builder.NodePoolBuilder(args.config)
+
+    def sigint_handler(signal, frame):
+        nb.stop()
+
+    if args.nodaemon:
+        signal.signal(signal.SIGINT, sigint_handler)
+        nb.runForever()
+    else:
+        pid = pid_file_module.TimeoutPIDLockFile(args.pidfile, 10)
+        with daemon.DaemonContext(pidfile=pid):
+            signal.signal(signal.SIGINT, sigint_handler)
+            nb.runForever()
+
+
+if __name__ == "__main__":
+    sys.exit(main())

nodepool/cmd/nodepoolcmd.py

@@ -246,7 +246,6 @@ class NodePoolCmd(object):
             raise Exception("Trying to build a non disk-image-builder "
                             "image: %s" % diskimage)
 
-        self.pool.reconfigureImageBuilder()
         self.pool.buildImage(self.pool.config.diskimages[diskimage])
         self.pool.waitForBuiltImages()
 
@@ -379,8 +378,7 @@ class NodePoolCmd(object):
         if self.args.command in ('config-validate'):
             return self.args.func()
 
-        self.pool = nodepool.NodePool(self.args.secure, self.args.config,
+        self.pool = nodepool.NodePool(self.args.secure, self.args.config)
-                                      run_builder=False)
         config = self.pool.loadConfig()
         self.pool.reconfigureGearmanClient(config)
         self.pool.reconfigureDatabase(config)

nodepool/cmd/nodepoold.py

@@ -30,6 +30,8 @@ import signal
 import traceback
 import threading
 
+from nodepool import builder
+
 # No nodepool imports here because they pull in paramiko which must not be
 # imported until after the daemonization.
 # https://github.com/paramiko/paramiko/issues/59
@@ -96,6 +98,8 @@ class NodePoolDaemon(object):
         parser.add_argument('-p', dest='pidfile',
                             help='path to pid file',
                             default='/var/run/nodepool/nodepool.pid')
+        parser.add_argument('--no-builder', dest='builder',
+                            action='store_false')
         parser.add_argument('--version', dest='version', action='store_true',
                             help='show version')
         self.args = parser.parse_args()
@@ -114,6 +118,8 @@ class NodePoolDaemon(object):
 
     def exit_handler(self, signum, frame):
         self.pool.stop()
+        if self.args.builder:
+            self.builder.stop()
 
     def term_handler(self, signum, frame):
         os._exit(0)
@@ -123,6 +129,8 @@ class NodePoolDaemon(object):
         self.setup_logging()
         self.pool = nodepool.nodepool.NodePool(self.args.secure,
                                                self.args.config)
+        if self.args.builder:
+            self.builder = builder.NodePoolBuilder(self.args.config)
 
         signal.signal(signal.SIGINT, self.exit_handler)
         # For back compatibility:
@@ -132,6 +140,9 @@ class NodePoolDaemon(object):
         signal.signal(signal.SIGTERM, self.term_handler)
 
         self.pool.start()
+        if self.args.builder:
+            nb_thread = threading.Thread(target=self.builder.runForever)
+            nb_thread.start()
 
         while True:
             signal.pause()

nodepool/config.py

@@ -122,7 +122,7 @@ def loadConfig(config_path):
         c.job = None
         c.timespec = config.get('cron', {}).get(name, default)
 
-    for addr in config['zmq-publishers']:
+    for addr in config.get('zmq-publishers', []):
         z = ZMQPublisher()
         z.name = addr
         z.listener = None
@@ -135,7 +135,7 @@ def loadConfig(config_path):
         g.name = g.host + '_' + str(g.port)
         newconfig.gearman_servers[g.name] = g
 
-    for provider in config['providers']:
+    for provider in config.get('providers', []):
         p = Provider()
         p.name = provider['name']
         newconfig.providers[p.name] = p
@@ -232,7 +232,7 @@ def loadConfig(config_path):
                     diskimage = newconfig.diskimages[image.diskimage]
                     diskimage.image_types.add(provider.image_type)
 
-    for label in config['labels']:
+    for label in config.get('labels', []):
         l = Label()
         l.name = label['name']
         newconfig.labels[l.name] = l
@@ -246,7 +246,7 @@ def loadConfig(config_path):
             p.name = provider['name']
             l.providers[p.name] = p
 
-    for target in config['targets']:
+    for target in config.get('targets', []):
         t = Target()
         t.name = target['name']
         newconfig.targets[t.name] = t

nodepool/nodepool.py

@@ -33,7 +33,6 @@ import zmq
 import allocation
 import jenkins_manager
 import nodedb
-import builder
 import nodeutils as utils
 import provider_manager
 from stats import statsd
@@ -1063,13 +1062,12 @@ class SnapshotImageUpdater(ImageUpdater):
 class NodePool(threading.Thread):
     log = logging.getLogger("nodepool.NodePool")
 
-    def __init__(self, securefile, configfile, watermark_sleep=WATERMARK_SLEEP,
+    def __init__(self, securefile, configfile,
-                 run_builder=True):
+                 watermark_sleep=WATERMARK_SLEEP):
         threading.Thread.__init__(self, name='NodePool')
         self.securefile = securefile
         self.configfile = configfile
         self.watermark_sleep = watermark_sleep
-        self.run_builder = run_builder
         self._stopped = False
         self.config = None
         self.zmq_context = None
@@ -1081,7 +1079,6 @@ class NodePool(threading.Thread):
         self._image_delete_threads_lock = threading.Lock()
         self._instance_delete_threads = {}
         self._instance_delete_threads_lock = threading.Lock()
-        self._image_builder_thread = None
         self._image_build_jobs = JobTracker()
         self._image_upload_jobs = JobTracker()
 
@@ -1095,8 +1092,6 @@ class NodePool(threading.Thread):
             self.zmq_context.destroy()
         if self.apsched:
             self.apsched.shutdown()
-        if self._image_builder_thread and self._image_builder_thread.running:
-            self._image_builder_thread.stop()
 
     def waitForBuiltImages(self):
         self.log.debug("Waiting for images to complete building.")
@@ -1235,13 +1230,6 @@ class NodePool(threading.Thread):
                 self.gearman_client.addServer(g.host, g.port)
             self.gearman_client.waitForServer()
 
-    def reconfigureImageBuilder(self):
-        # start disk image builder thread
-        if not self._image_builder_thread and self.run_builder:
-            self._image_builder_thread = builder.NodePoolBuilder(
-                self.configfile)
-            self._image_builder_thread.start()
-
     def setConfig(self, config):
         self.config = config
 
@@ -1434,7 +1422,6 @@ class NodePool(threading.Thread):
         self.reconfigureGearmanClient(config)
         self.reconfigureCrons(config)
         self.setConfig(config)
-        self.reconfigureImageBuilder()
 
     def startup(self):
         self.updateConfig()

nodepool/tests/__init__.py

@@ -275,16 +275,18 @@ class MySQLSchemaFixture(fixtures.Fixture):
 
 
 class BuilderFixture(fixtures.Fixture):
-    def __init__(self, nodepool):
+    def __init__(self, configfile):
         super(BuilderFixture, self).__init__()
-        self.nodepool = nodepool
+        self.configfile = configfile
         self.builder = None
 
     def setUp(self):
         super(BuilderFixture, self).setUp()
-        self.builder = builder.NodePoolBuilder(self.nodepool.configfile)
+        self.builder = builder.NodePoolBuilder(self.configfile)
+        nb_thread = threading.Thread(target=self.builder.runForever)
+        nb_thread.daemon = True
         self.addCleanup(self.cleanup)
-        self.builder.start()
+        nb_thread.start()
 
     def cleanup(self):
         self.builder.stop()
@@ -375,6 +377,9 @@ class DBTestCase(BaseTestCase):
         self.addCleanup(pool.stop)
         return pool
 
+    def _useBuilder(self, configfile):
+        self.useFixture(BuilderFixture(configfile))
+
 
 class IntegrationTestCase(DBTestCase):
     def setUpFakes(self):

nodepool/tests/test_builder.py

@@ -14,6 +14,8 @@
 # limitations under the License.
 
 import os
+import time
+import threading
 
 import fixtures
 
@@ -83,3 +85,18 @@ class TestNodepoolBuilderDibImage(tests.BaseTestCase):
 
         image = builder.DibImageFile('myid1234')
         self.assertRaises(exceptions.BuilderError, image.to_path, '/imagedir/')
+
+class TestNodepoolBuilder(tests.DBTestCase):
+    def test_start_stop(self):
+        config = self.setup_config('node_dib.yaml')
+        nb = builder.NodePoolBuilder(config)
+        nb_thread = threading.Thread(target=nb.runForever)
+        nb_thread.daemon = True
+
+        nb_thread.start()
+        while not nb.running:
+            time.sleep(.5)
+
+        nb.stop()
+        while nb_thread.isAlive():
+            time.sleep(.5)

nodepool/tests/test_commands.py

@@ -20,23 +20,10 @@ import fixtures
 import mock
 
 from nodepool.cmd import nodepoolcmd
-from nodepool import nodepool, tests
+from nodepool import tests
 
 
 class TestNodepoolCMD(tests.DBTestCase):
-    def _useBuilder(self, configfile):
-        # XXX:greghaynes This is a gross hack for while the builder depends on
-        # nodepool because we have a circular dep. Remove this when builder
-        # gets its own config.
-        pool = nodepool.NodePool(self._setup_secure(), configfile,
-                                 run_builder=False)
-        config = pool.loadConfig()
-        pool.reconfigureDatabase(config)
-        pool.reconfigureManagers(config)
-        pool.setConfig(config)
-        self.builder = tests.BuilderFixture(pool)
-        self.useFixture(self.builder)
-
     def patch_argv(self, *args):
         argv = ["nodepool", "-s", self.secure_conf]
         argv.extend(args)
@@ -162,6 +149,7 @@ class TestNodepoolCMD(tests.DBTestCase):
     def test_dib_image_list(self):
         configfile = self.setup_config('node_dib.yaml')
         pool = self.useNodepool(configfile, watermark_sleep=1)
+        self._useBuilder(configfile)
         pool.start()
         self.waitForImage(pool, 'fake-dib-provider', 'fake-dib-image')
         self.waitForNodes(pool)
@@ -170,6 +158,7 @@ class TestNodepoolCMD(tests.DBTestCase):
     def test_dib_image_delete(self):
         configfile = self.setup_config('node_dib.yaml')
         pool = self.useNodepool(configfile, watermark_sleep=1)
+        self._useBuilder(configfile)
         pool.start()
         self.waitForImage(pool, 'fake-dib-provider', 'fake-dib-image')
         self.waitForNodes(pool)
@@ -185,6 +174,7 @@ class TestNodepoolCMD(tests.DBTestCase):
     def test_image_upload(self):
         configfile = self.setup_config('node_dib.yaml')
         pool = self.useNodepool(configfile, watermark_sleep=1)
+        self._useBuilder(configfile)
         pool.start()
         self.waitForImage(pool, 'fake-dib-provider', 'fake-dib-image')
         self.waitForNodes(pool)
@@ -201,6 +191,7 @@ class TestNodepoolCMD(tests.DBTestCase):
     def test_image_upload_all(self):
         configfile = self.setup_config('node_dib.yaml')
         pool = self.useNodepool(configfile, watermark_sleep=1)
+        self._useBuilder(configfile)
         pool.start()
         self.waitForImage(pool, 'fake-dib-provider', 'fake-dib-image')
         self.waitForNodes(pool)

nodepool/tests/test_nodepool.py

@@ -71,6 +71,7 @@ class TestNodepool(tests.DBTestCase):
         """Test that a dib image and node are created"""
         configfile = self.setup_config('node_dib.yaml')
         pool = self.useNodepool(configfile, watermark_sleep=1)
+        self._useBuilder(configfile)
         pool.start()
         self.waitForImage(pool, 'fake-dib-provider', 'fake-dib-image')
         self.waitForNodes(pool)
@@ -86,6 +87,7 @@ class TestNodepool(tests.DBTestCase):
         """Test that a dib image and node are created vhd image"""
         configfile = self.setup_config('node_dib_vhd.yaml')
         pool = self.useNodepool(configfile, watermark_sleep=1)
+        self._useBuilder(configfile)
         pool.start()
         self.waitForImage(pool, 'fake-dib-provider', 'fake-dib-image')
         self.waitForNodes(pool)
@@ -101,6 +103,7 @@ class TestNodepool(tests.DBTestCase):
         """Test label provided by vhd and qcow2 images builds"""
         configfile = self.setup_config('node_dib_vhd_and_qcow2.yaml')
         pool = self.useNodepool(configfile, watermark_sleep=1)
+        self._useBuilder(configfile)
         pool.start()
         self.waitForImage(pool, 'fake-provider1', 'fake-dib-image')
         self.waitForImage(pool, 'fake-provider2', 'fake-dib-image')
@@ -122,6 +125,7 @@ class TestNodepool(tests.DBTestCase):
         """Test that a label with dib and snapshot images build."""
         configfile = self.setup_config('node_dib_and_snap.yaml')
         pool = self.useNodepool(configfile, watermark_sleep=1)
+        self._useBuilder(configfile)
         pool.start()
         self.waitForImage(pool, 'fake-provider1', 'fake-dib-image')
         self.waitForImage(pool, 'fake-provider2', 'fake-dib-image')
@@ -143,6 +147,7 @@ class TestNodepool(tests.DBTestCase):
         """Test that snap based nodes build when dib fails."""
         configfile = self.setup_config('node_dib_and_snap_fail.yaml')
         pool = self.useNodepool(configfile, watermark_sleep=1)
+        self._useBuilder(configfile)
         pool.start()
         self.addCleanup(pool.stop)
         # fake-provider1 will fail to build fake-dib-image
@@ -171,6 +176,7 @@ class TestNodepool(tests.DBTestCase):
         """Test that a dib and snap image upload failure is contained."""
         configfile = self.setup_config('node_dib_and_snap_upload_fail.yaml')
         pool = self.useNodepool(configfile, watermark_sleep=1)
+        self._useBuilder(configfile)
         pool.start()
         self.waitForImage(pool, 'fake-provider2', 'fake-dib-image')
         self.waitForNodes(pool)
@@ -193,6 +199,7 @@ class TestNodepool(tests.DBTestCase):
         """Test that a dib image (snapshot) can be deleted."""
         configfile = self.setup_config('node_dib.yaml')
         pool = self.useNodepool(configfile, watermark_sleep=1)
+        self._useBuilder(configfile)
         pool.start()
         self.waitForImage(pool, 'fake-dib-provider', 'fake-dib-image')
         self.waitForNodes(pool)
@@ -217,6 +224,7 @@ class TestNodepool(tests.DBTestCase):
         """Test that a dib image (snapshot) can be deleted."""
         configfile = self.setup_config('node_dib.yaml')
         pool = self.useNodepool(configfile, watermark_sleep=1)
+        self._useBuilder(configfile)
         pool.start()
         self.waitForImage(pool, 'fake-dib-provider', 'fake-dib-image')
         self.waitForNodes(pool)

setup.cfg

@@ -22,6 +22,7 @@ warnerrors = True
 [entry_points]
 console_scripts =
     nodepool = nodepool.cmd.nodepoolcmd:main
+    nodepool-builder = nodepool.cmd.builder:main
     nodepoold = nodepool.cmd.nodepoold:main
 
 [build_sphinx]