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
diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py
index e402342..e9eee54 100755
--- a/tests/unit/test_scheduler.py
+++ b/tests/unit/test_scheduler.py
@@ -5061,6 +5061,46 @@
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
diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py
index 7d84b1f..e60fc5f 100644
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -134,6 +134,71 @@
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(
"""
diff --git a/zuul/manager/__init__.py b/zuul/manager/__init__.py
index 20bc459..01429ce 100644
--- a/zuul/manager/__init__.py
+++ b/zuul/manager/__init__.py
@@ -401,7 +401,8 @@
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: