Browse Source

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
tags/3.6.0
James E. Blair 2 months ago
parent
commit
bd7a5d47bf
3 changed files with 116 additions and 10 deletions
  1. 5
    0
      tests/fixtures/layouts/provides-requires.yaml
  2. 105
    8
      tests/unit/test_v3.py
  3. 6
    2
      zuul/model.py

+ 5
- 0
tests/fixtures/layouts/provides-requires.yaml View File

@@ -57,12 +57,16 @@
57 57
     name: library-user
58 58
     requires: libraries
59 59
 
60
+- job:
61
+    name: hold
62
+
60 63
 - project:
61 64
     name: org/project1
62 65
     check:
63 66
       jobs:
64 67
         - image-builder
65 68
         - library-builder
69
+        - hold
66 70
     gate:
67 71
       queue: integrated
68 72
       jobs:
@@ -74,6 +78,7 @@
74 78
       jobs:
75 79
         - image-user
76 80
         - library-user
81
+        - hold
77 82
     gate:
78 83
       queue: integrated
79 84
       jobs:

+ 105
- 8
tests/unit/test_v3.py View File

@@ -4977,16 +4977,41 @@ class TestProvidesRequires(ZuulDBTestCase):
4977 4977
         self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
4978 4978
         self.waitUntilSettled()
4979 4979
 
4980
-        self.assertEqual(len(self.builds), 2)
4980
+        self.assertEqual(len(self.builds), 3)
4981 4981
 
4982
-        B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B')
4982
+        B = self.fake_gerrit.addFakeChange('org/project1', 'master', 'B')
4983 4983
         B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
4984 4984
             B.subject, A.data['id'])
4985
+        self.executor_server.returnData(
4986
+            'image-builder', B,
4987
+            {'zuul':
4988
+             {'artifacts': [
4989
+                 {'name': 'image2', 'url': 'http://example.com/image2'},
4990
+             ]}}
4991
+        )
4992
+        self.executor_server.returnData(
4993
+            'library-builder', B,
4994
+            {'zuul':
4995
+             {'artifacts': [
4996
+                 {'name': 'library2', 'url': 'http://example.com/library2'},
4997
+             ]}}
4998
+        )
4985 4999
         self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
4986 5000
         self.waitUntilSettled()
4987 5001
 
4988
-        self.assertEqual(len(self.builds), 2)
5002
+        self.assertEqual(len(self.builds), 6)
4989 5003
 
5004
+        C = self.fake_gerrit.addFakeChange('org/project2', 'master', 'C')
5005
+        C.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
5006
+            C.subject, B.data['id'])
5007
+        self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(1))
5008
+        self.waitUntilSettled()
5009
+
5010
+        self.assertEqual(len(self.builds), 7)
5011
+
5012
+        self.executor_server.release('image-*')
5013
+        self.executor_server.release('library-*')
5014
+        self.waitUntilSettled()
4990 5015
         self.executor_server.hold_jobs_in_build = False
4991 5016
         self.executor_server.release()
4992 5017
         self.waitUntilSettled()
@@ -4994,8 +5019,14 @@ class TestProvidesRequires(ZuulDBTestCase):
4994 5019
         self.assertHistory([
4995 5020
             dict(name='image-builder', result='SUCCESS', changes='1,1'),
4996 5021
             dict(name='library-builder', result='SUCCESS', changes='1,1'),
4997
-            dict(name='image-user', result='SUCCESS', changes='1,1 2,1'),
4998
-            dict(name='library-user', result='SUCCESS', changes='1,1 2,1'),
5022
+            dict(name='hold', result='SUCCESS', changes='1,1'),
5023
+            dict(name='image-builder', result='SUCCESS', changes='1,1 2,1'),
5024
+            dict(name='library-builder', result='SUCCESS', changes='1,1 2,1'),
5025
+            dict(name='hold', result='SUCCESS', changes='1,1 2,1'),
5026
+            dict(name='image-user', result='SUCCESS', changes='1,1 2,1 3,1'),
5027
+            dict(name='library-user', result='SUCCESS',
5028
+                 changes='1,1 2,1 3,1'),
5029
+            dict(name='hold', result='SUCCESS', changes='1,1 2,1 3,1'),
4999 5030
         ], ordered=False)
5000 5031
         image_user = self.getJobFromHistory('image-user')
5001 5032
         self.assertEqual(
@@ -5007,6 +5038,13 @@ class TestProvidesRequires(ZuulDBTestCase):
5007 5038
                 'job': 'image-builder',
5008 5039
                 'url': 'http://example.com/image',
5009 5040
                 'name': 'image',
5041
+            }, {
5042
+                'project': 'org/project1',
5043
+                'change': '2',
5044
+                'patchset': '1',
5045
+                'job': 'image-builder',
5046
+                'url': 'http://example.com/image2',
5047
+                'name': 'image2',
5010 5048
             }])
5011 5049
         library_user = self.getJobFromHistory('library-user')
5012 5050
         self.assertEqual(
@@ -5018,6 +5056,13 @@ class TestProvidesRequires(ZuulDBTestCase):
5018 5056
                 'job': 'library-builder',
5019 5057
                 'url': 'http://example.com/library',
5020 5058
                 'name': 'library',
5059
+            }, {
5060
+                'project': 'org/project1',
5061
+                'change': '2',
5062
+                'patchset': '1',
5063
+                'job': 'library-builder',
5064
+                'url': 'http://example.com/library2',
5065
+                'name': 'library2',
5021 5066
             }])
5022 5067
 
5023 5068
     @simple_layout('layouts/provides-requires.yaml')
@@ -5042,19 +5087,54 @@ class TestProvidesRequires(ZuulDBTestCase):
5042 5087
         self.assertHistory([
5043 5088
             dict(name='image-builder', result='SUCCESS', changes='1,1'),
5044 5089
             dict(name='library-builder', result='SUCCESS', changes='1,1'),
5090
+            dict(name='hold', result='SUCCESS', changes='1,1'),
5045 5091
         ], ordered=False)
5046 5092
 
5047
-        B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B')
5093
+        B = self.fake_gerrit.addFakeChange('org/project1', 'master', 'B')
5048 5094
         B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
5049 5095
             B.subject, A.data['id'])
5096
+        self.executor_server.returnData(
5097
+            'image-builder', B,
5098
+            {'zuul':
5099
+             {'artifacts': [
5100
+                 {'name': 'image2', 'url': 'http://example.com/image2'},
5101
+             ]}}
5102
+        )
5103
+        self.executor_server.returnData(
5104
+            'library-builder', B,
5105
+            {'zuul':
5106
+             {'artifacts': [
5107
+                 {'name': 'library2', 'url': 'http://example.com/library2'},
5108
+             ]}}
5109
+        )
5050 5110
         self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
5051 5111
         self.waitUntilSettled()
5112
+        self.assertHistory([
5113
+            dict(name='image-builder', result='SUCCESS', changes='1,1'),
5114
+            dict(name='library-builder', result='SUCCESS', changes='1,1'),
5115
+            dict(name='hold', result='SUCCESS', changes='1,1'),
5116
+            dict(name='image-builder', result='SUCCESS', changes='1,1 2,1'),
5117
+            dict(name='library-builder', result='SUCCESS', changes='1,1 2,1'),
5118
+            dict(name='hold', result='SUCCESS', changes='1,1 2,1'),
5119
+        ], ordered=False)
5120
+
5121
+        C = self.fake_gerrit.addFakeChange('org/project2', 'master', 'C')
5122
+        C.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % (
5123
+            C.subject, B.data['id'])
5124
+        self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(1))
5125
+        self.waitUntilSettled()
5052 5126
 
5053 5127
         self.assertHistory([
5054 5128
             dict(name='image-builder', result='SUCCESS', changes='1,1'),
5055 5129
             dict(name='library-builder', result='SUCCESS', changes='1,1'),
5056
-            dict(name='image-user', result='SUCCESS', changes='1,1 2,1'),
5057
-            dict(name='library-user', result='SUCCESS', changes='1,1 2,1'),
5130
+            dict(name='hold', result='SUCCESS', changes='1,1'),
5131
+            dict(name='image-builder', result='SUCCESS', changes='1,1 2,1'),
5132
+            dict(name='library-builder', result='SUCCESS', changes='1,1 2,1'),
5133
+            dict(name='hold', result='SUCCESS', changes='1,1 2,1'),
5134
+            dict(name='image-user', result='SUCCESS', changes='1,1 2,1 3,1'),
5135
+            dict(name='library-user', result='SUCCESS',
5136
+                 changes='1,1 2,1 3,1'),
5137
+            dict(name='hold', result='SUCCESS', changes='1,1 2,1 3,1'),
5058 5138
         ], ordered=False)
5059 5139
         image_user = self.getJobFromHistory('image-user')
5060 5140
         self.assertEqual(
@@ -5066,6 +5146,13 @@ class TestProvidesRequires(ZuulDBTestCase):
5066 5146
                 'job': 'image-builder',
5067 5147
                 'url': 'http://example.com/image',
5068 5148
                 'name': 'image',
5149
+            }, {
5150
+                'project': 'org/project1',
5151
+                'change': '2',
5152
+                'patchset': '1',
5153
+                'job': 'image-builder',
5154
+                'url': 'http://example.com/image2',
5155
+                'name': 'image2',
5069 5156
             }])
5070 5157
         library_user = self.getJobFromHistory('library-user')
5071 5158
         self.assertEqual(
@@ -5077,6 +5164,13 @@ class TestProvidesRequires(ZuulDBTestCase):
5077 5164
                 'job': 'library-builder',
5078 5165
                 'url': 'http://example.com/library',
5079 5166
                 'name': 'library',
5167
+            }, {
5168
+                'project': 'org/project1',
5169
+                'change': '2',
5170
+                'patchset': '1',
5171
+                'job': 'library-builder',
5172
+                'url': 'http://example.com/library2',
5173
+                'name': 'library2',
5080 5174
             }])
5081 5175
 
5082 5176
     @simple_layout('layouts/provides-requires.yaml')
@@ -5090,6 +5184,7 @@ class TestProvidesRequires(ZuulDBTestCase):
5090 5184
         self.assertHistory([
5091 5185
             dict(name='image-builder', result='FAILURE', changes='1,1'),
5092 5186
             dict(name='library-builder', result='FAILURE', changes='1,1'),
5187
+            dict(name='hold', result='SUCCESS', changes='1,1'),
5093 5188
         ], ordered=False)
5094 5189
 
5095 5190
         B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B')
@@ -5101,6 +5196,8 @@ class TestProvidesRequires(ZuulDBTestCase):
5101 5196
         self.assertHistory([
5102 5197
             dict(name='image-builder', result='FAILURE', changes='1,1'),
5103 5198
             dict(name='library-builder', result='FAILURE', changes='1,1'),
5199
+            dict(name='hold', result='SUCCESS', changes='1,1'),
5200
+            dict(name='hold', result='SUCCESS', changes='1,1 2,1'),
5104 5201
         ], ordered=False)
5105 5202
         self.assertIn('image-user : SKIPPED', B.messages[0])
5106 5203
         self.assertIn('not met by build', B.messages[0])

+ 6
- 2
zuul/model.py View File

@@ -2257,7 +2257,7 @@ class QueueItem(object):
2257 2257
                     data.append(artifact)
2258 2258
         return data
2259 2259
 
2260
-    def providesRequirements(self, requirements, data):
2260
+    def providesRequirements(self, requirements, data, recurse=True):
2261 2261
         # Mutates data and returns true/false if requirements
2262 2262
         # satisfied.
2263 2263
         if not requirements:
@@ -2271,7 +2271,8 @@ class QueueItem(object):
2271 2271
                     found = True
2272 2272
                     break
2273 2273
             if found:
2274
-                if not item.providesRequirements(requirements, data):
2274
+                if not item.providesRequirements(requirements, data,
2275
+                                                 recurse=False):
2275 2276
                     return False
2276 2277
             else:
2277 2278
                 # Look for this item in the SQL DB.
@@ -2299,6 +2300,8 @@ class QueueItem(object):
2299 2300
                     data += artifacts
2300 2301
         if not self.item_ahead:
2301 2302
             return True
2303
+        if not recurse:
2304
+            return True
2302 2305
         return self.item_ahead.providesRequirements(requirements, data)
2303 2306
 
2304 2307
     def jobRequirementsReady(self, job):
@@ -2307,6 +2310,7 @@ class QueueItem(object):
2307 2310
         try:
2308 2311
             data = []
2309 2312
             ret = self.item_ahead.providesRequirements(job.requires, data)
2313
+            data.reverse()
2310 2314
             job.updateArtifactData(data)
2311 2315
         except RequirementsError as e:
2312 2316
             self.warning(str(e))

Loading…
Cancel
Save