Merge ~raharper/curtin:fix/uefi-reorder-missing-boot-current into curtin:master

[Ryan Harper]

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 placing all
network boot entries first followed by the entry curtin installed
and appending all other entries afterward. The feature,
UEFI_REORDER_FALLBACK_SUPPORT, is enabled by default. Users may
disable both 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

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:3e1a8e20c28ed7feadda9714ef6823d2fd5d5ad0
https://jenkins.ubuntu.com/server/job/curtin-ci/161/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/161/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/161/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/161/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/161/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/161//rebuild

review: Approve (continuous-integration)

[Paride Legovini (paride)]

LGTM, the needs-fixing is just for the typo (see inline comment).

In another inline comment I suggest documenting the reason for the BootOrder reordering a bit more, but take it with "importance: nit".

I fixed a typo in the MP commit message s/perform/performance/. It's still present in the commit message for 3e1a8, but that will be squashed by CI.

review: Needs Fixing

[Ryan Harper (raharper)]

Thanks for the review. I've at least one update to push; I'm still working with Rod (you can see where we're at in the bug) to get positive feedback on addressing the issue. We're working on the logic to be reliable for the systems we know don't have BootCurrent.

I'll push an update and move back to Needs Review when we've got something working.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:01f308269818521ff42fd92c09a8af767d42a8a3
https://jenkins.ubuntu.com/server/job/curtin-ci/168/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/168/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/168/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/168/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/168/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/168//rebuild

review: Approve (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:fc95d59be45102e9adb51fbf1ef6e629079e791a
https://jenkins.ubuntu.com/server/job/curtin-ci/169/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/169/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/169/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/169/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/169/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/169//rebuild

review: Approve (continuous-integration)

[Paride Legovini (paride)]

I went through the code and it looks good, I'd just clarify a bit more the reason why we want to keep the current boot entry the first one (see inline comment). Take it as a "nit" comment: up to you to decide if it's worth fixing or not.

Just to be sure: this is the code that Rod validated in LP: #1789650 using your PPA, correct? While the code looks good and CI passes with the new tests it's not immediate to test it in practice.

review: Needs Information

[Paride Legovini (paride)]

Marking as Needs Fixing while waiting for the documentation commits (per IRC conversation).

review: Needs Fixing

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:d5b589e59bc550748d1b93e17b37930c0d524080
https://jenkins.ubuntu.com/server/job/curtin-ci/174/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/174/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/174/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/174/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/174/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/174//rebuild

review: Approve (continuous-integration)

[Paride Legovini (paride)]

Thanks!

review: Approve

[Server Team CI bot (server-team-bot)]

Commit message lints:
- Line #12 has 44 too many characters. Line starts with: "and appending all other"...

review: Needs Fixing

[Server Team CI bot (server-team-bot)]

review: Approve (continuous-integration)

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 51f11d6..92230fc 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..951f07f 100644
--- a/doc/topics/config.rst
+++ b/doc/topics/config.rst
@@ -226,6 +226,35 @@ 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>*
+
+Curtin is typically used with MAAS where the systems are configured to boot
+from the network leaving MAAS in control.  On UEFI systems, after installing
+a bootloader the systems BootOrder may be updated to boot from the new entry.
+This breaks MAAS control over the system as all subsequent reboots of the node
+will no longer boot over the network.  Therefore, if ``reorder_uefi`` is True
+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; for MAAS this will be a network device.
+
+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 update the BootOrder such that
+all Network related entries are placed before the newly installed boot entry and
+all other entries are placed at the end.  This enables the system to network
+boot first and on failure will boot the most recently installed entry.
+
+This setting is ignored if *update_nvram* is False.
+
+**reorder_uefi_force_fallback**: *<boolean: default False>*
+
+The fallback reodering mechanism is only active if BootCurrent is not present
+in the efibootmgr output.  The fallback reordering method may be enabled
+even if BootCurrent is present if *reorder_uefi_force_fallback* is True.
+
+This setting is ignored if *update_nvram* or *reorder_uefi* are False.
+
 
 **Example**::
 
@@ -264,6 +293,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 a6ae86e..ddae280 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