launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/tftp-bootloader-path into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/tftp-bootloader-path/+merge/112696

Well, that was easy.

As discussed with the team yesterday, and in some detail with Gavin, I'm centralizing the last vestige of ad-hoc TFTP path composition that's still lurking in the maas-import-pxe-files script.  This will involve a custom maas command similar to the one that installs a netboot image into the TFTP tree — but this time, for the pre-boot loader pxelinux.0.

In my drive for smaller, more focused branches I split off the work that centralizes the actual path composition into what may have ended up a slightly over-small, over-focused branch.  Which is what you see here.

The code may still grow.  PXE is technically an Intel standard that some other architectures merely simulate to a point.  An ARM system probably won't have a pxelinux.0 of its own; it'll just use the path to that file as given through DHCP for determining the directory that it should read its config from.  We're not sure if some architectures of that nature might require a file that actually exists in TFTP, but if we are going to create blank files for that purpose, then Julian wants them to be named “empty” for clarity.  Which would mean that we'd need some extra architecture-aware renaming logic, probably in here.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/tftp-bootloader-path/+merge/112696
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/tftp-bootloader-path into lp:maas.

=== modified file 'src/provisioningserver/pxe/tests/test_tftppath.py'
--- src/provisioningserver/pxe/tests/test_tftppath.py	2012-06-26 10:22:48 +0000
+++ src/provisioningserver/pxe/tests/test_tftppath.py	2012-06-29 05:05:34 +0000
@@ -18,6 +18,7 @@
 from maastesting.factory import factory
 from maastesting.testcase import TestCase
 from provisioningserver.pxe.tftppath import (
+    compose_bootloader_path,
     compose_config_path,
     compose_image_path,
     locate_tftp_path,
@@ -64,6 +65,20 @@
             compose_image_path(arch, subarch, release, purpose),
             Not(StartsWith(TFTPROOT)))
 
+    def test_compose_bootloader_path_follows_maas_pxe_directory_layout(self):
+        arch = factory.make_name('arch')
+        subarch = factory.make_name('subarch')
+        self.assertEqual(
+            '/maas/%s/%s/pxelinux.0' % (arch, subarch),
+            compose_bootloader_path(arch, subarch))
+
+    def test_compose_bootloader_path_does_not_include_tftp_root(self):
+        arch = factory.make_name('arch')
+        subarch = factory.make_name('subarch')
+        self.assertThat(
+            compose_bootloader_path(arch, subarch),
+            Not(StartsWith(TFTPROOT)))
+
     def test_locate_tftp_path_prefixes_tftp_root_by_default(self):
         pxefile = factory.make_name('pxefile')
         self.assertEqual(

=== modified file 'src/provisioningserver/pxe/tftppath.py'
--- src/provisioningserver/pxe/tftppath.py	2012-06-26 07:00:23 +0000
+++ src/provisioningserver/pxe/tftppath.py	2012-06-29 05:05:34 +0000
@@ -11,6 +11,7 @@
 
 __metaclass__ = type
 __all__ = [
+    'compose_bootloader_path',
     'compose_config_path',
     'compose_image_path',
     'locate_tftp_path',
@@ -21,6 +22,11 @@
 from celeryconfig import TFTPROOT
 
 
+def compose_bootloader_path(arch, subarch):
+    """Compose the TFTP path for a PXE pre-boot loader."""
+    return '/'.join(['/maas', arch, subarch, 'pxelinux.0'])
+
+
 def compose_config_path(arch, subarch, name):
     """Compose the TFTP path for a PXE configuration file.