Merge ~newell-jensen/maas:lp1828310 into maas:master

[Newell Jensen]

LP: #1828310 -- Only log start and failure messages from cloud-init and curtin.

[Blake Rouse (blake-rouse)]

review: Needs Information

0c315aa... by Newell Jensen on 2019-05-22

Review fixes.

[Newell Jensen (newell-jensen)]

Updated to use the original tests with 'start'. My previous changes didn't nullify the tests, it just took out the events that were no longer being created, however this shows that the previous tests also work with 'start' and that the events are created.

[Blake Rouse (blake-rouse)]

Thanks for the fixes.

review: Approve

[Lee Trager (ltrager)]

Looks good one small suggestion. maasserver.preseed defines CURTIN_INSTALL_LOG as /tmp/install.log. I would use that instead of hard coding it instead we ever need to change the location.

review: Approve

854cd75... by Newell Jensen on 2019-05-22

Replace /tmp/install.log with CURTIN_INSTALL_LOG

diff --git a/src/metadataserver/api_twisted.py b/src/metadataserver/api_twisted.py
index 9b30d89..e1d5ded 100644
--- a/src/metadataserver/api_twisted.py
+++ b/src/metadataserver/api_twisted.py
@@ -306,10 +306,11 @@ class StatusWorkerService(TimerService, object):
         failed = result in ['FAIL', 'FAILURE']
         default_exit_status = 1 if failed else 0
 
-        # Add this event to the node event log.
-        add_event_to_node_event_log(
-            node, origin, activity_name, description, event_type, result,
-            message['timestamp'])
+        # Add this event to the node event log if 'start' or a 'failure'.
+        if event_type == 'start' or failed:
+            add_event_to_node_event_log(
+                node, origin, activity_name, description, event_type, result,
+                message['timestamp'])
 
         # Group files together with the ScriptResult they belong.
         results = {}
diff --git a/src/metadataserver/tests/test_api_twisted.py b/src/metadataserver/tests/test_api_twisted.py
index 81b6fbf..4c2add2 100644
--- a/src/metadataserver/tests/test_api_twisted.py
+++ b/src/metadataserver/tests/test_api_twisted.py
@@ -376,6 +376,22 @@ class TestStatusWorkerService(MAASServerTestCase):
         worker = StatusWorkerService(sentinel.dbtasks)
         return worker._processMessage(node, payload)
 
+    def test_process_message_logs_event_for_start_event_type(self):
+        node = factory.make_Node(
+            status=NODE_STATUS.DEPLOYING, with_empty_script_sets=True)
+        payload = {
+            'event_type': 'start',
+            'origin': 'curtin',
+            'name': 'cmd-install',
+            'description': 'Installation has started.',
+            'timestamp': datetime.utcnow(),
+        }
+        self.processMessage(node, payload)
+        event = Event.objects.last()
+        self.assertEqual(
+            event.description,
+            CURTIN_INSTALL_LOG + " changed status from 'Pending' to 'Running'")
+
     def test_process_message_returns_false_when_node_deleted(self):
         node1 = factory.make_Node(status=NODE_STATUS.DEPLOYING)
         node1.delete()
@@ -390,10 +406,15 @@ class TestStatusWorkerService(MAASServerTestCase):
         self.assertFalse(self.processMessage(node1, payload))
 
     def test_status_installation_result_does_not_affect_other_node(self):
-        node1 = factory.make_Node(status=NODE_STATUS.DEPLOYING)
+        node1 = factory.make_Node(
+            status=NODE_STATUS.DEPLOYING, with_empty_script_sets=True)
         node2 = factory.make_Node(status=NODE_STATUS.DEPLOYING)
+        script = factory.make_Script(may_reboot=True)
+        factory.make_ScriptResult(
+            script=script, script_set=node1.current_installation_script_set,
+            status=SCRIPT_STATUS.RUNNING)
         payload = {
-            'event_type': 'finish',
+            'event_type': 'start',
             'result': 'SUCCESS',
             'origin': 'curtin',
             'name': 'cmd-install',
@@ -405,15 +426,21 @@ class TestStatusWorkerService(MAASServerTestCase):
             NODE_STATUS.DEPLOYING, reload_object(node2).status)
         # Check last node1 event.
         self.assertEqual(
-            "'curtin' Command Install",
+            CURTIN_INSTALL_LOG + " changed status from 'Pending' to 'Running'",
             Event.objects.filter(node=node1).last().description)
-        # There must me no events for node2.
+        # There must be no events for node2.
         self.assertFalse(Event.objects.filter(node=node2).exists())
 
     def test_status_installation_success_leaves_node_deploying(self):
-        node = factory.make_Node(interface=True, status=NODE_STATUS.DEPLOYING)
+        node = factory.make_Node(
+            interface=True, status=NODE_STATUS.DEPLOYING,
+            with_empty_script_sets=True)
+        script = factory.make_Script(may_reboot=True)
+        factory.make_ScriptResult(
+            script=script, script_set=node.current_installation_script_set,
+            status=SCRIPT_STATUS.RUNNING)
         payload = {
-            'event_type': 'finish',
+            'event_type': 'start',
             'result': 'SUCCESS',
             'origin': 'curtin',
             'name': 'cmd-install',
@@ -424,7 +451,7 @@ class TestStatusWorkerService(MAASServerTestCase):
         self.assertEqual(NODE_STATUS.DEPLOYING, reload_object(node).status)
         # Check last node event.
         self.assertEqual(
-            "'curtin' Command Install",
+            "/tmp/install.log changed status from 'Pending' to 'Running'",
             Event.objects.filter(node=node).last().description)
 
     def test_status_commissioning_failure_leaves_node_failed(self):