Raise an error if pipeline is defined twice

When pipelines are defined multiple times zuul currently silently uses
the last one which is defined. This is unsafe. Instead stick with the
first found pipeline and add a configuration error. While at it add
the missing start_mark to the pipeline object as it's needed when
throwing an error.

Change-Id: I98637ecd7d4978a803d6131b7cd61da0a74e669f

tests/fixtures/layouts/broken-double-gate.yaml

@@ -0,0 +1,47 @@
+- pipeline:
+    name: gate
+    manager: dependent
+    trigger:
+      gerrit:
+        - event: comment-added
+          approval:
+            - Approved: 1
+    success:
+      gerrit:
+        Verified: 2
+        submit: true
+    failure:
+      gerrit:
+        Verified: -2
+    start:
+      gerrit:
+        Verified: 0
+
+- pipeline:
+    name: gate
+    manager: dependent
+    trigger:
+      gerrit:
+        - event: comment-added
+          approval:
+            - Approved: 1
+    success:
+      gerrit:
+        Verified: 2
+        submit: true
+    failure:
+      gerrit:
+        Verified: -2
+    start:
+      gerrit:
+        Verified: 0
+
+- job:
+    name: base
+    parent: null
+
+- project:
+    name: org/project
+    gate:
+      jobs:
+        - base

tests/unit/test_model.py

@@ -154,10 +154,7 @@ class TestJob(BaseTestCase):
             job.applyVariant(bad_final, self.layout)
 
     def test_job_inheritance_job_tree(self):
-        pipeline = model.Pipeline('gate', self.tenant)
-        pipeline.source_context = self.context
-        self.layout.addPipeline(pipeline)
-        queue = model.ChangeQueue(pipeline)
+        queue = model.ChangeQueue(self.pipeline)
 
         base = self.pcontext.job_parser.fromYaml({
             '_source_context': self.context,
@@ -229,10 +226,7 @@ class TestJob(BaseTestCase):
         self.assertEqual(job.timeout, 70)
 
     def test_inheritance_keeps_matchers(self):
-        pipeline = model.Pipeline('gate', self.tenant)
-        pipeline.source_context = self.context
-        self.layout.addPipeline(pipeline)
-        queue = model.ChangeQueue(pipeline)
+        queue = model.ChangeQueue(self.pipeline)
 
         base = self.pcontext.job_parser.fromYaml({
             '_source_context': self.context,

tests/unit/test_v3.py

@@ -2496,6 +2496,17 @@ class TestBrokenConfig(ZuulTestCase):
             "Zuul encountered a syntax error",
             str(tenant.layout.loading_errors[0].error))
 
+    @simple_layout('layouts/broken-double-gate.yaml')
+    def test_broken_config_on_startup_double_gate(self):
+        # Verify that duplicated pipeline definitions raise config errors
+        tenant = self.sched.abide.tenants.get('tenant-one')
+        self.assertEquals(
+            len(tenant.layout.loading_errors), 1,
+            "An error should have been stored")
+        self.assertIn(
+            "Zuul encountered a syntax error",
+            str(tenant.layout.loading_errors[0].error))
+
     def test_dynamic_ignore(self):
         # Verify dynamic config behaviors inside a tenant broken config
         tenant = self.sched.abide.tenants.get('tenant-one')

zuul/configloader.py

@@ -1103,6 +1103,7 @@ class PipelineParser(object):
         self.schema(conf)
         pipeline = model.Pipeline(conf['name'], self.pcontext.tenant)
         pipeline.source_context = conf['_source_context']
+        pipeline.start_mark = conf['_start_mark']
         pipeline.description = conf.get('description')
 
         precedence = model.PRECEDENCE_MAP[conf.get('precedence')]

zuul/model.py

@@ -240,6 +240,7 @@ class Pipeline(object):
         # reach the currently active layout for that tenant.
         self.tenant = tenant
         self.source_context = None
+        self.start_mark = None
         self.description = None
         self.failure_message = None
         self.merge_failure_message = None
@@ -3372,6 +3373,11 @@ class Layout(object):
                             "may not be added to %s" % (
                                 pipeline.tenant,
                                 self.tenant))
+
+        if pipeline.name in self.pipelines:
+            raise Exception(
+                "Pipeline %s is already defined" % pipeline.name)
+
         self.pipelines[pipeline.name] = pipeline
 
     def addProjectTemplate(self, project_template):