Fix skipping child jobs with soft dependencies

Change I1fe06d7a889dbccf86116a310701b2755e5fb385 updated the model
to handle the case where zuul_return is used to filter the child
jobs when those jobs have soft dependencies.  In short, a filtered
job should no longer be counted as a dependency when its child job
runs (as if the filtered job had not matched the change in the first
place) which makes the behavior consistent with soft dependencies
of jobs which don't match.

However, it also changed the behavior for when a job fails.  This
seems to be a simple oversight in applying the new "skip_soft"
pattern in too many places.  When a job actually fails, we do not
want its child jobs to run, regardless of whether they have a hard
or soft dependency on the failing parent.  In that case, we should
always set them to 'skipped'.  Otherwise they will never run and
the item will sit in the queue indefinitely.

Change-Id: Ic19a73d1b6e0fbe5510e71947930de4c46cc1280

releasenotes/notes/fix-soft-deps-1e809c40790892e7.yaml

@@ -0,0 +1,3 @@
+fixes:
+  - Fixed a regression where changes with a failing job with soft
+    dependencies would remain in the queue indefinitely.

tests/unit/test_scheduler.py

@@ -6347,6 +6347,19 @@ class TestDependencyGraph(ZuulTestCase):
             dict(name='deploy', result='SUCCESS', changes='1,1'),
         ], ordered=False)
 
+    @simple_layout('layouts/soft-dependencies.yaml')
+    def test_soft_dependencies_failure(self):
+        file_dict = {'main.c': 'test'}
+        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
+                                           files=file_dict)
+        self.executor_server.failJob('build', A)
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+        self.assertHistory([
+            dict(name='build', result='FAILURE', changes='1,1'),
+        ], ordered=False)
+        self.assertIn('SKIPPED', A.messages[0])
+
 
 class TestDuplicatePipeline(ZuulTestCase):
     tenant_config_file = 'config/duplicate-pipeline/main.yaml'

zuul/model.py

@@ -2622,13 +2622,13 @@ class QueueItem(object):
                 build.job.name)
 
             if not zuul_return:
-                # If zuul.child_jobs exists and is empty, user want to skip all
-                # child jobs.
+                # If zuul.child_jobs exists and is empty, the user
+                # wants to skip all child jobs.
                 to_skip = self.job_graph.getDependentJobsRecursively(
                     build.job.name, skip_soft=True)
                 skipped += to_skip
             else:
-                # We have list of jobs to run.
+                # The user supplied a list of jobs to run.
                 intersect_jobs = dependent_jobs.intersection(zuul_return)
 
                 for skip in (dependent_jobs - intersect_jobs):
@@ -2640,7 +2640,7 @@ class QueueItem(object):
 
         elif build.result != 'SUCCESS' and not build.paused:
             to_skip = self.job_graph.getDependentJobsRecursively(
-                build.job.name, skip_soft=True)
+                build.job.name)
             skipped += to_skip
 
         for job in skipped: