launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #11281
[Merge] lp:~allenap/maas/pxe-intel-arch-config-2 into lp:maas
Gavin Panella has proposed merging lp:~allenap/maas/pxe-intel-arch-config-2 into lp:maas with lp:~allenap/maas/pxe-intel-arch-config as a prerequisite.
Requested reviews:
MAAS Maintainers (maas-maintainers)
For more details, see:
https://code.launchpad.net/~allenap/maas/pxe-intel-arch-config-2/+merge/121097
--
https://code.launchpad.net/~allenap/maas/pxe-intel-arch-config-2/+merge/121097
Your team MAAS Maintainers is requested to review the proposed merge of lp:~allenap/maas/pxe-intel-arch-config-2 into lp:maas.
=== modified file 'src/provisioningserver/pxe/config.py'
--- src/provisioningserver/pxe/config.py 2012-08-17 11:12:09 +0000
+++ src/provisioningserver/pxe/config.py 2012-08-23 22:27:20 +0000
@@ -25,6 +25,7 @@
from os import path
import posixpath
+from provisioningserver.kernel_opts import compose_kernel_command_line_new
from provisioningserver.pxe.tftppath import compose_image_path
import tempita
@@ -71,26 +72,25 @@
"No PXE template found in %r!" % template_dir)
-def render_pxe_config(
- arch, subarch, release, purpose, append, bootpath, **extra):
+def render_pxe_config(bootpath, kernel_params, **extra):
"""Render a PXE configuration file as a unicode string.
- :param arch: Main machine architecture.
- :param subarch: Sub-architecture, or "generic" if there is none.
- :param release: The OS release, e.g. "precise".
- :param purpose: What's the purpose of this boot, e.g. "install".
- :param append: Additional kernel parameters.
:param bootpath: The directory path of `pxelinux.0`.
+ :param kernel_params: An instance of `KernelParameters`.
:param extra: Allow for other arguments. This is a safety valve;
parameters generated in another component (for example, see
`TFTPBackend.get_config_reader`) won't cause this to break.
"""
- template = get_pxe_template(purpose, arch, subarch)
- image_dir = compose_image_path(arch, subarch, release, purpose)
+ 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.
namespace = {
- "append": append,
+ "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),
=== modified file 'src/provisioningserver/pxe/tests/test_config.py'
--- src/provisioningserver/pxe/tests/test_config.py 2012-08-17 11:12:09 +0000
+++ src/provisioningserver/pxe/tests/test_config.py 2012-08-23 22:27:20 +0000
@@ -23,6 +23,7 @@
from provisioningserver.pxe import config
from provisioningserver.pxe.config import render_pxe_config
from provisioningserver.pxe.tftppath import compose_image_path
+from provisioningserver.tests.test_kernel_opts import make_kernel_parameters
from testtools.matchers import (
IsInstance,
MatchesAll,
@@ -101,15 +102,9 @@
def test_render(self):
# Given the right configuration options, the PXE configuration is
# correctly rendered.
- options = {
- "arch": factory.make_name("arch"),
- "subarch": factory.make_name("subarch"),
- "release": factory.make_name("release"),
- "purpose": factory.make_name("purpose"),
- "append": factory.make_name("append"),
- }
- options["bootpath"] = "maas/%(arch)s/%(subarch)s" % options
- output = render_pxe_config(**options)
+ bootpath = factory.make_name("bootpath")
+ params = make_kernel_parameters()
+ output = render_pxe_config(bootpath=bootpath, kernel_params=params)
# The output is always a Unicode string.
self.assertThat(output, IsInstance(unicode))
# The template has rendered without error. PXELINUX configurations
@@ -117,9 +112,9 @@
self.assertThat(output, StartsWith("DEFAULT "))
# The PXE parameters are all set according to the options.
image_dir = compose_image_path(
- arch=options["arch"], subarch=options["subarch"],
- release=options["release"], purpose=options["purpose"])
- image_dir = posixpath.relpath(image_dir, options["bootpath"])
+ arch=params.arch, subarch=params.subarch,
+ release=params.release, purpose=params.purpose)
+ image_dir = posixpath.relpath(image_dir, bootpath)
self.assertThat(
output, MatchesAll(
MatchesRegex(
@@ -129,18 +124,14 @@
r'.*^\s+INITRD %s/initrd[.]gz$' % re.escape(image_dir),
re.MULTILINE | re.DOTALL),
MatchesRegex(
- r'.*^\s+APPEND %s$' % re.escape(options["append"]),
+ r'.*^\s+APPEND .+?$',
re.MULTILINE | re.DOTALL)))
def test_render_with_extra_arguments_does_not_affect_output(self):
# render_pxe_config() allows any keyword arguments as a safety valve.
options = {
- "arch": factory.make_name("arch"),
- "subarch": factory.make_name("subarch"),
- "release": factory.make_name("release"),
- "purpose": factory.make_name("purpose"),
"bootpath": factory.make_name("bootpath"),
- "append": factory.make_name("append"),
+ "kernel_params": make_kernel_parameters(),
}
# Capture the output before sprinking in some random options.
output_before = render_pxe_config(**options)
@@ -157,12 +148,9 @@
# If purpose is "local", the config.localboot.template should be
# used.
options = {
- "arch": factory.make_name("arch"),
- "subarch": factory.make_name("subarch"),
- "release": factory.make_name("release"),
- "purpose": "local",
"bootpath": factory.make_name("bootpath"),
- "append": factory.make_name("append"),
+ "kernel_params":
+ make_kernel_parameters()._replace(purpose="local"),
}
output = render_pxe_config(**options)
self.assertIn("LOCALBOOT 0", output)
@@ -171,12 +159,9 @@
# Intel i386 is a special case and needs to use the chain.c32
# loader as the LOCALBOOT PXE directive is unreliable.
options = {
- "arch": "i386",
- "subarch": factory.make_name("subarch"),
- "release": factory.make_name("release"),
- "purpose": "local",
"bootpath": factory.make_name("bootpath"),
- "append": factory.make_name("append"),
+ "kernel_params": make_kernel_parameters()._replace(
+ arch="i386", purpose="local"),
}
output = render_pxe_config(**options)
self.assertIn("chain.c32", output)
@@ -186,12 +171,9 @@
# Intel amd64 is a special case and needs to use the chain.c32
# loader as the LOCALBOOT PXE directive is unreliable.
options = {
- "arch": "amd64",
- "subarch": factory.make_name("subarch"),
- "release": factory.make_name("release"),
- "purpose": "local",
"bootpath": factory.make_name("bootpath"),
- "append": factory.make_name("append"),
+ "kernel_params": make_kernel_parameters()._replace(
+ arch="amd64", purpose="local"),
}
output = render_pxe_config(**options)
self.assertIn("chain.c32", output)
=== modified file 'src/provisioningserver/tests/test_kernel_opts.py'
--- src/provisioningserver/tests/test_kernel_opts.py 2012-08-23 08:32:27 +0000
+++ src/provisioningserver/tests/test_kernel_opts.py 2012-08-23 22:27:20 +0000
@@ -10,7 +10,9 @@
)
__metaclass__ = type
-__all__ = []
+__all__ = [
+ "make_kernel_parameters",
+ ]
import os
=== modified file 'src/provisioningserver/tests/test_tftp.py'
--- src/provisioningserver/tests/test_tftp.py 2012-08-17 13:55:00 +0000
+++ src/provisioningserver/tests/test_tftp.py 2012-08-23 22:27:20 +0000
@@ -23,7 +23,9 @@
from maastesting.factory import factory
from maastesting.testcase import TestCase
+import mock
from provisioningserver.pxe.tftppath import compose_config_path
+from provisioningserver.tests.test_kernel_opts import make_kernel_parameters
from provisioningserver.tftp import (
BytesReader,
TFTPBackend,
@@ -126,21 +128,19 @@
def test_get_generator_url(self):
# get_generator_url() merges the parameters obtained from the request
# file path (arch, subarch, name) into the configured generator URL.
- arch = factory.make_name("arch").encode("ascii")
- subarch = factory.make_name("subarch").encode("ascii")
+ bootpath = factory.make_name("bootpath")
mac = factory.getRandomMACAddress(b"-")
- append = factory.make_name("append").encode("ascii")
- backend_url = b"http://example.com/?"; + urlencode({b"append": append})
+ dummy = factory.make_name("dummy").encode("ascii")
+ backend_url = b"http://example.com/?"; + urlencode({b"dummy": dummy})
backend = TFTPBackend(self.make_dir(), backend_url)
# params is an example of the parameters obtained from a request.
- params = {"arch": arch, "subarch": subarch, "mac": mac}
+ params = {"bootpath": bootpath, "mac": mac}
generator_url = urlparse(backend.get_generator_url(params))
self.assertEqual("example.com", generator_url.hostname)
query = parse_qsl(generator_url.query)
query_expected = [
- ("append", append),
- ("arch", arch),
- ("subarch", subarch),
+ ("bootpath", bootpath),
+ ("dummy", dummy),
("mac", mac),
]
self.assertItemsEqual(query_expected, query)
@@ -187,36 +187,34 @@
# `IReader` of a PXE configuration, rendered by `render_pxe_config`.
backend = TFTPBackend(self.make_dir(), b"http://example.com/";)
# Fake configuration parameters, as discovered from the file path.
- fake_params = dict(
- arch=factory.make_name("arch"),
- subarch=factory.make_name("subarch"),
- mac=factory.getRandomMACAddress(b"-"))
- fake_params.update(
- bootpath="maas/%(arch)s/%(subarch)s" % fake_params)
- # Fake configuration parameters, as returned from the API call.
- fake_api_params = dict(fake_params)
- fake_api_params.update(
- append=factory.make_name("append"),
- purpose=factory.make_name("purpose"),
- release=factory.make_name("release"))
- # Add a purpose to the first set of parameters. This will later help
- # demonstrate that the API parameters take precedence over the file
- # path parameters.
- fake_params["purpose"] = factory.make_name("original-purpose")
+ fake_params = {
+ "bootpath": "maas",
+ "mac": factory.getRandomMACAddress(b"-"),
+ }
+ # Fake kernel configuration parameters, as returned from the API call.
+ fake_kernel_params = make_kernel_parameters()
+
# Stub get_page to return the fake API configuration parameters.
- fake_api_params_json = json.dumps(fake_api_params)
- backend.get_page = lambda url: succeed(fake_api_params_json)
+ fake_get_page_result = json.dumps(fake_kernel_params._asdict())
+ get_page_patch = mock.patch.object(backend, "get_page")
+ get_page_patch.start().return_value = succeed(fake_get_page_result)
+
# Stub render_pxe_config to return the render parameters.
- backend.render_pxe_config = lambda **kwargs: json.dumps(kwargs)
+ fake_render_result = factory.make_name("render")
+ render_patch = mock.patch.object(backend, "render_pxe_config")
+ render_patch.start().return_value = fake_render_result
+
# Get the rendered configuration, which will actually be a JSON dump
# of the render-time parameters.
- reader = yield backend.get_config_reader(fake_api_params)
+ reader = yield backend.get_config_reader(fake_params)
self.addCleanup(reader.finish)
self.assertIsInstance(reader, BytesReader)
output = reader.read(10000)
- # The expected render-time parameters are a merge of previous
- # parameters. Note that the API parameters take precedence.
- expected_render_params = dict(fake_params)
- expected_render_params.update(fake_api_params)
- observed_render_params = json.loads(output)
- self.assertEqual(expected_render_params, observed_render_params)
+
+ # The kernel parameters were fetched using `backend.get_page`.
+ backend.get_page.assert_called_once()
+
+ # The result has been rendered by `backend.render_pxe_config`.
+ self.assertEqual(fake_render_result.encode("utf-8"), output)
+ backend.render_pxe_config.assert_called_once_with(
+ kernel_params=fake_kernel_params, **fake_params)
=== modified file 'src/provisioningserver/tftp.py'
--- src/provisioningserver/tftp.py 2012-08-17 13:47:24 +0000
+++ src/provisioningserver/tftp.py 2012-08-23 22:27:20 +0000
@@ -25,6 +25,7 @@
)
from provisioningserver.enum import ARP_HTYPE
+from provisioningserver.kernel_opts import KernelParameters
from provisioningserver.pxe.config import render_pxe_config
from provisioningserver.utils import deferred
from tftp.backend import (
@@ -130,21 +131,36 @@
return url.geturl().encode("ascii")
@deferred
+ def get_kernel_params(self, params):
+ """Return kernel parameters obtained from the API.
+
+ :param params: Parameters so far obtained, typically from the file
+ path requested.
+ :return: A `KernelParameters` instance.
+ """
+ url = self.get_generator_url(params)
+
+ def reassemble(data):
+ return KernelParameters(**data)
+
+ d = self.get_page(url)
+ d.addCallback(json.loads)
+ d.addCallback(reassemble)
+ return d
+
+ @deferred
def get_config_reader(self, params):
"""Return an `IReader` for a PXE config.
:param params: Parameters so far obtained, typically from the file
path requested.
"""
- url = self.get_generator_url(params)
-
- def generate_config(api_params):
- params.update(api_params)
- config = self.render_pxe_config(**params)
+ def generate_config(kernel_params):
+ config = self.render_pxe_config(
+ kernel_params=kernel_params, **params)
return config.encode("utf-8")
- d = self.get_page(url)
- d.addCallback(json.loads)
+ d = self.get_kernel_params(params)
d.addCallback(generate_config)
d.addCallback(BytesReader)
return d
