summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJames E. Blair <jeblair@redhat.com>2019-02-26 16:28:34 -0800
committerJames E. Blair <jeblair@redhat.com>2019-02-27 13:13:16 -0800
commit08163359f7bebc13132affb4e3be456b5c9bd973 (patch)
tree87be2ee885db42ef67a8ccba419c26125babc3d6
parent62e0c131908298a36689b045638d04de35ebaac7 (diff)
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
Notes
Notes (review): Code-Review+2: Tobias Henkel <tobias.henkel@bmw.de> Code-Review+2: Monty Taylor <mordred@inaugust.com> Workflow+1: Monty Taylor <mordred@inaugust.com> Verified+2: Zuul Submitted-by: Zuul Submitted-at: Thu, 28 Feb 2019 17:09:06 +0000 Reviewed-on: https://review.openstack.org/639472 Project: openstack-infra/zuul Branch: refs/heads/master
-rw-r--r--tests/fixtures/layouts/provides-requires.yaml10
-rw-r--r--tests/unit/test_v3.py64
-rw-r--r--zuul/model.py11
3 files changed, 71 insertions, 14 deletions
diff --git a/tests/fixtures/layouts/provides-requires.yaml b/tests/fixtures/layouts/provides-requires.yaml
index 3947345..c71c49c 100644
--- a/tests/fixtures/layouts/provides-requires.yaml
+++ b/tests/fixtures/layouts/provides-requires.yaml
@@ -49,11 +49,20 @@
49 name: image-user 49 name: image-user
50 requires: images 50 requires: images
51 51
52- job:
53 name: library-builder
54 provides: libraries
55
56- job:
57 name: library-user
58 requires: libraries
59
52- project: 60- project:
53 name: org/project1 61 name: org/project1
54 check: 62 check:
55 jobs: 63 jobs:
56 - image-builder 64 - image-builder
65 - library-builder
57 gate: 66 gate:
58 queue: integrated 67 queue: integrated
59 jobs: 68 jobs:
@@ -64,6 +73,7 @@
64 check: 73 check:
65 jobs: 74 jobs:
66 - image-user 75 - image-user
76 - library-user
67 gate: 77 gate:
68 queue: integrated 78 queue: integrated
69 jobs: 79 jobs:
diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py
index 7c45a02..de59cc8 100644
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -4966,10 +4966,17 @@ class TestProvidesRequires(ZuulDBTestCase):
4966 {'name': 'image', 'url': 'http://example.com/image'}, 4966 {'name': 'image', 'url': 'http://example.com/image'},
4967 ]}} 4967 ]}}
4968 ) 4968 )
4969 self.executor_server.returnData(
4970 'library-builder', A,
4971 {'zuul':
4972 {'artifacts': [
4973 {'name': 'library', 'url': 'http://example.com/library'},
4974 ]}}
4975 )
4969 self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) 4976 self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
4970 self.waitUntilSettled() 4977 self.waitUntilSettled()
4971 4978
4972 self.assertEqual(len(self.builds), 1) 4979 self.assertEqual(len(self.builds), 2)
4973 4980
4974 B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B') 4981 B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B')
4975 B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( 4982 B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
@@ -4977,7 +4984,7 @@ class TestProvidesRequires(ZuulDBTestCase):
4977 self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) 4984 self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
4978 self.waitUntilSettled() 4985 self.waitUntilSettled()
4979 4986
4980 self.assertEqual(len(self.builds), 1) 4987 self.assertEqual(len(self.builds), 2)
4981 4988
4982 self.executor_server.hold_jobs_in_build = False 4989 self.executor_server.hold_jobs_in_build = False
4983 self.executor_server.release() 4990 self.executor_server.release()
@@ -4985,10 +4992,13 @@ class TestProvidesRequires(ZuulDBTestCase):
4985 4992
4986 self.assertHistory([ 4993 self.assertHistory([
4987 dict(name='image-builder', result='SUCCESS', changes='1,1'), 4994 dict(name='image-builder', result='SUCCESS', changes='1,1'),
4995 dict(name='library-builder', result='SUCCESS', changes='1,1'),
4988 dict(name='image-user', result='SUCCESS', changes='1,1 2,1'), 4996 dict(name='image-user', result='SUCCESS', changes='1,1 2,1'),
4989 ]) 4997 dict(name='library-user', result='SUCCESS', changes='1,1 2,1'),
4998 ], ordered=False)
4999 image_user = self.getJobFromHistory('image-user')
4990 self.assertEqual( 5000 self.assertEqual(
4991 self.history[-1].parameters['zuul']['artifacts'], 5001 image_user.parameters['zuul']['artifacts'],
4992 [{ 5002 [{
4993 'project': 'org/project1', 5003 'project': 'org/project1',
4994 'change': '1', 5004 'change': '1',
@@ -4997,6 +5007,17 @@ class TestProvidesRequires(ZuulDBTestCase):
4997 'url': 'http://example.com/image', 5007 'url': 'http://example.com/image',
4998 'name': 'image', 5008 'name': 'image',
4999 }]) 5009 }])
5010 library_user = self.getJobFromHistory('library-user')
5011 self.assertEqual(
5012 library_user.parameters['zuul']['artifacts'],
5013 [{
5014 'project': 'org/project1',
5015 'change': '1',
5016 'patchset': '1',
5017 'job': 'library-builder',
5018 'url': 'http://example.com/library',
5019 'name': 'library',
5020 }])
5000 5021
5001 @simple_layout('layouts/provides-requires.yaml') 5022 @simple_layout('layouts/provides-requires.yaml')
5002 def test_provides_requires_check_old_success(self): 5023 def test_provides_requires_check_old_success(self):
@@ -5008,11 +5029,19 @@ class TestProvidesRequires(ZuulDBTestCase):
5008 {'name': 'image', 'url': 'http://example.com/image'}, 5029 {'name': 'image', 'url': 'http://example.com/image'},
5009 ]}} 5030 ]}}
5010 ) 5031 )
5032 self.executor_server.returnData(
5033 'library-builder', A,
5034 {'zuul':
5035 {'artifacts': [
5036 {'name': 'library', 'url': 'http://example.com/library'},
5037 ]}}
5038 )
5011 self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) 5039 self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
5012 self.waitUntilSettled() 5040 self.waitUntilSettled()
5013 self.assertHistory([ 5041 self.assertHistory([
5014 dict(name='image-builder', result='SUCCESS', changes='1,1'), 5042 dict(name='image-builder', result='SUCCESS', changes='1,1'),
5015 ]) 5043 dict(name='library-builder', result='SUCCESS', changes='1,1'),
5044 ], ordered=False)
5016 5045
5017 B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B') 5046 B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B')
5018 B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( 5047 B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
@@ -5022,10 +5051,13 @@ class TestProvidesRequires(ZuulDBTestCase):
5022 5051
5023 self.assertHistory([ 5052 self.assertHistory([
5024 dict(name='image-builder', result='SUCCESS', changes='1,1'), 5053 dict(name='image-builder', result='SUCCESS', changes='1,1'),
5054 dict(name='library-builder', result='SUCCESS', changes='1,1'),
5025 dict(name='image-user', result='SUCCESS', changes='1,1 2,1'), 5055 dict(name='image-user', result='SUCCESS', changes='1,1 2,1'),
5026 ]) 5056 dict(name='library-user', result='SUCCESS', changes='1,1 2,1'),
5057 ], ordered=False)
5058 image_user = self.getJobFromHistory('image-user')
5027 self.assertEqual( 5059 self.assertEqual(
5028 self.history[-1].parameters['zuul']['artifacts'], 5060 image_user.parameters['zuul']['artifacts'],
5029 [{ 5061 [{
5030 'project': 'org/project1', 5062 'project': 'org/project1',
5031 'change': '1', 5063 'change': '1',
@@ -5034,17 +5066,30 @@ class TestProvidesRequires(ZuulDBTestCase):
5034 'url': 'http://example.com/image', 5066 'url': 'http://example.com/image',
5035 'name': 'image', 5067 'name': 'image',
5036 }]) 5068 }])
5069 library_user = self.getJobFromHistory('library-user')
5070 self.assertEqual(
5071 library_user.parameters['zuul']['artifacts'],
5072 [{
5073 'project': 'org/project1',
5074 'change': '1',
5075 'patchset': '1',
5076 'job': 'library-builder',
5077 'url': 'http://example.com/library',
5078 'name': 'library',
5079 }])
5037 5080
5038 @simple_layout('layouts/provides-requires.yaml') 5081 @simple_layout('layouts/provides-requires.yaml')
5039 def test_provides_requires_check_old_failure(self): 5082 def test_provides_requires_check_old_failure(self):
5040 A = self.fake_gerrit.addFakeChange('org/project1', 'master', 'A') 5083 A = self.fake_gerrit.addFakeChange('org/project1', 'master', 'A')
5041 self.executor_server.failJob('image-builder', A) 5084 self.executor_server.failJob('image-builder', A)
5085 self.executor_server.failJob('library-builder', A)
5042 self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) 5086 self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
5043 self.waitUntilSettled() 5087 self.waitUntilSettled()
5044 5088
5045 self.assertHistory([ 5089 self.assertHistory([
5046 dict(name='image-builder', result='FAILURE', changes='1,1'), 5090 dict(name='image-builder', result='FAILURE', changes='1,1'),
5047 ]) 5091 dict(name='library-builder', result='FAILURE', changes='1,1'),
5092 ], ordered=False)
5048 5093
5049 B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B') 5094 B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B')
5050 B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( 5095 B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
@@ -5054,7 +5099,8 @@ class TestProvidesRequires(ZuulDBTestCase):
5054 5099
5055 self.assertHistory([ 5100 self.assertHistory([
5056 dict(name='image-builder', result='FAILURE', changes='1,1'), 5101 dict(name='image-builder', result='FAILURE', changes='1,1'),
5057 ]) 5102 dict(name='library-builder', result='FAILURE', changes='1,1'),
5103 ], ordered=False)
5058 self.assertIn('image-user : SKIPPED', B.messages[0]) 5104 self.assertIn('image-user : SKIPPED', B.messages[0])
5059 self.assertIn('not met by build', B.messages[0]) 5105 self.assertIn('not met by build', B.messages[0])
5060 5106
diff --git a/zuul/model.py b/zuul/model.py
index 89a6e5c..24ebf41 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -1987,7 +1987,7 @@ class QueueItem(object):
1987 self.layout = None 1987 self.layout = None
1988 self.project_pipeline_config = None 1988 self.project_pipeline_config = None
1989 self.job_graph = None 1989 self.job_graph = None
1990 self._cached_sql_results = None 1990 self._cached_sql_results = {}
1991 1991
1992 def __repr__(self): 1992 def __repr__(self):
1993 if self.pipeline: 1993 if self.pipeline:
@@ -2186,7 +2186,8 @@ class QueueItem(object):
2186 2186
2187 def _getRequirementsResultFromSQL(self, requirements): 2187 def _getRequirementsResultFromSQL(self, requirements):
2188 # This either returns data or raises an exception 2188 # This either returns data or raises an exception
2189 if self._cached_sql_results is None: 2189 requirements_tuple = tuple(sorted(requirements))
2190 if requirements_tuple not in self._cached_sql_results:
2190 sql_driver = self.pipeline.manager.sched.connections.drivers['sql'] 2191 sql_driver = self.pipeline.manager.sched.connections.drivers['sql']
2191 conn = sql_driver.tenant_connections.get(self.pipeline.tenant.name) 2192 conn = sql_driver.tenant_connections.get(self.pipeline.tenant.name)
2192 if conn: 2193 if conn:
@@ -2197,16 +2198,16 @@ class QueueItem(object):
2197 change=self.change.number, 2198 change=self.change.number,
2198 branch=self.change.branch, 2199 branch=self.change.branch,
2199 patchset=self.change.patchset, 2200 patchset=self.change.patchset,
2200 provides=list(requirements)) 2201 provides=requirements_tuple)
2201 else: 2202 else:
2202 builds = [] 2203 builds = []
2203 # Just look at the most recent buildset. 2204 # Just look at the most recent buildset.
2204 # TODO: query for a buildset instead of filtering. 2205 # TODO: query for a buildset instead of filtering.
2205 builds = [b for b in builds 2206 builds = [b for b in builds
2206 if b.buildset.uuid == builds[0].buildset.uuid] 2207 if b.buildset.uuid == builds[0].buildset.uuid]
2207 self._cached_sql_results = builds 2208 self._cached_sql_results[requirements_tuple] = builds
2208 2209
2209 builds = self._cached_sql_results 2210 builds = self._cached_sql_results[requirements_tuple]
2210 data = [] 2211 data = []
2211 if not builds: 2212 if not builds:
2212 return data 2213 return data