Merge ~ltrager/maas:lp1840181_2.6 into maas:2.6

[Lee Trager]

Backport of df8ae4e LP: #1840181 - Use model to get device name for ScriptResult events

6049734... by Lee Trager on 2019-08-19

Fix merge conflict

[Lee Trager (ltrager)]

Approved in https://code.launchpad.net/~ltrager/maas/+git/maas/+merge/371387

review: Approve

diff --git a/src/maasserver/api/tests/test_machines.py b/src/maasserver/api/tests/test_machines.py
index 1ec635c..7849678 100644
--- a/src/maasserver/api/tests/test_machines.py
+++ b/src/maasserver/api/tests/test_machines.py
@@ -397,6 +397,8 @@ class TestMachinesAPI(APITestCase.ForUser):
         for _ in range(10):
             node = factory.make_Node_with_Interface_on_Subnet()
             factory.make_VirtualBlockDevice(node=node)
+        # XXX ltrager 2019-08-16 - Work around for LP:1840491
+        Node.objects.update(boot_disk=None)
 
         num_queries1, response1 = count_queries(
             self.client.get, reverse('machines_handler'))
@@ -404,6 +406,8 @@ class TestMachinesAPI(APITestCase.ForUser):
         for _ in range(10):
             node = factory.make_Node_with_Interface_on_Subnet()
             factory.make_VirtualBlockDevice(node=node)
+        # XXX ltrager 2019-08-16 - Work around for LP:1840491
+        Node.objects.update(boot_disk=None)
         num_queries2, response2 = count_queries(
             self.client.get, reverse('machines_handler'))
 
diff --git a/src/maasserver/api/tests/test_tag.py b/src/maasserver/api/tests/test_tag.py
index 682721c..a489be0 100644
--- a/src/maasserver/api/tests/test_tag.py
+++ b/src/maasserver/api/tests/test_tag.py
@@ -1,4 +1,4 @@
-# Copyright 2013-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2013-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for the Tags API."""
@@ -16,7 +16,10 @@ from apiclient.creds import convert_tuple_to_string
 from django.conf import settings
 from maasserver import middleware
 from maasserver.enum import NODE_STATUS
-from maasserver.models import Tag
+from maasserver.models import (
+    Node,
+    Tag,
+)
 from maasserver.models.node import generate_node_system_id
 from maasserver.models.user import (
     create_auth_token,
@@ -167,6 +170,8 @@ class TestTagAPI(APITestCase.ForUser):
         for _ in range(3):
             machine = factory.make_Node_with_Interface_on_Subnet()
             machine.tags.add(tag)
+        # XXX ltrager 2019-08-16 - Work around for LP:1840491
+        Node.objects.update(boot_disk=None)
         for _ in range(3):
             device = factory.make_Device()
             device.tags.add(tag)
@@ -176,6 +181,8 @@ class TestTagAPI(APITestCase.ForUser):
         for _ in range(3):
             machine = factory.make_Node_with_Interface_on_Subnet()
             machine.tags.add(tag)
+        # XXX ltrager 2019-08-16 - Work around for LP:1840491
+        Node.objects.update(boot_disk=None)
         for _ in range(3):
             device = factory.make_Device()
             device.tags.add(tag)
@@ -234,12 +241,16 @@ class TestTagAPI(APITestCase.ForUser):
         for _ in range(3):
             machine = factory.make_Node_with_Interface_on_Subnet()
             machine.tags.add(tag)
+        # XXX ltrager 2019-08-16 - Work around for LP:1840491
+        Node.objects.update(boot_disk=None)
         num_queries1, response1 = count_queries(
             self.client.get, self.get_tag_uri(tag), {'op': 'machines'})
 
         for _ in range(3):
             machine = factory.make_Node_with_Interface_on_Subnet()
             machine.tags.add(tag)
+        # XXX ltrager 2019-08-16 - Work around for LP:1840491
+        Node.objects.update(boot_disk=None)
         num_queries2, response2 = count_queries(
             self.client.get, self.get_tag_uri(tag), {'op': 'machines'})
 
diff --git a/src/maasserver/models/signals/scriptresult.py b/src/maasserver/models/signals/scriptresult.py
index a3295de..4a82e71 100644
--- a/src/maasserver/models/signals/scriptresult.py
+++ b/src/maasserver/models/signals/scriptresult.py
@@ -22,30 +22,32 @@ from provisioningserver.events import EVENT_TYPES
 signals = SignalsManager()
 
 
-def emit_script_result_status_transition_event(instance, old_values, **kwargs):
+def emit_script_result_status_transition_event(
+        script_result, old_values, **kwargs):
     """Send a status transition event."""
-    script_result = instance
     [old_status] = old_values
 
+    if script_result.physical_blockdevice:
+        script_name = '%s on %s' % (
+            script_result.name, script_result.physical_blockdevice.name)
+    else:
+        script_name = script_result.name
+
     if (script_result.script_set.result_type == RESULT_TYPE.TESTING and
-            old_status == SCRIPT_STATUS.PENDING and script_result.status in (
-                SCRIPT_STATUS.INSTALLING, SCRIPT_STATUS.RUNNING)):
-            storage_name = script_result.parameters.get(
-                'storage', {}).get('value', {}).get('name')
+            old_status == SCRIPT_STATUS.PENDING and (
+                script_result.status in [
+                    SCRIPT_STATUS.INSTALLING, SCRIPT_STATUS.RUNNING])):
             Event.objects.create_node_event(
                 script_result.script_set.node, EVENT_TYPES.RUNNING_TEST,
-                event_description="%s on %s" % (
-                    script_result.name, storage_name) if storage_name else
-                script_result.name)
-
-    elif script_result.status in (
-            SCRIPT_STATUS.FAILED, SCRIPT_STATUS.TIMEDOUT,
-            SCRIPT_STATUS.ABORTED):
+                event_description=script_name)
+    elif script_result.status in [
+            SCRIPT_STATUS.FAILED, SCRIPT_STATUS.FAILED_INSTALLING,
+            SCRIPT_STATUS.TIMEDOUT, SCRIPT_STATUS.ABORTED]:
         Event.objects.create_node_event(
             script_result.script_set.node,
             EVENT_TYPES.SCRIPT_DID_NOT_COMPLETE,
             event_description="%s %s" % (
-                script_result.name, SCRIPT_STATUS_CHOICES[
+                script_name, SCRIPT_STATUS_CHOICES[
                     script_result.status][1].lower()))
     else:
         old_status_name = None
@@ -59,7 +61,7 @@ def emit_script_result_status_transition_event(instance, old_values, **kwargs):
             script_result.script_set.node,
             EVENT_TYPES.SCRIPT_RESULT_CHANGED_STATUS,
             event_description="%s changed status from '%s' to '%s'" % (
-                script_result.name, old_status_name, new_status_name))
+                script_name, old_status_name, new_status_name))
         if (CURTIN_INSTALL_LOG == script_result.name and not
                 script_result.script_set.node.netboot):
             Event.objects.create_node_event(
diff --git a/src/maasserver/models/signals/tests/test_scriptresult.py b/src/maasserver/models/signals/tests/test_scriptresult.py
index f49e8e6..d538f89 100644
--- a/src/maasserver/models/signals/tests/test_scriptresult.py
+++ b/src/maasserver/models/signals/tests/test_scriptresult.py
@@ -6,7 +6,6 @@
 __all__ = []
 
 
-import json
 import random
 
 from maasserver.models import Event
@@ -17,6 +16,7 @@ from metadataserver.enum import (
     RESULT_TYPE,
     SCRIPT_STATUS,
     SCRIPT_STATUS_CHOICES,
+    SCRIPT_TYPE,
 )
 from provisioningserver.events import (
     EVENT_DETAILS,
@@ -26,22 +26,20 @@ from provisioningserver.events import (
 
 class TestStatusTransitionEvent(MAASServerTestCase):
 
-    def test__running_or_installing_emits_event_empty_storage_parameters(self):
-
+    def test__running_or_installing_emits_event(self):
+        script = factory.make_Script(script_type=SCRIPT_TYPE.TESTING)
         script_result = factory.make_ScriptResult(
-            status=SCRIPT_STATUS.PENDING, script_set=factory.make_ScriptSet(
-                result_type=RESULT_TYPE.TESTING), script=factory.make_Script())
+            status=SCRIPT_STATUS.PENDING, script=script)
+
         script_result.status = random.choice([
             SCRIPT_STATUS.INSTALLING, SCRIPT_STATUS.RUNNING])
-        script_result.parameters = json.dumps({})
         script_result.save()
 
         latest_event = Event.objects.last()
         self.assertEqual(
             (
                 EVENT_TYPES.RUNNING_TEST,
-                EVENT_DETAILS[
-                    EVENT_TYPES.RUNNING_TEST].description,
+                EVENT_DETAILS[EVENT_TYPES.RUNNING_TEST].description,
                 script_result.name,
             ),
             (
@@ -50,30 +48,25 @@ class TestStatusTransitionEvent(MAASServerTestCase):
                 latest_event.description,
             ))
 
-    def test__running_or_installing_emits_event_with_storage_parameters(self):
-
+    def test__running_or_installing_emits_event_with_storage_parameter(self):
+        node = factory.make_Node()
+        script = factory.make_Script(script_type=SCRIPT_TYPE.TESTING)
+        script_set = factory.make_ScriptSet(
+            result_type=RESULT_TYPE.TESTING, node=node)
         script_result = factory.make_ScriptResult(
-            status=SCRIPT_STATUS.PENDING, script_set=factory.make_ScriptSet(
-                result_type=RESULT_TYPE.TESTING), script=factory.make_Script())
+            status=SCRIPT_STATUS.PENDING, script=script, script_set=script_set,
+            physical_blockdevice=node.boot_disk)
+
         script_result.status = random.choice([
             SCRIPT_STATUS.INSTALLING, SCRIPT_STATUS.RUNNING])
-        script_result.parameters = json.dumps({
-            'storage': {
-                'value': {
-                    'name': factory.make_name('name')
-                    }
-                }
-            })
         script_result.save()
 
         latest_event = Event.objects.last()
         self.assertEqual(
             (
                 EVENT_TYPES.RUNNING_TEST,
-                EVENT_DETAILS[
-                    EVENT_TYPES.RUNNING_TEST].description,
-                "%s on %s" % (script_result.name, script_result.parameters.get(
-                    'storage').get('value').get('name')),
+                EVENT_DETAILS[EVENT_TYPES.RUNNING_TEST].description,
+                '%s on %s' % (script_result.name, node.boot_disk.name)
             ),
             (
                 latest_event.type.name,
diff --git a/src/maasserver/models/tests/test_blockdevice.py b/src/maasserver/models/tests/test_blockdevice.py
index 7d5ab6a..5a0969f 100644
--- a/src/maasserver/models/tests/test_blockdevice.py
+++ b/src/maasserver/models/tests/test_blockdevice.py
@@ -1,4 +1,4 @@
-# Copyright 2014-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for `BlockDevice`."""
@@ -338,7 +338,8 @@ class TestBlockDevice(MAASServerTestCase):
         mock_get_effective_filesystem = self.patch_autospec(
             blockdevice_module, "get_effective_filesystem")
         mock_get_effective_filesystem.return_value = sentinel.filesystem
-        block_device = factory.make_BlockDevice()
+        node = factory.make_Node(with_boot_disk=False)
+        block_device = factory.make_BlockDevice(node=node)
         self.assertEqual(
             sentinel.filesystem, block_device.get_effective_filesystem())
 
diff --git a/src/maasserver/testing/factory.py b/src/maasserver/testing/factory.py
index ec327a4..c28f600 100644
--- a/src/maasserver/testing/factory.py
+++ b/src/maasserver/testing/factory.py
@@ -473,6 +473,8 @@ class Factory(maastesting.factory.Factory):
             self.make_Filesystem(
                 fstype=fstype, partition=root_partition, mount_point='/',
                 acquired=acquired)
+            node.boot_disk = root_partition.partition_table.block_device
+            node.save()
 
         # Setup the BMC connected to rack controller if a BMC is created.
         if bmc_connected_to is not None:
@@ -803,7 +805,14 @@ class Factory(maastesting.factory.Factory):
             stdout=None, stderr=None, result=None, started=None, ended=None,
             suppressed=False, **kwargs):
         if script_set is None:
-            script_set = self.make_ScriptSet()
+            if script is not None:
+                script_set_type = (
+                    RESULT_TYPE.TESTING
+                    if script.script_type == SCRIPT_TYPE.TESTING else
+                    RESULT_TYPE.COMMISSIONING)
+            else:
+                script_set_type = None
+            script_set = self.make_ScriptSet(result_type=script_set_type)
         if script is None and script_name is None:
             if script_set.result_type == RESULT_TYPE.COMMISSIONING:
                 script = self.make_Script(
diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py
index d73ed3c..5cd28fd 100644
--- a/src/maasserver/websockets/handlers/tests/test_machine.py
+++ b/src/maasserver/websockets/handlers/tests/test_machine.py
@@ -210,7 +210,7 @@ class TestMachineHandler(MAASServerTestCase):
             "actions": list(compile_node_actions(node, handler.user).keys()),
             "architecture": node.architecture,
             "bmc": node.bmc_id,
-            "boot_disk": node.boot_disk,
+            "boot_disk": node.boot_disk.id if node.boot_disk else None,
             "bios_boot_method": node.bios_boot_method,
             "commissioning_script_count": commissioning_scripts.count(),
             "commissioning_status": get_status_from_qs(
@@ -618,7 +618,7 @@ class TestMachineHandler(MAASServerTestCase):
         # number means regiond has to do more work slowing down its process
         # and slowing down the client waiting for the response.
         self.assertEqual(
-            queries, 49,
+            queries, 50,
             "Number of queries has changed; make sure this is expected.")
 
     def test_trigger_update_updates_script_result_cache(self):