launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/get-preseed into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/get-preseed/+merge/111374

This branch adds the two top-level methods in the preseed module.  These methods are the only ones that should be used by the rest of the code.

= Pre-imp =

Discussed this change with Gavin as this a requirement for the exposing the preseed files on the metadata service (work that Gavin is doing) and for exposing the preseed files in the UI (work that I'm doing).

= Notes =

The only trick is that I had to modify `get_preseed_filenames` to deal gracefully when passed a Node that is None (this is the case when rendering the enlistment preseed).
-- 
https://code.launchpad.net/~rvb/maas/get-preseed/+merge/111374
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/get-preseed into lp:maas.

=== modified file 'src/maasserver/enum.py'
--- src/maasserver/enum.py	2012-04-30 13:10:04 +0000
+++ src/maasserver/enum.py	2012-06-21 10:32:18 +0000
@@ -17,6 +17,7 @@
     'NODE_STATUS',
     'NODE_STATUS_CHOICES',
     'NODE_STATUS_CHOICES_DICT',
+    'PRESEED_TYPE',
     ]
 
 from collections import OrderedDict
@@ -113,3 +114,9 @@
     VIEW = 'view_node'
     EDIT = 'edit_node'
     ADMIN = 'admin_node'
+
+
+class PRESEED_TYPE:
+    DEFAULT = ''
+    COMMISSIONING = 'commissioning'
+    ENLIST = 'enlist'

=== modified file 'src/maasserver/preseed.py'
--- src/maasserver/preseed.py	2012-06-20 16:27:58 +0000
+++ src/maasserver/preseed.py	2012-06-21 10:32:18 +0000
@@ -10,7 +10,10 @@
     )
 
 __metaclass__ = type
-__all__ = []
+__all__ = [
+    'get_enlist_preseed',
+    'get_preseed',
+    ]
 
 from collections import namedtuple
 from os.path import join
@@ -19,6 +22,10 @@
 from urlparse import urlparse
 
 from django.conf import settings
+from maasserver.enum import (
+    NODE_STATUS,
+    PRESEED_TYPE,
+    )
 from maasserver.models import Config
 from maasserver.provisioning import compose_preseed
 from maasserver.utils import absolute_reverse
@@ -28,6 +35,35 @@
 GENERIC_FILENAME = 'generic'
 
 
+def get_enlist_preseed():
+    """Return the enlistment preseed.
+
+    :return: The rendered preseed string.
+    :rtype: basestring.
+    """
+    return render_preseed(None, PRESEED_TYPE.ENLIST)
+
+
+# XXX: rvb 2012-06-21 bug=1013146:  'precise' is hardcoded here.
+def get_preseed(node, release="precise"):
+    """Return the preseed for a given node.  Depending on the node's status
+    this will be a commissioning preseed (if the node is commissioning) or the
+    standard preseed (normal installation preseed).
+
+    :param node: The node to return preseed for.
+    :type node: :class:`maasserver.models.Node`
+    :param release: The Ubuntu release to be used.
+    :type release: basestring
+    :return: The rendered preseed string.
+    :rtype: basestring.
+    """
+    if node.status == NODE_STATUS.COMMISSIONING:
+        return render_preseed(
+            node, PRESEED_TYPE.COMMISSIONING, release=release)
+    else:
+        return render_preseed(node, PRESEED_TYPE.DEFAULT, release=release)
+
+
 # XXX: rvb 2012-06-14 bug=1013146:  'precise' is hardcoded here.
 def get_preseed_filenames(node, prefix='', release='precise', default=False):
     """List possible preseed template filenames for the given node.
@@ -52,11 +88,19 @@
     {prefix}
     'generic'
     """
-    arch = split_subarch(node.architecture)
     elements = []
+    # Add prefix.
     if prefix != '':
         elements.append(prefix)
-    elements.extend(arch + [release, node.hostname])
+    # Add architecture/sub-architecture.
+    if node is not None:
+        arch = split_subarch(node.architecture)
+        elements.extend(arch)
+    # Add release.
+    elements.append(release)
+    # Add hostname.
+    if node is not None:
+        elements.append(node.hostname)
     while elements:
         yield compose_filename(elements)
         elements.pop()
@@ -165,19 +209,25 @@
     :rtype: dict.
     """
     server_host = get_maas_server_host()
-    # Create the url and the url-data (POST parameters) used to turn off
-    # PXE booting once the install of the node is finished.
-    node_disable_pxe_url = absolute_reverse(
-        'metadata-anon-node-edit', args=['latest', node.system_id])
-    node_disable_pxe_data = urlencode({'op': 'netboot_off'})
-    return {
-        'node': node,
+    context = {
         'release': release,
         'server_host': server_host,
-        'preseed_data': compose_preseed(node),
-        'node_disable_pxe_url': node_disable_pxe_url,
-        'node_disable_pxe_data': node_disable_pxe_data,
-    }
+        }
+    if node is not None:
+        # Create the url and the url-data (POST parameters) used to turn off
+        # PXE booting once the install of the node is finished.
+        node_disable_pxe_url = absolute_reverse(
+            'metadata-anon-node-edit', args=['latest', node.system_id])
+        node_disable_pxe_data = urlencode({'op': 'netboot_off'})
+        node_context = {
+            'node': node,
+            'preseed_data': compose_preseed(node),
+            'node_disable_pxe_url': node_disable_pxe_url,
+            'node_disable_pxe_data': node_disable_pxe_data,
+        }
+        context.update(node_context)
+
+    return context
 
 
 # XXX: rvb 2012-06-19 bug=1013146:  'precise' is hardcoded here.

=== modified file 'src/maasserver/tests/test_preseed.py'
--- src/maasserver/tests/test_preseed.py	2012-06-20 16:22:08 +0000
+++ src/maasserver/tests/test_preseed.py	2012-06-21 10:32:18 +0000
@@ -16,10 +16,16 @@
 from pipes import quote
 
 from django.conf import settings
+from maasserver.enum import (
+    NODE_STATUS,
+    PRESEED_TYPE,
+    )
 from maasserver.models import Config
 from maasserver.preseed import (
     GENERIC_FILENAME,
+    get_enlist_preseed,
     get_maas_server_host,
+    get_preseed,
     get_preseed_context,
     get_preseed_filenames,
     get_preseed_template,
@@ -31,6 +37,7 @@
     )
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import TestCase
+from maasserver.utils import map_enum
 from testtools.matchers import (
     AllMatch,
     IsInstance,
@@ -87,6 +94,16 @@
             ],
             list(get_preseed_filenames(node, prefix, release, default=True)))
 
+    def test_get_preseed_filenames_if_node_is_None(self):
+        release = factory.getRandomString()
+        prefix = factory.getRandomString()
+        self.assertSequenceEqual(
+            [
+                '%s_%s' % (prefix, release),
+                '%s' % prefix,
+            ],
+            list(get_preseed_filenames(None, prefix, release)))
+
     def test_get_preseed_filenames_supports_empty_prefix(self):
         hostname = factory.getRandomString()
         release = factory.getRandomString()
@@ -308,6 +325,16 @@
              'node_disable_pxe_url', 'node_disable_pxe_data'],
             context)
 
+    def test_get_preseed_context_if_node_None(self):
+        # If the provided Node is None (when're in the context of an
+        # enlistment preseed) the returned context does not include the
+        # node context.
+        release = factory.getRandomString()
+        context = get_preseed_context(None, release)
+        self.assertItemsEqual(
+            ['release', 'server_host'],
+            context)
+
 
 class TestPreseedTemplate(TestCase):
     """Tests for class:`PreseedTemplate`."""
@@ -323,20 +350,38 @@
     """Tests for `render_preseed`.
 
     These tests check that the templates render (i.e. that no variable is
-    missing) and 'look right'.
-    """
-
-    def test_render_default_preseed(self):
-        node = factory.make_node()
-        preseed = render_preseed(node, factory.getRandomString(), "precise")
+    missing).
+    """
+
+    # Create a scenario for each possible value of PRESEED_TYPE.
+    scenarios = [
+        (name, {'preseed': value})
+        for name, value in map_enum(PRESEED_TYPE).items()]
+
+    def test_render_preseed(self):
+        node = factory.make_node()
+        preseed = render_preseed(node, self.preseed, "precise")
+        # The test really is that the preseed is rendered without an
+        # error.
+        self.assertIsInstance(preseed, basestring)
+
+
+class TestPreseedMethods(TestCase):
+    """Tests for `get_enlist_preseed` and `get_preseed`.
+
+    These tests check that the preseed templates render and 'look right'.
+    """
+
+    def test_get_preseed_returns_default_preseed(self):
+        node = factory.make_node()
+        preseed = get_preseed(node)
         self.assertIn('preseed/late_command', preseed)
 
-    def test_render_enlist_preseed(self):
-        node = factory.make_node()
-        preseed = render_preseed(node, 'enlist', "precise")
+    def test_get_enlist_preseed_returns_enlist_preseed(self):
+        preseed = get_enlist_preseed()
         self.assertIn('maas-enlist-udeb', preseed)
 
-    def test_render_commissioning_preseed(self):
-        node = factory.make_node()
-        preseed = render_preseed(node, 'commissioning', "precise")
+    def test_get_preseed_returns_commissioning_preseed(self):
+        node = factory.make_node(status=NODE_STATUS.COMMISSIONING)
+        preseed = get_preseed(node)
         self.assertIn('cloud-init', preseed)