← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~allenap/maas/pxe-intel-arch-config-3 into lp:maas

 

Gavin Panella has proposed merging lp:~allenap/maas/pxe-intel-arch-config-3 into lp:maas with lp:~allenap/maas/pxe-intel-arch-config-2 as a prerequisite.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~allenap/maas/pxe-intel-arch-config-3/+merge/121107

This introduces the stuff needed to automatically boot with the
correct choice of a 32-bit or 64-bit image. It does this using the
ifcpu64 module from SYSLINUX.

The work to bring the kernel command line generation over to pserv is
paying off now. The config.commissioning.template is using it to
generate new kernel parameters on the fly. The kernel path and initrd
path can also be generated on the fly.

I didn't do a pre-implementation call for this.

-- 
https://code.launchpad.net/~allenap/maas/pxe-intel-arch-config-3/+merge/121107
Your team MAAS Maintainers is requested to review the proposed merge of lp:~allenap/maas/pxe-intel-arch-config-3 into lp:maas.
=== modified file 'scripts/maas-import-pxe-files'
--- scripts/maas-import-pxe-files	2012-08-17 14:34:14 +0000
+++ scripts/maas-import-pxe-files	2012-08-24 00:26:23 +0000
@@ -77,6 +77,8 @@
         maas-provision install-pxe-bootloader --loader='pxelinux.0'
         maas-provision install-pxe-bootloader \
             --loader='/usr/lib/syslinux/chain.c32'
+        maas-provision install-pxe-bootloader \
+            --loader='/usr/lib/syslinux/ifcpu64.c32'
     fi
 }
 

=== modified file 'src/provisioningserver/kernel_opts.py'
--- src/provisioningserver/kernel_opts.py	2012-08-23 09:18:40 +0000
+++ src/provisioningserver/kernel_opts.py	2012-08-24 00:26:23 +0000
@@ -27,8 +27,8 @@
     """The ephemeral images directory cannot be found."""
 
 
-KernelParameters = namedtuple(
-    "KernelParameters", (
+KernelParametersBase = namedtuple(
+    "KernelParametersBase", (
         "arch",  # Machine architecture, e.g. "i386"
         "subarch",  # Machine subarchitecture, e.g. "generic"
         "release",  # Ubuntu release, e.g. "precise"
@@ -41,6 +41,12 @@
         ))
 
 
+class KernelParameters(KernelParametersBase):
+
+    # foo._replace() is just ugly, so alias it to __call__.
+    __call__ = KernelParametersBase._replace
+
+
 def compose_initrd_opt(arch, subarch, release, purpose):
     path = "%s/initrd.gz" % compose_image_path(arch, subarch, release, purpose)
     return "initrd=%s" % path

=== added file 'src/provisioningserver/pxe/config.commissioning.template'
--- src/provisioningserver/pxe/config.commissioning.template	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/pxe/config.commissioning.template	2012-08-24 00:26:23 +0000
@@ -0,0 +1,21 @@
+DEFAULT execute
+TIMEOUT 20
+SAY Booting under MAAS direction in 20 seconds.
+
+LABEL execute
+  KERNEL ifcpu64.c32
+  APPEND amd64 -- i386
+
+LABEL amd64
+  SAY Booting (amd64) under MAAS direction...
+  KERNEL {{kernel_params(arch="amd64") | kernel_path | relative}}
+  INITRD {{kernel_params(arch="amd64") | initrd_path | relative}}
+  APPEND {{kernel_params(arch="amd64") | kernel_command}}
+  IPAPPEND 2
+
+LABEL i386
+  SAY Booting (i386) under MAAS direction...
+  KERNEL {{kernel_params(arch="i386") | kernel_path | relative}}
+  INITRD {{kernel_params(arch="i386") | initrd_path | relative}}
+  APPEND {{kernel_params(arch="i386") | kernel_command}}
+  IPAPPEND 2

=== modified file 'src/provisioningserver/pxe/config.py'
--- src/provisioningserver/pxe/config.py	2012-08-24 00:26:23 +0000
+++ src/provisioningserver/pxe/config.py	2012-08-24 00:26:23 +0000
@@ -21,7 +21,6 @@
 
 
 from errno import ENOENT
-from functools import partial
 from os import path
 
 import posixpath
@@ -84,15 +83,33 @@
     template = get_pxe_template(
         kernel_params.purpose, kernel_params.arch,
         kernel_params.subarch)
-    image_dir = compose_image_path(
-        kernel_params.arch, kernel_params.subarch,
-        kernel_params.release, kernel_params.purpose)
+
     # The locations of the kernel image and the initrd are defined by
     # update_install_files(), in scripts/maas-import-pxe-files.
+
+    def image_dir(params):
+        return compose_image_path(
+            params.arch, params.subarch,
+            params.release, params.purpose)
+
+    def initrd(params):
+        return "%s/initrd.gz" % image_dir(params)
+
+    def kernel(params):
+        return "%s/linux" % image_dir(params)
+
+    def kernel_command(params):
+        return compose_kernel_command_line_new(params)
+
+    def relative(path):
+        # Return `path` relative to `bootpath`.
+        return posixpath.relpath(path, start=bootpath)
+
     namespace = {
-        "append": compose_kernel_command_line_new(kernel_params),
-        "initrd": "%s/initrd.gz" % image_dir,
-        "kernel": "%s/linux" % image_dir,
-        "relpath": partial(posixpath.relpath, start=bootpath),
+        "initrd_path": initrd,
+        "kernel_command": kernel_command,
+        "kernel_params": kernel_params,
+        "kernel_path": kernel,
+        "relative": relative,
         }
     return template.substitute(namespace)

=== modified file 'src/provisioningserver/pxe/config.template'
--- src/provisioningserver/pxe/config.template	2012-08-16 14:03:06 +0000
+++ src/provisioningserver/pxe/config.template	2012-08-24 00:26:23 +0000
@@ -4,7 +4,7 @@
 
 LABEL execute
   SAY Booting under MAAS direction...
-  KERNEL {{kernel|relpath}}
-  INITRD {{initrd|relpath}}
-  APPEND {{append}}
+  KERNEL {{kernel_params | kernel_path | relative}}
+  INITRD {{kernel_params | initrd_path | relative}}
+  APPEND {{kernel_params | kernel_command}}
   IPAPPEND 2

=== modified file 'src/provisioningserver/pxe/tests/test_config.py'
--- src/provisioningserver/pxe/tests/test_config.py	2012-08-24 00:26:23 +0000
+++ src/provisioningserver/pxe/tests/test_config.py	2012-08-24 00:26:23 +0000
@@ -12,11 +12,13 @@
 __metaclass__ = type
 __all__ = []
 
+from collections import OrderedDict
 import errno
 from os import path
 import re
 
 from maastesting.factory import factory
+from maastesting.matchers import ContainsAll
 from maastesting.testcase import TestCase
 import mock
 import posixpath
@@ -25,6 +27,7 @@
 from provisioningserver.pxe.tftppath import compose_image_path
 from provisioningserver.tests.test_kernel_opts import make_kernel_parameters
 from testtools.matchers import (
+    Contains,
     IsInstance,
     MatchesAll,
     MatchesRegex,
@@ -96,6 +99,50 @@
             *factory.make_names("purpose", "arch", "subarch"))
 
 
+def parse_pxe_config(text):
+    """Parse a PXE config file.
+
+    Returns a structure like the following, defining the sections::
+
+      {"section_label": {"KERNEL": "...", "INITRD": "...", ...}, ...}
+
+    Additionally, the returned dict - which is actually an `OrderedDict`, as
+    are all mappings returned from this function - has a `header` attribute.
+    This is an `OrderedDict` of the settings in the top part of the PXE config
+    file, the part before any labelled sections.
+    """
+    result = OrderedDict()
+    sections = re.split("^LABEL ", text, flags=re.MULTILINE)
+    for index, section in enumerate(sections):
+        elements = [
+            line.split(None, 1) for line in section.splitlines()
+            if line and not line.isspace()
+            ]
+        if index == 0:
+            result.header = OrderedDict(elements)
+        else:
+            [label] = elements.pop(0)
+            if label in result:
+                raise AssertionError(
+                    "Section %r already defined" % label)
+            result[label] = OrderedDict(elements)
+    return result
+
+
+class TestParsePXEConfig(TestCase):
+    """Tests for `parse_pxe_config`."""
+
+    def test_parse_with_no_header(self):
+        config = parse_pxe_config("LABEL foo\nOPTION setting")
+        self.assertEqual({"foo": {"OPTION": "setting"}}, config)
+        self.assertEqual({}, config.header)
+
+    def test_parse_with_no_labels(self):
+        config = parse_pxe_config("OPTION setting")
+        self.assertEqual({"OPTION": "setting"}, config.header)
+        self.assertEqual({}, config)
+
+
 class TestRenderPXEConfig(TestCase):
     """Tests for `provisioningserver.pxe.config.render_pxe_config`."""
 
@@ -178,3 +225,38 @@
         output = render_pxe_config(**options)
         self.assertIn("chain.c32", output)
         self.assertNotIn("LOCALBOOT", output)
+
+    @mock.patch("provisioningserver.kernel_opts.get_ephemeral_name")
+    def test_render_pxe_config_for_commissioning(self, get_ephemeral_name):
+        # The commissioning config uses an extra PXELINUX module to auto
+        # select between i386 and amd64.
+        get_ephemeral_name.return_value = factory.make_name("ephemeral")
+        options = {
+            "bootpath": factory.make_name("bootpath"),
+            "kernel_params": make_kernel_parameters()._replace(
+                purpose="commissioning"),
+            }
+        output = render_pxe_config(**options)
+        config = parse_pxe_config(output)
+        # The default section is defined.
+        default_section_label = config.header["DEFAULT"]
+        self.assertThat(config, Contains(default_section_label))
+        default_section = config[default_section_label]
+        # The default section uses the ifcpu64 module, branching to the "i386"
+        # or "amd64" labels accordingly.
+        self.assertEqual("ifcpu64.c32", default_section["KERNEL"])
+        self.assertEqual(
+            ["amd64", "--", "i386"],
+            default_section["APPEND"].split())
+        # Both "i386" and "amd64" sections exist.
+        self.assertThat(config, ContainsAll(("i386", "amd64")))
+        # Each section defines KERNEL, INITRD, and APPEND settings, each
+        # containing paths referring to their architectures.
+        for section_label in ("i386", "amd64"):
+            section = config[section_label]
+            self.assertThat(
+                section, ContainsAll(("KERNEL", "INITRD", "APPEND")))
+            contains_arch_path = Contains("/%s/" % section_label)
+            self.assertThat(section["KERNEL"], contains_arch_path)
+            self.assertThat(section["INITRD"], contains_arch_path)
+            self.assertThat(section["APPEND"], contains_arch_path)

=== modified file 'src/provisioningserver/tests/test_kernel_opts.py'
--- src/provisioningserver/tests/test_kernel_opts.py	2012-08-24 00:26:23 +0000
+++ src/provisioningserver/tests/test_kernel_opts.py	2012-08-24 00:26:23 +0000
@@ -31,6 +31,14 @@
 from provisioningserver.testing.config import ConfigFixture
 
 
+def make_kernel_parameters():
+    """Make a randomly populated `KernelParameters` instance."""
+    return KernelParameters(**{
+            field: factory.make_name(field)
+            for field in KernelParameters._fields
+            })
+
+
 class TestUtilitiesKernelOpts(TestCase):
 
     def test_get_last_directory(self):
@@ -43,13 +51,11 @@
         os.makedirs(dir3)
         self.assertEqual(dir1, get_last_directory(root))
 
-
-def make_kernel_parameters():
-    """Make a randomly populated `KernelParameters` instance."""
-    return KernelParameters(**{
-            field: factory.make_name(field)
-            for field in KernelParameters._fields
-            })
+    def test_kernel_parameters_callable(self):
+        # KernelParameters instances are callable; an alias for _replace().
+        params = make_kernel_parameters()
+        self.assertTrue(callable(params))
+        self.assertIs(params._replace.im_func, params.__call__.im_func)
 
 
 class TestKernelOpts(TestCase):