diff --git a/doc/source/user/config.rst b/doc/source/user/config.rst index 5df5c7b430..a663303e1c 100644 --- a/doc/source/user/config.rst +++ b/doc/source/user/config.rst @@ -794,6 +794,15 @@ Here is an example of two job definitions: the addition of conflicting roles. Roles added by a child will appear before those it inherits from its parent. + If a project used for a Zuul role has branches, the usual + process of selecting which branch should be checked out applies. + See :attr:`job.override-checkout` for a description of that + process and how to override it. As a special case, if the role + project is the project in which this job definition appears, + then the branch in which this definition appears will be used. + In other words, a playbook may not use a role from a different + branch of the same project. + 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 the project), or a contained role (in which the role diff --git a/releasenotes/notes/role-checkouts-89632d2ff5eb8b78.yaml b/releasenotes/notes/role-checkouts-89632d2ff5eb8b78.yaml new file mode 100644 index 0000000000..86203c5b0b --- /dev/null +++ b/releasenotes/notes/role-checkouts-89632d2ff5eb8b78.yaml @@ -0,0 +1,15 @@ +--- +fixes: + - | + Zuul role repository checkouts now honor :attr:`job.override-checkout`. + + Previously, when a Zuul role was specified for a job, Zuul would + usually checkout the master branch, unless that repository + appeared in the dependency chain for a patch. It will now follow + the usual procedure for determining the branch to check out, + including honoring :attr:`job.override-checkout` options. + + This may alter the behavior of currently existing jobs. Depending + on circumstances, you may need to set + :attr:`job.override-checkout` or copy roles to other branches of + projects. diff --git a/tests/fixtures/config/role-branches/git/common-config/playbooks/pre-base.yaml b/tests/fixtures/config/role-branches/git/common-config/playbooks/pre-base.yaml new file mode 100644 index 0000000000..a70d0b00bc --- /dev/null +++ b/tests/fixtures/config/role-branches/git/common-config/playbooks/pre-base.yaml @@ -0,0 +1,3 @@ +- hosts: all + roles: + - pre-base diff --git a/tests/fixtures/config/role-branches/git/common-config/roles/pre-base/tasks/main.yaml b/tests/fixtures/config/role-branches/git/common-config/roles/pre-base/tasks/main.yaml new file mode 100644 index 0000000000..551ae4e6d6 --- /dev/null +++ b/tests/fixtures/config/role-branches/git/common-config/roles/pre-base/tasks/main.yaml @@ -0,0 +1,3 @@ +- name: base pre + debug: + msg: base pre diff --git a/tests/fixtures/config/role-branches/git/common-config/zuul.yaml b/tests/fixtures/config/role-branches/git/common-config/zuul.yaml new file mode 100644 index 0000000000..dc75fa844e --- /dev/null +++ b/tests/fixtures/config/role-branches/git/common-config/zuul.yaml @@ -0,0 +1,62 @@ +- pipeline: + name: check + manager: independent + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + Verified: 1 + failure: + gerrit: + Verified: -1 + +- pipeline: + name: gate + manager: dependent + post-review: True + trigger: + gerrit: + - event: comment-added + approval: + - Approved: 1 + success: + gerrit: + Verified: 2 + submit: true + failure: + gerrit: + Verified: -2 + start: + gerrit: + Verified: 0 + precedence: high + +- job: + name: base + parent: null + pre-run: playbooks/pre-base.yaml + +- project: + name: common-config + check: + jobs: [] + gate: + jobs: + - noop + +- project: + name: project1 + check: + jobs: [] + gate: + jobs: + - noop + +- project: + name: project2 + check: + jobs: [] + gate: + jobs: + - noop diff --git a/tests/fixtures/config/role-branches/git/project1/README b/tests/fixtures/config/role-branches/git/project1/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/role-branches/git/project1/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/role-branches/git/project1/playbooks/parent-job.yaml b/tests/fixtures/config/role-branches/git/project1/playbooks/parent-job.yaml new file mode 100644 index 0000000000..e4b64c62f1 --- /dev/null +++ b/tests/fixtures/config/role-branches/git/project1/playbooks/parent-job.yaml @@ -0,0 +1,3 @@ +- hosts: all + roles: + - run-parent-job diff --git a/tests/fixtures/config/role-branches/git/project1/roles/run-parent-job/tasks/main.yaml b/tests/fixtures/config/role-branches/git/project1/roles/run-parent-job/tasks/main.yaml new file mode 100644 index 0000000000..cdd2593940 --- /dev/null +++ b/tests/fixtures/config/role-branches/git/project1/roles/run-parent-job/tasks/main.yaml @@ -0,0 +1,3 @@ +- name: run parent job + debug: + msg: run parent job diff --git a/tests/fixtures/config/role-branches/git/project1/zuul.yaml b/tests/fixtures/config/role-branches/git/project1/zuul.yaml new file mode 100644 index 0000000000..8a40a7c2e9 --- /dev/null +++ b/tests/fixtures/config/role-branches/git/project1/zuul.yaml @@ -0,0 +1,6 @@ +- job: + name: parent-job + required-projects: + - project1 + pre-run: playbooks/parent-job-pre.yaml + run: playbooks/parent-job.yaml diff --git a/tests/fixtures/config/role-branches/git/project2/README b/tests/fixtures/config/role-branches/git/project2/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/role-branches/git/project2/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/role-branches/git/project2/playbooks/child-job.yaml b/tests/fixtures/config/role-branches/git/project2/playbooks/child-job.yaml new file mode 100644 index 0000000000..e4b64c62f1 --- /dev/null +++ b/tests/fixtures/config/role-branches/git/project2/playbooks/child-job.yaml @@ -0,0 +1,3 @@ +- hosts: all + roles: + - run-parent-job diff --git a/tests/fixtures/config/role-branches/git/project2/zuul.yaml b/tests/fixtures/config/role-branches/git/project2/zuul.yaml new file mode 100644 index 0000000000..e1b04f53c6 --- /dev/null +++ b/tests/fixtures/config/role-branches/git/project2/zuul.yaml @@ -0,0 +1,23 @@ +- job: + name: child-job + parent: parent-job + run: playbooks/child-job.yaml + +- job: + name: child-job-override + parent: child-job + override-checkout: stable + +- job: + name: child-job-project-override + parent: child-job + required-projects: + - name: project1 + override-checkout: stable + +- project: + check: + jobs: + - child-job + - child-job-override + - child-job-project-override diff --git a/tests/fixtures/config/role-branches/main.yaml b/tests/fixtures/config/role-branches/main.yaml new file mode 100644 index 0000000000..72609a6c5d --- /dev/null +++ b/tests/fixtures/config/role-branches/main.yaml @@ -0,0 +1,9 @@ +- tenant: + name: tenant-one + source: + gerrit: + config-projects: + - common-config + untrusted-projects: + - project1 + - project2 diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index e72eefd380..978dae130b 100755 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -2333,7 +2333,7 @@ class TestProjectKeys(ZuulTestCase): class RoleTestCase(ZuulTestCase): - def _assertRolePath(self, build, playbook, content): + def _getRolesPaths(self, build, playbook): path = os.path.join(self.test_root, build.uuid, 'ansible', playbook, 'ansible.cfg') roles_paths = [] @@ -2341,7 +2341,10 @@ class RoleTestCase(ZuulTestCase): for line in f: if line.startswith('roles_path'): roles_paths.append(line) - print(roles_paths) + return roles_paths + + def _assertRolePath(self, build, playbook, content): + roles_paths = self._getRolesPaths(build, playbook) if content: self.assertEqual(len(roles_paths), 1, "Should have one roles_path line in %s" % @@ -2352,6 +2355,121 @@ class RoleTestCase(ZuulTestCase): "Should have no roles_path line in %s" % (playbook,)) + def _assertInRolePath(self, build, playbook, files): + roles_paths = self._getRolesPaths(build, playbook)[0] + roles_paths = roles_paths.split('=')[-1].strip() + roles_paths = roles_paths.split(':') + + files = set(files) + matches = set() + for rpath in roles_paths: + for rolename in os.listdir(rpath): + if rolename in files: + matches.add(rolename) + self.assertEqual(files, matches) + + +class TestRoleBranches(RoleTestCase): + tenant_config_file = 'config/role-branches/main.yaml' + + def _addRole(self, project, branch, role, parent=None): + data = textwrap.dedent(""" + - name: %s + debug: + msg: %s + """ % (role, role)) + file_dict = {'roles/%s/tasks/main.yaml' % role: data} + A = self.fake_gerrit.addFakeChange(project, branch, + 'add %s' % role, + files=file_dict, + parent=parent) + A.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + self.fake_gerrit.addEvent(A.getChangeMergedEvent()) + self.waitUntilSettled() + return A.patchsets[-1]['ref'] + + def _addPlaybook(self, project, branch, playbook, role, parent=None): + data = textwrap.dedent(""" + - hosts: all + roles: + - %s + """ % role) + file_dict = {'playbooks/%s.yaml' % playbook: data} + A = self.fake_gerrit.addFakeChange(project, branch, + 'add %s' % playbook, + files=file_dict, + parent=parent) + A.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + self.fake_gerrit.addEvent(A.getChangeMergedEvent()) + self.waitUntilSettled() + return A.patchsets[-1]['ref'] + + def _assertInFile(self, path, content): + with open(path) as f: + self.assertIn(content, f.read()) + + def test_playbook_role_branches(self): + # This tests that the correct branch of a repo which contains + # a playbook or a role is checked out. Most of the action + # happens on project1, which holds a parent job, so that we + # can test the behavior of a project which is not in the + # dependency chain. + # First we create some branch-specific content in project1: + self.create_branch('project1', 'stable') + + # A pre-playbook with unique stable branch content. + p = self._addPlaybook('project1', 'stable', + 'parent-job-pre', 'parent-stable-role') + # A role that only exists on the stable branch. + self._addRole('project1', 'stable', 'stable-role', parent=p) + + # The same for the master branch. + p = self._addPlaybook('project1', 'master', + 'parent-job-pre', 'parent-master-role') + self._addRole('project1', 'master', 'master-role', parent=p) + + self.sched.reconfigure(self.config) + # Push a change to project2 which will run 3 jobs which + # inherit from project1. + self.executor_server.hold_jobs_in_build = True + A = self.fake_gerrit.addFakeChange('project2', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + self.assertEqual(len(self.builds), 3) + + # This job should use the master branch since that's the + # zuul.branch for this change. + build = self.getBuildByName('child-job') + self._assertInRolePath(build, 'playbook_0', ['master-role']) + self._assertInFile(build.jobdir.pre_playbooks[1].path, + 'parent-master-role') + + # The main playbook is on the master branch of project2, but + # there is a job-level branch override, so the project1 role + # should be from the stable branch. The job-level override + # will cause Zuul to select the project1 pre-playbook from the + # stable branch as well, so we should see it using the stable + # role. + build = self.getBuildByName('child-job-override') + self._assertInRolePath(build, 'playbook_0', ['stable-role']) + self._assertInFile(build.jobdir.pre_playbooks[1].path, + 'parent-stable-role') + + # The same, but using a required-projects override. + build = self.getBuildByName('child-job-project-override') + self._assertInRolePath(build, 'playbook_0', ['stable-role']) + self._assertInFile(build.jobdir.pre_playbooks[1].path, + 'parent-stable-role') + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + class TestRoles(RoleTestCase): tenant_config_file = 'config/roles/main.yaml' diff --git a/zuul/ansible/paths.py b/zuul/ansible/paths.py index 38761204aa..a53825530b 100644 --- a/zuul/ansible/paths.py +++ b/zuul/ansible/paths.py @@ -44,6 +44,8 @@ def _is_safe_path(path, allow_trusted=False): if allow_trusted: allowed_paths.append( os.path.abspath(os.path.join(home_path, '../trusted'))) + allowed_paths.append( + os.path.abspath(os.path.join(home_path, '../untrusted'))) def _is_safe(path_to_check): for allowed_path in allowed_paths: diff --git a/zuul/configloader.py b/zuul/configloader.py index 7a0b12e696..db1db3541a 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -801,16 +801,14 @@ class JobParser(object): return None return model.ZuulRole(role.get('name', name), - project.connection_name, - project.name) + project.canonical_name) def _makeImplicitRole(self, job): project = job.source_context.project name = project.name.split('/')[-1] name = JobParser.ANSIBLE_ROLE_RE.sub('', name) return model.ZuulRole(name, - project.connection_name, - project.name, + project.canonical_name, implicit=True) diff --git a/zuul/executor/client.py b/zuul/executor/client.py index 66860eef40..5b1e27f54a 100644 --- a/zuul/executor/client.py +++ b/zuul/executor/client.py @@ -196,10 +196,29 @@ class ExecutorClient(object): params['override_checkout'] = job.override_checkout params['repo_state'] = item.current_build_set.repo_state + def make_playbook(playbook): + d = playbook.toDict() + for role in d['roles']: + if role['type'] != 'zuul': + continue + project_config = item.layout.project_configs.get( + role['project_canonical_name'], None) + if project_config: + role['project_default_branch'] = \ + project_config.default_branch + else: + role['project_default_branch'] = 'master' + role_trusted, role_project = item.layout.tenant.getProject( + role['project_canonical_name']) + role_connection = role_project.source.connection + role['connection'] = role_connection.connection_name + role['project'] = role_project.name + return d + if job.name != 'noop': - 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['playbooks'] = [make_playbook(x) for x in job.run] + params['pre_playbooks'] = [make_playbook(x) for x in job.pre_run] + params['post_playbooks'] = [make_playbook(x) for x in job.post_run] nodes = [] for node in nodeset.getNodes(): diff --git a/zuul/executor/server.py b/zuul/executor/server.py index 4823acf11b..d292d76b0f 100644 --- a/zuul/executor/server.py +++ b/zuul/executor/server.py @@ -257,6 +257,7 @@ class JobDirPlaybook(object): def __init__(self, root): self.root = root self.trusted = None + self.project_canonical_name = None self.branch = None self.canonical_name_and_path = None self.path = None @@ -302,6 +303,10 @@ class JobDir(object): # project_0 # # + # untrusted (mounted in bwrap read-only) + # project_0 + # + # # work (mounted in bwrap read-write) # .ssh # known_hosts @@ -328,6 +333,8 @@ class JobDir(object): os.makedirs(self.ansible_root) self.trusted_root = os.path.join(self.root, 'trusted') os.makedirs(self.trusted_root) + self.untrusted_root = os.path.join(self.root, 'untrusted') + os.makedirs(self.untrusted_root) ssh_dir = os.path.join(self.work_root, '.ssh') os.mkdir(ssh_dir, 0o700) # Create ansible cache directory @@ -368,6 +375,8 @@ class JobDir(object): )) self.trusted_projects = [] self.trusted_project_index = {} + self.untrusted_projects = [] + self.untrusted_project_index = {} # Create a JobDirPlaybook for the Ansible setup run. This # doesn't use an actual playbook, but it lets us use the same @@ -392,6 +401,23 @@ class JobDir(object): def getTrustedProject(self, canonical_name, branch): return self.trusted_project_index.get((canonical_name, branch)) + def addUntrustedProject(self, canonical_name, branch): + # Similar to trusted projects, but these hold checkouts of + # projects which are allowed to have speculative changes + # applied. They might, however, be different branches than + # what is used in the working dir, so they need their own + # location. Moreover, we might avoid mischief if a job alters + # the contents of the working dir. + count = len(self.untrusted_projects) + root = os.path.join(self.untrusted_root, 'project_%i' % (count,)) + os.makedirs(root) + self.untrusted_projects.append(root) + self.untrusted_project_index[(canonical_name, branch)] = root + return root + + def getUntrustedProject(self, canonical_name, branch): + return self.untrusted_project_index.get((canonical_name, branch)) + def addPrePlaybook(self): count = len(self.pre_playbooks) root = os.path.join(self.ansible_root, 'pre_playbook_%i' % (count,)) @@ -431,6 +457,9 @@ class UpdateTask(object): def __init__(self, connection_name, project_name): self.connection_name = connection_name self.project_name = project_name + self.canonical_name = None + self.branches = None + self.refs = None self.event = threading.Event() def __eq__(self, other): @@ -584,6 +613,7 @@ class AnsibleJob(object): self.aborted = False self.aborted_reason = None self.thread = None + self.project_info = {} self.private_key_file = get_default(self.executor_server.config, 'executor', 'private_key_file', '~/.ssh/id_rsa') @@ -675,10 +705,16 @@ class AnsibleJob(object): for task in tasks: task.wait() + self.project_info[task.canonical_name] = { + 'refs': task.refs, + 'branches': task.branches, + } self.log.debug("Git updates complete") - merger = self.executor_server._getMerger(self.jobdir.src_root, - self.log) + merger = self.executor_server._getMerger( + self.jobdir.src_root, + self.executor_server.merge_root, + self.log) repos = {} for project in args['projects']: self.log.debug("Cloning %s/%s" % (project['connection'], @@ -710,19 +746,24 @@ class AnsibleJob(object): ref = args['zuul']['ref'] else: ref = None - selected = self.checkoutBranch(repo, - project['name'], - ref, - args['branch'], - args['override_branch'], - args['override_checkout'], - project['override_branch'], - project['override_checkout'], - project['default_branch']) + selected_ref, selected_desc = self.resolveBranch( + project['canonical_name'], + ref, + args['branch'], + args['override_branch'], + args['override_checkout'], + project['override_branch'], + project['override_checkout'], + project['default_branch']) + self.log.info("Checking out %s %s %s", + project['canonical_name'], selected_desc, + selected_ref) + repo.checkout(selected_ref) + # Update the inventory variables to indicate the ref we # checked out p = args['zuul']['projects'][project['canonical_name']] - p['checkout'] = selected + p['checkout'] = selected_ref # Delete the origin remote from each repo we set up since # it will not be valid within the jobs. for repo in repos.values(): @@ -811,51 +852,44 @@ class AnsibleJob(object): repo.setRef('refs/heads/' + branch, commit) return True - def checkoutBranch(self, repo, project_name, ref, zuul_branch, - job_override_branch, job_override_checkout, - project_override_branch, project_override_checkout, - project_default_branch): - branches = repo.getBranches() - refs = [r.name for r in repo.getRefs()] + def resolveBranch(self, project_canonical_name, ref, zuul_branch, + job_override_branch, job_override_checkout, + project_override_branch, project_override_checkout, + project_default_branch): + branches = self.project_info[project_canonical_name]['branches'] + refs = self.project_info[project_canonical_name]['refs'] selected_ref = None + selected_desc = None if project_override_checkout in refs: selected_ref = project_override_checkout - self.log.info("Checking out %s project override ref %s", - project_name, selected_ref) + selected_desc = 'project override ref' elif project_override_branch in branches: selected_ref = project_override_branch - self.log.info("Checking out %s project override branch %s", - project_name, selected_ref) + selected_desc = 'project override branch' elif job_override_checkout in refs: selected_ref = job_override_checkout - self.log.info("Checking out %s job override ref %s", - project_name, selected_ref) + selected_desc = 'job override ref' elif job_override_branch in branches: selected_ref = job_override_branch - self.log.info("Checking out %s job override branch %s", - project_name, selected_ref) + selected_desc = 'job override branch' elif ref and ref.startswith('refs/heads/'): selected_ref = ref[len('refs/heads/'):] - self.log.info("Checking out %s branch ref %s", - project_name, selected_ref) + selected_desc = 'branch ref' elif ref and ref.startswith('refs/tags/'): selected_ref = ref[len('refs/tags/'):] - self.log.info("Checking out %s tag ref %s", - project_name, selected_ref) + selected_desc = 'tag ref' elif zuul_branch and zuul_branch in branches: selected_ref = zuul_branch - self.log.info("Checking out %s zuul branch %s", - project_name, selected_ref) + selected_desc = 'zuul branch' elif project_default_branch in branches: selected_ref = project_default_branch - self.log.info("Checking out %s project default branch %s", - project_name, selected_ref) + selected_desc = 'project default branch' else: raise ExecutorError("Project %s does not have the " "default branch %s" % - (project_name, project_default_branch)) - repo.checkout(selected_ref) - return selected_ref + (project_canonical_name, + project_default_branch)) + return (selected_ref, selected_desc) def getAnsibleTimeout(self, start, timeout): if timeout is not None: @@ -1096,41 +1130,31 @@ class AnsibleJob(object): self.preparePlaybook(jobdir_playbook, playbook, args) def preparePlaybook(self, jobdir_playbook, playbook, args): - self.log.debug("Prepare playbook repo for %s" % - (playbook['project'],)) # Check out the playbook repo if needed and set the path to # the playbook that should be run. + self.log.debug("Prepare playbook repo for %s: %s@%s" % + (playbook['trusted'] and 'trusted' or 'untrusted', + playbook['project'], playbook['branch'])) source = self.executor_server.connections.getSource( playbook['connection']) project = source.getProject(playbook['project']) + branch = playbook['branch'] jobdir_playbook.trusted = playbook['trusted'] - jobdir_playbook.branch = playbook['branch'] + jobdir_playbook.branch = branch + jobdir_playbook.project_canonical_name = project.canonical_name jobdir_playbook.canonical_name_and_path = os.path.join( project.canonical_name, playbook['path']) 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 - # playbook - for i in args['items']: - if (i['connection'] == playbook['connection'] and - i['project'] == playbook['project']): - # We already have this repo prepared - path = os.path.join(self.jobdir.src_root, - project.canonical_hostname, - project.name, - playbook['path']) - 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']) + + if not jobdir_playbook.trusted: + path = self.checkoutUntrustedProject(project, branch, args) + else: + path = self.checkoutTrustedProject(project, branch) + path = os.path.join(path, playbook['path']) jobdir_playbook.path = self.findPlaybook( path, - trusted=playbook['trusted']) + trusted=jobdir_playbook.trusted) # If this playbook doesn't exist, don't bother preparing # roles. @@ -1154,9 +1178,57 @@ class AnsibleJob(object): if not root: root = self.jobdir.addTrustedProject(project.canonical_name, branch) - merger = self.executor_server._getMerger(root, self.log) + self.log.debug("Cloning %s@%s into new trusted space %s", + project, branch, root) + merger = self.executor_server._getMerger( + root, + self.executor_server.merge_root, + self.log) merger.checkoutBranch(project.connection_name, project.name, branch) + else: + self.log.debug("Using existing repo %s@%s in trusted space %s", + project, branch, root) + + path = os.path.join(root, + project.canonical_hostname, + project.name) + return path + + def checkoutUntrustedProject(self, project, branch, args): + root = self.jobdir.getUntrustedProject(project.canonical_name, + branch) + if not root: + root = self.jobdir.addUntrustedProject(project.canonical_name, + branch) + # If the project is in the dependency chain, clone from + # there so we pick up any speculative changes, otherwise, + # clone from the cache. + merger = None + for p in args['projects']: + if (p['connection'] == project.connection_name and + p['name'] == project.name): + # We already have this repo prepared + self.log.debug("Found workdir repo for untrusted project") + merger = self.executor_server._getMerger( + root, + self.jobdir.src_root, + self.log) + break + + if merger is None: + merger = self.executor_server._getMerger( + root, + self.executor_server.merge_root, + self.log) + + self.log.debug("Cloning %s@%s into new untrusted space %s", + project, branch, root) + merger.checkoutBranch(project.connection_name, project.name, + branch) + else: + self.log.debug("Using existing repo %s@%s in trusted space %s", + project, branch, root) path = os.path.join(root, project.canonical_hostname, @@ -1198,26 +1270,38 @@ class AnsibleJob(object): name = role['target_name'] path = None - 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; use it. - path = os.path.join(self.jobdir.src_root, - project.canonical_hostname, - project.name) + # Find the branch to use for this role. We should generally + # follow the normal fallback procedure, unless this role's + # project is the playbook's project, in which case we should + # use the playbook branch. + if jobdir_playbook.project_canonical_name == project.canonical_name: + branch = jobdir_playbook.branch + self.log.debug("Role project is playbook project, " + "using playbook branch %s", branch) + else: + # Find if the project is one of the job-specified projects. + # If it is, we can honor the project checkout-override options. + args_project = {} + for p in args['projects']: + if (p['canonical_name'] == project.canonical_name): + args_project = p break - 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. - path = self.checkoutTrustedProject(project, 'master') + branch, selected_desc = self.resolveBranch( + project.canonical_name, + None, + args['branch'], + args['override_branch'], + args['override_checkout'], + args_project.get('override_branch'), + args_project.get('override_checkout'), + role['project_default_branch']) + self.log.debug("Role using %s %s", selected_desc, branch) + + if not jobdir_playbook.trusted: + path = self.checkoutUntrustedProject(project, branch, args) + else: + path = self.checkoutTrustedProject(project, branch) # The name of the symlink is the requested name of the role # (which may be the repo name or may be something else; this @@ -1400,6 +1484,7 @@ class AnsibleJob(object): ro_paths.append(self.executor_server.ansible_dir) ro_paths.append(self.jobdir.ansible_root) ro_paths.append(self.jobdir.trusted_root) + ro_paths.append(self.jobdir.untrusted_root) ro_paths.append(playbook.root) rw_paths.append(self.jobdir.ansible_cache_root) @@ -1735,7 +1820,7 @@ class ExecutorServer(object): # up-to-date copies of all the repos that are used by jobs, as # well as to support the merger:cat functon to supply # configuration information to Zuul when it starts. - self.merger = self._getMerger(self.merge_root) + self.merger = self._getMerger(self.merge_root, None) self.update_queue = DeduplicateQueue() command_socket = get_default( @@ -1776,11 +1861,7 @@ class ExecutorServer(object): self.stopJobDiskFull, self.merge_root) - def _getMerger(self, root, logger=None): - if root != self.merge_root: - cache_root = self.merge_root - else: - cache_root = None + def _getMerger(self, root, cache_root, logger=None): return zuul.merger.merger.Merger( root, self.connections, self.merge_email, self.merge_name, self.merge_speed_limit, self.merge_speed_time, cache_root, logger) @@ -1960,6 +2041,12 @@ class ExecutorServer(object): self.log.info("Updating repo %s/%s" % ( task.connection_name, task.project_name)) self.merger.updateRepo(task.connection_name, task.project_name) + repo = self.merger.getRepo(task.connection_name, task.project_name) + source = self.connections.getSource(task.connection_name) + project = source.getProject(task.project_name) + task.canonical_name = project.canonical_name + task.branches = repo.getBranches() + task.refs = [r.name for r in repo.getRefs()] self.log.debug("Finished updating repo %s/%s" % (task.connection_name, task.project_name)) task.setComplete() diff --git a/zuul/model.py b/zuul/model.py index 275eb7e426..fa054d8efb 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -766,15 +766,14 @@ class Role(object, metaclass=abc.ABCMeta): class ZuulRole(Role): """A reference to an ansible role in a Zuul project.""" - def __init__(self, target_name, connection_name, project_name, - implicit=False): + def __init__(self, target_name, project_canonical_name, implicit=False): super(ZuulRole, self).__init__(target_name) - self.connection_name = connection_name - self.project_name = project_name + self.project_canonical_name = project_canonical_name self.implicit = implicit def __repr__(self): - return '' % (self.project_name, self.target_name) + return '' % (self.project_canonical_name, + self.target_name) __hash__ = object.__hash__ @@ -784,15 +783,13 @@ class ZuulRole(Role): # Implicit is not consulted for equality so that we can handle # implicit to explicit conversions. return (super(ZuulRole, self).__eq__(other) and - self.connection_name == other.connection_name and - self.project_name == other.project_name) + self.project_canonical_name == other.project_canonical_name) def toDict(self): # Render to a dict to use in passing json to the executor d = super(ZuulRole, self).toDict() d['type'] = 'zuul' - d['connection'] = self.connection_name - d['project'] = self.project_name + d['project_canonical_name'] = self.project_canonical_name d['implicit'] = self.implicit return d