Report dynamic layout config errors
If Zuul encounters a syntax error when loading an in-repo layout,
report that as a failure on the change. Try to provide as much
readable information as possible to allow the user to diagnose the
error.
All top-level configuration objects are updated to support a hidden
source context variable to aid in creating a useful error message.
Item.areAllJobsComplete is updated so that it returns true in the
case of a merge failure or config error. In Zuulv2, we knew what
jobs we would run even in those cases, but in Zuulv3, we don't, so
the item does not have a job tree, so in those cases, the item does
not report that all jobs are complete. Returining True in this case
allows the NNFI algorithm to continue and expel the item from the
queue, avoiding an inifinite loop.
The merge failure accessor method is simplified.
Change-Id: I9cf19b87c6af5926b5e8bb403b81ce0470e3592d
diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py
index f69ffe6..cf88265 100644
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -185,6 +185,27 @@
dict(name='project-test1', result='SUCCESS', changes='2,1'),
dict(name='project-test2', result='SUCCESS', changes='3,1')])
+ def test_dynamic_syntax_error(self):
+ in_repo_conf = textwrap.dedent(
+ """
+ - job:
+ name: project-test2
+ foo: error
+ """)
+
+ file_dict = {'.zuul.yaml': in_repo_conf}
+ A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
+ files=file_dict)
+ A.addApproval('code-review', 2)
+ self.fake_gerrit.addEvent(A.addApproval('approved', 1))
+ self.waitUntilSettled()
+
+ self.assertEqual(A.data['status'], 'NEW')
+ self.assertEqual(A.reported, 2,
+ "A should report start and failure")
+ self.assertIn('syntax error', A.messages[1],
+ "A should have a syntax error reported")
+
class TestAnsible(AnsibleZuulTestCase):
# A temporary class to hold new tests while others are disabled
diff --git a/zuul/configloader.py b/zuul/configloader.py
index 5a132fe..42616a8 100644
--- a/zuul/configloader.py
+++ b/zuul/configloader.py
@@ -10,11 +10,13 @@
# License for the specific language governing permissions and limitations
# under the License.
+from contextlib import contextmanager
import copy
import os
import logging
import six
import yaml
+import pprint
import voluptuous as vs
@@ -38,6 +40,35 @@
return [item]
+class ConfigurationSyntaxError(Exception):
+ pass
+
+
+@contextmanager
+def configuration_exceptions(stanza, conf):
+ try:
+ yield
+ except vs.Invalid as e:
+ conf = copy.deepcopy(conf)
+ context = conf.pop('_source_context')
+ m = """
+Zuul encountered a syntax error while parsing its configuration in the
+repo {repo} on branch {branch}. The error was:
+
+ {error}
+
+The offending content was a {stanza} stanza with the content:
+
+{content}
+"""
+ m = m.format(repo=context.project.name,
+ branch=context.branch,
+ error=str(e),
+ stanza=stanza,
+ content=pprint.pformat(conf))
+ raise ConfigurationSyntaxError(m)
+
+
class NodeSetParser(object):
@staticmethod
def getSchema():
@@ -47,13 +78,15 @@
nodeset = {vs.Required('name'): str,
vs.Required('nodes'): [node],
+ '_source_context': model.SourceContext,
}
return vs.Schema(nodeset)
@staticmethod
def fromYaml(layout, conf):
- NodeSetParser.getSchema()(conf)
+ with configuration_exceptions('nodeset', conf):
+ NodeSetParser.getSchema()(conf)
ns = model.NodeSet(conf['name'])
for conf_node in as_list(conf['nodes']):
node = model.Node(conf_node['name'], conf_node['image'])
@@ -136,7 +169,8 @@
@staticmethod
def fromYaml(tenant, layout, conf):
- JobParser.getSchema()(conf)
+ with configuration_exceptions('job', conf):
+ JobParser.getSchema()(conf)
# NB: The default detection system in the Job class requires
# that we always assign values directly rather than modifying
@@ -272,7 +306,8 @@
@staticmethod
def fromYaml(tenant, layout, conf):
- ProjectTemplateParser.getSchema(layout)(conf)
+ with configuration_exceptions('project or project-template', conf):
+ ProjectTemplateParser.getSchema(layout)(conf)
# Make a copy since we modify this later via pop
conf = copy.deepcopy(conf)
project_template = model.ProjectConfig(conf['name'])
@@ -338,11 +373,13 @@
for p in layout.pipelines.values():
project[p.name] = {'queue': str,
'jobs': [vs.Any(str, dict)]}
- return vs.Schema([project])
+ return vs.Schema(project)
@staticmethod
def fromYaml(tenant, layout, conf_list):
- ProjectParser.getSchema(layout)(conf_list)
+ for conf in conf_list:
+ with configuration_exceptions('project', conf):
+ ProjectParser.getSchema(layout)(conf)
project = model.ProjectConfig(conf_list[0]['name'])
mode = conf_list[0].get('merge-mode', 'merge-resolve')
project.merge_mode = model.MERGER_MAP[mode]
@@ -461,6 +498,7 @@
'window-increase-factor': window_factor,
'window-decrease-type': window_type,
'window-decrease-factor': window_factor,
+ '_source_context': model.SourceContext,
}
pipeline['trigger'] = vs.Required(
PipelineParser.getDriverSchema('trigger', connections))
@@ -472,7 +510,8 @@
@staticmethod
def fromYaml(layout, connections, scheduler, conf):
- PipelineParser.getSchema(layout, connections)(conf)
+ with configuration_exceptions('pipeline', conf):
+ PipelineParser.getSchema(layout, connections)(conf)
pipeline = model.Pipeline(conf['name'], layout)
pipeline.description = conf.get('description')
diff --git a/zuul/manager/__init__.py b/zuul/manager/__init__.py
index 18cf11b..4447615 100644
--- a/zuul/manager/__init__.py
+++ b/zuul/manager/__init__.py
@@ -265,6 +265,8 @@
# Similarly, reset the item state.
if item.current_build_set.unable_to_merge:
item.setUnableToMerge()
+ if item.current_build_set.config_error:
+ item.setConfigError(item.current_build_set.config_error)
if item.dequeued_needing_change:
item.setDequeuedNeedingChange()
@@ -482,8 +484,21 @@
import zuul.configloader
loader = zuul.configloader.ConfigLoader()
self.log.debug("Load dynamic layout with %s" % build_set.files)
- layout = loader.createDynamicLayout(item.pipeline.layout.tenant,
- build_set.files)
+ try:
+ layout = loader.createDynamicLayout(
+ item.pipeline.layout.tenant,
+ build_set.files)
+ except zuul.configloader.ConfigurationSyntaxError as e:
+ self.log.info("Configuration syntax error "
+ "in dynamic layout %s" %
+ build_set.files)
+ item.setConfigError(str(e))
+ return None
+ except Exception:
+ self.log.exception("Error in dynamic layout %s" %
+ build_set.files)
+ item.setConfigError("Unknown configuration error")
+ return None
return layout
build_set.merge_state = build_set.PENDING
self.log.debug("Preparing dynamic layout for: %s" % item.change)
@@ -556,6 +571,8 @@
ready = self.prepareLayout(item)
if item.current_build_set.unable_to_merge:
failing_reasons.append("it has a merge conflict")
+ if item.current_build_set.config_error:
+ failing_reasons.append("it has an invalid configuration")
if ready and self.provisionNodes(item):
changed = True
if actionable and ready and self.launchJobs(item):
@@ -693,7 +710,12 @@
def _reportItem(self, item):
self.log.debug("Reporting change %s" % item.change)
ret = True # Means error as returned by trigger.report
- if not item.getJobs():
+ if item.getConfigError():
+ self.log.debug("Invalid config for change %s" % item.change)
+ # TODOv3(jeblair): consider a new reporter action for this
+ actions = self.pipeline.merge_failure_actions
+ item.setReportedResult('CONFIG_ERROR')
+ elif not item.getJobs():
# We don't send empty reports with +1,
# and the same for -1's (merge failures or transient errors)
# as they cannot be followed by +1's
diff --git a/zuul/model.py b/zuul/model.py
index 96414be..19931ea 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -1012,6 +1012,7 @@
self.commit = None
self.zuul_url = None
self.unable_to_merge = False
+ self.config_error = None # None or an error message string.
self.failing_reasons = []
self.merge_state = self.NEW
self.nodesets = {} # job -> nodeset
@@ -1170,6 +1171,9 @@
return True
def areAllJobsComplete(self):
+ if (self.current_build_set.config_error or
+ self.current_build_set.unable_to_merge):
+ return True
if not self.hasJobTree():
return False
for job in self.getJobs():
@@ -1203,9 +1207,10 @@
return False
def didMergerFail(self):
- if self.current_build_set.unable_to_merge:
- return True
- return False
+ return self.current_build_set.unable_to_merge
+
+ def getConfigError(self):
+ return self.current_build_set.config_error
def isHoldingFollowingChanges(self):
if not self.live:
@@ -1325,6 +1330,10 @@
self.current_build_set.unable_to_merge = True
self._setAllJobsSkipped()
+ def setConfigError(self, error):
+ self.current_build_set.config_error = error
+ self._setAllJobsSkipped()
+
def _setAllJobsSkipped(self):
for job in self.getJobs():
fakebuild = Build(job, None)
@@ -2083,8 +2092,7 @@
"a single key (when parsing %s)" %
(conf,))
key, value = item.items()[0]
- if key in ['project', 'project-template', 'job']:
- value['_source_context'] = source_context
+ value['_source_context'] = source_context
if key == 'project':
name = value['name']
self.projects.setdefault(name, []).append(value)
diff --git a/zuul/reporter/__init__.py b/zuul/reporter/__init__.py
index 9ed6599..541f259 100644
--- a/zuul/reporter/__init__.py
+++ b/zuul/reporter/__init__.py
@@ -85,6 +85,8 @@
msg = 'This change depends on a change that failed to merge.\n'
elif item.didMergerFail():
msg = pipeline.merge_failure_message
+ elif item.getConfigError():
+ msg = item.getConfigError()
else:
msg = (pipeline.failure_message + '\n\n' +
self._formatItemReportJobs(pipeline, item))