Merge "configloader: Fix error report for job nodesets"

2 files changed, 68 insertions, 4 deletions

diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py
index fe5b46e0c..225c6d355 100644
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -2232,6 +2232,64 @@ class TestInRepoConfig(ZuulTestCase):
         self.assertIn('appears multiple times', A.messages[0],
                       "A should have a syntax error reported")
 
+    def test_group_in_job_with_invalid_node(self):
+        in_repo_conf = textwrap.dedent(
+            """
+            - job:
+                name: test job
+                nodeset:
+                  nodes: []
+                  groups:
+                    - name: a_group
+                      nodes:
+                       - a_node_that_does_not_exist
+            """)
+
+        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, 1,
+                         "A should report failure")
+        self.assertIn('which is not defined in the nodeset', A.messages[0],
+                      "A should have a syntax error reported")
+
+    def test_duplicate_group_in_job(self):
+        in_repo_conf = textwrap.dedent(
+            """
+            - job:
+                name: test job
+                nodeset:
+                  nodes:
+                   - name: controller
+                     label: ubuntu-focal
+                  groups:
+                    - name: a_duplicate_group
+                      nodes:
+                       - controller
+                    - name: a_duplicate_group
+                      nodes:
+                       - controller
+            """)
+
+        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, 1,
+                         "A should report failure")
+        self.assertIn(
+            'Group names must be unique within a nodeset.',
+            A.messages[0], "A should have a syntax error reported")
+
     def test_secret_not_found_error(self):
         in_repo_conf = textwrap.dedent(
             """
diff --git a/zuul/configloader.py b/zuul/configloader.py
index a92f5337c..2b696a832 100644
--- a/zuul/configloader.py
+++ b/zuul/configloader.py
@@ -81,7 +81,7 @@ class ConfigurationSyntaxError(Exception):
 class NodeFromGroupNotFoundError(Exception):
     def __init__(self, nodeset, node, group):
         message = textwrap.dedent("""\
-        In nodeset "{nodeset}" the group "{group}" contains a
+        In {nodeset} the group "{group}" contains a
         node named "{node}" which is not defined in the nodeset.""")
         message = textwrap.fill(message.format(nodeset=nodeset,
                                                node=node, group=group))
@@ -137,7 +137,7 @@ class MaxTimeoutError(Exception):
 class DuplicateGroupError(Exception):
     def __init__(self, nodeset, group):
         message = textwrap.dedent("""\
-        In nodeset "{nodeset}" the group "{group}" appears multiple times.
+        In {nodeset} the group "{group}" appears multiple times.
         Group names must be unique within a nodeset.""")
         message = textwrap.fill(message.format(nodeset=nodeset,
                                                group=group))
@@ -476,6 +476,7 @@ class NodeSetParser(object):
     def __init__(self, pcontext):
         self.log = logging.getLogger("zuul.NodeSetParser")
         self.pcontext = pcontext
+        self.anonymous = False
         self.schema = self.getSchema(False)
         self.anon_schema = self.getSchema(True)
 
@@ -513,6 +514,7 @@ class NodeSetParser(object):
     def fromYaml(self, conf, anonymous=False):
         if anonymous:
             self.anon_schema(conf)
+            self.anonymous = True
         else:
             self.schema(conf)
 
@@ -565,10 +567,14 @@ class NodeSetParser(object):
                 raise Exception("Groups named 'localhost' are not allowed.")
             for node_name in as_list(conf_group['nodes']):
                 if node_name not in node_names:
-                    raise NodeFromGroupNotFoundError(conf['name'], node_name,
+                    nodeset_str = 'the nodeset' if self.anonymous else \
+                        'the nodeset "%s"' % conf['name']
+                    raise NodeFromGroupNotFoundError(nodeset_str, node_name,
                                                      conf_group['name'])
             if conf_group['name'] in group_names:
-                raise DuplicateGroupError(conf['name'], conf_group['name'])
+                nodeset_str = 'the nodeset' if self.anonymous else \
+                    'the nodeset "%s"' % conf['name']
+                raise DuplicateGroupError(nodeset_str, conf_group['name'])
             group = model.Group(conf_group['name'],
                                 as_list(conf_group['nodes']))
             ns.addGroup(group)