diff --git a/awx/main/management/commands/inventory_import.py b/awx/main/management/commands/inventory_import.py index e8c9494831..3224edf5dd 100644 --- a/awx/main/management/commands/inventory_import.py +++ b/awx/main/management/commands/inventory_import.py @@ -413,6 +413,16 @@ class Command(BaseCommand): mem_host.instance_id = instance_id self.mem_instance_id_map[instance_id] = mem_host.name + def _existing_host_pks(self): + '''Returns cached set of existing / previous host primary key values + this is the starting set, meaning that it is pre-modification + by deletions and other things done in the course of this import + ''' + if not hasattr(self, '_cached_host_pk_set'): + self._cached_host_pk_set = frozenset( + self.inventory_source.hosts.values_list('pk', flat=True)) + return self._cached_host_pk_set + def _delete_hosts(self): ''' For each host in the database that is NOT in the local list, delete @@ -424,7 +434,7 @@ class Command(BaseCommand): queries_before = len(connection.queries) hosts_qs = self.inventory_source.hosts # Build list of all host pks, remove all that should not be deleted. - del_host_pks = set(hosts_qs.values_list('pk', flat=True)) + del_host_pks = set(self._existing_host_pks()) # makes mutable copy if self.instance_id_var: all_instance_ids = list(self.mem_instance_id_map.keys()) instance_ids = [] @@ -504,6 +514,10 @@ class Command(BaseCommand): group_group_count = 0 group_host_count = 0 db_groups = self.inventory_source.groups + # Set of all group names managed by this inventory source + all_source_group_names = frozenset(self.all_group.all_groups.keys()) + # Set of all host pks managed by this inventory source + all_source_host_pks = self._existing_host_pks() for db_group in db_groups.all(): if self.inventory_source.deprecated_group_id == db_group.id: # TODO: remove in 3.3 logger.debug( @@ -514,9 +528,18 @@ class Command(BaseCommand): # Delete child group relationships not present in imported data. db_children = db_group.children db_children_name_pk_map = dict(db_children.values_list('name', 'pk')) + # Exclude child groups from removal list if they were returned by + # the import, because this parent-child relationship has not changed mem_children = self.all_group.all_groups[db_group.name].children for mem_group in mem_children: db_children_name_pk_map.pop(mem_group.name, None) + # Exclude child groups from removal list if they were not imported + # by this specific inventory source, because + # those relationships are outside of the dominion of this inventory source + other_source_group_names = set(db_children_name_pk_map.keys()) - all_source_group_names + for group_name in other_source_group_names: + db_children_name_pk_map.pop(group_name, None) + # Removal list is complete - now perform the removals del_child_group_pks = list(set(db_children_name_pk_map.values())) for offset in range(0, len(del_child_group_pks), self._batch_size): child_group_pks = del_child_group_pks[offset:(offset + self._batch_size)] @@ -529,6 +552,12 @@ class Command(BaseCommand): # Delete group/host relationships not present in imported data. db_hosts = db_group.hosts del_host_pks = set(db_hosts.values_list('pk', flat=True)) + # Exclude child hosts from removal list if they were not imported + # by this specific inventory source, because + # those relationships are outside of the dominion of this inventory source + del_host_pks = del_host_pks & all_source_host_pks + # Exclude child hosts from removal list if they were returned by + # the import, because this group-host relationship has not changed mem_hosts = self.all_group.all_groups[db_group.name].hosts all_mem_host_names = [h.name for h in mem_hosts if not h.instance_id] for offset in range(0, len(all_mem_host_names), self._batch_size): @@ -543,6 +572,7 @@ class Command(BaseCommand): all_db_host_pks = [v for k,v in self.db_instance_id_map.items() if k in all_mem_instance_ids] for db_host_pk in all_db_host_pks: del_host_pks.discard(db_host_pk) + # Removal list is complete - now perform the removals del_host_pks = list(del_host_pks) for offset in range(0, len(del_host_pks), self._batch_size): del_pks = del_host_pks[offset:(offset + self._batch_size)] diff --git a/awx/main/tests/functional/commands/test_inventory_import.py b/awx/main/tests/functional/commands/test_inventory_import.py index 155bd8d7b3..979d97ebc9 100644 --- a/awx/main/tests/functional/commands/test_inventory_import.py +++ b/awx/main/tests/functional/commands/test_inventory_import.py @@ -4,6 +4,7 @@ # Python import pytest from unittest import mock +import os # Django from django.core.management.base import CommandError @@ -198,6 +199,65 @@ class TestINIImports: assert h.name == 'foo' assert h.variables_dict == {"some_hostvar": "foobar"} + @mock.patch.object(inventory_import, 'AnsibleInventoryLoader', MockLoader) + def test_memberships_are_respected(self, inventory): + """This tests that if import 1 added a group-group and group-host memberhip + that import 2 will not remove those memberships, even when adding + importing the same parent groups + """ + inventory_import.AnsibleInventoryLoader._data = { + "_meta": { + "hostvars": {"foo": {}} + }, + "all": { + "children": ["ungrouped", "is_a_parent", "has_a_host", "is_a_child"] + }, + "is_a_parent": { + "children": ["is_a_child"] + }, + "has_a_host": { + "hosts": ["foo"] + }, + "ungrouped": { + "hosts": [] + } + } + cmd = inventory_import.Command() + cmd.handle(inventory_id=inventory.pk, source=__file__) + assert inventory.hosts.count() == 1 # baseline worked + + inv_src2 = inventory.inventory_sources.create( + name='bar', overwrite=True + ) + os.environ['INVENTORY_SOURCE_ID'] = str(inv_src2.pk) + os.environ['INVENTORY_UPDATE_ID'] = str(inv_src2.create_unified_job().pk) + # scenario where groups are already imported, and overwrite is true + inv_src2.groups.add(inventory.groups.get(name='is_a_parent')) + inv_src2.groups.add(inventory.groups.get(name='has_a_host')) + + inventory_import.AnsibleInventoryLoader._data = { + "_meta": { + "hostvars": {"bar": {}} + }, + "all": { + "children": ["ungrouped", "is_a_parent", "has_a_host"] + }, + "ungrouped": { + "hosts": ["bar"] + } + } + cmd = inventory_import.Command() + cmd.handle(inventory_id=inventory.pk, source=__file__, overwrite=True) + + del os.environ['INVENTORY_SOURCE_ID'] + del os.environ['INVENTORY_UPDATE_ID'] + + # the overwriting import did not destroy relationships from first import + parent_group = inventory.groups.get(name='is_a_parent') + assert parent_group.children.count() == 1 + has_host_group = inventory.groups.get(name='has_a_host') + assert has_host_group.hosts.count() == 1 + @mock.patch.object(inventory_import, 'AnsibleInventoryLoader', MockLoader) def test_recursive_group_error(self, inventory): inventory_import.AnsibleInventoryLoader._data = {