Merge lp:~blake-rouse/maas/fix-1511437-and-1510120 into lp:~maas-committers/maas/trunk

[Blake Rouse]

Fix UEFI to download grub from archive. Drive-by fix for powerkvm not user the user set ports archive. Fix chainloading grub from disk instead of requiring grubnet to have lvm modules.

[Mike Pontillo (mpontillo)]

Looks good in general; I noticed some minor things you should look at.

review: Approve

[Blake Rouse (blake-rouse)]

Thanks for the quick review.

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~blake-rouse/maas/fix-1511437-and-1510120 into lp:maas failed. Below is the output from the failed tests.

Get:1 http://security.ubuntu.com trusty-security InRelease [64.4 kB]
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Get:2 http://nova.clouds.archive.ubuntu.com trusty-updates InRelease [64.4 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:3 http://security.ubuntu.com trusty-security/main Sources [98.0 kB]
Get:4 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [242 kB]
Get:5 http://security.ubuntu.com trusty-security/universe Sources [31.0 kB]
Get:6 http://security.ubuntu.com trusty-security/main amd64 Packages [357 kB]
Get:7 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [143 kB]
Get:8 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [638 kB]
Get:9 http://security.ubuntu.com trusty-security/universe amd64 Packages [117 kB]
Get:10 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [326 kB]
Get:11 http://security.ubuntu.com trusty-security/main Translation-en [194 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
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Get:12 http://security.ubuntu.com trusty-security/universe Translation-en [68.4 kB]
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Fetched 2,344 kB in 4s (576 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm pep8 phantomjs postgresql pyflakes python-apt python-bson python-bzrlib python-convoy python-coverage 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-extras python-fixtures python-flake8 python-formencode python-hivex python-httplib2 python-jinja2 python-jsonschema python-lxml python-mock python-netaddr python-netifaces python-nose python-oauth python-openssl python-paramiko python-pexpect python-pip python-pocket-lint python-psycopg2 python-pyinotify python-pyparsing python-seamicroclient python-simplejson python-simplestreams python-sphinx python-subunit python-tempita python-testres...

=== modified file 'etc/maas/templates/uefi/config.local.amd64.template'
--- etc/maas/templates/uefi/config.local.amd64.template	2014-05-01 13:43:14 +0000
+++ etc/maas/templates/uefi/config.local.amd64.template	2015-10-29 17:57:28 +0000
@@ -3,6 +3,6 @@
 
 menuentry 'Local' {
     echo 'Booting local disk...'
-    search --set=root --file /efi/ubuntu/grub.cfg
-    configfile /efi/ubuntu/grub.cfg
+    search --set=root --file /efi/ubuntu/shimx64.efi
+    chainloader /efi/ubuntu/shimx64.efi
 }

=== modified file 'src/provisioningserver/boot/powerkvm.py'
--- src/provisioningserver/boot/powerkvm.py	2015-08-14 13:48:59 +0000
+++ src/provisioningserver/boot/powerkvm.py	2015-10-29 17:57:28 +0000
@@ -19,10 +19,12 @@
 import glob
 import os.path
 from textwrap import dedent
+from urlparse import urlparse
 
 from provisioningserver.boot import (
     BootMethod,
     BootMethodInstallError,
+    get_ports_archive_url,
     utils,
 )
 from provisioningserver.boot.install_bootloader import install_bootloader
@@ -61,10 +63,12 @@
         """Installs the required files for PowerKVM/PowerVM booting into the
         tftproot.
         """
+        ports_archive_url = get_ports_archive_url()
+        archive_url = ports_archive_url.strip(urlparse(ports_archive_url).path)
         with tempdir() as tmp:
             # Download the grub-ieee1275-bin package
             data, filename = utils.get_updates_package(
-                'grub-ieee1275-bin', 'http://ports.ubuntu.com',
+                'grub-ieee1275-bin', archive_url,
                 'main', 'ppc64el')
             if data is None:
                 raise BootMethodInstallError(

=== modified file 'src/provisioningserver/boot/tests/test_powerkvm.py'
--- src/provisioningserver/boot/tests/test_powerkvm.py	2015-05-07 18:14:38 +0000
+++ src/provisioningserver/boot/tests/test_powerkvm.py	2015-10-29 17:57:28 +0000
@@ -49,6 +49,7 @@
 
     def test_install_bootloader_get_package_raises_error(self):
         method = PowerKVMBootMethod()
+        self.patch(powerkvm_module, 'get_ports_archive_url')
         self.patch(utils, 'get_updates_package').return_value = (None, None)
         self.assertRaises(
             BootMethodInstallError, method.install_bootloader, None)
@@ -67,6 +68,9 @@
             finally:
                 pass
 
+        mock_get_ports_archive_url = self.patch(
+            powerkvm_module, 'get_ports_archive_url')
+        mock_get_ports_archive_url.return_value = 'http://ports.ubuntu.com'
         mock_get_updates_package = self.patch(utils, 'get_updates_package')
         mock_get_updates_package.return_value = (data, filename)
         self.patch(powerkvm_module, 'call_and_check')

=== modified file 'src/provisioningserver/boot/tests/test_uefi.py'
--- src/provisioningserver/boot/tests/test_uefi.py	2015-06-11 21:21:22 +0000
+++ src/provisioningserver/boot/tests/test_uefi.py	2015-10-29 17:57:28 +0000
@@ -14,11 +14,20 @@
 __metaclass__ = type
 __all__ = []
 
+from contextlib import contextmanager
+import os
 import re
 
 from maastesting.factory import factory
+from maastesting.matchers import MockCallsMatch
 from maastesting.testcase import MAASTestCase
-from provisioningserver.boot import BytesReader
+from mock import call
+from provisioningserver.boot import (
+    BootMethodInstallError,
+    BytesReader,
+    uefi as uefi_module,
+    utils,
+)
 from provisioningserver.boot.tftppath import compose_image_path
 from provisioningserver.boot.uefi import (
     re_config_file,
@@ -116,7 +125,7 @@
                 purpose="local", arch="amd64"),
             }
         output = method.get_reader(**options).read(10000)
-        self.assertIn("configfile /efi/ubuntu/grub.cfg", output)
+        self.assertIn("chainloader /efi/ubuntu/shimx64.efi", output)
 
     def test_get_reader_with_enlist_purpose(self):
         # If purpose is "enlist", the config.enlist.template should be
@@ -246,3 +255,67 @@
         self.assertEqual(
             {'mac': None, 'arch': arch, 'subarch': subarch},
             match.groupdict())
+
+
+class TestUEFIBootMethod(MAASTestCase):
+    """Tests `provisioningserver.boot.uefi.UEFIBootMethod`."""
+
+    def test_install_bootloader_get_package_raises_error(self):
+        method = UEFIBootMethod()
+        self.patch(uefi_module, 'get_main_archive_url')
+        self.patch(utils, 'get_updates_package').return_value = (None, None)
+        self.assertRaises(
+            BootMethodInstallError, method.install_bootloader, None)
+
+    def test_install_bootloader(self):
+        method = UEFIBootMethod()
+        shim_filename = factory.make_name('shim-signed')
+        shim_data = factory.make_string()
+        grub_filename = factory.make_name('grub-efi-amd64-signed')
+        grub_data = factory.make_string()
+        tmp = self.make_dir()
+        dest = self.make_dir()
+
+        @contextmanager
+        def tempdir():
+            try:
+                yield tmp
+            finally:
+                pass
+
+        mock_get_main_archive_url = self.patch(
+            uefi_module, 'get_main_archive_url')
+        mock_get_main_archive_url.return_value = 'http://archive.ubuntu.com'
+        mock_get_updates_package = self.patch(utils, 'get_updates_package')
+        mock_get_updates_package.side_effect = [
+            (shim_data, shim_filename),
+            (grub_data, grub_filename),
+            ]
+        self.patch(uefi_module, 'call_and_check')
+        self.patch(uefi_module, 'tempdir').side_effect = tempdir
+
+        mock_install_bootloader = self.patch(
+            uefi_module, 'install_bootloader')
+
+        method.install_bootloader(dest)
+
+        with open(os.path.join(tmp, shim_filename), 'rb') as stream:
+            saved_shim_data = stream.read()
+        self.assertEqual(shim_data, saved_shim_data)
+
+        with open(os.path.join(tmp, grub_filename), 'rb') as stream:
+            saved_grub_data = stream.read()
+        self.assertEqual(grub_data, saved_grub_data)
+
+        shim_expected = os.path.join(
+            tmp, "usr", "lib", "shim", "shim.efi.signed")
+        shim_dest_expected = os.path.join(dest, method.bootloader_path)
+        grub_expected = os.path.join(
+            tmp, "usr", "lib", "grub", "x86_64-efi-signed",
+            "grubnetx64.efi.signed")
+        grub_dest_expected = os.path.join(dest, "grubx64.efi")
+        self.assertThat(
+            mock_install_bootloader,
+            MockCallsMatch(
+                call(shim_expected, shim_dest_expected),
+                call(grub_expected, grub_dest_expected)))

=== modified file 'src/provisioningserver/boot/uefi.py'
--- src/provisioningserver/boot/uefi.py	2015-08-14 13:48:59 +0000
+++ src/provisioningserver/boot/uefi.py	2015-10-29 17:57:28 +0000
@@ -20,8 +20,6 @@
 import os.path
 import re
 from textwrap import dedent
-import urllib2
-from urlparse import urljoin
 
 from provisioningserver.boot import (
     BootMethod,
@@ -39,8 +37,6 @@
 from provisioningserver.utils.shell import call_and_check
 
 
-ARCHIVE_PATH = "/main/uefi/grub2-amd64/current/grubnetx64.efi.signed"
-
 CONFIG_FILE = dedent("""
     # MAAS GRUB2 pre-loader configuration file
 
@@ -82,40 +78,6 @@
 re_config_file = re.compile(re_config_file, re.VERBOSE)
 
 
-def archive_grubnet_urls(main_url):
-    """Paths to try to download grubnetx64.efi.signed."""
-    release = utils.get_distro_release()
-    # grubnetx64 will not work below version trusty, as efinet is broken
-    # when loading kernel, force trusty. Note: This is only the grub version
-    # this should not block any of the previous release from running.
-    if release in ['lucid', 'precise', 'quantal', 'saucy']:
-        release = 'trusty'
-    if not main_url.endswith('/'):
-        main_url = main_url + '/'
-    dists_url = urljoin(main_url, 'dists')
-    for dist in ['%s-updates' % release, release]:
-        yield "%s/%s/%s" % (
-            dists_url.rstrip("/"),
-            dist,
-            ARCHIVE_PATH.rstrip("/"))
-
-
-def download_grubnet(main_url, destination):
-    """Downloads grubnetx64.efi.signed from the archive."""
-    for url in archive_grubnet_urls(main_url):
-        try:
-            response = urllib2.urlopen(url)
-        # Okay, if it fails as the updates area might not hold
-        # that file.
-        except urllib2.URLError:
-            continue
-
-        with open(destination, 'wb') as stream:
-            stream.write(response.read())
-        return True
-    return False
-
-
 class UEFIBootMethod(BootMethod):
 
     name = "uefi"
@@ -184,14 +146,27 @@
                 os.path.join(tmp, 'usr', 'lib', 'shim', 'shim.efi.signed'),
                 os.path.join(destination, self.bootloader_path))
 
-            # Download grubnetx64 from the archive and install
-            grub_tmp = os.path.join(tmp, 'grubnetx64.efi.signed')
-            if download_grubnet(archive_url, grub_tmp) is False:
+            # Download the grub-efi-amd64-signed package.
+            data, filename = utils.get_updates_package(
+                'grub-efi-amd64-signed', archive_url, 'main', 'amd64')
+            if data is None:
                 raise BootMethodInstallError(
-                    'Failed to download grubnetx64.efi.signed '
-                    'from the archive.')
-            grub_dst = os.path.join(destination, 'grubx64.efi')
-            install_bootloader(grub_tmp, grub_dst)
+                    'Failed to download grub-efi-amd64-signed package from '
+                    'the archive.')
+            grub_output = os.path.join(tmp, filename)
+            with open(grub_output, 'wb') as stream:
+                stream.write(data)
+
+            # Extract the package with dpkg.
+            call_and_check(["dpkg", "-x", grub_output, tmp])
+
+            # Install the grub boot loader.
+            grub_signed = os.path.join(
+                tmp, 'usr', 'lib', 'grub', 'x86_64-efi-signed',
+                'grubnetx64.efi.signed')
+            install_bootloader(
+                grub_signed,
+                os.path.join(destination, 'grubx64.efi'))
 
         config_path = os.path.join(destination, 'grub')
         config_dst = os.path.join(config_path, 'grub.cfg')