Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Validate task attributes with first finalized attrs after loop #80476

Merged
merged 5 commits into from Apr 13, 2023
+15 −5
Merged

Validate task attributes with first finalized attrs after loop #80476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
48cca24
Validate task attributes with first finalized attrs after individual …
s-hertel Apr 10, 2023
f74f3bd
changelog
s-hertel Apr 11, 2023
342355c
use last loop info?
s-hertel Apr 11, 2023
fbdd58f
Fix ignore_unreachable, which suffered from the same issue as run_onc…
s-hertel Apr 12, 2023
1ee5284
fix action too
s-hertel Apr 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/80476-fix-loop-task-post-validation.yml
View file
Open in desktop
@@ -0,0 +1,2 @@
bugfixes:
- Fix post-validating looped task fields so the strategy uses the correct values after task execution.
12 changes: 12 additions & 0 deletions lib/ansible/executor/task_executor.py
View file
Open in desktop
Expand Up @@ -137,6 +137,12 @@ def run(self):
self._task.ignore_errors = item_ignore
elif self._task.ignore_errors and not item_ignore:
self._task.ignore_errors = item_ignore
if 'unreachable' in item and item['unreachable']:
item_ignore_unreachable = item.pop('_ansible_ignore_unreachable')
if not res.get('unreachable'):
self._task.ignore_unreachable = item_ignore_unreachable
elif self._task.ignore_unreachable and not item_ignore_unreachable:
self._task.ignore_unreachable = item_ignore_unreachable

# ensure to accumulate these
for array in ['warnings', 'deprecations']:
Expand Down Expand Up @@ -277,6 +283,7 @@ def _run_loop(self, items):
u" to something else to avoid variable collisions and unexpected behavior." % (self._task, loop_var))

ran_once = False
task_fields = None
no_log = False
items_len = len(items)
results = []
Expand Down Expand Up @@ -348,6 +355,7 @@ def _run_loop(self, items):

res['_ansible_item_result'] = True
res['_ansible_ignore_errors'] = task_fields.get('ignore_errors')
res['_ansible_ignore_unreachable'] = task_fields.get('ignore_unreachable')

# gets templated here unlike rest of loop_control fields, depends on loop_var above
try:
Expand Down Expand Up @@ -392,6 +400,10 @@ def _run_loop(self, items):
del task_vars[var]

self._task.no_log = no_log
# NOTE: run_once cannot contain loop vars because it's templated earlier also
# This is saving the post-validated field from the last loop so the strategy can use the templated value post task execution
self._task.run_once = task_fields.get('run_once')
Copy link
Contributor Author

@s-hertel s-hertel Apr 12, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was worried self._task.from_attrs(task_fields) might cause issues, so I tried to identify all the FieldAttributes potentially affected by this issue. I believe it's limited to the fields action, run_once, ignore_unreachable, ignore_errors, loop, and no_log since these may be used after the looped task is executed.

  • no_log is fine, because it's set just above this change
  • loop is fine because empty loops == skipped task and the codepath with the issue (template string is always truthy) doesn't occur
  • ignore_errors is fine because the value for the Task is set to False if any of the individual items are False

So I fixed ignore_unreachable using the same pattern as ignore_errors, and limited this to just set run_once and action.

self._task.action = task_fields.get('action')

return results

Expand Down
6 changes: 1 addition & 5 deletions lib/ansible/plugins/strategy/linear.py
View file
Open in desktop
Expand Up @@ -35,7 +35,6 @@
from ansible.errors import AnsibleError, AnsibleAssertionError, AnsibleParserError
from ansible.executor.play_iterator import IteratingStates, FailedStates
from ansible.module_utils._text import to_text
from ansible.module_utils.parsing.convert_bool import boolean
from ansible.playbook.handler import Handler
from ansible.playbook.included_file import IncludedFile
from ansible.playbook.task import Task
Expand Down Expand Up @@ -214,10 +213,7 @@ def run(self, iterator, play_context):
skip_rest = True
break

if templar.is_template(task.run_once):
setattr(task, 'run_once', boolean(templar.template(task.run_once), strict=True))

run_once = task.run_once or action and getattr(action, 'BYPASS_HOST_LOOP', False)
run_once = templar.template(task.run_once) or action and getattr(action, 'BYPASS_HOST_LOOP', False)

if (task.any_errors_fatal or run_once) and not task.ignore_errors:
any_errors_fatal = True
Expand Down