From 3fe5f850203ba4cbb3503b45c278b87c4ff10604 Mon Sep 17 00:00:00 2001 From: Maru Newby Date: Tue, 13 Jan 2015 04:22:14 +0000 Subject: [PATCH] Add support for a skip-if filter on jobs There was previously no way to skip a job if the contents of a given patchset did not require it to be run. This meant that tempest jobs would run in response to patchsets that would not affect their execution, like those that only modified documentation or unit tests. This change introduces a 'skip-if' configuration option that allows jobs to be configured to not run when specified criteria are met. Change-Id: I9b261f03ec00426160d9396f3a20312e89b624d4 --- doc/source/zuul.rst | 41 ++++++++ tests/base.py | 10 +- tests/fixtures/layout-skip-if.yaml | 29 ++++++ tests/test_change_matcher.py | 154 +++++++++++++++++++++++++++++ tests/test_model.py | 45 +++++++++ tests/test_scheduler.py | 69 ++++++++++++- zuul/change_matcher.py | 132 +++++++++++++++++++++++++ zuul/layoutvalidator.py | 6 ++ zuul/model.py | 6 ++ zuul/scheduler.py | 49 +++++++-- 10 files changed, 531 insertions(+), 10 deletions(-) create mode 100644 tests/fixtures/layout-skip-if.yaml create mode 100644 tests/test_change_matcher.py create mode 100644 tests/test_model.py create mode 100644 zuul/change_matcher.py diff --git a/doc/source/zuul.rst b/doc/source/zuul.rst index 0d63512122..4cc897cf7b 100644 --- a/doc/source/zuul.rst +++ b/doc/source/zuul.rst @@ -800,6 +800,47 @@ each job as it builds a list from the project specification. file patterns listed here. This field is treated as a regular expression and multiple expressions may be listed. +**skip-if (optional)** + + This job should not be run if all the patterns specified by the + optional fields listed below match on their targets. When multiple + sets of parameters are provided, this job will be skipped if any set + matches. For example: :: + + jobs: + - name: check-tempest-dsvm-neutron + skip-if: + - project: ^openstack/neutron$ + branch: ^stable/juno$ + all-files-match-any: + - ^neutron/tests/.*$ + - ^tools/.*$ + - all-files-match-any: + - ^doc/.*$ + - ^.*\.rst$ + + With this configuration, the job would be skipped for a neutron + patchset for the stable/juno branch provided that every file in the + change matched at least one of the specified file regexes. The job + will also be skipped for any patchset that modified only the doc + tree or rst files. + + *project* (optional) + The regular expression to match against the project of the change. + + *branch* (optional) + The regular expression to match against the branch or ref of the + change. + + *all-files-match-any* (optional) + A list of regular expressions intended to match the files involved + in the change. This parameter will be considered matching a + change only if all files in a change match at least one of these + expressions. + + The pattern for '/COMMIT_MSG' is always matched on and does not + have to be included. + **voting (optional)** Boolean value (``true`` or ``false``) that indicates whatever a job is voting or not. Default: ``true``. diff --git a/tests/base.py b/tests/base.py index b872a85810..ec3b74d1dc 100755 --- a/tests/base.py +++ b/tests/base.py @@ -807,11 +807,11 @@ class FakeSwiftClientConnection(swiftclient.client.Connection): return endpoint, '' -class ZuulTestCase(testtools.TestCase): +class BaseTestCase(testtools.TestCase): log = logging.getLogger("zuul.test") def setUp(self): - super(ZuulTestCase, self).setUp() + super(BaseTestCase, self).setUp() test_timeout = os.environ.get('OS_TEST_TIMEOUT', 0) try: test_timeout = int(test_timeout) @@ -835,6 +835,12 @@ class ZuulTestCase(testtools.TestCase): level=logging.DEBUG, format='%(asctime)s %(name)-32s ' '%(levelname)-8s %(message)s')) + + +class ZuulTestCase(BaseTestCase): + + def setUp(self): + super(ZuulTestCase, self).setUp() if USE_TEMPDIR: tmp_root = self.useFixture(fixtures.TempDir( rootdir=os.environ.get("ZUUL_TEST_ROOT")) diff --git a/tests/fixtures/layout-skip-if.yaml b/tests/fixtures/layout-skip-if.yaml new file mode 100644 index 0000000000..0cfb445cd9 --- /dev/null +++ b/tests/fixtures/layout-skip-if.yaml @@ -0,0 +1,29 @@ +pipelines: + - name: check + manager: IndependentPipelineManager + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + verified: 1 + failure: + gerrit: + verified: -1 + + +jobs: + # Defining a metajob will validate that the skip-if attribute of the + # metajob is correctly copied to the job. + - name: ^.*skip-if$ + skip-if: + - project: ^org/project$ + branch: ^master$ + all-files-match-any: + - ^README$ + - name: project-test-skip-if + +projects: + - name: org/project + check: + - project-test-skip-if diff --git a/tests/test_change_matcher.py b/tests/test_change_matcher.py new file mode 100644 index 0000000000..1f4ab93d61 --- /dev/null +++ b/tests/test_change_matcher.py @@ -0,0 +1,154 @@ +# Copyright 2015 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from zuul import change_matcher as cm +from zuul import model + +from tests.base import BaseTestCase + + +class BaseTestMatcher(BaseTestCase): + + project = 'project' + + def setUp(self): + super(BaseTestMatcher, self).setUp() + self.change = model.Change(self.project) + + +class TestAbstractChangeMatcher(BaseTestMatcher): + + def test_str(self): + matcher = cm.ProjectMatcher(self.project) + self.assertEqual(str(matcher), '{ProjectMatcher:project}') + + def test_repr(self): + matcher = cm.ProjectMatcher(self.project) + self.assertEqual(repr(matcher), '') + + +class TestProjectMatcher(BaseTestMatcher): + + def test_matches_returns_true(self): + matcher = cm.ProjectMatcher(self.project) + self.assertTrue(matcher.matches(self.change)) + + def test_matches_returns_false(self): + matcher = cm.ProjectMatcher('not_project') + self.assertFalse(matcher.matches(self.change)) + + +class TestBranchMatcher(BaseTestMatcher): + + def setUp(self): + super(TestBranchMatcher, self).setUp() + self.matcher = cm.BranchMatcher('foo') + + def test_matches_returns_true_on_matching_branch(self): + self.change.branch = 'foo' + self.assertTrue(self.matcher.matches(self.change)) + + def test_matches_returns_true_on_matching_ref(self): + self.change.branch = 'bar' + self.change.ref = 'foo' + self.assertTrue(self.matcher.matches(self.change)) + + def test_matches_returns_false_for_no_match(self): + self.change.branch = 'bar' + self.change.ref = 'baz' + self.assertFalse(self.matcher.matches(self.change)) + + def test_matches_returns_false_for_missing_attrs(self): + delattr(self.change, 'branch') + # ref is by default not an attribute + self.assertFalse(self.matcher.matches(self.change)) + + +class TestFileMatcher(BaseTestMatcher): + + def setUp(self): + super(TestFileMatcher, self).setUp() + self.matcher = cm.FileMatcher('filename') + + def test_matches_returns_true(self): + self.change.files = ['filename'] + self.assertTrue(self.matcher.matches(self.change)) + + def test_matches_returns_false_when_no_files(self): + self.assertFalse(self.matcher.matches(self.change)) + + def test_matches_returns_false_when_files_attr_missing(self): + delattr(self.change, 'files') + self.assertFalse(self.matcher.matches(self.change)) + + +class TestAbstractMatcherCollection(BaseTestMatcher): + + def test_str(self): + matcher = cm.MatchAll([cm.FileMatcher('foo')]) + self.assertEqual(str(matcher), '{MatchAll:{FileMatcher:foo}}') + + def test_repr(self): + matcher = cm.MatchAll([]) + self.assertEqual(repr(matcher), '') + + +class TestMatchAllFiles(BaseTestMatcher): + + def setUp(self): + super(TestMatchAllFiles, self).setUp() + self.matcher = cm.MatchAllFiles([cm.FileMatcher('^docs/.*$')]) + + def _test_matches(self, expected, files=None): + if files is not None: + self.change.files = files + self.assertEqual(expected, self.matcher.matches(self.change)) + + def test_matches_returns_false_when_files_attr_missing(self): + delattr(self.change, 'files') + self._test_matches(False) + + def test_matches_returns_false_when_no_files(self): + self._test_matches(False) + + def test_matches_returns_false_when_not_all_files_match(self): + self._test_matches(False, files=['docs/foo', 'foo/bar']) + + def test_matches_returns_true_when_commit_message_matches(self): + self._test_matches(True, files=['/COMMIT_MSG']) + + def test_matches_returns_true_when_all_files_match(self): + self._test_matches(True, files=['docs/foo']) + + +class TestMatchAll(BaseTestMatcher): + + def test_matches_returns_true(self): + matcher = cm.MatchAll([cm.ProjectMatcher(self.project)]) + self.assertTrue(matcher.matches(self.change)) + + def test_matches_returns_false_for_missing_matcher(self): + matcher = cm.MatchAll([cm.ProjectMatcher('not_project')]) + self.assertFalse(matcher.matches(self.change)) + + +class TestMatchAny(BaseTestMatcher): + + def test_matches_returns_true(self): + matcher = cm.MatchAny([cm.ProjectMatcher(self.project)]) + self.assertTrue(matcher.matches(self.change)) + + def test_matches_returns_false(self): + matcher = cm.MatchAny([cm.ProjectMatcher('not_project')]) + self.assertFalse(matcher.matches(self.change)) diff --git a/tests/test_model.py b/tests/test_model.py new file mode 100644 index 0000000000..a97f0a073f --- /dev/null +++ b/tests/test_model.py @@ -0,0 +1,45 @@ +# Copyright 2015 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from zuul import change_matcher as cm +from zuul import model + +from tests.base import BaseTestCase + + +class TestJob(BaseTestCase): + + @property + def job(self): + job = model.Job('job') + job.skip_if_matcher = cm.MatchAll([ + cm.ProjectMatcher('^project$'), + cm.MatchAllFiles([cm.FileMatcher('^docs/.*$')]), + ]) + return job + + def test_change_matches_returns_false_for_matched_skip_if(self): + change = model.Change('project') + change.files = ['docs/foo'] + self.assertFalse(self.job.changeMatches(change)) + + def test_change_matches_returns_true_for_unmatched_skip_if(self): + change = model.Change('project') + change.files = ['foo'] + self.assertTrue(self.job.changeMatches(change)) + + def test_copy_retains_skip_if(self): + job = model.Job('job') + job.copy(self.job) + self.assertTrue(job.skip_if_matcher) diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 89056f4cd6..a4e47befce 100755 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -22,22 +22,62 @@ import shutil import time import urllib import urllib2 +import yaml import git import testtools +import zuul.change_matcher import zuul.scheduler import zuul.rpcclient import zuul.reporter.gerrit import zuul.reporter.smtp -from tests.base import ZuulTestCase, repack_repo +from tests.base import ( + BaseTestCase, + ZuulTestCase, + repack_repo, +) logging.basicConfig(level=logging.DEBUG, format='%(asctime)s %(name)-32s ' '%(levelname)-8s %(message)s') +class TestSchedulerConfigParsing(BaseTestCase): + + def test_parse_skip_if(self): + job_yaml = """ +jobs: + - name: job_name + skip-if: + - project: ^project_name$ + branch: ^stable/icehouse$ + all-files-match-any: + - ^filename$ + - project: ^project2_name$ + all-files-match-any: + - ^filename2$ + """.strip() + data = yaml.load(job_yaml) + config_job = data.get('jobs')[0] + sched = zuul.scheduler.Scheduler() + cm = zuul.change_matcher + expected = cm.MatchAny([ + cm.MatchAll([ + cm.ProjectMatcher('^project_name$'), + cm.BranchMatcher('^stable/icehouse$'), + cm.MatchAllFiles([cm.FileMatcher('^filename$')]), + ]), + cm.MatchAll([ + cm.ProjectMatcher('^project2_name$'), + cm.MatchAllFiles([cm.FileMatcher('^filename2$')]), + ]), + ]) + matcher = sched._parseSkipIf(config_job) + self.assertEqual(expected, matcher) + + class TestScheduler(ZuulTestCase): def test_jobs_launched(self): @@ -1737,6 +1777,33 @@ class TestScheduler(ZuulTestCase): self.assertEqual(B.data['status'], 'MERGED') self.assertEqual(B.reported, 2) + def _test_skip_if_jobs(self, branch, should_skip): + "Test that jobs with a skip-if filter run only when appropriate" + self.config.set('zuul', 'layout_config', + 'tests/fixtures/layout-skip-if.yaml') + self.sched.reconfigure(self.config) + self.registerJobs() + + change = self.fake_gerrit.addFakeChange('org/project', + branch, + 'test skip-if') + self.fake_gerrit.addEvent(change.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + tested_change_ids = [x.changes[0] for x in self.history + if x.name == 'project-test-skip-if'] + + if should_skip: + self.assertEqual([], tested_change_ids) + else: + self.assertIn(change.data['number'], tested_change_ids) + + def test_skip_if_match_skips_job(self): + self._test_skip_if_jobs(branch='master', should_skip=True) + + def test_skip_if_no_match_runs_job(self): + self._test_skip_if_jobs(branch='mp', should_skip=False) + def test_test_config(self): "Test that we can test the config" sched = zuul.scheduler.Scheduler() diff --git a/zuul/change_matcher.py b/zuul/change_matcher.py new file mode 100644 index 0000000000..ed380f0ae5 --- /dev/null +++ b/zuul/change_matcher.py @@ -0,0 +1,132 @@ +# Copyright 2015 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +""" +This module defines classes used in matching changes based on job +configuration. +""" + +import re + + +class AbstractChangeMatcher(object): + + def __init__(self, regex): + self._regex = regex + self.regex = re.compile(regex) + + def matches(self, change): + """Return a boolean indication of whether change matches + implementation-specific criteria. + """ + raise NotImplementedError() + + def copy(self): + return self.__class__(self._regex) + + def __eq__(self, other): + return str(self) == str(other) + + def __str__(self): + return '{%s:%s}' % (self.__class__.__name__, self._regex) + + def __repr__(self): + return '<%s %s>' % (self.__class__.__name__, self._regex) + + +class ProjectMatcher(AbstractChangeMatcher): + + def matches(self, change): + return self.regex.match(str(change.project)) + + +class BranchMatcher(AbstractChangeMatcher): + + def matches(self, change): + return ( + (hasattr(change, 'branch') and self.regex.match(change.branch)) or + (hasattr(change, 'ref') and self.regex.match(change.ref)) + ) + + +class FileMatcher(AbstractChangeMatcher): + + def matches(self, change): + if not hasattr(change, 'files'): + return False + for file_ in change.files: + if self.regex.match(file_): + return True + return False + + +class AbstractMatcherCollection(AbstractChangeMatcher): + + def __init__(self, matchers): + self.matchers = matchers + + def __eq__(self, other): + return str(self) == str(other) + + def __str__(self): + return '{%s:%s}' % (self.__class__.__name__, + ','.join([str(x) for x in self.matchers])) + + def __repr__(self): + return '<%s>' % self.__class__.__name__ + + def copy(self): + return self.__class__(self.matchers[:]) + + +class MatchAllFiles(AbstractMatcherCollection): + + commit_regex = re.compile('^/COMMIT_MSG$') + + @property + def regexes(self): + for matcher in self.matchers: + yield matcher.regex + yield self.commit_regex + + def matches(self, change): + if not (hasattr(change, 'files') and change.files): + return False + for file_ in change.files: + matched_file = False + for regex in self.regexes: + if regex.match(file_): + matched_file = True + break + if not matched_file: + return False + return True + + +class MatchAll(AbstractMatcherCollection): + + def matches(self, change): + for matcher in self.matchers: + if not matcher.matches(change): + return False + return True + + +class MatchAny(AbstractMatcherCollection): + + def matches(self, change): + for matcher in self.matchers: + if matcher.matches(change): + return True + return False diff --git a/zuul/layoutvalidator.py b/zuul/layoutvalidator.py index f71c853f43..68abbf5065 100644 --- a/zuul/layoutvalidator.py +++ b/zuul/layoutvalidator.py @@ -134,6 +134,11 @@ class LayoutSchema(object): 'logserver-prefix': str, } + skip_if = {'project': str, + 'branch': str, + 'all-files-match-any': toList(str), + } + job = {v.Required('name'): str, 'queue-name': str, 'failure-message': str, @@ -146,6 +151,7 @@ class LayoutSchema(object): 'branch': toList(str), 'files': toList(str), 'swift': toList(swift), + 'skip-if': toList(skip_if), } jobs = [job] diff --git a/zuul/model.py b/zuul/model.py index b705982bf2..5ac6f530d9 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -451,6 +451,7 @@ class Job(object): self._branches = [] self.files = [] self._files = [] + self.skip_if_matcher = None self.swift = {} def __str__(self): @@ -476,6 +477,8 @@ class Job(object): if other.files: self.files = other.files[:] self._files = other._files[:] + if other.skip_if_matcher: + self.skip_if_matcher = other.skip_if_matcher.copy() if other.swift: self.swift.update(other.swift) self.hold_following_changes = other.hold_following_changes @@ -500,6 +503,9 @@ class Job(object): if self.files and not matches_file: return False + if self.skip_if_matcher and self.skip_if_matcher.matches(change): + return False + return True diff --git a/zuul/scheduler.py b/zuul/scheduler.py index 7d41fb182d..fc21bf10d2 100644 --- a/zuul/scheduler.py +++ b/zuul/scheduler.py @@ -31,6 +31,7 @@ import layoutvalidator import model from model import ActionReporter, Pipeline, Project, ChangeQueue from model import EventFilter, ChangeishFilter +from zuul import change_matcher from zuul import version as zuul_version statsd = extras.try_import('statsd.statsd') @@ -166,6 +167,14 @@ class MergeCompletedEvent(ResultEvent): self.commit = commit +def toList(item): + if not item: + return [] + if isinstance(item, list): + return item + return [item] + + class Scheduler(threading.Thread): log = logging.getLogger("zuul.Scheduler") @@ -199,17 +208,38 @@ class Scheduler(threading.Thread): def testConfig(self, config_path): return self._parseConfig(config_path) + def _parseSkipIf(self, config_job): + cm = change_matcher + skip_matchers = [] + + for config_skip in config_job.get('skip-if', []): + nested_matchers = [] + + project_regex = config_skip.get('project') + if project_regex: + nested_matchers.append(cm.ProjectMatcher(project_regex)) + + branch_regex = config_skip.get('branch') + if branch_regex: + nested_matchers.append(cm.BranchMatcher(branch_regex)) + + file_regexes = toList(config_skip.get('all-files-match-any')) + if file_regexes: + file_matchers = [cm.FileMatcher(x) for x in file_regexes] + all_files_matcher = cm.MatchAllFiles(file_matchers) + nested_matchers.append(all_files_matcher) + + # All patterns need to match a given skip-if predicate + skip_matchers.append(cm.MatchAll(nested_matchers)) + + if skip_matchers: + # Any skip-if predicate can be matched to trigger a skip + return cm.MatchAny(skip_matchers) + def _parseConfig(self, config_path): layout = model.Layout() project_templates = {} - def toList(item): - if not item: - return [] - if isinstance(item, list): - return item - return [item] - if config_path: config_path = os.path.expanduser(config_path) if not os.path.exists(config_path): @@ -395,6 +425,9 @@ class Scheduler(threading.Thread): if files: job._files = files job.files = [re.compile(x) for x in files] + skip_if_matcher = self._parseSkipIf(config_job) + if skip_if_matcher: + job.skip_if_matcher = skip_if_matcher swift = toList(config_job.get('swift')) if swift: for s in swift: @@ -972,6 +1005,8 @@ class BasePipelineManager(object): efilters += str(b) for f in tree.job._files: efilters += str(f) + if tree.job.skip_if_matcher: + efilters += str(tree.job.skip_if_matcher) if efilters: efilters = ' ' + efilters hold = ''