launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #10448
[Merge] lp:~allenap/maas/kernel-initrd-parameters into lp:maas
Gavin Panella has proposed merging lp:~allenap/maas/kernel-initrd-parameters into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~allenap/maas/kernel-initrd-parameters/+merge/117432
Calculate the kernel and initrd options in the pxeconfig view, instead of relying on them being passed from the dynamic TFTP server. The latter does not have all the necessary information at its disposal.
--
https://code.launchpad.net/~allenap/maas/kernel-initrd-parameters/+merge/117432
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/kernel-initrd-parameters into lp:maas.
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2012-07-31 09:10:44 +0000
+++ src/maasserver/api.py 2012-07-31 13:32:22 +0000
@@ -1061,52 +1061,72 @@
query={'op': 'get_preseed'})
-def compose_preseed_kernel_opt(mac):
- """Compose a kernel option for preseed URL for node that owns `mac`."""
- macaddress_match = MACAddress.objects.filter(mac_address=mac)
- if len(macaddress_match) == 0:
+def compose_preseed_kernel_opt(node):
+ """Compose a kernel option for preseed URL for given `node`.
+
+ :param mac_address: A `Node`, or `None`.
+ """
+ if node is None:
preseed_url = compose_enlistment_preseed_url()
else:
- [macaddress] = macaddress_match
- preseed_url = compose_preseed_url(macaddress.node)
+ preseed_url = compose_preseed_url(node)
return "auto url=%s" % preseed_url
+def get_boot_purpose(node):
+ """Return a suitable "purpose" for this boot, e.g. "install"."""
+ if node is None:
+ # This node is enlisting, for which we use a commissioning
+ # image. TODO: Do we?
+ return "commissioning"
+ elif node.status == NODE_STATUS.COMMISSIONING:
+ # It is commissioning.
+ return "commissioning"
+ elif node.status == NODE_STATUS.ALLOCATED:
+ # Install the node if netboot is enabled, otherwise boot locally.
+ if node.netboot:
+ return "install"
+ else:
+ return "local" # TODO: Investigate.
+ else:
+ # Just poweroff? TODO: Investigate. Perhaps even send an IPMI signal
+ # to turn off power.
+ return "poweroff"
+
+
def pxeconfig(request):
"""Get the PXE configuration given a node's details.
: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 arch: Main machine architecture.
+ :param subarch: Sub-architecture, or "generic" if there is none.
:param 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.
-
- In addition, the following parameters are expected, but are not used:
-
- :param arch: Main machine architecture.
- :param subarch: Sub-architecture, or "generic" if there is none.
"""
- title = get_mandatory_param(request.GET, 'title')
- kernel = get_mandatory_param(request.GET, 'kernel')
- initrd = get_mandatory_param(request.GET, 'initrd')
- append = get_mandatory_param(request.GET, 'append')
mac = request.GET.get('mac', None)
-
- # The following two parameters - arch and subarch - typically must be
- # supplied, but are unused right now.
arch = get_mandatory_param(request.GET, 'arch')
subarch = request.GET.get('subarch', 'generic')
- arch, subarch # Suppress lint warnings.
+ title = get_mandatory_param(request.GET, 'title')
+ append = get_mandatory_param(request.GET, 'append')
+
+ # See if we have a record of this MAC address, and thus node.
+ try:
+ macaddress = MACAddress.objects.get(mac_address=mac)
+ except MACAddress.DoesNotExist:
+ macaddress = node = None
+ else:
+ node = macaddress.node
# In addition to the "append" parameter, also add a URL for the
# node's preseed to the kernel command line.
- append = "%s %s" % (append, compose_preseed_kernel_opt(mac))
+ append = "%s %s" % (append, compose_preseed_kernel_opt(node))
+ # TODO: don't hard-code release.
return HttpResponse(
render_pxe_config(
- title=title, kernel=kernel,
- initrd=initrd, append=append),
+ title=title, arch=arch, subarch=subarch, release="precise",
+ purpose=get_boot_purpose(node), append=append),
content_type="text/plain; charset=utf-8")
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2012-07-31 09:10:44 +0000
+++ src/maasserver/tests/test_api.py 2012-07-31 13:32:22 +0000
@@ -87,6 +87,7 @@
POWER_TYPE_CHOICES,
)
from testtools.matchers import (
+ Contains,
Equals,
MatchesListwise,
StartsWith,
@@ -2261,8 +2262,6 @@
'subarch': "armadaxp",
'mac': factory.make_mac_address().mac_address,
'title': factory.make_name("Menu"),
- 'kernel': factory.make_name("/my/kernel"),
- 'initrd': factory.make_name("/my/initrd"),
'append': factory.make_name("append"),
}
@@ -2300,8 +2299,6 @@
'subarch': httplib.OK,
'mac': httplib.OK,
'title': httplib.BAD_REQUEST,
- 'kernel': httplib.BAD_REQUEST,
- 'initrd': httplib.BAD_REQUEST,
'append': httplib.BAD_REQUEST,
}
observed = {
@@ -2337,15 +2334,15 @@
api.compose_preseed_url(node), StartsWith(url))
def test_compose_preseed_kernel_opt_returns_option_for_known_node(self):
- mac = factory.make_mac_address()
+ node = factory.make_node()
self.assertEqual(
- "auto url=%s" % api.compose_preseed_url(mac.node),
- api.compose_preseed_kernel_opt(mac.mac_address))
+ "auto url=%s" % api.compose_preseed_url(node),
+ api.compose_preseed_kernel_opt(node))
def test_compose_preseed_kernel_opt_returns_option_for_unknown_node(self):
self.assertEqual(
"auto url=%s" % api.compose_enlistment_preseed_url(),
- api.compose_preseed_kernel_opt(factory.getRandomMACAddress()))
+ api.compose_preseed_kernel_opt(None))
def test_pxe_config_appends_enlistment_preseed_url_for_unknown_node(self):
params = self.get_params()
@@ -2359,6 +2356,37 @@
response = self.client.get(reverse('pxeconfig'), params)
self.assertIn(api.compose_preseed_url(node), response.content)
+ def test_get_boot_purpose_unknown_node(self):
+ # A node that's not yet known to MAAS is assumed to be enlisting,
+ # which uses a "commissioning" image.
+ self.assertEqual("commissioning", api.get_boot_purpose(None))
+
+ def test_get_boot_purpose_known_node(self):
+ # The following table shows the expected boot "purpose" for each set
+ # of node parameters.
+ options = [
+ ("poweroff", {"status": NODE_STATUS.DECLARED}),
+ ("commissioning", {"status": NODE_STATUS.COMMISSIONING}),
+ ("poweroff", {"status": NODE_STATUS.FAILED_TESTS}),
+ ("poweroff", {"status": NODE_STATUS.MISSING}),
+ ("poweroff", {"status": NODE_STATUS.READY}),
+ ("poweroff", {"status": NODE_STATUS.RESERVED}),
+ ("install", {"status": NODE_STATUS.ALLOCATED, "netboot": True}),
+ ("local", {"status": NODE_STATUS.ALLOCATED, "netboot": False}),
+ ("poweroff", {"status": NODE_STATUS.RETIRED}),
+ ]
+ node = factory.make_node()
+ for purpose, parameters in options:
+ for name, value in parameters.items():
+ setattr(node, name, value)
+ self.assertEqual(purpose, api.get_boot_purpose(node))
+
+ def test_pxe_config_uses_boot_purpose(self):
+ fake_boot_purpose = factory.make_name("purpose")
+ self.patch(api, "get_boot_purpose", lambda node: fake_boot_purpose)
+ response = self.client.get(reverse('pxeconfig'), self.get_params())
+ self.assertThat(response.content, Contains(fake_boot_purpose))
+
class TestNodeGroupsAPI(APITestCase):
=== modified file 'src/provisioningserver/pxe/config.py'
--- src/provisioningserver/pxe/config.py 2012-07-30 20:59:31 +0000
+++ src/provisioningserver/pxe/config.py 2012-07-31 13:32:22 +0000
@@ -22,6 +22,7 @@
from os import path
+from provisioningserver.pxe.tftppath import compose_image_path
import tempita
@@ -30,13 +31,17 @@
template = tempita.Template.from_filename(template_filename, encoding="UTF-8")
-def render_pxe_config(title, kernel, initrd, append):
+def render_pxe_config(title, arch, subarch, release, purpose, append):
"""Render a PXE configuration file as a unicode string.
:param 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 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.
"""
+ image_dir = compose_image_path(arch, subarch, release, purpose)
return template.substitute(
- title=title, kernel=kernel, initrd=initrd, append=append)
+ title=title, kernel="{}/kernel".format(image_dir),
+ initrd="{}/initrd.gz".format(image_dir), append=append)
=== modified file 'src/provisioningserver/pxe/tests/test_config.py'
--- src/provisioningserver/pxe/tests/test_config.py 2012-07-31 08:34:59 +0000
+++ src/provisioningserver/pxe/tests/test_config.py 2012-07-31 13:32:22 +0000
@@ -12,13 +12,16 @@
__metaclass__ = type
__all__ = []
+import re
+
from maastesting.factory import factory
from maastesting.testcase import TestCase
from provisioningserver.pxe.config import render_pxe_config
+from provisioningserver.pxe.tftppath import compose_image_path
from testtools.matchers import (
- Contains,
IsInstance,
MatchesAll,
+ MatchesRegex,
StartsWith,
)
@@ -31,8 +34,10 @@
# correctly rendered.
options = {
"title": factory.make_name("title"),
- "kernel": factory.make_name("kernel"),
- "initrd": factory.make_name("initrd"),
+ "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"),
}
output = render_pxe_config(**options)
@@ -41,6 +46,21 @@
# The template has rendered without error. PXELINUX configurations
# typically start with a DEFAULT line.
self.assertThat(output, StartsWith("DEFAULT "))
- # All of the values put in are included somewhere in the output.
- expected = (Contains(value) for value in options.values())
- self.assertThat(output, MatchesAll(*expected))
+ # 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"])
+ self.assertThat(
+ output, MatchesAll(
+ MatchesRegex(
+ r'.*^MENU TITLE %s$' % re.escape(options["title"]),
+ re.MULTILINE | re.DOTALL),
+ MatchesRegex(
+ r'.*^\s+KERNEL %s/kernel$' % re.escape(image_dir),
+ re.MULTILINE | re.DOTALL),
+ MatchesRegex(
+ r'.*^\s+INITRD %s/initrd[.]gz$' % re.escape(image_dir),
+ re.MULTILINE | re.DOTALL),
+ MatchesRegex(
+ r'.*^\s+APPEND %s$' % re.escape(options["append"]),
+ re.MULTILINE | re.DOTALL)))
=== modified file 'src/provisioningserver/tests/test_tftp.py'
--- src/provisioningserver/tests/test_tftp.py 2012-07-30 21:28:47 +0000
+++ src/provisioningserver/tests/test_tftp.py 2012-07-31 13:32:22 +0000
@@ -131,16 +131,10 @@
arch = factory.make_name("arch").encode("ascii")
subarch = factory.make_name("subarch").encode("ascii")
mac = factory.getRandomMACAddress(b"-")
- kernel = factory.make_name("kernel").encode("ascii")
- initrd = factory.make_name("initrd").encode("ascii")
title = factory.make_name("menu-title").encode("ascii")
append = factory.make_name("append").encode("ascii")
- backend_url = b"http://example.com/?"; + urlencode({
- b"kernel": kernel,
- b"initrd": initrd,
- b"title": title,
- b"append": append,
- })
+ backend_url = b"http://example.com/?"; + (
+ urlencode({b"title": 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, "mac": mac}
@@ -149,8 +143,6 @@
query = parse_qsl(generator_url.query)
query_expected = [
("append", append),
- ("kernel", kernel),
- ("initrd", initrd),
("arch", arch),
("subarch", subarch),
("title", title),
=== modified file 'src/provisioningserver/tftp.py'
--- src/provisioningserver/tftp.py 2012-07-30 21:28:47 +0000
+++ src/provisioningserver/tftp.py 2012-07-31 13:32:22 +0000
@@ -111,8 +111,6 @@
# TODO: update query defaults.
query = {
b"title": b"",
- b"kernel": b"",
- b"initrd": b"",
b"append": b"",
}
# Merge parameters from the generator URL.
