Allow soft job dependencies

A "soft" dependency can be used to indicate that a job must run
after another completes, but only if it runs at all.  For example,
a deployment job which depends on a build job with different file
matcher criteria.

Change-Id: I4d7fc2b40942569323da273c4529fdb365a3b11a

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

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.
+

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

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

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):

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'

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,

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)

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 '<JobGraph %s>' % (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)
+                    dependency.name, soft=True)
-                ancestor_jobs.add(dependency)
+                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:
+                if soft or current_soft:
-                    current_parent_jobs = set()
+                    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
+            elif dependent_job != current_job:
-            jobs_to_iterate |= new_parent_jobs
+                all_parent_jobs.add(current_job)
-            all_parent_jobs |= new_parent_jobs
+            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,