Ensure correct re-enqueue on changing gate queue

When the queue for a project is changed during reconfiguration the
dependent pipeline manager was using the queue of the item
ahead in the old queue.

This lead to items ending up in the wrong queues and caused the
following exception in the run handler:

2019-11-28 13:33:47,313 zuul.Scheduler  ERROR  Exception in run handler:
Traceback (most recent call last):
  File "/tmp/zuul/zuul/scheduler.py", line 1145, in run
    while (pipeline.manager.processQueue() and
  File "/tmp/zuul/zuul/manager/__init__.py", line 914, in processQueue
    item, nnfi)
  File "/tmp/zuul/zuul/manager/__init__.py", line 898, in _processOneItem
    priority = item.getNodePriority()
  File "/tmp/zuul/zuul/model.py", line 2684, in getNodePriority
    return self.pipeline.manager.getNodePriority(self)
  File "/tmp/zuul/zuul/manager/dependent.py", line 101, in getNodePriority
    return items.index(item)
ValueError: <QueueItem 0x7f02583af4e0 for <Change 0x7f02583a9cf8 org/project2 2,1> in gate> is not in list

The issue is fixed, by ignoring the given existing queue in the
dependent pipeline manager, since it's always possible to get the
correct queue from the pipeline itself.

Change-Id: Ia5b1b58377e4420b9ab1440c0b9f67cb15967263

tests/fixtures/layouts/live-reconfiguration-shared-queue-removed.yaml

@@ -0,0 +1,93 @@
+- pipeline:
+    name: check
+    manager: independent
+    trigger:
+      gerrit:
+        - event: patchset-created
+    success:
+      gerrit:
+        Verified: 1
+    failure:
+      gerrit:
+        Verified: -1
+
+- pipeline:
+    name: gate
+    manager: dependent
+    failure-message: Build failed.  For information on how to proceed, see http://wiki.example.org/Test_Failures
+    trigger:
+      gerrit:
+        - event: comment-added
+          approval:
+            - Approved: 1
+    success:
+      gerrit:
+        Verified: 2
+        submit: true
+    failure:
+      gerrit:
+        Verified: -2
+    start:
+      gerrit:
+        Verified: 0
+    precedence: high
+
+- job:
+    name: base
+    parent: null
+    run: playbooks/base.yaml
+
+- job:
+    name: project-merge
+    hold-following-changes: true
+    run: playbooks/project-merge.yaml
+
+- job:
+    name: project-test1
+    run: playbooks/project-test1.yaml
+
+- job:
+    name: project-test2
+    run: playbooks/project-test2.yaml
+
+- job:
+    name: project1-project2-integration
+    run: playbooks/project1-project2-integration.yaml
+
+- project:
+    name: org/project1
+    check:
+      jobs:
+        - project-merge
+        - project-test1:
+            dependencies: project-merge
+        - project-test2:
+            dependencies: project-merge
+    gate:
+      jobs:
+        - project-merge
+        - project-test1:
+            dependencies: project-merge
+        - project-test2:
+            dependencies: project-merge
+
+- project:
+    name: org/project2
+    check:
+      jobs:
+        - project-merge
+        - project-test1:
+            dependencies: project-merge
+        - project-test2:
+            dependencies: project-merge
+        - project1-project2-integration:
+            dependencies: project-merge
+    gate:
+      jobs:
+        - project-merge
+        - project-test1:
+            dependencies: project-merge
+        - project-test2:
+            dependencies: project-merge
+        - project1-project2-integration:
+            dependencies: project-merge

tests/unit/test_scheduler.py

@@ -3486,6 +3486,35 @@ class TestScheduler(ZuulTestCase):
         # Ensure the removed job was not included in the report.
         self.assertNotIn('project1-project2-integration', A.messages[0])
 
+    def test_live_reconfiguration_shared_queue_removed(self):
+        # Test that changes in a shared queue survive a change of the
+        # queue during reconfiguration. This is a regression test
+        # for the dependent pipeline manager.
+        self.executor_server.hold_jobs_in_build = True
+
+        A = self.fake_gerrit.addFakeChange('org/project1', 'master', 'A')
+        B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B')
+        A.addApproval('Code-Review', 2)
+        B.addApproval('Code-Review', 2)
+
+        self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
+        self.fake_gerrit.addEvent(B.addApproval('Approved', 1))
+        self.waitUntilSettled()
+
+        # Remove the integration job.
+        self.commitConfigUpdate(
+            'common-config',
+            'layouts/live-reconfiguration-shared-queue-removed.yaml')
+        self.sched.reconfigure(self.config)
+        self.waitUntilSettled()
+
+        self.executor_server.hold_jobs_in_build = False
+        self.executor_server.release()
+        self.waitUntilSettled()
+
+        self.assertEqual(A.data['status'], 'MERGED')
+        self.assertEqual(B.data['status'], 'MERGED')
+
     def test_double_live_reconfiguration_shared_queue(self):
         # This was a real-world regression.  A change is added to
         # gate; a reconfigure happens, a second change which depends

zuul/manager/dependent.py

@@ -78,8 +78,9 @@ class DependentPipelineManager(PipelineManager):
     def getChangeQueue(self, change, event, existing=None):
         log = get_annotated_logger(self.log, event)
 
-        if existing:
-            return StaticChangeQueueContextManager(existing)
+        # Ignore the existing queue, since we can always get the correct queue
+        # from the pipeline. This avoids enqueuing changes in a wrong queue
+        # e.g. during re-configuration.
         queue = self.pipeline.getQueue(change.project)
         if queue:
             return StaticChangeQueueContextManager(queue)