Ignore parents on job variants
Job variants generally should not set the parent attribute. Instead,
the inheritance path of the reference definition of the job should be
used.
I had previously considered honoring parent in variants a harmless
undocumented feature (which permitted a sort of crude
multiple-inheritance). However, this makes it very difficult, if not
impossible, to create correct branch variants of jobs from the initial
branch point -- a new branch will end up with an identical copy of the
jobs from the master branch, including the parent attribute. Since
the behavior difference between the jobs on the two branches runs counter
to what the user would expect, let's ignore the parent attribute on
variants.
Change-Id: I85ffb014c73e39631c76debb04d8c9775ab097ad
diff --git a/tests/fixtures/config/branch-variants/git/project-config/playbooks/base/post-logs.yaml b/tests/fixtures/config/branch-variants/git/project-config/playbooks/base/post-logs.yaml
new file mode 100644
index 0000000..f679dce
--- /dev/null
+++ b/tests/fixtures/config/branch-variants/git/project-config/playbooks/base/post-logs.yaml
@@ -0,0 +1,2 @@
+- hosts: all
+ tasks: []
diff --git a/tests/fixtures/config/branch-variants/git/project-config/playbooks/base/post-ssh.yaml b/tests/fixtures/config/branch-variants/git/project-config/playbooks/base/post-ssh.yaml
new file mode 100644
index 0000000..f679dce
--- /dev/null
+++ b/tests/fixtures/config/branch-variants/git/project-config/playbooks/base/post-ssh.yaml
@@ -0,0 +1,2 @@
+- hosts: all
+ tasks: []
diff --git a/tests/fixtures/config/branch-variants/git/project-config/playbooks/base/pre.yaml b/tests/fixtures/config/branch-variants/git/project-config/playbooks/base/pre.yaml
new file mode 100644
index 0000000..f679dce
--- /dev/null
+++ b/tests/fixtures/config/branch-variants/git/project-config/playbooks/base/pre.yaml
@@ -0,0 +1,2 @@
+- hosts: all
+ tasks: []
diff --git a/tests/fixtures/config/branch-variants/git/project-config/zuul.yaml b/tests/fixtures/config/branch-variants/git/project-config/zuul.yaml
new file mode 100644
index 0000000..89d98a9
--- /dev/null
+++ b/tests/fixtures/config/branch-variants/git/project-config/zuul.yaml
@@ -0,0 +1,25 @@
+- pipeline:
+ name: check
+ manager: independent
+ trigger:
+ gerrit:
+ - event: patchset-created
+ success:
+ gerrit:
+ Verified: 1
+ failure:
+ gerrit:
+ Verified: -1
+
+- job:
+ name: base
+ parent: null
+ pre-run: playbooks/base/pre
+ post-run:
+ - playbooks/base/post-ssh
+ - playbooks/base/post-logs
+
+- project:
+ name: project-config
+ check:
+ jobs: []
diff --git a/tests/fixtures/config/branch-variants/git/puppet-integration/.zuul.yaml b/tests/fixtures/config/branch-variants/git/puppet-integration/.zuul.yaml
new file mode 100644
index 0000000..2545208
--- /dev/null
+++ b/tests/fixtures/config/branch-variants/git/puppet-integration/.zuul.yaml
@@ -0,0 +1,24 @@
+- job:
+ name: puppet-base
+ pre-run: playbooks/prepare-node-common
+
+- job:
+ name: puppet-module-base
+ parent: puppet-base
+ pre-run: playbooks/prepare-node-unit
+
+- job:
+ name: puppet-lint
+ parent: puppet-module-base
+ run: playbooks/run-lint
+
+- project-template:
+ name: puppet-check-jobs
+ check:
+ jobs:
+ - puppet-lint
+
+- project:
+ name: puppet-integration
+ templates:
+ - puppet-check-jobs
diff --git a/tests/fixtures/config/branch-variants/git/puppet-integration/README b/tests/fixtures/config/branch-variants/git/puppet-integration/README
new file mode 100644
index 0000000..9daeafb
--- /dev/null
+++ b/tests/fixtures/config/branch-variants/git/puppet-integration/README
@@ -0,0 +1 @@
+test
diff --git a/tests/fixtures/config/branch-variants/git/puppet-integration/playbooks/prepare-node-common.yaml b/tests/fixtures/config/branch-variants/git/puppet-integration/playbooks/prepare-node-common.yaml
new file mode 100644
index 0000000..f679dce
--- /dev/null
+++ b/tests/fixtures/config/branch-variants/git/puppet-integration/playbooks/prepare-node-common.yaml
@@ -0,0 +1,2 @@
+- hosts: all
+ tasks: []
diff --git a/tests/fixtures/config/branch-variants/git/puppet-integration/playbooks/prepare-node-unit.yaml b/tests/fixtures/config/branch-variants/git/puppet-integration/playbooks/prepare-node-unit.yaml
new file mode 100644
index 0000000..f679dce
--- /dev/null
+++ b/tests/fixtures/config/branch-variants/git/puppet-integration/playbooks/prepare-node-unit.yaml
@@ -0,0 +1,2 @@
+- hosts: all
+ tasks: []
diff --git a/tests/fixtures/config/branch-variants/git/puppet-integration/playbooks/run-lint.yaml b/tests/fixtures/config/branch-variants/git/puppet-integration/playbooks/run-lint.yaml
new file mode 100644
index 0000000..f679dce
--- /dev/null
+++ b/tests/fixtures/config/branch-variants/git/puppet-integration/playbooks/run-lint.yaml
@@ -0,0 +1,2 @@
+- hosts: all
+ tasks: []
diff --git a/tests/fixtures/config/branch-variants/main.yaml b/tests/fixtures/config/branch-variants/main.yaml
new file mode 100644
index 0000000..ad2b2f6
--- /dev/null
+++ b/tests/fixtures/config/branch-variants/main.yaml
@@ -0,0 +1,8 @@
+- tenant:
+ name: tenant-one
+ source:
+ gerrit:
+ config-projects:
+ - project-config
+ untrusted-projects:
+ - puppet-integration
diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py
index 0d081d5..bfb9088 100755
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -155,6 +155,29 @@
self.assertIn('Unable to inherit from final job', A.messages[0])
+class TestBranchVariants(ZuulTestCase):
+ tenant_config_file = 'config/branch-variants/main.yaml'
+
+ def test_branch_variants(self):
+ # Test branch variants of jobs with inheritance
+ self.executor_server.hold_jobs_in_build = True
+ # This creates a new branch with a copy of the config in master
+ self.create_branch('puppet-integration', 'stable')
+ self.fake_gerrit.addEvent(
+ self.fake_gerrit.getFakeBranchCreatedEvent(
+ 'puppet-integration', 'stable'))
+ self.waitUntilSettled()
+
+ A = self.fake_gerrit.addFakeChange('puppet-integration', 'stable', 'A')
+ self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+ self.waitUntilSettled()
+
+ self.assertEqual(len(self.builds[0].parameters['pre_playbooks']), 3)
+ self.executor_server.hold_jobs_in_build = False
+ self.executor_server.release()
+ self.waitUntilSettled()
+
+
class TestInRepoConfig(ZuulTestCase):
# A temporary class to hold new tests while others are disabled
diff --git a/zuul/configloader.py b/zuul/configloader.py
index 79c8b39..cee744d 100644
--- a/zuul/configloader.py
+++ b/zuul/configloader.py
@@ -499,26 +499,28 @@
job.source_line = conf.get('_start_mark').line + 1
is_variant = layout.hasJob(name)
- if 'parent' in conf:
- if conf['parent'] is not None:
- # Parent job is explicitly specified, so inherit from it.
- parent = layout.getJob(conf['parent'])
- job.inheritFrom(parent)
+ if not is_variant:
+ if 'parent' in conf:
+ if conf['parent'] is not None:
+ # Parent job is explicitly specified, so inherit from it.
+ parent = layout.getJob(conf['parent'])
+ job.inheritFrom(parent)
+ else:
+ # Parent is explicitly set as None, so user intends
+ # this to be a base job. That's only okay if we're in
+ # a config project.
+ if not conf['_source_context'].trusted:
+ raise Exception(
+ "Base jobs must be defined in config projects")
else:
- # Parent is explicitly set as None, so user intends
- # this to be a base job. That's only okay if we're in
- # a config project.
- if not conf['_source_context'].trusted:
- raise Exception(
- "Base jobs must be defined in config projects")
- else:
- # Parent is not explicitly set, so inherit from the
- # default -- but only if this is the primary definition
- # for the job (ie, not a variant -- variants don't need to
- # have a parent as long as the main job does).
- if not is_variant:
parent = layout.getJob(tenant.default_base_job)
job.inheritFrom(parent)
+ else:
+ if 'parent' in conf:
+ # TODO(jeblair): warn the user that we're ignoring the
+ # parent setting on this variant job definition.
+ pass
+
# Secrets are part of the playbook context so we must establish
# them earlier than playbooks.
secrets = []