From efe103cdff39326f27e12616d3cee76e7f30226c Mon Sep 17 00:00:00 2001 From: Rick Elrod Date: Thu, 4 Jun 2020 21:01:56 -0500 Subject: [PATCH] Make add_host be idempotent/show changed status (#69897) Change: - The `add_host` action now shows an accurate change status. Test Plan: - Added a plethora of integration tests. Tickets: Fixes #69881 Signed-off-by: Rick Elrod --- .../fragments/69881-add_host-show-changed.yml | 2 + lib/ansible/plugins/action/add_host.py | 2 +- lib/ansible/plugins/strategy/__init__.py | 22 ++++-- .../targets/add_host/tasks/main.yml | 78 +++++++++++++++++++ 4 files changed, 98 insertions(+), 6 deletions(-) create mode 100644 changelogs/fragments/69881-add_host-show-changed.yml diff --git a/changelogs/fragments/69881-add_host-show-changed.yml b/changelogs/fragments/69881-add_host-show-changed.yml new file mode 100644 index 00000000000..3dec2adbec7 --- /dev/null +++ b/changelogs/fragments/69881-add_host-show-changed.yml @@ -0,0 +1,2 @@ +bugfixes: + - add_host action now correctly shows idempotency/changed status diff --git a/lib/ansible/plugins/action/add_host.py b/lib/ansible/plugins/action/add_host.py index aed8b1437cd..28ccba6f3cd 100644 --- a/lib/ansible/plugins/action/add_host.py +++ b/lib/ansible/plugins/action/add_host.py @@ -85,6 +85,6 @@ class ActionModule(ActionBase): if k not in special_args: host_vars[k] = self._task.args[k] - result['changed'] = True + result['changed'] = False result['add_host'] = dict(host_name=name, groups=new_groups, host_vars=host_vars) return result diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py index 27d49fc26b4..4b4e6c2a3e1 100644 --- a/lib/ansible/plugins/strategy/__init__.py +++ b/lib/ansible/plugins/strategy/__init__.py @@ -641,7 +641,7 @@ class StrategyBase: if 'add_host' in result_item: # this task added a new host (add_host module) new_host_info = result_item.get('add_host', dict()) - self._add_host(new_host_info, iterator) + self._add_host(new_host_info, result_item) elif 'add_group' in result_item: # this task added a new group (group_by module) @@ -792,11 +792,13 @@ class StrategyBase: return ret_results - def _add_host(self, host_info, iterator): + def _add_host(self, host_info, result_item): ''' Helper function to add a new host to inventory based on a task result. ''' + changed = False + if host_info: host_name = host_info.get('host_name') @@ -804,20 +806,30 @@ class StrategyBase: if host_name not in self._inventory.hosts: self._inventory.add_host(host_name, 'all') self._hosts_cache_all.append(host_name) + changed = True new_host = self._inventory.hosts.get(host_name) # Set/update the vars for this host - new_host.vars = combine_vars(new_host.get_vars(), host_info.get('host_vars', dict())) + new_host_vars = new_host.get_vars() + new_host_combined_vars = combine_vars(new_host_vars, host_info.get('host_vars', dict())) + if new_host_vars != new_host_combined_vars: + new_host.vars = new_host_combined_vars + changed = True new_groups = host_info.get('groups', []) for group_name in new_groups: if group_name not in self._inventory.groups: group_name = self._inventory.add_group(group_name) + changed = True new_group = self._inventory.groups[group_name] - new_group.add_host(self._inventory.hosts[host_name]) + if new_group.add_host(self._inventory.hosts[host_name]): + changed = True # reconcile inventory, ensures inventory rules are followed - self._inventory.reconcile_inventory() + if changed: + self._inventory.reconcile_inventory() + + result_item['changed'] = changed def _add_group(self, host, result_item): ''' diff --git a/test/integration/targets/add_host/tasks/main.yml b/test/integration/targets/add_host/tasks/main.yml index cafd6bd4eb6..8032b31b57b 100644 --- a/test/integration/targets/add_host/tasks/main.yml +++ b/test/integration/targets/add_host/tasks/main.yml @@ -37,3 +37,81 @@ - groups['bogusgroup'] is not defined # same check as above to ensure that bogus groups are undefined... - groups['newdynamicgroup'] is defined - "'newdynamichost' in groups['newdynamicgroup']" + +# Tests for idempotency +- name: Add testhost01 dynamic host + add_host: + name: testhost01 + register: add_testhost01 + +- name: Try adding testhost01 again, with no changes + add_host: + name: testhost01 + register: add_testhost01_idem + +- name: Add a host variable to testhost01 + add_host: + name: testhost01 + foo: bar + register: hostvar_testhost01 + +- name: Add the same host variable to testhost01, with no changes + add_host: + name: testhost01 + foo: bar + register: hostvar_testhost01_idem + +- name: Add another host, testhost02 + add_host: + name: testhost02 + register: add_testhost02 + +- name: Add it again for good measure + add_host: + name: testhost02 + register: add_testhost02_idem + +- name: Add testhost02 to a group + add_host: + name: testhost02 + groups: + - testhostgroup + register: add_group_testhost02 + +- name: Add testhost01 to the same group + add_host: + name: testhost01 + groups: + - testhostgroup + register: add_group_testhost01 + +- name: Add testhost02 to the group again + add_host: + name: testhost02 + groups: + - testhostgroup + register: add_group_testhost02_idem + +- name: Add testhost01 to the group again + add_host: + name: testhost01 + groups: + - testhostgroup + register: add_group_testhost01_idem + +- assert: + that: + - add_testhost01 is changed + - add_testhost01_idem is not changed + - hostvar_testhost01 is changed + - hostvar_testhost01_idem is not changed + - add_testhost02 is changed + - add_testhost02_idem is not changed + - add_group_testhost02 is changed + - add_group_testhost01 is changed + - add_group_testhost02_idem is not changed + - add_group_testhost01_idem is not changed + - groups['testhostgroup']|length == 2 + - "'testhost01' in groups['testhostgroup']" + - "'testhost02' in groups['testhostgroup']" + - hostvars['testhost01']['foo'] == 'bar'