Verify nodes and requests are not leaked

Check that at the end of every test, there are no outstanding
nodepool requests and no locked nodes.

Move final state assertions into the tearDown method so that
they run right after the end of the test but before any
cleanup handlers are called (which can interfere with the
assertion checking by, say, deleting the zookeeper tree we
are trying to check).  Move the cleanup in test_webapp to
tearDown so that it ends the paused job that the tests in
that class use before the assertion check.

Fix some bugs uncovered by this testing:

* Two typos.
* When we re-launch a job, we need a new nodeset, so make sure
  to remove the nodeset from the buildset after the build
  completes if we are going to retry the build.
* Always report build results to the scheduler even for non-current
  buildsets so that it can return used nodes for aborted builds.
* Have the scheduler return the nodeset for a completed build rather
  than the pipeline manager to avoid the edge case where a build
  result is returned after a configuration that removes the pipeline
  (and therefore, there is no longer a manager to return the nodeset).
* When canceling jobs, return nodesets for any jobs which do not yet
  have builds (such as jobs which have nodes but have not yet
  launched).
* Return nodes for skipped jobs.

Normalize the debug messages in nodepool.py.

Change-Id: I32f6807ac95034fc2636993824f4a45ffe7c59d8
diff --git a/tests/base.py b/tests/base.py
index f10157d..56c83f2 100755
--- a/tests/base.py
+++ b/tests/base.py
@@ -874,6 +874,7 @@
 
 class FakeNodepool(object):
     REQUEST_ROOT = '/nodepool/requests'
+    NODE_ROOT = '/nodepool/nodes'
 
     log = logging.getLogger("zuul.test.FakeNodepool")
 
@@ -918,6 +919,28 @@
             reqs.append(data)
         return reqs
 
+    def getNodes(self):
+        try:
+            nodeids = self.client.get_children(self.NODE_ROOT)
+        except kazoo.exceptions.NoNodeError:
+            return []
+        nodes = []
+        for oid in sorted(nodeids):
+            path = self.NODE_ROOT + '/' + oid
+            data, stat = self.client.get(path)
+            data = json.loads(data)
+            data['_oid'] = oid
+            try:
+                lockfiles = self.client.get_children(path + '/lock')
+            except kazoo.exceptions.NoNodeError:
+                lockfiles = []
+            if lockfiles:
+                data['_lock'] = True
+            else:
+                data['_lock'] = False
+            nodes.append(data)
+        return nodes
+
     def makeNode(self, request_id, node_type):
         now = time.time()
         path = '/nodepool/nodes/'
@@ -1257,9 +1280,12 @@
         self.rpc.start()
         self.launch_client.gearman.waitForServer()
 
-        self.addCleanup(self.assertFinalState)
         self.addCleanup(self.shutdown)
 
+    def tearDown(self):
+        super(ZuulTestCase, self).tearDown()
+        self.assertFinalState()
+
     def configure_connections(self):
         # Register connections from the config
         self.smtp_messages = []
@@ -1366,6 +1392,17 @@
         self.addCommitToRepo(project, 'add content from fixture',
                              files, branch='master', tag='init')
 
+    def assertNodepoolState(self):
+        # Make sure that there are no pending requests
+
+        requests = self.fake_nodepool.getNodeRequests()
+        self.assertEqual(len(requests), 0)
+
+        nodes = self.fake_nodepool.getNodes()
+        for node in nodes:
+            self.assertFalse(node['_lock'], "Node %s is locked" %
+                             (node['_oid'],))
+
     def assertFinalState(self):
         # Make sure that git.Repo objects have been garbage collected.
         repos = []
@@ -1375,6 +1412,7 @@
                 repos.append(obj)
         self.assertEqual(len(repos), 0)
         self.assertEmptyQueues()
+        self.assertNodepoolState()
         ipm = zuul.manager.independent.IndependentPipelineManager
         for tenant in self.sched.abide.tenants.values():
             for pipeline in tenant.layout.pipelines.values():
diff --git a/tests/test_webapp.py b/tests/test_webapp.py
index e191244..8268ef7 100644
--- a/tests/test_webapp.py
+++ b/tests/test_webapp.py
@@ -26,14 +26,8 @@
 class TestWebapp(ZuulTestCase):
     tenant_config_file = 'config/single-tenant/main.yaml'
 
-    def _cleanup(self):
-        self.launch_server.hold_jobs_in_build = False
-        self.launch_server.release()
-        self.waitUntilSettled()
-
     def setUp(self):
         super(TestWebapp, self).setUp()
-        self.addCleanup(self._cleanup)
         self.launch_server.hold_jobs_in_build = True
         A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
         A.addApproval('code-review', 2)
@@ -44,6 +38,12 @@
         self.waitUntilSettled()
         self.port = self.webapp.server.socket.getsockname()[1]
 
+    def tearDown(self):
+        self.launch_server.hold_jobs_in_build = False
+        self.launch_server.release()
+        self.waitUntilSettled()
+        super(TestWebapp, self).tearDown()
+
     def test_webapp_status(self):
         "Test that we can filter to only certain changes in the webapp."