Merge lp:~blake-rouse/maas/powernv-support into lp:~maas-committers/maas/trunk

[Blake Rouse]

Support enlisting and booting PowerNV nodes. Added support for the path-prefix for dhcpd config architectures.

Add support for PowerNV. PowerNV boots using pxelinux emulation. The dhcpd option 210 is used to prefix all PXE requests with "ppc64el/". This allows the PowerNVBootMethod to identify that the booting node is architecture ppc64el.

compose_conditional_bootloader was modified to support the new path-prefix dhcpd config option. A boot method sets path_prefix attribute, and when the dhcpd config is generated that option will be placed in the generated dhcpd config for that architecture.

PowerNV has some issues that the boot method handles.

1. IPAPPEND 2 is not support.
  - The BOOTIF= kernel parameter has to be manual injected into the kernel parameters as PowerNV does not support the IPAPPEND option in the pxelinux.cfg. This is done by using the provided mac address from the pxelinux.cfg request, or by getting the mac address from the arp cache using the remote_host ip address.

2. LOCALBOOT is not supported.
  - Without localboot support MAAS needs to be able to tell the node to boot from local disk. The only work around for this, is to return an empty pxelinux.cfg config when booting local. This will cause the PowerNV node to ignore the PXE boot, and boot from the first disk.

[Jason Hobbs (jason-hobbs)]

Blake, not done reviewing yet but here's my comments so far:

[1]

=== added file 'etc/maas/templates/pxe/config.commissioning.ppc64el.template'
--- etc/maas/templates/pxe/config.commissioning.ppc64el.template 1970-01-01 00:00:00 +0000
+++ etc/maas/templates/pxe/config.commissioning.ppc64el.template 2014-05-28 21:38:08 +0000
@@ -0,0 +1,6 @@
+DEFAULT execute
+
+LABEL execute
+ KERNEL {{kernel_params | kernel_path }}
+ INITRD {{kernel_params | initrd_path }}
+ APPEND {{kernel_params | kernel_command}}

=== added file 'etc/maas/templates/pxe/config.install.ppc64el.template'
...
=== added file 'etc/maas/templates/pxe/config.xinstall.ppc64el.template'

I think the duplicate files here should be symlinks - armhf does this
right now.

[2]

+# 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)))

This is copied directly from pxe.py. It should either be imported
from there or some other common location.

+# The pxelinux.cfg path is prefixed with the architecture for the
+# PowerNV nodes. This prefix is set by the path-prefix dhcpd option.
+# We assume that the ARP HTYPE (hardware type) that PXELINUX sends is
+# always Ethernet.
+re_config_file = r'''
+ # Optional leading slash(es).
+ ^/*
+ ppc64el # PowerNV pxe prefix, set by dhcpd
+ /
+ pxelinux[.]cfg # PXELINUX expects this.
+ /
+ (?: # either a MAC
+ {htype:02x} # ARP HTYPE.
+ -
+ (?P<mac>{re_mac_address.pattern}) # Capture MAC.
+ | # or "default"
+ default
+ )
+ $
+'''

[3]

This is very similar to the re_config_file in pxe.py. Perhaps a common
template could be used to generate them?

+re_config_file = re_config_file.format(
+ htype=ARP_HTYPE.ETHERNET, re_mac_address=re_mac_address)
+re_config_file = re.compile(re_config_file, re.VERBOSE)

[4]

Copied from boot.py - should be common.

[5]

+ def get_reader(self, backend, kernel_params, **extra):
+ """Render a configuration file as a unicode string.
+
+ :param backend: requesting backend
+ :param kernel_params: An instance of `KernelParameters`.
+ :param extra: Allow for other arguments. This is a safety valve;
+ parameters generated in another component (for example, see
+ `TFTPBackend.get_config_reader`) won't cause this to break.
+ """
+ # Due to the path prefix, all requested files from the client will
+ # contain that prefix. Lets return the correct file, removing the
+ # prefix from the path
+ if 'path' in extra:
+ path = extra['path']
+ path = path.replace(self.path_prefix, '', 1)
+ target_path = backend.base.descendant(path.split('/'))
+ return FilesystemReader(target_path)
+
+ # Return empty config for PowerNV local. PowerNV fails to
+ # support the LOCALBOOT flag. Empty config will allow it
+ # to select the first device.
+ if kernel_params.purpose == 'local':
+ return BytesReader("".encode("utf-8"))...

[Blake Rouse (blake-rouse)]

[3] The regex is actually different in that it adds the ppc64el path, and it removes the arch and sub-arch parameters from the regex. So it needs to be seperate from PXEBootMethod.

[4] Also you cannot import those as when you import re_config_file, it is already the compiled regex, which would be the incorrect regex for ppc64el.

[5] Yes the method returns a ByteReader at the end of the method.

[7] MAAS will not boot any normally PXE machine without the PXEBootMethod. So it can easily be assumed that those files will be installed by the PXEBootMethod. No reason to copy the code from the PXEBootMethod, that will already do the same time.

[Jason Hobbs (jason-hobbs)]

Here's the rest of my review. Overall it looks really good! I approve as long as the duplication in 1 & 2 get cleaned up. I think some more near duplication could be cleaned up with some creativity but I wouldn't block on it.

[9]

- Architecture(name="ppc64el/generic", description="ppc64el"),
+ Architecture(
+ name="ppc64el/generic", description="ppc64el",
+ kernel_options=['rootdelay=60']),

A comment on why the rootdelay is needed and why it's 60 would be nice.

[10]

=== modified file 'src/provisioningserver/import_images/download_resources.py'
--- src/provisioningserver/import_images/download_resources.py 2014-05-23 09:48:49 +0000
+++ src/provisioningserver/import_images/download_resources.py 2014-05-28 21:38:08 +0000
@@ -19,6 +19,8 @@
 from datetime import datetime
 from gzip import GzipFile
 import os.path
+import tempfile
+from urlparse import urlsplit

 from provisioningserver.import_images.helpers import (
     get_signing_policy,

Why are these new imports added when there are no other changes to this file?

=== modified file 'src/provisioningserver/utils/__init__.py'
--- src/provisioningserver/utils/__init__.py 2014-05-05 23:55:46 +0000
+++ src/provisioningserver/utils/__init__.py 2014-05-28 21:38:08 +0000
@@ -829,3 +829,24 @@
         if len(columns) == 5 and columns[2] == mac:
             return columns[0]
     return None

[11]

+def find_mac_via_ip(ip):
+ """Find the MAC address for `ip` by reading the output of arp -n.
+
+ Returns `None` if the IP is not found.
+
+ We do this because we aren't necessarily the only DHCP server on the
+ network, so we can't check our own leases file and be guaranteed to find an
+ IP that matches.
+
+ :param ip: The ip address, e.g. '192.168.1.1'.
+ """
+
+ output = call_capture_and_check(['arp', '-n']).split('\n')
+
+ for line in sorted(output):
+ columns = line.split()
+ if len(columns) == 5 and columns[0] == ip:
+ return columns[2]
+ return None

I don't see a unit test for this method.

review: Approve

[Blake Rouse (blake-rouse)]

Fixed the requested items. Added tests for find_mac_via_arp.

[Jeroen T. Vermeulen (jtv)]

Great reviewing. I have some small things to add as well:

What does it mean for match_path to return None? Is that tested? If that's an "unthinkable" situation, it'd be better to raise an explicit error.

.

A comment in get_reader says: "Lets return the correct file." That's a bit vague — if you're going to return a file, yes, let's make the correct one! I find that "let's" and "the correct" are often telltales of things being left unclear.

.

In test_re_config_file_is_compatible_with_config_path_generator, is there any particular reason to repeat basically the same test in a loop? If you have a potential bug in mind that would only trigger very rarely, it'd be better to have a separate test for that case.

.

In find_mac_via_arp you use call_capture_and_check, and TestFindMACViaARP.patch_call patches it. That function is now call_and_check. (Or rather, there were two functions which have merged under that name.)

.

All in all the comments and docstrings are very helpful, although I still get lost in places!

review: Approve

[Blake Rouse (blake-rouse)]

The return None is required, that tells the TFTP backend that this boot method should not be used for the requested file.

Fixing the other comments. Thanks!

[MAAS Lander (maas-lander)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

[MAAS Lander (maas-lander)]

The attempt to merge lp:~blake-rouse/maas/powernv-support into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Get:1 http://security.ubuntu.com trusty-security Release.gpg [933 B]
Get:2 http://security.ubuntu.com trusty-security Release [58.5 kB]
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Get:3 http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg [933 B]
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:4 http://nova.clouds.archive.ubuntu.com trusty-updates Release [58.5 kB]
Get:5 http://security.ubuntu.com trusty-security/main Sources [16.6 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Get:6 http://security.ubuntu.com trusty-security/universe Sources [4,212 B]
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Get:7 http://security.ubuntu.com trusty-security/main amd64 Packages [53.2 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Get:8 http://security.ubuntu.com trusty-security/universe amd64 Packages [17.9 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Ign http://security.ubuntu.com trusty-security/main Translation-en_US
Ign http://security.ubuntu.com trusty-security/universe Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Get:9 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [47.6 kB]
Get:10 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [28.6 kB]
Get:11 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [117 kB]
Get:12 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [77.2 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en_US
Fetched 481 kB in 0s (2,077 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 bind9 bind9utils build-essential bzr-builddeb curl daemontools debhelper dh-apport distro-info dnsutils firefox freeipmi-tools ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make postgresql python-amqplib python-bzrlib python-celery python-convoy python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-formencode python-httplib2 python-jinja2 python...

=== modified file 'etc/maas/templates/dhcp/dhcpd.conf.template'
--- etc/maas/templates/dhcp/dhcpd.conf.template	2014-04-11 03:20:42 +0000
+++ etc/maas/templates/dhcp/dhcpd.conf.template	2014-06-02 19:13:29 +0000
@@ -6,6 +6,7 @@
 # the nodegroup's configuration in MAAS to trigger an update.
 
 option arch code 93 = unsigned integer 16; # RFC4578
+option path-prefix code 210 = text; #RFC5071
 {{for dhcp_subnet in dhcp_subnets}}
 subnet {{dhcp_subnet['subnet']}} netmask {{dhcp_subnet['subnet_mask']}} {
        {{bootloader}}

=== added file 'etc/maas/templates/pxe/config.commissioning.ppc64el.template'
--- etc/maas/templates/pxe/config.commissioning.ppc64el.template	1970-01-01 00:00:00 +0000
+++ etc/maas/templates/pxe/config.commissioning.ppc64el.template	2014-06-02 19:13:29 +0000
@@ -0,0 +1,6 @@
+DEFAULT execute
+
+LABEL execute
+  KERNEL {{kernel_params | kernel_path }}
+  INITRD {{kernel_params | initrd_path }}
+  APPEND {{kernel_params | kernel_command}}

=== added file 'etc/maas/templates/pxe/config.install.ppc64el.template'
--- etc/maas/templates/pxe/config.install.ppc64el.template	1970-01-01 00:00:00 +0000
+++ etc/maas/templates/pxe/config.install.ppc64el.template	2014-06-02 19:13:29 +0000
@@ -0,0 +1,6 @@
+DEFAULT execute
+
+LABEL execute
+  KERNEL {{kernel_params | kernel_path }}
+  INITRD {{kernel_params | initrd_path }}
+  APPEND {{kernel_params | kernel_command}}

=== added symlink 'etc/maas/templates/pxe/config.xinstall.ppc64el.template'
=== target is u'config.install.ppc64el.template'
=== modified file 'src/provisioningserver/boot/__init__.py'
--- src/provisioningserver/boot/__init__.py	2014-05-27 19:43:17 +0000
+++ src/provisioningserver/boot/__init__.py	2014-06-02 19:13:29 +0000
@@ -99,6 +99,10 @@
 
     __metaclass__ = ABCMeta
 
+    # Path prefix that is used for the pxelinux.cfg. Used for
+    # the dhcpd.conf that is generated.
+    path_prefix = None
+
     @abstractproperty
     def name(self):
         """Name of the boot method."""
@@ -223,12 +227,14 @@
 from provisioningserver.boot.pxe import PXEBootMethod
 from provisioningserver.boot.uefi import UEFIBootMethod
 from provisioningserver.boot.powerkvm import PowerKVMBootMethod
+from provisioningserver.boot.powernv import PowerNVBootMethod
 
 
 builtin_boot_methods = [
     PXEBootMethod(),
     UEFIBootMethod(),
     PowerKVMBootMethod(),
+    PowerNVBootMethod(),
 ]
 for method in builtin_boot_methods:
     BootMethodRegistry.register_item(method.name, method)

=== added file 'src/provisioningserver/boot/powernv.py'
--- src/provisioningserver/boot/powernv.py	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/boot/powernv.py	2014-06-02 19:13:29 +0000
@@ -0,0 +1,158 @@
+# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""PowerNV Boot Method"""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+str = None
+
+__metaclass__ = type
+__all__ = [
+    'PowerNVBootMethod',
+    ]
+
+import re
+
+from provisioningserver.boot import (
+    BootMethod,
+    BytesReader,
+    get_parameters,
+    )
+from provisioningserver.boot.pxe import (
+    ARP_HTYPE,
+    re_mac_address,
+    )
+from provisioningserver.kernel_opts import compose_kernel_command_line
+from provisioningserver.utils import find_mac_via_arp
+from tftp.backend import FilesystemReader
+from twisted.python.context import get
+
+# The pxelinux.cfg path is prefixed with the architecture for the
+# PowerNV nodes. This prefix is set by the path-prefix dhcpd option.
+# We assume that the ARP HTYPE (hardware type) that PXELINUX sends is
+# always Ethernet.
+re_config_file = r'''
+    # Optional leading slash(es).
+    ^/*
+    ppc64el           # PowerNV pxe prefix, set by dhcpd
+    /
+    pxelinux[.]cfg    # PXELINUX expects this.
+    /
+    (?: # either a MAC
+        {htype:02x}    # ARP HTYPE.
+        -
+        (?P<mac>{re_mac_address.pattern})    # Capture MAC.
+    | # or "default"
+        default
+    )
+    $
+'''
+
+re_config_file = re_config_file.format(
+    htype=ARP_HTYPE.ETHERNET, re_mac_address=re_mac_address)
+re_config_file = re.compile(re_config_file, re.VERBOSE)
+
+
+def format_bootif(mac):
+    """Formats a mac address into the BOOTIF format, expected by
+    the linux kernel."""
+    mac = mac.replace(':', '-')
+    mac = mac.upper()
+    return '%02x-%s' % (ARP_HTYPE.ETHERNET, mac)
+
+
+class PowerNVBootMethod(BootMethod):
+
+    name = "powernv"
+    template_subdir = "pxe"
+    bootloader_path = "pxelinux.0"
+    arch_octet = "00:0E"
+    path_prefix = "ppc64el/"
+
+    def get_remote_mac(self):
+        """Gets the requestors MAC address from arp cache.
+
+        This is used, when the pxelinux.cfg is requested without the mac
+        address appended. This is needed to inject the BOOTIF into the
+        pxelinux.cfg that is returned to the node.
+        """
+        remote_host, remote_port = get("remote", (None, None))
+        return find_mac_via_arp(remote_host)
+
+    def get_params(self, backend, path):
+        """Gets the matching parameters from the requested path."""
+        match = re_config_file.match(path)
+        if match is not None:
+            return get_parameters(match)
+        if path.lstrip('/').startswith(self.path_prefix):
+            return {'path': path}
+        return None
+
+    def match_path(self, backend, path):
+        """Checks path for the configuration file that needs to be
+        generated.
+
+        :param backend: requesting backend
+        :param path: requested path
+        :returns: dict of match params from path, None if no match
+        """
+        params = self.get_params(backend, path)
+        if params is None:
+            return None
+        params['arch'] = "ppc64el"
+        if 'mac' not in params:
+            mac = self.get_remote_mac()
+            if mac is not None:
+                params['mac'] = mac
+        return params
+
+    def get_reader(self, backend, kernel_params, **extra):
+        """Render a configuration file as a unicode string.
+
+        :param backend: requesting backend
+        :param kernel_params: An instance of `KernelParameters`.
+        :param extra: Allow for other arguments. This is a safety valve;
+            parameters generated in another component (for example, see
+            `TFTPBackend.get_config_reader`) won't cause this to break.
+        """
+        # Due to the path prefix, all requested files from the client will
+        # contain that prefix. Removing the prefix from the path will return
+        # the correct path in the tftp root.
+        if 'path' in extra:
+            path = extra['path']
+            path = path.replace(self.path_prefix, '', 1)
+            target_path = backend.base.descendant(path.split('/'))
+            return FilesystemReader(target_path)
+
+        # Return empty config for PowerNV local. PowerNV fails to
+        # support the LOCALBOOT flag. Empty config will allow it
+        # to select the first device.
+        if kernel_params.purpose == 'local':
+            return BytesReader("".encode("utf-8"))
+
+        template = self.get_template(
+            kernel_params.purpose, kernel_params.arch,
+            kernel_params.subarch)
+        namespace = self.compose_template_namespace(kernel_params)
+
+        # Modify the kernel_command to inject the BOOTIF. PowerNV fails to
+        # support the IPAPPEND pxelinux flag.
+        def kernel_command(params):
+            cmd_line = compose_kernel_command_line(params)
+            if 'mac' in extra:
+                mac = extra['mac']
+                mac = format_bootif(mac)
+                return '%s BOOTIF=%s' % (cmd_line, mac)
+            return cmd_line
+
+        namespace['kernel_command'] = kernel_command
+        return BytesReader(template.substitute(namespace).encode("utf-8"))
+
+    def install_bootloader(self, destination):
+        """Does nothing. No extra boot files are required. All of the boot
+        files from PXEBootMethod will suffice."""

=== added file 'src/provisioningserver/boot/tests/test_powernv.py'
--- src/provisioningserver/boot/tests/test_powernv.py	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/boot/tests/test_powernv.py	2014-06-02 19:13:29 +0000
@@ -0,0 +1,337 @@
+# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for `provisioningserver.boot.powernv`."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+str = None
+
+__metaclass__ = type
+__all__ = []
+
+import os
+import re
+
+from maastesting.factory import factory
+from maastesting.testcase import MAASTestCase
+from provisioningserver.boot import BytesReader
+from provisioningserver.boot.powernv import (
+    ARP_HTYPE,
+    format_bootif,
+    PowerNVBootMethod,
+    re_config_file,
+    )
+from provisioningserver.boot.tests.test_pxe import parse_pxe_config
+from provisioningserver.boot.tftppath import compose_image_path
+from provisioningserver.testing.config import set_tftp_root
+from provisioningserver.tests.test_kernel_opts import make_kernel_parameters
+from provisioningserver.tftp import TFTPBackend
+from testtools.matchers import (
+    IsInstance,
+    MatchesAll,
+    MatchesRegex,
+    Not,
+    StartsWith,
+    )
+
+
+def compose_config_path(mac):
+    """Compose the TFTP path for a PowerNV PXE configuration file.
+
+    The path returned is relative to the TFTP root, as it would be
+    identified by clients on the network.
+
+    :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 "ppc64el/pxelinux.cfg/{htype:02x}-{mac}".format(
+        htype=ARP_HTYPE.ETHERNET, mac=mac)
+
+
+def get_example_path_and_components():
+    """Return a plausible path and its components.
+
+    The path is intended to match `re_config_file`, and the components are
+    the expected groups from a match.
+    """
+    components = {"mac": factory.getRandomMACAddress("-")}
+    config_path = compose_config_path(components["mac"])
+    return config_path, components
+
+
+class TestPowerNVBootMethod(MAASTestCase):
+
+    def make_tftp_root(self):
+        """Set, and return, a temporary TFTP root directory."""
+        tftproot = self.make_dir()
+        self.useFixture(set_tftp_root(tftproot))
+        return tftproot
+
+    def test_compose_config_path_follows_maas_pxe_directory_layout(self):
+        name = factory.make_name('config')
+        self.assertEqual(
+            'ppc64el/pxelinux.cfg/%02x-%s' % (ARP_HTYPE.ETHERNET, name),
+            compose_config_path(name))
+
+    def test_compose_config_path_does_not_include_tftp_root(self):
+        tftproot = self.make_tftp_root()
+        name = factory.make_name('config')
+        self.assertThat(
+            compose_config_path(name),
+            Not(StartsWith(tftproot)))
+
+    def test_bootloader_path(self):
+        method = PowerNVBootMethod()
+        self.assertEqual('pxelinux.0', method.bootloader_path)
+
+    def test_bootloader_path_does_not_include_tftp_root(self):
+        tftproot = self.make_tftp_root()
+        method = PowerNVBootMethod()
+        self.assertThat(
+            method.bootloader_path,
+            Not(StartsWith(tftproot)))
+
+    def test_name(self):
+        method = PowerNVBootMethod()
+        self.assertEqual('powernv', method.name)
+
+    def test_template_subdir(self):
+        method = PowerNVBootMethod()
+        self.assertEqual('pxe', method.template_subdir)
+
+    def test_arch_octet(self):
+        method = PowerNVBootMethod()
+        self.assertEqual('00:0E', method.arch_octet)
+
+    def test_path_prefix(self):
+        method = PowerNVBootMethod()
+        self.assertEqual('ppc64el/', method.path_prefix)
+
+
+class TestPowerNVBootMethodMatchPath(MAASTestCase):
+    """Tests for
+    `provisioningserver.boot.powernv.PowerNVBootMethod.match_path`.
+    """
+
+    def test_match_path_pxe_config_with_mac(self):
+        method = PowerNVBootMethod()
+        config_path, expected = get_example_path_and_components()
+        params = method.match_path(None, config_path)
+        expected['arch'] = 'ppc64el'
+        self.assertEqual(expected, params)
+
+    def test_match_path_pxe_config_without_mac(self):
+        method = PowerNVBootMethod()
+        fake_mac = factory.getRandomMACAddress()
+        self.patch(method, 'get_remote_mac').return_value = fake_mac
+        config_path = 'ppc64el/pxelinux.cfg/default'
+        params = method.match_path(None, config_path)
+        expected = {
+            'arch': 'ppc64el',
+            'mac': fake_mac,
+            }
+        self.assertEqual(expected, params)
+
+    def test_match_path_pxe_prefix_request(self):
+        method = PowerNVBootMethod()
+        fake_mac = factory.getRandomMACAddress()
+        self.patch(method, 'get_remote_mac').return_value = fake_mac
+        file_path = 'ppc64el/file'
+        params = method.match_path(None, file_path)
+        expected = {
+            'arch': 'ppc64el',
+            'mac': fake_mac,
+            'path': file_path,
+            }
+        self.assertEqual(expected, params)
+
+
+class TestPowerNVBootMethodRenderConfig(MAASTestCase):
+    """Tests for
+    `provisioningserver.boot.powernv.PowerNVBootMethod.get_reader`
+    """
+
+    def test_get_reader_install(self):
+        # Given the right configuration options, the PXE configuration is
+        # correctly rendered.
+        method = PowerNVBootMethod()
+        params = make_kernel_parameters(self, purpose="install")
+        output = method.get_reader(backend=None, kernel_params=params)
+        # The output is a BytesReader.
+        self.assertThat(output, IsInstance(BytesReader))
+        output = output.read(10000)
+        # The template has rendered without error. PXELINUX configurations
+        # typically start with a DEFAULT line.
+        self.assertThat(output, StartsWith("DEFAULT "))
+        # The PXE parameters are all set according to the options.
+        image_dir = compose_image_path(
+            osystem=params.osystem, arch=params.arch, subarch=params.subarch,
+            release=params.release, label=params.label)
+        self.assertThat(
+            output, MatchesAll(
+                MatchesRegex(
+                    r'.*^\s+KERNEL %s/di-kernel$' % re.escape(image_dir),
+                    re.MULTILINE | re.DOTALL),
+                MatchesRegex(
+                    r'.*^\s+INITRD %s/di-initrd$' % re.escape(image_dir),
+                    re.MULTILINE | re.DOTALL),
+                MatchesRegex(
+                    r'.*^\s+APPEND .+?$',
+                    re.MULTILINE | re.DOTALL)))
+
+    def test_get_reader_with_extra_arguments_does_not_affect_output(self):
+        # get_reader() allows any keyword arguments as a safety valve.
+        method = PowerNVBootMethod()
+        options = {
+            "backend": None,
+            "kernel_params": make_kernel_parameters(self, purpose="install"),
+        }
+        # Capture the output before sprinking in some random options.
+        output_before = method.get_reader(**options).read(10000)
+        # Sprinkle some magic in.
+        options.update(
+            (factory.make_name("name"), factory.make_name("value"))
+            for _ in range(10))
+        # Capture the output after sprinking in some random options.
+        output_after = method.get_reader(**options).read(10000)
+        # The generated template is the same.
+        self.assertEqual(output_before, output_after)
+
+    def test_get_reader_with_local_purpose(self):
+        # If purpose is "local", output should be empty string.
+        method = PowerNVBootMethod()
+        options = {
+            "backend": None,
+            "kernel_params": make_kernel_parameters(purpose="local"),
+            }
+        output = method.get_reader(**options).read(10000)
+        self.assertIn("", output)
+
+    def test_get_reader_appends_bootif(self):
+        method = PowerNVBootMethod()
+        fake_mac = factory.getRandomMACAddress()
+        params = make_kernel_parameters(self, purpose="install")
+        output = method.get_reader(
+            backend=None, kernel_params=params, arch='ppc64el', mac=fake_mac)
+        output = output.read(10000)
+        config = parse_pxe_config(output)
+        expected = 'BOOTIF=%s' % format_bootif(fake_mac)
+        self.assertIn(expected, config['execute']['APPEND'])
+
+
+class TestPowerNVBootMethodPathPrefix(MAASTestCase):
+    """Tests for
+    `provisioningserver.boot.powernv.PowerNVBootMethod.get_reader`.
+    """
+
+    def test_get_reader_path_prefix(self):
+        data = factory.getRandomString().encode("ascii")
+        temp_file = self.make_file(name="example", contents=data)
+        temp_dir = os.path.dirname(temp_file)
+        backend = TFTPBackend(temp_dir, "http://nowhere.example.com/")
+        method = PowerNVBootMethod()
+        options = {
+            'backend': backend,
+            'kernel_params': make_kernel_parameters(),
+            'path': 'ppc64el/example',
+        }
+        reader = method.get_reader(**options)
+        self.addCleanup(reader.finish)
+        self.assertEqual(len(data), reader.size)
+        self.assertEqual(data, reader.read(len(data)))
+        self.assertEqual(b"", reader.read(1))
+
+    def test_get_reader_path_prefix_only_removes_first_occurrence(self):
+        data = factory.getRandomString().encode("ascii")
+        temp_dir = self.make_dir()
+        temp_subdir = os.path.join(temp_dir, 'ppc64el')
+        os.mkdir(temp_subdir)
+        factory.make_file(temp_subdir, "example", data)
+        backend = TFTPBackend(temp_dir, "http://nowhere.example.com/")
+        method = PowerNVBootMethod()
+        options = {
+            'backend': backend,
+            'kernel_params': make_kernel_parameters(),
+            'path': 'ppc64el/ppc64el/example',
+        }
+        reader = method.get_reader(**options)
+        self.addCleanup(reader.finish)
+        self.assertEqual(len(data), reader.size)
+        self.assertEqual(data, reader.read(len(data)))
+        self.assertEqual(b"", reader.read(1))
+
+
+class TestPowerNVBootMethodRegex(MAASTestCase):
+    """Tests for
+    `provisioningserver.boot.powernv.PowerNVBootMethod.re_config_file`.
+    """
+
+    def test_re_config_file_is_compatible_with_config_path_generator(self):
+        # The regular expression for extracting components of the file path is
+        # compatible with the PXE config path generator.
+        for iteration in range(10):
+            config_path, args = get_example_path_and_components()
+            match = re_config_file.match(config_path)
+            self.assertIsNotNone(match, config_path)
+            self.assertEqual(args, match.groupdict())
+
+    def test_re_config_file_with_leading_slash(self):
+        # The regular expression for extracting components of the file path
+        # doesn't care if there's a leading forward slash; the TFTP server is
+        # easy on this point, so it makes sense to be also.
+        config_path, args = get_example_path_and_components()
+        # Ensure there's a leading slash.
+        config_path = "/" + config_path.lstrip("/")
+        match = re_config_file.match(config_path)
+        self.assertIsNotNone(match, config_path)
+        self.assertEqual(args, match.groupdict())
+
+    def test_re_config_file_without_leading_slash(self):
+        # The regular expression for extracting components of the file path
+        # doesn't care if there's no leading forward slash; the TFTP server is
+        # easy on this point, so it makes sense to be also.
+        config_path, args = get_example_path_and_components()
+        # Ensure there's no leading slash.
+        config_path = config_path.lstrip("/")
+        match = re_config_file.match(config_path)
+        self.assertIsNotNone(match, config_path)
+        self.assertEqual(args, match.groupdict())
+
+    def test_re_config_file_matches_classic_pxelinux_cfg(self):
+        # The default config path is simply "pxelinux.cfg" (without
+        # leading slash).  The regex matches this.
+        mac = 'aa-bb-cc-dd-ee-ff'
+        match = re_config_file.match('ppc64el/pxelinux.cfg/01-%s' % mac)
+        self.assertIsNotNone(match)
+        self.assertEqual({'mac': mac}, match.groupdict())
+
+    def test_re_config_file_matches_pxelinux_cfg_with_leading_slash(self):
+        mac = 'aa-bb-cc-dd-ee-ff'
+        match = re_config_file.match('/ppc64el/pxelinux.cfg/01-%s' % mac)
+        self.assertIsNotNone(match)
+        self.assertEqual({'mac': mac}, match.groupdict())
+
+    def test_re_config_file_does_not_match_non_config_file(self):
+        self.assertIsNone(re_config_file.match('ppc64el/pxelinux.cfg/kernel'))
+
+    def test_re_config_file_does_not_match_file_in_root(self):
+        self.assertIsNone(re_config_file.match('01-aa-bb-cc-dd-ee-ff'))
+
+    def test_re_config_file_does_not_match_file_not_in_pxelinux_cfg(self):
+        self.assertIsNone(re_config_file.match('foo/01-aa-bb-cc-dd-ee-ff'))
+
+    def test_re_config_file_with_default(self):
+        match = re_config_file.match('ppc64el/pxelinux.cfg/default')
+        self.assertIsNotNone(match)
+        self.assertEqual({'mac': None}, match.groupdict())

=== modified file 'src/provisioningserver/dhcp/config.py'
--- src/provisioningserver/dhcp/config.py	2014-04-01 12:57:55 +0000
+++ src/provisioningserver/dhcp/config.py	2014-06-02 19:13:29 +0000
@@ -33,16 +33,22 @@
 
 # Used to generate the conditional bootloader behaviour
 CONDITIONAL_BOOTLOADER = """
-{behaviour} option arch = {arch_octet} {{
-          filename \"{bootloader}\";
-       }}
+{{behaviour}} option arch = {{arch_octet}} {
+          filename \"{{bootloader}}\";
+          {{if path_prefix}}
+          option path-prefix \"{{path_prefix}}\";
+          {{endif}}
+       }
 """
 
 # Used to generate the PXEBootLoader special case
 PXE_BOOTLOADER = """
-else {{
-          filename \"{bootloader}\";
-       }}
+else {
+          filename \"{{bootloader}}\";
+          {{if path_prefix}}
+          option path-prefix \"{{path_prefix}}\";
+          {{endif}}
+       }
 """
 
 
@@ -55,9 +61,13 @@
     behaviour = chain(["if"], repeat("elsif"))
     for name, method in BootMethodRegistry:
         if name != "pxe":
-            output += CONDITIONAL_BOOTLOADER.format(
-                behaviour=next(behaviour), arch_octet=method.arch_octet,
-                bootloader=method.bootloader_path).strip() + ' '
+            output += tempita.sub(
+                CONDITIONAL_BOOTLOADER,
+                behaviour=next(behaviour),
+                arch_octet=method.arch_octet,
+                bootloader=method.bootloader_path,
+                path_prefix=method.path_prefix,
+                ).strip() + ' '
 
     # The PXEBootMethod is used in an else statement for the generated
     # dhcpd config. This ensures that a booting node that does not
@@ -65,8 +75,11 @@
     # pxelinux can still boot.
     pxe_method = BootMethodRegistry.get_item('pxe')
     if pxe_method is not None:
-        output += PXE_BOOTLOADER.format(
-            bootloader=pxe_method.bootloader_path).strip()
+        output += tempita.sub(
+            PXE_BOOTLOADER,
+            bootloader=pxe_method.bootloader_path,
+            path_prefix=pxe_method.path_prefix,
+            ).strip()
     return output.strip()
 
 

=== modified file 'src/provisioningserver/driver/__init__.py'
--- src/provisioningserver/driver/__init__.py	2014-05-29 18:20:58 +0000
+++ src/provisioningserver/driver/__init__.py	2014-06-02 19:13:29 +0000
@@ -216,7 +216,14 @@
     Architecture(
         name="armhf/generic", description="armhf/generic",
         pxealiases=["arm"], kernel_options=["console=ttyAMA0"]),
-    Architecture(name="ppc64el/generic", description="ppc64el"),
+    # PPC64EL needs a rootdelay for PowerNV. The disk controller
+    # in the hardware, takes a little bit longer to come up then
+    # the initrd wants to wait. Set this to 60 seconds, just to
+    # give the booting machine enough time. This doesn't slow down
+    # the booting process, it just increases the timeout.
+    Architecture(
+        name="ppc64el/generic", description="ppc64el",
+        kernel_options=['rootdelay=60']),
 ]
 for arch in builtin_architectures:
     ArchitectureRegistry.register_item(arch.name, arch)

=== modified file 'src/provisioningserver/utils/__init__.py'
--- src/provisioningserver/utils/__init__.py	2014-05-29 17:42:18 +0000
+++ src/provisioningserver/utils/__init__.py	2014-06-02 19:13:29 +0000
@@ -823,3 +823,24 @@
         if len(columns) == 5 and columns[2] == mac:
             return columns[0]
     return None
+
+
+def find_mac_via_arp(ip):
+    """Find the MAC address for `ip` by reading the output of arp -n.
+
+    Returns `None` if the IP is not found.
+
+    We do this because we aren't necessarily the only DHCP server on the
+    network, so we can't check our own leases file and be guaranteed to find an
+    IP that matches.
+
+    :param ip: The ip address, e.g. '192.168.1.1'.
+    """
+
+    output = call_capture_and_check(['arp', '-n']).split('\n')
+
+    for line in sorted(output):
+        columns = line.split()
+        if len(columns) == 5 and columns[0] == ip:
+            return columns[2]
+    return None

=== modified file 'src/provisioningserver/utils/tests/test_utils.py'
--- src/provisioningserver/utils/tests/test_utils.py	2014-05-29 17:42:18 +0000
+++ src/provisioningserver/utils/tests/test_utils.py	2014-06-02 19:13:29 +0000
@@ -74,6 +74,7 @@
     ExternalProcessError,
     filter_dict,
     find_ip_via_arp,
+    find_mac_via_arp,
     get_all_interface_addresses,
     get_mtime,
     incremental_write,
@@ -1347,6 +1348,50 @@
         self.assertEqual(one_result, other_result)
 
 
+class TestFindMACViaARP(MAASTestCase):
+
+    def patch_call(self, output):
+        """Replace `call_capture_and_check` with one that returns `output`."""
+        fake = self.patch(provisioningserver.utils, 'call_capture_and_check')
+        fake.return_value = output
+        return fake
+
+    def test__resolves_IP_address_to_MAC(self):
+        sample = """\
+        Address HWtype  HWaddress Flags Mask            Iface
+        192.168.100.20 (incomplete)                              virbr1
+        192.168.0.104 (incomplete)                              eth0
+        192.168.0.5 (incomplete)                              eth0
+        192.168.0.2 (incomplete)                              eth0
+        192.168.0.100 (incomplete)                              eth0
+        192.168.122.20 ether   52:54:00:02:86:4b   C                     virbr0
+        192.168.0.4 (incomplete)                              eth0
+        192.168.0.1 ether   90:f6:52:f6:17:92   C                     eth0
+        """
+
+        call_capture_and_check = self.patch_call(sample)
+        mac_address_observed = find_mac_via_arp("192.168.122.20")
+        self.assertThat(
+            call_capture_and_check,
+            MockCalledOnceWith(['arp', '-n']))
+        self.assertEqual("52:54:00:02:86:4b", mac_address_observed)
+
+    def test__returns_consistent_output(self):
+        ip = factory.getRandomIPAddress()
+        macs = [
+            '52:54:00:02:86:4b',
+            '90:f6:52:f6:17:92',
+            ]
+        lines = ['%s ether %s C eth0' % (ip, mac) for mac in macs]
+        self.patch_call('\n'.join(lines))
+        one_result = find_mac_via_arp(ip)
+        self.patch_call('\n'.join(reversed(lines)))
+        other_result = find_mac_via_arp(ip)
+
+        self.assertIn(one_result, macs)
+        self.assertEqual(one_result, other_result)
+
+
 class TestAsynchronousDecorator(MAASTestCase):
 
     run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=5)