From 263998a7e46d90410287783de39b7121d0ed06b4 Mon Sep 17 00:00:00 2001 From: Tobias Henkel Date: Wed, 15 Apr 2020 12:39:08 +0200 Subject: [PATCH] Move queue from pipeline to project Referring to a queue makes more sense in the project where it has more use such as allowing circular dependencies later. This makes it possible to refer to the queue on project level which takes precedence over referring queues in the pipeline config. We'll also deprecate referring to the queue in the pipeline config and remove this in a later release. Change-Id: I14533ae57cfe688b55c26a1e17ab38e133180b28 --- doc/source/reference/project_def.rst | 53 +++++++++++-------- doc/source/reference/queue_def.rst | 2 +- ...change-queue-project-790553bd212b50eb.yaml | 6 +++ .../git/common-config/zuul.d/config.yaml | 7 +++ .../change-queues/git/org_project4/readme | 1 + tests/fixtures/config/change-queues/main.yaml | 1 + .../single-tenant/git/common-config/zuul.yaml | 4 +- .../layouts/template-project-queue.yaml | 27 ++++++++++ tests/unit/test_scheduler.py | 30 +++++++++++ tests/unit/test_web.py | 3 +- zuul/configloader.py | 10 +++- zuul/manager/__init__.py | 15 ++++-- zuul/manager/shared.py | 15 ++++-- zuul/model.py | 3 ++ 14 files changed, 143 insertions(+), 34 deletions(-) create mode 100644 releasenotes/notes/change-queue-project-790553bd212b50eb.yaml create mode 100644 tests/fixtures/config/change-queues/git/org_project4/readme create mode 100644 tests/fixtures/layouts/template-project-queue.yaml diff --git a/doc/source/reference/project_def.rst b/doc/source/reference/project_def.rst index 6f663d09e1..ea6f5e2677 100644 --- a/doc/source/reference/project_def.rst +++ b/doc/source/reference/project_def.rst @@ -148,6 +148,31 @@ pipeline. all pipelines of this project. For more information see :ref:`variable inheritance `. + .. attr:: queue + + This specifies the + name of the shared queue this project is in. Any projects + which interact with each other in tests should be part of the + same shared queue in order to ensure that they don't merge + changes which break the others. This is a free-form string; + just set the same value for each group of projects. + + The name can refer to the name of a :attr:`queue` which allows + further configuration of the queue. + + Each pipeline for a project can only belong to one queue, + therefore Zuul will use the first value that it encounters. + It need not appear in the first instance of a :attr:`project` + stanza; it may appear in secondary instances or even in a + :ref:`project-template` definition. + + Pipeline managers other than `dependent` do not use this + attribute, however, it may still be used if + :attr:`scheduler.relative_priority` is enabled. + + .. note:: This attribute is not evaluated speculatively and + its setting shall be merged to be effective. + .. attr:: Each pipeline that the project participates in should have an @@ -168,29 +193,13 @@ pipeline. .. attr:: queue - If this pipeline is a :value:`dependent - ` pipeline, this specifies the - name of the shared queue this project is in. Any projects - which interact with each other in tests should be part of the - same shared queue in order to ensure that they don't merge - changes which break the others. This is a free-form string; - just set the same value for each group of projects. + This is the same as :attr:`project.queue` but on per pipeline + level for backwards compatibility reasons. If :attr:`project.queue` + is defined this setting is ignored. - The name can refer to the name of a :attr:`queue` which allows - further configuration of the queue. - - Each pipeline for a project can only belong to one queue, - therefore Zuul will use the first value that it encounters. - It need not appear in the first instance of a :attr:`project` - stanza; it may appear in secondary instances or even in a - :ref:`project-template` definition. - - Pipeline managers other than `dependent` do not use this - attribute, however, it may still be used if - :attr:`scheduler.relative_priority` is enabled. - - .. note:: This attribute is not evaluated speculatively and - its setting shall be merged to be effective. + .. note:: It is deprecated to define the queue in the pipeline + configuration. Configure it on :attr:`project.queue` + instead. .. attr:: debug diff --git a/doc/source/reference/queue_def.rst b/doc/source/reference/queue_def.rst index d5f106a0b6..78441de25e 100644 --- a/doc/source/reference/queue_def.rst +++ b/doc/source/reference/queue_def.rst @@ -5,7 +5,7 @@ Queue Projects that interact with each other should share a ``queue``. This is especially used in a :value:`dependent ` -pipeline. The :attr:`project..queue` can optionally refer +pipeline. The :attr:`project.queue` can optionally refer to a specific :attr:`queue` object that can further configure the behavior of the queue. diff --git a/releasenotes/notes/change-queue-project-790553bd212b50eb.yaml b/releasenotes/notes/change-queue-project-790553bd212b50eb.yaml new file mode 100644 index 0000000000..a68db6f172 --- /dev/null +++ b/releasenotes/notes/change-queue-project-790553bd212b50eb.yaml @@ -0,0 +1,6 @@ +--- +deprecations: + - | + Shared ``queues`` should be configured per project now instead per + pipeline. Specifying :attr:`project..queue` is deprecated + and will be removed in a future release. diff --git a/tests/fixtures/config/change-queues/git/common-config/zuul.d/config.yaml b/tests/fixtures/config/change-queues/git/common-config/zuul.d/config.yaml index 0946ca3f95..0fe3738666 100644 --- a/tests/fixtures/config/change-queues/git/common-config/zuul.d/config.yaml +++ b/tests/fixtures/config/change-queues/git/common-config/zuul.d/config.yaml @@ -33,3 +33,10 @@ queue: integrated jobs: - project-test + +- project: + name: org/project4 + queue: integrated + gate: + jobs: + - project-test diff --git a/tests/fixtures/config/change-queues/git/org_project4/readme b/tests/fixtures/config/change-queues/git/org_project4/readme new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/change-queues/git/org_project4/readme @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/change-queues/main.yaml b/tests/fixtures/config/change-queues/main.yaml index 7eacee534d..17e042f563 100644 --- a/tests/fixtures/config/change-queues/main.yaml +++ b/tests/fixtures/config/change-queues/main.yaml @@ -8,3 +8,4 @@ - org/project - org/project2 - org/project3 + - org/project4 diff --git a/tests/fixtures/config/single-tenant/git/common-config/zuul.yaml b/tests/fixtures/config/single-tenant/git/common-config/zuul.yaml index c9c0ee6466..b29f5a654e 100644 --- a/tests/fixtures/config/single-tenant/git/common-config/zuul.yaml +++ b/tests/fixtures/config/single-tenant/git/common-config/zuul.yaml @@ -137,6 +137,7 @@ - project: name: org/project1 + queue: integrated check: jobs: - project-merge @@ -147,7 +148,8 @@ - project1-project2-integration: dependencies: project-merge gate: - queue: integrated + # This will be overridden on project level + queue: integrated-overridden jobs: - project-merge - project-test1: diff --git a/tests/fixtures/layouts/template-project-queue.yaml b/tests/fixtures/layouts/template-project-queue.yaml new file mode 100644 index 0000000000..ffebd95d6a --- /dev/null +++ b/tests/fixtures/layouts/template-project-queue.yaml @@ -0,0 +1,27 @@ +- pipeline: + name: gate + manager: dependent + trigger: {} + +- job: + name: base + parent: null + run: playbooks/base.yaml + +- project-template: + name: integrated-jobs + gate: + jobs: + - base + +- project: + name: org/project1 + queue: integrated + templates: + - integrated-jobs + +- project: + name: org/project2 + queue: integrated + templates: + - integrated-jobs diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index 1fa9ff9f32..afa1b09d50 100644 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -3195,6 +3195,27 @@ class TestScheduler(ZuulTestCase): self.assertEqual(q1.name, 'integrated') self.assertEqual(q2.name, 'integrated') + @simple_layout("layouts/template-project-queue.yaml") + def test_template_project_queue(self): + "Test a shared queue can be constructed from a project-template" + tenant = self.scheds.first.sched.abide.tenants.get('tenant-one') + (trusted, project1) = tenant.getProject('org/project1') + (trusted, project2) = tenant.getProject('org/project2') + + # Change queues are created lazy by the dependent pipeline manager + # so retrieve the queue first without having to really enqueue a + # change first. + gate = tenant.layout.pipelines['gate'] + FakeChange = namedtuple('FakeChange', ['project', 'branch']) + fake_a = FakeChange(project1, 'master') + fake_b = FakeChange(project2, 'master') + gate.manager.getChangeQueue(fake_a, None) + gate.manager.getChangeQueue(fake_b, None) + q1 = gate.getQueue(project1, None) + q2 = gate.getQueue(project2, None) + self.assertEqual(q1.name, 'integrated') + self.assertEqual(q2.name, 'integrated') + @simple_layout("layouts/regex-template-queue.yaml") def test_regex_template_queue(self): "Test a shared queue can be constructed from a regex project-template" @@ -6447,6 +6468,15 @@ class TestChangeQueues(ZuulTestCase): 'org/project3', queue_name='integrated-untrusted', queue_repo='org/project3') + def test_dependent_queues_per_branch_project_queue(self): + """ + Test that change queues can be different for different branches. + + In this case we create changes for two branches in a repo that + references the queue on project level instead of pipeline level. + """ + self._test_dependent_queues_per_branch('org/project4') + class TestJobUpdateBrokenConfig(ZuulTestCase): tenant_config_file = 'config/job-update-broken/main.yaml' diff --git a/tests/unit/test_web.py b/tests/unit/test_web.py index 68e485a986..48df4810d6 100644 --- a/tests/unit/test_web.py +++ b/tests/unit/test_web.py @@ -712,6 +712,7 @@ class TestWeb(BaseTestWeb): 'configs': [{ 'templates': [], 'default_branch': 'master', + 'queue_name': 'integrated', 'merge_mode': 'merge-resolve', 'pipelines': [{ 'name': 'check', @@ -719,7 +720,7 @@ class TestWeb(BaseTestWeb): 'jobs': jobs, }, { 'name': 'gate', - 'queue_name': 'integrated', + 'queue_name': 'integrated-overridden', 'jobs': jobs, }, {'name': 'post', 'queue_name': None, diff --git a/zuul/configloader.py b/zuul/configloader.py index cf2fd296ab..44291a1550 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -969,13 +969,14 @@ class ProjectTemplateParser(object): self.schema = self.getSchema() self.not_pipelines = ['name', 'description', 'templates', 'merge-mode', 'default-branch', 'vars', - '_source_context', '_start_mark'] + 'queue', '_source_context', '_start_mark'] def getSchema(self): job = {str: vs.Any(str, JobParser.job_attributes)} job_list = [vs.Any(str, job)] pipeline_contents = { + # TODO(tobiash): Remove pipeline specific queue after deprecation 'queue': str, 'debug': bool, 'fail-fast': bool, @@ -985,6 +986,7 @@ class ProjectTemplateParser(object): project = { 'name': str, 'description': str, + 'queue': str, 'vars': ansible_vars_dict, str: pipeline_contents, '_source_context': model.SourceContext, @@ -1001,11 +1003,13 @@ class ProjectTemplateParser(object): project_template = model.ProjectConfig(conf.get('name')) project_template.source_context = conf['_source_context'] project_template.start_mark = conf['_start_mark'] + project_template.queue_name = conf.get('queue') for pipeline_name, conf_pipeline in conf.items(): if pipeline_name in self.not_pipelines: continue project_pipeline = model.ProjectPipelineConfig() project_template.pipelines[pipeline_name] = project_pipeline + # TODO(tobiash): Remove pipeline specific queue after deprecation project_pipeline.queue_name = conf_pipeline.get('queue') project_pipeline.debug = conf_pipeline.get('debug') project_pipeline.fail_fast = conf_pipeline.get( @@ -1060,6 +1064,7 @@ class ProjectParser(object): job_list = [vs.Any(str, job)] pipeline_contents = { + # TODO(tobiash): Remove pipeline specific queue after deprecation 'queue': str, 'debug': bool, 'fail-fast': bool, @@ -1074,6 +1079,7 @@ class ProjectParser(object): 'merge-mode': vs.Any('merge', 'merge-resolve', 'cherry-pick', 'squash-merge'), 'default-branch': str, + 'queue': str, str: pipeline_contents, '_source_context': model.SourceContext, '_start_mark': ZuulMark, @@ -1139,6 +1145,8 @@ class ProjectParser(object): default_branch = conf.get('default-branch', 'master') project_config.default_branch = default_branch + project_config.queue_name = conf.get('queue', None) + variables = conf.get('vars', {}) if variables: if 'zuul' in variables or 'nodepool' in variables: diff --git a/zuul/manager/__init__.py b/zuul/manager/__init__.py index f511012642..00e3f48a24 100644 --- a/zuul/manager/__init__.py +++ b/zuul/manager/__init__.py @@ -72,7 +72,8 @@ class PipelineManager(metaclass=ABCMeta): for project_name, project_configs in layout_project_configs.items(): (trusted, project) = tenant.getProject(project_name) - queue_name = None + project_queue_name = None + pipeline_queue_name = None project_in_pipeline = False for project_config in layout.getAllProjectConfigs(project_name): project_pipeline_config = project_config.pipelines.get( @@ -80,11 +81,17 @@ class PipelineManager(metaclass=ABCMeta): if project_pipeline_config is None: continue project_in_pipeline = True - queue_name = project_pipeline_config.queue_name - if queue_name: - break + if not pipeline_queue_name: + pipeline_queue_name = project_pipeline_config.queue_name + if not project_queue_name: + project_queue_name = project_config.queue_name if not project_in_pipeline: continue + + # Note: we currently support queue name per pipeline and per + # project while project has precedence. + queue_name = project_queue_name or pipeline_queue_name + if not queue_name: continue if queue_name in change_queues: diff --git a/zuul/manager/shared.py b/zuul/manager/shared.py index f2ea53b61f..c45cd7d690 100644 --- a/zuul/manager/shared.py +++ b/zuul/manager/shared.py @@ -71,20 +71,27 @@ class SharedQueuePipelineManager(PipelineManager, metaclass=ABCMeta): for project_name, project_configs in layout_project_configs.items(): (trusted, project) = tenant.getProject(project_name) - queue_name = None + project_queue_name = None + pipeline_queue_name = None project_in_pipeline = False for project_config in layout.getAllProjectConfigs(project_name): project_pipeline_config = project_config.pipelines.get( self.pipeline.name) + if not project_queue_name: + project_queue_name = project_config.queue_name if project_pipeline_config is None: continue project_in_pipeline = True - queue_name = project_pipeline_config.queue_name - if queue_name: - break + # TODO(tobiash): Remove pipeline_queue_name after deprecation + if not pipeline_queue_name: + pipeline_queue_name = project_pipeline_config.queue_name if not project_in_pipeline: continue + # Note: we currently support queue name per pipeline and per + # project while project has precedence. + queue_name = project_queue_name or pipeline_queue_name + # Check if the queue is global or per branch queue = layout.queues.get(queue_name) per_branch = queue and queue.per_branch diff --git a/zuul/model.py b/zuul/model.py index 2ff3ff3ab8..73e2db2ffc 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -3489,6 +3489,7 @@ class ProjectConfig(ConfigObject): # stanzas. self.merge_mode = None self.default_branch = None + self.queue_name = None def __repr__(self): return '' % ( @@ -3504,6 +3505,7 @@ class ProjectConfig(ConfigObject): r.variables = self.variables r.merge_mode = self.merge_mode r.default_branch = self.default_branch + r.queue_name = self.queue_name return r def setImpliedBranchMatchers(self, branches): @@ -3531,6 +3533,7 @@ class ProjectConfig(ConfigObject): else: d['merge_mode'] = None d['templates'] = self.templates + d['queue_name'] = self.queue_name return d