Merge "Normalize semaphore branch handling"
diff --git a/doc/source/user/config.rst b/doc/source/user/config.rst
index 3f3a412..7fdf96e 100644
--- a/doc/source/user/config.rst
+++ b/doc/source/user/config.rst
@@ -1301,9 +1301,19 @@
represents the maximum number of jobs which use that semaphore at the
same time.
+Semaphores, like most configuration items, are globally unique, though
+a semaphore may be defined on multiple branches of the same project as
+long as the value is 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.
+
Semaphores are never subject to dynamic reconfiguration. If the value
of a semaphore is changed, it will take effect only when the change
-where it is updated is merged. An example follows:
+where it is updated is merged. However, Zuul will attempt to validate
+the configuration of semaphores in proposed updates, even if they
+aren't used.
+
+An example usage of semaphores follows:
.. code-block:: yaml
diff --git a/tests/fixtures/config/semaphore-branches/git/common-config/playbooks/base.yaml b/tests/fixtures/config/semaphore-branches/git/common-config/playbooks/base.yaml
new file mode 100644
index 0000000..f679dce
--- /dev/null
+++ b/tests/fixtures/config/semaphore-branches/git/common-config/playbooks/base.yaml
@@ -0,0 +1,2 @@
+- hosts: all
+ tasks: []
diff --git a/tests/fixtures/config/semaphore-branches/git/common-config/zuul.yaml b/tests/fixtures/config/semaphore-branches/git/common-config/zuul.yaml
new file mode 100644
index 0000000..e1e2fb7
--- /dev/null
+++ b/tests/fixtures/config/semaphore-branches/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/semaphore-branches/git/org_project1/README b/tests/fixtures/config/semaphore-branches/git/org_project1/README
new file mode 100644
index 0000000..9daeafb
--- /dev/null
+++ b/tests/fixtures/config/semaphore-branches/git/org_project1/README
@@ -0,0 +1 @@
+test
diff --git a/tests/fixtures/config/semaphore-branches/git/org_project1/zuul.yaml b/tests/fixtures/config/semaphore-branches/git/org_project1/zuul.yaml
new file mode 100644
index 0000000..73766e0
--- /dev/null
+++ b/tests/fixtures/config/semaphore-branches/git/org_project1/zuul.yaml
@@ -0,0 +1,13 @@
+- semaphore:
+ name: project1-semaphore
+ max: 2
+
+- job:
+ parent: base
+ name: project1-test
+ semaphore: project1-semaphore
+
+- project:
+ check:
+ jobs:
+ - project1-test
diff --git a/tests/fixtures/config/semaphore-branches/git/org_project2/README b/tests/fixtures/config/semaphore-branches/git/org_project2/README
new file mode 100644
index 0000000..9daeafb
--- /dev/null
+++ b/tests/fixtures/config/semaphore-branches/git/org_project2/README
@@ -0,0 +1 @@
+test
diff --git a/tests/fixtures/config/semaphore-branches/git/org_project2/zuul-semaphore.yaml b/tests/fixtures/config/semaphore-branches/git/org_project2/zuul-semaphore.yaml
new file mode 100644
index 0000000..db93fdb
--- /dev/null
+++ b/tests/fixtures/config/semaphore-branches/git/org_project2/zuul-semaphore.yaml
@@ -0,0 +1,16 @@
+- semaphore:
+ name: project2-semaphore
+ max: 2
+
+- job:
+ parent: base
+ name: project2-test
+ semaphore: project2-semaphore
+
+- project:
+ check:
+ jobs:
+ - project2-test
+ gate:
+ jobs:
+ - noop
diff --git a/tests/fixtures/config/semaphore-branches/git/org_project2/zuul.yaml b/tests/fixtures/config/semaphore-branches/git/org_project2/zuul.yaml
new file mode 100644
index 0000000..a4b42b1
--- /dev/null
+++ b/tests/fixtures/config/semaphore-branches/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/semaphore-branches/main.yaml b/tests/fixtures/config/semaphore-branches/main.yaml
new file mode 100644
index 0000000..950b117
--- /dev/null
+++ b/tests/fixtures/config/semaphore-branches/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 892f7f2..d3a6ec8 100755
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -3012,6 +3012,141 @@
A.messages[0])
+class TestSemaphoreBranches(ZuulTestCase):
+ tenant_config_file = 'config/semaphore-branches/main.yaml'
+
+ def test_semaphore_branch(self):
+ # Test that we can use a semaphore 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/semaphore-branches/git/',
+ 'org_project2/zuul-semaphore.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
+ semaphore: project2-semaphore
+
+ - 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')
+ ])
+
+ def test_semaphore_branch_duplicate(self):
+ # Test that we can create a duplicate semaphore on a different
+ # branch of the same project -- i.e., that when we branch
+ # master to stable on a project with a semaphore, 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')
+ ])
+
+ def test_semaphore_branch_error_same_branch(self):
+ # Test that we are unable to define a semaphore twice on the same
+ # project-branch.
+ in_repo_conf = textwrap.dedent(
+ """
+ - semaphore:
+ name: project1-semaphore
+ max: 2
+ - semaphore:
+ name: project1-semaphore
+ max: 2
+ """)
+ 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_semaphore_branch_error_same_project(self):
+ # Test that we are unable to create a semaphore which differs
+ # from another with the same name -- i.e., that if we have a
+ # duplicate semaphore 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(
+ """
+ - semaphore:
+ name: project1-semaphore
+ max: 4
+ """)
+ 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_semaphore_branch_error_other_project(self):
+ # Test that we are unable to create a semaphore with the same
+ # name as another. We're never allowed to have a semaphore with
+ # the same name outside of a project.
+ in_repo_conf = textwrap.dedent(
+ """
+ - semaphore:
+ name: project1-semaphore
+ max: 2
+ """)
+ 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 c3a1be3..4f93907 100644
--- a/zuul/configloader.py
+++ b/zuul/configloader.py
@@ -1622,23 +1622,22 @@
if parent:
layout.getJob(parent)
- if not skip_semaphores:
- for config_semaphore in data.semaphores:
- classes = TenantParser._getLoadClasses(
- tenant, config_semaphore)
- if 'semaphore' not in classes:
- continue
+ if skip_semaphores:
+ # We should not actually update the layout with new
+ # semaphores, but so that we can validate that the config
+ # is correct, create a shadow layout here to which we add
+ # new semaphores so validation is complete.
+ semaphore_layout = model.Layout(tenant)
+ else:
+ semaphore_layout = layout
+ for config_semaphore in data.semaphores:
+ classes = TenantParser._getLoadClasses(
+ tenant, config_semaphore)
+ if 'semaphore' not in classes:
+ continue
+ with configuration_exceptions('semaphore', config_semaphore):
semaphore = SemaphoreParser.fromYaml(config_semaphore)
- old_semaphore = layout.semaphores.get(semaphore.name)
- if (old_semaphore and
- (old_semaphore.source_context.project ==
- semaphore.source_context.project)):
- # If a semaphore shows up twice in the same
- # project, it's probably due to showing up in
- # two branches. Ignore subsequent
- # definitions.
- continue
- layout.addSemaphore(semaphore)
+ semaphore_layout.addSemaphore(semaphore)
project_template_parser = ProjectTemplateParser(tenant, layout)
for config_template in data.project_templates:
diff --git a/zuul/model.py b/zuul/model.py
index 5596533..96ec85b 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -2627,8 +2627,25 @@
self.secrets[secret.name] = secret
def addSemaphore(self, semaphore):
- if semaphore.name in self.semaphores:
- raise Exception("Semaphore %s already defined" % (semaphore.name,))
+ # It's ok to have a duplicate semaphore definition, but only if
+ # they are in different branches of the same repo, and have
+ # the same values.
+ other = self.semaphores.get(semaphore.name)
+ if other is not None:
+ if not semaphore.source_context.isSameProject(
+ other.source_context):
+ raise Exception("Semaphore %s already defined in project %s" %
+ (semaphore.name, other.source_context.project))
+ if semaphore.source_context.branch == other.source_context.branch:
+ raise Exception("Semaphore %s already defined" %
+ (semaphore.name,))
+ if semaphore != other:
+ raise Exception("Semaphore %s does not match existing"
+ " definition in branch %s" %
+ (semaphore.name, other.source_context.branch))
+ # Identical data in a different branch of the same project;
+ # ignore the duplicate definition
+ return
self.semaphores[semaphore.name] = semaphore
def addPipeline(self, pipeline):
@@ -2783,6 +2800,15 @@
self.name = name
self.max = int(max)
+ def __ne__(self, other):
+ return not self.__eq__(other)
+
+ def __eq__(self, other):
+ if not isinstance(other, Semaphore):
+ return False
+ return (self.name == other.name and
+ self.max == other.max)
+
class SemaphoreHandler(object):
log = logging.getLogger("zuul.SemaphoreHandler")