Handle nodesets in branches

This allows a nodeset to appear on multiple branches of a project.

See Ia9d5b77d1ce46e6461b370e951301ede4045bbb9 for more information.

Change-Id: If7ca83e8ec3cad8f9bd99a65b974d56a999f256c

12 files changed, 260 insertions, 5 deletions

diff --git a/doc/source/user/config.rst b/doc/source/user/config.rst
index 0f610a361..3f3a41241 100644
--- a/doc/source/user/config.rst
+++ b/doc/source/user/config.rst
@@ -1213,6 +1213,12 @@ specify what nodes they require individually, however, by defining
 groups of node types once and referring to them by name, job
 configuration may be simplified.
 
+Nodesets, like most configuration items, are globally unique, though a
+nodeset may be defined on multiple branches of the same project as long
+as the contents are the same.  This is to aid in branch maintenance,
+so that creating a new branch based on an existing branch will not
+immediately produce a configuration error.
+
 .. code-block:: yaml
 
    - nodeset:
diff --git a/tests/fixtures/config/nodesets/git/common-config/playbooks/base.yaml b/tests/fixtures/config/nodesets/git/common-config/playbooks/base.yaml
new file mode 100644
index 000000000..f679dceae
--- /dev/null
+++ b/tests/fixtures/config/nodesets/git/common-config/playbooks/base.yaml
@@ -0,0 +1,2 @@
+- hosts: all
+  tasks: []
diff --git a/tests/fixtures/config/nodesets/git/common-config/zuul.yaml b/tests/fixtures/config/nodesets/git/common-config/zuul.yaml
new file mode 100644
index 000000000..e1e2fb736
--- /dev/null
+++ b/tests/fixtures/config/nodesets/git/common-config/zuul.yaml
@@ -0,0 +1,39 @@
+- pipeline:
+    name: check
+    manager: independent
+    post-review: true
+    trigger:
+      gerrit:
+        - event: patchset-created
+    success:
+      gerrit:
+        Verified: 1
+    failure:
+      gerrit:
+        Verified: -1
+
+- pipeline:
+    name: gate
+    manager: dependent
+    success-message: Build succeeded (gate).
+    trigger:
+      gerrit:
+        - event: comment-added
+          approval:
+            - Approved: 1
+    success:
+      gerrit:
+        Verified: 2
+        submit: true
+    failure:
+      gerrit:
+        Verified: -2
+    start:
+      gerrit:
+        Verified: 0
+    precedence: high
+
+- job:
+    name: base
+    parent: null
+    run: playbooks/base.yaml
diff --git a/tests/fixtures/config/nodesets/git/org_project1/README b/tests/fixtures/config/nodesets/git/org_project1/README
new file mode 100644
index 000000000..9daeafb98
--- /dev/null
+++ b/tests/fixtures/config/nodesets/git/org_project1/README
@@ -0,0 +1 @@
+test
diff --git a/tests/fixtures/config/nodesets/git/org_project1/zuul.yaml b/tests/fixtures/config/nodesets/git/org_project1/zuul.yaml
new file mode 100644
index 000000000..398269e28
--- /dev/null
+++ b/tests/fixtures/config/nodesets/git/org_project1/zuul.yaml
@@ -0,0 +1,15 @@
+- nodeset:
+    name: project1-nodeset
+    nodes:
+      - name: controller
+        label: ubuntu-xenial
+
+- job:
+    parent: base
+    name: project1-test
+    nodeset: project1-nodeset
+
+- project:
+    check:
+      jobs:
+        - project1-test
diff --git a/tests/fixtures/config/nodesets/git/org_project2/README b/tests/fixtures/config/nodesets/git/org_project2/README
new file mode 100644
index 000000000..9daeafb98
--- /dev/null
+++ b/tests/fixtures/config/nodesets/git/org_project2/README
@@ -0,0 +1 @@
+test
diff --git a/tests/fixtures/config/nodesets/git/org_project2/zuul-nodeset.yaml b/tests/fixtures/config/nodesets/git/org_project2/zuul-nodeset.yaml
new file mode 100644
index 000000000..cb969a7b4
--- /dev/null
+++ b/tests/fixtures/config/nodesets/git/org_project2/zuul-nodeset.yaml
@@ -0,0 +1,18 @@
+- nodeset:
+    name: project2-nodeset
+    nodes:
+      name: controller
+      label: ubuntu-xenial
+
+- job:
+    parent: base
+    name: project2-test
+    nodeset: project2-nodeset
+
+- project:
+    check:
+      jobs:
+        - project2-test
+    gate:
+      jobs:
+        - noop
diff --git a/tests/fixtures/config/nodesets/git/org_project2/zuul.yaml b/tests/fixtures/config/nodesets/git/org_project2/zuul.yaml
new file mode 100644
index 000000000..a4b42b191
--- /dev/null
+++ b/tests/fixtures/config/nodesets/git/org_project2/zuul.yaml
@@ -0,0 +1,11 @@
+- job:
+    parent: base
+    name: project2-test
+
+- project:
+    check:
+      jobs:
+        - project2-test
+    gate:
+      jobs:
+        - noop
diff --git a/tests/fixtures/config/nodesets/main.yaml b/tests/fixtures/config/nodesets/main.yaml
new file mode 100644
index 000000000..950b1172c
--- /dev/null
+++ b/tests/fixtures/config/nodesets/main.yaml
@@ -0,0 +1,9 @@
+- tenant:
+    name: tenant-one
+    source:
+      gerrit:
+        config-projects:
+          - common-config
+        untrusted-projects:
+          - org/project1
+          - org/project2
diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py
index 20b89188c..892f7f2ab 100755
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -2875,6 +2875,143 @@ class TestSecretLeaks(AnsibleZuulTestCase):
         self._test_secret_file_fail()
 
 
+class TestNodesets(ZuulTestCase):
+    tenant_config_file = 'config/nodesets/main.yaml'
+
+    def test_nodeset_branch(self):
+        # Test that we can use a nodeset defined in another branch of
+        # the same project.
+        self.create_branch('org/project2', 'stable')
+        self.fake_gerrit.addEvent(
+            self.fake_gerrit.getFakeBranchCreatedEvent(
+                'org/project2', 'stable'))
+        self.waitUntilSettled()
+
+        with open(os.path.join(FIXTURE_DIR,
+                               'config/nodesets/git/',
+                               'org_project2/zuul-nodeset.yaml')) as f:
+            config = f.read()
+
+        file_dict = {'zuul.yaml': config}
+        A = self.fake_gerrit.addFakeChange('org/project2', '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'], 'MERGED')
+        self.fake_gerrit.addEvent(A.getChangeMergedEvent())
+        self.waitUntilSettled()
+
+        in_repo_conf = textwrap.dedent(
+            """
+            - job:
+                parent: base
+                name: project2-test
+                nodeset: project2-nodeset
+
+            - project:
+                check:
+                  jobs:
+                    - project2-test
+                gate:
+                  jobs:
+                    - noop
+            """)
+        file_dict = {'zuul.yaml': in_repo_conf}
+        B = self.fake_gerrit.addFakeChange('org/project2', 'stable', 'B',
+                                           files=file_dict)
+        self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+        self.assertEqual(B.reported, 1, "B should report success")
+        self.assertHistory([
+            dict(name='project2-test', result='SUCCESS', changes='2,1',
+                 node='ubuntu-xenial'),
+        ])
+
+    def test_nodeset_branch_duplicate(self):
+        # Test that we can create a duplicate secret on a different
+        # branch of the same project -- i.e., that when we branch
+        # master to stable on a project with a secret, nothing
+        # changes.
+        self.create_branch('org/project1', 'stable')
+        self.fake_gerrit.addEvent(
+            self.fake_gerrit.getFakeBranchCreatedEvent(
+                'org/project1', 'stable'))
+        self.waitUntilSettled()
+
+        A = self.fake_gerrit.addFakeChange('org/project1', 'stable', 'A')
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+        self.assertEqual(A.reported, 1,
+                         "A should report success")
+        self.assertHistory([
+            dict(name='project1-test', result='SUCCESS', changes='1,1',
+                 node='ubuntu-xenial'),
+        ])
+
+    def test_nodeset_branch_error_same_branch(self):
+        # Test that we are unable to define a nodeset twice on the same
+        # project-branch.
+        in_repo_conf = textwrap.dedent(
+            """
+            - nodeset:
+                name: project1-nodeset
+                nodes: []
+            - nodeset:
+                name: project1-nodeset
+                nodes: []
+            """)
+        file_dict = {'zuul.yaml': in_repo_conf}
+        A = self.fake_gerrit.addFakeChange('org/project1', 'master', 'A',
+                                           files=file_dict)
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+        self.assertIn('already defined', A.messages[0])
+
+    def test_nodeset_branch_error_same_project(self):
+        # Test that we are unable to create a nodeset which differs
+        # from another with the same name -- i.e., that if we have a
+        # duplicate nodeset on multiple branches of the same project,
+        # they must be identical.
+        self.create_branch('org/project1', 'stable')
+        self.fake_gerrit.addEvent(
+            self.fake_gerrit.getFakeBranchCreatedEvent(
+                'org/project1', 'stable'))
+        self.waitUntilSettled()
+
+        in_repo_conf = textwrap.dedent(
+            """
+            - nodeset:
+                name: project1-nodeset
+                nodes: []
+            """)
+        file_dict = {'zuul.yaml': in_repo_conf}
+        A = self.fake_gerrit.addFakeChange('org/project1', 'stable', 'A',
+                                           files=file_dict)
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+        self.assertIn('does not match existing definition in branch master',
+                      A.messages[0])
+
+    def test_nodeset_branch_error_other_project(self):
+        # Test that we are unable to create a nodeset with the same
+        # name as another.  We're never allowed to have a nodeset with
+        # the same name outside of a project.
+        in_repo_conf = textwrap.dedent(
+            """
+            - nodeset:
+                name: project1-nodeset
+                nodes: []
+            """)
+        file_dict = {'zuul.yaml': in_repo_conf}
+        A = self.fake_gerrit.addFakeChange('org/project2', 'master', 'A',
+                                           files=file_dict)
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+        self.assertIn('already defined in project org/project1',
+                      A.messages[0])
+
+
 class TestJobOutput(AnsibleZuulTestCase):
     tenant_config_file = 'config/job-output/main.yaml'
 
diff --git a/zuul/configloader.py b/zuul/configloader.py
index aa0e9f7a1..c3a1be3ff 100644
--- a/zuul/configloader.py
+++ b/zuul/configloader.py
@@ -407,7 +407,7 @@ class NodeSetParser(object):
     @staticmethod
     def fromYaml(conf, anonymous=False):
         NodeSetParser.getSchema(anonymous)(conf)
-        ns = model.NodeSet(conf.get('name'))
+        ns = model.NodeSet(conf.get('name'), conf.get('_source_context'))
         node_names = set()
         group_names = set()
         for conf_node in as_list(conf['nodes']):
diff --git a/zuul/model.py b/zuul/model.py
index 46e2c12b4..559653306 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -476,8 +476,9 @@ class NodeSet(object):
     or they may appears anonymously in in-line job definitions.
     """
 
-    def __init__(self, name=None):
+    def __init__(self, name=None, source_context=None):
         self.name = name or ''
+        self.source_context = source_context
         self.nodes = OrderedDict()
         self.groups = OrderedDict()
 
@@ -2586,8 +2587,23 @@ class Layout(object):
         return True
 
     def addNodeSet(self, nodeset):
-        if nodeset.name in self.nodesets:
-            raise Exception("NodeSet %s already defined" % (nodeset.name,))
+        # It's ok to have a duplicate nodeset definition, but only if
+        # they are in different branches of the same repo, and have
+        # the same values.
+        other = self.nodesets.get(nodeset.name)
+        if other is not None:
+            if not nodeset.source_context.isSameProject(other.source_context):
+                raise Exception("Nodeset %s already defined in project %s" %
+                                (nodeset.name, other.source_context.project))
+            if nodeset.source_context.branch == other.source_context.branch:
+                raise Exception("Nodeset %s already defined" % (nodeset.name,))
+            if nodeset != other:
+                raise Exception("Nodeset %s does not match existing definition"
+                                " in branch %s" %
+                                (nodeset.name, other.source_context.branch))
+            # Identical data in a different branch of the same project;
+            # ignore the duplicate definition
+            return
         self.nodesets[nodeset.name] = nodeset
 
     def addSecret(self, secret):
@@ -2595,7 +2611,7 @@ class Layout(object):
         # they are in different branches of the same repo, and have
         # the same values.
         other = self.secrets.get(secret.name)
-        if other:
+        if other is not None:
             if not secret.source_context.isSameProject(other.source_context):
                 raise Exception("Secret %s already defined in project %s" %
                                 (secret.name, other.source_context.project))