From 76867730bfb84fe39cd80b4e2c2f272409de8c9f Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Fri, 8 Jun 2018 15:36:22 -0500 Subject: [PATCH] Don't ignore a duplicate host for an already processed include (#40361) * Don't ignore a duplicate host for an already processed include, assume that the repetition indicates a new include. Fixes #40317 * Add intg tests to ensure duplicate items in loop are not deduped * Add note about relative indexing --- lib/ansible/playbook/included_file.py | 26 ++++++++++++++----- .../targets/include_import/runme.sh | 6 +++++ .../include_import/tasks/debug_item.yml | 2 ++ .../tasks/test_include_dupe_loop.yml | 8 ++++++ 4 files changed, 36 insertions(+), 6 deletions(-) create mode 100644 test/integration/targets/include_import/tasks/debug_item.yml create mode 100644 test/integration/targets/include_import/tasks/test_include_dupe_loop.yml diff --git a/lib/ansible/playbook/included_file.py b/lib/ansible/playbook/included_file.py index db4f89eae3d..a6c9f76283a 100644 --- a/lib/ansible/playbook/included_file.py +++ b/lib/ansible/playbook/included_file.py @@ -44,6 +44,8 @@ class IncludedFile: def add_host(self, host): if host not in self._hosts: self._hosts.append(host) + return + raise ValueError() def __eq__(self, other): return other._filename == self._filename and other._args == self._args and other._task._parent._uuid == self._task._parent._uuid @@ -160,12 +162,24 @@ class IncludedFile: inc_file = IncludedFile(role_name, include_variables, new_task, is_role=True) - try: - pos = included_files.index(inc_file) - inc_file = included_files[pos] - except ValueError: - included_files.append(inc_file) + idx = 0 + orig_inc_file = inc_file + while 1: + try: + pos = included_files[idx:].index(orig_inc_file) + # pos is relative to idx since we are slicing + # use idx + pos due to relative indexing + inc_file = included_files[idx + pos] + except ValueError: + included_files.append(orig_inc_file) + inc_file = orig_inc_file - inc_file.add_host(original_host) + try: + inc_file.add_host(original_host) + except ValueError: + # The host already exists for this include, advance forward, this is a new include + idx += pos + 1 + else: + break return included_files diff --git a/test/integration/targets/include_import/runme.sh b/test/integration/targets/include_import/runme.sh index af1abb15634..8f0550e2b3c 100755 --- a/test/integration/targets/include_import/runme.sh +++ b/test/integration/targets/include_import/runme.sh @@ -73,3 +73,9 @@ if [[ -z "$OUT" ]]; then echo "apply on import_tasks did not cause error" exit 1 fi + +# Test that duplicate items in loop are not deduped +ANSIBLE_STRATEGY='linear' ansible-playbook tasks/test_include_dupe_loop.yml -i ../../inventory "$@" | tee test_include_dupe_loop.out +test "$(grep -c 'item=foo' test_include_dupe_loop.out)" = 3 +ANSIBLE_STRATEGY='free' ansible-playbook tasks/test_include_dupe_loop.yml -i ../../inventory "$@" | tee test_include_dupe_loop.out +test "$(grep -c 'item=foo' test_include_dupe_loop.out)" = 3 \ No newline at end of file diff --git a/test/integration/targets/include_import/tasks/debug_item.yml b/test/integration/targets/include_import/tasks/debug_item.yml new file mode 100644 index 00000000000..025e132daa4 --- /dev/null +++ b/test/integration/targets/include_import/tasks/debug_item.yml @@ -0,0 +1,2 @@ +- debug: + msg: "item={{ item }}" diff --git a/test/integration/targets/include_import/tasks/test_include_dupe_loop.yml b/test/integration/targets/include_import/tasks/test_include_dupe_loop.yml new file mode 100644 index 00000000000..b7b9301de7a --- /dev/null +++ b/test/integration/targets/include_import/tasks/test_include_dupe_loop.yml @@ -0,0 +1,8 @@ +- name: Test Include Duplicate Loop Items + hosts: testhost + tasks: + - include_tasks: debug_item.yml + loop: + - foo + - foo + - foo