diff --git a/doc/source/user/config.rst b/doc/source/user/config.rst index 49b5e7696c..161e74ec69 100644 --- a/doc/source/user/config.rst +++ b/doc/source/user/config.rst @@ -1070,6 +1070,27 @@ Here is an example of two job definitions: completed successfully, and if one or more of them fail, this job will not be run. + The format for this attribute is either a list of strings or + dictionaries. Strings are interpreted as job names, + dictionaries, if used, may have the following attributes: + + .. attr:: name + :required: + + The name of the required job. + + .. attr:: soft + :default: false + + A boolean value which indicates whether this job is a *hard* + or *soft* dependency. A *hard* dependency will cause an + error if the specified job is not run. That is, if job B + depends on job A, but job A is not run for any reason (for + example, it containes a file matcher which does not match), + then Zuul will not run any jobs and report an error. A + *soft* dependency will simply be ignored if the dependent job + is not run. + .. attr:: allowed-projects A list of Zuul projects which may use this job. By default, a diff --git a/releasenotes/notes/soft-dependencies-08b02bf3133a6c57.yaml b/releasenotes/notes/soft-dependencies-08b02bf3133a6c57.yaml new file mode 100644 index 0000000000..0c707aa124 --- /dev/null +++ b/releasenotes/notes/soft-dependencies-08b02bf3133a6c57.yaml @@ -0,0 +1,8 @@ +--- +features: + - The :attr:`job.dependencies` attribute may now be used to express + "soft" dependencies -- that is, to indicate a job should run + after another completes, but only if it runs at all. For example, + a deployment job which should always run, but depends on a build + job which only runs if the source code is changed. + diff --git a/tests/fixtures/layouts/soft-dependencies-error.yaml b/tests/fixtures/layouts/soft-dependencies-error.yaml new file mode 100644 index 0000000000..dc5df41c33 --- /dev/null +++ b/tests/fixtures/layouts/soft-dependencies-error.yaml @@ -0,0 +1,34 @@ +- pipeline: + name: check + manager: independent + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + Verified: 1 + failure: + gerrit: + Verified: -1 + +- job: + name: base + parent: null + run: playbooks/base.yaml + +- job: + name: build + files: main.c + +- job: + name: deploy + +- project: + name: org/project + check: + jobs: + - build + - deploy: + dependencies: + - name: project-merge + soft: true diff --git a/tests/fixtures/layouts/soft-dependencies.yaml b/tests/fixtures/layouts/soft-dependencies.yaml new file mode 100644 index 0000000000..714e5794db --- /dev/null +++ b/tests/fixtures/layouts/soft-dependencies.yaml @@ -0,0 +1,34 @@ +- pipeline: + name: check + manager: independent + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + Verified: 1 + failure: + gerrit: + Verified: -1 + +- job: + name: base + parent: null + run: playbooks/base.yaml + +- job: + name: build + files: main.c + +- job: + name: deploy + +- project: + name: org/project + check: + jobs: + - build + - deploy: + dependencies: + - name: build + soft: true diff --git a/tests/unit/test_model.py b/tests/unit/test_model.py index fccd8d9602..1726259aa7 100644 --- a/tests/unit/test_model.py +++ b/tests/unit/test_model.py @@ -441,7 +441,8 @@ class TestGraph(BaseTestCase): prevjob = None for j in jobs[:3]: if prevjob: - j.dependencies = frozenset([prevjob.name]) + j.dependencies = frozenset([ + model.JobDependency(prevjob.name)]) graph.addJob(j) prevjob = j # 0 triggers 1 triggers 2 triggers 3... @@ -451,32 +452,95 @@ class TestGraph(BaseTestCase): Exception, "Dependency cycle detected in job jobX"): j = model.Job('jobX') - j.dependencies = frozenset([j.name]) + j.dependencies = frozenset([model.JobDependency(j.name)]) graph.addJob(j) # Disallow circular dependencies with testtools.ExpectedException( Exception, "Dependency cycle detected in job job3"): - jobs[4].dependencies = frozenset([jobs[3].name]) + jobs[4].dependencies = frozenset([ + model.JobDependency(jobs[3].name)]) graph.addJob(jobs[4]) - jobs[3].dependencies = frozenset([jobs[4].name]) + jobs[3].dependencies = frozenset([ + model.JobDependency(jobs[4].name)]) graph.addJob(jobs[3]) - jobs[5].dependencies = frozenset([jobs[4].name]) + jobs[5].dependencies = frozenset([model.JobDependency(jobs[4].name)]) graph.addJob(jobs[5]) with testtools.ExpectedException( Exception, "Dependency cycle detected in job job3"): - jobs[3].dependencies = frozenset([jobs[5].name]) + jobs[3].dependencies = frozenset([ + model.JobDependency(jobs[5].name)]) graph.addJob(jobs[3]) - jobs[3].dependencies = frozenset([jobs[2].name]) + jobs[3].dependencies = frozenset([ + model.JobDependency(jobs[2].name)]) graph.addJob(jobs[3]) - jobs[6].dependencies = frozenset([jobs[2].name]) + jobs[6].dependencies = frozenset([ + model.JobDependency(jobs[2].name)]) graph.addJob(jobs[6]) + def test_job_graph_allows_soft_dependencies(self): + parent = model.Job('parent') + child = model.Job('child') + child.dependencies = frozenset([ + model.JobDependency(parent.name, True)]) + + # With the parent + graph = model.JobGraph() + graph.addJob(parent) + graph.addJob(child) + self.assertEqual(graph.getParentJobsRecursively(child.name), + [parent]) + + # Skip the parent + graph = model.JobGraph() + graph.addJob(child) + self.assertEqual(graph.getParentJobsRecursively(child.name), []) + + def test_job_graph_allows_soft_dependencies4(self): + # A more complex scenario with multiple parents at each level + parents = [model.Job('parent%i' % i) for i in range(6)] + child = model.Job('child') + child.dependencies = frozenset([ + model.JobDependency(parents[0].name, True), + model.JobDependency(parents[1].name)]) + parents[0].dependencies = frozenset([ + model.JobDependency(parents[2].name), + model.JobDependency(parents[3].name, True)]) + parents[1].dependencies = frozenset([ + model.JobDependency(parents[4].name), + model.JobDependency(parents[5].name)]) + # Run them all + graph = model.JobGraph() + for j in parents: + graph.addJob(j) + graph.addJob(child) + self.assertEqual(set(graph.getParentJobsRecursively(child.name)), + set(parents)) + + # Skip first parent, therefore its recursive dependencies don't appear + graph = model.JobGraph() + for j in parents: + if j is not parents[0]: + graph.addJob(j) + graph.addJob(child) + self.assertEqual(set(graph.getParentJobsRecursively(child.name)), + set(parents) - + set([parents[0], parents[2], parents[3]])) + + # Skip a leaf node + graph = model.JobGraph() + for j in parents: + if j is not parents[3]: + graph.addJob(j) + graph.addJob(child) + self.assertEqual(set(graph.getParentJobsRecursively(child.name)), + set(parents) - set([parents[3]])) + class TestTenant(BaseTestCase): def test_add_project(self): diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index 3611cde34b..c0ec1ceb98 100644 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -5673,6 +5673,25 @@ class TestDependencyGraph(ZuulTestCase): self.assertEqual(change.data['status'], 'NEW') self.assertEqual(change.reported, 2) + @simple_layout('layouts/soft-dependencies-error.yaml') + def test_soft_dependencies_error(self): + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + self.assertHistory([]) + self.assertEqual(len(A.messages), 1) + self.assertTrue('Job project-merge not defined' in A.messages[0]) + print(A.messages) + + @simple_layout('layouts/soft-dependencies.yaml') + def test_soft_dependencies(self): + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + self.assertHistory([ + dict(name='deploy', result='SUCCESS', changes='1,1'), + ], ordered=False) + class TestDuplicatePipeline(ZuulTestCase): tenant_config_file = 'config/duplicate-pipeline/main.yaml' diff --git a/tests/unit/test_web.py b/tests/unit/test_web.py index f2e33c6de7..1d96b80c47 100755 --- a/tests/unit/test_web.py +++ b/tests/unit/test_web.py @@ -517,7 +517,8 @@ class TestWeb(BaseTestWeb): [{'abstract': False, 'attempts': 3, 'branches': [], - 'dependencies': ['project-merge'], + 'dependencies': [{'name': 'project-merge', + 'soft': False}], 'description': None, 'files': [], 'final': False, @@ -547,7 +548,8 @@ class TestWeb(BaseTestWeb): [{'abstract': False, 'attempts': 3, 'branches': [], - 'dependencies': ['project-merge'], + 'dependencies': [{'name': 'project-merge', + 'soft': False}], 'description': None, 'files': [], 'final': False, @@ -577,7 +579,8 @@ class TestWeb(BaseTestWeb): [{'abstract': False, 'attempts': 3, 'branches': [], - 'dependencies': ['project-merge'], + 'dependencies': [{'name': 'project-merge', + 'soft': False}], 'description': None, 'files': [], 'final': False, diff --git a/zuul/configloader.py b/zuul/configloader.py index e3d51861b0..41d3d26206 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -533,6 +533,9 @@ class JobParser(object): 'override-branch': str, 'override-checkout': str} + job_dependency = {vs.Required('name'): str, + 'soft': bool} + secret = {vs.Required('name'): str, vs.Required('secret'): str, 'pass-to-parent': bool} @@ -575,7 +578,7 @@ class JobParser(object): 'extra-vars': dict, 'host-vars': {str: dict}, 'group-vars': {str: dict}, - 'dependencies': to_list(str), + 'dependencies': to_list(vs.Any(job_dependency, str)), 'allowed-projects': to_list(str), 'override-branch': str, 'override-checkout': str, @@ -764,6 +767,20 @@ class JobParser(object): new_projects[project.canonical_name] = job_project job.required_projects = new_projects + if 'dependencies' in conf: + new_dependencies = [] + dependencies = as_list(conf.get('dependencies', [])) + for dep in dependencies: + if isinstance(dep, dict): + dep_name = dep['name'] + dep_soft = dep.get('soft', False) + else: + dep_name = dep + dep_soft = False + job_dependency = model.JobDependency(dep_name, dep_soft) + new_dependencies.append(job_dependency) + job.dependencies = new_dependencies + if 'semaphore' in conf: semaphore = conf.get('semaphore') if isinstance(semaphore, str): @@ -773,7 +790,7 @@ class JobParser(object): semaphore.get('name'), semaphore.get('resources-first', False)) - for k in ('tags', 'requires', 'provides', 'dependencies'): + for k in ('tags', 'requires', 'provides'): v = frozenset(as_list(conf.get(k))) if v: setattr(job, k, v) diff --git a/zuul/model.py b/zuul/model.py index 642f71b7c2..4211c0e163 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -1213,7 +1213,7 @@ class Job(ConfigObject): d['tags'] = list(self.tags) d['provides'] = list(self.provides) d['requires'] = list(self.requires) - d['dependencies'] = list(self.dependencies) + d['dependencies'] = list(map(lambda x: x.toDict(), self.dependencies)) d['attempts'] = self.attempts d['roles'] = list(map(lambda x: x.toDict(), self.roles)) d['run'] = list(map(lambda x: x.toSchemaDict(), self.run)) @@ -1649,12 +1649,25 @@ class JobList(ConfigObject): joblist.append(job) +class JobDependency(ConfigObject): + """ A reference to another job in the project-pipeline-config. """ + def __init__(self, name, soft=False): + super(JobDependency, self).__init__() + self.name = name + self.soft = soft + + def toDict(self): + return {'name': self.name, + 'soft': self.soft} + + class JobGraph(object): """ A JobGraph represents the dependency graph between Job.""" def __init__(self): self.jobs = OrderedDict() # job_name -> Job - self._dependencies = {} # dependent_job_name -> set(parent_job_names) + # dependent_job_name -> dict(parent_job_name -> soft) + self._dependencies = {} def __repr__(self): return '' % (self.jobs) @@ -1666,17 +1679,18 @@ class JobGraph(object): raise Exception("Job %s already added" % (job.name,)) self.jobs[job.name] = job # Append the dependency information - self._dependencies.setdefault(job.name, set()) + self._dependencies.setdefault(job.name, {}) try: for dependency in job.dependencies: # Make sure a circular dependency is never created ancestor_jobs = self._getParentJobNamesRecursively( - dependency, soft=True) - ancestor_jobs.add(dependency) + dependency.name, soft=True) + ancestor_jobs.add(dependency.name) if any((job.name == anc_job) for anc_job in ancestor_jobs): raise Exception("Dependency cycle detected in job %s" % (job.name,)) - self._dependencies[job.name].add(dependency) + self._dependencies[job.name][dependency.name] = \ + dependency.soft except Exception: del self.jobs[job.name] del self._dependencies[job.name] @@ -1703,25 +1717,34 @@ class JobGraph(object): all_dependent_jobs |= new_dependent_jobs return [self.jobs[name] for name in all_dependent_jobs] - def getParentJobsRecursively(self, dependent_job, soft=False): + def getParentJobsRecursively(self, dependent_job, layout=None): return [self.jobs[name] for name in - self._getParentJobNamesRecursively(dependent_job, soft)] + self._getParentJobNamesRecursively(dependent_job, + layout=layout)] - def _getParentJobNamesRecursively(self, dependent_job, soft=False): + def _getParentJobNamesRecursively(self, dependent_job, soft=False, + layout=None): all_parent_jobs = set() - jobs_to_iterate = set([dependent_job]) + jobs_to_iterate = set([(dependent_job, False)]) while len(jobs_to_iterate) > 0: - current_job = jobs_to_iterate.pop() + (current_job, current_soft) = jobs_to_iterate.pop() current_parent_jobs = self._dependencies.get(current_job) if current_parent_jobs is None: - if soft: - current_parent_jobs = set() + if soft or current_soft: + if layout: + # If the caller supplied a layout, verify that + # the job exists to provide a helpful error + # message. Called for exception side effect: + layout.getJob(current_job) + current_parent_jobs = {} else: raise Exception("Job %s depends on %s which was not run." % (dependent_job, current_job)) - new_parent_jobs = current_parent_jobs - all_parent_jobs - jobs_to_iterate |= new_parent_jobs - all_parent_jobs |= new_parent_jobs + elif dependent_job != current_job: + all_parent_jobs.add(current_job) + new_parent_jobs = set(current_parent_jobs.keys()) - all_parent_jobs + for j in new_parent_jobs: + jobs_to_iterate.add((j, current_parent_jobs[j])) return all_parent_jobs @@ -2066,7 +2089,7 @@ class QueueItem(object): for job in job_graph.getJobs(): # Ensure that each jobs's dependencies are fully # accessible. This will raise an exception if not. - job_graph.getParentJobsRecursively(job.name) + job_graph.getParentJobsRecursively(job.name, self.layout) self.job_graph = job_graph except Exception: self.project_pipeline_config = None @@ -2645,7 +2668,7 @@ class QueueItem(object): ret['jobs'].append({ 'name': job.name, - 'dependencies': list(job.dependencies), + 'dependencies': [x.name for x in job.dependencies], 'elapsed_time': elapsed, 'remaining_time': remaining, 'url': build_url,