launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #10613
[Merge] lp:~jtv/maas/basic-kernel-opts into lp:maas
Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/basic-kernel-opts into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/maas/basic-kernel-opts/+merge/118340
My apologies for the size of this thing: I'm really trying to do this in small, incremental branches but sometimes you discover too late how widespread the implications of a change are.
This is ongoing work to support Julian's attempts at getting a node up without Cobbler. The options are not quite complete: commissioning configs get a bunch of additional kernel options, and nodes in other statuses get “netcfg/choose_interface=auto” instead. But this branch should lay the groundwork for that. See? Serious attempt at incremental branches.
Some of the preseed-URL functions that were previously in api.py now have a home of their own. This shrinks api.py a bit and takes away some of the lower-level distractions.
One really important change is this: in the old code, the TFTP server suggested a basic APPEND line for the PXE config, and passed it to the pxeconfig API call which then appended kernel options of its own. After sanity-checking with Julian, I simplified this: the APPEND line is now simply generated by pxeconfig, rather than acting as an in/out parameter, and the TFTP server no longer has any knowledge of it.
Jeroen
--
https://code.launchpad.net/~jtv/maas/basic-kernel-opts/+merge/118340
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/basic-kernel-opts into lp:maas.
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2012-08-02 14:55:12 +0000
+++ src/maasserver/api.py 2012-08-06 11:57:19 +0000
@@ -115,6 +115,7 @@
get_node_create_form,
get_node_edit_form,
)
+from maasserver.kernel_opts import compose_kernel_command_line
from maasserver.models import (
Config,
DHCPLease,
@@ -123,7 +124,6 @@
Node,
NodeGroup,
)
-from maasserver.utils import absolute_reverse
from piston.doc import generate_doc
from piston.handler import (
AnonymousBaseHandler,
@@ -1042,36 +1042,6 @@
context_instance=RequestContext(request))
-def compose_enlistment_preseed_url():
- """Compose enlistment preseed URL."""
- # Always uses the latest version of the metadata API.
- version = 'latest'
- return absolute_reverse(
- 'metadata-enlist-preseed', args=[version],
- query={'op': 'get_enlist_preseed'})
-
-
-def compose_preseed_url(node):
- """Compose a metadata URL for `node`'s preseed data."""
- # Always uses the latest version of the metadata API.
- version = 'latest'
- return absolute_reverse(
- 'metadata-node-by-id', args=[version, node.system_id],
- query={'op': 'get_preseed'})
-
-
-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:
- 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"."""
# XXX: allenap bug=1031406 2012-07-31: The boot purpose is still in
@@ -1106,13 +1076,10 @@
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 append: Additional parameters to append to the kernel command
- line.
"""
mac = request.GET.get('mac', None)
arch = get_mandatory_param(request.GET, 'arch')
subarch = request.GET.get('subarch', 'generic')
- append = get_mandatory_param(request.GET, 'append')
# See if we have a record of this MAC address, and thus node.
try:
@@ -1122,9 +1089,8 @@
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(node))
+ purpose = get_boot_purpose(node)
+ append = compose_kernel_command_line(node, arch, subarch, purpose=purpose)
# XXX: allenap 2012-07-31 bug=1013146: 'precise' is hardcoded here.
release = "precise"
=== added file 'src/maasserver/kernel_opts.py'
--- src/maasserver/kernel_opts.py 1970-01-01 00:00:00 +0000
+++ src/maasserver/kernel_opts.py 2012-08-06 11:57:19 +0000
@@ -0,0 +1,108 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Generate kernel command-line options for inclusion in PXE configs."""
+
+from __future__ import (
+ absolute_import,
+ print_function,
+ unicode_literals,
+ )
+
+__metaclass__ = type
+__all__ = [
+ 'compose_kernel_command_line',
+ ]
+
+from maasserver.server_address import get_maas_facing_server_address
+from maasserver.utils import absolute_reverse
+from provisioningserver.pxe.tftppath import compose_image_path
+
+
+def compose_initrd_opt(arch, subarch, release, purpose):
+ path = "%s/initrd.gz" % compose_image_path(arch, subarch, release, purpose)
+ return "initrd=%s" % path
+
+
+def compose_enlistment_preseed_url():
+ """Compose enlistment preseed URL."""
+ # Always uses the latest version of the metadata API.
+ version = 'latest'
+ return absolute_reverse(
+ 'metadata-enlist-preseed', args=[version],
+ query={'op': 'get_enlist_preseed'})
+
+
+def compose_preseed_url(node):
+ """Compose a metadata URL for `node`'s preseed data."""
+ # Always uses the latest version of the metadata API.
+ version = 'latest'
+ return absolute_reverse(
+ 'metadata-node-by-id', args=[version, node.system_id],
+ query={'op': 'get_preseed'})
+
+
+def compose_preseed_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:
+ preseed_url = compose_preseed_url(node)
+ return "auto url=%s" % preseed_url
+
+
+def compose_suite_opt(release):
+ return "suite=%s" % release
+
+
+def compose_hostname_opt(node):
+ if node is None:
+ # Not a known host; still needs enlisting. Make up a name.
+ hostname = 'maas-enlist'
+ else:
+ hostname = node.hostname
+ return "hostname=%s" % hostname
+
+
+def compose_domain_opt(node):
+ # TODO: This is probably not enough!
+ domain = 'local.lan'
+ return "domain=%s" % domain
+
+
+def compose_locale_opt():
+ locale = 'en_US'
+ return "locale=%s" % locale
+
+
+def compose_logging_opts():
+ return [
+ 'log_host=%s' % get_maas_facing_server_address(),
+ 'log_port=%s' % 514,
+ 'text priority=%s' % 'critical',
+ ]
+
+
+def compose_kernel_command_line(node, arch, subarch, purpose):
+ """Generate a line of kernel options for booting `node`.
+
+ Include these options in the PXE config file's APPEND argument.
+
+ The node may be None, in which case it will boot into enlistment.
+ """
+ # TODO: Stop hard-coding this.
+ release = 'precise'
+
+ options = [
+ compose_initrd_opt(arch, subarch, release, purpose),
+ compose_preseed_opt(node),
+ compose_suite_opt(release),
+ compose_hostname_opt(node),
+ compose_domain_opt(node),
+ compose_locale_opt(),
+ ]
+ options += compose_logging_opts()
+ return ' '.join(options)
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2012-08-06 05:01:20 +0000
+++ src/maasserver/tests/test_api.py 2012-08-06 11:57:19 +0000
@@ -49,6 +49,10 @@
NODE_STATUS_CHOICES_DICT,
)
from maasserver.exceptions import Unauthorized
+from maasserver.kernel_opts import (
+ compose_enlistment_preseed_url,
+ compose_preseed_url,
+ )
from maasserver.models import (
Config,
DHCPLease,
@@ -60,10 +64,6 @@
create_auth_token,
get_auth_tokens,
)
-from maasserver.preseed import (
- get_enlist_preseed,
- get_preseed,
- )
from maasserver.testing import (
reload_object,
reload_objects,
@@ -2262,10 +2262,6 @@
'arch': "armhf",
'subarch': "armadaxp",
'mac': factory.make_mac_address().mac_address,
- 'append': "%s=%s" % (
- factory.make_name("opt"),
- factory.make_name("value"),
- ),
}
def get_optional_params(self):
@@ -2317,22 +2313,16 @@
def test_pxeconfig_adds_some_parameters(self):
params_in = self.get_params()
- optional_params = {'release', 'purpose'}
- for param in optional_params:
+ added_params = {'append', 'release', 'purpose'}
+ for param in added_params:
if param in params_in:
del params_in[param]
params_out = self.get_pxeconfig(params_in)
self.assertEqual(
- set(params_out).difference(params_in), optional_params)
+ set(params_out).difference(params_in), added_params)
# The release is always "precise".
self.assertEqual('precise', params_out['release'])
-
- def test_pxeconfig_appends_to_append_from_request(self):
- params_in = self.get_params()
- params_out = self.get_pxeconfig(params_in)
- self.assertThat(
- params_out['append'],
- Contains("%(append)s auto url=http://"; % params_in))
+ self.assertThat(params_out['append'], Contains("auto url=http://";))
def get_without_param(self, param):
"""Request a `pxeconfig()` response, but omit `param` from request."""
@@ -2348,7 +2338,6 @@
'arch': httplib.BAD_REQUEST,
'subarch': httplib.OK,
'mac': httplib.OK,
- 'append': httplib.BAD_REQUEST,
}
observed_response = {
param: self.get_without_param(param).status_code
@@ -2357,49 +2346,12 @@
self.assertEqual(
expected_response_to_missing_parameter, observed_response)
- def test_compose_enlistment_preseed_url_links_to_enlistment_preseed(self):
- response = self.client.get(api.compose_enlistment_preseed_url())
- self.assertEqual(
- (httplib.OK, get_enlist_preseed()),
- (response.status_code, response.content))
-
- def test_compose_enlistment_preseed_url_returns_absolute_link(self):
- url = 'http://%s' % factory.make_name('host')
- self.patch(settings, 'DEFAULT_MAAS_URL', url)
- self.assertThat(
- api.compose_enlistment_preseed_url(), StartsWith(url))
-
- def test_compose_preseed_url_links_to_preseed_for_node(self):
- node = factory.make_node()
- response = self.client.get(api.compose_preseed_url(node))
- self.assertEqual(
- (httplib.OK, get_preseed(node)),
- (response.status_code, response.content))
-
- def test_compose_preseed_url_returns_absolute_link(self):
- url = 'http://%s' % factory.make_name('host')
- self.patch(settings, 'DEFAULT_MAAS_URL', url)
- node = factory.make_node()
- self.assertThat(
- api.compose_preseed_url(node), StartsWith(url))
-
- def test_compose_preseed_kernel_opt_returns_option_for_known_node(self):
- node = factory.make_node()
- self.assertEqual(
- "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(None))
-
def test_pxeconfig_appends_enlistment_preseed_url_for_unknown_node(self):
params = self.get_params()
params['mac'] = factory.getRandomMACAddress()
response = self.client.get(reverse('pxeconfig'), params)
self.assertIn(
- api.compose_enlistment_preseed_url(),
+ compose_enlistment_preseed_url(),
json.loads(response.content)["append"])
def test_pxeconfig_appends_preseed_url_for_known_node(self):
@@ -2407,7 +2359,7 @@
node = MACAddress.objects.get(mac_address=params['mac']).node
response = self.client.get(reverse('pxeconfig'), params)
self.assertIn(
- api.compose_preseed_url(node),
+ compose_preseed_url(node),
json.loads(response.content)["append"])
def test_get_boot_purpose_unknown_node(self):
=== added file 'src/maasserver/tests/test_kernel_opts.py'
--- src/maasserver/tests/test_kernel_opts.py 1970-01-01 00:00:00 +0000
+++ src/maasserver/tests/test_kernel_opts.py 2012-08-06 11:57:19 +0000
@@ -0,0 +1,158 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test composition of kernel command lines."""
+
+from __future__ import (
+ absolute_import,
+ print_function,
+ unicode_literals,
+ )
+
+__metaclass__ = type
+__all__ = []
+
+import httplib
+
+from django.conf import settings
+from maasserver import server_address
+from maasserver.api import get_boot_purpose
+from maasserver.kernel_opts import (
+ compose_enlistment_preseed_url,
+ compose_kernel_command_line,
+ compose_preseed_opt,
+ compose_preseed_url,
+ )
+from maasserver.preseed import (
+ get_enlist_preseed,
+ get_preseed,
+ )
+from maasserver.testing.factory import factory
+from maasserver.testing.testcase import TestCase
+from maastesting.matchers import ContainsAll
+from provisioningserver.pxe.tftppath import compose_image_path
+from testtools.matchers import StartsWith
+
+
+class TestKernelOpts(TestCase):
+
+ def test_compose_kernel_command_line_accepts_None_for_unknown_node(self):
+ self.assertIn(
+ 'suite=precise',
+ compose_kernel_command_line(
+ None, factory.make_name('arch'), factory.make_name('subarch'),
+ purpose=factory.make_name('purpose')))
+
+ def test_compose_kernel_command_line_includes_preseed_url(self):
+ node = factory.make_node()
+ self.assertIn(
+ "auto url=%s" % compose_preseed_url(node),
+ compose_kernel_command_line(
+ node, node.architecture, 'generic',
+ purpose=factory.make_name('purpose')))
+
+ def test_compose_kernel_command_line_includes_initrd(self):
+ node = factory.make_node()
+ initrd_path = compose_image_path(
+ node.architecture, 'generic', 'precise',
+ purpose=get_boot_purpose(node))
+ self.assertIn(
+ "initrd=%s" % initrd_path,
+ compose_kernel_command_line(
+ node, node.architecture, 'generic',
+ purpose=get_boot_purpose(node)))
+
+ def test_compose_kernel_command_line_includes_suite(self):
+ # At the moment, the OS release we use is hard-coded to "precise."
+ node = factory.make_node()
+ suite = "precise"
+ self.assertIn(
+ "suite=%s" % suite,
+ compose_kernel_command_line(
+ node, node.architecture, 'generic',
+ purpose=factory.make_name('purpose')))
+
+ def test_compose_kernel_command_line_includes_hostname_and_domain(self):
+ node = factory.make_node()
+ # Cobbler seems to hard-code domain to "local.lan"; we may want
+ # to change it, and update this test.
+ domain = "local.lan"
+ self.assertThat(
+ compose_kernel_command_line(
+ node, node.architecture, 'generic',
+ purpose=factory.make_name('purpose')),
+ ContainsAll([
+ "hostname=%s" % node.hostname,
+ "domain=%s" % domain,
+ ]))
+
+ def test_compose_kernel_command_line_makes_up_hostname_for_new_node(self):
+ dummy_hostname = 'maas-enlist'
+ self.assertIn(
+ "hostname=%s" % dummy_hostname,
+ compose_kernel_command_line(
+ None, factory.make_name('arch'),
+ factory.make_name('subarch'),
+ purpose=factory.make_name('purpose')))
+
+ def test_compose_kernel_command_line_includes_locale(self):
+ node = factory.make_node()
+ locale = "en_US"
+ self.assertIn(
+ "locale=%s" % locale,
+ compose_kernel_command_line(
+ node, node.architecture, 'generic',
+ purpose=factory.make_name('purpose')))
+
+ def test_compose_kernel_command_line_includes_log_settings(self):
+ node = factory.make_node()
+ log_host = factory.getRandomIPAddress()
+ self.patch(settings, 'DEFAULT_MAAS_URL', 'http://%s/' % log_host)
+ # Port 514 (UDP) is syslog.
+ log_port = "514"
+ text_priority = "critical"
+ self.assertThat(
+ compose_kernel_command_line(
+ node, node.architecture, 'generic',
+ purpose=factory.make_name('purpose')),
+ ContainsAll([
+ "log_host=%s" % log_host,
+ "log_port=%s" % log_port,
+ "text priority=%s" % text_priority,
+ ]))
+
+ def test_compose_enlistment_preseed_url_links_to_enlistment_preseed(self):
+ response = self.client.get(compose_enlistment_preseed_url())
+ self.assertEqual(
+ (httplib.OK, get_enlist_preseed()),
+ (response.status_code, response.content))
+
+ def test_compose_enlistment_preseed_url_returns_absolute_link(self):
+ url = 'http://%s' % factory.make_name('host')
+ self.patch(settings, 'DEFAULT_MAAS_URL', url)
+ self.assertThat(
+ compose_enlistment_preseed_url(), StartsWith(url))
+
+ def test_compose_preseed_url_links_to_preseed_for_node(self):
+ node = factory.make_node()
+ response = self.client.get(compose_preseed_url(node))
+ self.assertEqual(
+ (httplib.OK, get_preseed(node)),
+ (response.status_code, response.content))
+
+ def test_compose_preseed_url_returns_absolute_link(self):
+ server_ip = server_address.get_maas_facing_server_address()
+ self.assertThat(
+ compose_preseed_url(factory.make_node()),
+ StartsWith('http://%s' % server_ip))
+
+ def test_compose_preseed_kernel_opt_returns_option_for_known_node(self):
+ node = factory.make_node()
+ self.assertEqual(
+ "auto url=%s" % compose_preseed_url(node),
+ compose_preseed_opt(node))
+
+ def test_compose_preseed_kernel_opt_returns_option_for_unknown_node(self):
+ self.assertEqual(
+ "auto url=%s" % compose_enlistment_preseed_url(),
+ compose_preseed_opt(None))
=== modified file 'src/provisioningserver/tftp.py'
--- src/provisioningserver/tftp.py 2012-08-02 16:15:06 +0000
+++ src/provisioningserver/tftp.py 2012-08-06 11:57:19 +0000
@@ -53,27 +53,35 @@
class TFTPBackend(FilesystemSynchronousBackend):
"""A partially dynamic read-only TFTP server.
- Requests for PXE configurations are forwarded to a configurable URL. See
- `re_config_file` and `re_mac_address`.
-
- This must be very selective about which requests to forward, because
+ Static files such as kernels and initrds, as well as any non-MAAS files
+ that the system may already be set up to serve, are served up normally.
+ But PXE configurations are generated on the fly.
+
+ When a PXE configuration file is requested, the server asynchronously
+ requests the appropriate parameters from the API (at a configurable
+ "generator URL") and generates a config file based on those.
+
+ The regular expressions `re_config_file` and `re_mac_address` specify
+ which files the server generates on the fly. Any other requests are
+ passed on to the filesystem.
+
+ Passing requests on to the API must be done very selectively, because
failures cause the boot process to halt. This is why the expression for
- matching the MAC address is so narrowly defined; PXELINUX attempts to
- fetch files at many similar paths, and this must respond to only one
- pattern.
+ matching the MAC address is so narrowly defined: PXELINUX attempts to
+ fetch files at many similar paths which must not be passed on.
"""
get_page = staticmethod(getPage)
render_pxe_config = staticmethod(render_pxe_config)
- # This is how PXELINUX represents a MAC address. See
- # http://www.syslinux.org/wiki/index.php/PXELINUX.
+ # PXELINUX represents a MAC address in IEEE 802 hyphen-separated
+ # format. See http://www.syslinux.org/wiki/index.php/PXELINUX.
re_mac_address_octet = r'[0-9a-f]{2}'
re_mac_address = re.compile(
"-".join(repeat(re_mac_address_octet, 6)))
# We assume that the ARP HTYPE (hardware type) that PXELINUX sends is
- # alway Ethernet.
+ # always Ethernet.
re_config_file = re.compile(
r'''
^/?
@@ -114,8 +122,7 @@
:param params: A dict, or iterable suitable for updating a dict, of
additional query parameters.
"""
- # Query defaults.
- query = {b"append": b""}
+ query = {}
# Merge parameters from the generator URL.
query.update(parse_qsl(self.generator_url.query))
# Merge parameters obtained from the request.
Follow ups
