diff --git a/awx/main/management/commands/inventory_import.py b/awx/main/management/commands/inventory_import.py index 0402f8e056..feb4331d81 100644 --- a/awx/main/management/commands/inventory_import.py +++ b/awx/main/management/commands/inventory_import.py @@ -409,10 +409,12 @@ class Command(BaseCommand): 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)] - for db_child in db_children.filter(pk__in=child_group_pks): - group_group_count += 1 - db_group.children.remove(db_child) - logger.debug('Group "%s" removed from group "%s"', db_child.name, db_group.name) + children_to_remove = list(db_children.filter(pk__in=child_group_pks)) + if children_to_remove: + group_group_count += len(children_to_remove) + db_group.children.remove(*children_to_remove) + for db_child in children_to_remove: + logger.debug('Group "%s" removed from group "%s"', db_child.name, db_group.name) # FIXME: Inventory source group relationships # Delete group/host relationships not present in imported data. db_hosts = db_group.hosts @@ -441,12 +443,12 @@ class Command(BaseCommand): 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)] - for db_host in db_hosts.filter(pk__in=del_pks): - group_host_count += 1 - if db_host not in db_group.hosts.all(): - continue - db_group.hosts.remove(db_host) - logger.debug('Host "%s" removed from group "%s"', db_host.name, db_group.name) + hosts_to_remove = list(db_hosts.filter(pk__in=del_pks)) + if hosts_to_remove: + group_host_count += len(hosts_to_remove) + db_group.hosts.remove(*hosts_to_remove) + for db_host in hosts_to_remove: + logger.debug('Host "%s" removed from group "%s"', db_host.name, db_group.name) if settings.SQL_DEBUG: logger.warning( 'group-group and group-host deletions took %d queries for %d relationships', diff --git a/awx/main/tests/functional/commands/test_inventory_import.py b/awx/main/tests/functional/commands/test_inventory_import.py index 6860889bee..d560b1fa70 100644 --- a/awx/main/tests/functional/commands/test_inventory_import.py +++ b/awx/main/tests/functional/commands/test_inventory_import.py @@ -305,6 +305,47 @@ class TestINIImports: 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_overwrite_removes_stale_memberships(self, inventory): + """When overwrite is enabled, host-group and group-group memberships + that are no longer in the imported data should be removed.""" + # First import: parent_group has two children, host_group has two hosts + inventory_import.AnsibleInventoryLoader._data = { + "_meta": {"hostvars": {"host1": {}, "host2": {}}}, + "all": {"children": ["ungrouped", "parent_group", "child_a", "child_b", "host_group"]}, + "parent_group": {"children": ["child_a", "child_b"]}, + "host_group": {"hosts": ["host1", "host2"]}, + "ungrouped": {"hosts": []}, + } + cmd = inventory_import.Command() + cmd.handle(inventory_id=inventory.pk, source=__file__, overwrite=True) + + parent = inventory.groups.get(name='parent_group') + assert set(parent.children.values_list('name', flat=True)) == {'child_a', 'child_b'} + host_grp = inventory.groups.get(name='host_group') + assert set(host_grp.hosts.values_list('name', flat=True)) == {'host1', 'host2'} + + # Second import: child_b removed from parent_group, host2 moved out of host_group + inventory_import.AnsibleInventoryLoader._data = { + "_meta": {"hostvars": {"host1": {}, "host2": {}}}, + "all": {"children": ["ungrouped", "parent_group", "child_a", "child_b", "host_group"]}, + "parent_group": {"children": ["child_a"]}, + "host_group": {"hosts": ["host1"]}, + "ungrouped": {"hosts": ["host2"]}, + } + cmd = inventory_import.Command() + cmd.handle(inventory_id=inventory.pk, source=__file__, overwrite=True) + + parent.refresh_from_db() + host_grp.refresh_from_db() + # child_b should be removed from parent_group + assert set(parent.children.values_list('name', flat=True)) == {'child_a'} + # host2 should be removed from host_group + assert set(host_grp.hosts.values_list('name', flat=True)) == {'host1'} + # host2 and child_b should still exist in the inventory, just not in those groups + assert inventory.hosts.filter(name='host2').exists() + assert inventory.groups.filter(name='child_b').exists() + @mock.patch.object(inventory_import, 'AnsibleInventoryLoader', MockLoader) def test_recursive_group_error(self, inventory): inventory_import.AnsibleInventoryLoader._data = {