zuul
/
zuul
Code Issues Proposed changes

Add intermediate flag for jobs 

I recently had an issue using our nodepool container jobs from the dib
repository.  Nodepool jobs define a base container job and then two
variants; one for installing with released tools and one for
installing with siblings from git.  In this case, you never really
want to inherit from the base job; you want to choose one of the two
variants.  Although the base job was marked abstract, it didn't really
stop me accidentally inheriting from it directly.

This proposes an "intermediate" flag that would be applied to such a
base job.  In a case such as above, it would have raised an error and
alerted me to use one of the two other jobs.

Change-Id: Ifbb9ffa65f86a6b86b63a38e3234a12b564ba3c1
This commit is contained in:
Ian Wienand 2020-04-23 15:29:10 +10:00
parent 37a7e372c4
commit ebf65d3de8
10 changed files with 186 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
doc/source/reference/job_def.rst
View File

@ -153,6 +153,21 @@ Here is an example of two job definitions:
limitation does not apply to jobs in a
:term:`config-project`.
.. attr:: intermediate
:default: false
An intermediate job must be inherited by an abstract job; it can
not be inherited by a final job. All ``intermediate`` jobs
*must* also be ``abstract``; a configuration error will be
raised if not.
For example, you may define a base abstract job `foo` and create
two abstract jobs that inherit from `foo` called
`foo-production` and `foo-development`. If it would be an error
to accidentally inherit from the base job `foo` instead of
choosing one of the two variants, `foo` could be marked as
``intermediate``.
.. attr:: success-message
:default: SUCCESS

5
releasenotes/notes/intermediate-jobs-101e04e7e1497af9.yaml Normal file
View File

@ -0,0 +1,5 @@
---
features:
- Jobs may specify the new ``intermediate`` flag to note they may only
be inherited by abstract jobs. This can be useful if building a job
hierarchy where wish to limit where a base job is instantiated.

2
tests/fixtures/config/intermediate/git/common-config/playbooks/base.yaml vendored Normal file
View File

@ -0,0 +1,2 @@
- hosts: all
tasks: []

50
tests/fixtures/config/intermediate/git/common-config/zuul.yaml vendored Normal file
View File

@ -0,0 +1,50 @@
- pipeline:
name: check
manager: independent
trigger:
gerrit:
- event: patchset-created
success:
gerrit:
Verified: 1
failure:
gerrit:
Verified: -1
- job:
name: base
parent: null
run: playbooks/base.yaml
- job:
name: job-abstract-intermediate
abstract: true
intermediate: true
- job:
name: job-abstract
abstract: true
parent: job-abstract-intermediate
# an intermediate, with an intermediate parent also
- job:
name: job-another-intermediate
parent: job-abstract-intermediate
abstract: true
intermediate: true
- job:
name: job-another-abstract
parent: job-another-intermediate
abstract: true
- job:
name: job-actual
parent: job-another-abstract
run: playbooks/base.yaml
- project:
name: org/project
check:
jobs: []

1
tests/fixtures/config/intermediate/git/org_project/README vendored Normal file
View File

@ -0,0 +1 @@
test

8
tests/fixtures/config/intermediate/main.yaml vendored Normal file
View File

@ -0,0 +1,8 @@
- tenant:
name: tenant-one
source:
gerrit:
config-projects:
- common-config
untrusted-projects:
- org/project

75
tests/unit/test_v3.py
View File

@ -221,6 +221,81 @@ class TestAbstract(ZuulTestCase):
self.assertEqual(A.patchsets[-1]['approvals'][0]['value'], '1')
class TestIntermediate(ZuulTestCase):
tenant_config_file = 'config/intermediate/main.yaml'
def test_intermediate_fail(self):
# you can not instantiate from an intermediate job
in_repo_conf = textwrap.dedent(
"""
- job:
name: job-instantiate-intermediate
parent: job-abstract-intermediate
- project:
check:
jobs:
- job-instantiate-intermediate
""")
file_dict = {'zuul.yaml': in_repo_conf}
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
files=file_dict)
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.assertEqual(A.reported, 1)
self.assertEqual(A.patchsets[-1]['approvals'][0]['value'], '-1')
self.assertIn('may only inherit to another abstract job',
A.messages[0])
def test_intermediate_config_fail(self):
# an intermediate job must also be abstract
in_repo_conf = textwrap.dedent(
"""
- job:
name: job-intermediate-but-not-abstract
intermediate: true
abstract: false
- project:
check:
jobs:
- job-intermediate-but-not-abstract
""")
file_dict = {'zuul.yaml': in_repo_conf}
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
files=file_dict)
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.assertEqual(A.reported, 1)
self.assertEqual(A.patchsets[-1]['approvals'][0]['value'], '-1')
self.assertIn('An intermediate job must also be abstract',
A.messages[0])
def test_intermediate_several(self):
# test passing through several intermediate jobs
in_repo_conf = textwrap.dedent(
"""
- project:
name: org/project
check:
jobs:
- job-actual
""")
file_dict = {'.zuul.yaml': in_repo_conf}
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
files=file_dict)
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.assertEqual(A.reported, 1)
self.assertEqual(A.patchsets[-1]['approvals'][0]['value'], '1')
class TestFinal(ZuulTestCase):
tenant_config_file = 'config/final/main.yaml'

8
tests/unit/test_web.py
View File

@ -335,6 +335,7 @@ class TestWeb(BaseTestWeb):
'dependencies': [],
'description': None,
'files': [],
'intermediate': False,
'irrelevant_files': [],
'match_on_config_updates': True,
'final': False,
@ -376,6 +377,7 @@ class TestWeb(BaseTestWeb):
'dependencies': [],
'description': None,
'files': [],
'intermediate': False,
'irrelevant_files': [],
'match_on_config_updates': True,
'final': False,
@ -422,6 +424,7 @@ class TestWeb(BaseTestWeb):
'description': None,
'files': [],
'final': False,
'intermediate': False,
'irrelevant_files': [],
'match_on_config_updates': True,
'name': 'test-job',
@ -540,6 +543,7 @@ class TestWeb(BaseTestWeb):
'description': None,
'files': [],
'final': False,
'intermediate': False,
'irrelevant_files': [],
'match_on_config_updates': True,
'name': 'project-merge',
@ -574,6 +578,7 @@ class TestWeb(BaseTestWeb):
'description': None,
'files': [],
'final': False,
'intermediate': False,
'irrelevant_files': [],
'match_on_config_updates': True,
'name': 'project-test1',
@ -608,6 +613,7 @@ class TestWeb(BaseTestWeb):
'description': None,
'files': [],
'final': False,
'intermediate': False,
'irrelevant_files': [],
'match_on_config_updates': True,
'name': 'project-test2',
@ -642,6 +648,7 @@ class TestWeb(BaseTestWeb):
'description': None,
'files': [],
'final': False,
'intermediate': False,
'irrelevant_files': [],
'match_on_config_updates': True,
'name': 'project1-project2-integration',
@ -696,6 +703,7 @@ class TestWeb(BaseTestWeb):
'description': None,
'files': [],
'final': False,
'intermediate': False,
'irrelevant_files': [],
'match_on_config_updates': True,
'name': 'project-post',

5
zuul/configloader.py
View File

@ -598,6 +598,7 @@ class JobParser(object):
'final': bool,
'abstract': bool,
'protected': bool,
'intermediate': bool,
'requires': to_list(str),
'provides': to_list(str),
'failure-message': str,
@ -650,6 +651,7 @@ class JobParser(object):
'final',
'abstract',
'protected',
'intermediate',
'timeout',
'post-timeout',
'workspace',
@ -805,6 +807,9 @@ class JobParser(object):
job.roles, secrets)
job.run = job.run + (run,)
if conf.get('intermediate', False) and not conf.get('abstract', False):
raise Exception("An intermediate job must also be abstract")
for k in self.simple_attributes:
a = k.replace('-', '_')
if k in conf:

18
zuul/model.py
View File

@ -1198,6 +1198,7 @@ class Job(ConfigObject):
attempts=3,
final=False,
abstract=False,
intermediate=False,
protected=None,
roles=(),
required_projects={},
@ -1260,6 +1261,7 @@ class Job(ConfigObject):
d['variables'] = self.variables
d['final'] = self.final
d['abstract'] = self.abstract
d['intermediate'] = self.intermediate
d['protected'] = self.protected
d['voting'] = self.voting
d['timeout'] = self.timeout
@ -1551,7 +1553,8 @@ class Job(ConfigObject):
for k in self.execution_attributes:
if (other._get(k) is not None and
k not in set(['final', 'abstract', 'protected'])):
k not in set(['final', 'abstract', 'protected',
'intermediate'])):
if self.final:
raise Exception("Unable to modify final job %s attribute "
"%s=%s with variant %s" % (
@ -1584,6 +1587,19 @@ class Job(ConfigObject):
elif other.abstract:
self.abstract = True
# An intermediate job may only be inherited by an abstract
# job. Note intermediate jobs must be also be abstract, that
# has been enforced during config reading. Similar to
# abstract, it is cleared by inheriting.
if self.intermediate and not other.abstract:
raise Exception("Intermediate job %s may only inherit "
"to another abstract job" %
(repr(self)))
if other.name != self.name:
self.intermediate = other.intermediate
elif other.intermediate:
self.intermediate = True
# Protected may only be set to true
if other.protected is not None:
# don't allow to reset protected flag