Merge "Include skip reason in build error_detail"

4 files changed, 43 insertions, 18 deletions

diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py
index 92e216fcf..6269d3d1c 100644
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -5052,6 +5052,17 @@ class TestDataReturn(AnsibleZuulTestCase):
             A.messages[-1])
         self.assertTrue('Skipped 1 job' in A.messages[-1])
         self.assertIn('Build succeeded', A.messages[-1])
+        connection = self.scheds.first.sched.sql.connection
+        builds = connection.getBuilds()
+        builds.sort(key=lambda x: x.job_name)
+        self.assertEqual(builds[0].job_name, 'child')
+        self.assertEqual(builds[0].error_detail,
+                         'Skipped due to child_jobs return value '
+                         'in job data-return-child-jobs')
+        self.assertEqual(builds[1].job_name, 'data-return')
+        self.assertIsNone(builds[1].error_detail)
+        self.assertEqual(builds[2].job_name, 'data-return-child-jobs')
+        self.assertIsNone(builds[2].error_detail)
 
     def test_data_return_invalid_child_job(self):
         A = self.fake_gerrit.addFakeChange('org/project2', 'master', 'A')
@@ -7918,12 +7929,12 @@ class TestProvidesRequiresMysql(ZuulTestCase):
         self.assertEqual(
             B.messages[0].count(
                 'Job image-user requires artifact(s) images'),
-            1,
+            2,
             B.messages[0])
         self.assertEqual(
             B.messages[0].count(
                 'Job library-user requires artifact(s) libraries'),
-            1,
+            2,
             B.messages[0])
 
     @simple_layout('layouts/provides-requires-single-project.yaml')
@@ -7958,7 +7969,7 @@ class TestProvidesRequiresMysql(ZuulTestCase):
         self.assertEqual(
             B.messages[0].count(
                 'Job image-user requires artifact(s) images'),
-            1, B.messages[0])
+            2, B.messages[0])
 
 
 class TestProvidesRequiresPostgres(TestProvidesRequiresMysql):
diff --git a/tests/unit/test_web.py b/tests/unit/test_web.py
index 02a1d76d1..ba495f59e 100644
--- a/tests/unit/test_web.py
+++ b/tests/unit/test_web.py
@@ -1804,6 +1804,11 @@ class TestBuildInfo(BaseTestWeb):
         self.assertEqual(builds[0]['result'], 'FAILURE')
         self.assertEqual(builds[1]['result'], 'SKIPPED')
         self.assertEqual(builds[2]['result'], 'SKIPPED')
+        self.assertIsNone(builds[0]['error_detail'])
+        self.assertEqual(builds[1]['error_detail'],
+                         'Skipped due to failed job project-merge')
+        self.assertEqual(builds[2]['error_detail'],
+                         'Skipped due to failed job project-merge')
 
         builds = self.get_url("api/tenant/tenant-one/builds?"
                               "exclude_result=SKIPPED").json()
diff --git a/zuul/manager/__init__.py b/zuul/manager/__init__.py
index 1e5d21eea..480a3f9ca 100644
--- a/zuul/manager/__init__.py
+++ b/zuul/manager/__init__.py
@@ -480,7 +480,7 @@ class PipelineManager(metaclass=ABCMeta):
                 if item.current_build_set.config_errors:
                     item.setConfigErrors(item.getConfigErrors())
                 if item.dequeued_needing_change:
-                    item.setDequeuedNeedingChange()
+                    item.setDequeuedNeedingChange(item.dequeued_needing_change)
                 if item.dequeued_missing_requirements:
                     item.setDequeuedMissingRequirements()
 
@@ -1510,7 +1510,7 @@ class PipelineManager(metaclass=ABCMeta):
                      "it can no longer merge" % item.change)
             self.cancelJobs(item)
             if item.isBundleFailing():
-                item.setDequeuedBundleFailing()
+                item.setDequeuedBundleFailing('Bundle is failing')
             elif not meets_reqs:
                 item.setDequeuedMissingRequirements()
             else:
@@ -1967,7 +1967,8 @@ class PipelineManager(metaclass=ABCMeta):
             build_set.jobNodeRequestComplete(request.job_name, nodeset)
         # Put a fake build through the cycle to clean it up.
         if not request.fulfilled:
-            build_set.item.setNodeRequestFailure(job)
+            build_set.item.setNodeRequestFailure(
+                job, f'Node request {request.id} failed')
             self._resumeBuilds(build_set)
             tenant = build_set.item.pipeline.tenant
             tenant.semaphore_handler.release(
diff --git a/zuul/model.py b/zuul/model.py
index 65140a4ae..1a7b1b79c 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -4927,7 +4927,7 @@ class QueueItem(zkobject.ZKObject):
             self.warning(str(e))
             fakebuild = Build.new(self.pipeline.manager.current_context,
                                   job=job, build_set=self.current_build_set,
-                                  result='FAILURE')
+                                  error_detail=str(e), result='FAILURE')
             self.addBuild(fakebuild)
             self.pipeline.manager.sql.reportBuildEnd(
                 fakebuild,
@@ -5227,11 +5227,16 @@ class QueueItem(zkobject.ZKObject):
         buildset = self.current_build_set
         job_graph = self.current_build_set.job_graph
         skipped = []
+        # We may skip several jobs, but if we do, they will all be for
+        # the same reason.
+        skipped_reason = None
         # NOTE(pabelanger): Check successful/paused jobs to see if
         # zuul_return includes zuul.child_jobs.
         build_result = build.result_data.get('zuul', {})
         if ((build.result == 'SUCCESS' or build.paused)
                 and 'child_jobs' in build_result):
+            skipped_reason = ('Skipped due to child_jobs return value in job '
+                              + build.job.name)
             zuul_return = build_result.get('child_jobs', [])
             dependent_jobs = job_graph.getDirectDependentJobs(
                 build.job.name)
@@ -5254,6 +5259,7 @@ class QueueItem(zkobject.ZKObject):
                     skipped += to_skip
 
         elif build.result not in ('SUCCESS', 'SKIPPED') and not build.paused:
+            skipped_reason = 'Skipped due to failed job ' + build.job.name
             to_skip = job_graph.getDependentJobsRecursively(
                 build.job.name)
             skipped += to_skip
@@ -5264,6 +5270,7 @@ class QueueItem(zkobject.ZKObject):
                 fakebuild = Build.new(self.pipeline.manager.current_context,
                                       job=job,
                                       build_set=self.current_build_set,
+                                      error_detail=skipped_reason,
                                       result='SKIPPED')
                 self.addBuild(fakebuild)
                 self.pipeline.manager.sql.reportBuildEnd(
@@ -5271,13 +5278,14 @@ class QueueItem(zkobject.ZKObject):
                     tenant=self.pipeline.tenant.name,
                     final=True)
 
-    def setNodeRequestFailure(self, job):
+    def setNodeRequestFailure(self, job, error):
         fakebuild = Build.new(
             self.pipeline.manager.current_context,
             job=job,
             build_set=self.current_build_set,
             start_time=time.time(),
             end_time=time.time(),
+            error_detail=error,
             result='NODE_FAILURE',
         )
         self.addBuild(fakebuild)
@@ -5291,19 +5299,19 @@ class QueueItem(zkobject.ZKObject):
         self.updateAttributes(
             self.pipeline.manager.current_context,
             dequeued_needing_change=msg)
-        self._setAllJobsSkipped()
+        self._setAllJobsSkipped(msg)
 
     def setDequeuedMissingRequirements(self):
         self.updateAttributes(
             self.pipeline.manager.current_context,
             dequeued_missing_requirements=True)
-        self._setAllJobsSkipped()
+        self._setAllJobsSkipped('Missing pipeline requirements')
 
-    def setDequeuedBundleFailing(self):
+    def setDequeuedBundleFailing(self, msg):
         self.updateAttributes(
             self.pipeline.manager.current_context,
             dequeued_bundle_failing=True)
-        self._setMissingJobsSkipped()
+        self._setMissingJobsSkipped(msg)
 
     def setUnableToMerge(self, errors=None):
         with self.current_build_set.activeContext(
@@ -5313,7 +5321,7 @@ class QueueItem(zkobject.ZKObject):
                 for msg in errors:
                     self.current_build_set.warning_messages.append(msg)
                     self.log.info(msg)
-        self._setAllJobsSkipped()
+        self._setAllJobsSkipped('Unable to merge')
 
     def setConfigError(self, error):
         err = ConfigurationError(None, None, error)
@@ -5333,27 +5341,27 @@ class QueueItem(zkobject.ZKObject):
             with self.current_build_set.activeContext(
                     self.pipeline.manager.current_context):
                 self.current_build_set.setConfigErrors(errors)
-        self._setAllJobsSkipped()
+        self._setAllJobsSkipped('Buildset configuration error')
 
-    def _setAllJobsSkipped(self):
+    def _setAllJobsSkipped(self, msg):
         for job in self.getJobs():
             fakebuild = Build.new(self.pipeline.manager.current_context,
                                   job=job, build_set=self.current_build_set,
-                                  result='SKIPPED')
+                                  error_detail=msg, result='SKIPPED')
             self.addBuild(fakebuild)
             self.pipeline.manager.sql.reportBuildEnd(
                 fakebuild,
                 tenant=self.pipeline.tenant.name,
                 final=True)
 
-    def _setMissingJobsSkipped(self):
+    def _setMissingJobsSkipped(self, msg):
         for job in self.getJobs():
             if job.name in self.current_build_set.builds:
                 # We already have a build for this job
                 continue
             fakebuild = Build.new(self.pipeline.manager.current_context,
                                   job=job, build_set=self.current_build_set,
-                                  result='SKIPPED')
+                                  error_detail=msg, result='SKIPPED')
             self.addBuild(fakebuild)
             self.pipeline.manager.sql.reportBuildEnd(
                 fakebuild,