From bba9f4d02d7e34ef2f7ceed03714aee83b61af3e Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Wed, 28 Aug 2024 21:16:35 +0200 Subject: [PATCH] Ensure skipped loop iteration register var is available (#83756) (#83789) Fixes #83619 (cherry picked from commit 9a54ba5a3910ec37590a1019078105a025a3a383) --- .../fragments/83619-loop-label-register.yml | 2 ++ lib/ansible/executor/task_executor.py | 12 ++++++++---- lib/ansible/playbook/task.py | 5 +++++ lib/ansible/plugins/strategy/__init__.py | 6 +----- test/integration/targets/loops/tasks/main.yml | 17 +++++++++++++++++ 5 files changed, 33 insertions(+), 9 deletions(-) create mode 100644 changelogs/fragments/83619-loop-label-register.yml diff --git a/changelogs/fragments/83619-loop-label-register.yml b/changelogs/fragments/83619-loop-label-register.yml new file mode 100644 index 00000000000..aab82f0dff9 --- /dev/null +++ b/changelogs/fragments/83619-loop-label-register.yml @@ -0,0 +1,2 @@ +bugfixes: + - Fix an issue where registered variable was not available for templating in ``loop_control.label`` on skipped looped tasks (https://github.com/ansible/ansible/issues/83619) diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index d20635adf22..3f2c84e9e68 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -32,7 +32,7 @@ from ansible.utils.listify import listify_lookup_plugin_terms from ansible.utils.unsafe_proxy import to_unsafe_text, wrap_var from ansible.vars.clean import namespace_facts, clean_facts from ansible.utils.display import Display -from ansible.utils.vars import combine_vars, isidentifier +from ansible.utils.vars import combine_vars display = Display() @@ -333,6 +333,13 @@ class TaskExecutor: (self._task, tmp_task) = (tmp_task, self._task) (self._play_context, tmp_play_context) = (tmp_play_context, self._play_context) res = self._execute(variables=task_vars) + + if self._task.register: + # Ensure per loop iteration results are registered in case `_execute()` + # returns early (when conditional, failure, ...). + # This is needed in case the registered variable is used in the loop label template. + task_vars[self._task.register] = res + task_fields = self._task.dump_attrs() (self._task, tmp_task) = (tmp_task, self._task) (self._play_context, tmp_play_context) = (tmp_play_context, self._play_context) @@ -658,9 +665,6 @@ class TaskExecutor: # update the local copy of vars with the registered value, if specified, # or any facts which may have been generated by the module execution if self._task.register: - if not isidentifier(self._task.register): - raise AnsibleError("Invalid variable name in 'register' specified: '%s'" % self._task.register) - vars_copy[self._task.register] = result if self._task.async_val > 0: diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py index ac87540df7f..3ee2c3ebc4f 100644 --- a/lib/ansible/playbook/task.py +++ b/lib/ansible/playbook/task.py @@ -39,6 +39,7 @@ from ansible.playbook.taggable import Taggable from ansible.utils.collection_loader import AnsibleCollectionConfig from ansible.utils.display import Display from ansible.utils.sentinel import Sentinel +from ansible.utils.vars import isidentifier __all__ = ['Task'] @@ -276,6 +277,10 @@ class Task(Base, Conditional, Taggable, CollectionSearch, Notifiable, Delegatabl if not isinstance(value, list): setattr(self, name, [value]) + def _validate_register(self, attr, name, value): + if value is not None and not isidentifier(value): + raise AnsibleParserError(f"Invalid variable name in 'register' specified: '{value}'") + def post_validate(self, templar): ''' Override of base class post_validate, to also do final validation on diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py index eb2f76d7a98..2a4ff6f3ee4 100644 --- a/lib/ansible/plugins/strategy/__init__.py +++ b/lib/ansible/plugins/strategy/__init__.py @@ -56,7 +56,7 @@ from ansible.utils.display import Display from ansible.utils.fqcn import add_internal_fqcns from ansible.utils.unsafe_proxy import wrap_var from ansible.utils.sentinel import Sentinel -from ansible.utils.vars import combine_vars, isidentifier +from ansible.utils.vars import combine_vars from ansible.vars.clean import strip_internal_keys, module_response_deepcopy display = Display() @@ -775,10 +775,6 @@ class StrategyBase: # register final results if original_task.register: - - if not isidentifier(original_task.register): - raise AnsibleError("Invalid variable name in 'register' specified: '%s'" % original_task.register) - host_list = self.get_task_hosts(iterator, original_host, original_task) clean_copy = strip_internal_keys(module_response_deepcopy(task_result._result)) diff --git a/test/integration/targets/loops/tasks/main.yml b/test/integration/targets/loops/tasks/main.yml index 03c7c440d84..1f1888f8f92 100644 --- a/test/integration/targets/loops/tasks/main.yml +++ b/test/integration/targets/loops/tasks/main.yml @@ -405,3 +405,20 @@ - assert: that: - foo[0] == 'foo1.0' + +# https://github.com/ansible/ansible/issues/83619 +- command: "echo {{ item }}" + register: r + loop: + - changed + - skipped + loop_control: + label: "{{ r.stdout }}" + when: + - item == "changed" + ignore_errors: true + +- name: test that the second iteration result was stored, since it was skipped no stdout should be present there + assert: + that: + - "r['results'][1]['msg'] is contains(\"no attribute 'stdout'\")"