Fix multiple jobs with provides/requires

To avoid querying the database repeatedly, we cache the results
of the query for completed jobs which provide artifacts needed
for a running job.  We only cached one such result per queue item,
so if a queue item had two jobs with differing requirements, then
we would only use the results associated with the first.

This change updates the cache to be a dictionary keyed by the
requirements used in the search.

Change-Id: Ic707013777303e696cf76120308724ec501979b2

tests/fixtures/layouts/provides-requires.yaml

@@ -49,11 +49,20 @@
     name: image-user
     requires: images
 
+- job:
+    name: library-builder
+    provides: libraries
+
+- job:
+    name: library-user
+    requires: libraries
+
 - project:
     name: org/project1
     check:
       jobs:
         - image-builder
+        - library-builder
     gate:
       queue: integrated
       jobs:
@@ -64,6 +73,7 @@
     check:
       jobs:
         - image-user
+        - library-user
     gate:
       queue: integrated
       jobs:

tests/unit/test_v3.py

@@ -4966,10 +4966,17 @@ class TestProvidesRequires(ZuulDBTestCase):
                  {'name': 'image', 'url': 'http://example.com/image'},
              ]}}
         )
+        self.executor_server.returnData(
+            'library-builder', A,
+            {'zuul':
+             {'artifacts': [
+                 {'name': 'library', 'url': 'http://example.com/library'},
+             ]}}
+        )
         self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
         self.waitUntilSettled()
 
-        self.assertEqual(len(self.builds), 1)
+        self.assertEqual(len(self.builds), 2)
 
         B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B')
         B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
@@ -4977,7 +4984,7 @@ class TestProvidesRequires(ZuulDBTestCase):
         self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
         self.waitUntilSettled()
 
-        self.assertEqual(len(self.builds), 1)
+        self.assertEqual(len(self.builds), 2)
 
         self.executor_server.hold_jobs_in_build = False
         self.executor_server.release()
@@ -4985,10 +4992,13 @@ class TestProvidesRequires(ZuulDBTestCase):
 
         self.assertHistory([
             dict(name='image-builder', result='SUCCESS', changes='1,1'),
+            dict(name='library-builder', result='SUCCESS', changes='1,1'),
             dict(name='image-user', result='SUCCESS', changes='1,1 2,1'),
-        ])
+            dict(name='library-user', result='SUCCESS', changes='1,1 2,1'),
+        ], ordered=False)
+        image_user = self.getJobFromHistory('image-user')
         self.assertEqual(
-            self.history[-1].parameters['zuul']['artifacts'],
+            image_user.parameters['zuul']['artifacts'],
             [{
                 'project': 'org/project1',
                 'change': '1',
@@ -4997,6 +5007,17 @@ class TestProvidesRequires(ZuulDBTestCase):
                 'url': 'http://example.com/image',
                 'name': 'image',
             }])
+        library_user = self.getJobFromHistory('library-user')
+        self.assertEqual(
+            library_user.parameters['zuul']['artifacts'],
+            [{
+                'project': 'org/project1',
+                'change': '1',
+                'patchset': '1',
+                'job': 'library-builder',
+                'url': 'http://example.com/library',
+                'name': 'library',
+            }])
 
     @simple_layout('layouts/provides-requires.yaml')
     def test_provides_requires_check_old_success(self):
@@ -5008,11 +5029,19 @@ class TestProvidesRequires(ZuulDBTestCase):
                  {'name': 'image', 'url': 'http://example.com/image'},
              ]}}
         )
+        self.executor_server.returnData(
+            'library-builder', A,
+            {'zuul':
+             {'artifacts': [
+                 {'name': 'library', 'url': 'http://example.com/library'},
+             ]}}
+        )
         self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
         self.waitUntilSettled()
         self.assertHistory([
             dict(name='image-builder', result='SUCCESS', changes='1,1'),
-        ])
+            dict(name='library-builder', result='SUCCESS', changes='1,1'),
+        ], ordered=False)
 
         B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B')
         B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
@@ -5022,10 +5051,13 @@ class TestProvidesRequires(ZuulDBTestCase):
 
         self.assertHistory([
             dict(name='image-builder', result='SUCCESS', changes='1,1'),
+            dict(name='library-builder', result='SUCCESS', changes='1,1'),
             dict(name='image-user', result='SUCCESS', changes='1,1 2,1'),
-        ])
+            dict(name='library-user', result='SUCCESS', changes='1,1 2,1'),
+        ], ordered=False)
+        image_user = self.getJobFromHistory('image-user')
         self.assertEqual(
-            self.history[-1].parameters['zuul']['artifacts'],
+            image_user.parameters['zuul']['artifacts'],
             [{
                 'project': 'org/project1',
                 'change': '1',
@@ -5034,17 +5066,30 @@ class TestProvidesRequires(ZuulDBTestCase):
                 'url': 'http://example.com/image',
                 'name': 'image',
             }])
+        library_user = self.getJobFromHistory('library-user')
+        self.assertEqual(
+            library_user.parameters['zuul']['artifacts'],
+            [{
+                'project': 'org/project1',
+                'change': '1',
+                'patchset': '1',
+                'job': 'library-builder',
+                'url': 'http://example.com/library',
+                'name': 'library',
+            }])
 
     @simple_layout('layouts/provides-requires.yaml')
     def test_provides_requires_check_old_failure(self):
         A = self.fake_gerrit.addFakeChange('org/project1', 'master', 'A')
         self.executor_server.failJob('image-builder', A)
+        self.executor_server.failJob('library-builder', A)
         self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
         self.waitUntilSettled()
 
         self.assertHistory([
             dict(name='image-builder', result='FAILURE', changes='1,1'),
-        ])
+            dict(name='library-builder', result='FAILURE', changes='1,1'),
+        ], ordered=False)
 
         B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B')
         B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
@@ -5054,7 +5099,8 @@ class TestProvidesRequires(ZuulDBTestCase):
 
         self.assertHistory([
             dict(name='image-builder', result='FAILURE', changes='1,1'),
-        ])
+            dict(name='library-builder', result='FAILURE', changes='1,1'),
+        ], ordered=False)
         self.assertIn('image-user : SKIPPED', B.messages[0])
         self.assertIn('not met by build', B.messages[0])
 

zuul/model.py

@@ -1987,7 +1987,7 @@ class QueueItem(object):
         self.layout = None
         self.project_pipeline_config = None
         self.job_graph = None
-        self._cached_sql_results = None
+        self._cached_sql_results = {}
 
     def __repr__(self):
         if self.pipeline:
@@ -2186,7 +2186,8 @@ class QueueItem(object):
 
     def _getRequirementsResultFromSQL(self, requirements):
         # This either returns data or raises an exception
-        if self._cached_sql_results is None:
+        requirements_tuple = tuple(sorted(requirements))
+        if requirements_tuple not in self._cached_sql_results:
             sql_driver = self.pipeline.manager.sched.connections.drivers['sql']
             conn = sql_driver.tenant_connections.get(self.pipeline.tenant.name)
             if conn:
@@ -2197,16 +2198,16 @@ class QueueItem(object):
                     change=self.change.number,
                     branch=self.change.branch,
                     patchset=self.change.patchset,
-                    provides=list(requirements))
+                    provides=requirements_tuple)
             else:
                 builds = []
             # Just look at the most recent buildset.
             # TODO: query for a buildset instead of filtering.
             builds = [b for b in builds
                       if b.buildset.uuid == builds[0].buildset.uuid]
-            self._cached_sql_results = builds
+            self._cached_sql_results[requirements_tuple] = builds
 
-        builds = self._cached_sql_results
+        builds = self._cached_sql_results[requirements_tuple]
         data = []
         if not builds:
             return data