launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #10302
[Merge] lp:~jtv/maas/pxe-config into lp:maas
Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/pxe-config into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/maas/pxe-config/+merge/116806
Not too much chance to discuss this with people, but it's based on some answers by roaksoax and I did get to sanity-check it very briefly with Raphaël today.
As far as I can see we didn't really need any changes to the PXE template we use. But I changed it anyway, to remove references to enlistment and make the styling of the file a bit more consistent. And I added an INITRD setting, which would otherwise have to go through the “append” parameter. I think having it as a separate parameter is more in line with the long-term design we discussed with Robie, where you more or less statelessly specify “boot node X with kernel Y and initrd Z.”
This branch changes the parameters to the pxeconfig API a bit. This would normally be bad, but in this case it's an API that isn't operational yet as long as we still use Cobbler. The changes are simple: kernelimage becomes kernel, an initrd is added, and menutitle becomes menu_title. I think the new spellings are more consistent with practice elsewhere, but in particular (and kudos to Raphers for bringing this up) it is a much closer reflection of what the config-file format itself uses.
Oh, and you'll notice that I found good reference documentation for the PXE config files. Added a link.
Jeroen
--
https://code.launchpad.net/~jtv/maas/pxe-config/+merge/116806
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/pxe-config into lp:maas.
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2012-07-25 13:45:14 +0000
+++ src/maasserver/api.py 2012-07-26 08:06:54 +0000
@@ -1078,14 +1078,18 @@
:param arch: Main machine architecture.
:param subarch: Sub-architecture, or "generic" if there is none.
- :param mac: If specified will return a mac-specific PXE file.
- If not specified will return a "default" 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.
+ :param mac: MAC address to produce a boot configuration for. This
+ parameter is optional. If it is not given, the configuration
+ will be the "default" one which boots into an enlistment image.
+ :param menu_title: Title that the node should show in its PXE menu.
+ :param kernel: TFTP path to the kernel image that is to be booted.
+ :param initrd: TFTP path to the initrd that is to be booted.
+ :param append: Additional parameters to append to the kernel command
+ line.
"""
- menutitle = get_mandatory_param(request.GET, 'menutitle')
- kernelimage = get_mandatory_param(request.GET, 'kernelimage')
+ menu_title = get_mandatory_param(request.GET, 'menu_title')
+ kernel = get_mandatory_param(request.GET, 'kernel')
+ initrd = get_mandatory_param(request.GET, 'initrd')
append = get_mandatory_param(request.GET, 'append')
arch = get_mandatory_param(request.GET, 'arch')
subarch = request.GET.get('subarch', 'generic')
@@ -1099,7 +1103,8 @@
try:
return HttpResponse(
config.get_config(
- menutitle=menutitle, kernelimage=kernelimage, append=append),
+ menu_title=menu_title, kernel=kernel, initrd=initrd,
+ append=append),
content_type="text/plain; charset=utf-8")
except PXEConfigFail as e:
raise ValidationError(e.message)
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2012-07-23 17:40:50 +0000
+++ src/maasserver/tests/test_api.py 2012-07-26 08:06:54 +0000
@@ -2245,9 +2245,10 @@
'arch': "armhf",
'subarch': "armadaxp",
'mac': factory.make_mac_address().mac_address,
- 'menutitle': "menutitle",
- 'kernelimage': "/my/kernel",
- 'append': "append",
+ 'menu_title': factory.make_name("Menu"),
+ 'kernel': factory.make_name("/my/kernel"),
+ 'initrd': factory.make_name("/my/initrd"),
+ 'append': factory.make_name("append"),
}
def get_optional_params(self):
@@ -2283,8 +2284,9 @@
'arch': httplib.BAD_REQUEST,
'subarch': httplib.OK,
'mac': httplib.OK,
- 'menutitle': httplib.BAD_REQUEST,
- 'kernelimage': httplib.BAD_REQUEST,
+ 'menu_title': httplib.BAD_REQUEST,
+ 'kernel': httplib.BAD_REQUEST,
+ 'initrd': httplib.BAD_REQUEST,
'append': httplib.BAD_REQUEST,
}
observed = {
=== modified file 'src/provisioningserver/pxe/pxeconfig.py'
--- src/provisioningserver/pxe/pxeconfig.py 2012-07-25 14:04:07 +0000
+++ src/provisioningserver/pxe/pxeconfig.py 2012-07-26 08:06:54 +0000
@@ -1,7 +1,12 @@
# Copyright 2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
-"""Generating PXE configuration files."""
+"""Generating PXE configuration files.
+
+For more about the format of these files:
+
+http://www.syslinux.org/wiki/index.php/SYSLINUX#How_do_I_Configure_SYSLINUX.3F
+"""
from __future__ import (
absolute_import,
@@ -48,8 +53,8 @@
and then produce a configuration file with:
>>> pxeconfig.get_config(
- ... menutitle="menutitle", kernelimage="/my/kernel",
- append="initrd=blah url=blah")
+ ... menu_title="Booting", kernel="/my/kernel", initrd="/my/initrd",
+ append="auto url=blah")
"""
def __init__(self, arch, subarch='generic'):
@@ -79,9 +84,11 @@
def get_config(self, **kwargs):
"""Return this PXE config file as a unicode string.
- :param menutitle: The PXE menu title shown.
- :param kernelimage: The path to the kernel in the TFTP server
- :param append: Kernel parameters to append.
+ :param menu_title: Title that the node should show on its boot menu.
+ :param kernel: TFTP path to the kernel image to boot.
+ :param initrd: TFTP path to the initrd file to boot from.
+ :param append: Additional parameters to append to the kernel
+ command line.
"""
template = self.get_template()
return self.render_template(template, **kwargs)
=== modified file 'src/provisioningserver/pxe/templates/maas.template'
--- src/provisioningserver/pxe/templates/maas.template 2012-06-18 23:15:38 +0000
+++ src/provisioningserver/pxe/templates/maas.template 2012-07-26 08:06:54 +0000
@@ -1,13 +1,14 @@
DEFAULT menu
PROMPT 0
-MENU TITLE {{menutitle}}
+MENU TITLE {{menu_title}}
TIMEOUT 1
-ONTIMEOUT ubuntu-enlist
+ONTIMEOUT execute
-LABEL ubuntu-enlist
- kernel {{kernelimage}}
- MENU LABEL ubuntu-enlist
- append {{append}}
- ipappend 2
+LABEL execute
+ KERNEL {{kernel}}
+ INITRD {{initrd}}
+ MENU LABEL Continue
+ APPEND {{append}}
+ IPAPPEND 2
MENU end
=== modified file 'src/provisioningserver/pxe/tests/test_pxeconfig.py'
--- src/provisioningserver/pxe/tests/test_pxeconfig.py 2012-07-25 13:45:14 +0000
+++ src/provisioningserver/pxe/tests/test_pxeconfig.py 2012-07-26 08:06:54 +0000
@@ -73,8 +73,8 @@
def test_render_template(self):
pxeconfig = PXEConfig("i386")
- template = tempita.Template("template: {{kernelimage}}")
- rendered = pxeconfig.render_template(template, kernelimage="myimage")
+ template = tempita.Template("template: {{kernel}}")
+ rendered = pxeconfig.render_template(template, kernel="myimage")
self.assertEqual("template: myimage", rendered)
def test_render_template_raises_PXEConfigFail(self):
@@ -83,23 +83,23 @@
pxeconfig = PXEConfig("i386")
template_name = factory.getRandomString()
template = tempita.Template(
- "template: {{kernelimage}}", name=template_name)
+ "template: {{kernel}}", 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+ "
+ "name 'kernel' is not defined at line \d+ column \d+ "
"in file %s" % re.escape(template_name)))
def test_get_config_returns_config(self):
pxeconfig = PXEConfig("armhf", "armadaxp")
template = pxeconfig.get_template()
expected = pxeconfig.render_template(
- template, menutitle="menutitle", kernelimage="/my/kernel",
- append="append")
+ template, menu_title="Title", kernel="/my/kernel",
+ initrd="/my/initrd", append="append")
self.assertEqual(
pxeconfig.get_config(
- menutitle="menutitle", kernelimage="/my/kernel",
+ menu_title="Title", kernel="/my/kernel", initrd="/my/initrd",
append="append"),
expected)
=== modified file 'src/provisioningserver/tests/test_tftp.py'
--- src/provisioningserver/tests/test_tftp.py 2012-07-06 15:58:34 +0000
+++ src/provisioningserver/tests/test_tftp.py 2012-07-26 08:06:54 +0000
@@ -96,12 +96,16 @@
arch = factory.make_name("arch").encode("ascii")
subarch = factory.make_name("subarch").encode("ascii")
name = factory.make_name("name").encode("ascii")
- kernelimage = factory.make_name("kernelimage").encode("ascii")
- menutitle = factory.make_name("menutitle").encode("ascii")
+ kernel = factory.make_name("kernel").encode("ascii")
+ initrd = factory.make_name("initrd").encode("ascii")
+ menu_title = factory.make_name("menu-title").encode("ascii")
append = factory.make_name("append").encode("ascii")
- backend_url = b"http://example.com/?"; + urlencode(
- {b"kernelimage": kernelimage, b"menutitle": menutitle,
- b"append": append})
+ backend_url = b"http://example.com/?"; + urlencode({
+ b"kernel": kernel,
+ b"initrd": initrd,
+ b"menu_title": menu_title,
+ b"append": append,
+ })
backend = TFTPBackend(self.make_dir(), backend_url)
# params is an example of the parameters obtained from a request.
params = {"arch": arch, "subarch": subarch, "name": name}
@@ -110,10 +114,11 @@
query = parse_qsl(generator_url.query)
query_expected = [
("append", append),
- ("kernelimage", kernelimage),
+ ("kernel", kernel),
+ ("initrd", initrd),
("arch", arch),
("subarch", subarch),
- ("menutitle", menutitle),
+ ("menu_title", menu_title),
("name", name),
]
self.assertItemsEqual(query_expected, query)
=== modified file 'src/provisioningserver/tftp.py'
--- src/provisioningserver/tftp.py 2012-07-06 16:04:53 +0000
+++ src/provisioningserver/tftp.py 2012-07-26 08:06:54 +0000
@@ -77,8 +77,9 @@
"""
# TODO: update query defaults.
query = {
- b"menutitle": b"",
- b"kernelimage": b"",
+ b"menu_title": b"",
+ b"kernel": b"",
+ b"initrd": b"",
b"append": b"",
}
# Merge parameters from the generator URL.
