From d0a3567221011eda22c9b42645887e5eb623e308 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Tue, 3 Apr 2018 15:12:09 -0700 Subject: [PATCH] Check out more appropriate branches of role and playbook repos Currently when a job adds a zuul role repo to a playbook, we only use the master branch of the role repo, unless the role repo appears in the dependency chain for the change under test. That means that if the role repo appears in required-projects, but not as a dependency, then we use the master branch instead of what was specified in required-projects. That doesn't seem to make much sense and is likely an oversight. We attempt to use the prepared repos where possible (ie, the requested branches match and the playbook is not trusted). However, the current check for that only looks at 'items', that is, the dependency chain. Instead, we should look at 'projects', which includes not only the projects which appear in 'items', but also those that appear in required-projects. The same check is performed for playbooks, and therefore is also updated. Also, in the case where a role repo doesn't appear in either the dependency chain or in required-projects, we were hard-coded to check out the 'master' branch. Instead, re-use some of the logic used when preparing required-projects to attempt to find the best branch to check out. We will try the job override branch first, then the zuul branch, then the project default branch. All playbook project repos are now prepared outside of the work dir, even in cases where their projects also appear in the work dir. If the playbook is untrusted, then the repo is cloned into the "untrusted/" jobdir directory (with speculative changes applied). To account for this, the "allow_trusted" flag in the ansible safe path checker is updated to allow access to both "trusted/" and "untrusted/" paths. Change-Id: If95a9b0aaff982040cd4e6e957f9588b26ef7935 --- doc/source/user/config.rst | 9 + .../role-checkouts-89632d2ff5eb8b78.yaml | 15 + .../git/common-config/playbooks/pre-base.yaml | 3 + .../roles/pre-base/tasks/main.yaml | 3 + .../role-branches/git/common-config/zuul.yaml | 62 +++++ .../config/role-branches/git/project1/README | 1 + .../git/project1/playbooks/parent-job.yaml | 3 + .../roles/run-parent-job/tasks/main.yaml | 3 + .../role-branches/git/project1/zuul.yaml | 6 + .../config/role-branches/git/project2/README | 1 + .../git/project2/playbooks/child-job.yaml | 3 + .../role-branches/git/project2/zuul.yaml | 23 ++ tests/fixtures/config/role-branches/main.yaml | 9 + tests/unit/test_v3.py | 122 ++++++++- zuul/ansible/paths.py | 2 + zuul/configloader.py | 6 +- zuul/executor/client.py | 25 +- zuul/executor/server.py | 257 ++++++++++++------ zuul/model.py | 15 +- 19 files changed, 465 insertions(+), 103 deletions(-) create mode 100644 releasenotes/notes/role-checkouts-89632d2ff5eb8b78.yaml create mode 100644 tests/fixtures/config/role-branches/git/common-config/playbooks/pre-base.yaml create mode 100644 tests/fixtures/config/role-branches/git/common-config/roles/pre-base/tasks/main.yaml create mode 100644 tests/fixtures/config/role-branches/git/common-config/zuul.yaml create mode 100644 tests/fixtures/config/role-branches/git/project1/README create mode 100644 tests/fixtures/config/role-branches/git/project1/playbooks/parent-job.yaml create mode 100644 tests/fixtures/config/role-branches/git/project1/roles/run-parent-job/tasks/main.yaml create mode 100644 tests/fixtures/config/role-branches/git/project1/zuul.yaml create mode 100644 tests/fixtures/config/role-branches/git/project2/README create mode 100644 tests/fixtures/config/role-branches/git/project2/playbooks/child-job.yaml create mode 100644 tests/fixtures/config/role-branches/git/project2/zuul.yaml create mode 100644 tests/fixtures/config/role-branches/main.yaml 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