Validate dependent job names
It is possible to merge an invalid Zuul config by having a dependency on a job which does not exist (e.g., due to a typo in the job name). If this happens, the problem will only be detected when the job graph is frozen. At this point, the user will need to try to find out where the erroneus reference is defined. To try to avoid this situation, we can validate job dependency references early, as we do most other configuration objects (such as references to nodesets from jobs). This should help users avoid merging new errors. Existing jobs and project stanzas with reference errors will become invalid and ineffective, which may cause some collatoral damage, so a release note is included. This may have escaped notice so far because within a single repo, attempting to add a bad config would trigger an error freezing the job graph and therefore bringing the error to the attention of the user. Having a project stanza in a config project is much more likely to produce the issue since the job graph would not be frozen as part of that change. Change-Id: I196a2fb13e93847bc4db4b20f30dea8fecba6325
This commit is contained in:
parent
7b1f2cf14b
commit
026de6587b
|
@ -0,0 +1,12 @@
|
|||
---
|
||||
fixes:
|
||||
- |
|
||||
Zuul now treats job dependencies that reference undefined jobs as
|
||||
a configuration error. Previously a job which depended on another
|
||||
job which did not exist would pass initial syntax validation and
|
||||
only cause a failure in freezing the job graph when Zuul attempted
|
||||
to run the job. Now incorrect or missing job dependencies are
|
||||
detected during configuration. This means that new config errors
|
||||
may be prevented from merging. It also means that existing
|
||||
erroneous job or project configurations will be regarded as
|
||||
configuration errors at startup.
|
|
@ -1479,6 +1479,38 @@ class TestInRepoConfig(ZuulTestCase):
|
|||
"A should have failed the check pipeline")
|
||||
self.assertHistory([])
|
||||
|
||||
def test_dynamic_nonexistent_job_dependency(self):
|
||||
# Tests that a reference to a nonexistent job dependency is an
|
||||
# error.
|
||||
in_repo_conf = textwrap.dedent(
|
||||
"""
|
||||
- job:
|
||||
name: project-test1
|
||||
run: playbooks/project-test1.yaml
|
||||
|
||||
- project:
|
||||
name: org/project
|
||||
check:
|
||||
jobs:
|
||||
- project-test1:
|
||||
dependencies:
|
||||
- name: non-existent-job
|
||||
soft: true
|
||||
""")
|
||||
|
||||
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,
|
||||
"A should report failure")
|
||||
self.assertEqual(A.patchsets[0]['approvals'][0]['value'], "-1")
|
||||
self.assertIn('Job non-existent-job not defined', A.messages[0],
|
||||
"A should have failed the check pipeline")
|
||||
self.assertNotIn('freezing', A.messages[0])
|
||||
self.assertHistory([])
|
||||
|
||||
def test_dynamic_config_new_patchset(self):
|
||||
self.executor_server.hold_jobs_in_build = True
|
||||
|
||||
|
|
|
@ -2708,6 +2708,8 @@ class Job(ConfigObject):
|
|||
job=self.name,
|
||||
maxnodes=layout.tenant.max_nodes_per_job))
|
||||
|
||||
for dependency in self.dependencies:
|
||||
layout.getJob(dependency.name)
|
||||
for pb in self.pre_run + self.run + self.post_run + self.cleanup_run:
|
||||
pb.validateReferences(layout)
|
||||
|
||||
|
|
Loading…
Reference in New Issue