launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #11178
[Merge] lp:~allenap/maas/pxe-template-lookup into lp:maas
Gavin Panella has proposed merging lp:~allenap/maas/pxe-template-lookup into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~allenap/maas/pxe-template-lookup/+merge/120155
This reorganises the PXE templates so that we can do a
most-specific-first lookup on them, based on boot purpose, arch and
subarch.
It also removes all the default values from the templates themselves,
and tidies up a little.
It introduces mock to help with testing. This is going to be shipped
as unittest.mock in Python 3.3 (it's already in the beta, so there's
no doubts there), so it seemed safe to add it in. Seems like a nice
tool.
The factory also grows a make_names() method. This is a lazy-hack so
that test setup can be marginally shorter and/or prettier.
This branch started out in order to reorganise the TFTP layout to boot
Intel machines correctly (w.r.t. 32/64 bit), but meandered a bit and
got big, hence why it's not the most focused change. It is safe to
land on its own though.
--
https://code.launchpad.net/~allenap/maas/pxe-template-lookup/+merge/120155
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/pxe-template-lookup into lp:maas.
=== modified file 'buildout.cfg'
--- buildout.cfg 2012-08-16 09:51:53 +0000
+++ buildout.cfg 2012-08-21 16:03:20 +0000
@@ -33,6 +33,7 @@
fixtures
lockfile
lxml
+ mock
netaddr
nose
nose-subunit
=== modified file 'setup.py'
--- setup.py 2012-08-15 12:54:49 +0000
+++ setup.py 2012-08-21 16:03:20 +0000
@@ -117,6 +117,7 @@
'lxml',
'sst',
'fixtures',
+ 'mock',
'nose',
'nose-subunit',
'python-subunit',
=== modified file 'src/maastesting/factory.py'
--- src/maastesting/factory.py 2012-08-20 07:14:33 +0000
+++ src/maastesting/factory.py 2012-08-21 16:03:20 +0000
@@ -144,6 +144,16 @@
return sep.join(
filter(None, [prefix, self.getRandomString(size=size)]))
+ def make_names(self, *prefixes):
+ """Generate random names.
+
+ Yields a name for each prefix specified.
+
+ :param prefixes: Zero or more prefixes. See `make_name`.
+ """
+ for prefix in prefixes:
+ yield self.make_name(prefix)
+
# Create factory singleton.
factory = Factory()
=== modified file 'src/maastesting/tests/test_factory.py'
--- src/maastesting/tests/test_factory.py 2012-08-20 08:05:30 +0000
+++ src/maastesting/tests/test_factory.py 2012-08-21 16:03:20 +0000
@@ -164,3 +164,9 @@
self.assertThat(
factory.make_name(size=100),
MatchesAll(*[Not(Contains(char)) for char in '/ \t\n\r\\']))
+
+ def test_make_names_calls_make_name_with_each_prefix(self):
+ self.patch(factory, "make_name", lambda prefix: prefix + "-xxx")
+ self.assertSequenceEqual(
+ ["abc-xxx", "def-xxx", "ghi-xxx"],
+ list(factory.make_names("abc", "def", "ghi")))
=== added symlink 'src/provisioningserver/pxe/config.local.amd64.template'
=== target is u'config.local.x86.template'
=== added symlink 'src/provisioningserver/pxe/config.local.i386.template'
=== target is u'config.local.x86.template'
=== renamed file 'src/provisioningserver/pxe/config.localboot.template' => 'src/provisioningserver/pxe/config.local.template'
--- src/provisioningserver/pxe/config.localboot.template 2012-08-13 08:54:29 +0000
+++ src/provisioningserver/pxe/config.local.template 2012-08-21 16:03:20 +0000
@@ -1,8 +1,5 @@
-DEFAULT execute
-TIMEOUT 0
-PROMPT 0
-
-
-LABEL execute
- SAY Booting local disk ...
- LOCALBOOT 0
+DEFAULT local
+
+LABEL local
+ SAY Booting local disk ...
+ LOCALBOOT 0
=== renamed file 'src/provisioningserver/pxe/config.localboot-intel.template' => 'src/provisioningserver/pxe/config.local.x86.template'
--- src/provisioningserver/pxe/config.localboot-intel.template 2012-08-13 08:11:11 +0000
+++ src/provisioningserver/pxe/config.local.x86.template 2012-08-21 16:03:20 +0000
@@ -1,9 +1,5 @@
DEFAULT local
-PROMPT 0
-TIMEOUT 0
-TOTALTIMEOUT 0
-ONTIMEOUT local
LABEL local
- KERNEL chain.c32
-
+ SAY Booting local disk ...
+ KERNEL chain.c32
=== modified file 'src/provisioningserver/pxe/config.py'
--- src/provisioningserver/pxe/config.py 2012-08-13 08:21:52 +0000
+++ src/provisioningserver/pxe/config.py 2012-08-21 16:03:20 +0000
@@ -20,6 +20,7 @@
]
+from errno import ENOENT
from functools import partial
from os import path
@@ -27,13 +28,47 @@
from provisioningserver.pxe.tftppath import compose_image_path
import tempita
-
+# TODO: make this configurable.
template_dir = path.dirname(__file__)
-template_filename = path.join(template_dir, "config.template")
-template_localboot_filename = path.join(
- template_dir, "config.localboot.template")
-template_localboot_intel_filename = path.join(
- template_dir, "config.localboot-intel.template")
+
+
+def gen_pxe_template_filenames(purpose, arch, subarch):
+ """List possible PXE template filenames.
+
+ :param purpose: The boot purpose, e.g. "local".
+ :param arch: Main machine architecture.
+ :param subarch: Sub-architecture, or "generic" if there is none.
+ :param release: The Ubuntu release to be used.
+
+ Returns a list of possible PXE template filenames using the following
+ lookup order:
+
+ config.{purpose}.{arch}.{subarch}.template
+ config.{purpose}.{arch}.template
+ config.{purpose}.template
+ config.template
+
+ """
+ elements = [purpose, arch, subarch]
+ while len(elements) >= 1:
+ yield "config.%s.template" % ".".join(elements)
+ elements.pop()
+ yield "config.template"
+
+
+def get_pxe_template(purpose, arch, subarch):
+ # Templates are loaded each time here so that they can be changed on
+ # the fly without restarting the provisioning server.
+ for filename in gen_pxe_template_filenames(purpose, arch, subarch):
+ try:
+ return tempita.Template.from_filename(
+ path.join(template_dir, filename), encoding="UTF-8")
+ except IOError as error:
+ if error.errno != ENOENT:
+ raise
+ else:
+ raise AssertionError(
+ "No PXE template found in %r!" % template_dir)
def render_pxe_config(
@@ -50,21 +85,7 @@
parameters generated in another component (for example, see
`TFTPBackend.get_config_reader`) won't cause this to break.
"""
- # Templates are loaded each time here so that they can be changed on
- # the fly without restarting the provisioning server.
- if purpose == "local":
- if arch in ("i386", "amd64"):
- template = tempita.Template.from_filename(
- template_localboot_intel_filename, encoding="UTF-8")
- else:
- template = tempita.Template.from_filename(
- template_localboot_filename, encoding="UTF-8")
- # No params in local boot pxeconfig, but using a template anyway
- # in case we decide to do so in the future.
- return template.substitute()
-
- template = tempita.Template.from_filename(
- template_filename, encoding="UTF-8")
+ template = get_pxe_template(purpose, arch, subarch)
image_dir = compose_image_path(arch, subarch, release, purpose)
# The locations of the kernel image and the initrd are defined by
# update_install_files(), in scripts/maas-import-pxe-files.
=== modified file 'src/provisioningserver/pxe/config.template'
--- src/provisioningserver/pxe/config.template 2012-08-02 12:40:52 +0000
+++ src/provisioningserver/pxe/config.template 2012-08-21 16:03:20 +0000
@@ -3,8 +3,8 @@
SAY Booting under MAAS direction in 20 seconds.
LABEL execute
- SAY Booting under MAAS direction...
- KERNEL {{kernel|relpath}}
- INITRD {{initrd|relpath}}
- APPEND {{append}}
- IPAPPEND 2
+ SAY Booting under MAAS direction...
+ KERNEL {{kernel|relpath}}
+ INITRD {{initrd|relpath}}
+ APPEND {{append}}
+ IPAPPEND 2
=== modified file 'src/provisioningserver/pxe/tests/test_config.py'
--- src/provisioningserver/pxe/tests/test_config.py 2012-08-13 08:54:29 +0000
+++ src/provisioningserver/pxe/tests/test_config.py 2012-08-21 16:03:20 +0000
@@ -12,11 +12,15 @@
__metaclass__ = type
__all__ = []
+import errno
+from os import path
import re
from maastesting.factory import factory
from maastesting.testcase import TestCase
+import mock
import posixpath
+from provisioningserver.pxe import config
from provisioningserver.pxe.config import render_pxe_config
from provisioningserver.pxe.tftppath import compose_image_path
from testtools.matchers import (
@@ -27,6 +31,70 @@
)
+class TestFunctions(TestCase):
+ """Test for functions in `provisioningserver.pxe.config`."""
+
+ def test_gen_pxe_template_filenames(self):
+ purpose = factory.make_name("purpose")
+ arch, subarch = factory.make_names("arch", "subarch")
+ expected = [
+ "config.%s.%s.%s.template" % (purpose, arch, subarch),
+ "config.%s.%s.template" % (purpose, arch),
+ "config.%s.template" % (purpose, ),
+ "config.template",
+ ]
+ observed = config.gen_pxe_template_filenames(purpose, arch, subarch)
+ self.assertSequenceEqual(expected, list(observed))
+
+ @mock.patch("tempita.Template.from_filename")
+ @mock.patch.object(config, "gen_pxe_template_filenames")
+ def test_get_pxe_template(self, gen_filenames, from_filename):
+ purpose = factory.make_name("purpose")
+ arch, subarch = factory.make_names("arch", "subarch")
+ filename = factory.make_name("filename")
+ # Set up the mocks that we've patched in.
+ gen_filenames.return_value = [filename]
+ from_filename.return_value = mock.sentinel.template
+ # The template returned matches the return value above.
+ template = config.get_pxe_template(purpose, arch, subarch)
+ self.assertEqual(mock.sentinel.template, template)
+ # gen_pxe_template_filenames is called to obtain filenames.
+ gen_filenames.assert_called_once_with(purpose, arch, subarch)
+ # Tempita.from_filename is called with an absolute path derived from
+ # the filename returned from gen_pxe_template_filenames.
+ from_filename.assert_called_once_with(
+ path.join(config.template_dir, filename), encoding="UTF-8")
+
+ def test_get_pxe_template_gets_default(self):
+ # There will not be a template matching the following purpose, arch,
+ # and subarch, so get_pxe_template() returns the default template.
+ purpose = factory.make_name("purpose")
+ arch, subarch = factory.make_names("arch", "subarch")
+ template = config.get_pxe_template(purpose, arch, subarch)
+ self.assertEqual(
+ path.join(config.template_dir, "config.template"),
+ template.name)
+
+ @mock.patch.object(config, "gen_pxe_template_filenames")
+ def test_get_pxe_template_not_found(self, gen_filenames):
+ # It is a critical and unrecoverable error if the default PXE template
+ # is not found.
+ gen_filenames.return_value = []
+ self.assertRaises(
+ AssertionError, config.get_pxe_template,
+ *factory.make_names("purpose", "arch", "subarch"))
+
+ @mock.patch("tempita.Template.from_filename")
+ def test_get_pxe_templates_only_suppresses_ENOENT(self, from_filename):
+ # The IOError arising from trying to load a template that doesn't
+ # exist is suppressed, but other errors are not.
+ from_filename.side_effect = IOError()
+ from_filename.side_effect.errno = errno.EACCES
+ self.assertRaises(
+ IOError, config.get_pxe_template,
+ *factory.make_names("purpose", "arch", "subarch"))
+
+
class TestRenderPXEConfig(TestCase):
"""Tests for `provisioningserver.pxe.config.render_pxe_config`."""
=== modified file 'versions.cfg'
--- versions.cfg 2012-08-16 07:24:47 +0000
+++ versions.cfg 2012-08-21 16:03:20 +0000
@@ -57,6 +57,7 @@
ipython = 0.12
# Bug 251 is problematic in 0.9.2.
django-debug-toolbar = 0.9.1
+mock = 1.0b1
[versions-auto]
# Added by Buildout Versions at 2012-02-24 15:51:04.865203
