Merge a175037466 into 6aa8192131

changelogs/fragments/end_role.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - Add a new meta task ``end_role`` (https://github.com/ansible/ansible/issues/22286)

lib/ansible/executor/play_iterator.py

@@ -42,6 +42,7 @@ class IteratingStates(IntEnum):
     ALWAYS = 3
     HANDLERS = 4
     COMPLETE = 5
+    SKIP_ROLE = 6
 
 
 class FailedStates(IntFlag):
@@ -67,7 +68,7 @@ class HostState:
         self.cur_handlers_task = 0
         self.run_state = IteratingStates.SETUP
         self.fail_state = FailedStates.NONE
-        self.pre_flushing_run_state = None
+        self.prior_run_state = None
         self.update_handlers = True
         self.pending_setup = False
         self.tasks_child_state = None
@@ -81,7 +82,7 @@ class HostState:
 
     def __str__(self):
         return ("HOST STATE: block=%d, task=%d, rescue=%d, always=%d, handlers=%d, run_state=%s, fail_state=%s, "
-                "pre_flushing_run_state=%s, update_handlers=%s, pending_setup=%s, "
+                "prior_run_state=%s, update_handlers=%s, pending_setup=%s, "
                 "tasks child state? (%s), rescue child state? (%s), always child state? (%s), "
                 "did rescue? %s, did start at task? %s" % (
                     self.cur_block,
@@ -91,7 +92,7 @@ class HostState:
                     self.cur_handlers_task,
                     self.run_state,
                     self.fail_state,
-                    self.pre_flushing_run_state,
+                    self.prior_run_state,
                     self.update_handlers,
                     self.pending_setup,
                     self.tasks_child_state,
@@ -107,7 +108,7 @@ class HostState:
 
         for attr in ('_blocks',
                      'cur_block', 'cur_regular_task', 'cur_rescue_task', 'cur_always_task', 'cur_handlers_task',
-                     'run_state', 'fail_state', 'pre_flushing_run_state', 'update_handlers', 'pending_setup',
+                     'run_state', 'fail_state', 'prior_run_state', 'update_handlers', 'pending_setup',
                      'tasks_child_state', 'rescue_child_state', 'always_child_state'):
             if getattr(self, attr) != getattr(other, attr):
                 return False
@@ -128,7 +129,7 @@ class HostState:
         new_state.cur_handlers_task = self.cur_handlers_task
         new_state.run_state = self.run_state
         new_state.fail_state = self.fail_state
-        new_state.pre_flushing_run_state = self.pre_flushing_run_state
+        new_state.prior_run_state = self.prior_run_state
         new_state.update_handlers = self.update_handlers
         new_state.pending_setup = self.pending_setup
         new_state.did_rescue = self.did_rescue
@@ -434,20 +435,34 @@ class PlayIterator:
                     except IndexError:
                         task = None
                         state.cur_handlers_task = 0
-                        state.run_state = state.pre_flushing_run_state
+                        state.run_state, state.prior_run_state = state.prior_run_state, None
                         state.update_handlers = True
                         break
                     else:
                         state.cur_handlers_task += 1
                         if task.is_host_notified(host):
-                            break
+                            return state, task
+
+            elif state.run_state == IteratingStates.SKIP_ROLE:
+                state.run_state, state.prior_run_state = state.prior_run_state, None
+                while True:
+                    state, task = self._get_next_task_from_state(state, host=host)
+                    if task.implicit and task.args.get("_raw_params") == "end_role":
+                        return state, task
+                    display.debug("'%s' skipped because role has been ended via 'end_role'" % task)
 
             elif state.run_state == IteratingStates.COMPLETE:
                 return (state, None)
 
-            # if something above set the task, break out of the loop now
             if task:
-                break
+                if (
+                    (role := task._role)
+                    and role._metadata.allow_duplicates is False
+                    and host.name in self._play._get_cached_role(role)._completed
+                ):
+                    display.debug("'%s' skipped because role has already run" % task)
+                else:
+                    break
 
         return (state, task)
 
@@ -489,7 +504,7 @@ class PlayIterator:
         if s.run_state == IteratingStates.HANDLERS:
             # we are failing `meta: flush_handlers`, so just reset the state to whatever
             # it was before and let `_set_failed_state` figure out the next state
-            s.run_state = s.pre_flushing_run_state
+            s.run_state, s.prior_run_state = s.prior_run_state, None
             s.update_handlers = True
         s = self._set_failed_state(s)
         display.debug("^ failed state is now: %s" % s)
@@ -620,7 +635,10 @@ class PlayIterator:
     def set_run_state_for_host(self, hostname: str, run_state: IteratingStates) -> None:
         if not isinstance(run_state, IteratingStates):
             raise AnsibleAssertionError('Expected run_state to be a IteratingStates but was %s' % (type(run_state)))
-        self._host_states[hostname].run_state = run_state
+        host_state = self._host_states[hostname]
+        if run_state in (IteratingStates.HANDLERS, IteratingStates.SKIP_ROLE):
+            host_state.prior_run_state = host_state.run_state
+        host_state.run_state = run_state
 
     def set_fail_state_for_host(self, hostname: str, fail_state: FailedStates) -> None:
         if not isinstance(fail_state, FailedStates):

lib/ansible/modules/meta.py

@@ -33,7 +33,12 @@ options:
         - V(end_host) (added in Ansible 2.8) is a per-host variation of V(end_play). Causes the play to end for the current host without failing it.
         - V(end_batch) (added in Ansible 2.12) causes the current batch (see C(serial)) to end without failing the host(s).
           Note that with C(serial=0) or undefined this behaves the same as V(end_play).
-    choices: [ clear_facts, clear_host_errors, end_host, end_play, flush_handlers, noop, refresh_inventory, reset_connection, end_batch ]
+        - V(end_role) (added in Ansible 2.18) causes the currently executing role to end without failing the host(s).
+          Effectively all tasks from within a role after V(end_role) is executed are ignored. Since handlers live in a global,
+          play scope, all handlers added via the role are unaffected and are still executed if notified. It is an error
+          to call V(end_role) from outside of a role or from a handler. Note that V(end_role) does not have an effect to
+          the parent roles or roles that depend (via dependencies in meta/main.yml) on a role executing V(end_role).
+    choices: [ clear_facts, clear_host_errors, end_host, end_play, flush_handlers, noop, refresh_inventory, reset_connection, end_batch, end_role ]
     required: true
 extends_documentation_fragment:
     - action_common_attributes

lib/ansible/playbook/helpers.py

@@ -293,8 +293,12 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h
             else:
                 if use_handlers:
                     t = Handler.load(task_ds, block=block, role=role, task_include=task_include, variable_manager=variable_manager, loader=loader)
+                    if t.action in C._ACTION_META and t.args.get('_raw_params') == "end_role":
+                        raise AnsibleParserError("Cannot execute 'end_role' from a handler")
                 else:
                     t = Task.load(task_ds, block=block, role=role, task_include=task_include, variable_manager=variable_manager, loader=loader)
+                    if t.action in C._ACTION_META and t.args.get('_raw_params') == "end_role" and role is None:
+                        raise AnsibleParserError("Cannot execute 'end_role' from outside of a role")
 
                 task_list.append(t)
 

lib/ansible/playbook/play.py

@@ -19,7 +19,7 @@ from __future__ import annotations
 
 from ansible import constants as C
 from ansible import context
-from ansible.errors import AnsibleParserError, AnsibleAssertionError
+from ansible.errors import AnsibleParserError, AnsibleAssertionError, AnsibleError
 from ansible.module_utils.common.text.converters import to_native
 from ansible.module_utils.common.collections import is_sequence
 from ansible.module_utils.six import binary_type, string_types, text_type
@@ -100,6 +100,15 @@ class Play(Base, Taggable, CollectionSearch):
     def __repr__(self):
         return self.get_name()
 
+    def _get_cached_role(self, role):
+        role_path = role.get_role_path()
+        role_cache = self.role_cache[role_path]
+        try:
+            idx = role_cache.index(role)
+            return role_cache[idx]
+        except ValueError:
+            raise AnsibleError(f'Cannot locate {role.get_name()} in role cache')
+
     def _validate_hosts(self, attribute, name, value):
         # Only validate 'hosts' if a value was passed in to original data set.
         if 'hosts' in self._ds:

lib/ansible/playbook/role/__init__.py

@@ -583,7 +583,7 @@ class Role(Base, Conditional, Taggable, CollectionSearch, Delegatable):
         Returns true if this role has been iterated over completely and
         at least one task was run
         '''
-
+        # FIXME unused, deprecate?
         return host.name in self._completed
 
     def compile(self, play, dep_chain=None):
@@ -626,7 +626,7 @@ class Role(Base, Conditional, Taggable, CollectionSearch, Delegatable):
         eor_task = Task(block=eor_block)
         eor_task._role = self
         eor_task.action = 'meta'
-        eor_task.args = {'_raw_params': 'role_complete'}
+        eor_task.args = {'_raw_params': 'end_role'}
         eor_task.implicit = True
         eor_task.tags = ['always']
         eor_task.when = True

lib/ansible/plugins/strategy/__init__.py

@@ -978,8 +978,7 @@ class StrategyBase:
                 if host_state.run_state == IteratingStates.HANDLERS:
                     raise AnsibleError('flush_handlers cannot be used as a handler')
                 if target_host.name not in self._tqm._unreachable_hosts:
-                    host_state.pre_flushing_run_state = host_state.run_state
-                    host_state.run_state = IteratingStates.HANDLERS
+                    iterator.set_run_state_for_host(target_host.name, IteratingStates.HANDLERS)
                 msg = "triggered running handlers for %s" % target_host.name
             else:
                 skipped = True
@@ -1038,14 +1037,19 @@ class StrategyBase:
                 skip_reason += ", continuing execution for %s" % target_host.name
                 # TODO: Nix msg here? Left for historical reasons, but skip_reason exists now.
                 msg = "end_host conditional evaluated to false, continuing execution for %s" % target_host.name
-        elif meta_action == 'role_complete':
-            # Allow users to use this in a play as reported in https://github.com/ansible/ansible/issues/22286?
-            # How would this work with allow_duplicates??
+        elif meta_action == 'end_role':
             if task.implicit:
                 role_obj = self._get_cached_role(task, iterator._play)
                 if target_host.name in role_obj._had_task_run:
                     role_obj._completed[target_host.name] = True
-                    msg = 'role_complete for %s' % target_host.name
+                    msg = 'role complete for %s' % target_host.name
+            else:
+                if _evaluate_conditional(target_host):
+                    iterator.set_run_state_for_host(target_host.name, IteratingStates.SKIP_ROLE)
+                    msg = 'ending role %s for %s' % (task._role.get_name(), target_host.name)
+                else:
+                    skipped = True
+                    skip_reason += ', continuing role'
         elif meta_action == 'reset_connection':
             all_vars = self._variable_manager.get_vars(play=iterator._play, host=target_host, task=task,
                                                        _hosts=self._hosts_cache, _hosts_all=self._hosts_cache_all)
@@ -1107,13 +1111,7 @@ class StrategyBase:
         return [res]
 
     def _get_cached_role(self, task, play):
-        role_path = task._role.get_role_path()
-        role_cache = play.role_cache[role_path]
-        try:
-            idx = role_cache.index(task._role)
-            return role_cache[idx]
-        except ValueError:
-            raise AnsibleError(f'Cannot locate {task._role.get_name()} in role cache')
+        return play._get_cached_role(task._role)
 
     def get_hosts_left(self, iterator):
         ''' returns list of available hosts for this iterator by filtering out unreachables '''

lib/ansible/plugins/strategy/free.py

@@ -171,15 +171,6 @@ class StrategyModule(StrategyBase):
                                 display.warning("Using run_once with the free strategy is not currently supported. This task will still be "
                                                 "executed for every host in the inventory list.")
 
-                        # check to see if this task should be skipped, due to it being a member of a
-                        # role which has already run (and whether that role allows duplicate execution)
-                        if not isinstance(task, Handler) and task._role:
-                            role_obj = self._get_cached_role(task, iterator._play)
-                            if role_obj.has_run(host) and task._role._metadata.allow_duplicates is False:
-                                display.debug("'%s' skipped because role has already run" % task, host=host_name)
-                                del self._blocked_hosts[host_name]
-                                continue
-
                         if task.action in C._ACTION_META:
                             self._execute_meta(task, play_context, iterator, target_host=host)
                             self._blocked_hosts[host_name] = False

lib/ansible/plugins/strategy/linear.py

@@ -166,14 +166,6 @@ class StrategyModule(StrategyBase):
                     run_once = False
                     work_to_do = True
 
-                    # check to see if this task should be skipped, due to it being a member of a
-                    # role which has already run (and whether that role allows duplicate execution)
-                    if not isinstance(task, Handler) and task._role:
-                        role_obj = self._get_cached_role(task, iterator._play)
-                        if role_obj.has_run(host) and task._role._metadata.allow_duplicates is False:
-                            display.debug("'%s' skipped because role has already run" % task)
-                            continue
-
                     display.debug("getting variables")
                     task_vars = self._variable_manager.get_vars(play=iterator._play, host=host, task=task,
                                                                 _hosts=self._hosts_cache, _hosts_all=self._hosts_cache_all)
@@ -198,7 +190,7 @@ class StrategyModule(StrategyBase):
                         # for the linear strategy, we run meta tasks just once and for
                         # all hosts currently being iterated over rather than one host
                         results.extend(self._execute_meta(task, play_context, iterator, host))
-                        if task.args.get('_raw_params', None) not in ('noop', 'reset_connection', 'end_host', 'role_complete', 'flush_handlers'):
+                        if task.args.get('_raw_params', None) not in ('noop', 'reset_connection', 'end_host', 'end_role', 'flush_handlers'):
                             run_once = True
                         if (task.any_errors_fatal or run_once) and not task.ignore_errors:
                             any_errors_fatal = True

test/integration/targets/roles/end_role.yml

@@ -0,0 +1,35 @@
+- hosts: localhost
+  gather_facts: false
+  pre_tasks:
+    - set_fact:
+        play_checkpoint: 1
+  roles:
+    - end_role_inside
+  tasks:
+    - set_fact:
+        play_checkpoint: "{{ play_checkpoint|int + 1 }}"
+
+    - import_role:
+        name: end_role_inside
+        allow_duplicates: true
+
+    - set_fact:
+        play_checkpoint: "{{ play_checkpoint|int + 1 }}"
+
+    - include_role:
+        name: end_role_inside
+        allow_duplicates: false
+
+    - set_fact:
+        play_checkpoint: "{{ play_checkpoint|int + 1 }}"
+  post_tasks:
+    - assert:
+        that:
+          - role_executed|int == 2
+          - after_end_role is undefined
+          - play_checkpoint|int == 4
+          - role_handler_ran is defined
+
+    - name: when running this playbook check this appears on stdout to ensure the above assert wasn't skipped
+      debug:
+        msg: CHECKPOINT

test/integration/targets/roles/end_role_handler_error.yml

@@ -0,0 +1,9 @@
+- hosts: localhost
+  gather_facts: false
+  tasks:
+    - debug:
+      changed_when: true
+      notify: invalid_handler
+  handlers:
+    - name: invalid_handler
+      meta: end_role

test/integration/targets/roles/end_role_nested.yml

@@ -0,0 +1,6 @@
+- hosts: host1,host2
+  gather_facts: false
+  tasks:
+    - include_role:
+        name: end_role_inside
+        tasks_from: nested.yml

test/integration/targets/roles/roles/end_role_inside/handlers/main.yml

@@ -0,0 +1,3 @@
+- name: role_handler
+  set_fact:
+    role_handler_ran: true

test/integration/targets/roles/roles/end_role_inside/tasks/main.yml

@@ -0,0 +1,10 @@
+- set_fact:
+    role_executed: "{{ role_executed|default(0)|int + 1 }}"
+
+- command: echo
+  notify: role_handler
+
+- meta: end_role
+
+- set_fact:
+    after_end_role: true

test/integration/targets/roles/roles/end_role_inside/tasks/nested.yml

@@ -0,0 +1,22 @@
+- set_fact:
+    end_role_cond: "{{ inventory_hostname == 'host1' }}"
+
+- include_role:
+    name: end_role_inside_nested
+
+- debug:
+    msg: CHECKPOINT
+
+- assert:
+    that:
+      - after_end_role is undefined
+  when: inventory_hostname == "host1"
+
+- assert:
+    that:
+      - after_end_role
+  when: inventory_hostname == "host2"
+
+- name: when running this playbook check this appears on stdout to ensure the above assert wasn't skipped
+  debug:
+    msg: CHECKPOINT

test/integration/targets/roles/roles/end_role_inside_nested/tasks/import_tasks.yml

@@ -0,0 +1,5 @@
+- meta: end_role
+  when: end_role_cond
+
+- set_fact:
+    after_end_role: true

test/integration/targets/roles/roles/end_role_inside_nested/tasks/include_tasks.yml

@@ -0,0 +1 @@
+- import_tasks: import_tasks.yml

test/integration/targets/roles/roles/end_role_inside_nested/tasks/main.yml

@@ -0,0 +1 @@
+- include_tasks: include_tasks.yml

test/integration/targets/roles/runme.sh

@@ -53,3 +53,11 @@ ansible-playbook role_dep_chain.yml -i ../../inventory "$@"
 ANSIBLE_PRIVATE_ROLE_VARS=1 ansible-playbook privacy.yml -e @vars/privacy_vars.yml "$@"
 ANSIBLE_PRIVATE_ROLE_VARS=0 ansible-playbook privacy.yml -e @vars/privacy_vars.yml "$@"
 ansible-playbook privacy.yml -e @vars/privacy_vars.yml "$@"
+
+for strategy in linear free; do
+  [ "$(ANSIBLE_STRATEGY=$strategy ansible-playbook end_role.yml | grep -c CHECKPOINT)" = "1" ]
+  [ "$(ANSIBLE_STRATEGY=$strategy ansible-playbook -i host1,host2 end_role_nested.yml | grep -c CHECKPOINT)" = "4" ]
+done
+
+[ "$(ansible localhost -m meta -a end_role 2>&1 | grep -c "ERROR! Cannot execute 'end_role' from outside of a role")" = "1" ]
+[ "$(ansible-playbook end_role_handler_error.yml 2>&1 | grep -c "ERROR! Cannot execute 'end_role' from a handler")" = "1" ]