Merge lp:~allenap/maas/new-tftp-layout into lp:maas/trunk

[Gavin Panella]

Entirely remove support for arch-dependent bootloaders.

We can't assume that we control DHCP, and we can't reliably
differentiate between 32-bit and 64-bit machines there anyway, so we
must do it in the PXE config.

This branch doesn't quite get all the way there - a follow-up branch
will complete this work - but it lays more ground. In short, it
entirely removes support for arch-dependent bootloaders. It also
changes a couple of test helpers for maas-import-pxe-files to be based
on the functions in tftppath instead of redefining them again.

[Julian Edwards (julian-edwards)]

Nice branch.

review: Approve

[lp:~allenap/maas/new-tftp-layout]

891. By Gavin Panella on 2012-08-23

Not using classes in DHCP any more.

=== modified file 'scripts/maas-import-pxe-files'
--- scripts/maas-import-pxe-files	2012-08-15 12:54:49 +0000
+++ scripts/maas-import-pxe-files	2012-08-23 08:19:22 +0000
@@ -72,13 +72,11 @@
     # version).  Otherwise, leave the filesystem alone.
     if [ -f pxelinux.0 ]
     then
-        # TODO: Pass sub-architecture once we support those.
         # TODO: pxelinux.0 is downloaded but there's no reason why it can't
         # come from the syslinux package like chain.c32 is.
-        maas-provision install-pxe-bootloader \
-            --arch=$arch --loader='pxelinux.0'
-        maas-provision install-pxe-bootloader \
-            --arch=$arch --loader='/usr/lib/syslinux/chain.c32'
+        maas-provision install-pxe-bootloader --loader='pxelinux.0'
+        maas-provision install-pxe-bootloader \
+            --loader='/usr/lib/syslinux/chain.c32'
     fi
 }
 

=== modified file 'src/provisioningserver/dhcp/config.py'
--- src/provisioningserver/dhcp/config.py	2012-08-17 04:28:56 +0000
+++ src/provisioningserver/dhcp/config.py	2012-08-23 08:19:22 +0000
@@ -26,28 +26,10 @@
     """Exception raised for errors processing the DHCP config."""
 
 
-# Architectures that we support netbooting for.
-# Actually we don't really do amd64; those systems get to use the i386
-# bootloader instead.
-bootloader_architectures = [
-    ('amd64', 'generic'),
-    ('arm', 'highbank'),
-    ('i386', 'generic'),
-    ]
-
 template_content = dedent("""\
-    class "pxe" {
-      match if substring (option vendor-class-identifier, 0, 9) = "PXEClient";
-      filename "{{bootloaders['i386', 'generic']}}";
-    }
-    class "uboot-highbank" {
-      match if substring (option vendor-class-identifier, 0, 21) = \
-        "U-boot.armv7.highbank";
-      filename "{{bootloaders['arm', 'highbank']}}";
-    }
-
     subnet {{subnet}} netmask {{subnet_mask}} {
            next-server {{next_server}};
+           filename "{{bootloader}}";
            option subnet-mask {{subnet_mask}};
            option broadcast-address {{broadcast_ip}};
            option domain-name-servers {{dns_servers}};
@@ -66,20 +48,6 @@
     template_content, name="%s.template" % __name__)
 
 
-def compose_bootloaders():
-    """Compose "bootloaders" dict for substitution in the DHCP template.
-
-    The bootloaders dict maps tuples of (architecture, subarchitecture) to
-    their respective TFTP bootloader paths.
-
-    Thus, bootloaders['i386', 'generic'] yields the TFTP path for the i386
-    bootloader.
-    """
-    return {
-        (arch, subarch): compose_bootloader_path(arch, subarch)
-        for arch, subarch in bootloader_architectures}
-
-
 def get_config(**params):
     """Return a DHCP config file based on the supplied parameters.
 
@@ -96,8 +64,7 @@
     :param high_range: The last IP address in the range of IP addresses to
         allocate
     """
-    params['bootloaders'] = compose_bootloaders()
-
+    params['bootloader'] = compose_bootloader_path()
     # This is a really simple substitution for now but it's encapsulated
     # here so that its implementation can be changed later if required.
     try:

=== modified file 'src/provisioningserver/dhcp/tests/test_config.py'
--- src/provisioningserver/dhcp/tests/test_config.py	2012-08-17 04:28:56 +0000
+++ src/provisioningserver/dhcp/tests/test_config.py	2012-08-23 08:19:22 +0000
@@ -14,7 +14,7 @@
 
 from textwrap import dedent
 
-from maastesting.matchers import ContainsAll
+from maastesting.matchers import Contains
 from provisioningserver.dhcp import config
 from provisioningserver.pxe.tftppath import compose_bootloader_path
 import tempita
@@ -83,19 +83,7 @@
                 "name 'subnet' is not defined at line \d+ column \d+ "
                 "in file %s" % template.name))
 
-    def test_config_refers_to_PXE_for_supported_architectures(self):
+    def test_config_refers_to_bootloader(self):
         params = make_sample_params()
-        bootloaders = config.compose_bootloaders()
-        archs = [
-            ('i386', 'generic'),
-            ('arm', 'highbank'),
-            ]
-        paths = [bootloaders[arch] for arch in archs]
         output = config.get_config(**params)
-        self.assertThat(output, ContainsAll(paths))
-
-    def test_compose_bootloaders_lists_tftp_paths(self):
-        sample_arch = ('i386', 'generic')
-        self.assertEqual(
-            compose_bootloader_path(*sample_arch),
-            config.compose_bootloaders()[sample_arch])
+        self.assertThat(output, Contains(compose_bootloader_path()))

=== modified file 'src/provisioningserver/pxe/install_bootloader.py'
--- src/provisioningserver/pxe/install_bootloader.py	2012-08-14 01:19:41 +0000
+++ src/provisioningserver/pxe/install_bootloader.py	2012-08-23 08:19:22 +0000
@@ -26,20 +26,16 @@
     )
 
 
-def make_destination(tftproot, arch, subarch):
+def make_destination(tftproot):
     """Locate a loader's destination, creating the directory if needed.
 
     :param tftproot: The root directory served up by the TFTP server,
         e.g. /var/lib/tftpboot/.
-    :param arch: Main architecture to locate the destination for.
-    :param subarch: Sub-architecture of the main architecture.
     :return: Full path describing the directory that the installed loader
-        should end up having.  For example, the loader for i386 (with
-        sub-architecture "generic") should install at
-        /maas/i386/generic/
+        should end up having.
     """
     path = locate_tftp_path(
-        compose_bootloader_path(arch, subarch),
+        compose_bootloader_path(),
         tftproot=tftproot)
     directory = os.path.dirname(path)
     if not os.path.isdir(directory):
@@ -89,12 +85,6 @@
 
 def add_arguments(parser):
     parser.add_argument(
-        '--arch', dest='arch', default=None,
-        help="Main system architecture that the bootloader is for.")
-    parser.add_argument(
-        '--subarch', dest='subarch', default='generic',
-        help="Sub-architecture of the main architecture [%(default)s].")
-    parser.add_argument(
         '--loader', dest='loader', default=None,
         help="PXE pre-boot loader to install.")
 
@@ -106,6 +96,6 @@
     """
     config = Config.load(args.config_file)
     tftproot = config["tftp"]["root"]
-    destination_path = make_destination(tftproot, args.arch, args.subarch)
+    destination_path = make_destination(tftproot)
     destination = os.path.join(destination_path, os.path.basename(args.loader))
     install_bootloader(args.loader, destination)

=== modified file 'src/provisioningserver/pxe/tests/test_install_bootloader.py'
--- src/provisioningserver/pxe/tests/test_install_bootloader.py	2012-08-13 08:01:31 +0000
+++ src/provisioningserver/pxe/tests/test_install_bootloader.py	2012-08-23 08:19:22 +0000
@@ -47,18 +47,16 @@
         self.useFixture(config_fixture)
 
         loader = self.make_file()
-        arch = factory.make_name('arch')
-        subarch = factory.make_name('subarch')
 
         action = factory.make_name("action")
         script = MainScript(action)
         script.register(action, provisioningserver.pxe.install_bootloader)
         script.execute(
-            ("--config-file", config_fixture.filename, action, "--arch", arch,
-             "--subarch", subarch, "--loader", loader))
+            ("--config-file", config_fixture.filename, action,
+             "--loader", loader))
 
         bootloader_filename = os.path.join(
-            os.path.dirname(compose_bootloader_path(arch, subarch)),
+            os.path.dirname(compose_bootloader_path()),
             os.path.basename(loader))
         self.assertThat(
             locate_tftp_path(
@@ -67,17 +65,13 @@
 
     def test_make_destination_creates_directory_if_not_present(self):
         tftproot = self.make_dir()
-        arch = factory.make_name('arch')
-        subarch = factory.make_name('subarch')
-        dest = make_destination(tftproot, arch, subarch)
+        dest = make_destination(tftproot)
         self.assertThat(dest, DirExists())
 
     def test_make_destination_returns_existing_directory(self):
         tftproot = self.make_dir()
-        arch = factory.make_name('arch')
-        subarch = factory.make_name('subarch')
-        make_destination(tftproot, arch, subarch)
-        dest = make_destination(tftproot, arch, subarch)
+        make_destination(tftproot)
+        dest = make_destination(tftproot)
         self.assertThat(dest, DirExists())
 
     def test_install_bootloader_installs_new_bootloader(self):

=== modified file 'src/provisioningserver/pxe/tests/test_tftppath.py'
--- src/provisioningserver/pxe/tests/test_tftppath.py	2012-08-01 16:16:09 +0000
+++ src/provisioningserver/pxe/tests/test_tftppath.py	2012-08-23 08:19:22 +0000
@@ -39,20 +39,15 @@
         self.useFixture(ConfigFixture(self.config))
 
     def test_compose_config_path_follows_maas_pxe_directory_layout(self):
-        arch = factory.make_name('arch')
-        subarch = factory.make_name('subarch')
         name = factory.make_name('config')
         self.assertEqual(
-            'maas/%s/%s/pxelinux.cfg/%02x-%s' % (
-                arch, subarch, ARP_HTYPE.ETHERNET, name),
-            compose_config_path(arch, subarch, name))
+            'maas/pxelinux.cfg/%02x-%s' % (ARP_HTYPE.ETHERNET, name),
+            compose_config_path(name))
 
     def test_compose_config_path_does_not_include_tftp_root(self):
-        arch = factory.make_name('arch')
-        subarch = factory.make_name('subarch')
         name = factory.make_name('config')
         self.assertThat(
-            compose_config_path(arch, subarch, name),
+            compose_config_path(name),
             Not(StartsWith(self.tftproot)))
 
     def test_compose_image_path_follows_maas_pxe_directory_layout(self):
@@ -74,17 +69,11 @@
             Not(StartsWith(self.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))
+        self.assertEqual('maas/pxelinux.0', compose_bootloader_path())
 
     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),
+            compose_bootloader_path(),
             Not(StartsWith(self.tftproot)))
 
     def test_locate_tftp_path_prefixes_tftp_root(self):

=== modified file 'src/provisioningserver/pxe/tftppath.py'
--- src/provisioningserver/pxe/tftppath.py	2012-08-01 16:16:09 +0000
+++ src/provisioningserver/pxe/tftppath.py	2012-08-23 08:19:22 +0000
@@ -22,29 +22,34 @@
 from provisioningserver.enum import ARP_HTYPE
 
 
-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_bootloader_path():
+    """Compose the TFTP path for a PXE pre-boot loader.
+
+    All Intel-like architectures will use `pxelinux.0`. Other architectures
+    simulate PXELINUX and don't actually load `pxelinux.0`, but use its path
+    to figure out where configuration files are located.
+    """
+    return "maas/pxelinux.0"
 
 
 # TODO: move this; it is now only used for testing.
-def compose_config_path(arch, subarch, name):
+def compose_config_path(mac):
     """Compose the TFTP path for a PXE configuration file.
 
     The path returned is relative to the TFTP root, as it would be
     identified by clients on the network.
 
-    :param arch: Main machine architecture.
-    :param subarch: Sub-architecture, or "generic" if there is none.
-    :param name: Configuration file's name.
+    :param mac: A MAC address, in IEEE 802 hyphen-separated form,
+        corresponding to the machine for which this configuration is
+        relevant. This relates to PXELINUX's lookup protocol.
     :return: Path for the corresponding PXE config file as exposed over
         TFTP.
     """
     # Not using os.path.join: this is a TFTP path, not a native path. Yes, in
     # practice for us they're the same. We always assume that the ARP HTYPE
     # (hardware type) that PXELINUX sends is Ethernet.
-    return "maas/{arch}/{subarch}/pxelinux.cfg/{htype:02x}-{name}".format(
-        arch=arch, subarch=subarch, htype=ARP_HTYPE.ETHERNET, name=name)
+    return "maas/pxelinux.cfg/{htype:02x}-{mac}".format(
+        htype=ARP_HTYPE.ETHERNET, mac=mac)
 
 
 def compose_image_path(arch, subarch, release, purpose):

=== modified file 'src/provisioningserver/tests/test_maas_import_pxe_files.py'
--- src/provisioningserver/tests/test_maas_import_pxe_files.py	2012-08-03 12:35:52 +0000
+++ src/provisioningserver/tests/test_maas_import_pxe_files.py	2012-08-23 08:19:22 +0000
@@ -21,6 +21,7 @@
     age_file,
     get_write_time,
     )
+from provisioningserver.pxe import tftppath
 from provisioningserver.testing.config import ConfigFixture
 from testtools.matchers import (
     FileContains,
@@ -55,14 +56,24 @@
         'images', 'netboot', 'ubuntu-installer', arch)
 
 
-def compose_tftp_path(tftproot, arch, *path):
+def compose_tftp_bootloader_path(tftproot):
+    """Compose path for MAAS TFTP bootloader."""
+    return tftppath.locate_tftp_path(
+        tftppath.compose_bootloader_path(), tftproot)
+
+
+def compose_tftp_path(tftproot, arch, release, purpose, *path):
     """Compose path for MAAS TFTP files for given architecture.
 
     After the TFTP root directory and the architecture, just append any path
     components you want to drill deeper, e.g. the release name to get to the
     files for a specific release.
     """
-    return os.path.join(tftproot, 'maas', arch, 'generic', *path)
+    return os.path.join(
+        tftppath.locate_tftp_path(
+            tftppath.compose_image_path(arch, "generic", release, purpose),
+            tftproot),
+        *path)
 
 
 class TestImportPXEFiles(TestCase):
@@ -129,7 +140,7 @@
         release = 'precise'
         archive = self.make_downloads(arch=arch, release=release)
         self.call_script(archive, self.tftproot, arch=arch, release=release)
-        tftp_path = compose_tftp_path(self.tftproot, arch, 'pxelinux.0')
+        tftp_path = compose_tftp_bootloader_path(self.tftproot)
         download_path = compose_download_dir(archive, arch, release)
         expected_contents = read_file(download_path, 'pxelinux.0')
         self.assertThat(tftp_path, FileContains(expected_contents))
@@ -141,13 +152,13 @@
         download_path = compose_download_dir(archive, arch, release)
         os.remove(os.path.join(download_path, 'pxelinux.0'))
         self.call_script(archive, self.tftproot, arch=arch, release=release)
-        tftp_path = compose_tftp_path(self.tftproot, arch, 'pxelinux.0')
+        tftp_path = compose_tftp_bootloader_path(self.tftproot)
         self.assertThat(tftp_path, Not(FileExists()))
 
     def test_updates_pre_boot_loader(self):
         arch = factory.make_name('arch')
         release = 'precise'
-        tftp_path = compose_tftp_path(self.tftproot, arch, 'pxelinux.0')
+        tftp_path = compose_tftp_bootloader_path(self.tftproot)
         os.makedirs(os.path.dirname(tftp_path))
         with open(tftp_path, 'w') as existing_file:
             existing_file.write(factory.getRandomString())

=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py	2012-08-20 11:15:44 +0000
+++ src/provisioningserver/tests/test_tasks.py	2012-08-23 08:19:22 +0000
@@ -215,7 +215,7 @@
             FileContains(
                 matcher=ContainsAll(
                     [
-                        'class "pxe"',
+                        "next-server ",
                         "option subnet-mask",
                     ])))
         self.assertEqual(

=== modified file 'src/provisioningserver/tests/test_tftp.py'
--- src/provisioningserver/tests/test_tftp.py	2012-08-02 16:15:06 +0000
+++ src/provisioningserver/tests/test_tftp.py	2012-08-23 08:19:22 +0000
@@ -70,15 +70,10 @@
         the expected groups from a match.
         """
         components = {
-            "arch": factory.make_name("arch"),
-            "subarch": factory.make_name("subarch"),
+            "bootpath": b"maas",  # Static.
             "mac": factory.getRandomMACAddress(b"-"),
             }
-        # The bootpath component is a superset of arch and subarch.
-        components["bootpath"] = "maas/{arch}/{subarch}".format(**components)
-        config_path = compose_config_path(
-            arch=components["arch"], subarch=components["subarch"],
-            name=components["mac"])
+        config_path = compose_config_path(components["mac"])
         return config_path, components
 
     def test_re_config_file(self):
@@ -168,10 +163,8 @@
     def test_get_reader_config_file(self):
         # For paths matching re_config_file, TFTPBackend.get_reader() returns
         # a Deferred that will yield a BytesReader.
-        arch = factory.make_name("arch").encode("ascii")
-        subarch = factory.make_name("subarch").encode("ascii")
         mac = factory.getRandomMACAddress(b"-")
-        config_path = compose_config_path(arch, subarch, mac)
+        config_path = compose_config_path(mac)
         backend = TFTPBackend(self.make_dir(), b"http://example.com/")
 
         @partial(self.patch, backend, "get_config_reader")
@@ -184,9 +177,7 @@
         output = reader.read(10000)
         # The expected parameters include bootpath; this is extracted from the
         # file path by re_config_file.
-        expected_params = dict(
-            arch=arch, subarch=subarch, mac=mac,
-            bootpath="maas/%s/%s" % (arch, subarch))
+        expected_params = dict(mac=mac, bootpath="maas")
         observed_params = json.loads(output)
         self.assertEqual(expected_params, observed_params)
 

=== modified file 'src/provisioningserver/tftp.py'
--- src/provisioningserver/tftp.py	2012-08-06 12:23:00 +0000
+++ src/provisioningserver/tftp.py	2012-08-23 08:19:22 +0000
@@ -87,10 +87,6 @@
         ^/?
         (?P<bootpath>
           maas     # Static namespacing.
-          /
-          (?P<arch>[^/]+)     # Capture arch.
-          /
-          (?P<subarch>[^/]+)    # Capture subarch.
         )    # Capture boot directory.
         /
         pxelinux[.]cfg    # PXELINUX expects this.