From eaadbe9730ec37d11c15c76ec7be6645b8a2da53 Mon Sep 17 00:00:00 2001
From: Ryan Petrello <rpetrell@redhat.com>
Date: Tue, 16 Jun 2020 12:36:51 -0400
Subject: [PATCH] fix a regression in how job host summaries are generated

this change fixes a bug introduced in the optimization at https://github.com/ansible/awx/pull/7352

1. Create inventory with multiple hosts
2. Run a playbook with a limit to match only one host
3. Run job, verify that it only acts on the one host
4. Go to inventory host list and see that all the hosts have last_job updated to point to the job that only acted on one host.
---
 awx/main/models/events.py                     |  6 +-
 .../tests/functional/models/test_events.py    | 57 ++++++++++++++++++-
 2 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/awx/main/models/events.py b/awx/main/models/events.py
index ac33a311f4..1f79b0e24b 100644
--- a/awx/main/models/events.py
+++ b/awx/main/models/events.py
@@ -338,7 +338,7 @@ class BasePlaybookEvent(CreatedModifiedModel):
 
             if isinstance(self, JobEvent):
                 hostnames = self._hostnames()
-                self._update_host_summary_from_stats(hostnames)
+                self._update_host_summary_from_stats(set(hostnames))
                 if self.job.inventory:
                     try:
                         self.job.inventory.update_computed_fields()
@@ -521,7 +521,9 @@ class JobEvent(BasePlaybookEvent):
                 for summary in JobHostSummary.objects.filter(job_id=job.id).values('id', 'host_id')
             )
             for h in all_hosts:
-                h.last_job_id = job.id
+                # if the hostname *shows up* in the playbook_on_stats event
+                if h.name in hostnames:
+                    h.last_job_id = job.id
                 if h.id in host_mapping:
                     h.last_job_host_summary_id = host_mapping[h.id]
             Host.objects.bulk_update(all_hosts, ['last_job_id', 'last_job_host_summary_id'])
diff --git a/awx/main/tests/functional/models/test_events.py b/awx/main/tests/functional/models/test_events.py
index 7f881a2fea..943bd34654 100644
--- a/awx/main/tests/functional/models/test_events.py
+++ b/awx/main/tests/functional/models/test_events.py
@@ -3,7 +3,7 @@ import pytest
 
 from django.utils.timezone import now
 
-from awx.main.models import Job, JobEvent, Inventory, Host
+from awx.main.models import Job, JobEvent, Inventory, Host, JobHostSummary
 
 
 @pytest.mark.django_db
@@ -153,3 +153,58 @@ def test_host_summary_generation_with_deleted_hosts():
     assert ids == [-1, -1, -1, -1, -1, 6, 7, 8, 9, 10]
     assert names == ['Host 0', 'Host 1', 'Host 2', 'Host 3', 'Host 4', 'Host 5',
                      'Host 6', 'Host 7', 'Host 8', 'Host 9']
+
+
+@pytest.mark.django_db
+def test_host_summary_generation_with_limit():
+    # Make an inventory with 10 hosts, run a playbook with a --limit
+    # pointed at *one* host,
+    # Verify that *only* that host has an associated JobHostSummary and that
+    # *only* that host has an updated value for .last_job.
+    hostnames = [f'Host {i}' for i in range(10)]
+    inv = Inventory()
+    inv.save()
+    Host.objects.bulk_create([
+        Host(created=now(), modified=now(), name=h, inventory_id=inv.id)
+        for h in hostnames
+    ])
+    j = Job(inventory=inv)
+    j.save()
+
+    # host map is a data structure that tracks a mapping of host name --> ID
+    # for the inventory, _regardless_ of whether or not there's a limit
+    # applied to the actual playbook run
+    host_map = dict((host.name, host.id) for host in inv.hosts.all())
+
+    # by making the playbook_on_stats *only* include Host 1, we're emulating
+    # the behavior of a `--limit=Host 1`
+    matching_host = Host.objects.get(name='Host 1')
+    JobEvent.create_from_data(
+        job_id=j.pk,
+        parent_uuid='abc123',
+        event='playbook_on_stats',
+        event_data={
+            'ok': {matching_host.name: len(matching_host.name)}, # effectively, limit=Host 1
+            'changed': {},
+            'dark': {},
+            'failures': {},
+            'ignored': {},
+            'processed': {},
+            'rescued': {},
+            'skipped': {},
+        },
+        host_map=host_map
+    ).save()
+
+    # since the playbook_on_stats only references one host,
+    # there should *only* be on JobHostSummary record (and it should
+    # be related to the appropriate Host)
+    assert JobHostSummary.objects.count() == 1
+    for h in Host.objects.all():
+        if h.name == 'Host 1':
+            assert h.last_job_id == j.id
+            assert h.last_job_host_summary_id == JobHostSummary.objects.first().id
+        else:
+            # all other hosts in the inventory should remain untouched
+            assert h.last_job_id is None
+            assert h.last_job_host_summary_id is None