From bb94dfa19a275d9aafd206b6104a04601ac84f20 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Tue, 11 Jul 2017 07:45:19 -0700 Subject: [PATCH] Add job's project as implicit role project Add the project in which a job is defined as an implicit role project. This is a convenience for job authors who may want to put roles in the root of the project (ie, not adjacent to job playbooks, since, after all, the roles may be useful outside of the job playbooks). In that case, they will not need to specify the job's own project in the roles: section of the job. Change-Id: Ia382c2da9f7eb7139ceb0b61cb986aace8dc8d8f --- doc/source/user/config.rst | 6 ++ .../git/common-config/zuul.yaml | 12 ++++ .../git/org_norole-project/.zuul.yaml | 15 +++++ .../git/org_norole-project/README | 1 + .../playbooks/explicit-role-fail.yaml | 2 + .../playbooks/implicit-role-fail.yaml | 2 + .../git/org_role-project/.zuul.yaml | 15 +++++ .../git/org_role-project/README | 1 + .../playbooks/explicit-role-ok.yaml | 2 + .../playbooks/implicit-role-ok.yaml | 2 + .../git/org_role-project/roles/README | 0 .../fixtures/config/implicit-roles/main.yaml | 9 +++ tests/unit/test_v3.py | 59 ++++++++++++++++++- zuul/configloader.py | 18 +++++- zuul/executor/server.py | 21 +++++-- zuul/model.py | 31 +++++++++- 16 files changed, 184 insertions(+), 12 deletions(-) create mode 100644 tests/fixtures/config/implicit-roles/git/common-config/zuul.yaml create mode 100644 tests/fixtures/config/implicit-roles/git/org_norole-project/.zuul.yaml create mode 100644 tests/fixtures/config/implicit-roles/git/org_norole-project/README create mode 100644 tests/fixtures/config/implicit-roles/git/org_norole-project/playbooks/explicit-role-fail.yaml create mode 100644 tests/fixtures/config/implicit-roles/git/org_norole-project/playbooks/implicit-role-fail.yaml create mode 100644 tests/fixtures/config/implicit-roles/git/org_role-project/.zuul.yaml create mode 100644 tests/fixtures/config/implicit-roles/git/org_role-project/README create mode 100644 tests/fixtures/config/implicit-roles/git/org_role-project/playbooks/explicit-role-ok.yaml create mode 100644 tests/fixtures/config/implicit-roles/git/org_role-project/playbooks/implicit-role-ok.yaml create mode 100644 tests/fixtures/config/implicit-roles/git/org_role-project/roles/README create mode 100644 tests/fixtures/config/implicit-roles/main.yaml diff --git a/doc/source/user/config.rst b/doc/source/user/config.rst index 76f73c3407..c4f09de5e2 100644 --- a/doc/source/user/config.rst +++ b/doc/source/user/config.rst @@ -738,6 +738,12 @@ unless otherwise specified: be installed (and therefore referenced from Ansible), the `name` attribute may be used to specify an alternate. + A job automatically has the project in which it is defined added to + the roles path if that project appears to contain a role or `roles/` + directory. By default, the project is added to the path under its + own name, however, that may be changed by explicitly listing the + project in the roles list in the usual way. + .. note:: galaxy roles are not yet implemented **galaxy** diff --git a/tests/fixtures/config/implicit-roles/git/common-config/zuul.yaml b/tests/fixtures/config/implicit-roles/git/common-config/zuul.yaml new file mode 100644 index 0000000000..ba91fb5557 --- /dev/null +++ b/tests/fixtures/config/implicit-roles/git/common-config/zuul.yaml @@ -0,0 +1,12 @@ +- pipeline: + name: check + manager: independent + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + verified: 1 + failure: + gerrit: + verified: -1 diff --git a/tests/fixtures/config/implicit-roles/git/org_norole-project/.zuul.yaml b/tests/fixtures/config/implicit-roles/git/org_norole-project/.zuul.yaml new file mode 100644 index 0000000000..74c8e8ea32 --- /dev/null +++ b/tests/fixtures/config/implicit-roles/git/org_norole-project/.zuul.yaml @@ -0,0 +1,15 @@ +- job: + name: implicit-role-fail + +- job: + name: explicit-role-fail + attempts: 1 + roles: + - zuul: org/norole-project + +- project: + name: org/norole-project + check: + jobs: + - implicit-role-fail + - explicit-role-fail diff --git a/tests/fixtures/config/implicit-roles/git/org_norole-project/README b/tests/fixtures/config/implicit-roles/git/org_norole-project/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/implicit-roles/git/org_norole-project/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/implicit-roles/git/org_norole-project/playbooks/explicit-role-fail.yaml b/tests/fixtures/config/implicit-roles/git/org_norole-project/playbooks/explicit-role-fail.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/implicit-roles/git/org_norole-project/playbooks/explicit-role-fail.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/implicit-roles/git/org_norole-project/playbooks/implicit-role-fail.yaml b/tests/fixtures/config/implicit-roles/git/org_norole-project/playbooks/implicit-role-fail.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/implicit-roles/git/org_norole-project/playbooks/implicit-role-fail.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/implicit-roles/git/org_role-project/.zuul.yaml b/tests/fixtures/config/implicit-roles/git/org_role-project/.zuul.yaml new file mode 100644 index 0000000000..42cae95dd7 --- /dev/null +++ b/tests/fixtures/config/implicit-roles/git/org_role-project/.zuul.yaml @@ -0,0 +1,15 @@ +- job: + name: implicit-role-ok + +- job: + name: explicit-role-ok + roles: + - zuul: org/role-project + name: role-name + +- project: + name: org/role-project + check: + jobs: + - implicit-role-ok + - explicit-role-ok diff --git a/tests/fixtures/config/implicit-roles/git/org_role-project/README b/tests/fixtures/config/implicit-roles/git/org_role-project/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/implicit-roles/git/org_role-project/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/implicit-roles/git/org_role-project/playbooks/explicit-role-ok.yaml b/tests/fixtures/config/implicit-roles/git/org_role-project/playbooks/explicit-role-ok.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/implicit-roles/git/org_role-project/playbooks/explicit-role-ok.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/implicit-roles/git/org_role-project/playbooks/implicit-role-ok.yaml b/tests/fixtures/config/implicit-roles/git/org_role-project/playbooks/implicit-role-ok.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/implicit-roles/git/org_role-project/playbooks/implicit-role-ok.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/implicit-roles/git/org_role-project/roles/README b/tests/fixtures/config/implicit-roles/git/org_role-project/roles/README new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/fixtures/config/implicit-roles/main.yaml b/tests/fixtures/config/implicit-roles/main.yaml new file mode 100644 index 0000000000..d5e481a543 --- /dev/null +++ b/tests/fixtures/config/implicit-roles/main.yaml @@ -0,0 +1,9 @@ +- tenant: + name: tenant-one + source: + gerrit: + config-projects: + - common-config + untrusted-projects: + - org/norole-project + - org/role-project diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index 734c45c954..b162469434 100644 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -716,9 +716,7 @@ class TestProjectKeys(ZuulTestCase): self.assertEqual(4096, private_key.key_size) -class TestRoles(ZuulTestCase): - tenant_config_file = 'config/roles/main.yaml' - +class RoleTestCase(ZuulTestCase): def _assertRolePath(self, build, playbook, content): path = os.path.join(self.test_root, build.uuid, 'ansible', playbook, 'ansible.cfg') @@ -738,6 +736,10 @@ class TestRoles(ZuulTestCase): "Should have no roles_path line in %s" % (playbook,)) + +class TestRoles(RoleTestCase): + tenant_config_file = 'config/roles/main.yaml' + def test_role(self): # This exercises a proposed change to a role being checked out # and used. @@ -822,6 +824,57 @@ class TestRoles(ZuulTestCase): A.messages[-1]) +class TestImplicitRoles(RoleTestCase): + tenant_config_file = 'config/implicit-roles/main.yaml' + + def test_missing_roles(self): + # Test implicit and explicit roles for a project which does + # not have roles. The implicit role should be silently + # ignored since the project doesn't supply roles, but if a + # user declares an explicit role, it should error. + self.executor_server.hold_jobs_in_build = True + A = self.fake_gerrit.addFakeChange('org/norole-project', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + self.assertEqual(len(self.builds), 2) + build = self.getBuildByName('implicit-role-fail') + self._assertRolePath(build, 'playbook_0', None) + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + # The retry_limit doesn't get recorded + self.assertHistory([ + dict(name='implicit-role-fail', result='SUCCESS', changes='1,1'), + ]) + + def test_roles(self): + # Test implicit and explicit roles for a project which does + # have roles. In both cases, we should end up with the role + # in the path. In the explicit case, ensure we end up with + # the name we specified. + self.executor_server.hold_jobs_in_build = True + A = self.fake_gerrit.addFakeChange('org/role-project', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + self.assertEqual(len(self.builds), 2) + build = self.getBuildByName('implicit-role-ok') + self._assertRolePath(build, 'playbook_0', 'role_0') + + build = self.getBuildByName('explicit-role-ok') + self._assertRolePath(build, 'playbook_0', 'role_0') + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + self.assertHistory([ + dict(name='implicit-role-ok', result='SUCCESS', changes='1,1'), + dict(name='explicit-role-ok', result='SUCCESS', changes='1,1'), + ], ordered=False) + + class TestShadow(ZuulTestCase): tenant_config_file = 'config/shadow/main.yaml' diff --git a/zuul/configloader.py b/zuul/configloader.py index f8e2d15bf5..48d9134bba 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -425,14 +425,19 @@ class JobParser(object): # Roles are part of the playbook context so we must establish # them earlier than playbooks. + roles = [] 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) + # A job's repo should be an implicit role source for that job, + # but not in a project-pipeline variant. + if not project_pipeline: + r = JobParser._makeImplicitRole(job) + roles.insert(0, r) + job.addRoles(roles) for pre_run_name in as_list(conf.get('pre-run')): pre_run = model.PlaybookContext(job.source_context, @@ -554,6 +559,15 @@ class JobParser(object): project.connection_name, project.name) + @staticmethod + def _makeImplicitRole(job): + project = job.source_context.project + name = project.name.split('/')[-1] + return model.ZuulRole(name, + project.connection_name, + project.name, + implicit=True) + class ProjectTemplateParser(object): log = logging.getLogger("zuul.ProjectTemplateParser") diff --git a/zuul/executor/server.py b/zuul/executor/server.py index f291dce9cd..ae362638fe 100644 --- a/zuul/executor/server.py +++ b/zuul/executor/server.py @@ -51,6 +51,10 @@ class ExecutorError(Exception): pass +class RoleNotFoundError(ExecutorError): + pass + + class Watchdog(object): def __init__(self, timeout, function, args): self.timeout = timeout @@ -1137,12 +1141,14 @@ class AnsibleJob(object): if os.path.isdir(d): # This repo has a collection of roles if not trusted: + self._blockPluginDirs(d) for entry in os.listdir(d): - if os.path.isdir(os.path.join(d, entry)): - self._blockPluginDirs(os.path.join(d, entry)) + entry_path = os.path.join(d, entry) + if os.path.isdir(entry_path): + self._blockPluginDirs(entry_path) return d # It is neither a bare role, nor a collection of roles - raise ExecutorError("Unable to find role in %s" % (path,)) + raise RoleNotFoundError("Unable to find role in %s" % (path,)) def prepareZuulRole(self, jobdir_playbook, role, args, root): self.log.debug("Prepare zuul role for %s" % (role,)) @@ -1183,10 +1189,17 @@ class AnsibleJob(object): raise ExecutorError("Invalid role name %s", name) os.symlink(path, link) - role_path = self.findRole(link, trusted=jobdir_playbook.trusted) + try: + role_path = self.findRole(link, trusted=jobdir_playbook.trusted) + except RoleNotFoundError: + if role['implicit']: + self.log.info("Implicit role not found in %s", link) + return + raise if role_path is None: # In the case of a bare role, add the containing directory role_path = root + self.log.debug("Adding role path %s", role_path) jobdir_playbook.roles_path.append(role_path) def prepareAnsibleFiles(self, args): diff --git a/zuul/model.py b/zuul/model.py index 1df70dbc06..693b729374 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -702,10 +702,12 @@ 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): + def __init__(self, target_name, connection_name, project_name, + implicit=False): super(ZuulRole, self).__init__(target_name) self.connection_name = connection_name self.project_name = project_name + self.implicit = implicit def __repr__(self): return '' % (self.project_name, self.target_name) @@ -715,6 +717,8 @@ class ZuulRole(Role): def __eq__(self, other): if not isinstance(other, ZuulRole): return False + # 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) @@ -725,6 +729,7 @@ class ZuulRole(Role): d['type'] = 'zuul' d['connection'] = self.connection_name d['project'] = self.project_name + d['implicit'] = self.implicit return d @@ -867,11 +872,31 @@ class Job(object): self.run = self.implied_run def addRoles(self, roles): - newroles = list(self.roles) + newroles = [] + # Start with a copy of the existing roles, but if any of them + # are implicit roles which are identified as explicit in the + # new roles list, replace them with the explicit version. + changed = False + for existing_role in self.roles: + if existing_role in roles: + new_role = roles[roles.index(existing_role)] + else: + new_role = None + if (new_role and + isinstance(new_role, ZuulRole) and + isinstance(existing_role, ZuulRole) and + existing_role.implicit and not new_role.implicit): + newroles.append(new_role) + changed = True + else: + newroles.append(existing_role) + # Now add the new roles. for role in reversed(roles): if role not in newroles: newroles.insert(0, role) - self.roles = tuple(newroles) + changed = True + if changed: + self.roles = tuple(newroles) def updateVariables(self, other_vars): v = self.variables