curtin-dev team mailing list archive

[Ryan Harper <rharper@xxxxxxxxxxx>]

Ryan Harper has proposed merging ~raharper/curtin:fix/uefi-reorder-missing-boot-current into curtin:master.

Commit message:
UEFI: Handle missing BootCurrent entry when reordering UEFI entries

Curtin typically reorders UEFI boot entries to place the currently
booted entry at the front of the list after install. In some cases a
system UEFI may not present a BootCurrent entry and curtin will not
perform any reordering leaving a node to boot from the newly
installed entry. For MAAS deployments, this causes issues as MAAS
expects to retain control over the node via PXE booting.

Curtin will attempt to reorder the boot menu when BootCurrent is
missing (or when forced via curtin config option) by detecting
which menu item was added by the curtin install and moving it back
once place in the list. The feature, UEFI_REORDER_FALLBACK_SUPPORT
is enabled by default. Users may disable reordering and fallback
support.

- unittest/helpers.py: Add with_logs support for CiTestCase
- docs: document grub: reorder_uefi and grub:
  reorder_uefi_force_fallback config options.

LP: #1789650

Requested reviews:
  Paride Legovini (paride)
  Server Team CI bot (server-team-bot): continuous-integration
Related bugs:
  Bug #1789650 in curtin: "Servers set to boot from disk after MAAS installation"
  https://bugs.launchpad.net/curtin/+bug/1789650

For more details, see:
https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/387981
-- 
Your team curtin developers is subscribed to branch curtin:master.

diff --git a/Makefile b/Makefile
index 68a3ad3..187132c 100644
--- a/Makefile
+++ b/Makefile
@@ -9,7 +9,7 @@ ifeq ($(COVERAGE), 1)
 endif
 CURTIN_VMTEST_IMAGE_SYNC ?= False
 export CURTIN_VMTEST_IMAGE_SYNC
-noseopts ?= -vv --nologcapture
+noseopts ?= -vv
 pylintopts ?= --rcfile=pylintrc --errors-only
 target_dirs ?= curtin tests tools
 
diff --git a/curtin/__init__.py b/curtin/__init__.py
index ae4ce11..092020b 100644
--- a/curtin/__init__.py
+++ b/curtin/__init__.py
@@ -34,6 +34,8 @@ FEATURES = [
     'APT_CONFIG_V1',
     # has version module
     'HAS_VERSION_MODULE',
+    # uefi_reoder has fallback support if BootCurrent is missing
+    'UEFI_REORDER_FALLBACK_SUPPORT',
 ]
 
 __version__ = "20.1"
diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
index 458eb9d..784188e 100644
--- a/curtin/commands/curthooks.py
+++ b/curtin/commands/curthooks.py
@@ -85,6 +85,8 @@ do_initrd = yes
 link_in_boot = {inboot}
 """
 
+UEFI_BOOT_ENTRY_IS_NETWORK = r'.*(Network|PXE|NIC|Ethernet|lan|IP4|IP6)+.*'
+
 
 def do_apt_config(cfg, target):
     cfg = apt_config.translate_old_apt_features(cfg)
@@ -411,6 +413,7 @@ def install_kernel(cfg, target):
 def uefi_remove_old_loaders(grubcfg, target):
     """Removes the old UEFI loaders from efibootmgr."""
     efi_output = util.get_efibootmgr(target)
+    LOG.debug('UEFI remove old olders efi output:\n%s', efi_output)
     current_uefi_boot = efi_output.get('current', None)
     old_efi_entries = {
         entry: info
@@ -437,18 +440,90 @@ def uefi_remove_old_loaders(grubcfg, target):
                     "should be removed.", info['name'])
 
 
-def uefi_reorder_loaders(grubcfg, target):
+def uefi_boot_entry_is_network(boot_entry_name):
+    """
+    Return boolean if boot entry name looks like a known network entry.
+    """
+    return re.match(UEFI_BOOT_ENTRY_IS_NETWORK,
+                    boot_entry_name, re.IGNORECASE) is not None
+
+
+def _reorder_new_entry(boot_order, efi_output, efi_orig=None, variant=None):
+    """
+    Reorder the EFI boot menu as follows
+
+    1. All PXE/Network boot entries
+    2. The newly installed entry variant (ubuntu/centos)
+    3. The other items in the boot order that are not in [1, 2]
+
+    returns a list of bootnum strings
+    """
+
+    if not boot_order:
+        raise RuntimeError('boot_order is not a list')
+
+    if efi_orig is None:
+        raise RuntimeError('Missing efi_orig boot dictionary')
+
+    if variant is None:
+        variant = ""
+
+    net_boot = []
+    other = []
+    target = []
+
+    LOG.debug("UEFI previous boot order: %s", efi_orig['order'])
+    LOG.debug("UEFI current  boot order: %s", boot_order)
+    new_entries = list(set(boot_order).difference(set(efi_orig['order'])))
+    if new_entries:
+        LOG.debug("UEFI Found new boot entries: %s", new_entries)
+    LOG.debug('UEFI Looking for installed entry variant=%s', variant.lower())
+    for bootnum in boot_order:
+        entry = efi_output['entries'][bootnum]
+        if uefi_boot_entry_is_network(entry['name']):
+            net_boot.append(bootnum)
+        else:
+            if entry['name'].lower() == variant.lower():
+                target.append(bootnum)
+            else:
+                other.append(bootnum)
+
+    if net_boot:
+        LOG.debug("UEFI found netboot entries: %s", net_boot)
+    if other:
+        LOG.debug("UEFI found other entries: %s", other)
+    if target:
+        LOG.debug("UEFI found target entry: %s", target)
+    else:
+        LOG.debug("UEFI Did not find an entry with variant=%s",
+                  variant.lower())
+    new_order = net_boot + target + other
+    if boot_order == new_order:
+        LOG.debug("UEFI Current and Previous bootorders match")
+    return new_order
+
+
+def uefi_reorder_loaders(grubcfg, target, efi_orig=None, variant=None):
     """Reorders the UEFI BootOrder to place BootCurrent first.
 
     The specifically doesn't try to do to much. The order in which grub places
     a new EFI loader is up to grub. This only moves the BootCurrent to the
     front of the BootOrder.
+
+    In some systems, BootCurrent may not be set/present.  In this case
+    curtin will attempt to place the new boot entry created when grub
+    is installed after the the previous first entry (before we installed grub).
+
     """
     if grubcfg.get('reorder_uefi', True):
         efi_output = util.get_efibootmgr(target=target)
+        LOG.debug('UEFI efibootmgr output after install:\n%s', efi_output)
         currently_booted = efi_output.get('current', None)
         boot_order = efi_output.get('order', [])
-        if currently_booted:
+        new_boot_order = None
+        force_fallback_reorder = config.value_as_boolean(
+            grubcfg.get('reorder_uefi_force_fallback', False))
+        if currently_booted and force_fallback_reorder is False:
             if currently_booted in boot_order:
                 boot_order.remove(currently_booted)
             boot_order = [currently_booted] + boot_order
@@ -456,6 +531,23 @@ def uefi_reorder_loaders(grubcfg, target):
             LOG.debug(
                 "Setting currently booted %s as the first "
                 "UEFI loader.", currently_booted)
+        else:
+            reason = (
+                "config 'reorder_uefi_force_fallback' is True" if
+                force_fallback_reorder else "missing 'BootCurrent' value")
+            LOG.debug("Using fallback UEFI reordering: " + reason)
+            if len(boot_order) < 2:
+                LOG.debug(
+                    'UEFI BootOrder has less than 2 entries, cannot reorder')
+                return
+            # look at efi entries before we added one to find the new addition
+            new_order = _reorder_new_entry(
+                    copy.deepcopy(boot_order), efi_output, efi_orig, variant)
+            if new_order != boot_order:
+                new_boot_order = ','.join(new_order)
+            else:
+                LOG.debug("UEFI No changes to boot order.")
+        if new_boot_order:
             LOG.debug(
                 "New UEFI boot order: %s", new_boot_order)
             with util.ChrootableTarget(target) as in_chroot:
@@ -592,7 +684,7 @@ def uefi_find_grub_device_ids(sconfig):
     return grub_device_ids
 
 
-def setup_grub(cfg, target, osfamily=DISTROS.debian):
+def setup_grub(cfg, target, osfamily=DISTROS.debian, variant=None):
     # target is the path to the mounted filesystem
 
     # FIXME: these methods need moving to curtin.block
@@ -692,13 +784,14 @@ def setup_grub(cfg, target, osfamily=DISTROS.debian):
 
     update_nvram = grubcfg.get('update_nvram', True)
     if uefi_bootable and update_nvram:
+        efi_orig_output = util.get_efibootmgr(target)
         uefi_remove_old_loaders(grubcfg, target)
 
     install_grub(instdevs, target, uefi=uefi_bootable, grubcfg=grubcfg)
 
     if uefi_bootable and update_nvram:
+        uefi_reorder_loaders(grubcfg, target, efi_orig_output, variant)
         uefi_remove_duplicate_entries(grubcfg, target)
-        uefi_reorder_loaders(grubcfg, target)
 
 
 def update_initramfs(target=None, all_kernels=False):
@@ -1734,7 +1827,8 @@ def builtin_curthooks(cfg, target, state):
                 name=stack_prefix + '/install-grub',
                 reporting_enabled=True, level="INFO",
                 description="installing grub to target devices"):
-            setup_grub(cfg, target, osfamily=osfamily)
+            setup_grub(cfg, target, osfamily=osfamily,
+                       variant=distro_info.variant)
 
 
 def curthooks(args):
diff --git a/doc/topics/config.rst b/doc/topics/config.rst
index 7f8396e..5ce1a4b 100644
--- a/doc/topics/config.rst
+++ b/doc/topics/config.rst
@@ -226,6 +226,27 @@ not provided, Curtin will set the value to 'console'.  If the ``terminal``
 value is 'unmodified' then Curtin will not set any value at all and will
 use Grub defaults.
 
+**reorder_uefi**: *<boolean: default True>*
+
+On UEFI systems curtin will modify the UEFI BootOrder settings to place
+the currently booted entry (BootCurrent) to the first option after installing
+the new target OS into the UEFI boot menu.  The result is that the system
+will boot from the same device that it booted to run curtin.
+
+This setting is ignored if *update_nvram* is False.
+
+
+**reorder_uefi_force_fallback**: *<boolean: default False>*
+
+On some UEFI systems the BootCurrent entry may not be present.  This can
+cause a system to not boot to the same device that it was previously booting.
+If BootCurrent is not present, curtin will attempt to move the newly added
+boot entry after the previous first entry in BootOrder.  If
+*reorder_uefi_force_fallback* is True, curtin will ignore BootCurrent value and
+use the fallback reordering method.
+
+This setting is ignored if *update_nvram* or *reorder_uefi* are False.
+
 
 **Example**::
 
@@ -264,6 +285,12 @@ use Grub defaults.
      probe_additional_os: True
      terminal: unmodified
 
+**Enable Fallback UEFI Reordering**::
+
+  grub:
+     reorder_uefi: true
+     reorder_uefi_force_fallback: true
+
 
 http_proxy
 ~~~~~~~~~~
diff --git a/tests/unittests/helpers.py b/tests/unittests/helpers.py
index 2f5e51a..64a79ca 100644
--- a/tests/unittests/helpers.py
+++ b/tests/unittests/helpers.py
@@ -2,11 +2,13 @@
 
 import imp
 import importlib
+import logging
 import mock
 import os
 import random
 import shutil
 import string
+import sys
 import tempfile
 from unittest import TestCase, skipIf
 from contextlib import contextmanager
@@ -55,6 +57,7 @@ def skipUnlessJsonSchema():
 class CiTestCase(TestCase):
     """Common testing class which all curtin unit tests subclass."""
 
+    with_logs = False
     allowed_subp = False
     SUBP_SHELL_TRUE = "shell=True"
 
@@ -69,6 +72,21 @@ class CiTestCase(TestCase):
 
     def setUp(self):
         super(CiTestCase, self).setUp()
+        if self.with_logs:
+            # Create a log handler so unit tests can search expected logs.
+            self.logger = logging.getLogger()
+            if sys.version_info[0] == 2:
+                import StringIO
+                self.logs = StringIO.StringIO()
+            else:
+                import io
+                self.logs = io.StringIO()
+            formatter = logging.Formatter('%(levelname)s: %(message)s')
+            handler = logging.StreamHandler(self.logs)
+            handler.setFormatter(formatter)
+            self.old_handlers = self.logger.handlers
+            self.logger.handlers = [handler]
+
         if self.allowed_subp is True:
             util.subp = _real_subp
         else:
diff --git a/tests/unittests/test_curthooks.py b/tests/unittests/test_curthooks.py
index 2349456..d88346f 100644
--- a/tests/unittests/test_curthooks.py
+++ b/tests/unittests/test_curthooks.py
@@ -1,5 +1,6 @@
 # This file is part of curtin. See LICENSE file for copyright and license info.
 
+import copy
 import os
 from mock import call, patch
 import textwrap
@@ -10,7 +11,7 @@ from curtin import distro
 from curtin import util
 from curtin import config
 from curtin.reporter import events
-from .helpers import CiTestCase, dir2dict, populate_dir
+from .helpers import CiTestCase, dir2dict, populate_dir, random
 
 
 class TestGetFlashKernelPkgs(CiTestCase):
@@ -531,12 +532,55 @@ class TestSetupZipl(CiTestCase):
             content)
 
 
+class EfiOutput(object):
+
+    def __init__(self, current=None, order=None, entries=None):
+        self.entries = {}
+        if entries:
+            for entry in entries:
+                self.entries.update(entry)
+        self.current = current
+        self.order = order
+        if not order and self.entries:
+            self.order = sorted(self.entries.keys())
+
+    def add_entry(self, bootnum=None, name=None, path=None, current=False):
+        if not bootnum:
+            bootnum = "%04x" % random.randint(0, 1000)
+        if not name:
+            name = CiTestCase.random_string()
+        if not path:
+            path = ''
+        if bootnum not in self.entries:
+            self.entries[bootnum] = {'name': name, 'path': path}
+            if not self.order:
+                self.order = []
+            self.order.append(bootnum)
+        if current:
+            self.current = bootnum
+
+    def set_order(self, new_order):
+        self.order = new_order
+
+    def as_dict(self):
+        output = {}
+        if self.current:
+            output['current'] = self.current
+        if self.order:
+            output['order'] = self.order
+        output['entries'] = self.entries
+        return output
+
+
 class TestSetupGrub(CiTestCase):
 
+    with_logs = True
+
     def setUp(self):
         super(TestSetupGrub, self).setUp()
         self.target = self.tmp_dir()
         self.distro_family = distro.DISTROS.debian
+        self.variant = 'ubuntu'
         self.add_patch('curtin.distro.lsb_release', 'mock_lsb_release')
         self.mock_lsb_release.return_value = {'codename': 'xenial'}
         self.add_patch('curtin.util.is_uefi_bootable',
@@ -556,7 +600,8 @@ class TestSetupGrub(CiTestCase):
         updated_cfg = {
             'install_devices': ['/dev/vdb']
         }
-        curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family)
+        curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family,
+                             variant=self.variant)
         self.m_install_grub.assert_called_with(
             ['/dev/vdb'], self.target, uefi=False, grubcfg=updated_cfg)
 
@@ -588,7 +633,8 @@ class TestSetupGrub(CiTestCase):
             },
         }
         m_exists.return_value = True
-        curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family)
+        curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family,
+                             variant=self.variant)
         self.m_install_grub.assert_called_with(
             ['/dev/vdb'], self.target, uefi=False,
             grubcfg={'install_devices': ['/dev/vdb']})
@@ -638,7 +684,8 @@ class TestSetupGrub(CiTestCase):
         }
         m_exists.return_value = True
         m_is_valid_device.side_effect = (False, True, False, True)
-        curthooks.setup_grub(cfg, self.target, osfamily=distro.DISTROS.redhat)
+        curthooks.setup_grub(cfg, self.target, osfamily=distro.DISTROS.redhat,
+                             variant='centos')
         self.m_install_grub.assert_called_with(
             ['/dev/vdb1'], self.target, uefi=True,
             grubcfg={'update_nvram': False, 'install_devices': ['/dev/vdb1']}
@@ -650,7 +697,8 @@ class TestSetupGrub(CiTestCase):
                 'install_devices': None,
             },
         }
-        curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family)
+        curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family,
+                             variant=self.variant)
         self.m_install_grub.assert_called_with(
             ['none'], self.target, uefi=False,
             grubcfg={'install_devices': None}
@@ -681,7 +729,8 @@ class TestSetupGrub(CiTestCase):
                 }
             }
         }
-        curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family)
+        curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family,
+                             variant=self.variant)
         self.m_install_grub.assert_called_with(
             ['/dev/vdb'], self.target, uefi=True, grubcfg=cfg.get('grub')
         )
@@ -721,7 +770,8 @@ class TestSetupGrub(CiTestCase):
             }
         }
         self.mock_haspkg.return_value = False
-        curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family)
+        curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family,
+                             variant=self.variant)
 
         expected_calls = [
             call(['efibootmgr', '-B', '-b', '0001'],
@@ -762,10 +812,217 @@ class TestSetupGrub(CiTestCase):
             }
         }
         self.mock_haspkg.return_value = False
-        curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family)
+        curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family,
+                             variant=self.variant)
+        self.assertEquals([
+            call(['efibootmgr', '-o', '0001,0000'], target=self.target)],
+            self.mock_subp.call_args_list)
+
+    @patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a)
+    def test_grub_install_uefi_reorders_no_current_new_entry(self):
+        self.add_patch('curtin.distro.install_packages', 'mock_install')
+        self.add_patch('curtin.distro.has_pkg_available', 'mock_haspkg')
+        self.add_patch('curtin.util.get_efibootmgr', 'mock_efibootmgr')
+        self.mock_is_uefi_bootable.return_value = True
+        cfg = {
+            'grub': {
+                'install_devices': ['/dev/vdb'],
+                'update_nvram': True,
+                'remove_old_uefi_loaders': False,
+                'reorder_uefi': True,
+            },
+        }
+
+        # Single existing entry 0001
+        efi_orig = EfiOutput()
+        efi_orig.add_entry(bootnum='0001', name='centos')
+
+        # After install add a second entry, 0000 to the front of order
+        efi_post = copy.deepcopy(efi_orig)
+        efi_post.add_entry(bootnum='0000', name='ubuntu')
+        efi_post.set_order(['0000', '0001'])
+
+        # After reorder we should have the target install first
+        efi_final = copy.deepcopy(efi_post)
+
+        self.mock_efibootmgr.side_effect = iter([
+            efi_orig.as_dict(),   # collect original order before install
+            efi_orig.as_dict(),   # remove_old_loaders query (no change)
+            efi_post.as_dict(),   # efi table after grub install, (changed)
+            efi_final.as_dict(),  # remove duplicates checks and finds reorder
+                                  # has changed
+        ])
+        self.mock_haspkg.return_value = False
+        curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family,
+                             variant=self.variant)
+        logs = self.logs.getvalue()
+        print(logs)
+        self.assertEquals([], self.mock_subp.call_args_list)
+        self.assertIn("Using fallback UEFI reordering:", logs)
+        self.assertIn("missing 'BootCurrent' value", logs)
+        self.assertIn("Found new boot entries: ['0000']", logs)
+
+    @patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a)
+    def test_grub_install_uefi_reorders_no_curr_same_size_order_no_match(self):
+        self.add_patch('curtin.distro.install_packages', 'mock_install')
+        self.add_patch('curtin.distro.has_pkg_available', 'mock_haspkg')
+        self.add_patch('curtin.util.get_efibootmgr', 'mock_efibootmgr')
+        self.add_patch('curtin.commands.curthooks.uefi_remove_old_loaders',
+                       'mock_remove_old_loaders')
+        self.mock_is_uefi_bootable.return_value = True
+        cfg = {
+            'grub': {
+                'install_devices': ['/dev/vdb'],
+                'update_nvram': True,
+                'remove_old_uefi_loaders': False,
+                'reorder_uefi': True,
+            },
+        }
+
+        # Existing Custom Ubuntu, usb and cd/dvd entry, booting Ubuntu
+        efi_orig = EfiOutput()
+        efi_orig.add_entry(bootnum='0001', name='Ubuntu Deluxe Edition')
+        efi_orig.add_entry(bootnum='0002', name='USB Device')
+        efi_orig.add_entry(bootnum='0000', name='CD/DVD')
+        efi_orig.set_order(['0001', '0002', '0000'])
+
+        # after install existing ubuntu entry is reused, no change in order
+        efi_post = efi_orig
+
+        # after reorder, no change is made due to the installed distro variant
+        # string 'ubuntu' is not found in the boot entries so we retain the
+        # original efi order.
+        efi_final = efi_post
+
+        self.mock_efibootmgr.side_effect = iter([
+            efi_orig.as_dict(),   # collect original order before install
+            efi_orig.as_dict(),   # remove_old_loaders query
+            efi_post.as_dict(),   # reorder entries queries post install
+            efi_final.as_dict(),  # remove duplicates checks and finds reorder
+        ])
+
+        self.mock_haspkg.return_value = False
+        curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family,
+                             variant=self.variant)
+
+        logs = self.logs.getvalue()
+        print(logs)
+        self.assertEquals([], self.mock_subp.call_args_list)
+        self.assertIn("Using fallback UEFI reordering:", logs)
+        self.assertIn("missing 'BootCurrent' value", logs)
+        self.assertIn("Current and Previous bootorders match", logs)
+        self.assertIn("Looking for installed entry variant=", logs)
+        self.assertIn("Did not find an entry with variant=", logs)
+        self.assertIn("No changes to boot order.", logs)
+
+    @patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a)
+    def test_grub_install_uefi_reorders_force_fallback(self):
+        self.add_patch('curtin.distro.install_packages', 'mock_install')
+        self.add_patch('curtin.distro.has_pkg_available', 'mock_haspkg')
+        self.add_patch('curtin.util.get_efibootmgr', 'mock_efibootmgr')
+        self.mock_is_uefi_bootable.return_value = True
+        cfg = {
+            'grub': {
+                'install_devices': ['/dev/vdb'],
+                'update_nvram': True,
+                'remove_old_uefi_loaders': True,
+                'reorder_uefi': True,
+                'reorder_uefi_force_fallback': True,
+            },
+        }
+        # Single existing entry 0001 and set as current, which should avoid
+        # any fallback logic, but we're forcing fallback pack via config
+        efi_orig = EfiOutput()
+        efi_orig.add_entry(bootnum='0001', name='PXE', current=True)
+        print(efi_orig.as_dict())
+
+        # After install add a second entry, 0000 to the front of order
+        efi_post = copy.deepcopy(efi_orig)
+        efi_post.add_entry(bootnum='0000', name='ubuntu')
+        efi_post.set_order(['0000', '0001'])
+        print(efi_orig.as_dict())
+
+        # After reorder we should have the original boot entry 0001 as first
+        efi_final = copy.deepcopy(efi_post)
+        efi_final.set_order(['0001', '0000'])
+
+        self.mock_efibootmgr.side_effect = iter([
+            efi_orig.as_dict(),   # collect original order before install
+            efi_orig.as_dict(),   # remove_old_loaders query (no change)
+            efi_post.as_dict(),   # efi table after grub install, (changed)
+            efi_final.as_dict(),  # remove duplicates checks and finds reorder
+                                  # has changed
+        ])
+
+        self.mock_haspkg.return_value = False
+        curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family,
+                             variant=self.variant)
+        logs = self.logs.getvalue()
+        print(logs)
         self.assertEquals([
             call(['efibootmgr', '-o', '0001,0000'], target=self.target)],
             self.mock_subp.call_args_list)
+        self.assertIn("Using fallback UEFI reordering:", logs)
+        self.assertIn("config 'reorder_uefi_force_fallback' is True", logs)
+
+    @patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a)
+    def test_grub_install_uefi_reorders_network_first(self):
+        self.add_patch('curtin.distro.install_packages', 'mock_install')
+        self.add_patch('curtin.distro.has_pkg_available', 'mock_haspkg')
+        self.add_patch('curtin.util.get_efibootmgr', 'mock_efibootmgr')
+        self.mock_is_uefi_bootable.return_value = True
+        cfg = {
+            'grub': {
+                'install_devices': ['/dev/vdb'],
+                'update_nvram': True,
+                'remove_old_uefi_loaders': True,
+                'reorder_uefi': True,
+            },
+        }
+
+        # Existing ubuntu, usb and cd/dvd entry, booting ubuntu
+        efi_orig = EfiOutput()
+        efi_orig.add_entry(bootnum='0001', name='centos')
+        efi_orig.add_entry(bootnum='0002', name='Network')
+        efi_orig.add_entry(bootnum='0003', name='PXE')
+        efi_orig.add_entry(bootnum='0004', name='LAN')
+        efi_orig.add_entry(bootnum='0000', name='CD/DVD')
+        efi_orig.set_order(['0001', '0002', '0003', '0004', '0000'])
+        print(efi_orig.as_dict())
+
+        # after install we add an ubuntu entry, and grub puts it first
+        efi_post = copy.deepcopy(efi_orig)
+        efi_post.add_entry(bootnum='0007', name='ubuntu')
+        efi_post.set_order(['0007'] + efi_orig.order)
+        print(efi_post.as_dict())
+
+        # reorder must place all network devices first, then ubuntu, and others
+        efi_final = copy.deepcopy(efi_post)
+        expected_order = ['0002', '0003', '0004', '0007', '0001', '0000']
+        efi_final.set_order(expected_order)
+
+        self.mock_efibootmgr.side_effect = iter([
+            efi_orig.as_dict(),   # collect original order before install
+            efi_orig.as_dict(),   # remove_old_loaders query
+            efi_post.as_dict(),   # reorder entries queries post install
+            efi_final.as_dict(),  # remove duplicates checks and finds reorder
+        ])
+        self.mock_haspkg.return_value = False
+        curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family,
+                             variant=self.variant)
+        logs = self.logs.getvalue()
+        print(logs)
+        print('Number of bootmgr calls: %s' % self.mock_efibootmgr.call_count)
+        self.assertEquals([
+            call(['efibootmgr', '-o', '%s' % (",".join(expected_order))],
+                 target=self.target)],
+            self.mock_subp.call_args_list)
+        self.assertIn("Using fallback UEFI reordering:", logs)
+        self.assertIn("missing 'BootCurrent' value", logs)
+        self.assertIn("Looking for installed entry variant=", logs)
+        self.assertIn("found netboot entries: ['0002', '0003', '0004']", logs)
+        self.assertIn("found other entries: ['0001', '0000']", logs)
+        self.assertIn("found target entry: ['0007']", logs)
 
 
 class TestUefiRemoveDuplicateEntries(CiTestCase):
diff --git a/tests/unittests/test_feature.py b/tests/unittests/test_feature.py
index 84325ef..8690ad8 100644
--- a/tests/unittests/test_feature.py
+++ b/tests/unittests/test_feature.py
@@ -27,4 +27,7 @@ class TestExportsFeatures(CiTestCase):
     def test_has_btrfs_swapfile_support(self):
         self.assertIn('BTRFS_SWAPFILE', curtin.FEATURES)
 
+    def test_has_uefi_reorder_fallback_support(self):
+        self.assertIn('UEFI_REORDER_FALLBACK_SUPPORT', curtin.FEATURES)
+
 # vi: ts=4 expandtab syntax=python