Merge lp:~newell-jensen/maas/move-get-boot-purpose-to-node-model into lp:~maas-committers/maas/trunk

[Newell Jensen]

Moving get_boot_purpose from pxeconfig to the node model.

This change is so that we can get the boot purpose from the node itself instead of from pxeconfig. This work is related to:

https://bugs.launchpad.net/maas/+bug/1371122

Ultimately these changes are so that we can get PXE Request events without WindowsBootMethod hitting the API for every file, as mentioned in the bug.

[Jeroen T. Vermeulen (jtv)]

Looking good. A bit of a shame to see the Node model module continue to grow: we've found the edge of manageability for a single source file to lie somewhere near the thousand-line mark. But yes, this code does look as if it belongs there.

review: Approve

=== modified file 'src/maasserver/api/pxeconfig.py'
--- src/maasserver/api/pxeconfig.py	2014-09-18 03:15:07 +0000
+++ src/maasserver/api/pxeconfig.py	2014-09-24 02:34:00 +0000
@@ -27,10 +27,7 @@
     get_optional_param,
     )
 from maasserver.clusterrpc.boot_images import get_boot_images_for
-from maasserver.enum import (
-    NODE_STATUS,
-    PRESEED_TYPE,
-    )
+from maasserver.enum import NODE_STATUS
 from maasserver.models import (
     BootResource,
     Config,
@@ -40,7 +37,6 @@
 from maasserver.preseed import (
     compose_enlistment_preseed_url,
     compose_preseed_url,
-    get_preseed_type_for,
     )
 from maasserver.server_address import get_maas_facing_server_address
 from maasserver.third_party_drivers import get_third_party_driver
@@ -82,36 +78,6 @@
     return macaddress.node if macaddress else None
 
 
-def get_boot_purpose(node):
-    """Return a suitable "purpose" for this boot, e.g. "install"."""
-    # XXX: allenap bug=1031406 2012-07-31: The boot purpose is still in
-    # flux. It may be that there will just be an "ephemeral" environment and
-    # an "install" environment, and the differing behaviour between, say,
-    # enlistment and commissioning - both of which will use the "ephemeral"
-    # environment - will be governed by varying the preseed or PXE
-    # configuration.
-    if node is None:
-        # This node is enlisting, for which we use a commissioning image.
-        return "commissioning"
-    elif node.status == NODE_STATUS.COMMISSIONING:
-        # It is commissioning.
-        return "commissioning"
-    elif node.status == NODE_STATUS.DEPLOYING:
-        # Install the node if netboot is enabled, otherwise boot locally.
-        if node.netboot:
-            preseed_type = get_preseed_type_for(node)
-            if preseed_type == PRESEED_TYPE.CURTIN:
-                return "xinstall"
-            else:
-                return "install"
-        else:
-            return "local"
-    elif node.status == NODE_STATUS.DEPLOYED:
-        return "local"
-    else:
-        return "poweroff"
-
-
 def get_boot_image(
         nodegroup, osystem, architecture, subarchitecture, series, purpose):
     """Obtain the first available boot image for this cluster for the given
@@ -214,7 +180,10 @@
 
     # If we are booting with "xinstall", then we should always return the
     # commissioning operating system and distro_series.
-    purpose = get_boot_purpose(node)
+    if node is None:
+        purpose = "commissioning"  # enlistment
+    else:
+        purpose = node.get_boot_purpose()
     if purpose == "xinstall":
         osystem = Config.objects.get_config('commissioning_osystem')
         series = Config.objects.get_config('commissioning_distro_series')

=== modified file 'src/maasserver/api/tests/test_pxeconfig.py'
--- src/maasserver/api/tests/test_pxeconfig.py	2014-09-20 15:42:12 +0000
+++ src/maasserver/api/tests/test_pxeconfig.py	2014-09-24 02:34:00 +0000
@@ -28,12 +28,10 @@
 from maasserver.api.pxeconfig import (
     find_nodegroup_for_pxeconfig_request,
     get_boot_image,
-    get_boot_purpose,
     )
 from maasserver.clusterrpc.testing.boot_images import make_rpc_boot_image
 from maasserver.enum import (
     BOOT_RESOURCE_TYPE,
-    NODE_BOOT,
     NODE_STATUS,
     NODEGROUPINTERFACE_MANAGEMENT,
     )
@@ -47,7 +45,6 @@
     )
 from maasserver.testing.architecture import make_usable_architecture
 from maasserver.testing.factory import factory
-from maasserver.testing.osystems import make_usable_osystem
 from maasserver.testing.testcase import MAASServerTestCase
 from maastesting.fakemethod import FakeMethod
 from mock import sentinel
@@ -367,60 +364,13 @@
             json.loads(response.content)["preseed_url"],
             StartsWith(ng_url))
 
-    def test_get_boot_purpose_unknown_node(self):
-        # A node that's not yet known to MAAS is assumed to be enlisting,
-        # which uses a "commissioning" image.
-        self.assertEqual("commissioning", get_boot_purpose(None))
-
-    def test_get_boot_purpose_known_node(self):
-        # The following table shows the expected boot "purpose" for each set
-        # of node parameters.
-        options = [
-            ("poweroff", {"status": NODE_STATUS.NEW}),
-            ("commissioning", {"status": NODE_STATUS.COMMISSIONING}),
-            ("poweroff", {"status": NODE_STATUS.FAILED_COMMISSIONING}),
-            ("poweroff", {"status": NODE_STATUS.MISSING}),
-            ("poweroff", {"status": NODE_STATUS.READY}),
-            ("poweroff", {"status": NODE_STATUS.RESERVED}),
-            ("install", {"status": NODE_STATUS.DEPLOYING, "netboot": True}),
-            ("xinstall", {"status": NODE_STATUS.DEPLOYING, "netboot": True}),
-            ("local", {"status": NODE_STATUS.DEPLOYING, "netboot": False}),
-            ("local", {"status": NODE_STATUS.DEPLOYED}),
-            ("poweroff", {"status": NODE_STATUS.RETIRED}),
-            ]
-        node = factory.make_Node(boot_type=NODE_BOOT.DEBIAN)
-        mock_get_boot_images_for = self.patch(
-            preseed_module, 'get_boot_images_for')
-        for purpose, parameters in options:
-            boot_image = make_rpc_boot_image(purpose=purpose)
-            mock_get_boot_images_for.return_value = [boot_image]
-            if purpose == "xinstall":
-                node.boot_type = NODE_BOOT.FASTPATH
-            for name, value in parameters.items():
-                setattr(node, name, value)
-            self.assertEqual(purpose, get_boot_purpose(node))
-
-    def test_get_boot_purpose_osystem_no_xinstall_support(self):
-        osystem = make_usable_osystem(self)
-        release = osystem['default_release']
-        node = factory.make_Node(
-            status=NODE_STATUS.DEPLOYING, netboot=True,
-            osystem=osystem['name'], distro_series=release,
-            boot_type=NODE_BOOT.FASTPATH)
-        boot_image = make_rpc_boot_image(purpose='install')
-        self.patch(
-            preseed_module, 'get_boot_images_for').return_value = [boot_image]
-        self.assertEqual('install', get_boot_purpose(node))
-
-    def test_pxeconfig_uses_boot_purpose(self):
-        fake_boot_purpose = factory.make_name("purpose")
-        self.patch(
-            pxeconfig_module, "get_boot_purpose"
-            ).return_value = fake_boot_purpose
+    def test_pxeconfig_uses_boot_purpose_enlistment(self):
+        # test that purpose is set to "commissioning" for
+        # enlistment (when node is None).
         params = self.get_default_params()
         response = self.client.get(reverse('pxeconfig'), params)
         self.assertEqual(
-            fake_boot_purpose,
+            "commissioning",
             json.loads(response.content)["purpose"])
 
     def test_pxeconfig_returns_fs_host_as_cluster_controller(self):

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2014-09-23 18:04:10 +0000
+++ src/maasserver/models/node.py	2014-09-24 02:34:00 +0000
@@ -70,6 +70,7 @@
     NODEGROUPINTERFACE_MANAGEMENT,
     POWER_STATE,
     POWER_STATE_CHOICES,
+    PRESEED_TYPE,
     )
 from maasserver.exceptions import (
     NodeStateViolation,
@@ -97,6 +98,7 @@
     is_failed_status,
     NODE_TRANSITIONS,
     )
+from maasserver.preseed import get_preseed_type_for
 from maasserver.rpc import getClientFor
 from maasserver.utils import (
     get_db_state,
@@ -1331,3 +1333,33 @@
             # Return a list instead of yielding mappings as they're ready
             # because it's all-or-nothing (hence the atomic context).
             return mappings
+
+    def get_boot_purpose(self):
+        """
+        Return a suitable "purpose" for this boot, e.g. "install".
+        """
+        # XXX: allenap bug=1031406 2012-07-31: The boot purpose is
+        # still in flux. It may be that there will just be an
+        # "ephemeral" environment and an "install" environment, and
+        # the differing behaviour between, say, enlistment and
+        # commissioning - both of which will use the "ephemeral"
+        # environment - will be governed by varying the preseed or PXE
+        # configuration.
+        if self.status == NODE_STATUS.COMMISSIONING:
+            # It is commissioning.
+            return "commissioning"
+        elif self.status == NODE_STATUS.DEPLOYING:
+            # Install the node if netboot is enabled,
+            # otherwise boot locally.
+            if self.netboot:
+                preseed_type = get_preseed_type_for(self)
+                if preseed_type == PRESEED_TYPE.CURTIN:
+                    return "xinstall"
+                else:
+                    return "install"
+            else:
+                return "local"
+        elif self.status == NODE_STATUS.DEPLOYED:
+            return "local"
+        else:
+            return "poweroff"

=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py	2014-09-23 18:04:10 +0000
+++ src/maasserver/models/tests/test_node.py	2014-09-24 02:34:00 +0000
@@ -23,7 +23,9 @@
 
 import crochet
 from django.core.exceptions import ValidationError
+from maasserver import preseed as preseed_module
 from maasserver.clusterrpc.power_parameters import get_power_types
+from maasserver.clusterrpc.testing.boot_images import make_rpc_boot_image
 from maasserver.dns import config as dns_config
 from maasserver.enum import (
     IPADDRESS_TYPE,
@@ -68,6 +70,7 @@
     )
 from maasserver.testing.factory import factory
 from maasserver.testing.orm import reload_object
+from maasserver.testing.osystems import make_usable_osystem
 from maasserver.testing.testcase import MAASServerTestCase
 from maasserver.utils import ignore_unused
 from maastesting.djangotestcase import count_queries
@@ -1337,6 +1340,46 @@
         node = reload_object(node)
         self.assertEqual(status, node.status)
 
+    def test_get_boot_purpose_known_node(self):
+        # The following table shows the expected boot "purpose" for each set
+        # of node parameters.
+        options = [
+            ("poweroff", {"status": NODE_STATUS.NEW}),
+            ("commissioning", {"status": NODE_STATUS.COMMISSIONING}),
+            ("poweroff", {"status": NODE_STATUS.FAILED_COMMISSIONING}),
+            ("poweroff", {"status": NODE_STATUS.MISSING}),
+            ("poweroff", {"status": NODE_STATUS.READY}),
+            ("poweroff", {"status": NODE_STATUS.RESERVED}),
+            ("install", {"status": NODE_STATUS.DEPLOYING, "netboot": True}),
+            ("xinstall", {"status": NODE_STATUS.DEPLOYING, "netboot": True}),
+            ("local", {"status": NODE_STATUS.DEPLOYING, "netboot": False}),
+            ("local", {"status": NODE_STATUS.DEPLOYED}),
+            ("poweroff", {"status": NODE_STATUS.RETIRED}),
+            ]
+        node = factory.make_Node(boot_type=NODE_BOOT.DEBIAN)
+        mock_get_boot_images_for = self.patch(
+            preseed_module, 'get_boot_images_for')
+        for purpose, parameters in options:
+            boot_image = make_rpc_boot_image(purpose=purpose)
+            mock_get_boot_images_for.return_value = [boot_image]
+            if purpose == "xinstall":
+                node.boot_type = NODE_BOOT.FASTPATH
+            for name, value in parameters.items():
+                setattr(node, name, value)
+            self.assertEqual(purpose, node.get_boot_purpose())
+
+    def test_get_boot_purpose_osystem_no_xinstall_support(self):
+        osystem = make_usable_osystem(self)
+        release = osystem['default_release']
+        node = factory.make_Node(
+            status=NODE_STATUS.DEPLOYING, netboot=True,
+            osystem=osystem['name'], distro_series=release,
+            boot_type=NODE_BOOT.FASTPATH)
+        boot_image = make_rpc_boot_image(purpose='install')
+        self.patch(
+            preseed_module, 'get_boot_images_for').return_value = [boot_image]
+        self.assertEqual('install', node.get_boot_purpose())
+
 
 class NodeRoutersTest(MAASServerTestCase):