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

@@ -39,6 +39,7 @@ Ansible Changes By Release
 - ipmi
   * ipmi_boot
   * ipmi_power
+- include_role
 - letsencrypt
 - logicmonitor
 - logicmonitor_facts

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'])

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:

lib/ansible/modules/core

@@ -1 +1 @@
-Subproject commit 4912ec30a71b09577a78f940c6a772b38b76f1a2
+Subproject commit 91a839f1e3de58be8b981d3278aa5dc8ff59c508

lib/ansible/modules/extras

@@ -1 +1 @@
-Subproject commit f29efb56264a9ad95b97765e367ef5b7915ab877
+Subproject commit 1aeb9f8a8c6d54663bbad6db385f568c04182ec6

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: <stuff> 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:

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

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))

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 <http://www.gnu.org/licenses/>.
+
+# 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

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]):

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