From 61d42b33c7908e9fec5b7f1f0987f17594c3e1e8 Mon Sep 17 00:00:00 2001 From: Andreas Jaeger Date: Wed, 20 Jan 2016 19:31:34 +0100 Subject: [PATCH 1/4] Remove argparse from requirements argparse was external in python 2.6 but not anymore, remove it from requirements. This should help with pip 8.0 that gets confused in this situation. Installation of the external argparse is not needed. Change-Id: Ib7e74912b36c1b5ccb514e31fac35efeff57378d --- requirements.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 01fd2453c2..8388f0bd2b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,5 @@ pbr>=1.1.0 -argparse PyYAML>=3.1.0 Paste WebOb>=1.2.3 From 4bd7da32fab60df0702491e2394519432389ad28 Mon Sep 17 00:00:00 2001 From: Joshua Hesketh Date: Wed, 17 Feb 2016 20:58:47 +1100 Subject: [PATCH 2/4] Cache is held and managed by connections Add reconfigure test case. This test previously fails currently due to a regression introduced with the connections changes. Because multiple sources share a connection, a pipeline that does not hold and therefore require any changes in the cache may clear a connections cache before a pipeline that does need said change has an opportunity to add it to the relevant list. Allow connections to manage their cache directly rather than the source doing it vicariously ignorant of other pipelines/sources. Collect the relevant changes from all pipelines and ask any connections holding a cache for that item to keep it on reconfiguration. Co-Authored-By: James E. Blair Change-Id: I2bf8ba6b9deda58114db9e9b96985a2a0e2a69cb --- tests/test_scheduler.py | 56 +++++++++++++++++++++++++++++++++---- zuul/connection/__init__.py | 7 +++++ zuul/scheduler.py | 14 ++++++---- zuul/source/__init__.py | 7 ----- zuul/source/gerrit.py | 3 -- 5 files changed, 66 insertions(+), 21 deletions(-) diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 81c294824d..b2ec5f7ae8 100755 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -693,8 +693,8 @@ class TestScheduler(ZuulTestCase): # triggering events. Since it will have the changes cached # already (without approvals), we need to clear the cache # first. - source = self.sched.layout.pipelines['gate'].source - source.maintainCache([]) + for connection in self.connections.values(): + connection.maintainCache([]) self.worker.hold_jobs_in_build = True A.addApproval('APRV', 1) @@ -791,7 +791,6 @@ class TestScheduler(ZuulTestCase): A.addApproval('APRV', 1) a = source._getChange(1, 2, refresh=True) self.assertTrue(source.canMerge(a, mgr.getSubmitAllowNeeds())) - source.maintainCache([]) def test_build_configuration(self): "Test that zuul merges the right commits for testing" @@ -2609,6 +2608,53 @@ class TestScheduler(ZuulTestCase): # Ensure the removed job was not included in the report. self.assertNotIn('project1-project2-integration', A.messages[0]) + def test_double_live_reconfiguration_shared_queue(self): + # This was a real-world regression. A change is added to + # gate; a reconfigure happens, a second change which depends + # on the first is added, and a second reconfiguration happens. + # Ensure that both changes merge. + + # A failure may indicate incorrect caching or cleaning up of + # references during a reconfiguration. + self.worker.hold_jobs_in_build = True + + A = self.fake_gerrit.addFakeChange('org/project1', 'master', 'A') + B = self.fake_gerrit.addFakeChange('org/project1', 'master', 'B') + B.setDependsOn(A, 1) + A.addApproval('CRVW', 2) + B.addApproval('CRVW', 2) + + # Add the parent change. + self.fake_gerrit.addEvent(A.addApproval('APRV', 1)) + self.waitUntilSettled() + self.worker.release('.*-merge') + self.waitUntilSettled() + + # Reconfigure (with only one change in the pipeline). + self.sched.reconfigure(self.config) + self.waitUntilSettled() + + # Add the child change. + self.fake_gerrit.addEvent(B.addApproval('APRV', 1)) + self.waitUntilSettled() + self.worker.release('.*-merge') + self.waitUntilSettled() + + # Reconfigure (with both in the pipeline). + self.sched.reconfigure(self.config) + self.waitUntilSettled() + + self.worker.hold_jobs_in_build = False + self.worker.release() + self.waitUntilSettled() + + self.assertEqual(len(self.history), 8) + + self.assertEqual(A.data['status'], 'MERGED') + self.assertEqual(A.reported, 2) + self.assertEqual(B.data['status'], 'MERGED') + self.assertEqual(B.reported, 2) + def test_live_reconfiguration_del_project(self): # Test project deletion from layout # while changes are enqueued @@ -3656,8 +3702,8 @@ For CI problems and help debugging, contact ci@example.org""" self.assertEqual(A.data['status'], 'NEW') self.assertEqual(B.data['status'], 'NEW') - source = self.sched.layout.pipelines['gate'].source - source.maintainCache([]) + for connection in self.connections.values(): + connection.maintainCache([]) self.worker.hold_jobs_in_build = True B.addApproval('APRV', 1) diff --git a/zuul/connection/__init__.py b/zuul/connection/__init__.py index 402528f084..066b4db5a5 100644 --- a/zuul/connection/__init__.py +++ b/zuul/connection/__init__.py @@ -62,3 +62,10 @@ class BaseConnection(object): def registerUse(self, what, instance): self.attached_to[what].append(instance) + + def maintainCache(self, relevant): + """Make cache contain relevant changes. + + 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.""" diff --git a/zuul/scheduler.py b/zuul/scheduler.py index e1aa0c213b..bcbe5558ca 100644 --- a/zuul/scheduler.py +++ b/zuul/scheduler.py @@ -841,7 +841,7 @@ class Scheduler(threading.Thread): "Exception while canceling build %s " "for change %s" % (build, item.change)) self.layout = layout - self.maintainTriggerCache() + self.maintainConnectionCache() for trigger in self.triggers.values(): trigger.postConfig() for pipeline in self.layout.pipelines.values(): @@ -971,16 +971,18 @@ class Scheduler(threading.Thread): finally: self.run_handler_lock.release() - def maintainTriggerCache(self): + def maintainConnectionCache(self): relevant = set() for pipeline in self.layout.pipelines.values(): - self.log.debug("Start maintain trigger cache for: %s" % pipeline) + self.log.debug("Gather relevant cache items for: %s" % pipeline) for item in pipeline.getAllItems(): relevant.add(item.change) relevant.update(item.change.getRelatedChanges()) - pipeline.source.maintainCache(relevant) - self.log.debug("End maintain trigger cache for: %s" % pipeline) - self.log.debug("Trigger cache size: %s" % len(relevant)) + for connection in self.connections.values(): + connection.maintainCache(relevant) + self.log.debug( + "End maintain connection cache for: %s" % connection) + self.log.debug("Connection cache size: %s" % len(relevant)) def process_event_queue(self): self.log.debug("Fetching trigger event") diff --git a/zuul/source/__init__.py b/zuul/source/__init__.py index 25fe9744e4..cb4501aeb9 100644 --- a/zuul/source/__init__.py +++ b/zuul/source/__init__.py @@ -49,13 +49,6 @@ class BaseSource(object): def canMerge(self, change, allow_needs): """Determine if change can merge.""" - def maintainCache(self, relevant): - """Make cache contain relevant changes. - - 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.""" - def postConfig(self): """Called after configuration has been processed.""" diff --git a/zuul/source/gerrit.py b/zuul/source/gerrit.py index f35ab73da4..eb8705d9a2 100644 --- a/zuul/source/gerrit.py +++ b/zuul/source/gerrit.py @@ -319,6 +319,3 @@ class GerritSource(BaseSource): def _getGitwebUrl(self, project, sha=None): return self.connection.getGitwebUrl(project, sha) - - def maintainCache(self, relevant): - self.connection.maintainCache(relevant) From 0a6a0c422cdffe668bec2a9420d9bdd32182a0d8 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Sat, 20 Feb 2016 09:19:19 -0800 Subject: [PATCH 3/4] Cloner: use cache if dest exists The logic to decide whether or not to use the cache attempted to detect whether the repo had previously been cloned. It only did that by checking whether the destination directory exists. However, it's perfectly valid for the dest dir to exist if it is empty. Adjust the check to look for a .git dir within the dest dir to decide if the repo has already been cloned. Change-Id: I17926efcf0f38d6229f0e666e53e6730f455d8ef --- zuul/lib/cloner.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/zuul/lib/cloner.py b/zuul/lib/cloner.py index 0ac7f0fdb7..257b95ded7 100644 --- a/zuul/lib/cloner.py +++ b/zuul/lib/cloner.py @@ -70,9 +70,10 @@ class Cloner(object): # Check for a cached git repo first git_cache = '%s/%s' % (self.cache_dir, project) git_upstream = '%s/%s' % (self.git_url, project) + repo_is_cloned = os.path.exists(os.path.join(dest, '.git')) if (self.cache_dir and os.path.exists(git_cache) and - not os.path.exists(dest)): + not repo_is_cloned): # file:// tells git not to hard-link across repos git_cache = 'file://%s' % git_cache self.log.info("Creating repo %s from cache %s", From 456f2fb74885ba8b69432fccfeda2f7e6dbb4124 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Tue, 9 Feb 2016 09:29:33 -0800 Subject: [PATCH 4/4] Add job tags This allows us to add arbitrary string 'tags' to jobs. These may then be inspected inside of parameter functions for any purpose. In OpenStack, we will likely pass these through untouched to the the build so that they can be picked up by the logstash worker and we can record extra metadata about jobs. Change-Id: Ibc00c6d30cdfe4678864adb13421a4d9f71f5128 --- doc/source/zuul.rst | 6 ++++ tests/base.py | 1 + tests/fixtures/layout-tags.yaml | 42 +++++++++++++++++++++++++ tests/fixtures/layout.yaml | 5 +++ tests/fixtures/tags_custom_functions.py | 2 ++ tests/test_scheduler.py | 19 +++++++++++ zuul/layoutvalidator.py | 1 + zuul/model.py | 6 ++++ zuul/scheduler.py | 7 +++++ 9 files changed, 89 insertions(+) create mode 100644 tests/fixtures/layout-tags.yaml create mode 100644 tests/fixtures/tags_custom_functions.py diff --git a/doc/source/zuul.rst b/doc/source/zuul.rst index b5b8d7bf19..d8d72e69ce 100644 --- a/doc/source/zuul.rst +++ b/doc/source/zuul.rst @@ -765,6 +765,12 @@ each job as it builds a list from the project specification. Boolean value (``true`` or ``false``) that indicates whatever a job is voting or not. Default: ``true``. +**tags (optional)** + A list of arbitrary strings which will be associated with the job. + Can be used by the parameter-function to alter behavior based on + their presence on a job. If the job name is a regular expression, + tags will accumulate on jobs that match. + **parameter-function (optional)** Specifies a function that should be applied to the parameters before the job is launched. The function should be defined in a python file diff --git a/tests/base.py b/tests/base.py index f3bfa4ea8a..405caa0ded 100755 --- a/tests/base.py +++ b/tests/base.py @@ -620,6 +620,7 @@ class FakeBuild(threading.Thread): BuildHistory(name=self.name, number=self.number, result=result, changes=changes, node=self.node, uuid=self.unique, description=self.description, + parameters=self.parameters, pipeline=self.parameters['ZUUL_PIPELINE']) ) diff --git a/tests/fixtures/layout-tags.yaml b/tests/fixtures/layout-tags.yaml new file mode 100644 index 0000000000..d5b8bf94f4 --- /dev/null +++ b/tests/fixtures/layout-tags.yaml @@ -0,0 +1,42 @@ +includes: + - python-file: tags_custom_functions.py + +pipelines: + - name: check + manager: IndependentPipelineManager + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + verified: 1 + failure: + gerrit: + verified: -1 + +jobs: + - name: ^.*$ + parameter-function: apply_tags + - name: ^.*-merge$ + failure-message: Unable to merge change + hold-following-changes: true + tags: merge + - name: project1-merge + tags: + - project1 + - extratag + +projects: + - name: org/project1 + check: + - project1-merge: + - project1-test1 + - project1-test2 + - project1-project2-integration + + - name: org/project2 + check: + - project2-merge: + - project2-test1 + - project2-test2 + - project1-project2-integration diff --git a/tests/fixtures/layout.yaml b/tests/fixtures/layout.yaml index e8f035e5f4..2e48ff1d0c 100644 --- a/tests/fixtures/layout.yaml +++ b/tests/fixtures/layout.yaml @@ -107,6 +107,7 @@ jobs: - name: ^.*-merge$ failure-message: Unable to merge change hold-following-changes: true + tags: merge - name: nonvoting-project-test2 voting: false - name: project-testfile @@ -120,6 +121,10 @@ jobs: mutex: test-mutex - name: mutex-two mutex: test-mutex + - name: project1-merge + tags: + - project1 + - extratag project-templates: - name: test-one-and-two diff --git a/tests/fixtures/tags_custom_functions.py b/tests/fixtures/tags_custom_functions.py new file mode 100644 index 0000000000..67e7ef1449 --- /dev/null +++ b/tests/fixtures/tags_custom_functions.py @@ -0,0 +1,2 @@ +def apply_tags(item, job, params): + params['BUILD_TAGS'] = ' '.join(sorted(job.tags)) diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 8960e3af75..744a7f9d29 100755 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -2747,6 +2747,25 @@ class TestScheduler(ZuulTestCase): self.assertEqual(B.data['status'], 'MERGED') self.assertEqual(B.reported, 2) + def test_tags(self): + "Test job tags" + self.config.set('zuul', 'layout_config', + 'tests/fixtures/layout-tags.yaml') + self.sched.reconfigure(self.config) + + A = self.fake_gerrit.addFakeChange('org/project1', 'master', 'A') + B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + results = {'project1-merge': 'extratag merge project1', + 'project2-merge': 'merge'} + + for build in self.history: + self.assertEqual(results.get(build.name, ''), + build.parameters.get('BUILD_TAGS')) + def test_timer(self): "Test that a periodic job is triggered" self.worker.hold_jobs_in_build = True diff --git a/zuul/layoutvalidator.py b/zuul/layoutvalidator.py index a01eed3e76..e1e8ac6048 100644 --- a/zuul/layoutvalidator.py +++ b/zuul/layoutvalidator.py @@ -104,6 +104,7 @@ class LayoutSchema(object): 'hold-following-changes': bool, 'voting': bool, 'mutex': str, + 'tags': toList(str), 'parameter-function': str, 'branch': toList(str), 'files': toList(str), diff --git a/zuul/model.py b/zuul/model.py index 75f727dfc3..d2cf13bc7f 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -444,6 +444,7 @@ class Job(object): self.failure_pattern = None self.success_pattern = None self.parameter_function = None + self.tags = set() self.mutex = None # A metajob should only supply values for attributes that have # been explicitly provided, so avoid setting boolean defaults. @@ -493,6 +494,11 @@ class Job(object): self.swift.update(other.swift) if other.mutex: self.mutex = other.mutex + # Tags are merged via a union rather than a destructive copy + # because they are intended to accumulate as metajobs are + # applied. + if other.tags: + self.tags = self.tags.union(other.tags) # Only non-None values should be copied for boolean attributes. if other.hold_following_changes is not None: self.hold_following_changes = other.hold_following_changes diff --git a/zuul/scheduler.py b/zuul/scheduler.py index e1aa0c213b..d44006b729 100644 --- a/zuul/scheduler.py +++ b/zuul/scheduler.py @@ -527,6 +527,13 @@ class Scheduler(threading.Thread): m = config_job.get('mutex', None) if m is not None: job.mutex = m + tags = toList(config_job.get('tags')) + if tags: + # Tags are merged via a union rather than a + # destructive copy because they are intended to + # accumulate onto any previously applied tags from + # metajobs. + job.tags = job.tags.union(set(tags)) fname = config_job.get('parameter-function', None) if fname: func = config_env.get(fname, None)