Merge ~d0ugal/maas:scriptset-refactor into maas:master

[Dougal Matthews]

Refactor of the script selection code

[Dougal Matthews (d0ugal)]

jenkins: !test

[MAAS Lander (maas-lander)]

UNIT TESTS
-b scriptset-refactor lp:~d0ugal/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/8753/console
COMMIT: dbaee7fb1b7a3e9d3d14c3d4a285a3d73503a6d1

review: Needs Fixing

[Dougal Matthews (d0ugal)]

jenkins: !test

[MAAS Lander (maas-lander)]

UNIT TESTS
-b scriptset-refactor lp:~d0ugal/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: cbc9f743ff734b723862eb18da3339ed773e71d3

review: Approve

[Alberto Donato (ack)]

Nice cleanup, +1

A couple of nits inline

review: Approve

4a48bce... by Dougal Matthews on 2020-12-01

Updates from feedback

diff --git a/src/metadataserver/models/scriptset.py b/src/metadataserver/models/scriptset.py
index d18027c..4e8983d 100644
--- a/src/metadataserver/models/scriptset.py
+++ b/src/metadataserver/models/scriptset.py
@@ -45,6 +45,19 @@ from provisioningserver.events import EVENT_TYPES
 from provisioningserver.refresh.node_info_scripts import NODE_INFO_SCRIPTS
 
 
+def _get_hw_pairs(script_set):
+    """
+    Given a ScriptSet return hardware pairs as a list of "key:value" strings.
+    """
+    hw_pairs_qs = script_set.node.nodemetadata_set.filter(
+        Q(key__startswith="system_")
+        | Q(key__startswith="mainboard_")
+        | Q(key__startswith="firmware_")
+        | Q(key__startswith="chassis_")
+    ).values_list("key", "value")
+    return [":".join(pair) for pair in hw_pairs_qs]
+
+
 def get_status_from_qs(qs):
     """Given a QuerySet or list of ScriptResults return the set's status."""
     # If no tests have been run the QuerySet or list has no status.
@@ -207,6 +220,22 @@ class ScriptSetManager(Manager):
         self._clean_old(node, RESULT_TYPE.INSTALLATION, script_set)
         return script_set
 
+    def _find_scripts(self, script_qs, hw_pairs, modaliases):
+        for script in script_qs:
+            # If a script is not for specific hardware, it is always included
+            if not script.for_hardware:
+                yield script
+                continue
+
+            # If a script with the for_hardware field is selected by tag only
+            # add it if matching hardware is found.
+            for hardware in script.for_hardware:
+                if hardware in hw_pairs:
+                    yield script
+                    break
+            if filter_modaliases(modaliases, *script.ForHardware):
+                yield script
+
     def _add_user_selected_scripts(
         self, script_set, scripts=None, script_input=None
     ):
@@ -226,27 +255,9 @@ class ScriptSetManager(Manager):
             Q(name__in=scripts) | Q(tags__overlap=scripts) | Q(id__in=ids),
             script_type=script_type,
         )
+        hw_pairs = _get_hw_pairs(script_set)
         modaliases = script_set.node.modaliases
-        hw_pairs_qs = script_set.node.nodemetadata_set.filter(
-            Q(key__startswith="system_")
-            | Q(key__startswith="mainboard_")
-            | Q(key__startswith="firmware_")
-            | Q(key__startswith="chassis_")
-        ).values_list("key", "value")
-        hw_pairs = [":".join(t) for t in hw_pairs_qs]
-        for script in qs:
-            # If a script with the for_hardware field is selected by tag only
-            # add it if matching hardware is found.
-            if script.for_hardware:
-                found_hw_match = False
-                if hw_pairs:
-                    for hardware in script.for_hardware:
-                        if hardware in hw_pairs:
-                            found_hw_match = True
-                            break
-                matches = filter_modaliases(modaliases, *script.ForHardware)
-                if len(matches) == 0 and not found_hw_match:
-                    continue
+        for script in self._find_scripts(qs, hw_pairs, modaliases):
             try:
                 script_set.add_pending_script(script, script_input)
             except ValidationError:
@@ -464,14 +475,7 @@ class ScriptSet(CleanSave, Model):
 
         if modaliases is None:
             modaliases = self.node.modaliases
-
-        hw_pairs_qs = self.node.nodemetadata_set.filter(
-            Q(key__startswith="system_")
-            | Q(key__startswith="mainboard_")
-            | Q(key__startswith="firmware_")
-            | Q(key__startswith="chassis_")
-        ).values_list("key", "value")
-        hw_pairs = [":".join(t) for t in hw_pairs_qs]
+        hw_pairs = _get_hw_pairs(self)
 
         # Remove scripts autoselected at the start of commissioning but updated
         # commissioning data shows the Script is no longer applicable.