From bd9094c9250975e7b556311b5e20562c2cc52af7 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Fri, 26 Aug 2016 13:42:13 -0400 Subject: [PATCH] include_role (role revamp implementation) (#17232) * attempt #11 to role_include * fixes from jimi-c * do not override load_data, move all to load * removed debugging * implemented tasks_from parameter, must break cache * fixed issue with cache and tasks_from * make resolution of from_tasks prioritize literal * avoid role dependency dedupe when include_role * fixed role deps and handlers are now loaded * simplified code, enabled k=v parsing used example from jimi-c * load role defaults for task when include_role * fixed issue with from_Tasks overriding all subdirs * corrected priority order of main candidates * made tasks_from a more generic interface to roles * fix block inheritance and handler order * allow vars: clause into included role * pull vars already processed vs from raw data * fix from jimi-c blocks i broke * added back append for dynamic includes * only allow for basename in from parameter * fix for docs when no default * fixed notes * added include_role to changelog --- CHANGELOG.md | 1 + lib/ansible/cli/doc.py | 6 +- lib/ansible/executor/task_executor.py | 9 +++ lib/ansible/modules/core | 2 +- lib/ansible/modules/extras | 2 +- lib/ansible/parsing/mod_args.py | 8 +-- lib/ansible/playbook/helpers.py | 21 ++++-- lib/ansible/playbook/role/__init__.py | 47 ++++++++++---- lib/ansible/playbook/role_include.py | 82 ++++++++++++++++++++++++ lib/ansible/plugins/strategy/__init__.py | 5 +- lib/ansible/vars/__init__.py | 10 +-- 11 files changed, 163 insertions(+), 30 deletions(-) create mode 100644 lib/ansible/playbook/role_include.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 83eb3603e6..55f17d9821 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ Ansible Changes By Release - ipmi * ipmi_boot * ipmi_power +- include_role - letsencrypt - logicmonitor - logicmonitor_facts diff --git a/lib/ansible/cli/doc.py b/lib/ansible/cli/doc.py index 14ce38b809..702b27ff10 100644 --- a/lib/ansible/cli/doc.py +++ b/lib/ansible/cli/doc.py @@ -283,13 +283,15 @@ class DocCLI(CLI): choices = '' if 'choices' in opt: choices = "(Choices: " + ", ".join(str(i) for i in opt['choices']) + ")" + default = '' if 'default' in opt or not required: default = "[Default: " + str(opt.get('default', '(null)')) + "]" text.append(textwrap.fill(CLI.tty_ify(choices + default), limit, initial_indent=opt_indent, subsequent_indent=opt_indent)) if 'notes' in doc and doc['notes'] and len(doc['notes']) > 0: - notes = " ".join(doc['notes']) - text.append("Notes:%s\n" % textwrap.fill(CLI.tty_ify(notes), limit-6, initial_indent=" ", subsequent_indent=opt_indent)) + text.append("Notes:") + for note in doc['notes']: + text.append(textwrap.fill(CLI.tty_ify(note), limit-6, initial_indent=" * ", subsequent_indent=opt_indent)) if 'requirements' in doc and doc['requirements'] is not None and len(doc['requirements']) > 0: req = ", ".join(doc['requirements']) diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index a0982d9a8b..295116209f 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -404,6 +404,15 @@ class TaskExecutor: include_file = templar.template(include_file) return dict(include=include_file, include_variables=include_variables) + #TODO: not needed? + # if this task is a IncludeRole, we just return now with a success code so the main thread can expand the task list for the given host + elif self._task.action == 'include_role': + include_variables = self._task.args.copy() + role = include_variables.pop('name') + if not role: + return dict(failed=True, msg="No role was specified to include") + return dict(name=role, include_variables=include_variables) + # Now we do final validation on the task, which sets all fields to their final values. self._task.post_validate(templar=templar) if '_variable_params' in self._task.args: diff --git a/lib/ansible/modules/core b/lib/ansible/modules/core index 4912ec30a7..91a839f1e3 160000 --- a/lib/ansible/modules/core +++ b/lib/ansible/modules/core @@ -1 +1 @@ -Subproject commit 4912ec30a71b09577a78f940c6a772b38b76f1a2 +Subproject commit 91a839f1e3de58be8b981d3278aa5dc8ff59c508 diff --git a/lib/ansible/modules/extras b/lib/ansible/modules/extras index f29efb5626..1aeb9f8a8c 160000 --- a/lib/ansible/modules/extras +++ b/lib/ansible/modules/extras @@ -1 +1 @@ -Subproject commit f29efb56264a9ad95b97765e367ef5b7915ab877 +Subproject commit 1aeb9f8a8c6d54663bbad6db385f568c04182ec6 diff --git a/lib/ansible/parsing/mod_args.py b/lib/ansible/parsing/mod_args.py index 96fb583a71..a4704bd8cc 100644 --- a/lib/ansible/parsing/mod_args.py +++ b/lib/ansible/parsing/mod_args.py @@ -264,7 +264,6 @@ class ModuleArgsParser: if 'action' in self._task_ds: # an old school 'action' statement thing = self._task_ds['action'] - action, args = self._normalize_parameters(thing, additional_args=additional_args) # local_action if 'local_action' in self._task_ds: @@ -273,19 +272,20 @@ class ModuleArgsParser: raise AnsibleParserError("action and local_action are mutually exclusive", obj=self._task_ds) thing = self._task_ds.get('local_action', '') delegate_to = 'localhost' - action, args = self._normalize_parameters(thing, additional_args=additional_args) # module: is the more new-style invocation # walk the input dictionary to see we recognize a module name for (item, value) in iteritems(self._task_ds): - if item in module_loader or item == 'meta' or item == 'include': + if item in module_loader or item in ['meta', 'include', 'include_role']: # finding more than one module name is a problem if action is not None: raise AnsibleParserError("conflicting action statements", obj=self._task_ds) action = item thing = value - action, args = self._normalize_parameters(value, action=action, additional_args=additional_args) + #TODO: find out if we should break here? Currently last matching action, break would make it first one + + action, args = self._normalize_parameters(thing, action=action, additional_args=additional_args) # if we didn't see any module in the task at all, it's not a task really if action is None: diff --git a/lib/ansible/playbook/helpers.py b/lib/ansible/playbook/helpers.py index fc6837dfbc..123e2d3f98 100644 --- a/lib/ansible/playbook/helpers.py +++ b/lib/ansible/playbook/helpers.py @@ -22,8 +22,7 @@ import os from ansible import constants as C from ansible.compat.six import string_types -from ansible.errors import AnsibleParserError, AnsibleUndefinedVariable, AnsibleFileNotFound -from ansible.parsing.yaml.objects import AnsibleBaseYAMLObject, AnsibleSequence +from ansible.errors import AnsibleParserError, AnsibleUndefinedVariable, AnsibleFileNotFound, AnsibleError try: from __main__ import display @@ -81,6 +80,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h from ansible.playbook.handler import Handler from ansible.playbook.task import Task from ansible.playbook.task_include import TaskInclude + from ansible.playbook.role_include import IncludeRole from ansible.playbook.handler_task_include import HandlerTaskInclude from ansible.template import Templar @@ -172,7 +172,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h if not found: try: include_target = templar.template(t.args['_raw_params']) - except AnsibleUndefinedVariable as e: + except AnsibleUndefinedVariable: raise AnsibleParserError( "Error when evaluating variable in include name: %s.\n\n" \ "When using static includes, ensure that any variables used in their names are defined in vars/vars_files\n" \ @@ -198,7 +198,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h # because the recursive nature of helper methods means we may be loading # nested includes, and we want the include order printed correctly display.display("statically included: %s" % include_file, color=C.COLOR_SKIP) - except AnsibleFileNotFound as e: + except AnsibleFileNotFound: if t.static or \ C.DEFAULT_TASK_INCLUDES_STATIC or \ C.DEFAULT_HANDLER_INCLUDES_STATIC and use_handlers: @@ -258,11 +258,24 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h task_list.extend(included_blocks) else: task_list.append(t) + + elif 'include_role' in task_ds: + task_list.extend( + IncludeRole.load( + task_ds, + block=block, + role=role, + task_include=None, + variable_manager=variable_manager, + loader=loader + ) + ) else: if use_handlers: t = Handler.load(task_ds, block=block, role=role, task_include=task_include, variable_manager=variable_manager, loader=loader) else: t = Task.load(task_ds, block=block, role=role, task_include=task_include, variable_manager=variable_manager, loader=loader) + task_list.append(t) return task_list diff --git a/lib/ansible/playbook/role/__init__.py b/lib/ansible/playbook/role/__init__.py index bccf860b26..6f91740d7c 100644 --- a/lib/ansible/playbook/role/__init__.py +++ b/lib/ansible/playbook/role/__init__.py @@ -66,7 +66,7 @@ class Role(Base, Become, Conditional, Taggable): _delegate_to = FieldAttribute(isa='string') _delegate_facts = FieldAttribute(isa='bool', default=False) - def __init__(self, play=None): + def __init__(self, play=None, from_files=None): self._role_name = None self._role_path = None self._role_params = dict() @@ -83,6 +83,10 @@ class Role(Base, Become, Conditional, Taggable): self._had_task_run = dict() self._completed = dict() + if from_files is None: + from_files = {} + self._tasks_from = from_files.get('tasks') + super(Role, self).__init__() def __repr__(self): @@ -92,7 +96,10 @@ class Role(Base, Become, Conditional, Taggable): return self._role_name @staticmethod - def load(role_include, play, parent_role=None): + def load(role_include, play, parent_role=None, from_files=None): + + if from_files is None: + from_files = {} try: # The ROLE_CACHE is a dictionary of role names, with each entry # containing another dictionary corresponding to a set of parameters @@ -104,6 +111,10 @@ class Role(Base, Become, Conditional, Taggable): params['when'] = role_include.when if role_include.tags is not None: params['tags'] = role_include.tags + if from_files is not None: + params['from_files'] = from_files + if role_include.vars: + params['vars'] = role_include.vars hashed_params = hash_params(params) if role_include.role in play.ROLE_CACHE: for (entry, role_obj) in iteritems(play.ROLE_CACHE[role_include.role]): @@ -112,7 +123,7 @@ class Role(Base, Become, Conditional, Taggable): role_obj.add_parent(parent_role) return role_obj - r = Role(play=play) + r = Role(play=play, from_files=from_files) r._load_role_data(role_include, parent_role=parent_role) if role_include.role not in play.ROLE_CACHE: @@ -163,7 +174,7 @@ class Role(Base, Become, Conditional, Taggable): else: self._metadata = RoleMetadata() - task_data = self._load_role_yaml('tasks') + task_data = self._load_role_yaml('tasks', main=self._tasks_from) if task_data: try: self._task_blocks = load_list_of_blocks(task_data, play=self._play, role=self, loader=self._loader, variable_manager=self._variable_manager) @@ -190,23 +201,36 @@ class Role(Base, Become, Conditional, Taggable): elif not isinstance(self._default_vars, dict): raise AnsibleParserError("The defaults/main.yml file for role '%s' must contain a dictionary of variables" % self._role_name) - def _load_role_yaml(self, subdir): + def _load_role_yaml(self, subdir, main=None): file_path = os.path.join(self._role_path, subdir) if self._loader.path_exists(file_path) and self._loader.is_directory(file_path): - main_file = self._resolve_main(file_path) + main_file = self._resolve_main(file_path, main) if self._loader.path_exists(main_file): return self._loader.load_from_file(main_file) return None - def _resolve_main(self, basepath): + def _resolve_main(self, basepath, main=None): ''' flexibly handle variations in main filenames ''' + + post = False + # allow override if set, otherwise use default + if main is None: + main = 'main' + post = True + + bare_main = os.path.join(basepath, main) + possible_mains = ( - os.path.join(basepath, 'main.yml'), - os.path.join(basepath, 'main.yaml'), - os.path.join(basepath, 'main.json'), - os.path.join(basepath, 'main'), + os.path.join(basepath, '%s.yml' % main), + os.path.join(basepath, '%s.yaml' % main), + os.path.join(basepath, '%s.json' % main), ) + if post: + possible_mains = possible_mains + (bare_main,) + else: + possible_mains = (bare_main,) + possible_mains + if sum([self._loader.is_file(x) for x in possible_mains]) > 1: raise AnsibleError("found multiple main files at %s, only one allowed" % (basepath)) else: @@ -274,6 +298,7 @@ class Role(Base, Become, Conditional, Taggable): for dep in self.get_all_dependencies(): all_vars = combine_vars(all_vars, dep.get_vars(include_params=include_params)) + all_vars = combine_vars(all_vars, self.vars) all_vars = combine_vars(all_vars, self._role_vars) if include_params: all_vars = combine_vars(all_vars, self.get_role_params(dep_chain=dep_chain)) diff --git a/lib/ansible/playbook/role_include.py b/lib/ansible/playbook/role_include.py new file mode 100644 index 0000000000..ce2d6aec8f --- /dev/null +++ b/lib/ansible/playbook/role_include.py @@ -0,0 +1,82 @@ +# Copyright (c) 2012 Red Hat, Inc. All rights reserved. +# +# This file is part of Ansible +# +# Ansible is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Ansible is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Ansible. If not, see . + +# Make coding more python3-ish +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +from os.path import basename + +from ansible.playbook.attribute import FieldAttribute +from ansible.playbook.task import Task +from ansible.playbook.role import Role +from ansible.playbook.role.include import RoleInclude + +try: + from __main__ import display +except ImportError: + from ansible.utils.display import Display + display = Display() + +__all__ = ['IncludeRole'] + + +class IncludeRole(Task): + + """ + A Role include is derived from a regular role to handle the special + circumstances related to the `- include_role: ...` + """ + + # ================================================================================= + # ATTRIBUTES + + _name = FieldAttribute(isa='string', default=None) + _tasks_from = FieldAttribute(isa='string', default=None) + + # these should not be changeable? + _static = FieldAttribute(isa='bool', default=False) + _private = FieldAttribute(isa='bool', default=True) + + @staticmethod + def load(data, block=None, role=None, task_include=None, variable_manager=None, loader=None): + + r = IncludeRole().load_data(data, variable_manager=variable_manager, loader=loader) + args = r.preprocess_data(data).get('args', dict()) + + ri = RoleInclude.load(args.get('name'), play=block._play, variable_manager=variable_manager, loader=loader) + ri.vars.update(r.vars) + + # build options for roles + from_files = {} + if args.get('tasks_from'): + from_files['tasks'] = basename(args.get('tasks_from')) + + #build role + actual_role = Role.load(ri, block._play, parent_role=role, from_files=from_files) + + # compile role + blocks = actual_role.compile(play=block._play) + + # set parent to ensure proper inheritance + for b in blocks: + b._parent = block + + # updated available handlers in play + block._play.handlers = block._play.handlers + actual_role.get_handler_blocks(play=block._play) + + return blocks diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py index 4a7c9f539e..88c5866444 100644 --- a/lib/ansible/plugins/strategy/__init__.py +++ b/lib/ansible/plugins/strategy/__init__.py @@ -40,6 +40,7 @@ from ansible.module_utils.facts import Facts from ansible.playbook.helpers import load_list_of_blocks from ansible.playbook.included_file import IncludedFile from ansible.playbook.task_include import TaskInclude +from ansible.playbook.role_include import IncludeRole from ansible.plugins import action_loader, connection_loader, filter_loader, lookup_loader, module_loader, test_loader from ansible.template import Templar from ansible.utils.unicode import to_unicode @@ -258,7 +259,7 @@ class StrategyBase: def parent_handler_match(target_handler, handler_name): if target_handler: - if isinstance(target_handler, TaskInclude): + if isinstance(target_handler, (TaskInclude, IncludeRole)): try: handler_vars = self._variable_manager.get_vars(loader=self._loader, play=iterator._play, task=target_handler) templar = Templar(loader=self._loader, variables=handler_vars) @@ -477,7 +478,7 @@ class StrategyBase: # If this is a role task, mark the parent role as being run (if # the task was ok or failed, but not skipped or unreachable) - if original_task._role is not None and role_ran: + if original_task._role is not None and role_ran and original_task.action != 'include_role': # lookup the role in the ROLE_CACHE to make sure we're dealing # with the correct object and mark it as executed for (entry, role_obj) in iteritems(iterator._play.ROLE_CACHE[original_task._role._role_name]): diff --git a/lib/ansible/vars/__init__.py b/lib/ansible/vars/__init__.py index 793e346ada..5f714ce116 100644 --- a/lib/ansible/vars/__init__.py +++ b/lib/ansible/vars/__init__.py @@ -232,11 +232,11 @@ class VariableManager: for role in play.get_roles(): all_vars = combine_vars(all_vars, role.get_default_vars()) - # if we have a task in this context, and that task has a role, make - # sure it sees its defaults above any other roles, as we previously - # (v1) made sure each task had a copy of its roles default vars - if task and task._role is not None: - all_vars = combine_vars(all_vars, task._role.get_default_vars(dep_chain=task.get_dep_chain())) + # if we have a task in this context, and that task has a role, make + # sure it sees its defaults above any other roles, as we previously + # (v1) made sure each task had a copy of its roles default vars + if task and task._role is not None and (play or task.action == 'include_role'): + all_vars = combine_vars(all_vars, task._role.get_default_vars(dep_chain=task.get_dep_chain())) if host: # next, if a host is specified, we load any vars from group_vars