Merge ~ltrager/maas:2.3_fix_scriptresult_updates_on_trigger into maas:2.3

[Lee Trager]

Update ScriptResult cache on node listen from trigger

Backport of f0e252b for LP: #1830365

[Alberto Donato (ack)]

+1

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b 2.3_fix_scriptresult_updates_on_trigger lp:~ltrager/maas/+git/maas into -b 2.3 lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/5802/consoleText

05937ba... by Lee Trager on 2019-05-31

Merge branch '2.3' into 2.3_fix_scriptresult_updates_on_trigger

diff --git a/src/maasserver/websockets/handlers/node.py b/src/maasserver/websockets/handlers/node.py
index d53e445..fda47a7 100644
--- a/src/maasserver/websockets/handlers/node.py
+++ b/src/maasserver/websockets/handlers/node.py
@@ -313,9 +313,8 @@ class NodeHandler(TimestampedModelHandler):
 
         return data
 
-    def _cache_pks(self, nodes):
+    def _cache_script_results(self, nodes):
         """Refresh the ScriptResult cache from the given node."""
-        super()._cache_pks(nodes)
         script_results = ScriptResult.objects.filter(
             script_set__node__in=nodes)
         script_results = script_results.select_related('script_set', 'script')
@@ -324,6 +323,7 @@ class NodeHandler(TimestampedModelHandler):
             '-id')
         script_results = script_results.distinct(
             'script_name', 'physical_blockdevice_id', 'script_set__node_id')
+        nodes_reset = set()
         for script_result in script_results:
             node_id = script_result.script_set.node_id
             if script_result.script is not None:
@@ -331,8 +331,14 @@ class NodeHandler(TimestampedModelHandler):
             else:
                 hardware_type = HARDWARE_TYPE.NODE
 
-            if node_id not in self._script_results:
+            # _cache_script_results is only called once per list(), get()
+            # Postgres trigger update. Make sure the cache is cleared for the
+            # node so if list(), get(), or a trigger is called multiple times
+            # with the same instance of NodesHandler() only one set of results
+            # is stored.
+            if node_id not in nodes_reset:
                 self._script_results[node_id] = {}
+                nodes_reset.add(node_id)
 
             if hardware_type not in self._script_results[node_id]:
                 self._script_results[node_id][hardware_type] = []
@@ -352,6 +358,14 @@ class NodeHandler(TimestampedModelHandler):
                 self._script_results[node_id][hardware_type].append(
                     script_result)
 
+    def _cache_pks(self, nodes):
+        super()._cache_pks(nodes)
+        self._cache_script_results(nodes)
+
+    def on_listen_for_active_pk(self, action, pk, obj):
+        self._cache_script_results([obj])
+        return super().on_listen_for_active_pk(action, pk, obj)
+
     def dehydrate_blockdevice(self, blockdevice, obj):
         """Return `BlockDevice` formatted for JSON encoding."""
         # model and serial are currently only avalible on physical block
diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py
index c6bbbbf..007bf55 100644
--- a/src/maasserver/websockets/handlers/tests/test_machine.py
+++ b/src/maasserver/websockets/handlers/tests/test_machine.py
@@ -498,6 +498,58 @@ class TestMachineHandler(MAASServerTestCase):
             queries_total, 12,
             "Number of queries has changed; make sure this is expected.")
 
+    def test_trigger_update_updates_script_result_cache(self):
+        owner = factory.make_User()
+        node = factory.make_Node(owner=owner)
+        commissioning_script_set = factory.make_ScriptSet(
+            node=node, result_type=RESULT_TYPE.COMMISSIONING)
+        testing_script_set = factory.make_ScriptSet(
+            node=node, result_type=RESULT_TYPE.TESTING)
+        node.current_commissioning_script_set = commissioning_script_set
+        node.current_testing_script_set = testing_script_set
+        node.save()
+        for _ in range(10):
+            factory.make_ScriptResult(
+                status=SCRIPT_STATUS.PASSED,
+                script_set=commissioning_script_set)
+            factory.make_ScriptResult(
+                status=SCRIPT_STATUS.PASSED,
+                script_set=testing_script_set)
+
+        handler = MachineHandler(owner, {})
+        # Simulate a trigger pushing an update to the UI
+        _, _, ret = handler.on_listen_for_active_pk(
+            'update', node.system_id, node)
+        self.assertEquals(ret['commissioning_script_count'], 10)
+        self.assertEquals(ret['testing_script_count'], 10)
+
+    def test_cache_clears_on_reload(self):
+        owner = factory.make_User()
+        node = factory.make_Node(owner=owner)
+        commissioning_script_set = factory.make_ScriptSet(
+            node=node, result_type=RESULT_TYPE.COMMISSIONING)
+        testing_script_set = factory.make_ScriptSet(
+            node=node, result_type=RESULT_TYPE.TESTING)
+        node.current_commissioning_script_set = commissioning_script_set
+        node.current_testing_script_set = testing_script_set
+        node.save()
+        for _ in range(10):
+            factory.make_ScriptResult(
+                status=SCRIPT_STATUS.PASSED,
+                script_set=commissioning_script_set)
+            factory.make_ScriptResult(
+                status=SCRIPT_STATUS.PASSED,
+                script_set=testing_script_set)
+
+        handler = MachineHandler(owner, {})
+        handler.list({})
+        handler.list({})
+        count = 0
+        for result_type in handler._script_results[node.id].values():
+            for _ in result_type:
+                count += 1
+        self.assertEqual(20, count)
+
     def test_dehydrate_owner_empty_when_None(self):
         owner = factory.make_User()
         handler = MachineHandler(owner, {})