From 22da903e9c4adc9ef2e99c794c5f7abeafa014bd Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Wed, 3 Jun 2020 12:46:40 -0400 Subject: [PATCH] return changed for group_by (#32057) * return changed for group_by * added tests and fixed 'early registeration' --- changelogs/fragments/group_by_changed.yml | 2 + lib/ansible/inventory/data.py | 7 ++-- lib/ansible/inventory/group.py | 11 +++++- lib/ansible/inventory/host.py | 8 +++- lib/ansible/plugins/strategy/__init__.py | 37 ++++++++++--------- .../targets/group_by/create_groups.yml | 8 ++++ 6 files changed, 48 insertions(+), 25 deletions(-) create mode 100644 changelogs/fragments/group_by_changed.yml diff --git a/changelogs/fragments/group_by_changed.yml b/changelogs/fragments/group_by_changed.yml new file mode 100644 index 00000000000..83ca44df150 --- /dev/null +++ b/changelogs/fragments/group_by_changed.yml @@ -0,0 +1,2 @@ +bugfixes: + - group_by now should correctly refect changed status. diff --git a/lib/ansible/inventory/data.py b/lib/ansible/inventory/data.py index 84966ab2bd8..df4af766cd6 100644 --- a/lib/ansible/inventory/data.py +++ b/lib/ansible/inventory/data.py @@ -254,19 +254,20 @@ class InventoryData(object): def add_child(self, group, child): ''' Add host or group to group ''' - + added = False if group in self.groups: g = self.groups[group] if child in self.groups: - g.add_child_group(self.groups[child]) + added = g.add_child_group(self.groups[child]) elif child in self.hosts: - g.add_host(self.hosts[child]) + added = g.add_host(self.hosts[child]) else: raise AnsibleError("%s is not a known host nor group" % child) self._groups_dict_cache = {} display.debug('Group %s now contains %s' % (group, child)) else: raise AnsibleError("%s is not a known group" % group) + return added def get_groups_dict(self): """ diff --git a/lib/ansible/inventory/group.py b/lib/ansible/inventory/group.py index cead925911f..0dd91bb76dd 100644 --- a/lib/ansible/inventory/group.py +++ b/lib/ansible/inventory/group.py @@ -169,7 +169,7 @@ class Group: return self.name def add_child_group(self, group): - + added = False if self == group: raise Exception("can't add group to itself") @@ -184,6 +184,7 @@ class Group: new_ancestors.add(self) new_ancestors.difference_update(start_ancestors) + added = True self.child_groups.append(group) # update the depth of the child @@ -200,6 +201,7 @@ class Group: h.populate_ancestors(additions=new_ancestors) self.clear_hosts_cache() + return added def _check_children_depth(self): @@ -221,19 +223,24 @@ class Group: raise AnsibleError("The group named '%s' has a recursive dependency loop." % to_native(self.name)) def add_host(self, host): + added = False if host.name not in self.host_names: self.hosts.append(host) self._hosts.add(host.name) host.add_group(self) self.clear_hosts_cache() + added = True + return added def remove_host(self, host): - + removed = False if host.name in self.host_names: self.hosts.remove(host) self._hosts.remove(host.name) host.remove_group(self) self.clear_hosts_cache() + removed = True + return removed def set_variable(self, key, value): diff --git a/lib/ansible/inventory/host.py b/lib/ansible/inventory/host.py index 91bb5570b0a..7ad300790d0 100644 --- a/lib/ansible/inventory/host.py +++ b/lib/ansible/inventory/host.py @@ -113,7 +113,7 @@ class Host: self.groups.append(group) def add_group(self, group): - + added = False # populate ancestors first for oldg in group.get_ancestors(): if oldg not in self.groups: @@ -122,11 +122,14 @@ class Host: # actually add group if group not in self.groups: self.groups.append(group) + added = True + return added def remove_group(self, group): - + removed = False if group in self.groups: self.groups.remove(group) + removed = True # remove exclusive ancestors, xcept all! for oldg in group.get_ancestors(): @@ -136,6 +139,7 @@ class Host: break else: self.remove_group(oldg) + return removed def set_variable(self, key, value): if key in self.vars and isinstance(self.vars[key], MutableMapping) and isinstance(value, Mapping): diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py index d10ff6f1633..27d49fc26b4 100644 --- a/lib/ansible/plugins/strategy/__init__.py +++ b/lib/ansible/plugins/strategy/__init__.py @@ -528,16 +528,6 @@ class StrategyBase: self._tqm.send_callback('v2_runner_item_on_ok', task_result) continue - if original_task.register: - host_list = self.get_task_hosts(iterator, original_host, original_task) - - clean_copy = strip_internal_keys(module_response_deepcopy(task_result._result)) - if 'invocation' in clean_copy: - del clean_copy['invocation'] - - for target_host in host_list: - self._variable_manager.set_nonpersistent_facts(target_host, {original_task.register: clean_copy}) - # all host status messages contain 2 entries: (msg, task_result) role_ran = False if task_result.is_failed(): @@ -714,6 +704,17 @@ class StrategyBase: # finally, send the ok for this task self._tqm.send_callback('v2_runner_on_ok', task_result) + # register final results + if original_task.register: + host_list = self.get_task_hosts(iterator, original_host, original_task) + + clean_copy = strip_internal_keys(module_response_deepcopy(task_result._result)) + if 'invocation' in clean_copy: + del clean_copy['invocation'] + + for target_host in host_list: + self._variable_manager.set_nonpersistent_facts(target_host, {original_task.register: clean_copy}) + if do_handlers: self._pending_handler_results -= 1 else: @@ -850,20 +851,20 @@ class StrategyBase: group = self._inventory.groups[group_name] for parent_group_name in parent_group_names: parent_group = self._inventory.groups[parent_group_name] - parent_group.add_child_group(group) + new = parent_group.add_child_group(group) + if new and not changed: + changed = True - if real_host.name not in group.get_hosts(): - group.add_host(real_host) - changed = True + if real_host not in group.get_hosts(): + changed = group.add_host(real_host) - if group_name not in host.get_groups(): - real_host.add_group(group) - changed = True + if group not in real_host.get_groups(): + changed = real_host.add_group(group) if changed: self._inventory.reconcile_inventory() - return changed + result_item['changed'] = changed def _copy_included_file(self, included_file): ''' diff --git a/test/integration/targets/group_by/create_groups.yml b/test/integration/targets/group_by/create_groups.yml index 588c48a3471..3494a20f7dc 100644 --- a/test/integration/targets/group_by/create_groups.yml +++ b/test/integration/targets/group_by/create_groups.yml @@ -20,6 +20,14 @@ - name: group by genus group_by: key={{ genus }} + register: grouped_by_genus + +- debug: var=grouped_by_genus + +- name: ensure we reflect 'changed' on change + assert: + that: + - grouped_by_genus is changed - name: group by first three letters of genus with key in quotes group_by: key="{{ genus[:3] }}"