Fix duplicate and reversed artifacts
The "current" provides/requires test (which is supposed to test live changes) was inadvertently following the SQL code path in some cases. The test has been updated to include a third job which continues running until the end of the test to ensure that the changes remain in the pipeline the entire time. It, and the SQL version, have been extended with one more change to illustrate a bug where artifact data were being included twice. This was because the data collection method has two recursion points: a) if it is called on a non-live change and it finds a live version of the same change, it recursively calls itself on the non-live version of the same change. Essentially, it "switches" to that version of the change and continues. b) at the end of normal processing of a change, it recurses "up" the queue to the next change ahead. The (a) case did not return in some cases, so in those cases, it would "switch" to a change, add its data, recurse up the queue, then, after unrolling back to the point of the switch, would then recurse up the queue again in case (b). The solution is to avoid recursing up the queue in the (a) case and allow it only in the (b) case -- that is, we only walk up the queue of dependent changes for the original change. Finally, the artifacts were being returned in reverse order -- it is more useful for them to be returned such that the latest version of a given artifact is returned last so that competing versions are overwritten in the correct order. Change-Id: I41ac336c1bc776609645e3662003f2df076dd1d5
This commit is contained in:
parent
967828b1f0
commit
bd7a5d47bf
|
@ -57,12 +57,16 @@
|
|||
name: library-user
|
||||
requires: libraries
|
||||
|
||||
- job:
|
||||
name: hold
|
||||
|
||||
- project:
|
||||
name: org/project1
|
||||
check:
|
||||
jobs:
|
||||
- image-builder
|
||||
- library-builder
|
||||
- hold
|
||||
gate:
|
||||
queue: integrated
|
||||
jobs:
|
||||
|
@ -74,6 +78,7 @@
|
|||
jobs:
|
||||
- image-user
|
||||
- library-user
|
||||
- hold
|
||||
gate:
|
||||
queue: integrated
|
||||
jobs:
|
||||
|
|
|
@ -4977,16 +4977,41 @@ class TestProvidesRequires(ZuulDBTestCase):
|
|||
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
|
||||
self.waitUntilSettled()
|
||||
|
||||
self.assertEqual(len(self.builds), 2)
|
||||
self.assertEqual(len(self.builds), 3)
|
||||
|
||||
B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B')
|
||||
B = self.fake_gerrit.addFakeChange('org/project1', 'master', 'B')
|
||||
B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
|
||||
B.subject, A.data['id'])
|
||||
self.executor_server.returnData(
|
||||
'image-builder', B,
|
||||
{'zuul':
|
||||
{'artifacts': [
|
||||
{'name': 'image2', 'url': 'http://example.com/image2'},
|
||||
]}}
|
||||
)
|
||||
self.executor_server.returnData(
|
||||
'library-builder', B,
|
||||
{'zuul':
|
||||
{'artifacts': [
|
||||
{'name': 'library2', 'url': 'http://example.com/library2'},
|
||||
]}}
|
||||
)
|
||||
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
|
||||
self.waitUntilSettled()
|
||||
|
||||
self.assertEqual(len(self.builds), 2)
|
||||
self.assertEqual(len(self.builds), 6)
|
||||
|
||||
C = self.fake_gerrit.addFakeChange('org/project2', 'master', 'C')
|
||||
C.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
|
||||
C.subject, B.data['id'])
|
||||
self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(1))
|
||||
self.waitUntilSettled()
|
||||
|
||||
self.assertEqual(len(self.builds), 7)
|
||||
|
||||
self.executor_server.release('image-*')
|
||||
self.executor_server.release('library-*')
|
||||
self.waitUntilSettled()
|
||||
self.executor_server.hold_jobs_in_build = False
|
||||
self.executor_server.release()
|
||||
self.waitUntilSettled()
|
||||
|
@ -4994,8 +5019,14 @@ 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'),
|
||||
dict(name='hold', result='SUCCESS', changes='1,1'),
|
||||
dict(name='image-builder', result='SUCCESS', changes='1,1 2,1'),
|
||||
dict(name='library-builder', result='SUCCESS', changes='1,1 2,1'),
|
||||
dict(name='hold', result='SUCCESS', changes='1,1 2,1'),
|
||||
dict(name='image-user', result='SUCCESS', changes='1,1 2,1 3,1'),
|
||||
dict(name='library-user', result='SUCCESS',
|
||||
changes='1,1 2,1 3,1'),
|
||||
dict(name='hold', result='SUCCESS', changes='1,1 2,1 3,1'),
|
||||
], ordered=False)
|
||||
image_user = self.getJobFromHistory('image-user')
|
||||
self.assertEqual(
|
||||
|
@ -5007,6 +5038,13 @@ class TestProvidesRequires(ZuulDBTestCase):
|
|||
'job': 'image-builder',
|
||||
'url': 'http://example.com/image',
|
||||
'name': 'image',
|
||||
}, {
|
||||
'project': 'org/project1',
|
||||
'change': '2',
|
||||
'patchset': '1',
|
||||
'job': 'image-builder',
|
||||
'url': 'http://example.com/image2',
|
||||
'name': 'image2',
|
||||
}])
|
||||
library_user = self.getJobFromHistory('library-user')
|
||||
self.assertEqual(
|
||||
|
@ -5018,6 +5056,13 @@ class TestProvidesRequires(ZuulDBTestCase):
|
|||
'job': 'library-builder',
|
||||
'url': 'http://example.com/library',
|
||||
'name': 'library',
|
||||
}, {
|
||||
'project': 'org/project1',
|
||||
'change': '2',
|
||||
'patchset': '1',
|
||||
'job': 'library-builder',
|
||||
'url': 'http://example.com/library2',
|
||||
'name': 'library2',
|
||||
}])
|
||||
|
||||
@simple_layout('layouts/provides-requires.yaml')
|
||||
|
@ -5042,19 +5087,54 @@ 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='hold', result='SUCCESS', changes='1,1'),
|
||||
], ordered=False)
|
||||
|
||||
B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B')
|
||||
B = self.fake_gerrit.addFakeChange('org/project1', 'master', 'B')
|
||||
B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
|
||||
B.subject, A.data['id'])
|
||||
self.executor_server.returnData(
|
||||
'image-builder', B,
|
||||
{'zuul':
|
||||
{'artifacts': [
|
||||
{'name': 'image2', 'url': 'http://example.com/image2'},
|
||||
]}}
|
||||
)
|
||||
self.executor_server.returnData(
|
||||
'library-builder', B,
|
||||
{'zuul':
|
||||
{'artifacts': [
|
||||
{'name': 'library2', 'url': 'http://example.com/library2'},
|
||||
]}}
|
||||
)
|
||||
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
|
||||
self.waitUntilSettled()
|
||||
self.assertHistory([
|
||||
dict(name='image-builder', result='SUCCESS', changes='1,1'),
|
||||
dict(name='library-builder', result='SUCCESS', changes='1,1'),
|
||||
dict(name='hold', result='SUCCESS', changes='1,1'),
|
||||
dict(name='image-builder', result='SUCCESS', changes='1,1 2,1'),
|
||||
dict(name='library-builder', result='SUCCESS', changes='1,1 2,1'),
|
||||
dict(name='hold', result='SUCCESS', changes='1,1 2,1'),
|
||||
], ordered=False)
|
||||
|
||||
C = self.fake_gerrit.addFakeChange('org/project2', 'master', 'C')
|
||||
C.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
|
||||
C.subject, B.data['id'])
|
||||
self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(1))
|
||||
self.waitUntilSettled()
|
||||
|
||||
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'),
|
||||
dict(name='hold', result='SUCCESS', changes='1,1'),
|
||||
dict(name='image-builder', result='SUCCESS', changes='1,1 2,1'),
|
||||
dict(name='library-builder', result='SUCCESS', changes='1,1 2,1'),
|
||||
dict(name='hold', result='SUCCESS', changes='1,1 2,1'),
|
||||
dict(name='image-user', result='SUCCESS', changes='1,1 2,1 3,1'),
|
||||
dict(name='library-user', result='SUCCESS',
|
||||
changes='1,1 2,1 3,1'),
|
||||
dict(name='hold', result='SUCCESS', changes='1,1 2,1 3,1'),
|
||||
], ordered=False)
|
||||
image_user = self.getJobFromHistory('image-user')
|
||||
self.assertEqual(
|
||||
|
@ -5066,6 +5146,13 @@ class TestProvidesRequires(ZuulDBTestCase):
|
|||
'job': 'image-builder',
|
||||
'url': 'http://example.com/image',
|
||||
'name': 'image',
|
||||
}, {
|
||||
'project': 'org/project1',
|
||||
'change': '2',
|
||||
'patchset': '1',
|
||||
'job': 'image-builder',
|
||||
'url': 'http://example.com/image2',
|
||||
'name': 'image2',
|
||||
}])
|
||||
library_user = self.getJobFromHistory('library-user')
|
||||
self.assertEqual(
|
||||
|
@ -5077,6 +5164,13 @@ class TestProvidesRequires(ZuulDBTestCase):
|
|||
'job': 'library-builder',
|
||||
'url': 'http://example.com/library',
|
||||
'name': 'library',
|
||||
}, {
|
||||
'project': 'org/project1',
|
||||
'change': '2',
|
||||
'patchset': '1',
|
||||
'job': 'library-builder',
|
||||
'url': 'http://example.com/library2',
|
||||
'name': 'library2',
|
||||
}])
|
||||
|
||||
@simple_layout('layouts/provides-requires.yaml')
|
||||
|
@ -5090,6 +5184,7 @@ class TestProvidesRequires(ZuulDBTestCase):
|
|||
self.assertHistory([
|
||||
dict(name='image-builder', result='FAILURE', changes='1,1'),
|
||||
dict(name='library-builder', result='FAILURE', changes='1,1'),
|
||||
dict(name='hold', result='SUCCESS', changes='1,1'),
|
||||
], ordered=False)
|
||||
|
||||
B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B')
|
||||
|
@ -5101,6 +5196,8 @@ class TestProvidesRequires(ZuulDBTestCase):
|
|||
self.assertHistory([
|
||||
dict(name='image-builder', result='FAILURE', changes='1,1'),
|
||||
dict(name='library-builder', result='FAILURE', changes='1,1'),
|
||||
dict(name='hold', result='SUCCESS', changes='1,1'),
|
||||
dict(name='hold', result='SUCCESS', changes='1,1 2,1'),
|
||||
], ordered=False)
|
||||
self.assertIn('image-user : SKIPPED', B.messages[0])
|
||||
self.assertIn('not met by build', B.messages[0])
|
||||
|
|
|
@ -2257,7 +2257,7 @@ class QueueItem(object):
|
|||
data.append(artifact)
|
||||
return data
|
||||
|
||||
def providesRequirements(self, requirements, data):
|
||||
def providesRequirements(self, requirements, data, recurse=True):
|
||||
# Mutates data and returns true/false if requirements
|
||||
# satisfied.
|
||||
if not requirements:
|
||||
|
@ -2271,7 +2271,8 @@ class QueueItem(object):
|
|||
found = True
|
||||
break
|
||||
if found:
|
||||
if not item.providesRequirements(requirements, data):
|
||||
if not item.providesRequirements(requirements, data,
|
||||
recurse=False):
|
||||
return False
|
||||
else:
|
||||
# Look for this item in the SQL DB.
|
||||
|
@ -2299,6 +2300,8 @@ class QueueItem(object):
|
|||
data += artifacts
|
||||
if not self.item_ahead:
|
||||
return True
|
||||
if not recurse:
|
||||
return True
|
||||
return self.item_ahead.providesRequirements(requirements, data)
|
||||
|
||||
def jobRequirementsReady(self, job):
|
||||
|
@ -2307,6 +2310,7 @@ class QueueItem(object):
|
|||
try:
|
||||
data = []
|
||||
ret = self.item_ahead.providesRequirements(job.requires, data)
|
||||
data.reverse()
|
||||
job.updateArtifactData(data)
|
||||
except RequirementsError as e:
|
||||
self.warning(str(e))
|
||||
|
|
Loading…
Reference in New Issue