Merge ~ltrager/curtin:lp1895067 into curtin:master

[Lee Trager]

Don't install grub if it is already found on CentOS/RHEL

LP: #1895067

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

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

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

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

review: Needs Fixing

[Dimitri John Ledkov (xnox)]

ltrager any further updates on this? or should someone else pick this up?

[Lee Trager (ltrager)]

Sorry for taking so long I was investing another issue I thought was related and got caught up with other things. Updated as suggested and verified.

[Ryan Harper (raharper)]

Thanks, LGTM

review: Approve

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

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

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

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

LGTM. Review comments addressed, logic addresses the bug.

review: Approve

diff --git a/curtin/commands/install_grub.py b/curtin/commands/install_grub.py
index 5f8311f..030087c 100644
--- a/curtin/commands/install_grub.py
+++ b/curtin/commands/install_grub.py
@@ -254,12 +254,19 @@ def gen_uefi_install_commands(grub_name, grub_target, grub_cmd, update_nvram,
         install_cmds.append(['update-grub'])
     elif distroinfo.family == distro.DISTROS.redhat:
         loader = find_efi_loader(target, bootid)
-        if loader and update_nvram:
-            grub_cmd = None  # don't install just add entry
-            efi_disk, efi_part_num = get_efi_disk_part(devices)
-            install_cmds.append(['efibootmgr', '--create', '--write-signature',
-                                 '--label', bootid, '--disk', efi_disk,
-                                 '--part', efi_part_num, '--loader', loader])
+        if loader:
+            # Disable running grub's install command. CentOS/RHEL ships
+            # a pre-built signed grub which installs into /boot. grub2-install
+            # will generated a new unsigned grub which breaks UEFI secure boot.
+            grub_cmd = None
+            if update_nvram:
+                efi_disk, efi_part_num = get_efi_disk_part(devices)
+                # Add entry to the EFI boot menu
+                install_cmds.append(['efibootmgr', '--create',
+                                     '--write-signature', '--label', bootid,
+                                     '--disk', efi_disk,
+                                     '--part', efi_part_num,
+                                     '--loader', loader])
             post_cmds.append(['grub2-mkconfig', '-o',
                               '/boot/efi/EFI/%s/grub.cfg' % bootid])
         else:
diff --git a/tests/unittests/test_commands_install_grub.py b/tests/unittests/test_commands_install_grub.py
index 8808159..b1d132e 100644
--- a/tests/unittests/test_commands_install_grub.py
+++ b/tests/unittests/test_commands_install_grub.py
@@ -774,6 +774,47 @@ class TestGenUefiInstallCommands(CiTestCase):
                 grub_name, grub_target, grub_cmd, update_nvram, distroinfo,
                 devices, self.target))
 
+    def test_redhat_install_existing_no_nvram(self):
+        # verify grub install command is not executed if update_nvram is False
+        # on redhat.
+        def _enable_loaders(bootid):
+            efi_path = 'boot/efi/EFI'
+            target_efi_path = os.path.join(self.target, efi_path)
+            loaders = [
+                os.path.join(target_efi_path, bootid, 'shimx64.efi'),
+                os.path.join(target_efi_path, 'BOOT', 'BOOTX64.EFI'),
+                os.path.join(target_efi_path, bootid, 'grubx64.efi'),
+            ]
+            for loader in loaders:
+                util.write_file(loader, content="")
+
+        self.m_os_release.return_value = {'ID': 'redhat'}
+        distroinfo = install_grub.distro.get_distroinfo()
+        bootid = distroinfo.variant
+        _enable_loaders(bootid)
+        grub_name = 'grub2-efi-x64'
+        grub_target = 'x86_64-efi'
+        grub_cmd = 'grub2-install'
+        update_nvram = False
+        devices = ['/dev/disk-a-part1']
+        disk = '/dev/disk-a'
+        part = '1'
+        self.m_get_disk_part.return_value = (disk, part)
+
+        expected_install = [
+            ['efibootmgr', '-v'],
+        ]
+        expected_post = [
+            ['grub2-mkconfig', '-o', '/boot/efi/EFI/%s/grub.cfg' % bootid],
+            ['efibootmgr', '-v']
+        ]
+
+        self.assertEqual(
+            (expected_install, expected_post),
+            install_grub.gen_uefi_install_commands(
+                grub_name, grub_target, grub_cmd, update_nvram, distroinfo,
+                devices, self.target))
+
 
 class TestGenInstallCommands(CiTestCase):