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

changelogs/fragments/69521-free-strategy-include-fix.yml

@@ -0,0 +1,2 @@
+bugfixes:
+- Fixed the equality check for IncludedFiles to ensure they are not accidently merged when process_include_results runs.

lib/ansible/playbook/included_file.py

@@ -51,6 +51,7 @@ class IncludedFile:
         return (other._filename == self._filename and
                 other._args == self._args and
                 other._vars == self._vars and
+                other._task._uuid == self._task._uuid and
                 other._task._parent._uuid == self._task._parent._uuid)
 
     def __repr__(self):

test/integration/targets/includes_race/aliases

@@ -0,0 +1,2 @@
+shippable/posix/group5
+skip/aix

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 }}"

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 }}"

test/integration/targets/includes_race/roles/set_a_fact/tasks/fact1.yml

@@ -0,0 +1,4 @@
+---
+- name: Set fact1
+  set_fact:
+    fact1: yay

test/integration/targets/includes_race/roles/set_a_fact/tasks/fact2.yml

@@ -0,0 +1,4 @@
+---
+- name: Set fact2
+  set_fact:
+    fact2: yay

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 "$@"

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)

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)