launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #12771
[Merge] lp:~racb/maas/arch-detect into lp:maas
Robie Basak has proposed merging lp:~racb/maas/arch-detect into lp:maas.
Commit message:
Add architecture detection
Rather than always responding to pxelinux.cfg/01-<mac> dynamically, instead cause nodes to fall back to pxelinux.cfg/default if the architecture of the machine with the provided MAC address is unknown. This allows non-Intel machines to fall back to pxelinux.cfg/default.<arch>-<subarch> in the middle, at which point serve a configuration for the correct non-Intel architecture.
This allows multiple architectures to be served the correct pxelinux.cfg configuration for enlistment without any needed intervention.
See bug 1041092 for details on pxelinux.cfg/default.<arch>-<subarch>.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1041092 in MAAS: "Add netboot architecture detection without using DHCP"
https://bugs.launchpad.net/maas/+bug/1041092
For more details, see:
https://code.launchpad.net/~racb/maas/arch-detect/+merge/127458
--
https://code.launchpad.net/~racb/maas/arch-detect/+merge/127458
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~racb/maas/arch-detect into lp:maas.
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2012-10-02 10:12:12 +0000
+++ src/maasserver/api.py 2012-10-02 11:00:28 +0000
@@ -1492,33 +1492,90 @@
return "poweroff"
+def get_node_from_mac_string(mac_string):
+ """Get a Node object from a MAC address string.
+
+ Returns a Node object or None if no node with the given MAC address exists.
+
+ :param mac_string: MAC address string in the form "12-34-56-78-9a-bc"
+ :return: Node object or None
+ """
+ if mac_string is None:
+ return None
+ macaddress = get_one(MACAddress.objects.filter(mac_address=mac_string))
+ if macaddress:
+ return macaddress.node
+ else:
+ return None
+
+
def pxeconfig(request):
"""Get the PXE configuration given a node's details.
Returns a JSON object corresponding to a
:class:`provisioningserver.kernel_opts.KernelParameters` instance.
+ This is now fairly decoupled from pxelinux's TFTP filename encoding
+ mechanism, with one notable exception. Call this function with (mac, arch,
+ subarch) and it will do the right thing. If details it needs are missing
+ (ie. arch/subarch missing when the MAC is supplied but unknown), then it
+ will as an exception return an HTTP NO_CONTENT (204) in the expectation
+ that this will be translated to a TFTP file not found and pxelinux (or an
+ emulator) will fall back to default.<arch>-<subarch> (in the case of an
+ alternate architecture emulator) or just straight to default (in the case
+ of native pxelinux on i386 or amd64). See bug 1041092 for details and
+ discussion.
+
:param mac: MAC address to produce a boot configuration for.
+ :param arch: Architecture name (in the pxelinux namespace, eg. 'arm' not
+ 'armhf').
+ :param subarch: Subarchitecture name (in the pxelinux namespace).
"""
- mac = get_mandatory_param(request.GET, 'mac')
+ node = get_node_from_mac_string(request.GET.get('mac', None))
- macaddress = get_one(MACAddress.objects.filter(mac_address=mac))
- if macaddress is None:
- # Default to i386 as a works-for-all solution. This will not support
- # non-x86 architectures, but for now this assumption holds.
- node = None
- arch, subarch = ARCHITECTURE.i386.split('/')
- preseed_url = compose_enlistment_preseed_url()
- hostname = 'maas-enlist'
- domain = Config.objects.get_config('enlistment_domain')
- else:
- node = macaddress.node
+ if node:
arch, subarch = node.architecture.split('/')
preseed_url = compose_preseed_url(node)
# The node's hostname may include a domain, but we ignore that
# and use the one from the nodegroup instead.
hostname = node.hostname.split('.', 1)[0]
domain = node.nodegroup.name
+ else:
+ try:
+ pxelinux_arch = request.GET['arch']
+ except KeyError:
+ if 'mac' in request.GET:
+ # Request was pxelinux.cfg/01-<mac>, so attempt fall back
+ # to pxelinux.cfg/default-<arch> for arch detection.
+ return HttpResponse(status=httplib.NO_CONTENT)
+ else:
+ # Request has already fallen back, so if arch is still not
+ # provided then use i386.
+ arch = ARCHITECTURE.i386.split('/')[0]
+ else:
+ # Map from pxelinux namespace architecture names to MAAS namespace
+ # architecture names. If this gets bigger, an external lookup table
+ # would make sense. But here is fine for something as trivial as it
+ # is right now.
+ if pxelinux_arch == 'arm':
+ arch = 'armhf'
+ else:
+ arch = pxelinux_arch
+
+ # Use subarch if supplied; otherwise assume 'generic'.
+ try:
+ pxelinux_subarch = request.GET['subarch']
+ except KeyError:
+ subarch = 'generic'
+ else:
+ # Map from pxelinux namespace subarchitecture names to MAAS
+ # namespace subarchitecture names. Right now this happens to be a
+ # 1-1 mapping.
+ subarch = pxelinux_subarch
+
+ preseed_url = compose_enlistment_preseed_url()
+ hostname = 'maas-enlist'
+ domain = Config.objects.get_config('enlistment_domain')
if node is None or node.status == NODE_STATUS.COMMISSIONING:
series = Config.objects.get_config('commissioning_distro_series')
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2012-10-02 10:12:12 +0000
+++ src/maasserver/tests/test_api.py 2012-10-02 11:00:28 +0000
@@ -2715,21 +2715,22 @@
class TestPXEConfigAPI(AnonAPITestCase):
- def get_params(self):
+ def get_mac_params(self):
return {'mac': factory.make_mac_address().mac_address}
- def get_optional_params(self):
- return ['mac']
+ def get_default_params(self):
+ return dict()
def get_pxeconfig(self, params=None):
"""Make a request to `pxeconfig`, and return its response dict."""
if params is None:
- params = self.get_params()
+ params = self.get_default_params()
response = self.client.get(reverse('pxeconfig'), params)
return json.loads(response.content)
def test_pxeconfig_returns_json(self):
- response = self.client.get(reverse('pxeconfig'), self.get_params())
+ response = self.client.get(reverse('pxeconfig'),
+ self.get_default_params())
self.assertThat(
(
response.status_code,
@@ -2751,7 +2752,26 @@
self.get_pxeconfig(),
ContainsAll(KernelParameters._fields))
- def test_pxeconfig_defaults_to_i386_when_node_unknown(self):
+ def test_pxeconfig_returns_data_for_known_node(self):
+ params = self.get_mac_params()
+ node = MACAddress.objects.get(mac_address=params['mac']).node
+ response = self.client.get(reverse('pxeconfig'), params)
+ self.assertEqual(httplib.OK, response.status_code)
+
+ def test_pxeconfig_returns_no_content_for_unknown_node(self):
+ params = dict(mac=factory.getRandomMACAddress(delimiter=b'-'))
+ response = self.client.get(reverse('pxeconfig'), params)
+ self.assertEqual(httplib.NO_CONTENT, response.status_code)
+
+ def test_pxeconfig_returns_data_for_detailed_but_unknown_node(self):
+ architecture = factory.getRandomEnum(ARCHITECTURE)
+ arch, subarch = architecture.split('/')
+ params = dict(mac=factory.getRandomMACAddress(delimiter=b'-'),
+ arch=arch, subarch=subarch)
+ response = self.client.get(reverse('pxeconfig'), params)
+ self.assertEqual(httplib.OK, response.status_code)
+
+ def test_pxeconfig_defaults_to_i386_for_default(self):
# As a lowest-common-denominator, i386 is chosen when the node is not
# yet known to MAAS.
expected_arch = tuple(ARCHITECTURE.i386.split('/'))
@@ -2760,16 +2780,12 @@
self.assertEqual(expected_arch, observed_arch)
def test_pxeconfig_uses_fixed_hostname_for_enlisting_node(self):
- new_mac = factory.getRandomMACAddress()
- self.assertEqual(
- 'maas-enlist',
- self.get_pxeconfig({'mac': new_mac}).get('hostname'))
+ self.assertEqual('maas-enlist', self.get_pxeconfig().get('hostname'))
def test_pxeconfig_uses_enlistment_domain_for_enlisting_node(self):
- new_mac = factory.getRandomMACAddress()
self.assertEqual(
Config.objects.get_config('enlistment_domain'),
- self.get_pxeconfig({'mac': new_mac}).get('domain'))
+ self.get_pxeconfig().get('domain'))
def test_pxeconfig_splits_domain_from_node_hostname(self):
host = factory.make_name('host')
@@ -2800,24 +2816,16 @@
kernel_opts, 'get_ephemeral_name',
FakeMethod(result=factory.getRandomString()))
- def test_pxeconfig_requires_mac_address(self):
- # The `mac` parameter is mandatory.
- self.silence_get_ephemeral_name()
- self.assertEqual(
- httplib.BAD_REQUEST,
- self.get_without_param("mac").status_code)
-
- def test_pxeconfig_has_enlistment_preseed_url_for_unknown_node(self):
- self.silence_get_ephemeral_name()
- params = self.get_params()
- params['mac'] = factory.getRandomMACAddress()
+ def test_pxeconfig_has_enlistment_preseed_url_for_default(self):
+ self.silence_get_ephemeral_name()
+ params = self.get_default_params()
response = self.client.get(reverse('pxeconfig'), params)
self.assertEqual(
compose_enlistment_preseed_url(),
json.loads(response.content)["preseed_url"])
def test_pxeconfig_has_preseed_url_for_known_node(self):
- params = self.get_params()
+ params = self.get_mac_params()
node = MACAddress.objects.get(mac_address=params['mac']).node
response = self.client.get(reverse('pxeconfig'), params)
self.assertEqual(
@@ -2852,7 +2860,8 @@
def test_pxeconfig_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())
+ response = self.client.get(reverse('pxeconfig'),
+ self.get_default_params())
self.assertEqual(
fake_boot_purpose,
json.loads(response.content)["purpose"])
=== modified file 'src/provisioningserver/tests/test_tftp.py'
--- src/provisioningserver/tests/test_tftp.py 2012-08-30 10:37:26 +0000
+++ src/provisioningserver/tests/test_tftp.py 2012-10-02 11:00:28 +0000
@@ -70,7 +70,9 @@
The path is intended to match `re_config_file`, and the components are
the expected groups from a match.
"""
- components = {"mac": factory.getRandomMACAddress(b"-")}
+ components = {"mac": factory.getRandomMACAddress(b"-"),
+ "arch": None,
+ "subarch": None}
config_path = compose_config_path(components["mac"])
return config_path, components
@@ -112,13 +114,15 @@
mac = 'aa-bb-cc-dd-ee-ff'
match = TFTPBackend.re_config_file.match('pxelinux.cfg/01-%s' % mac)
self.assertIsNotNone(match)
- self.assertEqual({'mac': mac}, match.groupdict())
+ self.assertEqual({'mac': mac, 'arch': None, 'subarch': None},
+ match.groupdict())
def test_re_config_file_matches_pxelinux_cfg_with_leading_slash(self):
mac = 'aa-bb-cc-dd-ee-ff'
match = TFTPBackend.re_config_file.match('/pxelinux.cfg/01-%s' % mac)
self.assertIsNotNone(match)
- self.assertEqual({'mac': mac}, match.groupdict())
+ self.assertEqual({'mac': mac, 'arch': None, 'subarch': None},
+ match.groupdict())
def test_re_config_file_does_not_match_non_config_file(self):
self.assertIsNone(
@@ -132,6 +136,29 @@
self.assertIsNone(
TFTPBackend.re_config_file.match('foo/01-aa-bb-cc-dd-ee-ff'))
+ def test_re_config_file_with_default(self):
+ match = TFTPBackend.re_config_file.match('pxelinux.cfg/default')
+ self.assertIsNotNone(match)
+ self.assertEqual({'mac': None, 'arch': None, 'subarch': None},
+ match.groupdict())
+
+ def test_re_config_file_with_default_arch(self):
+ arch = factory.make_name('arch', sep='')
+ match = TFTPBackend.re_config_file.match('pxelinux.cfg/default.%s' %
+ arch)
+ self.assertIsNotNone(match)
+ self.assertEqual({'mac': None, 'arch': arch, 'subarch': None},
+ match.groupdict())
+
+ def test_re_config_file_with_default_arch_and_subarch(self):
+ arch = factory.make_name('arch', sep='')
+ subarch = factory.make_name('subarch', sep='')
+ match = TFTPBackend.re_config_file.match(
+ 'pxelinux.cfg/default.%s-%s' % (arch, subarch))
+ self.assertIsNotNone(match)
+ self.assertEqual({'mac': None, 'arch': arch, 'subarch': subarch},
+ match.groupdict())
+
class TestTFTPBackend(TestCase):
"""Tests for `provisioningserver.tftp.TFTPBackend`."""
=== modified file 'src/provisioningserver/tftp.py'
--- src/provisioningserver/tftp.py 2012-09-12 19:56:23 +0000
+++ src/provisioningserver/tftp.py 2012-10-02 11:00:28 +0000
@@ -14,6 +14,7 @@
"TFTPBackend",
]
+import httplib
from io import BytesIO
from itertools import repeat
import json
@@ -32,7 +33,9 @@
FilesystemSynchronousBackend,
IReader,
)
+from tftp.errors import FileNotFound
from twisted.web.client import getPage
+import twisted.web.error
from zope.interface import implementer
@@ -89,9 +92,17 @@
^/*
pxelinux[.]cfg # PXELINUX expects this.
/
- {htype:02x} # ARP HTYPE.
- -
- (?P<mac>{re_mac_address.pattern}) # Capture MAC.
+ ( # either a MAC
+ {htype:02x} # ARP HTYPE.
+ -
+ (?P<mac>{re_mac_address.pattern}) # Capture MAC.
+ | # or "default"
+ default
+ ( # perhaps with specified arch
+ [.-](?P<arch>\w+) # arch
+ (-(?P<subarch>\w+))? # optional subarch
+ )?
+ )
$
'''.format(
htype=ARP_HTYPE.ETHERNET,
@@ -162,6 +173,25 @@
d.addCallback(BytesReader)
return d
+ @staticmethod
+ def get_page_errback(failure, file_name):
+ failure.trap(twisted.web.error.Error)
+ # This twisted.web.error.Error.status object ends up being a
+ # string for some reason, but the constants we can compare against
+ # (both in httplib and twisted.web.http) are ints.
+ try:
+ status_int = int(failure.value.status)
+ except ValueError:
+ # Assume that it's some other error and propagate it
+ return failure
+
+ if status_int == httplib.NO_CONTENT:
+ # Convert HTTP No Content to a TFTP file not found
+ raise FileNotFound(file_name)
+ else:
+ # Otherwise propogate the unknown error
+ return failure
+
@deferred
def get_reader(self, file_name):
"""See `IBackend.get_reader()`.
@@ -174,5 +204,10 @@
if config_file_match is None:
return super(TFTPBackend, self).get_reader(file_name)
else:
- params = config_file_match.groupdict()
- return self.get_config_reader(params)
+ # Do not include any element that has not matched (ie. is None)
+ params = {k: v
+ for k, v in config_file_match.groupdict().iteritems()
+ if v is not None}
+ d = self.get_config_reader(params)
+ d.addErrback(self.get_page_errback, file_name)
+ return d
