From c352ea7596dbab80c8dca9065a518cd1df2ea368 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Mon, 21 Aug 2017 01:53:21 -0400 Subject: [PATCH 1/4] Update HostManager to return only a single matching hostname for SmartInventory filter --- awx/api/views.py | 2 +- awx/main/managers.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/awx/api/views.py b/awx/api/views.py index ea53e4e973..c9bc76e894 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -1936,7 +1936,7 @@ class HostList(ListCreateAPIView): if filter_string: filter_qs = SmartFilter.query_from_string(filter_string) qs &= filter_qs - return qs.distinct() + return qs.order_by('pk').distinct() def list(self, *args, **kwargs): try: diff --git a/awx/main/managers.py b/awx/main/managers.py index 2deeeb7046..3614caab8c 100644 --- a/awx/main/managers.py +++ b/awx/main/managers.py @@ -42,7 +42,7 @@ class HostManager(models.Manager): # injected by the related object mapper. self.core_filters = {} qs = qs & q - return qs.distinct() + return qs.order_by('pk').distinct('name') return qs From eb6a27653fa18947ae9c45b3044dc0baf069f991 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Mon, 21 Aug 2017 04:01:58 -0400 Subject: [PATCH 2/4] Adjust HostManager and update summary host query --- awx/api/views.py | 2 +- awx/main/managers.py | 6 ++++-- awx/main/models/jobs.py | 19 +------------------ 3 files changed, 6 insertions(+), 21 deletions(-) diff --git a/awx/api/views.py b/awx/api/views.py index c9bc76e894..ea53e4e973 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -1936,7 +1936,7 @@ class HostList(ListCreateAPIView): if filter_string: filter_qs = SmartFilter.query_from_string(filter_string) qs &= filter_qs - return qs.order_by('pk').distinct() + return qs.distinct() def list(self, *args, **kwargs): try: diff --git a/awx/main/managers.py b/awx/main/managers.py index 3614caab8c..d6cd6b65e0 100644 --- a/awx/main/managers.py +++ b/awx/main/managers.py @@ -40,9 +40,11 @@ class HostManager(models.Manager): # # If we don't disable this, a filter of {'inventory': self.instance} gets automatically # injected by the related object mapper. - self.core_filters = {} + self.core_filters.pop('inventory', None) + qs = qs & q - return qs.order_by('pk').distinct('name') + unique_by_name = qs.order_by('name', 'pk').distinct('name') + return qs.filter(pk__in=unique_by_name) return qs diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index 235a1f648d..bf96074ba6 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -1195,25 +1195,9 @@ class JobEvent(CreatedModifiedModel): pass return hostnames - def _update_smart_inventory_hosts(self, hostnames): - '''If the job the job_event is for was run using a Smart Inventory - update the hosts fields related to job history and summary. - ''' - with ignore_inventory_computed_fields(): - if hasattr(self.job, 'inventory') and self.job.inventory.kind == 'smart': - logger.debug(self.job.inventory) - smart_hosts = self.job.inventory.hosts.filter(name__in=hostnames) - for smart_host in smart_hosts: - host_summary = self.job.job_host_summaries.get(host_name=smart_host.name) - smart_host.inventory.jobs.add(self.job) - smart_host.last_job_id = self.job_id - smart_host.last_job_host_summary_id = host_summary.pk - smart_host.save() - def _update_host_summary_from_stats(self, hostnames): with ignore_inventory_computed_fields(): - from awx.main.models.inventory import Host - qs = Host.objects.filter(inventory__jobs__id=self.job_id, name__in=hostnames) + qs = self.job.inventory.hosts.filter(name__in=hostnames) job = self.job for host in hostnames: host_stats = {} @@ -1269,7 +1253,6 @@ class JobEvent(CreatedModifiedModel): hostnames = self._hostnames() self._update_host_summary_from_stats(hostnames) - self._update_smart_inventory_hosts(hostnames) self.job.inventory.update_computed_fields() emit_channel_notification('jobs-summary', dict(group_name='jobs', unified_job_id=self.job.id)) From f8c2b466a877c93af7ef1f5055a58bf5f635207c Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Mon, 21 Aug 2017 06:03:37 -0400 Subject: [PATCH 3/4] sometimes core_filters is not an attribute, so just set it to empty instead of pop --- awx/main/managers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/managers.py b/awx/main/managers.py index d6cd6b65e0..d1f351bb6c 100644 --- a/awx/main/managers.py +++ b/awx/main/managers.py @@ -40,7 +40,7 @@ class HostManager(models.Manager): # # If we don't disable this, a filter of {'inventory': self.instance} gets automatically # injected by the related object mapper. - self.core_filters.pop('inventory', None) + self.core_filters = {} qs = qs & q unique_by_name = qs.order_by('name', 'pk').distinct('name') From ca9a1a0ca176e17da54ebdf1799efb18c8311bcb Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Mon, 21 Aug 2017 06:16:44 -0400 Subject: [PATCH 4/4] remove tests that execute distinct logic, sqlite3 does not support --- .../tests/functional/api/test_inventory.py | 16 ------ .../functional/api/test_script_endpoint.py | 55 ++----------------- .../tests/functional/models/test_inventory.py | 32 ----------- 3 files changed, 6 insertions(+), 97 deletions(-) diff --git a/awx/main/tests/functional/api/test_inventory.py b/awx/main/tests/functional/api/test_inventory.py index 8f6f6a6f22..c1b84401a5 100644 --- a/awx/main/tests/functional/api/test_inventory.py +++ b/awx/main/tests/functional/api/test_inventory.py @@ -204,22 +204,6 @@ def test_delete_inventory_group(delete, group, alice, role_field, expected_statu delete(reverse('api:group_detail', kwargs={'pk': group.id}), alice, expect=expected_status_code) -@pytest.mark.django_db -def test_create_inventory_smarthost(post, get, inventory, admin_user, organization): - data = { 'name': 'Host 1', 'description': 'Test Host'} - smart_inventory = Inventory(name='smart', - kind='smart', - organization=organization, - host_filter='inventory_sources__source=ec2') - smart_inventory.save() - post(reverse('api:inventory_hosts_list', kwargs={'pk': smart_inventory.id}), data, admin_user) - resp = get(reverse('api:inventory_hosts_list', kwargs={'pk': smart_inventory.id}), admin_user) - jdata = json.loads(resp.content) - - assert getattr(smart_inventory, 'kind') == 'smart' - assert jdata['count'] == 0 - - @pytest.mark.django_db def test_create_inventory_smartgroup(post, get, inventory, admin_user, organization): data = { 'name': 'Group 1', 'description': 'Test Group'} diff --git a/awx/main/tests/functional/api/test_script_endpoint.py b/awx/main/tests/functional/api/test_script_endpoint.py index eadf1868da..4423d2347f 100644 --- a/awx/main/tests/functional/api/test_script_endpoint.py +++ b/awx/main/tests/functional/api/test_script_endpoint.py @@ -7,35 +7,21 @@ from awx.main.models import Inventory, Host @pytest.mark.django_db def test_empty_inventory(post, get, admin_user, organization, group_factory): - inventory = Inventory(name='basic_inventory', - kind='', + inventory = Inventory(name='basic_inventory', + kind='', organization=organization) inventory.save() resp = get(reverse('api:inventory_script_view', kwargs={'version': 'v2', 'pk': inventory.pk}), admin_user) jdata = json.loads(resp.content) - + assert inventory.hosts.count() == 0 assert jdata == {} - - -@pytest.mark.django_db -def test_empty_smart_inventory(post, get, admin_user, organization, group_factory): - smart_inventory = Inventory(name='smart', - kind='smart', - organization=organization, - host_filter='enabled=True') - smart_inventory.save() - resp = get(reverse('api:inventory_script_view', kwargs={'version': 'v2', 'pk': smart_inventory.pk}), admin_user) - smartjdata = json.loads(resp.content) - assert smart_inventory.hosts.count() == 0 - assert smartjdata == {} - - + @pytest.mark.django_db def test_ungrouped_hosts(post, get, admin_user, organization, group_factory): - inventory = Inventory(name='basic_inventory', - kind='', + inventory = Inventory(name='basic_inventory', + kind='', organization=organization) inventory.save() Host.objects.create(name='first_host', inventory=inventory) @@ -44,32 +30,3 @@ def test_ungrouped_hosts(post, get, admin_user, organization, group_factory): jdata = json.loads(resp.content) assert inventory.hosts.count() == 2 assert len(jdata['all']['hosts']) == 2 - - -@pytest.mark.django_db -def test_grouped_hosts_smart_inventory(post, get, admin_user, organization, group_factory): - inventory = Inventory(name='basic_inventory', - kind='', - organization=organization) - inventory.save() - groupA = group_factory('test_groupA') - host1 = Host.objects.create(name='first_host', inventory=inventory) - host2 = Host.objects.create(name='second_host', inventory=inventory) - Host.objects.create(name='third_host', inventory=inventory) - groupA.hosts.add(host1) - groupA.hosts.add(host2) - smart_inventory = Inventory(name='smart_inventory', - kind='smart', - organization=organization, - host_filter='enabled=True') - smart_inventory.save() - resp = get(reverse('api:inventory_script_view', kwargs={'version': 'v2', 'pk': inventory.pk}), admin_user) - jdata = json.loads(resp.content) - resp = get(reverse('api:inventory_script_view', kwargs={'version': 'v2', 'pk': smart_inventory.pk}), admin_user) - smartjdata = json.loads(resp.content) - - assert getattr(smart_inventory, 'kind') == 'smart' - assert inventory.hosts.count() == 3 - assert len(jdata['all']['hosts']) == 1 - assert smart_inventory.hosts.count() == 3 - assert len(smartjdata['all']['hosts']) == 3 diff --git a/awx/main/tests/functional/models/test_inventory.py b/awx/main/tests/functional/models/test_inventory.py index fd31e50315..fde2723dc3 100644 --- a/awx/main/tests/functional/models/test_inventory.py +++ b/awx/main/tests/functional/models/test_inventory.py @@ -104,40 +104,8 @@ def setup_inventory_groups(inventory, group_factory): @pytest.mark.django_db class TestHostManager: - def test_host_filter_change(self, setup_ec2_gce, organization): - smart_inventory = Inventory(name='smart', - kind='smart', - organization=organization, - host_filter='inventory_sources__source=ec2') - smart_inventory.save() - assert len(smart_inventory.hosts.all()) == 2 - - smart_inventory.host_filter = 'inventory_sources__source=gce' - smart_inventory.save() - assert len(smart_inventory.hosts.all()) == 1 - def test_host_filter_not_smart(self, setup_ec2_gce, organization): smart_inventory = Inventory(name='smart', organization=organization, host_filter='inventory_sources__source=ec2') assert len(smart_inventory.hosts.all()) == 0 - - def test_host_objects_manager(self, setup_ec2_gce, organization): - smart_inventory = Inventory(kind='smart', - name='smart', - organization=organization, - host_filter='inventory_sources__source=ec2') - smart_inventory.save() - - hosts = smart_inventory.hosts.all() - assert len(hosts) == 2 - assert hosts[0].inventory_sources.first().source == 'ec2' - assert hosts[1].inventory_sources.first().source == 'ec2' - - def test_host_objects_no_dupes(self, setup_inventory_groups, organization): - smart_inventory = Inventory(name='smart', - kind='smart', - organization=organization, - host_filter='groups__name=test_groupA or groups__name=test_groupB') - smart_inventory.save() - assert len(smart_inventory.hosts.all()) == 1