Merge "Maintain the trigger cache after reconfiguring"

3 files changed, 38 insertions, 8 deletions

diff --git a/tests/test_zuultrigger.py b/tests/test_zuultrigger.py
index 9e9bc61d2..9a90a982e 100644
--- a/tests/test_zuultrigger.py
+++ b/tests/test_zuultrigger.py
@@ -84,11 +84,17 @@ class TestZuulTrigger(ZuulTestCase):
         # A, B, C;  B conflicts with A, but C does not.
         # When A is merged, B and C should be checked for conflicts,
         # and B should receive a -1.
+        # D and E are used to repeat the test in the second part, but
+        # are defined here to that they end up in the trigger cache.
         A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
         B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B')
         C = self.fake_gerrit.addFakeChange('org/project', 'master', 'C')
+        D = self.fake_gerrit.addFakeChange('org/project', 'master', 'D')
+        E = self.fake_gerrit.addFakeChange('org/project', 'master', 'E')
         A.addPatchset(['conflict'])
         B.addPatchset(['conflict'])
+        D.addPatchset(['conflict2'])
+        E.addPatchset(['conflict2'])
         A.addApproval('CRVW', 2)
         self.fake_gerrit.addEvent(A.addApproval('APRV', 1))
         self.waitUntilSettled()
@@ -98,8 +104,33 @@ class TestZuulTrigger(ZuulTestCase):
         self.assertEqual(A.reported, 2)
         self.assertEqual(B.reported, 1)
         self.assertEqual(C.reported, 0)
+        self.assertEqual(D.reported, 0)
+        self.assertEqual(E.reported, 0)
         self.assertEqual(B.messages[0],
             "Merge Failed.\n\nThis change was unable to be automatically "
             "merged with the current state of the repository. Please rebase "
             "your change and upload a new patchset.")
         self.assertEqual(self.fake_gerrit.queries[0], "project:org/project status:open")
+
+        # Reconfigure and run the test again.  This is a regression
+        # check to make sure that we don't end up with a stale trigger
+        # cache that has references to projects from the old
+        # configuration.
+        self.sched.reconfigure(self.config)
+
+        D.addApproval('CRVW', 2)
+        self.fake_gerrit.addEvent(D.addApproval('APRV', 1))
+        self.waitUntilSettled()
+
+        self.assertEqual(len(self.history), 2)
+        self.assertEqual(self.history[1].name, 'project-gate')
+        self.assertEqual(A.reported, 2)
+        self.assertEqual(B.reported, 1)
+        self.assertEqual(C.reported, 0)
+        self.assertEqual(D.reported, 2)
+        self.assertEqual(E.reported, 1)
+        self.assertEqual(E.messages[0],
+            "Merge Failed.\n\nThis change was unable to be automatically "
+            "merged with the current state of the repository. Please rebase "
+            "your change and upload a new patchset.")
+        self.assertEqual(self.fake_gerrit.queries[1], "project:org/project status:open")
diff --git a/zuul/scheduler.py b/zuul/scheduler.py
index a2e07cd0f..d6c51e27d 100644
--- a/zuul/scheduler.py
+++ b/zuul/scheduler.py
@@ -183,7 +183,6 @@ class Scheduler(threading.Thread):
         self.triggers = dict()
         self.reporters = dict()
         self.config = None
-        self._maintain_trigger_cache = False
 
         self.trigger_event_queue = Queue.Queue()
         self.result_event_queue = Queue.Queue()
@@ -667,6 +666,7 @@ class Scheduler(threading.Thread):
                             "Exception while canceling build %s "
                             "for change %s" % (build, item.change))
             self.layout = layout
+            self.maintainTriggerCache()
             for trigger in self.triggers.values():
                 trigger.postConfig()
             if statsd:
@@ -784,10 +784,6 @@ class Scheduler(threading.Thread):
                     while pipeline.manager.processQueue():
                         pass
 
-                if self._maintain_trigger_cache:
-                    self.maintainTriggerCache()
-                    self._maintain_trigger_cache = False
-
             except Exception:
                 self.log.exception("Exception in run handler:")
                 # There may still be more events to process
@@ -1171,7 +1167,6 @@ class BasePipelineManager(object):
         self.log.debug("Removing change %s from queue" % item.change)
         change_queue = self.pipeline.getQueue(item.change.project)
         change_queue.dequeueItem(item)
-        self.sched._maintain_trigger_cache = True
 
     def removeChange(self, change):
         # Remove a change from the queue, probably because it has been
diff --git a/zuul/trigger/gerrit.py b/zuul/trigger/gerrit.py
index 4d4deb8b6..69664886e 100644
--- a/zuul/trigger/gerrit.py
+++ b/zuul/trigger/gerrit.py
@@ -280,8 +280,12 @@ class Gerrit(object):
         # This lets the user supply a list of change objects that are
         # still in use.  Anything in our cache that isn't in the supplied
         # list should be safe to remove from the cache.
-        # TODO(jeblair): consider removing this feature
-        return
+        remove = []
+        for key, change in self._change_cache.items():
+            if change not in relevant:
+                remove.append(key)
+        for key in remove:
+            del self._change_cache[key]
 
     def postConfig(self):
         pass