Merge pull request #3206 from AlanCoding/learn_to_share

Do not remove edges from other inventory sources

Reviewed-by: https://github.com/softwarefactory-project-zuul[bot]

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)]

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 = {