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.

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

Using set_fact with delegate_to, delegate_facts and with_items only sets the fact on the first host #20508

Closed
alexjrussell opened this issue Jan 20, 2017 · 11 comments
Labels
affects_2.2 This issue/PR affects Ansible v2.2 bug This issue/PR relates to a bug. c:plugins/strategy

Comments

@alexjrussell
Copy link

ISSUE TYPE
  • Bug Report
COMPONENT NAME

set_fact

ANSIBLE VERSION
ansible 2.2.1.0
  config file = 
  configured module search path = Default w/o overrides
CONFIGURATION
OS / ENVIRONMENT

Red Hat Enterprise Linux Client release 7.3 (Maipo)

SUMMARY

In Ansible 2.2.1.0, if you use set_fact, delegate_to and delegate_facts to set facts on several hosts listed in with_items, only the first host in the list will have the fact set. In all other versions of Ansible 2.x, the fact will be set on all hosts.

STEPS TO REPRODUCE

Create a playbook like this:

---

- hosts: localhost
  tasks:
  - name: "Set fact on localhost"
    set_fact:
      my_test_fact: "Hello"

  - name: "Copy fact to other servers"
    set_fact:
      my_test_fact: "{{ my_test_fact }}"
    delegate_to: "{{ item }}"
    delegate_facts: True
    with_items: "{{ groups['all'] }}"
    when: item != "localhost"

- hosts: server1
  gather_facts: True
  tasks:
  - name: "Show fact on server1"
    debug: var=my_test_fact

- hosts: server2
  gather_facts: True
  tasks:
  - name: "Show fact on server2"
    debug: var=my_test_fact

I am using a simple inventory file:

[server1]
192.168.1.105 ansible_user=admin

[server2]
192.168.1.106 ansible_user=admin
EXPECTED RESULTS

If I run the playbook with Ansible 2.2.0.0 I get these results:

PLAY [localhost] ***************************************************************

TASK [setup] *******************************************************************
ok: [localhost]

TASK [Set fact on localhost] ***************************************************
ok: [localhost]

TASK [Copy fact to other servers] **********************************************
ok: [localhost -> 192.168.1.105] => (item=192.168.1.105)
ok: [localhost -> 192.168.1.106] => (item=192.168.1.106)

PLAY [server1] *****************************************************************

TASK [setup] *******************************************************************
ok: [192.168.1.105]

TASK [Show fact on server1] ****************************************************
ok: [192.168.1.105] => {
    "my_test_fact": "Hello"
}

PLAY [server2] *****************************************************************

TASK [setup] *******************************************************************
ok: [192.168.1.106]

TASK [Show fact on server2] ****************************************************
ok: [192.168.1.106] => {
    "my_test_fact": "Hello"
}

PLAY RECAP *********************************************************************
192.168.1.105              : ok=2    changed=0    unreachable=0    failed=0   
192.168.1.106              : ok=2    changed=0    unreachable=0    failed=0   
localhost                  : ok=3    changed=0    unreachable=0    failed=0   
ACTUAL RESULTS

If I run with Ansible 2.2.1.0, the fact is only set for the first host:


PLAY [localhost] ***************************************************************

TASK [setup] *******************************************************************
ok: [localhost]

TASK [Set fact on localhost] ***************************************************
ok: [localhost]

TASK [Copy fact to other servers] **********************************************
ok: [localhost -> 192.168.1.105] => (item=192.168.1.105)
ok: [localhost -> 192.168.1.106] => (item=192.168.1.106)

PLAY [server1] *****************************************************************

TASK [setup] *******************************************************************
ok: [192.168.1.105]

TASK [Show fact on server1] ****************************************************
ok: [192.168.1.105] => {
    "my_test_fact": "Hello"
}

PLAY [server2] *****************************************************************

TASK [setup] *******************************************************************
ok: [192.168.1.106]

TASK [Show fact on server2] ****************************************************
ok: [192.168.1.106] => {
    "my_test_fact": "VARIABLE IS NOT DEFINED!"
}

PLAY RECAP *********************************************************************
192.168.1.105              : ok=2    changed=0    unreachable=0    failed=0   
192.168.1.106              : ok=2    changed=0    unreachable=0    failed=0   
localhost                  : ok=3    changed=0    unreachable=0    failed=0   

The bug seems to be related to a change to plugins/strategy/init.py here: 2d8ebbf

@ansibot
Copy link
Contributor

ansibot commented Jan 20, 2017

@ansibot ansibot added affects_2.2 This issue/PR affects Ansible v2.2 bug_report module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. labels Jan 20, 2017
@jctanner jctanner added c:plugins/strategy and removed needs_triage Needs a first human triage before being processed. module This issue/PR relates to a module. labels Jan 20, 2017
@hobgoblinsmaster
Copy link

In fact, delegated_facts seams not to work at all with version 2.2.1.0 (at list the way descrubed in Delegation, Rolling Updates, and Local Actions.

ansible 2.2.1.0
  config file = /tmp/ansible/ansible.cfg
  configured module search path = Default w/o overrides

Config file:

[defaults]
gathering = smart
# le cache nécessite la version 1.8 minimum
fact_caching = jsonfile
fact_caching_connection = facts_cache
fact_caching_timeout = 86400

nocows = 1
retry_files_enabled = False
remote_user = root

playbook:

---
- hosts: all
  tags: ['always']
  gather_facts: no
  vars:
    debug: false
  tasks:
    - name: gather facts on all hosts
      setup: gather_subset=!all
      delegate_to: "{{ item }}"
      delegate_facts: true
      with_items: "{{ groups['all'] }}"
      when: inventory_hostname == ansible_play_hosts[0]

    - debug: var=hostvars[item].ansible_fqdn
      with_items: "{{ groups['all'] }}"

Result with version 2.2.0.0:

PLAY [all] *********************************************************************

TASK [gather facts on all hosts] ***********************************************
ok: [dedi2.hobgoblins-master.info -> dedi2.hobgoblins-master.info] => (item=dedi2.hobgoblins-master.info)
ok: [dedi2.hobgoblins-master.info -> guenievre.minoza.info] => (item=guenievre.minoza.info)

TASK [debug] *******************************************************************
ok: [dedi2.hobgoblins-master.info] => (item=dedi2.hobgoblins-master.info) => {
    "hostvars[item].ansible_fqdn": "dedi2.hobgoblins-master.info",
    "item": "dedi2.hobgoblins-master.info"
}
ok: [dedi2.hobgoblins-master.info] => (item=guenievre.minoza.info) => {
    "hostvars[item].ansible_fqdn": "guenievre.minoza.info",
    "item": "guenievre.minoza.info"
}

PLAY RECAP *********************************************************************
dedi2.hobgoblins-master.info : ok=2    changed=0    unreachable=0    failed=0

Result with 2.2.1.0:

PLAY [all] *********************************************************************

TASK [gather facts on all hosts] ***********************************************
ok: [dedi2.hobgoblins-master.info -> dedi2.hobgoblins-master.info] => (item=dedi2.hobgoblins-master.info)
ok: [dedi2.hobgoblins-master.info -> guenievre.minoza.info] => (item=guenievre.minoza.info)

TASK [debug] *******************************************************************
ok: [dedi2.hobgoblins-master.info] => (item=dedi2.hobgoblins-master.info) => {
    "hostvars[item].ansible_fqdn": "guenievre.minoza.info",
    "item": "dedi2.hobgoblins-master.info"
}
ok: [dedi2.hobgoblins-master.info] => (item=guenievre.minoza.info) => {
    "hostvars[item].ansible_fqdn": "VARIABLE IS NOT DEFINED!",
    "item": "guenievre.minoza.info"
}

PLAY RECAP *********************************************************************
dedi2.hobgoblins-master.info : ok=2    changed=0    unreachable=0    failed=0

@xNaaro
Copy link

xNaaro commented Feb 6, 2017

I can confirm the issue:

- name: Gather facts for all hosts (if using --limit)
  hosts: all
  serial: '{{ serial|default("0") }}'
  gather_facts: false
  tasks:
    - setup:
      delegate_to: "{{item}}"
      delegate_facts: true
      with_items:
        - "{{ groups.all }}"
- name: Debug delegated facts
  hosts: all
  tasks:
    - name: Debug facts
      debug:
         msg: "{{ hostvars['192.168.100.215']['ansible_' + hostvars['192.168.100.215']['api_interface']]['ipv4']['address'] }}"

Expected result:

TASK [Debug facts] *************************************************************
ok: [192.168.100.170] => {
    "msg": "192.168.100.215"

Actual result:

TASK [Debug facts] *************************************************************
ok: [192.168.100.170] => {
    "msg": "192.168.100.170"
}

With Ansible 2.1 works as expected

@samdoran
Copy link
Contributor

This is still broken in 2.2.0-0.1.rc1 but works properly in 2.1.5.0-0.1.rc1.

@dagwieers
Copy link
Contributor

The problem is not really related to the set_fact module itself. Maybe best to involve the person that was responsible of the change ? @jimi-c

PS And we should definitely add these (failing) examples to the integration tests too !

chrulri referenced this issue Feb 24, 2017
Since we no longer use a post-validated task in _process_pending_results, we
need to be sure to template fields used in original_task as they are raw and
may contain variables.

This patch also moves the handler tracking to be per-uuid, not per-object.
Doing it per-object had implications for the above due to the fact that the
copy of the original task is now being used, so the only sure way is to track
based on the uuid instead.

Fixes #18289

(cherry picked from commit dd0257b)
@chrulri
Copy link

chrulri commented Feb 24, 2017

I added a comment to the responsible changes in the commit ( 2d8ebbf ) that caused this issue.

Ansible version: Ansible from source, commit ref: 2d8ebbf
Tested with this simplified play:

- hosts: controller
  tasks:
    - name: "gather data from nodes"
      setup:
      delegate_to: "{{ item }}"
      delegate_facts: true
      with_items: "{{ groups.nodes }}"
    - name: "print ipv4s"
      debug: var=hostvars[item].ansible_all_ipv4_addresses
      with_items: "{{ groups.nodes }}"

And using this simplified inventory:

controller ansible_connection=local
node1      ansible_ssh_host=172.17.2.11
node2      ansible_ssh_host=172.17.2.12

[nodes]
node1
node2

Result with commit 2d8ebbf:

PLAY [controller] **************************************************************

TASK [setup] *******************************************************************
ok: [controller]

TASK [gather data from nodes] **************************************************
ok: [controller -> None] => (item=node1)
ok: [controller -> None] => (item=node2)

TASK [print ipv4s] *************************************************************
ok: [controller] => (item=node1) => {
    "hostvars[item].ansible_all_ipv4_addresses": [
        "172.17.2.12"
    ],
    "item": "node1"
}
ok: [controller] => (item=node2) => {
    "hostvars[item].ansible_all_ipv4_addresses": "VARIABLE IS NOT DEFINED!",
    "item": "node2"
}

Result with same commit, but reverted the three lines:

PLAY [controller] **************************************************************

TASK [setup] *******************************************************************
ok: [controller]

TASK [gather data from nodes] **************************************************
ok: [controller -> None] => (item=node1)
ok: [controller -> None] => (item=node2)

TASK [print ipv4s] *************************************************************
ok: [controller] => (item=node1) => {
    "hostvars[item].ansible_all_ipv4_addresses": [
        "172.17.2.11"
    ],
    "item": "node1"
}
ok: [controller] => (item=node2) => {
    "hostvars[item].ansible_all_ipv4_addresses": [
        "172.17.2.12"
    ],
    "item": "node2"
}

The reverted changes / lines:

diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py
index 1d4fee5..7f7a3e6 100644
--- a/lib/ansible/plugins/strategy/__init__.py
+++ b/lib/ansible/plugins/strategy/__init__.py
@@ -483,9 +483,12 @@ class StrategyBase:

                         # if delegated fact and we are delegating facts, we need to change target host for them
                         if original_task.delegate_to is not None and original_task.delegate_facts:
+                            task_vars = self._variable_manager.get_vars(loader=self._loader, play=iterator._play, host=original_host, task=original_task)
+                            self.add_tqm_variables(task_vars, play=iterator._play)
                             item = result_item.get(loop_var, None)
                             if item is not None:
                                 task_vars[loop_var] = item
+                            templar = Templar(loader=self._loader, variables=task_vars)
                             host_name = templar.template(original_task.delegate_to)
                             actual_host = self._inventory.get_host(host_name)
                             if actual_host is None:

@alikins
Copy link
Contributor

alikins commented Feb 27, 2017

Likely related:
#18178
#15365

related prs:

#22003
#22010

@chrulri
Copy link

chrulri commented Apr 14, 2017

Our project is still stuck with 2.2.0.0 due to this bug. I finally got around to dig deeper, see pull request #23599

@Jorge-Rodriguez
Copy link
Contributor

This PR seems to be very similar to the change that fixed #17582 and was later removed.

@IngussNeilands
Copy link

ansible inventory

[private]
test-1 ansible_host=1.1.1.1
test-2 ansible_host=2.2.2.2

ansible.cfg

fact_caching_connection = .cache
fact_caching_timeout = 86400
fact_caching = jsonfile
gathering = smart

test.yml

- hosts: 127.0.0.1
  connection: local
  tasks:
    - name: Ansible Cache Dir state=absent
      file: path=.cache state=absent

    - name: Ansible Cache Dir state=present
      file: path=.cache state=directory

    - name: Setup Ansible Facts with_inventory_hostnames
      setup:
      delegate_to: "{{ item }}"
      delegate_facts: True
      when: hostvars[ item ].ansible_fqdn is undefined
      with_inventory_hostnames: private

    - name: Debug Ansible Facts with_inventory_hostnames
      debug:
        var: hostvars[ item ].ansible_fqdn
      with_inventory_hostnames: private

[dot]cache dir contents

{{ item }}
1.1.1.1

last working version:

pip install git+git://github.com/ansible/ansible.git@v2.2.1.0-0.2.rc2 --upgrade

laurentgo pushed a commit to laurentgo/ansible that referenced this issue Apr 29, 2017
Logic based on Ansible 2.2.0.0 source, code adapted to changes introduced with ansible#22003
jimi-c added a commit that referenced this issue May 16, 2017
In _process_pending_results (strategy/__init__.py), we were using the delegate_to
field directly from the original task, which was not being templated correctly.
As an alternate to #23599, this patch instead pulls the host name out of the delegated
vars passed back in the task result dictionary to avoid having to re-template things.

Fixes #23599
Fixes #20508
@jimi-c
Copy link
Member

jimi-c commented May 16, 2017

Closing This Ticket

Hi!

We believe the above commit should resolve this problem for you. This will also be included in the next release.

If you continue seeing any problems related to this issue, or if you have any further questions, please let us know by stopping by one of the two mailing lists, as appropriate:

Because this project is very active, we're unlikely to see comments made on closed tickets, but the mailing list is a great way to ask questions, or post if you don't think this particular issue is resolved.

Thank you!

@jimi-c jimi-c closed this as completed May 16, 2017
jimi-c added a commit that referenced this issue May 16, 2017
In _process_pending_results (strategy/__init__.py), we were using the delegate_to
field directly from the original task, which was not being templated correctly.
As an alternate to #23599, this patch instead pulls the host name out of the delegated
vars passed back in the task result dictionary to avoid having to re-template things.

Fixes #23599
Fixes #20508
jimi-c added a commit that referenced this issue May 16, 2017
In _process_pending_results (strategy/__init__.py), we were using the delegate_to
field directly from the original task, which was not being templated correctly.
As an alternate to #23599, this patch instead pulls the host name out of the delegated
vars passed back in the task result dictionary to avoid having to re-template things.

Fixes #23599
Fixes #20508

(cherry picked from commit e5cd675)
KKoukiou pushed a commit to KKoukiou/ansible that referenced this issue May 22, 2017
In _process_pending_results (strategy/__init__.py), we were using the delegate_to
field directly from the original task, which was not being templated correctly.
As an alternate to ansible#23599, this patch instead pulls the host name out of the delegated
vars passed back in the task result dictionary to avoid having to re-template things.

Fixes ansible#23599
Fixes ansible#20508
samdoran added a commit to samdoran/ansible that referenced this issue Jun 29, 2017
Related to issue ansible#20508, issue ansible#20980 where delegated facts are not set correctly, probably more.

Fixed in PR ansible#25880

Use multi-line syntax for all tasks.

Cleanup
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bug_report labels Mar 7, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.2 This issue/PR affects Ansible v2.2 bug This issue/PR relates to a bug. c:plugins/strategy
Projects
None yet
Development

Successfully merging a pull request may close this issue.