Merge lp:~julian-edwards/maas/utopic-syslinux-location into lp:~maas-committers/maas/trunk

[Julian Edwards]

Make the boot images importer look through different system directories when copying the pxe files to the TFTP root. This is to enable the same code to work for different Ubuntu releases which may store the files in different places.

[Raphaël Badin (rvb)]

Nice! Thanks for fixing this bug!

review: Approve

[Raphaël Badin (rvb)]

I'm landing this because I'd like very much to make sure the Utopic package passes the CI tests today.

[Raphaël Badin (rvb)]

41 + # locate_bootloader might return None but happy to let that
42 + # traceback here is it should never happen unless there's a
43 + # serious problem with packaging.

Now that I've seen the traceback IRL (packaging problem, unrelated to your change), I can't say it was very helpful: http://paste.ubuntu.com/7674410/

[Julian Edwards (julian-edwards)]

How the hell?

[Julian Edwards (julian-edwards)]

On 23/06/14 09:36, Julian Edwards wrote:
> How the hell?

Aff, saw your other MP. I was about to show you the canonistack
instance "ls" output I had from Friday, but my instances have done yet
another disappearing act.

[Raphaël Badin (rvb)]

On 06/23/2014 03:24 AM, Julian Edwards wrote:
> On 23/06/14 09:36, Julian Edwards wrote:
>> How the hell?
>
> Aff, saw your other MP. I was about to show you the canonistack
> instance "ls" output I had from Friday, but my instances have done yet
> another disappearing act.

Yeah, this is fixed now. The Utopic CI job is still failing… but the
image import step works (it's failing to enlist nodes now which happens
*after* the images have been imported).

=== modified file 'src/provisioningserver/boot/pxe.py'
--- src/provisioningserver/boot/pxe.py	2014-06-10 14:45:14 +0000
+++ src/provisioningserver/boot/pxe.py	2014-06-20 04:51:25 +0000
@@ -27,10 +27,13 @@
     )
 from provisioningserver.boot.install_bootloader import install_bootloader
 
-
+# Bootloader file names to install.
 BOOTLOADERS = ['pxelinux.0', 'chain.c32', 'ifcpu64.c32']
 
-BOOTLOADER_DIR = '/usr/lib/syslinux'
+# Possible locations in which to find the files. Search these in this
+# order for each file.  (This exists because locations differ across
+# Ubuntu releases)
+BOOTLOADER_DIRS = ['/usr/lib/syslinux', '/usr/lib/syslinux/bios']
 
 
 class ARP_HTYPE:
@@ -107,11 +110,25 @@
         namespace = self.compose_template_namespace(kernel_params)
         return BytesReader(template.substitute(namespace).encode("utf-8"))
 
+    def locate_bootloader(self, bootloader):
+        """Search BOOTLOADER_DIRS for bootloader.
+
+        :return: The full file path where the bootloader was found, or None.
+        """
+        for dir in BOOTLOADER_DIRS:
+            filename = os.path.join(dir, bootloader)
+            if os.path.exists(filename):
+                return filename
+        return None
+
     def install_bootloader(self, destination):
         """Installs the required files for PXE booting into the
         tftproot.
         """
         for bootloader in BOOTLOADERS:
-            bootloader_src = os.path.join(BOOTLOADER_DIR, bootloader)
+            # locate_bootloader might return None but happy to let that
+            # traceback here is it should never happen unless there's a
+            # serious problem with packaging.
+            bootloader_src = self.locate_bootloader(bootloader)
             bootloader_dst = os.path.join(destination, bootloader)
             install_bootloader(bootloader_src, bootloader_dst)

=== modified file 'src/provisioningserver/boot/tests/test_pxe.py'
--- src/provisioningserver/boot/tests/test_pxe.py	2014-06-10 14:45:14 +0000
+++ src/provisioningserver/boot/tests/test_pxe.py	2014-06-20 04:51:25 +0000
@@ -15,14 +15,21 @@
 __all__ = []
 
 from collections import OrderedDict
+import os
 import re
 
 from maastesting.factory import factory
+from maastesting.matchers import MockCallsMatch
 from maastesting.testcase import MAASTestCase
+import mock
 from provisioningserver import kernel_opts
-from provisioningserver.boot import BytesReader
+from provisioningserver.boot import (
+    BytesReader,
+    pxe as pxe_module,
+    )
 from provisioningserver.boot.pxe import (
     ARP_HTYPE,
+    BOOTLOADERS,
     PXEBootMethod,
     re_config_file,
     )
@@ -67,6 +74,18 @@
         self.useFixture(set_tftp_root(tftproot))
         return tftproot
 
+    def make_dummy_bootloader_sources(self, destination, loader_names):
+        """install_bootloader requires real files to exist, this method
+        creates them in the requested location.
+
+        :return: list of created filenames
+        """
+        created = []
+        for loader in loader_names:
+            name = factory.make_file(destination, loader)
+            created.append(name)
+        return created
+
     def test_compose_config_path_follows_maas_pxe_directory_layout(self):
         name = factory.make_name('config')
         self.assertEqual(
@@ -103,6 +122,44 @@
         method = PXEBootMethod()
         self.assertEqual('00:00', method.arch_octet)
 
+    def test_locate_bootloader(self):
+        # Put all the BOOTLOADERS except one in dir1, and the last in
+        # dir2.
+        dir1 = self.make_dir()
+        dir2 = self.make_dir()
+        dirs = [dir1, dir2]
+        self.patch(pxe_module, "BOOTLOADER_DIRS", dirs)
+        self.make_dummy_bootloader_sources(dir1, BOOTLOADERS[:-1])
+        [displaced_loader] = self.make_dummy_bootloader_sources(
+            dir2, BOOTLOADERS[-1:])
+        method = PXEBootMethod()
+        observed = method.locate_bootloader(BOOTLOADERS[-1])
+
+        self.assertEqual(displaced_loader, observed)
+
+    def test_locate_bootloader_returns_None_if_not_found(self):
+        method = PXEBootMethod()
+        self.assertIsNone(method.locate_bootloader("foo"))
+
+    def test_install_bootloader_installs_to_destination(self):
+        tftproot = self.make_tftp_root()
+        source_dir = self.make_dir()
+        self.patch(pxe_module, "BOOTLOADER_DIRS", [source_dir])
+        self.make_dummy_bootloader_sources(source_dir, BOOTLOADERS)
+        install_bootloader_call = self.patch(pxe_module, "install_bootloader")
+        method = PXEBootMethod()
+        method.install_bootloader(tftproot)
+
+        expected = [
+            mock.call(
+                os.path.join(source_dir, bootloader),
+                os.path.join(tftproot, bootloader)
+            )
+            for bootloader in BOOTLOADERS]
+        self.assertThat(
+            install_bootloader_call,
+            MockCallsMatch(*expected))
+
 
 def parse_pxe_config(text):
     """Parse a PXE config file.