diff options
author | Alex Schultz <aschultz@next-development.com> | 2020-06-04 17:41:46 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-06-04 17:41:46 +0200 |
commit | 247e43b25244720f81e41febf0993f4b435fcb3d (patch) | |
tree | ecbabbcae90422d092e57c2c2f485bb672b978c2 /test | |
parent | __eq__ should be redefined if __hash__ is defined. (diff) | |
download | ansible-247e43b25244720f81e41febf0993f4b435fcb3d.tar.xz ansible-247e43b25244720f81e41febf0993f4b435fcb3d.zip |
Fix IncludedFile equality check (#69524)
In the case of a free style strategy, it is possible to end up with
multiple hosts trying to include from the same role, however the tasks
being included may be different with the use of tasks_from. Previously
if you had two hosts that were included the same role when the
process_include_results function tries to determine if a included needs
to be run on a specific host, it would end up merging two different
tasks into which ever one was processed first.
This change updates the equality check to also check if the task uuid
associated with the IncludedFile is the same. The previous check only
checked if the task's parent uuid was the same. This breaks down when
both includes have the same parent.
- hosts: all
strategy: free
gather_facts: false
tasks:
- include_role:
name: random_sleep
- block:
- name: set a fact (1)
include_role:
name: set_a_fact
tasks_from: fact1.yml
- name: set a fact (2)
include_role:
name: set_a_fact
tasks_from: fact2.yml
- name: include didn't run
fail:
msg: >
set_a_fact didn't run
fact1: {{ fact1 | default('not defined')}}
fact2: {{ fact2 | default('not defined') }}"
when: (fact1 is not defined or fact2 is not defined)
Closes #69521
Diffstat (limited to 'test')
8 files changed, 213 insertions, 0 deletions
diff --git a/test/integration/targets/includes_race/aliases b/test/integration/targets/includes_race/aliases new file mode 100644 index 0000000000..fff62d9f20 --- /dev/null +++ b/test/integration/targets/includes_race/aliases @@ -0,0 +1,2 @@ +shippable/posix/group5 +skip/aix diff --git a/test/integration/targets/includes_race/inventory b/test/integration/targets/includes_race/inventory new file mode 100644 index 0000000000..878792949f --- /dev/null +++ b/test/integration/targets/includes_race/inventory @@ -0,0 +1,30 @@ +host001 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host002 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host003 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host004 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host005 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host006 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host007 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host008 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host009 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host010 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host011 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host012 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host013 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host014 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host015 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host016 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host017 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host018 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host019 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host020 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host021 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host022 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host023 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host024 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host025 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host026 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host027 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host028 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host029 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" +host030 ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" diff --git a/test/integration/targets/includes_race/roles/random_sleep/tasks/main.yml b/test/integration/targets/includes_race/roles/random_sleep/tasks/main.yml new file mode 100644 index 0000000000..cee459a248 --- /dev/null +++ b/test/integration/targets/includes_race/roles/random_sleep/tasks/main.yml @@ -0,0 +1,8 @@ +--- +# tasks file for random_sleep +- name: Generate sleep time + set_fact: + sleep_time: "{{ 3 | random }}" + +- name: Do random sleep + shell: sleep "{{ sleep_time }}" diff --git a/test/integration/targets/includes_race/roles/set_a_fact/tasks/fact1.yml b/test/integration/targets/includes_race/roles/set_a_fact/tasks/fact1.yml new file mode 100644 index 0000000000..36b08dcb78 --- /dev/null +++ b/test/integration/targets/includes_race/roles/set_a_fact/tasks/fact1.yml @@ -0,0 +1,4 @@ +--- +- name: Set fact1 + set_fact: + fact1: yay diff --git a/test/integration/targets/includes_race/roles/set_a_fact/tasks/fact2.yml b/test/integration/targets/includes_race/roles/set_a_fact/tasks/fact2.yml new file mode 100644 index 0000000000..865f130db1 --- /dev/null +++ b/test/integration/targets/includes_race/roles/set_a_fact/tasks/fact2.yml @@ -0,0 +1,4 @@ +--- +- name: Set fact2 + set_fact: + fact2: yay diff --git a/test/integration/targets/includes_race/runme.sh b/test/integration/targets/includes_race/runme.sh new file mode 100755 index 0000000000..2261d2718a --- /dev/null +++ b/test/integration/targets/includes_race/runme.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash + +set -eux + +ansible-playbook test_includes_race.yml -i inventory -v "$@" diff --git a/test/integration/targets/includes_race/test_includes_race.yml b/test/integration/targets/includes_race/test_includes_race.yml new file mode 100644 index 0000000000..20f7dddd8d --- /dev/null +++ b/test/integration/targets/includes_race/test_includes_race.yml @@ -0,0 +1,19 @@ +- hosts: all + strategy: free + gather_facts: false + tasks: + - include_role: + name: random_sleep + - block: + - name: set a fact (1) + include_role: + name: set_a_fact + tasks_from: fact1.yml + - name: set a fact (2) + include_role: + name: set_a_fact + tasks_from: fact2.yml + - name: include didn't run + fail: + msg: "set_a_fact didn't run fact1 {{ fact1 | default('not defined')}} fact2: {{ fact2 | default('not defined') }}" + when: (fact1 is not defined or fact2 is not defined) diff --git a/test/units/playbook/test_included_file.py b/test/units/playbook/test_included_file.py index 511f3169e6..f143acb93a 100644 --- a/test/units/playbook/test_included_file.py +++ b/test/units/playbook/test_included_file.py @@ -26,8 +26,10 @@ import pytest from units.compat.mock import MagicMock from units.mock.loader import DictDataLoader +from ansible.playbook.block import Block from ansible.playbook.task import Task from ansible.playbook.task_include import TaskInclude +from ansible.playbook.role_include import IncludeRole from ansible.executor import task_result from ansible.playbook.included_file import IncludedFile @@ -49,6 +51,48 @@ def mock_variable_manager(): return mock_variable_manager +def test_equals_ok(): + uuid = '111-111' + parent = MagicMock(name='MockParent') + parent._uuid = uuid + task = MagicMock(name='MockTask') + task._uuid = uuid + task._parent = parent + inc_a = IncludedFile('a.yml', {}, {}, task) + inc_b = IncludedFile('a.yml', {}, {}, task) + assert inc_a == inc_b + + +def test_equals_different_tasks(): + parent = MagicMock(name='MockParent') + parent._uuid = '111-111' + task_a = MagicMock(name='MockTask') + task_a._uuid = '11-11' + task_a._parent = parent + task_b = MagicMock(name='MockTask') + task_b._uuid = '22-22' + task_b._parent = parent + inc_a = IncludedFile('a.yml', {}, {}, task_a) + inc_b = IncludedFile('a.yml', {}, {}, task_b) + assert inc_a != inc_b + + +def test_equals_different_parents(): + parent_a = MagicMock(name='MockParent') + parent_a._uuid = '111-111' + parent_b = MagicMock(name='MockParent') + parent_b._uuid = '222-222' + task_a = MagicMock(name='MockTask') + task_a._uuid = '11-11' + task_a._parent = parent_a + task_b = MagicMock(name='MockTask') + task_b._uuid = '11-11' + task_b._parent = parent_b + inc_a = IncludedFile('a.yml', {}, {}, task_a) + inc_b = IncludedFile('a.yml', {}, {}, task_b) + assert inc_a != inc_b + + def test_included_file_instantiation(): filename = 'somefile.yml' @@ -170,6 +214,103 @@ def test_process_include_simulate_free(mock_iterator, mock_variable_manager): assert res[1]._vars == {} +def test_process_include_simulate_free_block_role_tasks(mock_iterator, + mock_variable_manager): + """Test loading the same role returns different included files + + In the case of free, we may end up with included files from roles that + have the same parent but are different tasks. Previously the comparison + for equality did not check if the tasks were the same and only checked + that the parents were the same. This lead to some tasks being run + incorrectly and some tasks being silient dropped.""" + + fake_loader = DictDataLoader({ + 'include_test.yml': "", + '/etc/ansible/roles/foo_role/tasks/task1.yml': """ + - debug: msg=task1 + """, + '/etc/ansible/roles/foo_role/tasks/task2.yml': """ + - debug: msg=task2 + """, + }) + + hostname = "testhost1" + hostname2 = "testhost2" + + role1_ds = { + 'name': 'task1 include', + 'include_role': { + 'name': 'foo_role', + 'tasks_from': 'task1.yml' + } + } + role2_ds = { + 'name': 'task2 include', + 'include_role': { + 'name': 'foo_role', + 'tasks_from': 'task2.yml' + } + } + parent_task_ds = { + 'block': [ + role1_ds, + role2_ds + ] + } + parent_block = Block.load(parent_task_ds, loader=fake_loader) + + parent_block._play = None + + include_role1_ds = { + 'include_args': { + 'name': 'foo_role', + 'tasks_from': 'task1.yml' + } + } + include_role2_ds = { + 'include_args': { + 'name': 'foo_role', + 'tasks_from': 'task2.yml' + } + } + + include_role1 = IncludeRole.load(role1_ds, + block=parent_block, + loader=fake_loader) + include_role2 = IncludeRole.load(role2_ds, + block=parent_block, + loader=fake_loader) + + result1 = task_result.TaskResult(host=hostname, + task=include_role1, + return_data=include_role1_ds) + result2 = task_result.TaskResult(host=hostname2, + task=include_role2, + return_data=include_role2_ds) + results = [result1, result2] + + res = IncludedFile.process_include_results(results, + mock_iterator, + fake_loader, + mock_variable_manager) + assert isinstance(res, list) + # we should get two different includes + assert len(res) == 2 + assert res[0]._filename == 'foo_role' + assert res[1]._filename == 'foo_role' + # with different tasks + assert res[0]._task != res[1]._task + + assert res[0]._hosts == ['testhost1'] + assert res[1]._hosts == ['testhost2'] + + assert res[0]._args == {} + assert res[1]._args == {} + + assert res[0]._vars == {} + assert res[1]._vars == {} + + def test_empty_raw_params(): parent_task_ds = {'debug': 'msg=foo'} parent_task = Task.load(parent_task_ds) |