Merge ~wpk/maas:IPXE-Boot into maas:master

[Witold Krecicki]

This change introduces iPXE boot method - iPXE (used for example by virsh pods, but it can also be burned into ROM and used in servers) can load kernel and initrd directly without any intermediate bootloader as pxelinux.0. It also, unlike pxelinux.0, works on IPv6.

[Blake Rouse (blake-rouse)]

These changes look great, I like the idea of support iPXE directly instead of chainloading pxelinux.0.

You are missing unit tests. You need to add unit tests for your new boot method, the changes to the boot method generation code for dhcpd configurations.

Also have a couple of questions.

review: Needs Fixing

[Andres Rodriguez (andreserl)]

Olease do not land this in MAAS 2.3, as it is now closed for development.
This will need to wait until 2.4 is open!

On Mon, Oct 23, 2017 at 9:27 AM Blake Rouse <email address hidden>
wrote:

> Review: Needs Fixing
>
> These changes look great, I like the idea of support iPXE directly instead
> of chainloading pxelinux.0.
>
> You are missing unit tests. You need to add unit tests for your new boot
> method, the changes to the boot method generation code for dhcpd
> configurations.
>
> Also have a couple of questions.
>
> Diff comments:
>
> > diff --git a/src/provisioningserver/dhcp/config.py
> b/src/provisioningserver/dhcp/config.py
> > index 160d30d..9af33e3 100644
> > --- a/src/provisioningserver/dhcp/config.py
> > +++ b/src/provisioningserver/dhcp/config.py
> > @@ -83,12 +84,21 @@ else {
> > class DHCPConfigError(Exception):
> > """Exception raised for errors processing the DHCP config."""
> >
> > +def user_class_to_hex(value):
> > + """isc-dhcp does not understand dhcpv6 User Class option as string,
> > + so we need to give it to it as a series of octets."""
> > + if value is None or value == "" or len(value) > 65535:
> > + return None
> > + rv = "%.2x:%.2x:" % (len(value)//256, len(value)%256)
>
> I beleive this should fail lint, as operations should be seperated by
> spaces.
>
> rv = "%.2x:%.2x:" % (len(value) // 256, len(value) % 256)
>
> > + rv += ":".join(map(lambda x: "%.2x" % ord(x), value))
> > + return rv
> >
> > def compose_conditional_bootloader(ipv6, rack_ip=None):
> > output = ""
> > behaviour = chain(["if"], repeat("elsif"))
> > for name, method in BootMethodRegistry:
> > - if method.arch_octet is not None:
> > + if method.arch_octet is not None or method.user_class is not
> None:
> > + hexified_user_class = user_class_to_hex(method.user_class)
> > url = ('tftp://[%s]/' if ipv6 else 'tftp://%s/') % rack_ip
> > if method.path_prefix:
> > url += method.path_prefix
> > diff --git
> a/src/provisioningserver/templates/ipxe/config.install.template
> b/src/provisioningserver/templates/ipxe/config.install.template
> > new file mode 100644
> > index 0000000..bf87001
> > --- /dev/null
> > +++ b/src/provisioningserver/templates/ipxe/config.install.template
> > @@ -0,0 +1,5 @@
> > +#!ipxe
> > +kernel {{kernel_params | kernel_path }}
> > +imgargs {{kernel_params | kernel_name }} {{kernel_params |
> kernel_command}} BOOTIF=01-${net_default_mac}
>
> ${net_default_mac} is a grub thing? Is it the same for iPXE?
>
> > +initrd {{kernel_params | initrd_path }}
> > +boot
> > \ No newline at end of file
>
>
> --
> https://code.launchpad.net/~wpk/maas/+git/maas/+merge/332552
> Your team MAAS Committers is subscribed to branch maas:master.
>
--
Andres Rodriguez
Engineering Manager, MAAS
Canonical USA, Inc.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b IPXE-Boot lp:~wpk/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/539/console
COMMIT: b0150ac402c00d328f92b7932beb63bca618dc0b

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b IPXE-Boot lp:~wpk/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/539/console
COMMIT: b0150ac402c00d328f92b7932beb63bca618dc0b

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b IPXE-Boot lp:~wpk/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/539/console
COMMIT: b0150ac402c00d328f92b7932beb63bca618dc0b

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b IPXE-Boot lp:~wpk/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/539/console
COMMIT: b0150ac402c00d328f92b7932beb63bca618dc0b

review: Needs Fixing

[Andres Rodriguez (andreserl)]

Marking this as rejected as this has now been fix in MAAS 2.6.

Unmerged commits

b0150ac... by Witold Krecicki on 2017-10-20

iPXE boot method

d310824... by Witold Krecicki on 2017-10-20

Merge branch 'master' of git+ssh://git.launchpad.net/maas

95a3f7e... by Witold Krecicki on 2017-10-13

Merge branch 'master' of git+ssh://git.launchpad.net/maas

6e06021... by Witold Krecicki on 2017-09-28

Merge branch 'master' of git+ssh://git.launchpad.net/maas

5648a31... by Witold Krecicki on 2017-09-19

Fix maas_enlist with IPv6 addresses in server_url LP: #1717287

diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py
index a397404..9ea811f 100644
--- a/src/maasserver/models/node.py
+++ b/src/maasserver/models/node.py
@@ -234,7 +234,7 @@ maaslog = get_maas_logger("node")
 
 # Holds the known `bios_boot_methods`. If `bios_boot_method` is not in this
 # list then it will fallback to `DEFAULT_BIOS_BOOT_METHOD`.
-KNOWN_BIOS_BOOT_METHODS = ["pxe", "uefi", "powernv", "powerkvm"]
+KNOWN_BIOS_BOOT_METHODS = ["pxe", "uefi", "powernv", "powerkvm", "ipxe"]
 
 # Default `bios_boot_method`. See `KNOWN_BIOS_BOOT_METHOD` above for usage.
 DEFAULT_BIOS_BOOT_METHOD = "pxe"
diff --git a/src/provisioningserver/boot/__init__.py b/src/provisioningserver/boot/__init__.py
index 5f32111..bd6698f 100644
--- a/src/provisioningserver/boot/__init__.py
+++ b/src/provisioningserver/boot/__init__.py
@@ -172,6 +172,11 @@ class BootMethod(metaclass=ABCMeta):
         dhcpv6-parameters.xhtml#processor-architecture
         """
 
+    @abstractproperty
+    def user_class(self):
+        """ User class that supports this method. Used for the
+        dhcpd.conf file that is generated."""
+
     def match_path(self, backend, path):
         """Checks path for a file the boot method needs to handle.
 
@@ -282,6 +287,7 @@ class BootMethod(metaclass=ABCMeta):
         assert isinstance(self.bootloader_files, list) and all(
             isinstance(element, str) for element in self.bootloader_files)
         assert isinstance(self.arch_octet, str) or self.arch_octet is None
+        assert isinstance(self.user_class, str) or self.user_class is None
 
     def get_template_dir(self):
         """Gets the template directory for the boot method."""
@@ -339,6 +345,12 @@ class BootMethod(metaclass=ABCMeta):
                 initrd = 'boot-initrd'
             return "%s/%s" % (image_dir(params), initrd)
 
+        def kernel_name(params):
+            if params.kernel is not None:
+                return params.kernel
+            else:
+                return 'boot-kernel'
+
         def kernel_path(params):
             # Normally the kernel filename is the SimpleStream filetype. If
             # no filetype is given try the filetype.
@@ -368,6 +380,7 @@ class BootMethod(metaclass=ABCMeta):
             "kernel_command": kernel_command,
             "kernel_params": kernel_params,
             "kernel_path": kernel_path,
+            "kernel_name": kernel_name,
             "dtb_path": dtb_path,
             }
 
@@ -379,6 +392,7 @@ class BootMethodRegistry(Registry):
 
 
 # Import the supported boot methods after defining BootMethod.
+from provisioningserver.boot.ipxe import IPXEBootMethod
 from provisioningserver.boot.pxe import PXEBootMethod
 from provisioningserver.boot.uefi_amd64 import UEFIAMD64BootMethod
 from provisioningserver.boot.uefi_arm64 import UEFIARM64BootMethod
@@ -390,6 +404,7 @@ from provisioningserver.boot.windows import WindowsPXEBootMethod
 
 
 builtin_boot_methods = [
+    IPXEBootMethod(),
     PXEBootMethod(),
     UEFIAMD64BootMethod(),
     UEFIARM64BootMethod(),
diff --git a/src/provisioningserver/boot/ipxe.py b/src/provisioningserver/boot/ipxe.py
new file mode 100644
index 0000000..bc75f38
--- /dev/null
+++ b/src/provisioningserver/boot/ipxe.py
@@ -0,0 +1,83 @@
+# Copyright 2017 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""IPXE Boot Method"""
+
+__all__ = [
+    'IPXEBootMethod',
+    ]
+
+from itertools import repeat
+import os
+import re
+import shutil
+
+from provisioningserver.boot import (
+    BootMethod,
+    BytesReader,
+    get_parameters,
+    get_remote_mac,
+)
+from provisioningserver.events import (
+    EVENT_TYPES,
+    try_send_rack_event,
+)
+from provisioningserver.logger import (
+    get_maas_logger,
+    LegacyLogger,
+)
+from provisioningserver.utils.fs import (
+    atomic_copy,
+    atomic_symlink,
+)
+
+
+maaslog = get_maas_logger('ipxe')
+log = LegacyLogger()
+
+
+re_ipxe_file = r'''^/*maas-ipxe[.]cfg$'''
+
+re_ipxe_file = re_ipxe_file.encode("ascii")
+re_ipxe_file = re.compile(re_ipxe_file, re.VERBOSE)
+
+
+class IPXEBootMethod(BootMethod):
+    name = 'ipxe'
+    bios_boot_method = 'ipxe'
+    template_subdir = 'ipxe'
+    bootloader_arches = ['i386', 'amd64']
+    bootloader_path = 'maas-ipxe.cfg'
+    bootloader_files = [ 'maas-ipxe.cfg' ]
+    arch_octet = None
+    user_class = 'iPXE'
+
+    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
+        :return: dict of match params from path, None if no match
+        """
+        match = re_ipxe_file.match(path)
+        if match is None:
+            return None
+        else:
+            return {'mac': get_remote_mac()}
+
+    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_boot_method_reader`) won't cause this to break.
+        """
+        template = self.get_template(
+            kernel_params.purpose, kernel_params.arch,
+            kernel_params.subarch)
+        namespace = self.compose_template_namespace(kernel_params)
+        return BytesReader(template.substitute(namespace).encode("utf-8"))
+
diff --git a/src/provisioningserver/boot/open_firmware_ppc64el.py b/src/provisioningserver/boot/open_firmware_ppc64el.py
index 574bbbc..4ad4da7 100644
--- a/src/provisioningserver/boot/open_firmware_ppc64el.py
+++ b/src/provisioningserver/boot/open_firmware_ppc64el.py
@@ -19,3 +19,4 @@ class OpenFirmwarePPC64ELBootMethod(BootMethod):
     bootloader_path = 'bootppc64.bin'
     bootloader_files = ['bootppc64.bin']
     arch_octet = '00:0C'
+    user_class = None
diff --git a/src/provisioningserver/boot/powernv.py b/src/provisioningserver/boot/powernv.py
index eac0d9a..4535f2b 100644
--- a/src/provisioningserver/boot/powernv.py
+++ b/src/provisioningserver/boot/powernv.py
@@ -79,6 +79,7 @@ class PowerNVBootMethod(BootMethod):
     template_subdir = "pxe"
     bootloader_path = "pxelinux.0"
     arch_octet = "00:0E"
+    user_class = None
     path_prefix = "ppc64el/"
 
     def get_params(self, backend, path):
diff --git a/src/provisioningserver/boot/pxe.py b/src/provisioningserver/boot/pxe.py
index 9884054..e8b6d8a 100644
--- a/src/provisioningserver/boot/pxe.py
+++ b/src/provisioningserver/boot/pxe.py
@@ -88,6 +88,7 @@ class PXEBootMethod(BootMethod):
         'libutil.c32',
     ]
     arch_octet = '00:00'
+    user_class = None
 
     def match_path(self, backend, path):
         """Checks path for the configuration file that needs to be
diff --git a/src/provisioningserver/boot/uefi_amd64.py b/src/provisioningserver/boot/uefi_amd64.py
index 430268f..b5b78f9 100644
--- a/src/provisioningserver/boot/uefi_amd64.py
+++ b/src/provisioningserver/boot/uefi_amd64.py
@@ -81,6 +81,7 @@ class UEFIAMD64BootMethod(BootMethod):
     bootloader_path = 'bootx64.efi'
     bootloader_files = ['bootx64.efi', 'grubx64.efi']
     arch_octet = '00:07'
+    user_class = None
 
     def match_path(self, backend, path):
         """Checks path for the configuration file that needs to be
diff --git a/src/provisioningserver/boot/uefi_arm64.py b/src/provisioningserver/boot/uefi_arm64.py
index 086dbc1..50fba08 100644
--- a/src/provisioningserver/boot/uefi_arm64.py
+++ b/src/provisioningserver/boot/uefi_arm64.py
@@ -19,3 +19,4 @@ class UEFIARM64BootMethod(BootMethod):
     bootloader_path = 'grubaa64.efi'
     bootloader_files = ['grubaa64.efi']
     arch_octet = '00:0B'
+    user_class = None
diff --git a/src/provisioningserver/boot/windows.py b/src/provisioningserver/boot/windows.py
index f89e92b..9846c41 100644
--- a/src/provisioningserver/boot/windows.py
+++ b/src/provisioningserver/boot/windows.py
@@ -197,6 +197,7 @@ class WindowsPXEBootMethod(BootMethod):
     template_subdir = "windows"
     bootloader_path = "pxeboot.0"
     arch_octet = None
+    user_class = None
 
     @deferred
     def get_node_info(self):
diff --git a/src/provisioningserver/dhcp/config.py b/src/provisioningserver/dhcp/config.py
index 160d30d..9af33e3 100644
--- a/src/provisioningserver/dhcp/config.py
+++ b/src/provisioningserver/dhcp/config.py
@@ -47,12 +47,13 @@ logger = logging.getLogger(__name__)
 # Used to generate the conditional bootloader behaviour
 CONDITIONAL_BOOTLOADER = ("""
 {{if ipv6}}
-           {{behaviour}} exists dhcp6.client-arch-type and
-             option dhcp6.client-arch-type = {{arch_octet}} {
+           {{behaviour}} {{if arch_octet}} exists dhcp6.client-arch-type and option dhcp6.client-arch-type = {{arch_octet}} {{if user_class}} and {{endif}}{{endif}} \
+ {{if user_class}} exists dhcp6.user-class and option dhcp6.user-class = {{user_class}} {{endif}} {
                option dhcp6.bootfile-url \"{{url}}\";
            }
 {{else}}
-{{behaviour}} option arch = {{arch_octet}} {
+{{behaviour}} {{if arch_octet}} exists arch and option arch = {{arch_octet}} {{if user_class}} and {{endif}}{{endif}} \
+ {{if user_class}} exists user-class and option user-class == {{user_class}} {{endif}} {
     # {{name}}
     filename \"{{bootloader}}\";
     {{if path_prefix}}
@@ -83,12 +84,21 @@ else {
 class DHCPConfigError(Exception):
     """Exception raised for errors processing the DHCP config."""
 
+def user_class_to_hex(value):
+    """isc-dhcp does not understand dhcpv6 User Class option as string,
+       so we need to give it to it as a series of octets."""
+    if value is None or value == "" or len(value) > 65535:
+        return None
+    rv = "%.2x:%.2x:" % (len(value)//256, len(value)%256)
+    rv += ":".join(map(lambda x: "%.2x" % ord(x), value))
+    return rv
 
 def compose_conditional_bootloader(ipv6, rack_ip=None):
     output = ""
     behaviour = chain(["if"], repeat("elsif"))
     for name, method in BootMethodRegistry:
-        if method.arch_octet is not None:
+        if method.arch_octet is not None or method.user_class is not None:
+            hexified_user_class = user_class_to_hex(method.user_class)
             url = ('tftp://[%s]/' if ipv6 else 'tftp://%s/') % rack_ip
             if method.path_prefix:
                 url += method.path_prefix
@@ -98,6 +108,7 @@ def compose_conditional_bootloader(ipv6, rack_ip=None):
                 ipv6=ipv6, rack_ip=rack_ip, url=url,
                 behaviour=next(behaviour),
                 arch_octet=method.arch_octet,
+                user_class=hexified_user_class,
                 bootloader=method.bootloader_path,
                 path_prefix=method.path_prefix,
                 name=method.name,
diff --git a/src/provisioningserver/templates/dhcp/dhcpd6.conf.template b/src/provisioningserver/templates/dhcp/dhcpd6.conf.template
index e7b2394..7c0a004 100644
--- a/src/provisioningserver/templates/dhcp/dhcpd6.conf.template
+++ b/src/provisioningserver/templates/dhcp/dhcpd6.conf.template
@@ -2,6 +2,7 @@
 # overwrite any changes made there.  Instead, you can modify dhcpd6.conf by
 # using DHCP snippets over the API or through the web interface.
 
+option dhcp6.user-class code 15 = string;
 option dhcp6.client-arch-type code 61 = array of unsigned integer 16; # RFC5970
 option path-prefix code 210 = text; #RFC5071
 
diff --git a/src/provisioningserver/templates/ipxe/config.commissioning.template b/src/provisioningserver/templates/ipxe/config.commissioning.template
new file mode 120000
index 0000000..44389e4
--- /dev/null
+++ b/src/provisioningserver/templates/ipxe/config.commissioning.template
@@ -0,0 +1 @@
+config.install.template
\ No newline at end of file
diff --git a/src/provisioningserver/templates/ipxe/config.enlist.template b/src/provisioningserver/templates/ipxe/config.enlist.template
new file mode 120000
index 0000000..44389e4
--- /dev/null
+++ b/src/provisioningserver/templates/ipxe/config.enlist.template
@@ -0,0 +1 @@
+config.install.template
\ No newline at end of file
diff --git a/src/provisioningserver/templates/ipxe/config.install.template b/src/provisioningserver/templates/ipxe/config.install.template
new file mode 100644
index 0000000..bf87001
--- /dev/null
+++ b/src/provisioningserver/templates/ipxe/config.install.template
@@ -0,0 +1,5 @@
+#!ipxe
+kernel  {{kernel_params | kernel_path }}
+imgargs {{kernel_params | kernel_name }} {{kernel_params | kernel_command}} BOOTIF=01-${net_default_mac}
+initrd {{kernel_params | initrd_path }}
+boot
\ No newline at end of file
diff --git a/src/provisioningserver/templates/ipxe/config.local.template b/src/provisioningserver/templates/ipxe/config.local.template
new file mode 100644
index 0000000..4fe0777
--- /dev/null
+++ b/src/provisioningserver/templates/ipxe/config.local.template
@@ -0,0 +1,2 @@
+#!ipxe
+exit
diff --git a/src/provisioningserver/templates/ipxe/config.poweroff.template b/src/provisioningserver/templates/ipxe/config.poweroff.template
new file mode 100644
index 0000000..9fa5030
--- /dev/null
+++ b/src/provisioningserver/templates/ipxe/config.poweroff.template
@@ -0,0 +1,3 @@
+#!ipxe
+poweroff
+
diff --git a/src/provisioningserver/templates/ipxe/config.xinstall.template b/src/provisioningserver/templates/ipxe/config.xinstall.template
new file mode 120000
index 0000000..44389e4
--- /dev/null
+++ b/src/provisioningserver/templates/ipxe/config.xinstall.template
@@ -0,0 +1 @@
+config.install.template
\ No newline at end of file
diff --git a/src/provisioningserver/utils/registry.py b/src/provisioningserver/utils/registry.py
index c816368..f9d3ee6 100644
--- a/src/provisioningserver/utils/registry.py
+++ b/src/provisioningserver/utils/registry.py
@@ -7,10 +7,10 @@ __all__ = [
     "Registry",
 ]
 
-from collections import defaultdict
+from collections import defaultdict, OrderedDict
 
 
-_registry = defaultdict(dict)
+_registry = defaultdict(OrderedDict)
 
 
 class RegistryType(type):