Fix reenqueue wrong item on new patchset

When uploading a new pachset of a change with currently running jobs
zuul did not enqueue the correct patchset. The reason for this is that
when trying to abort the build it unlocks the (eventually existing)
semaphore. As the semaphore handler is located in the tenant it must
access the tenant object which it did in the layout of the old build
set. However this object can be None in some cases. The tenant also
can be accessed via the pipeline in the item which always works.

This also adds a test case for this as dynamic variant and also the
according test case for the semaphores.

Change-Id: Ic21ce36421d0805517f761aab7fbe776067960a8

tests/unit/test_scheduler.py

@@ -5061,6 +5061,46 @@ class TestSemaphore(ZuulTestCase):
         self.executor_server.release()
         self.waitUntilSettled()
 
+    def test_semaphore_new_patchset(self):
+        "Test new patchset with job semaphores"
+        self.executor_server.hold_jobs_in_build = True
+        tenant = self.sched.abide.tenants.get('tenant-one')
+        check_pipeline = tenant.layout.pipelines['check']
+
+        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
+        self.assertFalse('test-semaphore' in
+                         tenant.semaphore_handler.semaphores)
+
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        self.assertTrue('test-semaphore' in
+                        tenant.semaphore_handler.semaphores)
+        semaphore = tenant.semaphore_handler.semaphores['test-semaphore']
+        self.assertEqual(len(semaphore), 1)
+
+        A.addPatchset()
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(2))
+        self.waitUntilSettled()
+
+        self.assertTrue('test-semaphore' in
+                        tenant.semaphore_handler.semaphores)
+        semaphore = tenant.semaphore_handler.semaphores['test-semaphore']
+        self.assertEqual(len(semaphore), 1)
+
+        items = check_pipeline.getAllItems()
+        self.assertEqual(items[0].change.number, '1')
+        self.assertEqual(items[0].change.patchset, '2')
+        self.assertTrue(items[0].live)
+
+        self.executor_server.hold_jobs_in_build = False
+        self.executor_server.release()
+        self.waitUntilSettled()
+
+        # The semaphore should be released
+        self.assertFalse('test-semaphore' in
+                         tenant.semaphore_handler.semaphores)
+
     def test_semaphore_reconfigure(self):
         "Test reconfigure with job semaphores"
         self.executor_server.hold_jobs_in_build = True

tests/unit/test_v3.py

@@ -134,6 +134,71 @@ class TestInRepoConfig(ZuulTestCase):
             dict(name='project-test2', result='SUCCESS', changes='1,1'),
             dict(name='project-test2', result='SUCCESS', changes='2,1')])
 
+    def test_dynamic_config_new_patchset(self):
+        self.executor_server.hold_jobs_in_build = True
+
+        tenant = self.sched.abide.tenants.get('tenant-one')
+        check_pipeline = tenant.layout.pipelines['check']
+
+        in_repo_conf = textwrap.dedent(
+            """
+            - job:
+                name: project-test2
+
+            - project:
+                name: org/project
+                check:
+                  jobs:
+                    - project-test2
+            """)
+
+        in_repo_playbook = textwrap.dedent(
+            """
+            - hosts: all
+              tasks: []
+            """)
+
+        file_dict = {'.zuul.yaml': in_repo_conf,
+                     'playbooks/project-test2.yaml': in_repo_playbook}
+        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
+                                           files=file_dict)
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        items = check_pipeline.getAllItems()
+        self.assertEqual(items[0].change.number, '1')
+        self.assertEqual(items[0].change.patchset, '1')
+        self.assertTrue(items[0].live)
+
+        in_repo_conf = textwrap.dedent(
+            """
+            - job:
+                name: project-test2
+
+            - project:
+                name: org/project
+                check:
+                  jobs:
+                    - project-test1
+                    - project-test2
+            """)
+        file_dict = {'.zuul.yaml': in_repo_conf,
+                     'playbooks/project-test2.yaml': in_repo_playbook}
+
+        A.addPatchset(files=file_dict)
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(2))
+
+        self.waitUntilSettled()
+
+        items = check_pipeline.getAllItems()
+        self.assertEqual(items[0].change.number, '1')
+        self.assertEqual(items[0].change.patchset, '2')
+        self.assertTrue(items[0].live)
+
+        self.executor_server.hold_jobs_in_build = False
+        self.executor_server.release()
+        self.waitUntilSettled()
+
     def test_in_repo_branch(self):
         in_repo_conf = textwrap.dedent(
             """

zuul/manager/__init__.py

@@ -401,7 +401,8 @@ class PipelineManager(object):
                 self.log.exception("Exception while canceling build %s "
                                    "for change %s" % (build, item.change))
             finally:
-                old_build_set.layout.tenant.semaphore_handler.release(
+                tenant = old_build_set.item.pipeline.layout.tenant
+                tenant.semaphore_handler.release(
                     old_build_set.item, build.job)
 
             if not was_running: