Fix dashboard inventory graph to only count unique hostnames and reflect when licensed hosts count drops; API response is slower than before. Fixes https://trello.com/c/Kfw7QlRE

awx/api/views.py

@@ -396,58 +396,40 @@ class DashboardInventoryGraphView(APIView):
     def get(self, request, format=None):
         period = request.QUERY_PARAMS.get('period', 'month')
 
-        start_date = datetime.datetime.now()
+        end_date = now()
         if period == 'month':
-            end_date = start_date - dateutil.relativedelta.relativedelta(months=1)
+            start_date = end_date - dateutil.relativedelta.relativedelta(months=1)
-            interval = 'days'
+            start_date = start_date.replace(hour=0, minute=0, second=0, microsecond=0)
+            delta = dateutil.relativedelta.relativedelta(days=1)
         elif period == 'week':
-            end_date = start_date - dateutil.relativedelta.relativedelta(weeks=1)
+            start_date = end_date - dateutil.relativedelta.relativedelta(weeks=1)
-            interval = 'days'
+            start_date = start_date.replace(hour=0, minute=0, second=0, microsecond=0)
+            delta = dateutil.relativedelta.relativedelta(days=1)
         elif period == 'day':
-            end_date = start_date - dateutil.relativedelta.relativedelta(days=1)
+            start_date = end_date - dateutil.relativedelta.relativedelta(days=1)
-            interval = 'hours'
+            start_date = start_date.replace(minute=0, second=0, microsecond=0)
+            delta = dateutil.relativedelta.relativedelta(hours=1)
         else:
-            return Response({'error': 'Unknown period "%s"' % str(period)}, status=status.HTTP_400_BAD_REQUEST)
+            raise ParseError(u'Unknown period "%s"' % unicode(period))
 
-        user_hosts = get_user_queryset(request.user, Host)
+        host_stats = []
-        created_hosts = qsstats.QuerySetStats(user_hosts, 'created')
+        date = start_date
-        count_hosts = user_hosts.all().count()
+        while date < end_date:
+            next_date = date + delta
+            # Find all hosts that existed at end of intevral that are still
+            # active or were deleted after the end of interval.  Slow but
+            # accurate; haven't yet found a better way to do it.
+            hosts_qs = Host.objects.filter(created__lt=next_date)
+            hosts_qs = hosts_qs.filter(Q(active=True) | Q(active=False, modified__gte=next_date))
+            hostnames = set()
+            for name, active in hosts_qs.values_list('name', 'active').iterator():
+                if not active:
+                    name = re.sub(r'^_deleted_.*?_', '', name)
+                hostnames.add(name)
+            host_stats.append((time.mktime(date.timetuple()), len(hostnames)))
+            date = next_date
 
-        dashboard_data = {'hosts': [], 'inventory': []}
+        return Response({'hosts': host_stats})
-        last_delta = 0
-        host_data = []
-        for element in created_hosts.time_series(end_date, start_date, interval=interval)[::-1]:
-            host_data.append([time.mktime(element[0].timetuple()),
-                              count_hosts - last_delta])
-            count_hosts -= last_delta
-            last_delta = element[1]
-
-        dashboard_data['hosts'] = host_data[::-1]
-
-        hosts_by_inventory = user_hosts.all().values('inventory__id', 'inventory__name', 'has_active_failures', 'inventory_sources__id').annotate(Count("id"))
-        inventories = {}
-        for aggreg in hosts_by_inventory:
-            if (aggreg['inventory__id'], aggreg['inventory__name']) not in inventories:
-                inventories[(aggreg['inventory__id'], aggreg['inventory__name'])] = {}
-            if aggreg['inventory_sources__id'] not in inventories[(aggreg['inventory__id'], aggreg['inventory__name'])]:
-                inventories[(aggreg['inventory__id'], aggreg['inventory__name'])][aggreg['inventory_sources__id']] = {'successful': 0, 'failed': 0}
-            if aggreg['has_active_failures']:
-                inventories[(aggreg['inventory__id'], aggreg['inventory__name'])][aggreg['inventory_sources__id']]['failed'] = aggreg['id__count']
-            else:
-                inventories[(aggreg['inventory__id'], aggreg['inventory__name'])][aggreg['inventory_sources__id']]['successful'] = aggreg['id__count']
-        for inventory_id, inventory_name in inventories:
-            this_inventory = {'id': inventory_id, 'name': inventory_name, 'sources': []}
-            for source_id in inventories[(inventory_id, inventory_name)]:
-                if source_id is None:
-                    continue
-                i = InventorySource.objects.get(id=source_id)
-                this_source = {'name': i.name, 'source': i.source,
-                               'successful': inventories[(inventory_id, inventory_name)][source_id]['successful'],
-                               'failed': inventories[(inventory_id, inventory_name)][source_id]['failed']}
-                this_inventory['sources'].append(this_source)
-            dashboard_data['inventory'].append(this_inventory)
-
-        return Response(dashboard_data)
 
 
 class ScheduleList(ListAPIView):

awx/main/tests/inventory.py

@@ -14,6 +14,7 @@ from django.conf import settings
 from django.contrib.auth.models import User
 from django.core.urlresolvers import reverse
 from django.test.utils import override_settings
+from django.utils.timezone import now
 
 # AWX
 from awx.main.models import *
@@ -1091,6 +1092,60 @@ class InventoryTest(BaseTest):
         self.assertEqual(set(h_e.all_groups.values_list('pk', flat=True)),
                          set([g_e.pk]))
 
+    def test_dashboard_inventory_graph_view(self):
+        url = reverse('api:dashboard_inventory_graph_view')
+        # Test with zero hosts.
+        with self.current_user(self.super_django_user):
+            response = self.get(url)
+        self.assertFalse(sum([x[1] for x in response['hosts']]))
+        # Create hosts in inventory_a, with created one day apart, and check
+        # the time series results.
+        dtnow = now()
+        hostnames = list('abcdefg')
+        for x in xrange(len(hostnames) - 1, -1, -1):
+            hostname = hostnames[x]
+            created = dtnow - datetime.timedelta(days=x, seconds=60)
+            self.inventory_a.hosts.create(name=hostname, created=created)
+        with self.current_user(self.super_django_user):
+            response = self.get(url)
+        for n, d in enumerate(reversed(response['hosts'])):
+            self.assertEqual(d[1], max(len(hostnames) - n, 0))
+        # Create more hosts a day apart in inventory_b and check the time
+        # series results.
+        hostnames2 = list('hijklmnop')
+        for x in xrange(len(hostnames2) - 1, -1, -1):
+            hostname = hostnames2[x]
+            created = dtnow - datetime.timedelta(days=x, seconds=120)
+            self.inventory_b.hosts.create(name=hostname, created=created)
+        with self.current_user(self.super_django_user):
+            response = self.get(url)
+        for n, d in enumerate(reversed(response['hosts'])):
+            self.assertEqual(d[1], max(len(hostnames2) - n, 0) + max(len(hostnames) - n, 0))
+        # Now create some hosts in inventory_a with the same hostnames already
+        # used in inventory_b; duplicate hostnames should only be counted the
+        # first time they were seen in inventory_b.
+        hostnames3 = list('lmnop')
+        for x in xrange(len(hostnames3) - 1, -1, -1):
+            hostname = hostnames3[x]
+            created = dtnow - datetime.timedelta(days=x, seconds=180)
+            self.inventory_a.hosts.create(name=hostname, created=created)
+        with self.current_user(self.super_django_user):
+            response = self.get(url)
+        for n, d in enumerate(reversed(response['hosts'])):
+            self.assertEqual(d[1], max(len(hostnames2) - n, 0) + max(len(hostnames) - n, 0))
+        # Delete recently added hosts and verify the count drops.
+        hostnames4 = list('defg')
+        for host in Host.objects.filter(name__in=hostnames4):
+            host.mark_inactive()
+        with self.current_user(self.super_django_user):
+            response = self.get(url)
+        for n, d in enumerate(reversed(response['hosts'])):
+            count = max(len(hostnames2) - n, 0) + max(len(hostnames) - n, 0)
+            if n == 0:
+                count -= 4
+            self.assertEqual(d[1], count)
+
+
 @override_settings(CELERY_ALWAYS_EAGER=True,
                    CELERY_EAGER_PROPAGATES_EXCEPTIONS=True,
                    IGNORE_CELERY_INSPECTOR=True,