launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/maas/pxe-templates into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~julian-edwards/maas/pxe-templates/+merge/110949

After a quick pre-imp with Raphers we agreed that the class-encapsulating-template meme is the right way to go for things needing a template.  In this case it's an encapsulation of PXE config files.

The branch adds a PXEConfig class that writes out files in the right location (which depends on the architecture of the node) and contents that come from a new template.

TODO: the directory locations are deliberately in the celeryconfig because I may move the whole thing to a celery Task at some point.
-- 
https://code.launchpad.net/~julian-edwards/maas/pxe-templates/+merge/110949
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/maas/pxe-templates into lp:maas.

=== modified file 'etc/celeryconfig.py'
--- etc/celeryconfig.py	2012-05-21 15:51:33 +0000
+++ etc/celeryconfig.py	2012-06-19 04:46:22 +0000
@@ -24,6 +24,14 @@
     os.path.dirname(__file__), os.pardir, "src", "provisioningserver", "power",
     "templates")
 
+# Location of PXE config templates. Use an absolute path.
+PXE_TEMPLATES_DIR = os.path.join(
+    os.path.dirname(__file__), os.pardir, "src", "provisioningserver", "pxe",
+    "templates")
+
+# Where to write PXE config files.
+PXE_TARGET_DIR = "/var/lib/tftp/maas"
+
 
 try:
     import user_maasceleryconfig

=== added directory 'src/provisioningserver/pxe'
=== added file 'src/provisioningserver/pxe/__init__.py'
=== added file 'src/provisioningserver/pxe/pxeconfig.py'
--- src/provisioningserver/pxe/pxeconfig.py	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/pxe/pxeconfig.py	2012-06-19 04:46:22 +0000
@@ -0,0 +1,112 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""PXE configuration management."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = [
+    'PXEConfig',
+    'PXEConfigFail',
+    ]
+
+
+import os
+import tempita
+
+from celeryconfig import (
+    PXE_TARGET_DIR,
+    PXE_TEMPLATES_DIR,
+    )
+
+
+class PXEConfigFail(Exception):
+    """Raised if there's a problem with a PXE config."""
+
+
+class PXEConfig:
+    """PXE Configuration management.
+
+    Encapsulation of PXE config templates and parameter substitution.
+
+    :param arch: The architecture of the context node.
+    :type arch: string
+    :param subarch: The sub-architecture of the context node. This is
+        optional because some architectures such as i386 don't have a
+        sub-architecture.  If not passed, a directory name of "generic"
+        is used in the subarch part of the path to the target file.
+    :type subarch: string
+    :param mac: If specified will write out a mac-specific pxe file.
+        If not specified will write out a "default" file.
+        Note: Ensure the mac is passed in a colon-separated format like
+        aa:bb:cc:dd:ee:ff.  This is the default for MAC addresses coming
+        from the database fields in MAAS, so it's not heavily checked here.
+    """
+
+    def __init__(self, arch, subarch=None, mac=None):
+        if subarch is None:
+            subarch = "generic"
+        self.mac = self._process_mac(mac)
+
+        self.template = os.path.join(self.template_basedir, "maas.template")
+
+        self.target_dir = os.path.join(
+            self.target_basedir,
+            arch,
+            subarch)
+        if self.mac is not None:
+            filename = self.mac
+        else:
+            filename = "default"
+        self.target_file = os.path.join(self.target_dir, filename)
+
+    @property
+    def template_basedir(self):
+        return PXE_TEMPLATES_DIR
+
+    @property
+    def target_basedir(self):
+        return PXE_TARGET_DIR
+
+    def _process_mac(self, mac):
+        # A MAC address should be of the form aa:bb:cc:dd:ee:ff with
+        # precisely five colons in it.  We do a cursory check since most
+        # MACs will come from the DB which are already checked and
+        # formatted.
+        if mac is None:
+            return None
+        colon_count = mac.count(":")
+        if colon_count != 5:
+            raise PXEConfigFail(
+                "Expecting exactly five ':' chars, found %s" % colon_count)
+        return mac.replace(':', '-')
+
+    def get_template(self):
+        with open(self.template, "rb") as f:
+            return tempita.Template(f.read(), name=self.template)
+
+    def render_template(self, template, **kwargs):
+        try:
+            return template.substitute(kwargs)
+        except NameError as error:
+            raise PXEConfigFail(*error.args)
+
+    def write_config(self, **kwargs):
+        """Write out this PXE config file.
+
+        :param menutitle: The PXE menu title shown.
+        :param kernelimage: The path to the kernel in the TFTP server
+        :param append: Kernel parameters to append.
+
+        Any required directories will be created.
+        """
+        template = self.get_template()
+        rendered = self.render_template(template, **kwargs)
+        os.makedirs(self.target_dir)
+        with open(self.target_file, "wb") as f:
+            f.write(rendered)

=== added directory 'src/provisioningserver/pxe/templates'
=== added file 'src/provisioningserver/pxe/templates/maas.template'
--- src/provisioningserver/pxe/templates/maas.template	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/pxe/templates/maas.template	2012-06-19 04:46:22 +0000
@@ -0,0 +1,13 @@
+DEFAULT menu
+PROMPT 0
+MENU TITLE {{menutitle}}
+TIMEOUT 1
+ONTIMEOUT ubuntu-enlist
+
+LABEL ubuntu-enlist
+        kernel {{kernelimage}}
+        MENU LABEL ubuntu-enlist
+        append {{append}}
+        ipappend 2
+
+MENU end

=== added directory 'src/provisioningserver/pxe/tests'
=== added file 'src/provisioningserver/pxe/tests/test_pxeconfig.py'
--- src/provisioningserver/pxe/tests/test_pxeconfig.py	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/pxe/tests/test_pxeconfig.py	2012-06-19 04:46:22 +0000
@@ -0,0 +1,114 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for `provisioningserver.pxe`."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = []
+
+import os
+import re
+import tempita
+
+from celeryconfig import (
+    PXE_TARGET_DIR,
+    PXE_TEMPLATES_DIR,
+    )
+from maastesting.factory import factory
+from maastesting.testcase import TestCase
+from provisioningserver.pxe.pxeconfig import (
+    PXEConfig,
+    PXEConfigFail,
+    )
+from testtools.matchers import (
+    MatchesRegex,
+    )
+
+
+class TestPXEConfig(TestCase):
+    """Tests for PXEConfig."""
+
+    def test_init_sets_up_paths(self):
+        pxeconfig = PXEConfig("armhf", "armadaxp")
+
+        expected_template = os.path.join(PXE_TEMPLATES_DIR, "maas.template")
+        expected_target = os.path.join(PXE_TARGET_DIR, "armhf", "armadaxp")
+        self.assertEqual(expected_template, pxeconfig.template)
+        self.assertEqual(expected_target, pxeconfig.target_dir)
+
+    def test_init_with_no_subarch_makes_path_with_generic(self):
+        pxeconfig = PXEConfig("i386")
+        expected_target = os.path.join(PXE_TARGET_DIR, "i386", "generic")
+        self.assertEqual(expected_target, pxeconfig.target_dir)
+
+    def test_init_with_no_mac_sets_default_filename(self):
+        pxeconfig = PXEConfig("armhf", "armadaxp")
+        expected_filename = os.path.join(
+            PXE_TARGET_DIR, "armhf", "armadaxp", "default")
+        self.assertEqual(expected_filename, pxeconfig.target_file)
+
+    def test_init_with_dodgy_mac(self):
+        # !=5 colons is bad.
+        bad_mac = "aa:bb:cc:dd:ee"
+        exception = self.assertRaises(
+            PXEConfigFail, PXEConfig, "armhf", "armadaxp", bad_mac)
+        self.assertEqual(
+            exception.message, "Expecting exactly five ':' chars, found 4")
+
+    def test_init_with_mac_sets_filename(self):
+        pxeconfig = PXEConfig("armhf", "armadaxp", mac="00:a1:b2:c3:e4:d5")
+        expected_filename = os.path.join(
+            PXE_TARGET_DIR, "armhf", "armadaxp", "00-a1-b2-c3-e4-d5")
+        self.assertEqual(expected_filename, pxeconfig.target_file)
+
+    def test_get_template(self):
+        pxeconfig = PXEConfig("i386")
+        template = pxeconfig.get_template()
+        with open(pxeconfig.template, "rb") as f:
+            expected = f.read()
+        self.assertIsInstance(template, tempita.Template)
+        self.assertEqual(expected, template.content)
+
+    def test_render_template(self):
+        pxeconfig = PXEConfig("i386")
+        template = tempita.Template("template: {{kernelimage}}")
+        rendered = pxeconfig.render_template(template, kernelimage="myimage")
+        self.assertEqual("template: myimage", rendered)
+
+    def test_render_template_raises_PXEConfigFail(self):
+        # If not enough arguments are supplied to fill in template
+        # variables then a PXEConfigFail is raised.
+        pxeconfig = PXEConfig("i386")
+        template_name = factory.getRandomString()
+        template = tempita.Template(
+            "template: {{kernelimage}}", name=template_name)
+        exception = self.assertRaises(
+            PXEConfigFail, pxeconfig.render_template, template)
+        self.assertThat(
+            exception.message, MatchesRegex(
+                "name 'kernelimage' is not defined at line \d+ column \d+ "
+                "in file %s" % re.escape(template_name)))
+
+    def test_write_config(self):
+        # Ensure that a rendered template is written to the right place.
+        out_dir = self.make_dir()
+        self.patch(PXEConfig, 'target_basedir', out_dir)
+        pxeconfig = PXEConfig("armhf", "armadaxp")
+        pxeconfig.write_config(
+            menutitle="menutitle", kernelimage="/my/kernel", append="append")
+
+        with open(pxeconfig.target_file, "rb") as f:
+            output = f.read()
+        template = pxeconfig.get_template()
+        expected = pxeconfig.render_template(
+            template, menutitle="menutitle", kernelimage="/my/kernel",
+            append="append")
+
+        self.assertEqual(expected, output)
+