Merge "Add config-error reporter and report config errors to DB"

9 files changed, 129 insertions, 10 deletions

diff --git a/doc/source/config/pipeline.rst b/doc/source/config/pipeline.rst
index f1c294775..f4d7cce69 100644
--- a/doc/source/config/pipeline.rst
+++ b/doc/source/config/pipeline.rst
@@ -332,9 +332,16 @@ success, the pipeline reports back to Gerrit with ``Verified`` vote of
    .. attr:: merge-conflict
 
       These reporters describe what Zuul should do if it is unable to
-      merge in the patchset. If no merge-conflict reporters are listed
-      then the ``failure`` reporters will be used to notify of
-      unsuccessful merges.
+      merge the patchset into the current state of the target
+      branch. If no merge-conflict reporters are listed then the
+      ``failure`` reporters will be used.
+
+   .. attr:: config-error
+
+      These reporters describe what Zuul should do if it encounters a
+      configuration error while trying to enqueue the item.  If no
+      config-error reporters are listed then the ``failure`` reporters
+      will be used.
 
    .. attr:: enqueue
 
diff --git a/releasenotes/notes/config-error-reporter-34887223d91544d1.yaml b/releasenotes/notes/config-error-reporter-34887223d91544d1.yaml
new file mode 100644
index 000000000..51690f2fa
--- /dev/null
+++ b/releasenotes/notes/config-error-reporter-34887223d91544d1.yaml
@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    A new :attr:`pipeline.config-error` pipeline reporter is available
+    for customizing reporter actions related to Zuul configuration
+    errors.
diff --git a/tests/fixtures/layouts/freeze-job-failure.yaml b/tests/fixtures/layouts/freeze-job-failure.yaml
new file mode 100644
index 000000000..ae3f48324
--- /dev/null
+++ b/tests/fixtures/layouts/freeze-job-failure.yaml
@@ -0,0 +1,32 @@
+- 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: project-test1
+    run: playbooks/project-test1.yaml
+
+- job:
+    name: project-test2
+    run: playbooks/project-test2.yaml
+
+- project:
+    name: org/project
+    check:
+      jobs:
+        - project-test2:
+            dependencies: project-test1
diff --git a/tests/fixtures/layouts/timer-freeze-job-failure.yaml b/tests/fixtures/layouts/timer-freeze-job-failure.yaml
new file mode 100644
index 000000000..2e6d709bb
--- /dev/null
+++ b/tests/fixtures/layouts/timer-freeze-job-failure.yaml
@@ -0,0 +1,26 @@
+- pipeline:
+    name: periodic
+    manager: independent
+    trigger:
+      timer:
+        - time: '* * * * * */1'
+
+- job:
+    name: base
+    parent: null
+    run: playbooks/base.yaml
+
+- job:
+    name: project-test1
+    run: playbooks/project-test1.yaml
+
+- job:
+    name: project-test2
+    run: playbooks/project-test2.yaml
+
+- project:
+    name: org/project
+    periodic:
+      jobs:
+        - project-test2:
+            dependencies: project-test1
diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py
index 66c508fea..978bc00a4 100644
--- a/tests/unit/test_scheduler.py
+++ b/tests/unit/test_scheduler.py
@@ -5343,6 +5343,11 @@ For CI problems and help debugging, contact ci@example.org"""
         self.assertIn('Error merging gerrit/org/project', B.messages[0])
         self.assertNotIn('logs.example.com', B.messages[0])
         self.assertNotIn('SKIPPED', B.messages[0])
+        buildsets = list(
+            self.scheds.first.connections.connections[
+                'database'].getBuildsets())
+        self.assertEqual(buildsets[0].result, 'MERGE_CONFLICT')
+        self.assertIn('This change or one of', buildsets[0].message)
 
     def test_submit_failure(self):
         A = self.fake_gerrit.addFakeChange('org/project1', 'master', 'A')
@@ -5357,6 +5362,33 @@ For CI problems and help debugging, contact ci@example.org"""
                 'database'].getBuildsets())
         self.assertEqual(buildsets[0].result, 'MERGE_FAILURE')
 
+    @simple_layout('layouts/timer-freeze-job-failure.yaml')
+    def test_periodic_freeze_job_failure(self):
+        self.waitUntilSettled()
+
+        for x in iterate_timeout(30, 'buildset complete'):
+            buildsets = list(
+                self.scheds.first.connections.connections[
+                    'database'].getBuildsets())
+            if buildsets:
+                break
+        self.assertEqual(buildsets[0].result, 'CONFIG_ERROR')
+        self.assertIn('Job project-test2 depends on project-test1 '
+                      'which was not run', buildsets[0].message)
+
+    @simple_layout('layouts/freeze-job-failure.yaml')
+    def test_freeze_job_failure(self):
+        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        buildsets = list(
+            self.scheds.first.connections.connections[
+                'database'].getBuildsets())
+        self.assertEqual(buildsets[0].result, 'CONFIG_ERROR')
+        self.assertIn('Job project-test2 depends on project-test1 '
+                      'which was not run', buildsets[0].message)
+
     @simple_layout('layouts/nonvoting-pipeline.yaml')
     def test_nonvoting_pipeline(self):
         "Test that a nonvoting pipeline (experimental) can still report"
diff --git a/zuul/configloader.py b/zuul/configloader.py
index eb468518f..365967d56 100644
--- a/zuul/configloader.py
+++ b/zuul/configloader.py
@@ -1176,6 +1176,7 @@ class PipelineParser(object):
         'success': 'success_actions',
         'failure': 'failure_actions',
         'merge-conflict': 'merge_conflict_actions',
+        'config-error': 'config_error_actions',
         'no-jobs': 'no_jobs_actions',
         'disabled': 'disabled_actions',
         'dequeue': 'dequeue_actions',
@@ -1250,7 +1251,7 @@ class PipelineParser(object):
         pipeline['trigger'] = vs.Required(self.getDriverSchema('trigger'))
         for action in ['enqueue', 'start', 'success', 'failure',
                        'merge-conflict', 'merge-failure', 'no-jobs',
-                       'disabled', 'dequeue']:
+                       'disabled', 'dequeue', 'config-error']:
             pipeline[action] = self.getDriverSchema('reporter')
         return vs.Schema(pipeline)
 
@@ -1318,6 +1319,10 @@ class PipelineParser(object):
         if not pipeline.merge_conflict_actions:
             pipeline.merge_conflict_actions = pipeline.failure_actions
 
+        # If config-error actions aren't explicit, use the failure actions
+        if not pipeline.config_error_actions:
+            pipeline.config_error_actions = pipeline.failure_actions
+
         pipeline.disable_at = conf.get(
             'disable-after-consecutive-failures', None)
 
diff --git a/zuul/driver/gerrit/gerritreporter.py b/zuul/driver/gerrit/gerritreporter.py
index b99133dce..c38a9484a 100644
--- a/zuul/driver/gerrit/gerritreporter.py
+++ b/zuul/driver/gerrit/gerritreporter.py
@@ -36,6 +36,9 @@ class GerritReporter(BaseReporter):
         self._checks_api = action.pop('checks-api', None)
         self._labels = action
 
+    def __repr__(self):
+        return f"<GerritReporter: {self._action}>"
+
     def report(self, item, phase1=True, phase2=True):
         """Send a message to gerrit."""
         log = get_annotated_logger(self.log, item.event)
diff --git a/zuul/manager/__init__.py b/zuul/manager/__init__.py
index 31c4b54d7..637d31d9f 100644
--- a/zuul/manager/__init__.py
+++ b/zuul/manager/__init__.py
@@ -322,9 +322,10 @@ class PipelineManager(metaclass=ABCMeta):
                                (item, ret))
 
     def reportNormalBuildsetEnd(self, build_set, action, final, result=None):
-        # Report a buildset end, but only if there are jobs
-        if (build_set.job_graph and
-            len(build_set.job_graph.jobs) > 0):
+        # Report a buildset end if there are jobs or errors
+        if ((build_set.job_graph and len(build_set.job_graph.jobs) > 0) or
+            build_set.config_errors or
+            build_set.unable_to_merge):
             self.sql.reportBuildsetEnd(build_set, action,
                                        final, result)
 
@@ -2032,9 +2033,8 @@ class PipelineManager(metaclass=ABCMeta):
             item.setReportedResult('NO_JOBS')
         elif item.getConfigErrors():
             log.debug("Invalid config for change %s", item.change)
-            # TODOv3(jeblair): consider a new reporter action for this
-            action = 'merge-conflict'
-            actions = self.pipeline.merge_conflict_actions
+            action = 'config-error'
+            actions = self.pipeline.config_error_actions
             item.setReportedResult('CONFIG_ERROR')
         elif item.didMergerFail():
             log.debug("Merge conflict")
diff --git a/zuul/reporter/__init__.py b/zuul/reporter/__init__.py
index 9cd60bff6..5af48abc6 100644
--- a/zuul/reporter/__init__.py
+++ b/zuul/reporter/__init__.py
@@ -135,6 +135,7 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
             'failure': self._formatItemReportFailure,
             'merge-conflict': self._formatItemReportMergeConflict,
             'merge-failure': self._formatItemReportMergeFailure,
+            'config-error': self._formatItemReportConfigError,
             'no-jobs': self._formatItemReportNoJobs,
             'disabled': self._formatItemReportDisabled,
             'dequeue': self._formatItemReportDequeue,
@@ -226,6 +227,13 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
     def _formatItemReportMergeFailure(self, item, with_jobs=True):
         return 'This change was not merged by the code review system.\n'
 
+    def _formatItemReportConfigError(self, item, with_jobs=True):
+        if item.getConfigErrors():
+            msg = str(item.getConfigErrors()[0].error)
+        else:
+            msg = "Unknown configuration error"
+        return msg
+
     def _formatItemReportNoJobs(self, item, with_jobs=True):
         status_url = get_default(self.connection.sched.config,
                                  'web', 'status_url', '')