Merge "Run playbooks with only those roles defined thus far" into feature/zuulv3
diff --git a/doc/source/user/config.rst b/doc/source/user/config.rst
index c75085d..744414a 100644
--- a/doc/source/user/config.rst
+++ b/doc/source/user/config.rst
@@ -709,8 +709,19 @@
roles: a Galaxy role, which is simply a role that is installed from
Ansible Galaxy, or a Zuul role, which is a role provided by a
project managed by Zuul. Zuul roles are able to benefit from
- speculative merging and cross-project dependencies when used by jobs
- in untrusted projects.
+ speculative merging and cross-project dependencies when used by
+ playbooks in untrusted projects.
+
+ In the case of job inheritance or variance, the roles used for each
+ of the playbooks run by the job will be only those which were
+ defined along with that playbook. If a child job inherits from a
+ parent which defines a pre and post playbook, then the pre and post
+ playbooks it inherits from the parent job will run only with the
+ roles that were defined on the parent. If the child adds its own
+ pre and post playbooks, then any roles added by the child will be
+ available to the child's playbooks. This is so that a job which
+ inherits from a parent does not inadvertantly alter the behavior of
+ the parent's playbooks by the addition of conflicting roles.
A project which supplies a role may be structured in one of two
configurations: a bare role (in which the role exists at the root of
diff --git a/tests/base.py b/tests/base.py
index 9709bf7..1cc9999 100755
--- a/tests/base.py
+++ b/tests/base.py
@@ -1359,12 +1359,12 @@
self.recordResult(result)
return result
- def runAnsible(self, cmd, timeout, trusted=False):
+ def runAnsible(self, cmd, timeout, config_file, trusted):
build = self.executor_server.job_builds[self.job.unique]
if self.executor_server._run_ansible:
result = super(RecordingAnsibleJob, self).runAnsible(
- cmd, timeout, trusted=trusted)
+ cmd, timeout, config_file, trusted)
else:
result = build.run()
return result
diff --git a/tests/unit/test_model.py b/tests/unit/test_model.py
index 7fe101e..23a6f26 100644
--- a/tests/unit/test_model.py
+++ b/tests/unit/test_model.py
@@ -95,9 +95,9 @@
def test_job_inheritance(self):
# This is standard job inheritance.
- base_pre = model.PlaybookContext(self.context, 'base-pre')
- base_run = model.PlaybookContext(self.context, 'base-run')
- base_post = model.PlaybookContext(self.context, 'base-post')
+ base_pre = model.PlaybookContext(self.context, 'base-pre', [])
+ base_run = model.PlaybookContext(self.context, 'base-run', [])
+ base_post = model.PlaybookContext(self.context, 'base-post', [])
base = model.Job('base')
base.timeout = 30
@@ -121,9 +121,9 @@
def test_job_variants(self):
# This simulates freezing a job.
- py27_pre = model.PlaybookContext(self.context, 'py27-pre')
- py27_run = model.PlaybookContext(self.context, 'py27-run')
- py27_post = model.PlaybookContext(self.context, 'py27-post')
+ py27_pre = model.PlaybookContext(self.context, 'py27-pre', [])
+ py27_run = model.PlaybookContext(self.context, 'py27-run', [])
+ py27_post = model.PlaybookContext(self.context, 'py27-post', [])
py27 = model.Job('py27')
py27.timeout = 30
diff --git a/zuul/configloader.py b/zuul/configloader.py
index 254527a..023d01e 100644
--- a/zuul/configloader.py
+++ b/zuul/configloader.py
@@ -400,21 +400,34 @@
parent = layout.getJob(conf['parent'])
job.inheritFrom(parent)
+ # Roles are part of the playbook context so we must establish
+ # them earlier than playbooks.
+ if 'roles' in conf:
+ roles = []
+ for role in conf.get('roles', []):
+ if 'zuul' in role:
+ r = JobParser._makeZuulRole(tenant, job, role)
+ if r:
+ roles.append(r)
+ job.addRoles(roles)
+
for pre_run_name in as_list(conf.get('pre-run')):
pre_run = model.PlaybookContext(job.source_context,
- pre_run_name)
+ pre_run_name, job.roles)
job.pre_run = job.pre_run + (pre_run,)
for post_run_name in as_list(conf.get('post-run')):
post_run = model.PlaybookContext(job.source_context,
- post_run_name)
+ post_run_name, job.roles)
job.post_run = (post_run,) + job.post_run
if 'run' in conf:
- run = model.PlaybookContext(job.source_context, conf['run'])
+ run = model.PlaybookContext(job.source_context, conf['run'],
+ job.roles)
job.run = (run,)
else:
if not project_pipeline:
run_name = os.path.join('playbooks', job.name)
- run = model.PlaybookContext(job.source_context, run_name)
+ run = model.PlaybookContext(job.source_context, run_name,
+ job.roles)
job.implied_run = (run,) + job.implied_run
for k in JobParser.simple_attributes:
@@ -460,15 +473,6 @@
job.dependencies = frozenset(as_list(conf.get('dependencies')))
- if 'roles' in conf:
- roles = []
- for role in conf.get('roles', []):
- if 'zuul' in role:
- r = JobParser._makeZuulRole(tenant, job, role)
- if r:
- roles.append(r)
- job.addRoles(roles)
-
variables = conf.get('vars', None)
if variables:
job.updateVariables(variables)
diff --git a/zuul/executor/client.py b/zuul/executor/client.py
index c36d569..2a205bf 100644
--- a/zuul/executor/client.py
+++ b/zuul/executor/client.py
@@ -237,7 +237,6 @@
params['playbooks'] = [x.toDict() for x in job.run]
params['pre_playbooks'] = [x.toDict() for x in job.pre_run]
params['post_playbooks'] = [x.toDict() for x in job.post_run]
- params['roles'] = [x.toDict() for x in job.roles]
nodeset = item.current_build_set.getJobNodeSet(job.name)
nodes = []
diff --git a/zuul/executor/server.py b/zuul/executor/server.py
index cdd082e..25f2f86 100644
--- a/zuul/executor/server.py
+++ b/zuul/executor/server.py
@@ -72,13 +72,6 @@
self._running = False
-class JobDirPlaybook(object):
- def __init__(self, root):
- self.root = root
- self.trusted = None
- self.path = None
-
-
class SshAgent(object):
log = logging.getLogger("zuul.ExecutorServer")
@@ -151,6 +144,24 @@
return result
+class JobDirPlaybook(object):
+ def __init__(self, root):
+ self.root = root
+ self.trusted = None
+ self.path = None
+ self.roles = []
+ self.roles_path = []
+ self.ansible_config = os.path.join(self.root, 'ansible.cfg')
+ self.project_link = os.path.join(self.root, 'project')
+
+ def addRole(self):
+ count = len(self.roles)
+ root = os.path.join(self.root, 'role_%i' % (count,))
+ os.makedirs(root)
+ self.roles.append(root)
+ return root
+
+
class JobDir(object):
def __init__(self, root, keep, build_uuid):
'''
@@ -163,11 +174,23 @@
'''
# root
# ansible
- # trusted.cfg
- # untrusted.cfg
+ # inventory.yaml
+ # playbook_0
+ # project -> ../trusted/project_0/...
+ # role_0 -> ../trusted/project_0/...
+ # trusted
+ # project_0
+ # <git.example.com>
+ # <project>
# work
+ # .ssh
+ # known_hosts
# src
+ # <git.example.com>
+ # <project>
# logs
+ # job-output.txt
+ # results.json
self.keep = keep
if root:
tmpdir = root
@@ -175,16 +198,16 @@
tmpdir = tempfile.gettempdir()
self.root = os.path.join(tmpdir, build_uuid)
os.mkdir(self.root, 0o700)
- # Work
self.work_root = os.path.join(self.root, 'work')
os.makedirs(self.work_root)
self.src_root = os.path.join(self.work_root, 'src')
os.makedirs(self.src_root)
self.log_root = os.path.join(self.work_root, 'logs')
os.makedirs(self.log_root)
- # Ansible
self.ansible_root = os.path.join(self.root, 'ansible')
os.makedirs(self.ansible_root)
+ self.trusted_root = os.path.join(self.root, 'trusted')
+ os.makedirs(self.trusted_root)
ssh_dir = os.path.join(self.work_root, '.ssh')
os.mkdir(ssh_dir, 0o700)
self.result_data_file = os.path.join(self.work_root, 'results.json')
@@ -196,13 +219,23 @@
self.playbook = None # A pointer to the candidate we have chosen
self.pre_playbooks = []
self.post_playbooks = []
- self.roles = []
- self.trusted_roles_path = []
- self.untrusted_roles_path = []
- self.untrusted_config = os.path.join(
- self.ansible_root, 'untrusted.cfg')
- self.trusted_config = os.path.join(self.ansible_root, 'trusted.cfg')
self.job_output_file = os.path.join(self.log_root, 'job-output.txt')
+ self.trusted_projects = []
+ self.trusted_project_index = {}
+
+ def addTrustedProject(self, canonical_name, branch):
+ # Trusted projects are placed in their own directories so that
+ # we can support using different branches of the same project
+ # in different playbooks.
+ count = len(self.trusted_projects)
+ root = os.path.join(self.trusted_root, 'project_%i' % (count,))
+ os.makedirs(root)
+ self.trusted_projects.append(root)
+ self.trusted_project_index[(canonical_name, branch)] = root
+ return root
+
+ def getTrustedProject(self, canonical_name, branch):
+ return self.trusted_project_index.get((canonical_name, branch))
def addPrePlaybook(self):
count = len(self.pre_playbooks)
@@ -228,17 +261,6 @@
self.playbooks.append(playbook)
return playbook
- def addRole(self):
- count = len(self.roles)
- root = os.path.join(self.ansible_root, 'role_%i' % (count,))
- os.makedirs(root)
- trusted = os.path.join(root, 'trusted')
- os.makedirs(trusted)
- untrusted = os.path.join(root, 'untrusted')
- os.makedirs(untrusted)
- self.roles.append(root)
- return root
-
def cleanup(self):
if not self.keep:
shutil.rmtree(self.root)
@@ -760,8 +782,13 @@
projects.add((project['connection'], project['name']))
# ...as well as all playbook and role projects.
- repos = (args['pre_playbooks'] + args['playbooks'] +
- args['post_playbooks'] + args['roles'])
+ repos = []
+ playbooks = (args['pre_playbooks'] + args['playbooks'] +
+ args['post_playbooks'])
+ for playbook in playbooks:
+ repos.append(playbook)
+ repos += playbook['roles']
+
for repo in repos:
self.log.debug("Job %s: updating playbook or role %s" %
(self.job.unique, repo))
@@ -806,12 +833,9 @@
for repo in repos.values():
repo.deleteRemote('origin')
- # is the playbook in a repo that we have already prepared?
- trusted, untrusted = self.preparePlaybookRepos(args)
+ # This prepares each playbook and the roles needed for each.
+ self.preparePlaybooks(args)
- self.prepareRoles(args, trusted, untrusted)
-
- # TODOv3: Ansible the ansible thing here.
self.prepareAnsibleFiles(args)
data = {
@@ -997,42 +1021,29 @@
raise Exception("Unable to find playbook %s" % path)
return None
- def preparePlaybookRepos(self, args):
- trusted = untrusted = False
+ def preparePlaybooks(self, args):
for playbook in args['pre_playbooks']:
jobdir_playbook = self.jobdir.addPrePlaybook()
- self.preparePlaybookRepo(jobdir_playbook, playbook,
- args, required=True)
- if playbook['trusted']:
- trusted = True
- else:
- untrusted = True
+ self.preparePlaybook(jobdir_playbook, playbook,
+ args, required=True)
for playbook in args['playbooks']:
jobdir_playbook = self.jobdir.addPlaybook()
- self.preparePlaybookRepo(jobdir_playbook, playbook,
- args, required=False)
- if playbook['trusted']:
- trusted = True
- else:
- untrusted = True
+ self.preparePlaybook(jobdir_playbook, playbook,
+ args, required=False)
if jobdir_playbook.path is not None:
self.jobdir.playbook = jobdir_playbook
break
+
if self.jobdir.playbook is None:
raise Exception("No valid playbook found")
for playbook in args['post_playbooks']:
jobdir_playbook = self.jobdir.addPostPlaybook()
- self.preparePlaybookRepo(jobdir_playbook, playbook,
- args, required=True)
- if playbook['trusted']:
- trusted = True
- else:
- untrusted = True
- return (trusted, untrusted)
+ self.preparePlaybook(jobdir_playbook, playbook,
+ args, required=True)
- def preparePlaybookRepo(self, jobdir_playbook, playbook, args, required):
+ def preparePlaybook(self, jobdir_playbook, playbook, args, required):
self.log.debug("Prepare playbook repo for %s" % (playbook,))
# Check out the playbook repo if needed and set the path to
# the playbook that should be run.
@@ -1040,6 +1051,7 @@
source = self.executor_server.connections.getSource(
playbook['connection'])
project = source.getProject(playbook['project'])
+ path = None
if not playbook['trusted']:
# This is a project repo, so it is safe to use the already
# checked out version (from speculative merging) of the
@@ -1052,34 +1064,48 @@
project.canonical_hostname,
project.name,
playbook['path'])
- jobdir_playbook.path = self.findPlaybook(
- path,
- required=required,
- trusted=playbook['trusted'])
- return
- # The playbook repo is either a config repo, or it isn't in
- # the stack of changes we are testing, so check out the branch
- # tip into a dedicated space.
+ break
+ if not path:
+ # The playbook repo is either a config repo, or it isn't in
+ # the stack of changes we are testing, so check out the branch
+ # tip into a dedicated space.
+ path = self.checkoutTrustedProject(project, playbook['branch'])
+ path = os.path.join(path, playbook['path'])
- merger = self.executor_server._getMerger(jobdir_playbook.root,
- self.log)
- merger.checkoutBranch(playbook['connection'], project.name,
- playbook['branch'])
-
- path = os.path.join(jobdir_playbook.root,
- project.canonical_hostname,
- project.name,
- playbook['path'])
jobdir_playbook.path = self.findPlaybook(
path,
required=required,
trusted=playbook['trusted'])
- def prepareRoles(self, args, trusted, untrusted):
- for role in args['roles']:
- if role['type'] == 'zuul':
- root = self.jobdir.addRole()
- self.prepareZuulRole(args, role, root, trusted, untrusted)
+ # If this playbook doesn't exist, don't bother preparing
+ # roles.
+ if not jobdir_playbook.path:
+ return
+
+ for role in playbook['roles']:
+ self.prepareRole(jobdir_playbook, role, args)
+
+ self.writeAnsibleConfig(jobdir_playbook)
+
+ def checkoutTrustedProject(self, project, branch):
+ root = self.jobdir.getTrustedProject(project.canonical_name,
+ branch)
+ if not root:
+ root = self.jobdir.addTrustedProject(project.canonical_name,
+ branch)
+ merger = self.executor_server._getMerger(root, self.log)
+ merger.checkoutBranch(project.connection_name, project.name,
+ branch)
+
+ path = os.path.join(root,
+ project.canonical_hostname,
+ project.name)
+ return path
+
+ def prepareRole(self, jobdir_playbook, role, args):
+ if role['type'] == 'zuul':
+ root = jobdir_playbook.addRole()
+ self.prepareZuulRole(jobdir_playbook, role, args, root)
def findRole(self, path, trusted=False):
d = os.path.join(path, 'tasks')
@@ -1099,97 +1125,50 @@
# It is neither a bare role, nor a collection of roles
raise Exception("Unable to find role in %s" % (path,))
- def prepareZuulRole(self, args, role, root, trusted, untrusted):
+ def prepareZuulRole(self, jobdir_playbook, role, args, root):
self.log.debug("Prepare zuul role for %s" % (role,))
# Check out the role repo if needed
source = self.executor_server.connections.getSource(
role['connection'])
project = source.getProject(role['project'])
- untrusted_role_repo = None
- trusted_role_repo = None
- trusted_root = os.path.join(root, 'trusted')
- untrusted_root = os.path.join(root, 'untrusted')
name = role['target_name']
+ path = None
- if untrusted:
- # There is at least one untrusted playbook. For that
- # case, use the already checked out version (from
- # speculative merging) of the role.
+ if not jobdir_playbook.trusted:
+ # This playbook is untrested. Use the already checked out
+ # version (from speculative merging) of the role if it
+ # exists.
for i in args['items']:
if (i['connection'] == role['connection'] and
i['project'] == role['project']):
- # We already have this repo prepared;
- # copy it into location.
-
+ # We already have this repo prepared; use it.
path = os.path.join(self.jobdir.src_root,
project.canonical_hostname,
project.name)
- # The name of the symlink is the requested name of
- # the role (which may be the repo name or may be
- # something else; this can come into play if this
- # is a bare role).
- link = os.path.join(untrusted_root, name)
- link = os.path.realpath(link)
- if not link.startswith(os.path.realpath(untrusted_root)):
- raise Exception("Invalid role name %s", name)
- os.symlink(path, link)
- untrusted_role_repo = link
break
- if trusted or not untrusted_role_repo:
- # There is at least one trusted playbook which will need a
- # trusted checkout of the role, or the role did not appear
+ if not path:
+ # This is a trusted playbook or the role did not appear
# in the dependency chain for the change (in which case,
# there is no existing untrusted checkout of it). Check
# out the branch tip into a dedicated space.
- merger = self.executor_server._getMerger(trusted_root,
- self.log)
- merger.checkoutBranch(role['connection'], project.name,
- 'master')
- orig_repo_path = os.path.join(trusted_root,
- project.canonical_hostname,
- project.name)
- if name != project.name:
- # The requested name of the role is not the same as
- # the project name, so rename the git repo as the
- # requested name. It is the only item in this
- # directory, so we don't need to worry about
- # collisions.
- target = os.path.join(trusted_root,
- project.canonical_hostname,
- name)
- target = os.path.realpath(target)
- if not target.startswith(os.path.realpath(trusted_root)):
- raise Exception("Invalid role name %s", name)
- os.rename(orig_repo_path, target)
- trusted_role_repo = target
- else:
- trusted_role_repo = orig_repo_path
+ path = self.checkoutTrustedProject(project, 'master')
- if not untrusted_role_repo:
- # In the case that there was no untrusted checkout,
- # use the trusted checkout.
- untrusted_role_repo = trusted_role_repo
- untrusted_root = trusted_root
+ # The name of the symlink is the requested name of the role
+ # (which may be the repo name or may be something else; this
+ # can come into play if this is a bare role).
+ link = os.path.join(root, name)
+ link = os.path.realpath(link)
+ if not link.startswith(os.path.realpath(root)):
+ raise Exception("Invalid role name %s", name)
+ os.symlink(path, link)
- if untrusted:
- untrusted_role_path = self.findRole(untrusted_role_repo,
- trusted=False)
- if untrusted_role_path is None:
- # In the case of a bare role, add the containing directory
- untrusted_role_path = os.path.join(untrusted_root,
- project.canonical_hostname)
- self.jobdir.untrusted_roles_path.append(untrusted_role_path)
-
- if trusted:
- trusted_role_path = self.findRole(trusted_role_repo,
- trusted=True)
- if trusted_role_path is None:
- # In the case of a bare role, add the containing directory
- trusted_role_path = os.path.join(trusted_root,
- project.canonical_hostname)
- self.jobdir.trusted_roles_path.append(trusted_role_path)
+ role_path = self.findRole(link, trusted=jobdir_playbook.trusted)
+ if role_path is None:
+ # In the case of a bare role, add the containing directory
+ role_path = root
+ jobdir_playbook.roles_path.append(role_path)
def prepareAnsibleFiles(self, args):
all_vars = dict(args['vars'])
@@ -1213,11 +1192,10 @@
for key in node['host_keys']:
known_hosts.write('%s\n' % key)
- self.writeAnsibleConfig(self.jobdir.untrusted_config)
- self.writeAnsibleConfig(self.jobdir.trusted_config, trusted=True)
+ def writeAnsibleConfig(self, jobdir_playbook):
+ trusted = jobdir_playbook.trusted
- def writeAnsibleConfig(self, config_path, trusted=False):
- with open(config_path, 'w') as config:
+ with open(jobdir_playbook.ansible_config, 'w') as config:
config.write('[defaults]\n')
config.write('hostfile = %s\n' % self.jobdir.inventory)
config.write('local_tmp = %s/.ansible/local_tmp\n' %
@@ -1240,12 +1218,10 @@
% self.executor_server.action_dir)
config.write('lookup_plugins = %s\n'
% self.executor_server.lookup_dir)
- roles_path = self.jobdir.untrusted_roles_path
- else:
- roles_path = self.jobdir.trusted_roles_path
- if roles_path:
- config.write('roles_path = %s\n' % ':'.join(roles_path))
+ if jobdir_playbook.roles_path:
+ config.write('roles_path = %s\n' % ':'.join(
+ jobdir_playbook.roles_path))
# On trusted jobs, we want to prevent the printing of args,
# since trusted jobs might have access to secrets that they may
@@ -1286,7 +1262,7 @@
except Exception:
self.log.exception("Exception while killing ansible process:")
- def runAnsible(self, cmd, timeout, trusted=False):
+ def runAnsible(self, cmd, timeout, config_file, trusted):
env_copy = os.environ.copy()
env_copy.update(self.ssh_agent.env)
env_copy['LOGNAME'] = 'zuul'
@@ -1301,10 +1277,8 @@
env_copy['PYTHONPATH'] = os.path.pathsep.join(pythonpath)
if trusted:
- config_file = self.jobdir.trusted_config
opt_prefix = 'trusted'
else:
- config_file = self.jobdir.untrusted_config
opt_prefix = 'untrusted'
ro_dirs = get_default(self.executor_server.config, 'executor',
'%s_ro_dirs' % opt_prefix)
@@ -1409,7 +1383,9 @@
cmd.extend(['-e', 'zuul_execution_phase_index=%s' % index])
result, code = self.runAnsible(
- cmd=cmd, timeout=timeout, trusted=playbook.trusted)
+ cmd=cmd, timeout=timeout,
+ config_file=playbook.ansible_config,
+ trusted=playbook.trusted)
self.log.debug("Ansible complete, result %s code %s" % (
self.RESULT_MAP[result], code))
return result, code
diff --git a/zuul/model.py b/zuul/model.py
index 80ef285..a17a9cb 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -633,11 +633,17 @@
Jobs refer to objects of this class for their main, pre, and post
playbooks so that we can keep track of which repos and security
- contexts are needed in order to run them."""
+ contexts are needed in order to run them.
- def __init__(self, source_context, path):
+ We also keep a list of roles so that playbooks only run with the
+ roles which were defined at the point the playbook was defined.
+
+ """
+
+ def __init__(self, source_context, path, roles):
self.source_context = source_context
self.path = path
+ self.roles = roles
def __repr__(self):
return '<PlaybookContext %s %s>' % (self.source_context,
@@ -650,7 +656,8 @@
if not isinstance(other, PlaybookContext):
return False
return (self.source_context == other.source_context and
- self.path == other.path)
+ self.path == other.path and
+ self.roles == other.roles)
def toDict(self):
# Render to a dict to use in passing json to the executor
@@ -659,6 +666,7 @@
project=self.source_context.project.name,
branch=self.source_context.branch,
trusted=self.source_context.trusted,
+ roles=[r.toDict() for r in self.roles],
path=self.path)