Merge ~ack/maas:1976516-scriptresult-update-3.2 into maas:3.2

[Alberto Donato]

LP:1976516 link Script to ScriptResult when creating a new one

This was causing no script to be returned for LXD VM hosts that are not deployed by MAAS.
It also fixes commissioning data sent back from the refresh operation on LXD VM hosts to return data for both resources script (to match what regular commissioning does).

[Alberto Donato (ack)]

backport of 23bcfa88d from master to 3.2

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b 1976516-scriptresult-update-3.2 lp:~ack/maas/+git/maas into -b 3.2 lp:~maas-committers/maas

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

[MAAS Lander (maas-lander)]

UNIT TESTS
-b 1976516-scriptresult-update-3.2 lp:~ack/maas/+git/maas into -b 3.2 lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: ff1f55a7fb3e23e94fa1aeffb3083e035ec9dbe8

review: Approve

diff --git a/src/metadataserver/api.py b/src/metadataserver/api.py
index fce2220..5c4ad54 100644
--- a/src/metadataserver/api.py
+++ b/src/metadataserver/api.py
@@ -287,19 +287,21 @@ def process_file(
     )
     if script_result is None:
         # If the script_result_id doesn't exist or wasn't sent try to find the
-        # ScriptResult by script_name. Since ScriptResults can get their name
-        # from the Script they are linked to or its own script_name field we
-        # have to iterate over the list of script_results.
-        script_result_found = False
-        for script_result in script_set:
-            if script_result.name == script_name:
-                script_result_found = True
-                break
+        # ScriptResult by script_name.
+        script_result = script_set.scriptresult_set.filter(
+            script_name=script_name
+        ).first()
 
         # If the ScriptResult wasn't found by id or name create an entry for
         # it.
-        if not script_result_found:
+        if not script_result:
+            script_id = (
+                Script.objects.filter(name=script_name)
+                .values_list("id", flat=True)
+                .first()
+            )
             script_result = ScriptResult.objects.create(
+                script_id=script_id,
                 script_set=script_set,
                 script_name=script_name,
                 status=SCRIPT_STATUS.RUNNING,
@@ -1318,7 +1320,7 @@ class MAASScriptsHandler(OperationsHandler):
     anonymous = AnonMAASScriptsHandler
 
     def _add_script_set_to_tar(
-        self, script_set, tar, prefix, mtime, include_finshed=False
+        self, script_set, tar, prefix, mtime, include_finished=False
     ):
         if script_set is None:
             return []
@@ -1326,7 +1328,7 @@ class MAASScriptsHandler(OperationsHandler):
         for script_result in script_set:
             # Don't rerun Scripts which have already run.
             if (
-                not include_finshed
+                not include_finished
                 and script_result.status
                 not in SCRIPT_STATUS_RUNNING_OR_PENDING
             ):
@@ -1425,9 +1427,11 @@ class MAASScriptsHandler(OperationsHandler):
                 )
                 and node.current_commissioning_script_set is not None
             ):
+                script_set = node.current_commissioning_script_set
                 # Prefetch all the data we need.
-                qs = node.current_commissioning_script_set.scriptresult_set
-                qs = qs.select_related("script", "script__script")
+                qs = script_set.scriptresult_set.select_related(
+                    "script", "script__script"
+                )
                 # After the script runner finishes sending all commissioning
                 # results it redownloads the script tar. It does this in-case
                 # a commissioning script discovers hardware associated with
@@ -1439,7 +1443,6 @@ class MAASScriptsHandler(OperationsHandler):
                 # data.
                 for script_result in qs:
                     if script_result.status != SCRIPT_STATUS.PENDING:
-                        script_set = node.current_commissioning_script_set
                         script_set.select_for_hardware_scripts()
                         break
                 meta_data = self._add_script_set_to_tar(
@@ -1447,9 +1450,9 @@ class MAASScriptsHandler(OperationsHandler):
                     tar,
                     "commissioning",
                     mtime,
-                    include_finshed=node.status == NODE_STATUS.DEPLOYED,
+                    include_finished=node.status == NODE_STATUS.DEPLOYED,
                 )
-                if meta_data != []:
+                if meta_data:
                     tar_meta_data["commissioning_scripts"] = sorted(
                         meta_data, key=itemgetter("name", "script_result_id")
                     )
@@ -1457,12 +1460,13 @@ class MAASScriptsHandler(OperationsHandler):
             # Always send testing scripts.
             if node.current_testing_script_set is not None:
                 # prefetch all the data we need
-                qs = node.current_testing_script_set.scriptresult_set
-                qs = qs.select_related("script", "script__script")
+                qs = node.current_testing_script_set.scriptresult_set.select_related(
+                    "script", "script__script"
+                )
                 meta_data = self._add_script_set_to_tar(
                     qs, tar, "testing", mtime
                 )
-                if meta_data != []:
+                if meta_data:
                     tar_meta_data["testing_scripts"] = sorted(
                         meta_data, key=itemgetter("name", "script_result_id")
                     )
diff --git a/src/metadataserver/migrations/0030_scriptresult_script_link.py b/src/metadataserver/migrations/0030_scriptresult_script_link.py
new file mode 100644
index 0000000..a2827da
--- /dev/null
+++ b/src/metadataserver/migrations/0030_scriptresult_script_link.py
@@ -0,0 +1,66 @@
+# Generated by Django 2.2.12 on 2022-06-02 15:09
+
+from django.db import migrations
+from django.db.models import F, OuterRef, Subquery
+
+
+def scriptresult_fix_script_link(apps, schema_editor):
+    """Link ScriptResult to Script with matching name, if any."""
+    ScriptResult = apps.get_model("metadataserver", "ScriptResult")
+    Script = apps.get_model("metadataserver", "Script")
+    (
+        ScriptResult.objects.filter(script__isnull=True)
+        .annotate(
+            new_script_id=Subquery(
+                Script.objects.filter(name=OuterRef("script_name")).values(
+                    "id"
+                )[:1]
+            )
+        )
+        .update(script_id=F("new_script_id"))
+    )
+
+
+RUN_MACHINE_RESOURCES = "20-maas-03-machine-resources"
+COMMISSIONING_OUTPUT_NAME = "50-maas-01-commissioning"
+
+
+def add_run_machine_resources_scriptresult(apps, schema_editor):
+    """Create missing script results for machine-resources output.
+
+    This duplicates the output from the commissioning output when the former
+    script is not present (for LXD VM hosts).
+    """
+    ScriptResult = apps.get_model("metadataserver", "ScriptResult")
+    Script = apps.get_model("metadataserver", "Script")
+    run_resources_script = (
+        Script.objects.prefetch_related("script")
+        .filter(name=RUN_MACHINE_RESOURCES)
+        .first()
+    )
+    if not run_resources_script:
+        return
+
+    script_results = ScriptResult.objects.filter(
+        script_name=COMMISSIONING_OUTPUT_NAME
+    ).exclude(script_set__scriptresult__script_name=RUN_MACHINE_RESOURCES)
+    for script_result in script_results:
+        # create a new result with the same content, pointing to the right
+        # script
+        script_result.id = None
+        script_result.script_name = RUN_MACHINE_RESOURCES
+        script_result.script = run_resources_script
+        script_result.script_version = run_resources_script.script
+        script_result.save(force_insert=True)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("metadataserver", "0029_scriptset_tags_cleanup"),
+    ]
+
+    operations = [
+        migrations.RunPython(scriptresult_fix_script_link),
+        migrations.RunPython(add_run_machine_resources_scriptresult),
+    ]
diff --git a/src/metadataserver/tests/test_api.py b/src/metadataserver/tests/test_api.py
index 8efb8b7..a020b2e 100644
--- a/src/metadataserver/tests/test_api.py
+++ b/src/metadataserver/tests/test_api.py
@@ -629,6 +629,23 @@ class TestHelpers(MAASServerTestCase):
             {"exit_status": request["exit_status"], "output": output}, value
         )
 
+    def test_creates_new_script_result_linked_to_script(self):
+        results = {}
+        script = factory.make_Script()
+        script_set = factory.make_ScriptSet()
+        output = factory.make_string()
+        request = {"exit_status": random.randint(0, 255)}
+
+        process_file(results, script_set, script.name, output, request)
+        script_result, value = list(results.items())[0]
+
+        self.assertEqual(script.name, script_result.name)
+        self.assertEqual(script_result.script, script)
+        self.assertEqual(SCRIPT_STATUS.RUNNING, script_result.status)
+        self.assertDictEqual(
+            {"exit_status": request["exit_status"], "output": output}, value
+        )
+
     def test_uses_default_exit_status_when_undef(self):
         results = {}
         script_result = factory.make_ScriptResult(status=SCRIPT_STATUS.RUNNING)
diff --git a/src/provisioningserver/drivers/pod/lxd.py b/src/provisioningserver/drivers/pod/lxd.py
index 9c02b86..0265d4f 100644
--- a/src/provisioningserver/drivers/pod/lxd.py
+++ b/src/provisioningserver/drivers/pod/lxd.py
@@ -44,6 +44,7 @@ from provisioningserver.logger import get_maas_logger
 from provisioningserver.prometheus.metrics import PROMETHEUS_METRICS
 from provisioningserver.refresh.node_info_scripts import (
     COMMISSIONING_OUTPUT_NAME,
+    RUN_MACHINE_RESOURCES,
 )
 from provisioningserver.rpc.exceptions import PodInvalidResources
 from provisioningserver.utils import (
@@ -427,7 +428,11 @@ class LXDPodDriver(PodDriver):
                 # /1.0/networks/<network>/state
                 "networks": _get_lxd_network_states(client),
             }
-        return {COMMISSIONING_OUTPUT_NAME: resources}
+        # return the output for both scripts to match what commissioning does
+        return {
+            RUN_MACHINE_RESOURCES: resources,
+            COMMISSIONING_OUTPUT_NAME: resources,
+        }
 
     @threadDeferred
     def compose(self, pod_id: int, context: dict, request: RequestedMachine):
diff --git a/src/provisioningserver/drivers/pod/tests/test_lxd.py b/src/provisioningserver/drivers/pod/tests/test_lxd.py
index c98abcb..45f0951 100644
--- a/src/provisioningserver/drivers/pod/tests/test_lxd.py
+++ b/src/provisioningserver/drivers/pod/tests/test_lxd.py
@@ -33,6 +33,7 @@ from provisioningserver.drivers.pod import (
 from provisioningserver.drivers.pod import lxd as lxd_module
 from provisioningserver.refresh.node_info_scripts import (
     COMMISSIONING_OUTPUT_NAME,
+    RUN_MACHINE_RESOURCES,
 )
 from provisioningserver.rpc.exceptions import PodInvalidResources
 from provisioningserver.testing.certificates import (
@@ -1159,18 +1160,20 @@ class TestLXDPodDriver(MAASTestCase):
             1, self.make_context()
         )
 
+        details = {
+            **self.fake_lxd.host_info,
+            "resources": self.fake_lxd.resources,
+            "networks": {
+                "eth0": {"hwaddr": "aa:bb:cc:dd:ee:ff"},
+                "eth1": {"hwaddr": "ff:ee:dd:cc:bb:aa"},
+            },
+        }
         self.assertEqual(
+            commissioning_data,
             {
-                COMMISSIONING_OUTPUT_NAME: {
-                    **self.fake_lxd.host_info,
-                    "resources": self.fake_lxd.resources,
-                    "networks": {
-                        "eth0": {"hwaddr": "aa:bb:cc:dd:ee:ff"},
-                        "eth1": {"hwaddr": "ff:ee:dd:cc:bb:aa"},
-                    },
-                }
+                RUN_MACHINE_RESOURCES: details,
+                COMMISSIONING_OUTPUT_NAME: details,
             },
-            commissioning_data,
         )
 
     @inlineCallbacks
@@ -1205,17 +1208,19 @@ class TestLXDPodDriver(MAASTestCase):
             1, self.make_context()
         )
 
+        details = {
+            **self.fake_lxd.host_info,
+            "resources": self.fake_lxd.resources,
+            "networks": {
+                "eth2": {"hwaddr": "ee:dd:cc:bb:aa:ff"},
+            },
+        }
         self.assertEqual(
+            commissioning_data,
             {
-                COMMISSIONING_OUTPUT_NAME: {
-                    **self.fake_lxd.host_info,
-                    "resources": self.fake_lxd.resources,
-                    "networks": {
-                        "eth2": {"hwaddr": "ee:dd:cc:bb:aa:ff"},
-                    },
-                }
+                RUN_MACHINE_RESOURCES: details,
+                COMMISSIONING_OUTPUT_NAME: details,
             },
-            commissioning_data,
         )
 
     @inlineCallbacks